Address comments from PR ethereum#2067

Author: fselmo

src/ethereum_clis/types.py

@@ -63,8 +63,8 @@ class Result(CamelModel):
     blob_gas_used: HexNumber | None = None
     requests_hash: Hash | None = None
     requests: List[Bytes] | None = None
-    block_access_list: BlockAccessList | None = Field(None, alias="blockAccessList")
-    block_access_list_hash: Hash | None = Field(None, alias="blockAccessListHash")
+    block_access_list: BlockAccessList | None
+    block_access_list_hash: Hash | None
     block_exception: Annotated[
         BlockExceptionWithMessage | UndefinedException | None, ExceptionMapperValidator
     ] = None

src/ethereum_test_fixtures/blockchain.py

@@ -234,8 +234,7 @@ def genesis(cls, fork: Fork, env: Environment, state_root: Hash) -> "FixtureHead
         extras = {
             "state_root": state_root,
             "requests_hash": Requests() if fork.header_requests_required(0, 0) else None,
-            # TODO: How should we handle the genesis block access list? Is `Hash(0)` fine?
-            "block_access_list_hash": Hash(0) if fork.header_bal_hash_required(0, 0) else None,
+            "block_access_list_hash": Hash([]) if fork.header_bal_hash_required(0, 0) else None,
             "fork": fork,
         }
         return FixtureHeader(**environment_values, **extras)
@@ -267,6 +266,10 @@ class FixtureExecutionPayload(CamelModel):
     transactions: List[Bytes]
     withdrawals: List[Withdrawal] | None = None
 
+    block_access_list: Bytes | None = Field(
+        None, description="RLP-serialized EIP-7928 Block Access List"
+    )
+
     @classmethod
     def from_fixture_header(
         cls,

src/ethereum_test_forks/forks/forks.py

@@ -1819,7 +1819,7 @@ class Amsterdam(Osaka):
     @classmethod
     def header_bal_hash_required(cls, block_number: int = 0, timestamp: int = 0) -> bool:
         """From Amsterdam, header must contain block access list hash (EIP-7928)."""
-        return block_number > 0  # Not required in genesis block
+        return True
 
     @classmethod
     def is_deployed(cls) -> bool:

src/ethereum_test_specs/blockchain.py

@@ -430,12 +430,6 @@ class BlockchainTest(BaseTest):
     Exclude the post state from the fixture output.
     In this case, the state verification is only performed based on the state root.
     """
-    expected_block_access_list: BlockAccessListExpectation | None = None
-    """
-    Expected block access list for verification.
-    If set, verifies that the block access list returned by the client matches expectations.
-    Use BlockAccessListExpectation to define partial validation expectations.
-    """
 
     supported_fixture_formats: ClassVar[Sequence[FixtureFormat | LabeledFixtureFormat]] = [
         BlockchainFixture,
@@ -612,15 +606,18 @@ def generate_block_data(
             requests_list = block.requests
 
         if fork.header_bal_hash_required(header.number, header.timestamp):
-            if transition_tool_output.result.block_access_list is not None:
-                rlp = transition_tool_output.result.block_access_list.rlp()
-                computed_bal_hash = Hash(rlp.keccak256())
-                if computed_bal_hash != header.block_access_list_hash:
-                    raise Exception(
-                        "Block access list hash in header does not match the "
-                        f"computed hash from BAL: {header.block_access_list_hash} "
-                        f"!= {computed_bal_hash}"
-                    )
+            assert transition_tool_output.result.block_access_list is not None, (
+                "Block access list is required for this block but was not provided "
+                "by the transition tool"
+            )
+
+            rlp = transition_tool_output.result.block_access_list.rlp()
+            computed_bal_hash = Hash(rlp.keccak256())
+            assert computed_bal_hash == header.block_access_list_hash, (
+                "Block access list hash in header does not match the "
+                f"computed hash from BAL: {header.block_access_list_hash} "
+                f"!= {computed_bal_hash}"
+            )
 
         if block.rlp_modifier is not None:
             # Modify any parameter specified in the `rlp_modifier` after
@@ -708,28 +705,6 @@ def verify_post_state(self, t8n, t8n_state: Alloc, expected_state: Alloc | None
             print_traces(t8n.get_traces())
             raise e
 
-    def verify_block_access_list(
-        self, actual_bal: BlockAccessList | None, expected_bal: BlockAccessListExpectation | None
-    ):
-        """
-        Verify that the actual block access list matches expectations.
-
-        Args:
-            actual_bal: The BlockAccessList returned by the client
-            expected_bal: The expected BlockAccessList object
-
-        """
-        if expected_bal is None:
-            return
-
-        if actual_bal is None:
-            raise Exception("Expected block access list but got none.")
-
-        try:
-            expected_bal.verify_against(actual_bal)
-        except Exception as e:
-            raise Exception("Block access list verification failed.") from e
-
     def make_fixture(
         self,
         t8n: TransitionTool,

src/ethereum_test_types/block_access_list/__init__.py

@@ -5,7 +5,7 @@
 these are simple data classes that can be composed together.
 """
 
-from typing import Any, Callable, ClassVar, List, Optional
+from typing import Any, Callable, ClassVar, List
 
 import ethereum_rlp as eth_rlp
 from pydantic import Field
@@ -169,7 +169,7 @@ class BlockAccessListExpectation(CamelModel):
         default_factory=list, description="Expected account changes to verify"
     )
 
-    modifier: Optional[Callable[["BlockAccessList"], "BlockAccessList"]] = Field(
+    modifier: Callable[["BlockAccessList"], "BlockAccessList"] | None = Field(
         None,
         exclude=True,
         description="Optional modifier to modify the BAL for invalid tests",
@@ -281,10 +281,6 @@ def _compare_account_changes(
                     )
                 continue
 
-            # If actual is ``None`` but we expected something, raise
-            if actual_value is None:
-                raise AssertionError(f"Expected {field_name} but found none")
-
             if field_name == "storage_reads":
                 # Convert to comparable format (both are lists of 32-byte values)
                 expected_set = {bytes(v) if hasattr(v, "__bytes__") else v for v in expected_value}

tests/amsterdam/eip7928_block_level_access_lists/spec.py

@@ -21,8 +21,8 @@ class ReferenceSpec:
 class Spec:
     """Constants and parameters from EIP-7928."""
 
-    # SSZ encoding is used for block access list data structures
-    BAL_ENCODING_FORMAT: str = "SSZ"
+    # RLP encoding is used for block access list data structures
+    BAL_ENCODING_FORMAT: str = "RLP"
 
     # Maximum limits for block access list data structures
     TARGET_MAX_GAS_LIMIT = 600_000_000