Skip to content

Align debug_traceCall base fee with Geth - #12804

Merged
flcl42 merged 4 commits into
masterfrom
fix/debug-trace-call-zero-base-fee
Aug 19, 2026
Merged

flcl42 merged 4 commits into
masterfrom
fix/debug-trace-call-zero-base-fee

Conversation

@flcl42

@flcl42 flcl42 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Changes

  • Align debug_traceCall with Geth by using a zero base fee when the call has no gas pricing.
  • Apply the zeroing after block overrides so an unpriced call remains unpriced even when an override supplies a base fee.
  • Add a regression test covering both the default and overridden base-fee paths.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

  • The focused debug_traceCall_without_gas_pricing_uses_zero_base_fee regression test passed.
  • The release solution build completed with warnings treated as errors.

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

Remarks

Geth lowers the base fee to zero for unpriced debug_traceCall requests. Without that behavior, BASEFEE-dependent calls can diverge from Geth or fail in the trace context when the live block base fee is non-zero.

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @flcl42's task in 6m 53s —— View job


Code review — PR #12804

  • Read PR diff
  • Analyze GethStyleTracer change for correctness/regressions
  • Compare against Geth semantics and existing eth_call handling
  • Check for regressions in existing debug_traceCall tests
  • Review test coverage
  • Post review

The core idea is right: Geth lowers the base fee to zero for unpriced call-like execution (vm.Config.NoBaseFee + zero effective gas price resets BlockContext.BaseFee), and Nethermind's debug_traceCall did not. The fix is small and correctly placed after ApplyOverrides. My concerns are about reusing the mechanism the codebase already has rather than adding a second, subtly different one.

Findings

# Severity Finding
1 Medium Duplicate/divergent "unpriced" predicate — eth_call and debug_traceCall will disagree
2 Medium Base-fee-override ordering is the opposite of eth_call's
3 Medium Regression test doesn't pin down the negative (priced) case
4 Low Trace(ulong blockNumber, Transaction tx, …) overload not covered
5 Low Comment omits the non-obvious "why after overrides"

1. Medium — duplicate/divergent "unpriced call" predicate (inline)

GethStyleTracer.cs:69 derives "unpriced" as tx.MaxFeePerGas.IsZero. The repo already models this exact concept as TransactionForRpc.ShouldSetBaseFee() (LegacyTransactionForRpc.cs:135, EIP1559TransactionForRpc.cs:66-67), which drives NoBaseFeeclonedHeader.BaseFeePerGas = 0 in EthRpcModule.TransactionExecutor.cs:63-66,81.

They disagree for {"maxPriorityFeePerGas":"0x1"} with no maxFeePerGas:

eth_call debug_traceCall (this PR)
predicate ShouldSetBaseFee()true (MaxPriorityFeePerGas.IsPositive()) tx.DecodedMaxFeePerGas == 0 (EIP1559TransactionForRpc.cs:60) → zero it
BASEFEE live base fee 0

Prefer computing the flag from call.ShouldSetBaseFee() in DebugRpcModule.debug_traceCall (the TransactionForRpc is still in scope at line 117) and threading it through — e.g. on GethTraceOptions — rather than re-deriving it from the built Transaction inside the consensus layer.

2. Medium — override ordering is the opposite of eth_call's (inline)

eth_call zeroes the base fee on the cloned header before the block override is applied inside the bridge (see the comment at EthRpcModule.TransactionExecutor.cs:70-71), so there an explicit blockOverrides.baseFeePerGas wins. This PR applies the override at line 194 and then unconditionally discards it at line 198, so in debug_traceCall it loses.

The PR body explicitly justifies this ordering, and it does match Geth. The unacknowledged part is that Nethermind now answers the same question two opposite ways depending on the RPC. Please either align eth_call (separately is fine) or call the divergence out.

3. Medium — test doesn't cover the negative case (inline)

Both assertions in DebugRpcModuleTests.cs:118-134 expect BASEFEE == 0. Dropping the zeroBaseFeeForUnpricedCall guard and zeroing unconditionally would still pass. The guard that matters is the complement: a priced call must still see the live base fee (and 0x100 under the override). Per AGENTS.md the two near-identical requests should also be a single [TestCase]-parameterized test rather than a copy-paste with a duplicated data literal.

4. Low — sibling overload untouched

GethStyleTracer.cs:96 (Trace(ulong blockNumber, Transaction tx, …)) is the other synthetic-transaction entry point and keeps the old behavior. It has no in-repo callers today, but it is on IGethStyleTracer, so a plugin using it gets inconsistent semantics.

5. Low — comment

// EIP-1559 requires the base fee to be below the fee cap; match geth for unpriced calls. states the invariant, which is fine, but omits the genuinely non-obvious bit: why it must run after ApplyOverrides. That's the line someone will "tidy up" later.


Checked and clear

  • No cached-header corruption. Trace(BlockParameter, tx, …) goes through WithReplacedBodyCloned, which clones the header (Block.cs:53), so mutating BaseFeePerGas cannot leak into the block-tree cache. Other TraceImpl callers pass block-tree-owned blocks but leave the flag false, so nothing regresses.
  • No regression in the existing Geth-compat failure tests. balanceCheck includes Value, so the InsufficientFundsForTransfer and IntrinsicGasTooLow cases still produce the same error prefixes with a zeroed base fee (TransactionProcessor.cs:1123-1136); the third case is priced, so it is unaffected.
  • Predicate is correct for the common shapes. Legacy {gasPrice} maps to MaxFeePerGas via Transaction.cs:51 and PromoteToEip1559IfTypeDefaulted, so priced legacy calls keep the live base fee.
  • debug_traceCallMany / streaming both route through the same overload, so they pick the fix up consistently.
  • Blob base fee is not zeroed, but Geth only zeroes BlobBaseFee when maxFeePerBlobGas is explicitly 0, which does not apply to plain calls — not a divergence.
  • No security or hot-path performance impact — debug RPC only, one UInt256 assignment.
  • Unused usings: none; all three added test usings (Nethermind.Specs, .Forks, .Test) resolve TestSpecProvider, London, OverridableReleaseSpec.

Not verified

I could not execute dotnet test in this environment (the command requires approval), so I did not independently confirm that the new test passes or that it fails without the fix. All findings above are from static analysis.
· branch fix/debug-trace-call-zero-base-fee

@flcl42 flcl42 added the wip Work in Progress label Aug 13, 2026
Comment thread src/Nethermind/Nethermind.Consensus/Tracing/GethStyleTracer.cs Outdated
Comment thread src/Nethermind/Nethermind.Consensus/Tracing/GethStyleTracer.cs
Comment thread src/Nethermind/Nethermind.JsonRpc.Test/Modules/DebugRpcModuleTests.cs Outdated
@flcl42
flcl42 marked this pull request as ready for review August 18, 2026 12:34

@wurdum wurdum 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.

It looks like it makes sense to unify this base fee override logic in one place. NoBaseFee is already computed from TransactionForRpc.ShouldSetBaseFee() in both RPC modules; letting that flag travel with the BlockOverride into the single place that applies it would make one line decide that an unpriced call keeps a zero base fee.

The natural home is IOverridableEnv.BuildAndOverride, which is plugin-visible surface the project's guidance asks not to widen. A cheaper variant reaches most of the benefit without touching an interface: move GetBlockOverrideForExecution up into TxExecutor<TResult> so all three eth executors share it, and set NoBaseFee in DebugBridge.GetBundleTrace from the TransactionForRpc already in hand there.

Comment thread src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.TransactionExecutor.cs Outdated
Comment thread src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.TransactionExecutor.cs Outdated
Comment thread src/Nethermind/Nethermind.Consensus/Tracing/GethStyleTracer.cs
@github-actions

Copy link
Copy Markdown
Contributor

EVM Opcode Benchmark Diff

Aggregated runs: base=1, pr=1

No significant regressions or improvements detected.

@flcl42
flcl42 merged commit 58ecde1 into master Aug 19, 2026
770 of 774 checks passed
@flcl42
flcl42 deleted the fix/debug-trace-call-zero-base-fee branch August 19, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants