Skip to content

rpc: fix BlockOverrides in trace endpoints - #22183

Merged
taratorio merged 14 commits into
mainfrom
lupin012/fix_blockoverrides_trace_endpoints
Jul 9, 2026
Merged

taratorio merged 14 commits into
mainfrom
lupin012/fix_blockoverrides_trace_endpoints

Conversation

@lupin012

@lupin012 lupin012 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

BlockOverrides (lets callers override block-level fields — baseFeePerGas, gasLimit, number, timestamp, feeRecipient, prevRandao, blobBaseFee — for simulation/tracing) was silently ignored in several RPC methods. Fixed by consistently applying the BlockContext override (and, where needed, the baseFee used to build the message) at every site that required it. debug_traceCall already applied these overrides on main; the change there is a behavior-preserving refactor onto the same shared helpers, not a fix (its two tests are regression guards, not fix-provers).

APIs modified

  • trace_callMany (CallMany/doCallBlock)
  • trace_replayTransaction (callTransaction)
  • trace_replayBlockTransactions — sequential path (callBlock/doCallBlock) and parallel path (doCallBlockParallel)
  • trace_filter (filterV3)
  • debug_traceCall (TraceCall) — refactor only, see above

callBlock and callTransaction are also shared by trace_block, trace_transaction, and trace_get, so those endpoints now honor BlockOverrides too.

How the fix works

  • rpc/ethapi/block_overrides.go: new OverrideBaseFee(baseFee) (*uint256.Int, error) method on the BlockOverrides type.
  • rpc/jsonrpc/trace_adhoc.go: two shared helpers — overrideBaseFee (scalar, preserves nil, used by CallMany and TraceCall where legacy vs EIP-1559 must be distinguished) and overrideBlockContext (applied to blockCtx after construction, used everywhere else).
  • Sites that replay real, already-typed transactions (callTransaction, callBlock, filterV3) read the baseFee directly from &blockCtx.BaseFee (already overridden) instead of computing it separately.
  • callTransaction, callBlock, and filterV3 now derive their types.Signer from the overridden blockCtx.BlockNumber/blockCtx.Time instead of the block's real, un-overridden header — fork-rule-dependent signer selection (e.g. the Spurious Dragon EIP-155 boundary) would otherwise disagree with the overridden lastRules/rules used for the rest of the trace.

Tests added

baseFeePerGas coverage (one per endpoint):

  • TestCallManyBlockOverridesBaseFeeAffectsGasPrice
  • TestReplayTransactionBlockOverridesBaseFeeAffectsGasPrice
  • TestReplayBlockTransactionsBlockOverridesBaseFeeAffectsGasPrice
  • TestReplayBlockTransactionsParallelPathBlockOverridesBaseFee
  • TestFilterBlockOverridesBaseFeeAffectsGasPrice
  • TestDebugTraceCallBlockOverridesBaseFeeAffectsGasPrice

Other-fields coverage (number, timestamp, gasLimit, feeRecipient, prevRandao, blobBaseFee — table-driven, 6 subtests each):

  • TestCallManyBlockOverridesOtherFieldsAffectOpcodes
  • TestReplayBlockTransactionsParallelPathBlockOverridesOtherFieldsAffectOpcodes
  • TestFilterBlockOverridesOtherFieldsAffectOpcodes
  • TestDebugTraceCallBlockOverridesOtherFieldsAffectOpcodes

Rejected-override coverage (a request-level BlockOverrides failure — e.g. an unsupported field or a uint256 overflow — must surface as a normal RPC error, not corrupt per-block state used by later transactions):

  • TestFilterRejectedBlockOverrideReturnsError

Signer/fork-rules coverage (the block-number override must also shift which signer recovers the sender, not just which fork rules apply — otherwise an EIP-155-protected transaction can be replayed under a pre-Spurious-Dragon signer that rejects it):

  • TestReplayTransactionSignerReflectsBlockOverridesNumber
  • TestFilterSignerReflectsBlockOverridesNumber

lupin012 added 2 commits July 2, 2026 23:03
…ction, trace_replayBlockTransactions, trace_filter, and debug_traceCall

BlockOverrides was silently ignored on several trace_* RPC methods and
debug_traceCall because BlockContext (and, where relevant, the baseFee
used for message construction) was never overridden at those call
sites, unlike the sibling trace_call implementation.

Adds overrideBaseFee/overrideBlockContext helpers reused across the
affected call sites, plus regression coverage for baseFeePerGas and
the other BlockOverrides fields (number, timestamp, gasLimit,
feeRecipient, prevRandao, blobBaseFee) on each fixed endpoint.
@lupin012
lupin012 marked this pull request as ready for review July 3, 2026 13:04
@lupin012
lupin012 requested a review from yperbasis as a code owner July 3, 2026 13:04
@lupin012 lupin012 added the RPC label Jul 5, 2026

@yperbasis yperbasis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is red

@lupin012
lupin012 requested a review from yperbasis July 6, 2026 18:33

@yperbasis yperbasis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. One blocking issue and a few minor points.

🔴 Blocking — filterV3 can panic on override failure (rpc/jsonrpc/trace_filtering.go)

The new block-level handler writes an error to the stream and continues without setting lastRules/lastBaseFee:

blockCtx := transactions.NewEVMBlockContext(...)
if err := overrideBlockContext(traceConfig, &blockCtx); err != nil {
    ... rpc.HandleError(err, stream) ...
    continue            // lastRules stays nil on the first block
}
lastRules   = blockCtx.Rules(chainConfig)
lastBaseFee = blockCtx.BaseFee

overrideBlockContextBlockOverrides.Override fails on easily-supplied input — beaconRoot, withdrawals, blockHash are rejected outright (block_overrides.go), plus baseFee/blobBaseFee overflow. Since traceConfig is constant for the request, the first block's override fails, so lastRules is never assigned. The next same-block tx then calls:

msg, err := txn.AsMessage(*lastSigner, &lastBaseFee, lastRules)  // lastRules == nil

and DynamicFeeTransaction.AsMessage dereferences it (dynamic_fee_tx.go: if !rules.IsLondon) → nil-pointer panic. It's caught by the per-request recover and returned as "method handler crashed", but only after partial JSON was already streamed. On main these override fields were silently ignored (no error, no panic), so this is a regression, and it's reachable from public trace_filter.

An override rejection is a request-level error (identical for every block), not a per-block transient like the surrounding HandleError+continue sites. Please validate the override once up front — e.g. a dry-run overrideBlockContext(traceConfig, &evmtypes.BlockContext{}) in Filter/top of filterV3, returning its error before any streaming — or at minimum return err in both new filterV3 handlers instead of continue.

Minor

  • No test covers the override-failure path (the bug above). Please add one for trace_filter with a rejected/overflowing override.
  • OverrideBaseFee docstring (block_overrides.go) is longer than the repo comment policy wants — trim the callsite/scenario narration to the invariant.
  • debug_traceCall already applied Override(&blockCtx) and the baseFee override on main (tracing.go), so that change is a behavior-preserving refactor and its two tests are regression guards, not fix-provers. Listing it under "silently ignored … Fixed" overstates it a bit.

lupin012 added 2 commits July 7, 2026 21:42
overrideBlockContext could fail mid-block-loop in filterV3 (e.g.
rejected beaconRoot/withdrawals/blockHash, or a uint256 overflow on
baseFee/blobBaseFee) without setting lastRules for the block, causing
a nil-pointer panic in a later transaction's AsMessage call. Validate
BlockOverrides once up front in Filter, before any streaming starts,
and return the two now-unreachable overrideBlockContext failures in
filterV3 directly instead of continuing with stale state.

Also trims the OverrideBaseFee docstring and adds a regression test
for the rejected-override path.
@lupin012

lupin012 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@yperbasis

  • Blocking filterV3 (panic): Filter() now calls overrideBlockContext() once, up front, before filterV3 starts streaming
    if it's going to fail, it fails identically for every block in the request. As defense-in-depth, I also changed both continue sites inside filterV3 (block-level and per-tx) to return err directly, matching the ctx.Err() pattern already used elsewhere in the function — so the fragile continue-without-resetting-state pattern that caused this bug is gone even if that invariant is ever broken by a future change to Override.

Minor points:

  • Added TestFilterRejectedBlockOverrideReturnsError — reproduces the exact panic via a rejected beaconRoot override; confirmed it panics before the fix and passes after.
  • Trimmed the OverrideBaseFee docstring — dropped the ToMessage callsite mention, kept the "why a separate method" invariant.
  • Fixed the PR description — debug_traceCall is now called out explicitly as a behavior-preserving refactor, not a fix, and its tests are labeled as regression guards.

@lupin012
lupin012 requested a review from yperbasis July 7, 2026 19:51

@yperbasis yperbasis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix — the up-front validation plus return err resolves the blocking issue, and I verified TestFilterRejectedBlockOverrideReturnsError fails on the pre-fix revision and passes on the head.

Two non-blocking nits:

  • OverrideBaseFee docstring: the claim that Override/OverrideHeader/OverrideBlockContext "never introduce a base fee on pre-London blocks" is inaccurate for OverrideHeader, which sets h.BaseFee unconditionally when BaseFeePerGas is present. Suggest dropping the "matching …" clause.
  • PR description: callBlock/callTransaction also serve trace_block, trace_transaction, and trace_get, so those endpoints now honor BlockOverrides too — worth a line in the description.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes multiple tracing/debug RPC endpoints that previously ignored BlockOverrides by consistently applying overrides to EVM BlockContext and (where needed) the base fee used to build messages, with expanded regression coverage across endpoints.

Changes:

  • Refactors debug_traceCall to use shared override helpers and applies baseFee overrides consistently.
  • Applies BlockOverrides to BlockContext across trace_* replay/filter paths (including parallel replay workers) and ensures overridden baseFee is used when building tx messages.
  • Adds extensive tests covering baseFee and other overridden block fields across trace/debug endpoints, plus an error-surfacing regression test for rejected overrides.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
rpc/jsonrpc/tracing.go Refactors debug_traceCall baseFee override handling onto shared helper.
rpc/jsonrpc/trace_filtering.go Applies BlockOverrides to BlockContext in filter/replay paths and uses overridden baseFee when building messages.
rpc/jsonrpc/trace_adhoc.go Adds shared helpers to apply baseFee and BlockContext overrides; wires them into ad-hoc trace helpers.
rpc/jsonrpc/trace_adhoc_test.go Adds a reusable test chain harness and many new override regression tests for trace endpoints.
rpc/jsonrpc/eth_call_test.go Extracts fundedBankGenesis helper for reuse by tracing tests.
rpc/jsonrpc/debug_api_test.go Adds debug_traceCall override regression tests using the shared harness.
rpc/jsonrpc/call_traces_test.go Adds trace_filter override regression tests (including rejected override behavior).
rpc/ethapi/block_overrides.go Adds OverrideBaseFee helper for nil-preserving baseFee overrides.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rpc/jsonrpc/trace_filtering.go
Comment thread rpc/jsonrpc/trace_filtering.go
Comment thread rpc/jsonrpc/trace_filtering.go
Comment thread rpc/ethapi/block_overrides.go Outdated
Comment thread rpc/ethapi/block_overrides.go
lupin012 added 2 commits July 8, 2026 21:51
callTransaction, callBlock, and filterV3 derived fork rules from the
BlockOverrides-adjusted BlockContext but still built their
types.Signer from the block's real, un-overridden number/time. Since
signer selection is fork-dependent (e.g. the Spurious Dragon EIP-155
boundary), this could recover a transaction's sender under rules
inconsistent with the overridden BlockContext used for the rest of
the trace. Move signer construction after the override and derive it
from blockCtx.BlockNumber/blockCtx.Time.

Also tightens the OverrideBaseFee docstring further and fixes the
overflow error message to reference the actual baseFeePerGas field
name.
@lupin012
lupin012 enabled auto-merge July 8, 2026 20:18
@lupin012
lupin012 disabled auto-merge July 8, 2026 20:42
@lupin012
lupin012 enabled auto-merge July 8, 2026 21:06
@lupin012
lupin012 added this pull request to the merge queue Jul 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 8, 2026
@lupin012
lupin012 added this pull request to the merge queue Jul 9, 2026
@taratorio
taratorio removed this pull request from the merge queue due to a manual request Jul 9, 2026
@taratorio

Copy link
Copy Markdown
Member

@lupin012 sorry had to remove it from the merge queue because of this https://discord.com/channels/1133875232453693480/1134047852818071582/1524692575905120326 (benchmarks run sporadically get stuck and take up to 6 hours)

@lupin012
lupin012 added this pull request to the merge queue Jul 9, 2026
@taratorio
taratorio removed this pull request from the merge queue due to a manual request Jul 9, 2026
@taratorio
taratorio enabled auto-merge July 9, 2026 12:19
@taratorio
taratorio added this pull request to the merge queue Jul 9, 2026
Merged via the queue into main with commit 49e0ef4 Jul 9, 2026
92 checks passed
@taratorio
taratorio deleted the lupin012/fix_blockoverrides_trace_endpoints branch July 9, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants