fix(spec-specs): Check delegation access gas before reading

Author: fselmo

src/ethereum/forks/amsterdam/vm/eoa_delegation.py

@@ -116,11 +116,15 @@ def recover_authority(authorization: Authorization) -> Address:
     return Address(keccak256(public_key)[12:32])
 
 
-def check_delegation(
+def calculate_delegation_cost(
     evm: Evm, address: Address
-) -> Tuple[bool, Address, Address, Bytes, Uint]:
+) -> Tuple[bool, Address, Optional[Address], Uint]:
     """
-    Check delegation info without modifying state or tracking.
+    Check if address has delegation and calculate delegation target gas cost.
+
+    This function reads the original account's code to check for delegation
+    and tracks it in state_changes. It calculates the delegation target's
+    gas cost but does NOT read the delegation target yet.
 
     Parameters
     ----------
@@ -131,77 +135,65 @@ def check_delegation(
 
     Returns
     -------
-    delegation : `Tuple[bool, Address, Address, Bytes, Uint]`
-        (is_delegated, original_address, final_address, code,
-        additional_gas_cost)
+    delegation_info : `Tuple[bool, Address, Optional[Address], Uint]`
+        (is_delegated, original_address, delegated_address_or_none,
+        delegation_gas_cost)
 
     """
     state = evm.message.block_env.state
 
+    # Read original account's code, already checked gas for this
     code = get_account(state, address).code
+    track_address(evm.state_changes, address)
+
     if not is_valid_delegation(code):
-        return False, address, address, code, Uint(0)
+        return False, address, None, Uint(0)
 
     delegated_address = Address(code[EOA_DELEGATION_MARKER_LENGTH:])
 
+    # Calculate gas cost for delegation target access
     if delegated_address in evm.accessed_addresses:
-        additional_gas_cost = GAS_WARM_ACCESS
+        delegation_gas_cost = GAS_WARM_ACCESS
     else:
-        additional_gas_cost = GAS_COLD_ACCOUNT_ACCESS
+        delegation_gas_cost = GAS_COLD_ACCOUNT_ACCESS
 
-    delegated_code = get_account(state, delegated_address).code
+    return True, address, delegated_address, delegation_gas_cost
 
-    return (
-        True,
-        address,
-        delegated_address,
-        delegated_code,
-        additional_gas_cost,
-    )
 
-
-def apply_delegation_tracking(
-    evm: Evm, original_address: Address, delegated_address: Address
-) -> None:
+def read_delegation_target(evm: Evm, delegated_address: Address) -> Bytes:
     """
-    Apply delegation tracking after gas check passes.
+    Read the delegation target's code and track the access.
+
+    Should ONLY be called AFTER verifying we have gas for the access.
+
+    This function:
+    1. Reads the delegation target's code from state
+    2. Adds it to accessed_addresses (if not already there)
+    3. Tracks it in state_changes for BAL
 
     Parameters
     ----------
     evm : `Evm`
         The execution frame.
-    original_address : `Address`
-        The original address that was called.
     delegated_address : `Address`
-        The address delegated to.
+        The delegation target address.
+
+    Returns
+    -------
+    code : `Bytes`
+        The delegation target's code.
 
     """
-    track_address(evm.state_changes, original_address)
+    state = evm.message.block_env.state
 
+    # Add to accessed addresses for warm/cold gas accounting
     if delegated_address not in evm.accessed_addresses:
         evm.accessed_addresses.add(delegated_address)
 
+    # Track the address for BAL
     track_address(evm.state_changes, delegated_address)
 
-
-def access_delegation(
-    evm: Evm, address: Address
-) -> Tuple[bool, Address, Bytes, Uint]:
-    """
-    Access delegation info and track state changes.
-
-    DEPRECATED: Use check_delegation and apply_delegation_tracking
-    for proper gas check ordering.
-
-    """
-    is_delegated, orig_addr, final_addr, code, gas_cost = check_delegation(
-        evm, address
-    )
-
-    if is_delegated:
-        apply_delegation_tracking(evm, orig_addr, final_addr)
-
-    return is_delegated, final_addr, code, gas_cost
+    return get_account(state, delegated_address).code
 
 
 def set_delegation(message: Message) -> U256:

src/ethereum/forks/amsterdam/vm/instructions/system.py

@@ -34,8 +34,8 @@
     to_address_masked,
 )
 from ...vm.eoa_delegation import (
-    apply_delegation_tracking,
-    check_delegation,
+    calculate_delegation_cost,
+    read_delegation_target,
 )
 from .. import (
     Evm,
@@ -397,13 +397,36 @@ def call(evm: Evm) -> None:
     if is_cold_access:
         evm.accessed_addresses.add(to)
 
+    # Calculate base gas cost for accessing 'to' account
+    base_gas_cost = extend_memory.cost + access_gas_cost
+
+    # Check gas for base access BEFORE reading 'to' account
+    check_gas(evm, base_gas_cost)
+
+    # Now safe to read 'to' account's code to check for delegation
+    # This reads 'to' account's code and tracks it, but does NOT
+    # read the delegation target yet
     (
         is_delegated,
         original_address,
-        final_address,
-        code,
+        delegated_address,
         delegation_gas_cost,
-    ) = check_delegation(evm, to)
+    ) = calculate_delegation_cost(evm, to)
+
+    # Check gas for delegation target access before reading it
+    # Note: We haven't charged base_gas_cost yet (only checked it),
+    # so we need to verify we have TOTAL gas (base + delegation)
+    if is_delegated and delegation_gas_cost > Uint(0):
+        check_gas(evm, base_gas_cost + delegation_gas_cost)
+
+    if is_delegated:
+        assert delegated_address is not None
+        code = read_delegation_target(evm, delegated_address)
+        final_address = delegated_address
+    else:
+        code = get_account(evm.message.block_env.state, to).code
+        final_address = to
+
     access_gas_cost += delegation_gas_cost
 
     code_address = final_address
@@ -421,12 +444,6 @@ def call(evm: Evm) -> None:
         access_gas_cost + create_gas_cost + transfer_gas_cost,
     )
 
-    check_gas(evm, message_call_gas.cost + extend_memory.cost)
-
-    track_address(evm.state_changes, to)
-    if is_delegated:
-        apply_delegation_tracking(evm, original_address, final_address)
-
     charge_gas(evm, message_call_gas.cost + extend_memory.cost)
     if evm.message.is_static and value != U256(0):
         raise WriteInStaticContext
@@ -497,13 +514,33 @@ def callcode(evm: Evm) -> None:
     if is_cold_access:
         evm.accessed_addresses.add(code_address)
 
+    # Check we have enough gas for base access before reading state
+    base_gas_cost = extend_memory.cost + access_gas_cost
+    check_gas(evm, base_gas_cost)
+
+    # Check delegation cost without reading delegation target yet
     (
         is_delegated,
         original_address,
-        final_address,
-        code,
+        delegated_address,
         delegation_gas_cost,
-    ) = check_delegation(evm, code_address)
+    ) = calculate_delegation_cost(evm, code_address)
+
+    # Check gas for delegation target access BEFORE reading it
+    # Note: We haven't charged base_gas_cost yet (only checked it),
+    # so we need to verify we have TOTAL gas (base + delegation)
+    if is_delegated and delegation_gas_cost > Uint(0):
+        check_gas(evm, base_gas_cost + delegation_gas_cost)
+
+    # Now safe to read delegation target since we verified gas
+    if is_delegated:
+        assert delegated_address is not None
+        code = read_delegation_target(evm, delegated_address)
+        final_address = delegated_address
+    else:
+        code = get_account(evm.message.block_env.state, code_address).code
+        final_address = code_address
+
     access_gas_cost += delegation_gas_cost
 
     code_address = final_address
@@ -518,12 +555,6 @@ def callcode(evm: Evm) -> None:
         access_gas_cost + transfer_gas_cost,
     )
 
-    check_gas(evm, message_call_gas.cost + extend_memory.cost)
-
-    track_address(evm.state_changes, original_address)
-    if is_delegated:
-        apply_delegation_tracking(evm, original_address, final_address)
-
     charge_gas(evm, message_call_gas.cost + extend_memory.cost)
 
     # OPERATION
@@ -665,16 +696,37 @@ def delegatecall(evm: Evm) -> None:
     access_gas_cost = (
         GAS_COLD_ACCOUNT_ACCESS if is_cold_access else GAS_WARM_ACCESS
     )
+
+    # Check we have enough gas for base access before reading state
+    base_gas_cost = extend_memory.cost + access_gas_cost
+    check_gas(evm, base_gas_cost)
+
     if is_cold_access:
         evm.accessed_addresses.add(code_address)
 
+    # Check delegation cost without reading delegation target yet
     (
         is_delegated,
         original_address,
-        final_address,
-        code,
+        delegated_address,
         delegation_gas_cost,
-    ) = check_delegation(evm, code_address)
+    ) = calculate_delegation_cost(evm, code_address)
+
+    # Check gas for delegation target access BEFORE reading it
+    # Note: We haven't charged base_gas_cost yet (only checked it),
+    # so we need to verify we have TOTAL gas (base + delegation)
+    if is_delegated and delegation_gas_cost > Uint(0):
+        check_gas(evm, base_gas_cost + delegation_gas_cost)
+
+    # Now safe to read delegation target since we verified gas
+    if is_delegated:
+        assert delegated_address is not None
+        code = read_delegation_target(evm, delegated_address)
+        final_address = delegated_address
+    else:
+        code = get_account(evm.message.block_env.state, code_address).code
+        final_address = code_address
+
     access_gas_cost += delegation_gas_cost
 
     code_address = final_address
@@ -684,12 +736,6 @@ def delegatecall(evm: Evm) -> None:
         U256(0), gas, Uint(evm.gas_left), extend_memory.cost, access_gas_cost
     )
 
-    check_gas(evm, message_call_gas.cost + extend_memory.cost)
-
-    track_address(evm.state_changes, original_address)
-    if is_delegated:
-        apply_delegation_tracking(evm, original_address, final_address)
-
     charge_gas(evm, message_call_gas.cost + extend_memory.cost)
 
     # OPERATION
@@ -749,13 +795,33 @@ def staticcall(evm: Evm) -> None:
     if is_cold_access:
         evm.accessed_addresses.add(to)
 
+    # Check we have enough gas for base access before reading state
+    base_gas_cost = extend_memory.cost + access_gas_cost
+    check_gas(evm, base_gas_cost)
+
+    # Check delegation cost without reading delegation target yet
     (
         is_delegated,
         original_address,
-        final_address,
-        code,
+        delegated_address,
         delegation_gas_cost,
-    ) = check_delegation(evm, to)
+    ) = calculate_delegation_cost(evm, to)
+
+    # Check gas for delegation target access BEFORE reading it
+    # Note: We haven't charged base_gas_cost yet (only checked it),
+    # so we need to verify we have TOTAL gas (base + delegation)
+    if is_delegated and delegation_gas_cost > Uint(0):
+        check_gas(evm, base_gas_cost + delegation_gas_cost)
+
+    # Now safe to read delegation target since we verified gas
+    if is_delegated:
+        assert delegated_address is not None
+        code = read_delegation_target(evm, delegated_address)
+        final_address = delegated_address
+    else:
+        code = get_account(evm.message.block_env.state, to).code
+        final_address = to
+
     access_gas_cost += delegation_gas_cost
 
     code_address = final_address
@@ -769,12 +835,6 @@ def staticcall(evm: Evm) -> None:
         access_gas_cost,
     )
 
-    check_gas(evm, message_call_gas.cost + extend_memory.cost)
-
-    track_address(evm.state_changes, to)
-    if is_delegated:
-        apply_delegation_tracking(evm, original_address, final_address)
-
     charge_gas(evm, message_call_gas.cost + extend_memory.cost)
 
     # OPERATION

tests/prague/eip7702_set_code_tx/test_gas.py

@@ -18,6 +18,9 @@
     Address,
     Alloc,
     AuthorizationTuple,
+    BalAccountExpectation,
+    BalNonceChange,
+    BlockAccessListExpectation,
     Bytecode,
     Bytes,
     ChainConfig,
@@ -1269,6 +1272,25 @@ def test_call_to_pre_authorized_oog(
         sender=pre.fund_eoa(),
     )
 
+    expected_block_access_list = None
+    if fork.header_bal_hash_required():
+        # Sender nonce changes, callee is accessed but storage unchanged (OOG)
+        # auth_signer is tracked (we read its code to check delegation)
+        # delegation is NOT tracked (OOG before reading it)
+        account_expectations = {
+            tx.sender: BalAccountExpectation(
+                nonce_changes=[BalNonceChange(tx_index=1, post_nonce=1)],
+            ),
+            callee_address: BalAccountExpectation.empty(),
+            # read for calculating delegation access cost:
+            auth_signer: BalAccountExpectation.empty(),
+            # OOG - not enough gas for delegation access:
+            delegation: None,
+        }
+        expected_block_access_list = BlockAccessListExpectation(
+            account_expectations=account_expectations
+        )
+
     state_test(
         pre=pre,
         tx=tx,
@@ -1277,4 +1299,5 @@ def test_call_to_pre_authorized_oog(
             auth_signer: Account(code=Spec.delegation_designation(delegation)),
             delegation: Account(storage=Storage()),
         },
+        expected_block_access_list=expected_block_access_list,
     )