refactor: uplift combined warp package from evm repositories #4156
refactor: uplift combined warp package from evm repositories #4156JonathanOppenheimer wants to merge 72 commits intomasterfrom
warp package from evm repositories #4156Conversation
|
This PR has become stale because it has been open for 30 days with no activity. Adding the |
warp package from evm repositories
warp package from evm repositories warp package from evm repositories
|
FYI: I plan to review this PR thoroughly myself first, i.e. leave comments etc. and clean it up before seeking external review. This PR is finally passing CI now though! |
vms/evm/warp/backend.go
Outdated
| return fmt.Errorf("failed to put warp signature in db: %w", err) | ||
| } | ||
|
|
||
| if _, err := b.signMessage(unsignedMessage); err != nil { |
There was a problem hiding this comment.
what happens if we put the message in the database but for whatever reason fail to sign the message and put it in the cache (I actually do not know)
This is no longer blocked and ready for review. |
graft/coreth/plugin/evm/vm.go
Outdated
| // Avalanche Warp Messaging components | ||
| // Used to serve BLS signatures of warp messages over RPC | ||
| warpBackend warp.Backend | ||
| warpMsgDB *warp.DB |
There was a problem hiding this comment.
When naming, a general guideline is to avoid having multiple names for the same logical concept unless we need to disambiguate - since this is a warp.DB we should call this warpDB rather than a warpMsgDB because there aren't other warp.DB's in this context.
If we find that consumers are frequently referring to a type in a common way it might suggest that the type needs renaming.
There was a problem hiding this comment.
There is already a variable called warpDB declared in the VM struct:
avalanchego/graft/coreth/plugin/evm/vm.go
Lines 218 to 220 in 5213f1c
I assume Msg was added to differentiate these two databases.
warp package from evm repositories warp package from evm repositories
|
Marking as frozen -- this is no longer a current priority. |
Putting this in the backlog until we revisit it. |
Why this should be merged
There are packages in subnet-evm and coreth that share a lot of functionality / are mirrored. These packages should be uplifted to AvalancheGo to prevent the duplicative maintenance of two sets of the same code. This is part of that effort. This would close #4129 .
How this works
This code represents the 'merged' difference of
coreth/warpandsubnet-evm/warp.The two packages diverged significantly, and deference was given to additions.Key merges:
snowtest.Context()whensnowctxis needed (defer tocoreth)ValidatorReaderis needed, imported it fromwarptestverifier_stats.gofromsubnet-evmverifier_backend.gofromsubnet-evmbackend_test.gofromsubnet-evmNote:
Linting changes can be viewed standalone in 017a635
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