From d1e344c88018f2f821fc41b1b2437e42ab0e0c16 Mon Sep 17 00:00:00 2001 From: Kubudak89 <95706654+Kubudak90@users.noreply.github.com> Date: Wed, 8 Apr 2026 06:44:48 +0300 Subject: [PATCH 1/2] fix: Remove inaccurate 'unimplemented stub' comments from all fork encode_node functions (#2627) * fix: Remove inaccurate 'unimplemented stub' comment from encode_node The encode_node function in berlin/trie.py was documented as 'Currently mostly an unimplemented stub' but it is actually fully implemented, handling Account, LegacyTransaction, Receipt, U256, Uint, Bytes types, and falling back to previous_trie.encode_node. This misleading docstring could confuse developers reading the code. Fixes #1186 * fix: Remove inaccurate 'unimplemented stub' comments from all fork encode_node functions The encode_node function is fully implemented across all Ethereum fork implementations, handling Account, LegacyTransaction, Receipt, U256, Uint, Bytes types, and falling back to previous_trie.encode_node. However, 24 fork versions had docstrings claiming the function was 'Currently mostly an unimplemented stub', which is factually incorrect and could mislead developers reading the code. This commit removes the inaccurate comment from all affected forks: amsterdam, arrow_glacier, bpo1-5, byzantium, cancun, constantinople, dao_fork, frontier, gray_glacier, homestead, istanbul, london, muir_glacier, osaka, paris, prague, shanghai, spurious_dragon, tangerine_whistle Fixes #1186 * Update trie.py --------- Co-authored-by: Kubudak90 Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> --- src/ethereum/forks/amsterdam/trie.py | 2 -- src/ethereum/forks/arrow_glacier/trie.py | 2 -- src/ethereum/forks/berlin/trie.py | 2 -- src/ethereum/forks/bpo1/trie.py | 2 -- src/ethereum/forks/bpo2/trie.py | 2 -- src/ethereum/forks/bpo3/trie.py | 2 -- src/ethereum/forks/bpo4/trie.py | 2 -- src/ethereum/forks/bpo5/trie.py | 2 -- src/ethereum/forks/byzantium/trie.py | 2 -- src/ethereum/forks/cancun/trie.py | 2 -- src/ethereum/forks/constantinople/trie.py | 2 -- src/ethereum/forks/dao_fork/trie.py | 2 -- src/ethereum/forks/frontier/trie.py | 2 -- src/ethereum/forks/gray_glacier/trie.py | 2 -- src/ethereum/forks/homestead/trie.py | 2 -- src/ethereum/forks/istanbul/trie.py | 2 -- src/ethereum/forks/london/trie.py | 2 -- src/ethereum/forks/muir_glacier/trie.py | 2 -- src/ethereum/forks/osaka/trie.py | 2 -- src/ethereum/forks/paris/trie.py | 2 -- src/ethereum/forks/prague/trie.py | 2 -- src/ethereum/forks/shanghai/trie.py | 2 -- src/ethereum/forks/spurious_dragon/trie.py | 2 -- src/ethereum/forks/tangerine_whistle/trie.py | 2 -- 24 files changed, 48 deletions(-) diff --git a/src/ethereum/forks/amsterdam/trie.py b/src/ethereum/forks/amsterdam/trie.py index b6b055a3c8..bf5ee0d6c4 100644 --- a/src/ethereum/forks/amsterdam/trie.py +++ b/src/ethereum/forks/amsterdam/trie.py @@ -133,8 +133,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/arrow_glacier/trie.py b/src/ethereum/forks/arrow_glacier/trie.py index c8c2ccfb8f..42be5c8456 100644 --- a/src/ethereum/forks/arrow_glacier/trie.py +++ b/src/ethereum/forks/arrow_glacier/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/berlin/trie.py b/src/ethereum/forks/berlin/trie.py index 1432406eff..f6371c3991 100644 --- a/src/ethereum/forks/berlin/trie.py +++ b/src/ethereum/forks/berlin/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/bpo1/trie.py b/src/ethereum/forks/bpo1/trie.py index 6f4c1708ef..81e0fd6c22 100644 --- a/src/ethereum/forks/bpo1/trie.py +++ b/src/ethereum/forks/bpo1/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/bpo2/trie.py b/src/ethereum/forks/bpo2/trie.py index c48e40ec8f..5132bb42fe 100644 --- a/src/ethereum/forks/bpo2/trie.py +++ b/src/ethereum/forks/bpo2/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/bpo3/trie.py b/src/ethereum/forks/bpo3/trie.py index b511be0f52..edd126ce9c 100644 --- a/src/ethereum/forks/bpo3/trie.py +++ b/src/ethereum/forks/bpo3/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/bpo4/trie.py b/src/ethereum/forks/bpo4/trie.py index 400519f411..bdd3c0456e 100644 --- a/src/ethereum/forks/bpo4/trie.py +++ b/src/ethereum/forks/bpo4/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/bpo5/trie.py b/src/ethereum/forks/bpo5/trie.py index e39160db4c..bcc7ed64c3 100644 --- a/src/ethereum/forks/bpo5/trie.py +++ b/src/ethereum/forks/bpo5/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/byzantium/trie.py b/src/ethereum/forks/byzantium/trie.py index b7a8fc3573..42cf24a86f 100644 --- a/src/ethereum/forks/byzantium/trie.py +++ b/src/ethereum/forks/byzantium/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/cancun/trie.py b/src/ethereum/forks/cancun/trie.py index 768a438089..979117f1c9 100644 --- a/src/ethereum/forks/cancun/trie.py +++ b/src/ethereum/forks/cancun/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/constantinople/trie.py b/src/ethereum/forks/constantinople/trie.py index 4a3fabfdd0..493fda074c 100644 --- a/src/ethereum/forks/constantinople/trie.py +++ b/src/ethereum/forks/constantinople/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/dao_fork/trie.py b/src/ethereum/forks/dao_fork/trie.py index 82d45a0868..d8db45abac 100644 --- a/src/ethereum/forks/dao_fork/trie.py +++ b/src/ethereum/forks/dao_fork/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/frontier/trie.py b/src/ethereum/forks/frontier/trie.py index d7665cccd8..a1aed10f30 100644 --- a/src/ethereum/forks/frontier/trie.py +++ b/src/ethereum/forks/frontier/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/gray_glacier/trie.py b/src/ethereum/forks/gray_glacier/trie.py index 55dd03bc82..4dc21a2573 100644 --- a/src/ethereum/forks/gray_glacier/trie.py +++ b/src/ethereum/forks/gray_glacier/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/homestead/trie.py b/src/ethereum/forks/homestead/trie.py index a04cde22a4..d037bec1c5 100644 --- a/src/ethereum/forks/homestead/trie.py +++ b/src/ethereum/forks/homestead/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/istanbul/trie.py b/src/ethereum/forks/istanbul/trie.py index 47de5debd6..52e1dd2ea5 100644 --- a/src/ethereum/forks/istanbul/trie.py +++ b/src/ethereum/forks/istanbul/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/london/trie.py b/src/ethereum/forks/london/trie.py index 04b5a365e4..b0a751f1c9 100644 --- a/src/ethereum/forks/london/trie.py +++ b/src/ethereum/forks/london/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/muir_glacier/trie.py b/src/ethereum/forks/muir_glacier/trie.py index 5acd07500f..04487f2526 100644 --- a/src/ethereum/forks/muir_glacier/trie.py +++ b/src/ethereum/forks/muir_glacier/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/osaka/trie.py b/src/ethereum/forks/osaka/trie.py index c0cf15e0ac..a99b562bfe 100644 --- a/src/ethereum/forks/osaka/trie.py +++ b/src/ethereum/forks/osaka/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/paris/trie.py b/src/ethereum/forks/paris/trie.py index 2adda05edc..35882f949f 100644 --- a/src/ethereum/forks/paris/trie.py +++ b/src/ethereum/forks/paris/trie.py @@ -174,8 +174,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/prague/trie.py b/src/ethereum/forks/prague/trie.py index 8e7c9555b4..a25868fbef 100644 --- a/src/ethereum/forks/prague/trie.py +++ b/src/ethereum/forks/prague/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/shanghai/trie.py b/src/ethereum/forks/shanghai/trie.py index d434c63faa..2cf4cf73fe 100644 --- a/src/ethereum/forks/shanghai/trie.py +++ b/src/ethereum/forks/shanghai/trie.py @@ -175,8 +175,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/spurious_dragon/trie.py b/src/ethereum/forks/spurious_dragon/trie.py index d9d5564fb8..132537161a 100644 --- a/src/ethereum/forks/spurious_dragon/trie.py +++ b/src/ethereum/forks/spurious_dragon/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None diff --git a/src/ethereum/forks/tangerine_whistle/trie.py b/src/ethereum/forks/tangerine_whistle/trie.py index bd652ffc5d..efc44cfd11 100644 --- a/src/ethereum/forks/tangerine_whistle/trie.py +++ b/src/ethereum/forks/tangerine_whistle/trie.py @@ -173,8 +173,6 @@ def encode_internal_node(node: Optional[InternalNode]) -> Extended: def encode_node(node: Node, storage_root: Optional[Bytes] = None) -> Bytes: """ Encode a Node for storage in the Merkle Trie. - - Currently mostly an unimplemented stub. """ if isinstance(node, Account): assert storage_root is not None From a35219260251ff44776fa8e41f256dfb970faa5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Wed, 8 Apr 2026 09:46:58 +0200 Subject: [PATCH 2/2] feat(tests): add state restoration tests for reverted sub-calls (#2494) --- .../test_warm_status_revert.py | 139 +++++++++++++ .../test_journal_revert.py | 82 ++++++++ .../eip1014_create2/test_create2_revert.py | 183 ++++++++++++++++++ .../test_collision_selfdestruct.py | 115 +++++++++++ 4 files changed, 519 insertions(+) create mode 100644 tests/berlin/eip2929_gas_cost_increases/test_warm_status_revert.py create mode 100644 tests/cancun/eip6780_selfdestruct/test_journal_revert.py create mode 100644 tests/constantinople/eip1014_create2/test_create2_revert.py create mode 100644 tests/paris/eip7610_create_collision/test_collision_selfdestruct.py diff --git a/tests/berlin/eip2929_gas_cost_increases/test_warm_status_revert.py b/tests/berlin/eip2929_gas_cost_increases/test_warm_status_revert.py new file mode 100644 index 0000000000..3925cd8995 --- /dev/null +++ b/tests/berlin/eip2929_gas_cost_increases/test_warm_status_revert.py @@ -0,0 +1,139 @@ +""" +Tests that warm/cold access status is reverted when a sub-call reverts. +""" + +import pytest +from execution_testing import ( + Account, + Alloc, + CodeGasMeasure, + Conditional, + Environment, + Fork, + Op, + StateTestFiller, + Transaction, +) + +REFERENCE_SPEC_GIT_PATH = "EIPS/eip-2929.md" +REFERENCE_SPEC_VERSION = "0e11417265a623adb680c527b15d0cb6701b870b" + + +@pytest.mark.valid_from("Berlin") +def test_storage_warm_status_reverted_by_subcall( + state_test: StateTestFiller, + pre: Alloc, + fork: Fork, +) -> None: + """ + Test that storage slot warm status is reverted when a sub-call reverts. + + Inner self-call does SLOAD(0) and SSTORE(0, 2) then REVERTs. After + revert, SLOAD(0) must be a cold access and storage[0] must still + hold its original value. + """ + env = Environment() + + # Inner behavior (no calldata): warm slot 0 via SLOAD+SSTORE, revert. + inner_code = ( + Op.POP(Op.SLOAD(0)) + Op.SSTORE(0, 2) + Op.REVERT(offset=0, size=0) + ) + + # Overhead: PUSH instructions for the SLOAD key argument. + sload_push_cost = (Op.PUSH1(0) * len(Op.SLOAD.kwargs)).gas_cost(fork) + cold_sload_cost = Op.SLOAD(key_warm=False).gas_cost(fork) + + # After revert, measure gas of SLOAD(0) — should be cold. + sload_measure = CodeGasMeasure( + code=Op.SLOAD(0), + overhead_cost=sload_push_cost, + extra_stack_items=1, + sstore_key=1, + stop=False, + ) + + # Also verify storage[0] value (should still be 1). + verify_value = Op.SSTORE(2, Op.SLOAD(0)) + + # Outer behavior (has calldata): call self (inner), measure, verify. + outer_code = ( + Op.POP(Op.CALL(gas=100_000, address=Op.ADDRESS)) + + sload_measure + + verify_value + + Op.STOP + ) + + code = Conditional( + condition=Op.CALLDATASIZE, + if_true=outer_code, + if_false=inner_code, + ) + + contract = pre.deploy_contract(code, storage={0: 1}) + sender = pre.fund_eoa() + + state_test( + env=env, + pre=pre, + post={ + contract: Account( + storage={0: 1, 1: cold_sload_cost, 2: 1}, + ), + }, + tx=Transaction( + sender=sender, + to=contract, + gas_limit=1_000_000, + data=b"\x01", + ), + ) + + +@pytest.mark.valid_from("Berlin") +def test_account_warm_status_reverted_by_subcall( + state_test: StateTestFiller, + pre: Alloc, + fork: Fork, +) -> None: + """ + Test that account warm status is reverted when a sub-call reverts. + + Inner call does BALANCE(target) then REVERTs. After revert, + BALANCE(target) in the outer call must be a cold access. + """ + env = Environment() + + target = pre.fund_eoa(amount=1) + + # Inner: BALANCE(target) warms target, then reverts. + inner = pre.deploy_contract( + Op.POP(Op.BALANCE(target)) + Op.REVERT(offset=0, size=0) + ) + + # Overhead: PUSH for the BALANCE address argument. + balance_push_cost = (Op.PUSH1(0) * len(Op.BALANCE.kwargs)).gas_cost(fork) + cold_balance_cost = Op.BALANCE(address_warm=False).gas_cost(fork) + + # Outer: call inner (reverts), then measure BALANCE(target) gas. + outer = pre.deploy_contract( + Op.POP(Op.CALL(gas=100_000, address=inner)) + + CodeGasMeasure( + code=Op.BALANCE(target), + overhead_cost=balance_push_cost, + extra_stack_items=1, + sstore_key=0, + ) + ) + + sender = pre.fund_eoa() + + state_test( + env=env, + pre=pre, + post={outer: Account(storage={0: cold_balance_cost})}, + tx=Transaction( + sender=sender, + to=outer, + gas_limit=1_000_000, + ), + ) diff --git a/tests/cancun/eip6780_selfdestruct/test_journal_revert.py b/tests/cancun/eip6780_selfdestruct/test_journal_revert.py new file mode 100644 index 0000000000..9168e1e640 --- /dev/null +++ b/tests/cancun/eip6780_selfdestruct/test_journal_revert.py @@ -0,0 +1,82 @@ +""" +Tests for SELFDESTRUCT balance transfer revert behavior (EIP-6780). +""" + +import pytest +from execution_testing import ( + Account, + Alloc, + Environment, + Op, + StateTestFiller, + Storage, + Transaction, +) + +REFERENCE_SPEC_GIT_PATH = "EIPS/eip-6780.md" +REFERENCE_SPEC_VERSION = "1b6a0e94cc47e859b9866e570391cf37dc55059a" + + +@pytest.mark.valid_from("Cancun") +def test_selfdestruct_balance_transfer_reverted( + state_test: StateTestFiller, + env: Environment, + pre: Alloc, +) -> None: + """ + Test that SELFDESTRUCT balance transfer is reverted on sub-call revert. + + Post-Cancun, SELFDESTRUCT does not destroy the contract but still + transfers balance. When the sub-call containing SELFDESTRUCT reverts, + the balance transfer must also be reverted. + """ + storage = Storage() + + victim_balance = 1 + + beneficiary_balance = 1 + beneficiary = pre.fund_eoa(amount=beneficiary_balance) + + victim = pre.deploy_contract( + code=Op.SELFDESTRUCT(beneficiary), + balance=victim_balance, + ) + + # Controller calls victim (triggers SELFDESTRUCT) then reverts. + controller = pre.deploy_contract( + Op.POP(Op.CALL(gas=100_000, address=victim)) + + Op.REVERT(offset=0, size=0) + ) + + # Outer calls controller, then checks beneficiary balance. + outer = pre.deploy_contract( + Op.POP(Op.CALL(gas=200_000, address=controller)) + + Op.SSTORE( + storage.store_next(beneficiary_balance, "beneficiary_balance"), + Op.BALANCE(beneficiary), + ) + + Op.SSTORE( + storage.store_next(victim_balance, "victim_balance"), + Op.BALANCE(victim), + ) + + Op.STOP + ) + + sender = pre.fund_eoa() + + state_test( + env=env, + pre=pre, + post={ + outer: Account(storage=storage), + # Beneficiary keeps only its initial balance (transfer reverted). + beneficiary: Account(balance=beneficiary_balance), + # Victim still has its balance. + victim: Account(balance=victim_balance), + }, + tx=Transaction( + sender=sender, + to=outer, + gas_limit=1_000_000, + ), + ) diff --git a/tests/constantinople/eip1014_create2/test_create2_revert.py b/tests/constantinople/eip1014_create2/test_create2_revert.py new file mode 100644 index 0000000000..c611d06f4b --- /dev/null +++ b/tests/constantinople/eip1014_create2/test_create2_revert.py @@ -0,0 +1,183 @@ +""" +Tests for CREATE2 state restoration after reverted sub-calls. +""" + +import pytest +from execution_testing import ( + Account, + Alloc, + Environment, + Initcode, + Op, + StateTestFiller, + Storage, + Transaction, + compute_create2_address, +) + +from .spec import ref_spec_1014 + +REFERENCE_SPEC_GIT_PATH = ref_spec_1014.git_path +REFERENCE_SPEC_VERSION = ref_spec_1014.version + + +@pytest.mark.valid_from("Constantinople") +@pytest.mark.pre_alloc_mutable +def test_create2_revert_preserves_balance( + state_test: StateTestFiller, + pre: Alloc, +) -> None: + """ + Test that CREATE2 revert preserves pre-existing balance at target. + + Address X has a pre-existing balance but no code. CREATE2 targets X + with init code that reverts. After the revert, X must still have its + original balance, nonce=0, and no code or storage. + """ + env = Environment() + factory_storage = Storage() + salt = 0 + pre_balance = 1 + + # Init code that writes storage then reverts. + initcode = Op.SSTORE(0, 1) + Op.REVERT(offset=0, size=0) + + # Factory receives initcode via calldata, does CREATE2. + factory = pre.deploy_contract( + Op.CALLDATACOPY(0, 0, Op.CALLDATASIZE) + + Op.SSTORE( + factory_storage.store_next(0, "create2_result"), + Op.CREATE2( + value=0, + offset=0, + size=Op.CALLDATASIZE, + salt=salt, + ), + ) + + Op.STOP, + storage=factory_storage.canary(), + ) + + target = compute_create2_address(factory, salt, initcode) + + # Pre-allocate target with balance only. + pre[target] = Account(balance=pre_balance) + + sender = pre.fund_eoa() + + state_test( + env=env, + pre=pre, + post={ + # CREATE2 returns 0 on failure. + factory: Account(storage=factory_storage), + # Target keeps its balance, no code deployed. + target: Account(balance=pre_balance, nonce=0, code=b""), + }, + tx=Transaction( + sender=sender, + to=factory, + gas_limit=1_000_000, + data=initcode, + ), + ) + + +@pytest.mark.valid_from("Constantinople") +def test_create2_succeeds_after_reverted_create2( + state_test: StateTestFiller, + pre: Alloc, +) -> None: + """ + Test that CREATE2 succeeds after a previous CREATE2 at the same address + was reverted. + + Inner call does CREATE2 then REVERTs. Outer call then does the same + CREATE2 which should succeed since the first was rolled back. + """ + env = Environment() + storage = Storage() + salt = 1 + + runtime_code = Op.SSTORE(0, 1) + Op.STOP + initcode = Initcode(deploy_code=runtime_code) + + # The "creator" contract that does CREATE2 when called. + creator_code = ( + Op.CALLDATACOPY(0, 0, Op.CALLDATASIZE) + + Op.SSTORE( + 0, + Op.CREATE2( + value=0, + offset=0, + size=Op.CALLDATASIZE, + salt=salt, + ), + ) + + Op.STOP + ) + creator = pre.deploy_contract(creator_code) + + expected_address = compute_create2_address(creator, salt, initcode) + + # Outer contract: + # 1. Call creator wrapped in a sub-call that reverts. + # 2. Call creator again (should succeed). + # + # Use a "reverter" contract that calls creator then reverts. + reverter_code = ( + Op.CALLDATACOPY(0, 0, Op.CALLDATASIZE) + + Op.POP( + Op.CALL( + gas=200_000, + address=creator, + args_size=Op.CALLDATASIZE, + ) + ) + + Op.REVERT(offset=0, size=0) + ) + reverter = pre.deploy_contract(reverter_code) + + outer_code = ( + Op.CALLDATACOPY(0, 0, Op.CALLDATASIZE) + # First attempt: call reverter (which calls creator then reverts) + + Op.SSTORE( + storage.store_next(0, "reverter_call_result"), + Op.CALL( + gas=300_000, + address=reverter, + args_size=Op.CALLDATASIZE, + ), + ) + # Second attempt: call creator directly (should succeed) + + Op.SSTORE( + storage.store_next(1, "creator_call_result"), + Op.CALL( + gas=300_000, + address=creator, + args_size=Op.CALLDATASIZE, + ), + ) + + Op.STOP + ) + outer = pre.deploy_contract(outer_code, storage=storage.canary()) + + sender = pre.fund_eoa() + + state_test( + env=env, + pre=pre, + post={ + outer: Account(storage=storage), + # The creator stored the CREATE2 result. + creator: Account(storage={0: expected_address}), + # The contract was deployed. + expected_address: Account(code=runtime_code), + }, + tx=Transaction( + sender=sender, + to=outer, + gas_limit=2_000_000, + data=initcode, + ), + ) diff --git a/tests/paris/eip7610_create_collision/test_collision_selfdestruct.py b/tests/paris/eip7610_create_collision/test_collision_selfdestruct.py new file mode 100644 index 0000000000..dea29c7523 --- /dev/null +++ b/tests/paris/eip7610_create_collision/test_collision_selfdestruct.py @@ -0,0 +1,115 @@ +""" +Test CREATE2 collision interaction with SELFDESTRUCT (EIP-6780). + +Verify that a failed CREATE2 (due to collision) does not cause a +subsequent SELFDESTRUCT to destroy the pre-existing contract. +""" + +import pytest +from execution_testing import ( + Account, + Alloc, + Environment, + Initcode, + Op, + StateTestFiller, + Storage, + Transaction, + compute_create2_address, +) + +REFERENCE_SPEC_GIT_PATH = "EIPS/eip-7610.md" +REFERENCE_SPEC_VERSION = "80ef48d0bbb5a4939ade51caaaac57b5df6acd4e" + + +@pytest.mark.valid_from("Cancun") +@pytest.mark.pre_alloc_mutable +def test_selfdestruct_after_create2_collision( + state_test: StateTestFiller, + pre: Alloc, +) -> None: + """ + Test that a failed CREATE2 collision does not count as creation. + + A CREATE2 that collides with an existing contract fails. A + subsequent SELFDESTRUCT on the same address must not destroy the + contract because EIP-6780 only allows destruction if the contract + was created in the same transaction. + """ + env = Environment() + storage = Storage() + + salt = 0 + initcode = Initcode(deploy_code=Op.STOP) + + deployer_storage = Storage() + deployer_code = Op.CALLDATACOPY(0, 0, Op.CALLDATASIZE) + Op.SSTORE( + deployer_storage.store_next(0, "create2_result"), + Op.CREATE2(value=0, offset=0, size=Op.CALLDATASIZE, salt=salt), + ) + deployer = pre.deploy_contract( + deployer_code, storage=deployer_storage.canary() + ) + + target_address = compute_create2_address(deployer, salt, initcode) + + beneficiary = pre.fund_eoa(amount=0) + + # Target already exists with balance and code (causes collision). + target_code = Op.SELFDESTRUCT(beneficiary) + pre[target_address] = Account( + balance=1, + nonce=1, + code=target_code, + ) + + # Controller: attempt CREATE2 (will collide), then call target + # (SELFDESTRUCT should NOT destroy since target was not created + # in this tx). + controller = pre.deploy_contract( + Op.CALLDATACOPY(0, 0, Op.CALLDATASIZE) + # CREATE2 via deployer — will fail (collision) + + Op.SSTORE( + storage.store_next(1, "create2_call_success"), + Op.CALL( + gas=500_000, + address=deployer, + args_size=Op.CALLDATASIZE, + ), + ) + # Call target to trigger SELFDESTRUCT + + Op.SSTORE( + storage.store_next(1, "selfdestruct_call_success"), + Op.CALL(gas=100_000, address=target_address), + ) + + Op.STOP + ) + + sender = pre.fund_eoa() + + post = { + controller: Account(storage=storage), + # CREATE2 failed due to collision — returned 0. + deployer: Account(storage=deployer_storage), + # Target must still exist (SELFDESTRUCT did not destroy because + # it was NOT created in this tx). Balance was transferred to + # beneficiary. + target_address: Account( + balance=0, + nonce=1, + code=target_code, + ), + beneficiary: Account(balance=1), + } + + state_test( + env=env, + pre=pre, + post=post, + tx=Transaction( + sender=sender, + to=controller, + gas_limit=2_000_000, + data=initcode, + ), + )