diff --git a/src/ethereum/forks/amsterdam/vm/__init__.py b/src/ethereum/forks/amsterdam/vm/__init__.py index e1aca5e609b..98c8618a337 100644 --- a/src/ethereum/forks/amsterdam/vm/__init__.py +++ b/src/ethereum/forks/amsterdam/vm/__init__.py @@ -181,7 +181,6 @@ class Evm: accessed_storage_keys: Set[Tuple[Address, Bytes32]] regular_gas_used: Uint = Uint(0) state_gas_used: Uint = Uint(0) - state_gas_refund: Uint = Uint(0) state_gas_refund_pending: Uint = Uint(0) @@ -191,10 +190,8 @@ def credit_state_gas_refund(evm: Evm, amount: Uint) -> None: Clamp the applied portion to this frame's `state_gas_used` — the matching charge may sit in an ancestor sharing storage via - CALLCODE/DELEGATECALL. Track it in `state_gas_refund` so - `incorporate_child_on_error` can undo the inflation, and defer the - unapplied remainder in `state_gas_refund_pending` for propagation - on success. + CALLCODE/DELEGATECALL. Defer the unapplied remainder in + `state_gas_refund_pending` for propagation on success. Parameters ---------- @@ -207,7 +204,6 @@ def credit_state_gas_refund(evm: Evm, amount: Uint) -> None: applied = min(amount, evm.state_gas_used) evm.state_gas_left += applied evm.state_gas_used -= applied - evm.state_gas_refund += applied evm.state_gas_refund_pending += amount - applied @@ -215,10 +211,9 @@ def incorporate_child_on_success(evm: Evm, child_evm: Evm) -> None: """ Incorporate the state of a successful `child_evm` into the parent `evm`. - Propagate `state_gas_refund` (inline credits the child applied) so - an ancestor revert can undo the inflation, and apply - `state_gas_refund_pending` (the unapplied remainder) to the parent - via `credit_state_gas_refund`; any leftover propagates further up. + Apply `state_gas_refund_pending` (the unapplied remainder of any + refund credited inside the child) to the parent via + `credit_state_gas_refund`; any leftover propagates further up. Parameters ---------- @@ -237,7 +232,6 @@ def incorporate_child_on_success(evm: Evm, child_evm: Evm) -> None: evm.accessed_storage_keys.update(child_evm.accessed_storage_keys) evm.regular_gas_used += child_evm.regular_gas_used evm.state_gas_used += child_evm.state_gas_used - evm.state_gas_refund += child_evm.state_gas_refund credit_state_gas_refund(evm, child_evm.state_gas_refund_pending) @@ -253,11 +247,11 @@ def incorporate_child_on_error( that spilled into `gas_left`, is restored to the parent's reservoir and the child's `state_gas_used` is not accumulated. - Inline state-gas refunds (SSTORE 0 to x to 0, CREATE silent failure) - credited by the child inflated its `state_gas_left`; subtract - `state_gas_refund` from the amount returned to the parent's - reservoir so the inflation does not leak across the error boundary. - `state_gas_refund_pending` is discarded with the child frame. + `state_gas_refund_pending` is discarded with the child frame: any + inline credits the child applied are keyed to charges (its own + SSTORE or CREATE pre-charge) that are themselves rolled back, so + the matching `state_gas_left + state_gas_used` sum already reflects + the correct amount to return to the parent. Parameters ---------- @@ -269,9 +263,7 @@ def incorporate_child_on_error( """ evm.gas_left += child_evm.gas_left evm.state_gas_left += ( - child_evm.state_gas_used - + child_evm.state_gas_left - - child_evm.state_gas_refund + child_evm.state_gas_used + child_evm.state_gas_left ) evm.regular_gas_used += child_evm.regular_gas_used diff --git a/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_reservoir.py b/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_reservoir.py index 041a0bd4244..03ec64df1b9 100644 --- a/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_reservoir.py +++ b/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_reservoir.py @@ -1328,9 +1328,7 @@ def test_nested_failure_resets_to_tx_reservoir( - `cumulative_gas_used` (receipt) pins `tx.gas - gas_left - state_gas_left`, catching bugs in the leftover split. - `header.gas_used` pins `max(block_regular, block_state)` via - the block accumulators. They differ from the receipt by - exactly `non_top_burns` (inline refunds the user pays for but - the block doesn't track in either accumulator). + the block accumulators. """ gas_limit_cap = fork.transaction_gas_limit_cap() assert gas_limit_cap is not None @@ -1359,27 +1357,13 @@ def test_nested_failure_resets_to_tx_reservoir( else: top, frame_codes = _build_create_chain(pre, frame_bodies, terminator) - # Non-top inline state-gas refunds (body SSTORE x→0 plus CREATE - # pre-charge credits on child failure) accumulate in each frame's - # `state_gas_refund` and get subtracted at the parent's - # `incorporate_child_on_error` boundary so the inflation does not - # leak across the rolled-back state change. The top frame's - # refund is preserved by the tx-level error handler. - non_top_body_refund_burn = sum( - b.state_refund(fork) for b in frame_bodies[1:] - ) - non_top_create_credit_burn = max(0, n_creates - 1) * new_account_state_gas - non_top_burns = non_top_body_refund_burn + non_top_create_credit_burn - sum_regular = sum(code.regular_cost(fork) for code in frame_codes) spill = max(0, total_state_charges - reservoir) if failure_mode == "halt": # Policy A (updated EIP): all state-gas — body charges, spilled # portions, and CREATE pre-charges (returned via credit) — folds # into state_gas_left at tx end. gas_left is zeroed by halt. - # `non_top_burns` is the inline refund burn at incorporate - # boundaries that does not return to the user's reservoir. - state_gas_at_end = max(reservoir, total_state_charges) - non_top_burns + state_gas_at_end = max(reservoir, total_state_charges) expected_cumulative = tx_gas - state_gas_at_end # Header: block_regular = gas_limit_cap - spill (spilled # state-gas drained gas_left but is no longer reclassified to @@ -1387,13 +1371,11 @@ def test_nested_failure_resets_to_tx_reservoir( expected_header_gas_used = gas_limit_cap - spill elif failure_mode == "revert": # Revert preserves gas_left; full state-gas refund. - # User pays only regular costs + intrinsic + non-top burns. - expected_cumulative = intrinsic_cost + sum_regular + non_top_burns + # User pays only regular costs + intrinsic. + expected_cumulative = intrinsic_cost + sum_regular # Header reflects the regular-vs-state attribution directly: # state_gas_used is zeroed by the tx error handler, so only - # regular gas usage shows up. The refund burn lives in the - # `state_gas_left` shortfall (visible in cumulative), not - # the regular accumulator. + # regular gas usage shows up. expected_header_gas_used = intrinsic_cost + sum_regular else: raise ValueError("Invariant, unreachable code.") diff --git a/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_sstore.py b/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_sstore.py index 0001613dce6..899a3da55c8 100644 --- a/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_sstore.py +++ b/tests/amsterdam/eip8037_state_creation_gas_cost_increase/test_state_gas_sstore.py @@ -828,12 +828,15 @@ def test_sstore_restoration_sub_frame_revert( call_opcode: Op, ) -> None: """ - Verify 0 to x to 0 reservoir refund unwinds on sub-frame REVERT. - - The sub-call performs 0 to x to 0 then REVERTs. If the reservoir - refund is not rolled back with the reverted frame, the reservoir - stays inflated by `sstore_state_gas`. A single-SSTORE probe sized - to OOG by 1 would then succeed; the test asserts it OOGs. + Verify 0 to x to 0 reservoir refund returns to the caller on + sub-frame REVERT. + + The sub-call performs 0 to x to 0 then REVERTs. Since both the + set-charge and its refund roll back together, the + `state_gas_used + state_gas_left` sum reflects the unconsumed + reservoir and is returned to the caller via + `incorporate_child_on_error`. A single-SSTORE probe sized to OOG + by 1 succeeds, confirming the caller's reservoir was replenished. """ gas_costs = fork.gas_costs() # Probe SSTORE(0, 1): 2 pushes + cold storage write + state gas - 1, @@ -853,7 +856,7 @@ def test_sstore_restoration_sub_frame_revert( # and REVERT without a hard-coded budget. caller_storage = Storage() caller_code = Op.POP(call_opcode(gas=Op.GAS, address=child)) + Op.SSTORE( - caller_storage.store_next(0, "probe_must_fail"), + caller_storage.store_next(1, "probe_must_succeed"), Op.CALL(gas=probe_gas, address=probe), ) caller = pre.deploy_contract(code=caller_code) @@ -876,16 +879,15 @@ def test_sstore_restoration_ancestor_revert( fork: Fork, ) -> None: """ - Verify the SSTORE 0 to x to 0 refund unwinds when an ancestor frame - (not the applying frame itself) reverts. + Verify the SSTORE 0 to x to 0 refund returns to the caller when an + ancestor frame (not the applying frame itself) reverts. Inner frame applies the refund and returns successfully; its - refund propagates to middle via `incorporate_child_on_success`. - Middle then REVERTs; its refund must be dropped by the caller's - `incorporate_child_on_error`, rather than propagating up. This - exercises the recursive scope that single-frame revert tests do - not: a bug in the success propagation of `state_gas_refund` would - leak the refund into the caller's reservoir. + `state_gas_left` (inflated by the refund) propagates to middle + via `incorporate_child_on_success`. Middle then REVERTs; the + refunded reservoir flows back to the caller via + `incorporate_child_on_error`, so the caller's reservoir is + replenished by `sstore_state_gas`. """ gas_costs = fork.gas_costs() # Probe SSTORE(0, 1): 2 pushes + cold storage write + state gas - 1, @@ -910,7 +912,7 @@ def test_sstore_restoration_ancestor_revert( code=( Op.POP(Op.CALL(gas=Op.GAS, address=middle)) + Op.SSTORE( - caller_storage.store_next(0, "probe_must_fail"), + caller_storage.store_next(1, "probe_must_succeed"), Op.CALL(gas=probe_gas, address=probe), ) ), @@ -936,16 +938,17 @@ def test_sstore_restoration_create_init_revert( create_opcode: Op, ) -> None: """ - Verify reservoir refunds unwind when CREATE init code REVERTs - inside a sub-frame that also REVERTs. + Verify reservoir refunds return to the caller when CREATE init + code REVERTs inside a sub-frame that also REVERTs. Wrapping the CREATE in an outer reverting frame isolates the rollback concern from the legitimate CREATE silent-failure refund (`create_account_state_gas` credited to the frame executing the - CREATE opcode). When the outer frame reverts, every refund that - occurred inside it must unwind, leaving the caller's reservoir at - its pre-call value. A single-SSTORE probe sized to OOG by 1 - detects any leaked refund. + CREATE opcode). When the outer frame reverts, the refunded + reservoir flows back to the caller via + `incorporate_child_on_error`, replenishing the caller's + reservoir by at least `sstore_state_gas`. A single-SSTORE probe + sized to OOG by 1 succeeds, confirming the propagation. """ gas_costs = fork.gas_costs() # Probe SSTORE(0, 1): 2 pushes + cold storage write + state gas - 1, @@ -965,9 +968,7 @@ def test_sstore_restoration_create_init_revert( else: create_call = Op.CREATE2(0, 0, len(init_code), 0) - # Inner contract performs the CREATE then REVERTs, so any refunds - # (SSTORE restoration or CREATE silent-failure) applied during its - # execution must unwind with the frame. + # Inner contract performs the CREATE then REVERTs. inner = pre.deploy_contract( code=( Op.MSTORE( @@ -985,7 +986,7 @@ def test_sstore_restoration_create_init_revert( code=( Op.POP(Op.CALL(gas=Op.GAS, address=inner)) + Op.SSTORE( - caller_storage.store_next(0, "probe_must_fail"), + caller_storage.store_next(1, "probe_must_succeed"), Op.CALL(gas=probe_gas, address=probe), ) ),