fix zero-value transfer tracking (#6)

* fix zero-value transfer tracking
* fix reverted frame tracking
* rename variables
* fix missing addresses bug
* fix: docs run & move imports to top of file

---------

Co-authored-by: fselmo <[email protected]>

Author: nerolation

src/ethereum/forks/amsterdam/block_access_lists/__init__.py

@@ -19,7 +19,10 @@
 )
 from .tracker import (
     StateChangeTracker,
-    set_transaction_index,
+    begin_call_frame,
+    commit_call_frame,
+    rollback_call_frame,
+    set_block_access_index,
     track_address_access,
     track_balance_change,
     track_code_change,
@@ -37,9 +40,12 @@
     "add_storage_read",
     "add_storage_write",
     "add_touched_account",
+    "begin_call_frame",
     "build_block_access_list",
+    "commit_call_frame",
     "compute_block_access_list_hash",
-    "set_transaction_index",
+    "rollback_call_frame",
+    "set_block_access_index",
     "rlp_encode_block_access_list",
     "track_address_access",
     "track_balance_change",

src/ethereum/forks/amsterdam/block_access_lists/tracker.py

@@ -16,7 +16,7 @@
 """
 
 from dataclasses import dataclass, field
-from typing import TYPE_CHECKING, Dict
+from typing import TYPE_CHECKING, Dict, List, Set, Tuple
 
 from ethereum_types.bytes import Bytes, Bytes32
 from ethereum_types.numeric import U64, U256, Uint
@@ -37,6 +37,39 @@
     from ..state import State  # noqa: F401
 
 
+@dataclass
+class CallFrameSnapshot:
+    """
+    Snapshot of block access list state for a single call frame.
+
+    Used to track changes within a call frame to enable proper handling
+    of reverts as specified in EIP-7928.
+    """
+
+    touched_addresses: Set[Address] = field(default_factory=set)
+    """Addresses touched during this call frame."""
+
+    storage_writes: Dict[Tuple[Address, Bytes32], U256] = field(
+        default_factory=dict
+    )
+    """Storage writes made during this call frame."""
+
+    balance_changes: Set[Tuple[Address, BlockAccessIndex, U256]] = field(
+        default_factory=set
+    )
+    """Balance changes made during this call frame."""
+
+    nonce_changes: Set[Tuple[Address, BlockAccessIndex, U64]] = field(
+        default_factory=set
+    )
+    """Nonce changes made during this call frame."""
+
+    code_changes: Set[Tuple[Address, BlockAccessIndex, Bytes]] = field(
+        default_factory=set
+    )
+    """Code changes made during this call frame."""
+
+
 @dataclass
 class StateChangeTracker:
     """
@@ -70,16 +103,25 @@ class StateChangeTracker:
     1..n for transactions, n+1 for post-execution).
     """
 
+    call_frame_snapshots: List[CallFrameSnapshot] = field(default_factory=list)
+    """
+    Stack of snapshots for nested call frames to handle reverts properly.
+    """
 
-def set_transaction_index(
+
+def set_block_access_index(
     tracker: StateChangeTracker, block_access_index: Uint
 ) -> None:
     """
     Set the current block access index for tracking changes.
 
     Must be called before processing each transaction/system contract
-    to ensure changes
-    are associated with the correct block access index.
+    to ensure changes are associated with the correct block access index.
+
+    Note: Block access indices differ from transaction indices:
+    - 0: Pre-execution (system contracts like beacon roots, block hashes)
+    - 1..n: Transactions (tx at index i gets block_access_index i+1)
+    - n+1: Post-execution (withdrawals, requests)
 
     Parameters
     ----------
@@ -221,6 +263,10 @@ def track_storage_write(
             BlockAccessIndex(tracker.current_block_access_index),
             value_bytes,
         )
+        # Record in current call frame snapshot if exists
+        if tracker.call_frame_snapshots:
+            snapshot = tracker.call_frame_snapshots[-1]
+            snapshot.storage_writes[(address, key)] = new_value
     else:
         add_storage_read(tracker.block_access_list_builder, address, key)
 
@@ -249,13 +295,21 @@ def track_balance_change(
     """
     track_address_access(tracker, address)
 
+    block_access_index = BlockAccessIndex(tracker.current_block_access_index)
     add_balance_change(
         tracker.block_access_list_builder,
         address,
-        BlockAccessIndex(tracker.current_block_access_index),
+        block_access_index,
         new_balance,
     )
 
+    # Record in current call frame snapshot if exists
+    if tracker.call_frame_snapshots:
+        snapshot = tracker.call_frame_snapshots[-1]
+        snapshot.balance_changes.add(
+            (address, block_access_index, new_balance)
+        )
+
 
 def track_nonce_change(
     tracker: StateChangeTracker, address: Address, new_nonce: Uint
@@ -282,13 +336,20 @@ def track_nonce_change(
     [`CREATE2`]: ref:ethereum.forks.amsterdam.vm.instructions.system.create2
     """
     track_address_access(tracker, address)
+    block_access_index = BlockAccessIndex(tracker.current_block_access_index)
+    nonce_u64 = U64(new_nonce)
     add_nonce_change(
         tracker.block_access_list_builder,
         address,
-        BlockAccessIndex(tracker.current_block_access_index),
-        U64(new_nonce),
+        block_access_index,
+        nonce_u64,
     )
 
+    # Record in current call frame snapshot if exists
+    if tracker.call_frame_snapshots:
+        snapshot = tracker.call_frame_snapshots[-1]
+        snapshot.nonce_changes.add((address, block_access_index, nonce_u64))
+
 
 def track_code_change(
     tracker: StateChangeTracker, address: Address, new_code: Bytes
@@ -313,13 +374,19 @@ def track_code_change(
     [`CREATE2`]: ref:ethereum.forks.amsterdam.vm.instructions.system.create2
     """
     track_address_access(tracker, address)
+    block_access_index = BlockAccessIndex(tracker.current_block_access_index)
     add_code_change(
         tracker.block_access_list_builder,
         address,
-        BlockAccessIndex(tracker.current_block_access_index),
+        block_access_index,
         new_code,
     )
 
+    # Record in current call frame snapshot if exists
+    if tracker.call_frame_snapshots:
+        snapshot = tracker.call_frame_snapshots[-1]
+        snapshot.code_changes.add((address, block_access_index, new_code))
+
 
 def finalize_transaction_changes(
     tracker: StateChangeTracker, state: "State"
@@ -339,3 +406,120 @@ def finalize_transaction_changes(
         The current execution state.
     """
     pass
+
+
+def begin_call_frame(tracker: StateChangeTracker) -> None:
+    """
+    Begin a new call frame for tracking reverts.
+
+    Creates a new snapshot to track changes within this call frame.
+    This allows proper handling of reverts as specified in EIP-7928.
+
+    Parameters
+    ----------
+    tracker :
+        The state change tracker instance.
+    """
+    tracker.call_frame_snapshots.append(CallFrameSnapshot())
+
+
+def rollback_call_frame(tracker: StateChangeTracker) -> None:
+    """
+    Rollback changes from the current call frame.
+
+    When a call reverts, this function:
+    - Converts storage writes to reads
+    - Removes balance, nonce, and code changes
+    - Preserves touched addresses
+
+    This implements EIP-7928 revert handling where reverted writes
+    become reads and addresses remain in the access list.
+
+    Parameters
+    ----------
+    tracker :
+        The state change tracker instance.
+    """
+    if not tracker.call_frame_snapshots:
+        return
+
+    snapshot = tracker.call_frame_snapshots.pop()
+    builder = tracker.block_access_list_builder
+
+    # Convert storage writes to reads
+    for (address, slot), _ in snapshot.storage_writes.items():
+        # Remove the write from storage_changes
+        if address in builder.accounts:
+            account_data = builder.accounts[address]
+            if slot in account_data.storage_changes:
+                # Filter out changes from this call frame
+                account_data.storage_changes[slot] = [
+                    change
+                    for change in account_data.storage_changes[slot]
+                    if change.block_access_index
+                    != tracker.current_block_access_index
+                ]
+                if not account_data.storage_changes[slot]:
+                    del account_data.storage_changes[slot]
+            # Add as a read instead
+            account_data.storage_reads.add(slot)
+
+    # Remove balance changes from this call frame
+    for address, block_access_index, new_balance in snapshot.balance_changes:
+        if address in builder.accounts:
+            account_data = builder.accounts[address]
+            # Filter out balance changes from this call frame
+            account_data.balance_changes = [
+                change
+                for change in account_data.balance_changes
+                if not (
+                    change.block_access_index == block_access_index
+                    and change.post_balance == new_balance
+                )
+            ]
+
+    # Remove nonce changes from this call frame
+    for address, block_access_index, new_nonce in snapshot.nonce_changes:
+        if address in builder.accounts:
+            account_data = builder.accounts[address]
+            # Filter out nonce changes from this call frame
+            account_data.nonce_changes = [
+                change
+                for change in account_data.nonce_changes
+                if not (
+                    change.block_access_index == block_access_index
+                    and change.new_nonce == new_nonce
+                )
+            ]
+
+    # Remove code changes from this call frame
+    for address, block_access_index, new_code in snapshot.code_changes:
+        if address in builder.accounts:
+            account_data = builder.accounts[address]
+            # Filter out code changes from this call frame
+            account_data.code_changes = [
+                change
+                for change in account_data.code_changes
+                if not (
+                    change.block_access_index == block_access_index
+                    and change.new_code == new_code
+                )
+            ]
+
+    # All touched addresses remain in the access list (already tracked)
+
+
+def commit_call_frame(tracker: StateChangeTracker) -> None:
+    """
+    Commit changes from the current call frame.
+
+    Removes the current call frame snapshot without rolling back changes.
+    Called when a call completes successfully.
+
+    Parameters
+    ----------
+    tracker :
+        The state change tracker instance.
+    """
+    if tracker.call_frame_snapshots:
+        tracker.call_frame_snapshots.pop()

src/ethereum/forks/amsterdam/fork.py

@@ -33,7 +33,7 @@
 from .block_access_lists.builder import build_block_access_list
 from .block_access_lists.rlp_utils import compute_block_access_list_hash
 from .block_access_lists.tracker import (
-    set_transaction_index,
+    set_block_access_index,
     track_balance_change,
 )
 from .blocks import Block, Header, Log, Receipt, Withdrawal, encode_receipt
@@ -764,9 +764,9 @@ def apply_body(
     """
     block_output = vm.BlockOutput()
 
-    # Set system transaction index for pre-execution system contracts
+    # Set block access index for pre-execution system contracts
     # EIP-7928: System contracts use block_access_index 0
-    set_transaction_index(block_env.state.change_tracker, Uint(0))
+    set_block_access_index(block_env.state.change_tracker, Uint(0))
 
     process_unchecked_system_transaction(
         block_env=block_env,
@@ -785,7 +785,9 @@ def apply_body(
 
     # EIP-7928: Post-execution uses block_access_index len(transactions) + 1
     post_execution_index = ulen(transactions) + Uint(1)
-    set_transaction_index(block_env.state.change_tracker, post_execution_index)
+    set_block_access_index(
+        block_env.state.change_tracker, post_execution_index
+    )
 
     process_withdrawals(block_env, block_output, withdrawals)
 
@@ -874,7 +876,8 @@ def process_transaction(
         Index of the transaction in the block.
     """
     # EIP-7928: Transactions use block_access_index 1 to len(transactions)
-    set_transaction_index(block_env.state.change_tracker, index + Uint(1))
+    # Transaction at index i gets block_access_index i+1
+    set_block_access_index(block_env.state.change_tracker, index + Uint(1))
 
     trie_set(
         block_output.transactions_trie,

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

@@ -30,6 +30,12 @@
     evm_trace,
 )
 
+from ..block_access_lists.tracker import (
+    begin_call_frame,
+    commit_call_frame,
+    rollback_call_frame,
+    track_address_access,
+)
 from ..blocks import Log
 from ..fork_types import Address
 from ..state import (
@@ -239,6 +245,11 @@ def process_message(message: Message) -> Evm:
     # take snapshot of state before processing the message
     begin_transaction(state, transient_storage)
 
+    if hasattr(state, 'change_tracker') and state.change_tracker:
+        begin_call_frame(state.change_tracker)
+        # Track target address access when processing a message
+        track_address_access(state.change_tracker, message.current_target)
+
     if message.should_transfer_value and message.value != 0:
         move_ether(
             state, message.caller, message.current_target, message.value
@@ -249,8 +260,12 @@ def process_message(message: Message) -> Evm:
         # revert state to the last saved checkpoint
         # since the message call resulted in an error
         rollback_transaction(state, transient_storage)
+        if hasattr(state, 'change_tracker') and state.change_tracker:
+            rollback_call_frame(state.change_tracker)
     else:
         commit_transaction(state, transient_storage)
+        if hasattr(state, 'change_tracker') and state.change_tracker:
+            commit_call_frame(state.change_tracker)
     return evm
 
 

src/ethereum_spec_tools/evm_tools/loaders/fork_loader.py

@@ -137,10 +137,10 @@ def compute_block_access_list_hash(self) -> Any:
         )
 
     @property
-    def set_transaction_index(self) -> Any:
-        """set_transaction_index function of the fork"""
+    def set_block_access_index(self) -> Any:
+        """set_block_access_index function of the fork"""
         return (
-            self._module("block_access_lists").set_transaction_index
+            self._module("block_access_lists").set_block_access_index
         )
 
     @property