fix(eth): fix state hooks in API #30830#2066
fix(eth): fix state hooks in API #30830#2066AnilChinchawale merged 1 commit intoXinFinOrg:dev-upgradefrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This pull request refactors the EVM initialization pattern to fix state hooks in the API. The core change is modifying the vm.NewEVM constructor to no longer accept a TxContext parameter, instead introducing a SetTxContext method to update the transaction context separately. This allows the EVM to be created once per block with a hooked state database and reused across multiple transactions within the same block, ensuring that state change hooks (like OnBalanceChange, OnNonceChange, and OnStorageChange) are properly triggered.
Changes:
- Refactored
vm.NewEVMto accept only block context and state database, removing the transaction context parameter - Added
evm.SetTxContext()method to update transaction context separately after EVM creation - Updated all EVM instantiation sites across the codebase to use the new two-step pattern: create EVM with block context, then call
SetTxContextbefore transaction execution - Moved balance setting operations from
eth/api_backend.gotointernal/ethapi/api.gowhere they are used
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| core/vm/evm.go | Refactored NewEVM signature to remove TxContext parameter; added SetTxContext and SetTracer methods with improved documentation |
| core/state_processor.go | Updated to create EVM once per block with tracingStateDB and reuse it across transactions; simplified ApplyTransaction signature |
| miner/worker.go | Moved EVM creation to Work struct initialization for reuse; removed redundant EVM creation in commitTransaction |
| internal/ethapi/api.go | Moved SetBalance call into applyMessage; updated to call SetTxContext after EVM creation |
| internal/ethapi/backend.go | Simplified GetEVM signature to remove Message parameter; moved balance setting to caller |
| eth/api_backend.go | Removed unused imports; updated GetEVM implementation to match new signature |
| internal/ethapi/simulate.go | Updated to use tracingStateDB pattern and call SetTxContext |
| eth/tracers/api.go | Updated tracing code to properly create hooked state and call SetTxContext |
| eth/state_accessor.go | Refactored to create EVM once and reuse across transactions when replaying block state |
| eth/gasestimator/gasestimator.go | Updated to call SetTxContext after EVM creation |
| core/vm/runtime/env.go | Updated to call SetTxContext after EVM creation |
| tests/*.go | Updated all test files to use new EVM initialization pattern |
| eth/tracers/*_test.go | Updated tracer tests to use new pattern; added TestStateHooks to verify state hooks work correctly |
| core/state_processor_test.go | Added TestApplyTransactionWithEVMStateChangeHooks to verify state hooks are invoked |
| consensus/tests/*/helper.go | Updated helper functions to create EVM with new pattern |
| accounts/abi/bind/backends/simulated.go | Updated simulated backend to use new EVM initialization pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4c0864c to
0ffb353
Compare
Proposed changes
Ref: ethereum#30830
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that