feat(spec-specs, tests): EIP-8037 drop double refund on incorporate_child_on_error

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,18 +204,16 @@ 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
 
 
 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
 

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,41 +1357,25 @@ 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
         # regular under Policy A); block_state ≈ 0 for plain CALLs.
         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.")

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),
             )
         ),