fix: use engine_newPayloadV5 for Glamsterdam (#2170)

* fix: use engine_newPayloadV5 for Amsterdam

* fix: update exception mapper for geth

* fix: We should update BAL hash in fixture for invalid tests

* Clean up extraneous BAL exceptions

* refactor: use fork check for BAL in engine payload

Author: fselmo

docs/CHANGELOG.md

@@ -21,6 +21,7 @@ Test fixtures for use by clients are available for each release on the [Github r
 
 - ✨ Add tighter validation for EIP-7928 model coming from t8n when filling ([#2138](https://github.com/ethereum/execution-spec-tests/pull/2138)).
 - ✨ Add flexible API for absence checks for EIP-7928 (BAL) tests ([#2124](https://github.com/ethereum/execution-spec-tests/pull/2124)).
+- 🐞 Use ``engine_newPayloadV5`` for `>=Amsterdam` forks in `consume engine` ([#2170](https://github.com/ethereum/execution-spec-tests/pull/2170)).
 
 ### 🧪 Test Cases
 

src/ethereum_clis/clis/geth.py

@@ -71,6 +71,7 @@ class GethExceptionMapper(ExceptionMapper):
         TransactionException.TYPE_4_TX_PRE_FORK: ("transaction type not supported"),
         TransactionException.INITCODE_SIZE_EXCEEDED: "max initcode size exceeded",
         TransactionException.NONCE_MISMATCH_TOO_LOW: "nonce too low",
+        TransactionException.NONCE_MISMATCH_TOO_HIGH: "nonce too high",
         BlockException.INVALID_DEPOSIT_EVENT_LAYOUT: "unable to parse deposit data",
         BlockException.INCORRECT_BLOB_GAS_USED: "blob gas used mismatch",
         BlockException.INCORRECT_EXCESS_BLOB_GAS: "invalid excessBlobGas",
@@ -79,6 +80,9 @@ class GethExceptionMapper(ExceptionMapper):
         BlockException.SYSTEM_CONTRACT_CALL_FAILED: "system call failed to execute:",
         BlockException.INVALID_BLOCK_HASH: "blockhash mismatch",
         BlockException.RLP_BLOCK_LIMIT_EXCEEDED: "block RLP-encoded size exceeds maximum",
+        BlockException.INVALID_BAL_EXTRA_ACCOUNT: "BAL change not reported in computed",
+        BlockException.INVALID_BAL_MISSING_ACCOUNT: "additional mutations compared to BAL",
+        BlockException.INVALID_BLOCK_ACCESS_LIST: "unequal",
     }
     mapping_regex: ClassVar[Dict[ExceptionBase, str]] = {
         TransactionException.TYPE_3_TX_MAX_BLOB_GAS_ALLOWANCE_EXCEEDED: (

src/ethereum_test_exceptions/exceptions/block.py

@@ -216,3 +216,20 @@ class BlockException(ExceptionBase):
     Transaction emits a `DepositEvent` in the deposit contract (EIP-6110), but the layout
     of the event does not match the required layout.
     """
+    # --- Block-Level Access Lists (EIP-7928) --- #
+    INVALID_BLOCK_ACCESS_LIST = auto()
+    """
+    Block's access list is invalid.
+    """
+    INVALID_BAL_HASH = auto()
+    """
+    Block header's BAL hash does not match the computed BAL hash.
+    """
+    INVALID_BAL_EXTRA_ACCOUNT = auto()
+    """
+    Block BAL contains an account change that is not present in the computed BAL.
+    """
+    INVALID_BAL_MISSING_ACCOUNT = auto()
+    """
+    Block BAL is missing an account change that is present in the computed BAL.
+    """

src/ethereum_test_fixtures/blockchain.py

@@ -301,10 +301,12 @@ def from_fixture_header(
     Hash,
     List[Bytes],
 ]
+EngineNewPayloadV5Parameters = EngineNewPayloadV4Parameters
 
 # Important: We check EngineNewPayloadV3Parameters first as it has more fields, and pydantic
 # has a weird behavior when the smaller tuple is checked first.
 EngineNewPayloadParameters = Union[
+    EngineNewPayloadV5Parameters,
     EngineNewPayloadV4Parameters,
     EngineNewPayloadV3Parameters,
     EngineNewPayloadV1Parameters,
@@ -354,6 +356,13 @@ def from_fixture_header(
         )
 
         assert new_payload_version is not None, "Invalid header for engine_newPayload"
+
+        if fork.engine_execution_payload_block_access_list(header.number, header.timestamp):
+            if block_access_list is None:
+                raise ValueError(
+                    f"`block_access_list` is required in engine `ExecutionPayload` for >={fork}."
+                )
+
         execution_payload = FixtureExecutionPayload.from_fixture_header(
             header=header,
             transactions=transactions,

src/ethereum_test_forks/base_fork.py

@@ -553,6 +553,17 @@ def engine_new_payload_target_blobs_per_block(
         """
         pass
 
+    @classmethod
+    @abstractmethod
+    def engine_execution_payload_block_access_list(
+        cls, block_number: int = 0, timestamp: int = 0
+    ) -> bool:
+        """
+        Return `True` if the engine api version requires execution payload to include a
+        `block_access_list`.
+        """
+        pass
+
     @classmethod
     @abstractmethod
     def engine_payload_attribute_target_blobs_per_block(

src/ethereum_test_forks/forks/forks.py

@@ -347,6 +347,13 @@ def engine_new_payload_requests(cls, block_number: int = 0, timestamp: int = 0)
         """At genesis, payloads do not have requests."""
         return False
 
+    @classmethod
+    def engine_execution_payload_block_access_list(
+        cls, block_number: int = 0, timestamp: int = 0
+    ) -> bool:
+        """At genesis, payloads do not have block access list."""
+        return False
+
     @classmethod
     def engine_new_payload_target_blobs_per_block(
         cls,
@@ -1836,6 +1843,23 @@ def is_deployed(cls) -> bool:
         """Return True if this fork is deployed."""
         return False
 
+    @classmethod
+    def engine_new_payload_version(
+        cls, block_number: int = 0, timestamp: int = 0
+    ) -> Optional[int]:
+        """From Amsterdam, new payload calls must use version 5."""
+        return 5
+
+    @classmethod
+    def engine_execution_payload_block_access_list(
+        cls, block_number: int = 0, timestamp: int = 0
+    ) -> bool:
+        """
+        From Amsterdam, engine execution payload includes `block_access_list` as
+        a parameter.
+        """
+        return True
+
 
 class EOFv1(Prague, solc_name="cancun"):
     """EOF fork."""

src/ethereum_test_specs/blockchain.py

@@ -629,12 +629,15 @@ def generate_block_data(
             header.fork = fork  # Deleted during `apply` because `exclude=True`
 
         # Process block access list - apply transformer if present for invalid tests
-        bal = transition_tool_output.result.block_access_list
-        if block.expected_block_access_list is not None and bal is not None:
-            # Use to_fixture_bal to validate and potentially transform the BAL
-            bal = block.expected_block_access_list.to_fixture_bal(bal)
-            # Don't update the header hash - leave it as the hash of the correct BAL
-            # This creates a mismatch that should cause the block to be rejected
+        t8n_bal = transition_tool_output.result.block_access_list
+        bal = t8n_bal
+        if block.expected_block_access_list is not None and t8n_bal is not None:
+            block.expected_block_access_list.verify_against(t8n_bal)
+
+            bal = block.expected_block_access_list.modify_if_invalid_test(t8n_bal)
+            if bal != t8n_bal:
+                # If the BAL was modified, update the header hash
+                header.block_access_list_hash = Hash(bal.rlp.keccak256())
 
         built_block = BuiltBlock(
             header=header,

src/ethereum_test_types/block_access_list/__init__.py

@@ -256,12 +256,9 @@ def modify(
         new_instance._modifier = compose(*modifiers)
         return new_instance
 
-    def to_fixture_bal(self, t8n_bal: "BlockAccessList") -> "BlockAccessList":
+    def modify_if_invalid_test(self, t8n_bal: "BlockAccessList") -> "BlockAccessList":
         """
-        Convert t8n BAL to fixture BAL, optionally applying transformations.
-
-        1. First validates expectations are met (if any)
-        2. Then applies modifier if specified (for invalid tests)
+        Apply the modifier to the given BAL if this is an invalid test case.
 
         Args:
             t8n_bal: The BlockAccessList from t8n tool
@@ -270,13 +267,8 @@ def to_fixture_bal(self, t8n_bal: "BlockAccessList") -> "BlockAccessList":
             The potentially transformed BlockAccessList for the fixture
 
         """
-        if self.account_expectations:
-            self.verify_against(t8n_bal)
-
-        # Apply modifier if present (for invalid tests)
         if self._modifier:
             return self._modifier(t8n_bal)
-
         return t8n_bal
 
     def verify_against(self, actual_bal: "BlockAccessList") -> None:

tests/amsterdam/eip7928_block_level_access_lists/test_block_access_lists_invalid.py

@@ -72,7 +72,7 @@ def test_bal_invalid_missing_nonce(
         blocks=[
             Block(
                 txs=[tx],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BLOCK_ACCESS_LIST,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         sender: BalAccountExpectation(
@@ -111,7 +111,7 @@ def test_bal_invalid_nonce_value(
         blocks=[
             Block(
                 txs=[tx],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BLOCK_ACCESS_LIST,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         sender: BalAccountExpectation(
@@ -155,7 +155,7 @@ def test_bal_invalid_storage_value(
         blocks=[
             Block(
                 txs=[tx],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BLOCK_ACCESS_LIST,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         contract: BalAccountExpectation(
@@ -219,7 +219,7 @@ def test_bal_invalid_tx_order(
         blocks=[
             Block(
                 txs=[tx1, tx2],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BLOCK_ACCESS_LIST,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         sender1: BalAccountExpectation(
@@ -269,7 +269,7 @@ def test_bal_invalid_account(
         blocks=[
             Block(
                 txs=[tx],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BAL_EXTRA_ACCOUNT,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         sender: BalAccountExpectation(
@@ -412,7 +412,7 @@ def test_bal_invalid_complex_corruption(
         blocks=[
             Block(
                 txs=[tx1, tx2],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BLOCK_ACCESS_LIST,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         sender: BalAccountExpectation(
@@ -474,7 +474,7 @@ def test_bal_invalid_missing_account(
         blocks=[
             Block(
                 txs=[tx],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BAL_MISSING_ACCOUNT,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         sender: BalAccountExpectation(
@@ -516,7 +516,7 @@ def test_bal_invalid_balance_value(
         blocks=[
             Block(
                 txs=[tx],
-                exception=BlockException.INCORRECT_BLOCK_FORMAT,
+                exception=BlockException.INVALID_BLOCK_ACCESS_LIST,
                 expected_block_access_list=BlockAccessListExpectation(
                     account_expectations={
                         receiver: BalAccountExpectation(