Skip to content

Add firewood wrapper #4661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joshua-kim wants to merge 3 commits into
base: x-chain-merkle-db-shared-memory
Choose a base branch
from
Open

Add firewood wrapper #4661

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37a820e
add firewood wrapper
joshua-kim Dec 4, 2025
0f55be0
nit
joshua-kim Dec 4, 2025
b3857f9
nit
joshua-kim Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
nit 
Signed-off-by: Joshua Kim <[email protected]>
  • Loading branch information
@joshua-kim
joshua-kim committed Dec 4, 2025
commit 0f55be0ee7fb63c42f505859a830949511171cd4
12 changes: 6 additions & 6 deletions firewood/firewood.go
View file
Open in desktop
Copy link
Contributor

@alarso16 alarso16 Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This location is weird to me, for a lot of reasons.

  1. I think Firewood is eventually joining the monorepo, and this is definitely the logical spot to put it
  2. This won't be the only firewood implementation (since there's also the C-Chain). Should that go here as well? I would expect not, but maybe.
  3. There's a whole database/ folder, which seems to currently be only key-value databases. Either way, this does seem like a database

Copy link
Contributor Author

@joshua-kim joshua-kim Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it here because I wasn't sure what I wanted to do with this... we need to figure out what to do before merging because this is in an admittedly weird spot right now. Some thoughts:

  1. If firewood is joining the monorepo then this needs to move
  2. database seems like a good spot to put this at first, but the problem is that this type doesn't implement the database interface. We could implement the interface if we wanted to... but it would also mean that we can do wrapping with the other database types (like prefixdb) and I don't think we want to use those wrappers (e.g prefixdb hashes prefixes, leading to unpredictable prefixes). Against my point however - we are using database.ErrNotFound to avoid unintended behavior changes which makes it seem like we are a database.Database - so we need to pick a lane.

I need to review the EVM wrapper over firewood to see if there's a way we can share test coverage + code and do some thinking over where this lives and its relationship to database.

Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ func New(path string) (*DB, error) {
}, nil
}

// Get returns a key value pair or database.ErrNotFound if `key` is not in the
// DB.
// Get returns a key value pair or [database.ErrNotFound] if `key` is not in the
// [DB].
func (db *DB) Get(key []byte) ([]byte, error) {
key = Prefix(appPrefix, key)

Expand All @@ -116,7 +116,7 @@ func (db *DB) Get(key []byte) ([]byte, error) {
return val, err
}

// Put inserts a key value pair into DB.
// Put inserts a key value pair into [DB].
func (db *DB) Put(key []byte, val []byte) {
db.pending.Put(Prefix(appPrefix, key), val)
}
Expand All @@ -125,7 +125,7 @@ func (db *DB) Delete(key []byte) {
db.pending.Delete(Prefix(appPrefix, key))
}

// Height returns the last height of DB written to by Flush.
// Height returns the last height of [DB] written to by [DB.Flush].
//
// If this returns false, the height has not been initialized yet.
func (db *DB) Height() (uint64, bool) {
Expand All @@ -151,7 +151,7 @@ func (db *DB) Abort() {
db.pending = changes{}
}

// Flush flushes pending writes to disk and increments Height.
// Flush flushes pending writes to disk and increments [DB.Height].
func (db *DB) Flush() error {
if !db.heightInitialized {
db.heightInitialized = true
Expand Down Expand Up @@ -179,7 +179,7 @@ func (db *DB) Close(ctx context.Context) error {
return db.db.Close(ctx)
}

// Prefix prefixes `key` with `prefix` + PrefixDelimiter.
// Prefix prefixes `key` with `prefix` + [PrefixDelimiter].
func Prefix(prefix []byte, key []byte) []byte {
k := make([]byte, len(prefix)+1+len(key))

Expand Down
Loading