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

marioevz · 2025-04-04T18:27:30Z

What was wrong?

EIPs 7002 and 7251 have been updated to define the expected outcome in case of execution failure or code absence:

How was it fixed?

Conditions for zero-length code and execution failure have been added to the system contract calls.

I'm not sure if this change affects EIP-2935, although I think we should not cherry-pick which system-contracts invalidate a block and which don't.

Cute Animal Picture

gurukamath · 2025-04-07T09:22:36Z

Consider re-directing this PR at the forks/prague branch instead. That has the latest spec updates including the latest re-factors from Prague.

marioevz · 2025-04-14T17:38:45Z

Might need to take another look at this since it might modify behavior of prior system contracts, but according to this message https://discord.com/channels/595666850260713488/1351309992837251195/1361391543595962669, that should not be the case.

marioevz · 2025-04-16T21:15:44Z

Ready to review, it's now independent of #1182 since I've removed the commits from that branch, although it will have a conflict because it touches the same code as that PR.

SamWilsn · 2025-04-22T19:01:09Z

src/ethereum/cancun/fork.py

+    raise_on_empty_code: bool,
+    raise_on_error: bool,


Something feels really weird about this, but I can't quite put my finger on it.

If we were going heavily into traditional exceptions, I'd expect to see a pattern like:

try: process_system_transaction(block_env, target_address, data) except SystemContractEmpty: pass except SystemContractRevert: pass

But we've taken the approach of minimizing our use of exceptions as much as possible to keep the code flow extremely readable.

Conditionally throwing exceptions seems to violate traditional exception patterns and our own minimal exception style.

Instead, what if we did something like:

def process_checked_system_transaction( block_env: vm.BlockEnvironment, target_address: Address, data: Bytes, ) -> MessageCallOutput: 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}" ) def process_unchecked_system_transaction( block_env: vm.BlockEnvironment, target_address: Address, data: Bytes, ) -> MessageCallOutput: system_contract_code = get_account(block_env.state, target_address).code return process_system_transaction( block_env, target_address, system_contract_code, data, )

I like it! thanks for the suggestion, I'll work on the modifications 👍

marioevz · 2025-04-23T00:23:17Z

@SamWilsn applied the changes, let me know what you think.

SamWilsn · 2025-04-24T17:39:00Z

It's possible I broke something while rebasing, but I'm getting this error in the tests:

ethereum.exceptions.InvalidBlock: System contract address 00000961ef480eb55e80d19ad83579a64c007002 does not contain code

From test:

tests/prague/test_state_transition.py::test_ethereum_tests[stEIP1153-transientStorage - 10_revertUndoesStoreAfterReturn_d0g0v0_Prague]

marioevz · 2025-04-24T18:47:40Z

It's possible I broke something while rebasing, but I'm getting this error in the tests:

ethereum.exceptions.InvalidBlock: System contract address 00000961ef480eb55e80d19ad83579a64c007002 does not contain code

From test:

tests/prague/test_state_transition.py::test_ethereum_tests[stEIP1153-transientStorage - 10_revertUndoesStoreAfterReturn_d0g0v0_Prague]

Interesting, perhaps we broke state tests parsing/running with this change? Because I think this check should be skipped on state tests.

marioevz · 2025-04-24T18:53:40Z

@SamWilsn wait, I'm confused, is this failure happening on the execution of a state test or blockchain test?

If it's on a blockchain test then it's expected because ethereum/tests I believe don't add the Prague system contracts to the blockchain tests.

SamWilsn · 2025-04-25T04:29:03Z

I would guess this is a blockchain test:

execution-specs/tests/prague/test_state_transition.py

Lines 87 to 94 in b1edd6b

    
           # Run tests from ethereum/tests 
        
           @pytest.mark.parametrize( 
        
               "test_case", 
        
               fetch_tests(ETHEREUM_BLOCKCHAIN_TESTS_DIR), 
        
               ids=idfn, 
        
           ) 
        
           def test_ethereum_tests(test_case: Dict) -> None: 
        
               run_tests(test_case)

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.

gurukamath · 2025-04-07T09:15:32Z

src/ethereum/prague/fork.py

@@ -573,6 +573,10 @@ def process_system_transaction(
        Output of processing the system transaction.
    """
    system_contract_code = get_account(state, target_address).code
+    if len(system_contract_code) == 0:


Introducing these checks here will also apply these checks to the History storage contract from EIP-2935 and the parent beacon block root contract from cancun. If that is not the intent, we should apply the checks more selectively to the the relevant contracts

This is taken care of with process_unchecked_system_transaction

src/ethereum/prague/fork.py

gurukamath · 2025-05-14T09:23:04Z

tests/cancun/test_evm_tools.py

-    "loopExp",
-    "loopMul",
+    "GeneralStateTests/stTimeConsuming/CALLBlake2f_MaxRounds.json::CALLBlake2f_MaxRounds-fork_[Cancun-Prague]-d0g0v0",
+    "GeneralStateTests/stTimeConsuming/CALLBlake2f_MaxRounds.json::CALLBlake2f_MaxRounds-fork_[Cancun-Prague]-d0g0v0",


These two entries seem to be the same

gurukamath · 2025-05-14T11:35:27Z

tests/helpers/__init__.py

    "evm_tools_testdata": {
        "url": "https://github.com/gurukamath/evm-tools-testdata.git",
        "commit_hash": "792422d",
        "fixture_path": "tests/fixtures/evm_tools_testdata",
    },
    "ethereum_tests": {
        "url": "https://github.com/ethereum/tests.git",
-        "commit_hash": "afed83b",
+        "commit_hash": "07f85aafbf2966b4db2b7d9cdf55248f91661ef9",


tox -e py3 ends up throwing stderr: 'fatal: reference is not a tree: 07f85aafbf2966b4db2b7d9cdf55248f91661ef9' on my local

* 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]>

marioevz force-pushed the invalid-block-on-missing-system-contracts branch from bea2827 to 739829b Compare April 15, 2025 12:47

marioevz changed the base branch from devnets/prague/6 to forks/prague April 15, 2025 12:48

marioevz force-pushed the invalid-block-on-missing-system-contracts branch 2 times, most recently from 28fd22f to c159057 Compare April 16, 2025 21:14

marioevz mentioned this pull request Apr 16, 2025

refactor(tests): EIP-7002, EIP-7251: Test block invalidation if system contract is missing ethereum/execution-spec-tests#1394

Merged

8 tasks

SamWilsn reviewed Apr 22, 2025

View reviewed changes

marioevz force-pushed the invalid-block-on-missing-system-contracts branch from c159057 to 2cce276 Compare April 23, 2025 00:21

SamWilsn force-pushed the invalid-block-on-missing-system-contracts branch 2 times, most recently from 6a8d3eb to a7fc1fe Compare April 24, 2025 15:00

marioevz and others added 2 commits May 6, 2025 14:58

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

f001c8d

SamWilsn force-pushed the invalid-block-on-missing-system-contracts branch from 073ba3c to d168673 Compare May 6, 2025 19:00

Ignore more coverage files

7da7f87

gurukamath reviewed May 14, 2025

View reviewed changes

SamWilsn force-pushed the invalid-block-on-missing-system-contracts branch 2 times, most recently from 620c0ed to ca5f0ea Compare May 15, 2025 04:49

Update to more recent ethereum/tests

3e5358e

SamWilsn force-pushed the invalid-block-on-missing-system-contracts branch from ca5f0ea to 3e5358e Compare May 15, 2025 04:49

gurukamath approved these changes May 15, 2025

View reviewed changes

gurukamath merged commit 3a0eb89 into ethereum:forks/prague May 15, 2025
3 of 5 checks passed

gurukamath mentioned this pull request Jun 5, 2025

download_fixtures() in tests/conftest.py does not check if pre-existing downloaded fixtures are stale #1132

Closed

This was referenced Jun 5, 2025

download_fixtures function does not check for the fixtures version #728

Closed

Implement check for fixtures version #1166

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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 #1183

marioevz commented Apr 4, 2025

Uh oh!

gurukamath commented Apr 7, 2025

Uh oh!

marioevz commented Apr 14, 2025

Uh oh!

marioevz commented Apr 16, 2025

Uh oh!

SamWilsn Apr 22, 2025

Uh oh!

marioevz Apr 23, 2025

Uh oh!

marioevz commented Apr 23, 2025

Uh oh!

SamWilsn commented Apr 24, 2025

Uh oh!

marioevz commented Apr 24, 2025

Uh oh!

marioevz commented Apr 24, 2025

Uh oh!

SamWilsn commented Apr 25, 2025

Uh oh!

gurukamath Apr 7, 2025

Uh oh!

SamWilsn May 14, 2025

Uh oh!

Uh oh!

gurukamath May 14, 2025

Uh oh!

gurukamath May 14, 2025

Uh oh!

Uh oh!

Uh oh!

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 #1183

Conversation

marioevz commented Apr 4, 2025

What was wrong?

How was it fixed?

Cute Animal Picture

Uh oh!

gurukamath commented Apr 7, 2025

Uh oh!

marioevz commented Apr 14, 2025

Uh oh!

marioevz commented Apr 16, 2025

Uh oh!

SamWilsn Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

marioevz Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

marioevz commented Apr 23, 2025

Uh oh!

SamWilsn commented Apr 24, 2025

Uh oh!

marioevz commented Apr 24, 2025

Uh oh!

marioevz commented Apr 24, 2025

Uh oh!

SamWilsn commented Apr 25, 2025

Uh oh!

gurukamath Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

SamWilsn May 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gurukamath May 14, 2025

Choose a reason for hiding this comment

Uh oh!

gurukamath May 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!