execution: implement 8037 changes for 7.2.0 fixtures#21275
Conversation
…tetest-speed-regression
…tetest-speed-regression
…/erigon into worktree-eest-spec-v7.2.0
…ontech/erigon into worktree-eest-spec-v7.2.0
There was a problem hiding this comment.
Pull request overview
Updates Erigon to consume EEST tests-bal@v7.2.0 fixtures and implements the corresponding EIP-8037 spec adjustment ("SSTORE/collision clear dynamics"), in which inline state-gas refunds are credited to the local frame's reservoir immediately and the per-frame state-gas counter becomes signed.
Changes:
- Make
MdGasUsage.Statea signedint64and addPlusIntrinsic/StateClamped/Totalhelpers; remove thePendingStateGasCreditplumbing. - Reshape the EVM call/create flow:
frameStateUsedis signed,creditStateGasRefundis now unclamped, andhandleFrameRevertrestores the parent reservoir via the signedstate_gas_left + state_gas_usedinvariant (panicking if it goes negative). A newderiveFrameRegularGasUsedhandles refund-heavy frames in signed arithmetic. - Collapse the Amsterdam branches in
TxnExecutor.Executeto a singlePlusIntrinsic/StateClamped/Totalform, bump theeest_devnetfixture pin totests-bal@v7.2.0, and addTestDeriveFrameRegularGasUsed.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test-fixtures.json | Pin eest_devnet to tests-bal@v7.2.0 (new URL, sha256, size). |
| execution/protocol/mdgas/md_gas.go | Signed MdGasUsage.State; add PlusIntrinsic, StateClamped, Total; drop PendingStateGasCredit. |
| execution/vm/interpreter.go | CallContext.frameStateUsed becomes int64; remove refundCreditPending; unclamped creditStateGasRefund; rename leftOver to gasRemaining in Run. |
| execution/vm/instructions.go | Drop PendingStateGasCredit absorption in CALL/CALLCODE/DELEGATECALL/STATICCALL/CREATE success paths; fold signed childUsed.State into parent. |
| execution/vm/evm.go | Rename leftOverGas → gasRemaining; signed handleFrameRevert with invariant panic; new deriveFrameRegularGasUsed; depth-0 error semantics fold into gasUsed.State. |
| execution/protocol/txn_executor.go | Replace Amsterdam refund/no-refund branches with combined := gasUsed.PlusIntrinsic(imdGas) and use StateClamped/Total. |
| execution/vm/evm_test.go | New TestDeriveFrameRegularGasUsed covering charges-only, spillover, refund-exceeds-charges, and pure-refund cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
TL;DR: Design is solid and largely matches what a faithful port of execution-specs PR #2863 should look like. Two CI checks fail (race-tests / execution-other, sonar) on the same root cause: an int64(uint64) cast in handleFrameRevert (and deriveFrameRegularGasUsed) wraps for the very-large state reservoir that runtime.Execute produces from GasLimit = MaxUint64. This needs to be fixed before merge. The PR description's claim "go test ./execution/vm/... ./execution/protocol/... — unit tests green" is incorrect — TestEip2929Cases panics locally too.
Blocking — int64(uint64) wrap
TestEip2929Cases (and any caller of runtime.Execute with GasLimit = MaxUint64) panics:
panic: EIP-8037 invariant violated: state_gas_left (18446744073692774399) + child state_gas_used (0) = -16777217 < 0
18446744073692774399 = MaxUint64 - MaxTxnGasLimit (2^24) — SplitTxnGasLimit allocates the entire MaxUint64 - 16777216 overflow to the state reservoir. int64(gasRemaining.State) reinterprets that as -16777217, and the + 0 check panics on a value that is mathematically ~2^64 ≥ 0.
Two callsites have the same hazard:
handleFrameRevert—int64(gasRemaining.State) + stateGasUsed.deriveFrameRegularGasUsed—int64(inputTotal) - int64(gasRemainingTotal). Less likely to actually wrap (the result is then converted back to uint64 anyway), but still a foot-gun ifinputTotal≥2^63.
Suggested fix — keep the spec invariant but do it in sign-aware uint64:
if evm.chainRules.IsAmsterdam && depth > 0 {
if stateGasUsed >= 0 {
gasRemaining.State += uint64(stateGasUsed)
} else {
absUsed := uint64(-stateGasUsed)
if absUsed > gasRemaining.State {
panic(fmt.Sprintf("EIP-8037 invariant violated: state_gas_left (%d) < |child state_gas_used| (%d)",
gasRemaining.State, absUsed))
}
gasRemaining.State -= absUsed
}
}Same shape works for deriveFrameRegularGasUsed (split on sign of stateGasUsed, then operate on inputTotal/gasRemainingTotal in uint64). The existing unit-test cases pass through unchanged; add one with inputTotal and gasRemainingTotal near MaxUint64 to lock in the regression.
Minor
- PR description test plan is stale on
go test ./execution/vm/...— re-run with the fix and update. - Worth verifying the panic message in
handleFrameRevertmatches the corrected check (printing|child state_gas_used|rather than the signed value avoids the misleading "-16777217" in the message you'd see for a wrapped value).
Summary
Bumps the EEST devnet fixtures from
tests-bal@v7.1.1totests-bal@v7.2.0and adapts Erigon to the EIP-8037 spec change that ships with it.What changed in EEST v7.2.0
The only behavioural change in the new fixtures is execution-specs PR #2863 — "EIP-8037 SSTORE/collision clear dynamics":
state_gasrefunds (SSTOREoriginal-zero reset,CREATEcollision/revert) now credit the local frame'sstate_gas_leftimmediately, instead of being clamped to a per-framestate_gas_usedcounter and bubbled up as a pending credit.Evm.state_gas_usedbecomes a signed counter — it goes negative when refunds in a frame outweigh the frame's own charges (typical forDELEGATECALL/CALLCODEcallees that clear a slot an ancestor set).incorporate_child_on_successno longer propagates a pending refund;incorporate_child_on_errorrestores the parent's reservoir via thestate_gas_left + state_gas_usedinvariant.process_transactioncomputes block state gas asUint(max(0, intrinsic_state + state_gas_used − state_refund)).Bulk of the rest of the v7.2.0 diff is additional BAL test coverage synchronised from
forks/amsterdam.Erigon changes
test-fixtures.json: bumpeest_devnettotests-bal@v7.2.0(sha256fc1d9ae1…, 611 743 632 bytes).execution/protocol/mdgas/md_gas.go:MdGasUsagekeepsRegular uint64butStatebecomesint64. AddsMdGasUsage.PlusIntrinsic(MdGas),MdGasUsage.StateClamped()(=uint64(max(0, State))), andMdGasUsage.Total() = Regular + StateClamped().MdGas(reservoirs / leftover) keeps both fieldsuint64.execution/vm/interpreter.go:CallContext.frameStateUsedis nowint64.refundCreditPendingis removed.creditStateGasRefunddrops the clamp — it doesstateGas += amount; frameStateUsed -= int64(amount)directly.execution/vm/instructions.go:op{Call,CallCode,DelegateCall,StaticCall,execCreate}drop thePendingStateGasCreditabsorption; success path is justscope.frameStateUsed += childUsed.State(signed).execution/vm/evm.go:evm.call/evm.createuse a named returngasRemaining(renamed fromleftOverGas) andgasas the parameter; mutations are visible in the body.handleFrameRevert(gasRemaining *MdGas, …, stateGasUsed int64)takes the signed state usage and restores the parent reservoir viagasRemaining.State + stateGasUsed. Includes a panic on the invariant violationstate_gas_left + state_gas_used < 0so any future regression in theuseMdGas/creditStateGasRefundpairing surfaces loudly rather than silently wrapping theuint64cast.gasUsed.State: CALL error resets it to0; CREATE error sets it to-StateGasNewAccount(mirrors the spec'sstate_refund += STATE_BYTES_PER_NEW_ACCOUNT * COST_PER_STATE_BYTEon CREATE-tx failure).TxnExecutorthen readsgasUsed.Statedirectly without an error-path branch.deriveFrameRegularGasUsed(inputTotal, gasRemainingTotal uint64, stateGasUsed int64) uint64—Regular = (input − leftover) − statein signedint64so a refund-heavy frame whosegasRemaining.Stategrew above the input still yields the correct positive regular-ops count (a guardeduint64subtraction would mis-skip and return 0).execution/protocol/txn_executor.go: Amsterdam branches collapse the spec'stx_state_gas = intrinsic_state + state_gas_used+block_state_gas_used += max(0, tx_state_gas)intocombined := gasUsed.PlusIntrinsic(imdGas); st.blockStateGasUsed = combined.StateClamped(); st.txnGasUsedB4Refunds = combined.Total().Tests
execution/vm/evm_test.go(new):TestDeriveFrameRegularGasUsedwith four sub-cases — charges-only, with-spillover, refunds-exceed-charges (the edge case the old guarded subtraction returned0for), and pure-refund. Worked numeric examples are documented inline.Test plan
make lintclean.go test ./execution/vm/... ./execution/protocol/...— unit tests green.statetests-devnet(65 202 tests)blocktests-devnet(82 941 tests)blocktests-devnet-race-amsterdam(21 368 tests)statetests-stable(63 556 tests)blocktests-stable-{sequential,parallel}(69 256 tests each)enginextests-stable-{sequential,parallel}(63 920 tests each)tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_sstore.py::test_sstore_restoration_refund_credits_local_reservoir[fork_Amsterdam-state_test-depth_{1,3,10}-refund_funds_create].