refactor(core,eth,internal): switch EVM tx context in ApplyMessage #30809#2284
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.
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.
|
@gzliudan conflict |
…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.
3f37098 to
d09f274
Compare
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