refactor(core,eth,internal): switch EVM tx context in ApplyMessage #30809#2284
refactor(core,eth,internal): switch EVM tx context in ApplyMessage #30809#2284gzliudan wants to merge 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 ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 PR refactors EVM transaction-context handling by moving evm.SetTxContext(core.NewEVMTxContext(msg)) into core.ApplyMessage, removing redundant caller-side setup across RPC/simulation/test paths. It also updates tracer plumbing/tests so tracers can observe execution-time gas price (which may differ from raw tx price in XDPoS due to TRC21/fixed-price fee handling).
Changes:
- Centralize EVM tx-context seeding inside
core.ApplyMessageand refactor state transition internals accordingly. - Remove caller-side
SetTxContextin multiple call sites; switch one simulated backend path from direct transition execution toApplyMessage. - Extend tracer VM context and update JS tracer/tests to read gas price from execution context.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
core/state_transition.go |
ApplyMessage now seeds tx context; state transition refactored to unexported internals. |
core/state_processor.go |
Ensures tx context is set before OnTxStart hooks (XDPoS execution pricing visibility). |
core/tracing/hooks.go |
Extends VMContext with BaseFee and clarifies GasPrice semantics for XDPoS tracers. |
core/vm/evm.go |
Populates BaseFee and GasPrice into tracer VMContext. |
eth/tracers/js/goja.go |
JS tracer reads execution-time gas price from VMContext. |
eth/tracers/js/tracer_test.go |
Adjusts harness and adds coverage for execution-time gas price on OnTxStart. |
eth/tracers/api.go |
Removes redundant tx-context setup before ApplyMessage calls. |
eth/tracers/api_test.go |
Drops redundant tx-context setup in test backend transaction replay. |
eth/tracers/tracers_test.go |
Benchmark path updated to use ApplyMessage and per-iteration tx context. |
eth/tracers/internal/tracetest/calltrace_test.go |
Updates tracer bench/tests to align with new ApplyMessage behavior. |
internal/ethapi/api.go |
Removes redundant SetTxContext before applyMessageWithEVM. |
internal/ethapi/simulate.go |
Removes redundant SetTxContext in simulated block processing. |
eth/gasestimator/gasestimator.go |
Drops redundant tx-context creation; uses ApplyMessage seeding. |
eth/state_accessor.go |
Removes redundant tx-context setup during historical tx replay. |
ethclient/simulated/backend.go |
Switches from manual state transition execution to core.ApplyMessage. |
core/state_prefetcher.go |
Uses ApplyMessage directly for prefetch, removes helper. |
core/token_validator.go |
Updates call path to new state transition internals. |
tests/state_test_util.go |
Drops redundant tx-context setup; relies on ApplyMessage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2089e65 to
51afb30
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
51afb30 to
d365098
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
core/state_transition.go:239
- The exported StateTransition/NewStateTransition API appears to have been removed/hidden by renaming to stateTransition/newStateTransition. If external packages import core and rely on these symbols, this is a breaking change; consider keeping exported wrappers (possibly deprecated) or otherwise ensuring downstream users are migrated as part of the same release.
type stateTransition struct {
gp *GasPool
msg *Message
gasRemaining uint64
initialGas uint64
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…hereum#30809 Move EVM tx context switching into ApplyMessage and remove the redundant caller-side setup. Keep the execution gas price visible to tracers in XDPoS, because TransactionToMessage can rewrite msg.GasPrice for TRC21 and fixed-price fee paths before execution. Seed the tx context before OnTxStart and restore VMContext.GasPrice so tracers observe the execution-time pricing instead of the raw transaction price. Update tracer tests to cover the execution gas price path.
d365098 to
3f37098
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed changes
Move EVM tx context switching into ApplyMessage and remove the redundant caller-side setup.
Keep the execution gas price visible to tracers in XDPoS, because TransactionToMessage can rewrite msg.GasPrice for TRC21 and fixed-price fee paths before execution.
Seed the tx context before OnTxStart and restore VMContext.GasPrice so tracers observe the execution-time pricing instead of the raw transaction price. Update tracer tests to cover the execution gas price path.
Ref: ethereum#30809
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
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