Invalid block if system contract is empty on call or call fails (#1183)

* Invalid block if system contract is empty on call or call fails

* Improve fixture fetching

Hoists the file system locks to the session level (instead of only while
fetching) so the whole fixtures folder is protected while the tests run.
Previously, running a second instance of the tests could swap out the
fixtures while the first instance is using them.

Re-download and re-extract HTTP tarball fixtures for every pytest
session. Previously it was assumed that if the directory exists, it was
the correct version of the fixtures. Now the fixtures are guaranteed to
be up-to-date.

HTTP responses are now cached in the user's home directory to avoid
re-downloading from GitHub when possible. The cache gets cleaned during
each pytest session, so it isn't particularly effective when changing
versions, but at least runs with the same version don't waste bandwidth.

* Ignore more coverage files

* Update to more recent ethereum/tests

---------

Co-authored-by: Sam Wilson <[email protected]>

Author: marioevz

.gitignore

@@ -50,6 +50,7 @@ pip-delete-this-directory.txt
 /docs
 
 .coverage
+.coverage.*
 
 /doc/diffs
 /doc/autoapi

setup.cfg

@@ -171,6 +171,7 @@ test =
     GitPython>=3.1.0,<3.2
     filelock>=3.12.3,<3.13
     requests
+    requests-cache>=1.2.1,<2
 
 lint =
     types-setuptools>=68.1.0.1,<69

src/ethereum/cancun/fork.py

@@ -482,17 +482,23 @@ def make_receipt(
 def process_system_transaction(
     block_env: vm.BlockEnvironment,
     target_address: Address,
+    system_contract_code: Bytes,
     data: Bytes,
 ) -> MessageCallOutput:
     """
-    Process a system transaction.
+    Process a system transaction with the given code.
+
+    Prefer calling `process_unchecked_system_transaction` unless the contract
+    code has already been read from the state.
 
     Parameters
     ----------
     block_env :
         The block scoped environment.
     target_address :
         Address of the contract to call.
+    system_contract_code :
+        Code of the contract to call.
     data :
         Data to pass to the contract.
 
@@ -501,8 +507,6 @@ def process_system_transaction(
     system_tx_output : `MessageCallOutput`
         Output of processing the system transaction.
     """
-    system_contract_code = get_account(block_env.state, target_address).code
-
     tx_env = vm.TransactionEnvironment(
         origin=SYSTEM_ADDRESS,
         gas_price=block_env.base_fee_per_gas,
@@ -540,6 +544,38 @@ def process_system_transaction(
     return system_tx_output
 
 
+def process_unchecked_system_transaction(
+    block_env: vm.BlockEnvironment,
+    target_address: Address,
+    data: Bytes,
+) -> MessageCallOutput:
+    """
+    Process a system transaction without checking if the contract contains code
+    or if the transaction fails.
+
+    Parameters
+    ----------
+    block_env :
+        The block scoped environment.
+    target_address :
+        Address of the contract to call.
+    data :
+        Data to pass to the contract.
+
+    Returns
+    -------
+    system_tx_output : `MessageCallOutput`
+        Output of processing the system transaction.
+    """
+    system_contract_code = get_account(block_env.state, target_address).code
+    return process_system_transaction(
+        block_env,
+        target_address,
+        system_contract_code,
+        data,
+    )
+
+
 def apply_body(
     block_env: vm.BlockEnvironment,
     transactions: Tuple[Union[LegacyTransaction, Bytes], ...],
@@ -571,7 +607,7 @@ def apply_body(
     """
     block_output = vm.BlockOutput()
 
-    process_system_transaction(
+    process_unchecked_system_transaction(
         block_env=block_env,
         target_address=BEACON_ROOTS_ADDRESS,
         data=block_env.parent_beacon_block_root,

src/ethereum/prague/fork.py

@@ -514,17 +514,25 @@ def make_receipt(
 def process_system_transaction(
     block_env: vm.BlockEnvironment,
     target_address: Address,
+    system_contract_code: Bytes,
     data: Bytes,
 ) -> MessageCallOutput:
     """
-    Process a system transaction.
+    Process a system transaction with the given code.
+
+    Prefer calling `process_checked_system_transaction` or
+    `process_unchecked_system_transaction` depending on
+    whether missing code or an execution error should cause
+    the block to be rejected.
 
     Parameters
     ----------
     block_env :
         The block scoped environment.
     target_address :
         Address of the contract to call.
+    system_contract_code :
+        Code of the contract to call.
     data :
         Data to pass to the contract.
 
@@ -533,8 +541,6 @@ def process_system_transaction(
     system_tx_output : `MessageCallOutput`
         Output of processing the system transaction.
     """
-    system_contract_code = get_account(block_env.state, target_address).code
-
     tx_env = vm.TransactionEnvironment(
         origin=SYSTEM_ADDRESS,
         gas_price=block_env.base_fee_per_gas,
@@ -574,6 +580,85 @@ def process_system_transaction(
     return system_tx_output
 
 
+def process_checked_system_transaction(
+    block_env: vm.BlockEnvironment,
+    target_address: Address,
+    data: Bytes,
+) -> MessageCallOutput:
+    """
+    Process a system transaction and raise an error if the contract does not
+    contain code or if the transaction fails.
+
+    Parameters
+    ----------
+    block_env :
+        The block scoped environment.
+    target_address :
+        Address of the contract to call.
+    data :
+        Data to pass to the contract.
+
+    Returns
+    -------
+    system_tx_output : `MessageCallOutput`
+        Output of processing the system transaction.
+    """
+    system_contract_code = get_account(block_env.state, target_address).code
+
+    if len(system_contract_code) == 0:
+        raise InvalidBlock(
+            f"System contract address {target_address.hex()} does not "
+            "contain code"
+        )
+
+    system_tx_output = process_system_transaction(
+        block_env,
+        target_address,
+        system_contract_code,
+        data,
+    )
+
+    if system_tx_output.error:
+        raise InvalidBlock(
+            f"System contract ({target_address.hex()}) call failed: "
+            f"{system_tx_output.error}"
+        )
+
+    return system_tx_output
+
+
+def process_unchecked_system_transaction(
+    block_env: vm.BlockEnvironment,
+    target_address: Address,
+    data: Bytes,
+) -> MessageCallOutput:
+    """
+    Process a system transaction without checking if the contract contains code
+    or if the transaction fails.
+
+    Parameters
+    ----------
+    block_env :
+        The block scoped environment.
+    target_address :
+        Address of the contract to call.
+    data :
+        Data to pass to the contract.
+
+    Returns
+    -------
+    system_tx_output : `MessageCallOutput`
+        Output of processing the system transaction.
+    """
+    system_contract_code = get_account(block_env.state, target_address).code
+    return process_system_transaction(
+        block_env,
+        target_address,
+        system_contract_code,
+        data,
+    )
+
+
 def apply_body(
     block_env: vm.BlockEnvironment,
     transactions: Tuple[Union[LegacyTransaction, Bytes], ...],
@@ -605,13 +690,13 @@ def apply_body(
     """
     block_output = vm.BlockOutput()
 
-    process_system_transaction(
+    process_unchecked_system_transaction(
         block_env=block_env,
         target_address=BEACON_ROOTS_ADDRESS,
         data=block_env.parent_beacon_block_root,
     )
 
-    process_system_transaction(
+    process_unchecked_system_transaction(
         block_env=block_env,
         target_address=HISTORY_STORAGE_ADDRESS,
         data=block_env.block_hashes[-1],  # The parent hash
@@ -650,7 +735,7 @@ def process_general_purpose_requests(
     if len(deposit_requests) > 0:
         requests_from_execution.append(DEPOSIT_REQUEST_TYPE + deposit_requests)
 
-    system_withdrawal_tx_output = process_system_transaction(
+    system_withdrawal_tx_output = process_checked_system_transaction(
         block_env=block_env,
         target_address=WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS,
         data=b"",
@@ -661,7 +746,7 @@ def process_general_purpose_requests(
             WITHDRAWAL_REQUEST_TYPE + system_withdrawal_tx_output.return_data
         )
 
-    system_consolidation_tx_output = process_system_transaction(
+    system_consolidation_tx_output = process_checked_system_transaction(
         block_env=block_env,
         target_address=CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS,
         data=b"",

src/ethereum_spec_tools/evm_tools/loaders/fork_loader.py

@@ -63,9 +63,9 @@ def process_general_purpose_requests(self) -> Any:
         return self._module("fork").process_general_purpose_requests
 
     @property
-    def process_system_transaction(self) -> Any:
-        """process_system_transaction function of the given fork."""
-        return self._module("fork").process_system_transaction
+    def process_unchecked_system_transaction(self) -> Any:
+        """process_unchecked_system_transaction function of the given fork."""
+        return self._module("fork").process_unchecked_system_transaction
 
     @property
     def process_withdrawals(self) -> Any:

src/ethereum_spec_tools/evm_tools/t8n/__init__.py

@@ -203,14 +203,14 @@ def run_state_test(self) -> Any:
 
     def _run_blockchain_test(self, block_env: Any, block_output: Any) -> None:
         if self.fork.is_after_fork("ethereum.prague"):
-            self.fork.process_system_transaction(
+            self.fork.process_unchecked_system_transaction(
                 block_env=block_env,
                 target_address=self.fork.HISTORY_STORAGE_ADDRESS,
                 data=block_env.block_hashes[-1],  # The parent hash
             )
 
         if self.fork.is_after_fork("ethereum.cancun"):
-            self.fork.process_system_transaction(
+            self.fork.process_unchecked_system_transaction(
                 block_env=block_env,
                 target_address=self.fork.BEACON_ROOTS_ADDRESS,
                 data=block_env.parent_beacon_block_root,

tests/cancun/test_evm_tools.py

@@ -15,11 +15,9 @@
 FORK_NAME = "Cancun"
 
 SLOW_TESTS = (
-    "CALLBlake2f_MaxRounds",
-    "CALLCODEBlake2f",
-    "CALLBlake2f",
-    "loopExp",
-    "loopMul",
+    "GeneralStateTests/stTimeConsuming/CALLBlake2f_MaxRounds.json::CALLBlake2f_MaxRounds-fork_[Cancun-Prague]-d0g0v0",
+    "GeneralStateTests/VMTests/vmPerformance/loopExp.json::loopExp-fork_[Cancun-Prague]-d[0-14]g0v0",
+    "GeneralStateTests/VMTests/vmPerformance/loopMul.json::loopMul-fork_[Cancun-Prague]-d[0-2]g0v0",
 )