refactor: Share EVM Utils #4739
Conversation
alarso16
force-pushed
the
alarso16/shared-utils
branch
from
408c0e7 to
b6b6e19
Compare
alarso16
force-pushed
the
alarso16/shared-utils
branch
from
b6b6e19 to
9fa2894
Compare
JonathanOppenheimer
added
evm
There was a problem hiding this comment.
Pull request overview
This PR consolidates EVM utility functions and modules that were duplicated across subnet-evm and coreth into a shared graft/evm package. The main goal is to reduce code duplication and improve maintainability by having a single source of truth for common EVM utilities.
Key changes:
- Moved common utility functions (numbers, bytes, denomination, etc.) from individual EVM implementations to
graft/evm/utils - Consolidated test utilities, particularly snow context helpers, into
graft/evm/utils/utilstest - Updated all import paths across both
subnet-evmandcorethto reference the shared utilities
Reviewed changes
Copilot reviewed 144 out of 161 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| graft/evm/utils/* | New shared utility packages containing consolidated functions previously duplicated in subnet-evm and coreth |
| graft/subnet-evm/utils/* | Removed files that have been moved to shared location |
| graft/coreth/utils/* | Removed files that have been moved to shared location |
| graft/subnet-evm/**/*.go | Updated imports to reference shared utils package |
| graft/coreth/**/*.go | Updated imports to reference shared utils package |
| graft/*/go.mod | Updated module dependencies to include shared evm package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JonathanOppenheimer
left a comment
JonathanOppenheimer
left a comment
There was a problem hiding this comment.
Some of the copilot reviews seem valid, and just one question
JonathanOppenheimer
left a comment
JonathanOppenheimer
left a comment
There was a problem hiding this comment.
I think updating the function signature for all the old uses is fitting for this (as the imports are changing), but changes are solid nonethless.
| dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= | ||
| github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8= | ||
| github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= | ||
| github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg= |
There was a problem hiding this comment.
Why is this downgraded in this PR?
| github.com/gopherjs/gopherjs v1.17.2/go.mod h1:pRRIvn/QzFLrKfvEz3qUuEhtE/zLCWfreZ6J5gM2i+k= | ||
| github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= | ||
| github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= | ||
| github.com/gorilla/rpc v1.2.0 h1:WvvdC2lNeT1SP32zrIce5l0ECBfbAlmrmSBsuc57wfk= |
There was a problem hiding this comment.
upgrade seems less harmful, but also why is this in this PR?
alarso16
force-pushed
the
alarso16/shared-utils
branch
from
3fab297 to
c0b7926
Compare
Why this should be merged
The EVM utils are mostly identical, and could simply be shared
How this works
Moves and consolidates differences between the utils packages.
How this was tested
CI
Need to be documented in RELEASES.md?
No