refactor: Share Firewood for EVM #4696
Conversation
alarso16
added
cleanup
alarso16
force-pushed
the
alarso16/shared-firewood
branch
from
6b31642 to
9ab772f
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the Firewood implementation to be shared between coreth and subnet-evm, eliminating code duplication. The implementation is moved to graft/evm/firewood as a common package, with both coreth and subnet-evm now importing from this shared location.
Key changes:
- Moved Firewood implementation from individual projects to a shared package
- Renamed
Databasetype toTrieDBto avoid naming conflicts - Made trie types (
accountTrie,storageTrie) unexported and added wrapper functions for state management
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| graft/evm/firewood/triedb.go | Renamed Database to TrieDB throughout the implementation |
| graft/evm/firewood/account_trie.go | Made AccountTrie unexported as accountTrie with constructor newAccountTrie |
| graft/evm/firewood/storage_trie.go | Made StorageTrie unexported as storageTrie with constructor newStorageTrie |
| graft/evm/firewood/state.go | Added new NewStateWrapper function to create firewood state database wrapper |
| graft/evm/firewood/hash_test.go | Updated test to use shared implementation and removed duplicate test code |
| graft/evm/go.mod | Added necessary dependencies for the shared firewood package |
| graft/subnet-evm/* | Updated imports to reference shared graft/evm/firewood package |
| graft/coreth/* | Updated imports to reference shared graft/evm/firewood package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type AccountTrie struct { | ||
| fw *Database | ||
| type accountTrie struct { | ||
| fw *TrieDB |
There was a problem hiding this comment.
excuse my ignorance, what motivated this change?
There was a problem hiding this comment.
Clarity between state.Database and triedb.DBOverride
JonathanOppenheimer
left a comment
JonathanOppenheimer
left a comment
There was a problem hiding this comment.
There are no functional changes in this PR. Files are moved around, and imports adjusted accordingly. Duplicated code is deleted wholesale.
I am in favor of consolidating all the firewood code into a single place, but not sure if others agree.
Why this should be merged
We should only have one Firewood implementation for coreth and subnet-evm, since all behavior is the same between them. This shares with minimal refactoring to ensure that types don't clash.
How this works
A move and import changes
How this was tested
CI
Need to be documented in RELEASES.md?
No