uplift: add database package from evm repositories
#4331
Conversation
JonathanOppenheimer
requested a review
from StephenButtolph
as a code owner
There was a problem hiding this comment.
Pull Request Overview
This PR uplifts the database package from EVM repositories (subnet-evm and coreth) to AvalancheGo to eliminate code duplication and centralize maintenance. This is part of a team-wide effort to consolidate shared functionality between repositories.
Key changes:
- Adds a new
wrapped_database.gofile containing database wrapper implementations - Implements ethdb.KeyValueStore interface wrapper for avalanchego database types
- Provides batch operations wrapper for database operations
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: rodrigo <77309055+RodrigoVillar@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
|
Thanks for the suggestions rodrigo :) |
JonathanOppenheimer
changed the title
database package from evm repositoriesdatabase package from evm repositories
vms/evm/database/wrapped_database.go
Outdated
|
|
||
| func WrapDatabase(db database.Database) ethdb.KeyValueStore { return ethDBWrapper{db} } | ||
|
|
||
| type ethDBWrapper struct{ database.Database } |
There was a problem hiding this comment.
- nit: I would just call this
databaseordbor something. It's implied that this is an eth-compatible db because we're in theevmpackage. It's also obvious from looking at the struct definition that this is a wrapper pattern. - avoid embed if we can, see later comment
There was a problem hiding this comment.
- We can't do
database(it's the import) ordbbc of the intervening var definition. I'd say we doethdb, but that's also an import. Any other suggestions? - done
There was a problem hiding this comment.
You can alias imports if there's a namespace collision (you could call database avalanchegodatabase or avalanchegodb or something)
vms/evm/database/wrapped_database.go
Outdated
|
|
||
| // TODO: propagate size through avalanchego Database interface | ||
| func (db ethDBWrapper) NewBatchWithSize(int) ethdb.Batch { | ||
| return ethBatchWrapper{db.Database.NewBatch()} |
There was a problem hiding this comment.
We could just return db.NewBatch() here
There was a problem hiding this comment.
If we do that, then we does not implement ethdb.KeyValueStore:
have NewBatch() "github.com/ava-labs/avalanchego/database".Batch
want NewBatch() ethdb.Batch
Why this should be merged
There are packages in subnet-evm and coreth that share a lot of functionality / are mirrored. As has been discussed as an evm team wide effort, we are working to uplift these packages to AvalancheGo to prevent the duplicative maintenance of two sets of the same code. This particular package,
databaseis a relatively easy lift.How this was tested
This is currently 'dead' code; after a new AvalancheGo release is dropped, we can use the code in the evm repositories.
Need to be documented in RELEASES.md?
No