fix(evm): align EIP-8037 spill-refund accounting with the EELS reference

[Marchhill]

Follow-up to the PR #12369 review discussion (@LukaszRozmej, @AnkushinDaniil). Stacked on fix/devnet6-review-findings; will retarget to master when #12369 merges.

Changes

Every open question was settled by differential testing against the EELS amsterdam reference at execution-specs d0338f561 — the exact commit the tests-glamsterdam-devnet@v6.1.1 fixtures are built from: identical bytecode scenarios were executed through both implementations and the (spentGas, blockRegularGas, blockStateGas) triples compared (reading block_output.block_gas_used / block_state_gas_used directly on the EELS side).

Answers to the review questions:

• The Math.Min(childState.StateGasSpillRefundAdvanced, unappliedRefund) combinator (Q1): neither Min nor Max(0, S - applied) is correct — under EELS's source-based refunds, advanced refunds carry no spill marks at all. Marks are applied once, at credit time, in the frame doing the LIFO refill, and reach ancestors via the ordinary child-gas merge. VmState.StateGasSpillRefundAdvanced and the combinator are removed.
• The pre-existing double-marking (Q2): confirmed real and fixed — DiscardStateGas no longer re-marks the parent's StateGasSpillRefunded when discharging a child's advance.

Further divergences the differential surfaced (all consensus-relevant, all fixed):

• Reverted/halted children no longer merge their gross StateGasSpill/StateGasSpillRefunded into the parent (EELS refill_frame_state_gas semantics). Previously a reverted grandchild's spill riding through a child halt docked the parent reservoir and overcharged the sender by the spill amount (95k+ gas in the test scenarios).
• The advanced portion of a refund now continues the source-based LIFO refill (gas_left up to the frame's unrefunded spill, remainder to reservoir). Previously all advance money was parked in the reservoir, where it could survive a later exceptional halt and undercharge the sender (2×SSET in the combinator scenario).
• Advance revocation claws the full amount from the reservoir (going negative if needed) without unmarking — the permanent mark keeps net spill consistent through rollbacks, matching EELS's algebra exactly.
• A top-level exceptional halt keeps the full post-refund intrinsic state gas in the state dimension; burned spill stays in the regular dimension (EELS: tx_state_gas = intrinsic_state - state_refund). The StateGasSpillBurned reattribution shifted header.GasUsed by the burned spill in halt-with-authorization scenarios. StateGasSpillBurned and the dead StateGasSpillReclassified field are removed.

Regression tests: Eip8037GasAccountingTests pins 13 scenarios (nested revert-with-spill → cross-frame refund → outer halts, incl. both review traces, top-level-halt-with-auth block splits, create-refund shapes) to EELS-derived gas triples. On the pre-fix branch, 6 of 13 scenarios diverged from EELS in both directions; all 13 now match exactly.

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

• Yes

If yes, did you write tests?

• Yes

Notes on testing

• New Eip8037GasAccountingTests: 13/13 pass (6 failed pre-fix).
• Eip8037Tests policy unit tests updated to the corrected semantics (49/49).
• Full Nethermind.Evm.Test (4898) and Nethermind.Blockchain.Test (1505): green.
• All 298 tests-glamsterdam-devnet@v6.1.1 EIP-8037/EIP-8038 fixture files (812 tests) via nethtest: 0 failures.

Documentation

Requires documentation update

• No

Requires explanation in Release Notes

• No

🤖 Generated with Claude Code

[claude]

Claude finished @Marchhill's task in 4m 48s —— View job

---

Review: EIP-8037 spill-refund accounting alignment with EELS

I reviewed the full diff against origin/fix/devnet6-review-findings, traced the state-gas algebra through EthereumGasPolicy, VirtualMachine, and TransactionProcessor, and checked the new/updated tests.

• Read changed files (gas policy, VM, tx processor)
• Analyze correctness vs EELS reference semantics
• Check edge cases & regressions
• Review tests
• Post findings

---

Verdict: ✅ No blocking issues found

This is a clean, net-removing simplification (−209 lines, three fields deleted) that also fixes real consensus divergences. The design is exactly what the repo rules ask for: it removes state (StateGasSpillBurned, StateGasSpillReclassified, StateGasSpillRefundAdvanced) and the Min(...) combinator rather than adding more corrective machinery, and every remaining path was pinned to a differential oracle.

Correctness — the algebra checks out. I verified the key invariants by hand:

• Spill non-inheritance on revert/halt (RestoreChildStateGas / RestoreChildStateGasOnHalt, EthereumGasPolicy.cs:163,179): dropping the child's StateGasSpill/StateGasSpillRefunded while Refund (:151, success path) still propagates them is the correct LIFO source-based asymmetry — a rolled-back frame's charges must not grow the parent's unrefunded spill. The revert-vs-halt split (parentGas.Value += childNetSpill on revert only) correctly models "revert returns burned gas_left, halt burns it."
• The DiscardStateGas double-mark fix (:362): no longer marking StateGasSpillRefunded when discharging a child's advance is right — the LIFO refill was already accounted in the crediting frame and re-marking here would double-exclude spill from block-regular gas.
• AddStateGasRefundToReservoir LIFO refill (:374): I traced the sole integration caller (CreditStateGasRefund, VirtualMachine.cs:669). By the time the pending branch runs, RefundStateGas has already drained this frame's unrefunded spill (since unrefundedSpill ≤ StateGasUsed − floor = refundableStateGas), so toGasLeft is 0 there — the pending refund lands in the reservoir. The toGasLeft > 0 path is nonetheless correct-by-construction and is directly exercised by the new Revoking_advanced_refund_keeps_refilled_gas_and_marked_spill unit test. Not dead code.
• RemoveStateGasRefundFromReservoir clawback (:385): driving StateReservoir negative without unmarking StateGasSpillRefunded is safe — negative reservoirs are a normal state (ConsumeStateGas produces them), and the only reader that asserts reservoir >= 0 (RefundOnTopLevelHalt, TransactionProcessor.cs:1550) runs after ResetForHalt clamps the reservoir back up to postIntrinsicStateReservoir.
• Top-level-halt state dimension (TransactionProcessor.cs:1562): effectiveStateGas = GetStateGasUsed(gas) after ResetForHalt equals intrinsic_state − state_refund (postHaltIntrinsicStateGas, :1469), matching the EELS tx_state_gas formula; the previous − spillBurned term was the source of the header.GasUsed shift in halt-with-auth blocks.

Signature consistency. All call sites of the three re-signed methods and both deleted-field accessors were updated (VirtualMachine.cs, IGasPolicy.cs, tests). EthereumGasPolicy is the only IGasPolicy<> implementer, so no plugin/Optimism/Taiko/zkEVM policy breaks — the zkEVM CreditStateGasRefund calls go through the unchanged public entry point.

Tests. Eip8037EelsReferenceTests is a strong regression harness — 13 scenarios pinned to EELS-derived (spentGas, blockRegularGas, blockStateGas) triples, covering both review traces, nested revert→cross-frame-refund→halt shapes, top-level-halt-with-auth block splits, and create-refund shapes. Disabling access tracing (:379) to keep cold-access costs aligned with the reference is the right call. The Eip8037Tests unit updates correctly assert the new LIFO/permanent-mark semantics.

Minor / non-blocking observations

• AddStateGasRefundToReservoir's trackSpillRefund parameter is now near-vestigial at the integration level (the only caller always reaches it with unrefunded spill already drained to 0). It's still meaningful in isolation and unit-tested, so this is purely informational — no change requested.
• Nit (comment scope): the <remarks> on RemoveStateGasRefundFromReservoir in IGasPolicy.cs:232 documents the concrete EthereumGasPolicy clawback behavior on the interface's default (no-op) member. Harmless, but the behavioral detail arguably belongs on the implementation. Not worth churn.

I did not independently run the build or test suite in this review (relied on static analysis + the PR's reported 13/13 reference, 49/49 policy, 812 fixture, and full Evm/Blockchain green runs). The signature and semantic analysis gives me high confidence the change is sound.

[github-actions]

EVM Opcode Benchmark Diff

Aggregated runs: base=1, pr=1

No significant regressions or improvements detected.

[AnkushinDaniil]

One coverage gap in Eip8037EelsReferenceTests: all 13 scenarios use gasLimit <= 3M, so the initial reservoir R0 is always 0. R0 > 0 is reachable on amsterdam when tx.gas > 16_777_216 (EIP-7825 cap overflow -> reservoir), which hits an otherwise-untested path (ConsumeStateGas from a large reservoir, ResetForHalt with initialStateReservoir > 0). It behaved correctly when I checked, so just a missing test — a case with tx.gas > 16_777_216 would be worth adding.

The base branch was changed.

[Marchhill]

Good catch — added in 50561a6 as three scenarios with reference-derived expectations: reservoir_funded_sstore (tx.gas = 20M, charge fully reservoir-funded), reservoir_boundary_partial_spill (R0 = 65,000 < STORAGE_SET so the charge drains the reservoir and spills the remainder — the CalculateStateGasSpill partial branch), and reservoir_restored_on_top_halt (child halt + top-level halt with R0 > 0; spent = gasLimit − R0 = 16,777,216 confirms ResetForHalt restores the full initial reservoir). All three match the reference implementation, consistent with your check.