-
Notifications
You must be signed in to change notification settings - Fork 456
feat(spec-spec, tests): Implement eip-8246 and testing scenario #2842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: eips/amsterdam/eip-8246
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| """ | ||
| EIP-8246: Remove SELFDESTRUCT balance burn. | ||
|
|
||
| https://eips.ethereum.org/EIPS/eip-8246 | ||
| """ | ||
|
|
||
| from ....base_fork import BaseFork | ||
|
|
||
|
|
||
| class EIP8246(BaseFork): | ||
| """EIP-8246 class.""" | ||
|
|
||
| pass |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -434,7 +434,7 @@ def destroy_account(tx_state: TransactionState, address: Address) -> None: | |
| This function is made available exclusively for the ``SELFDESTRUCT`` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docstring to be updated |
||
| opcode. It is expected that ``SELFDESTRUCT`` will be disabled in a | ||
| future hardfork and this function will be removed. Only supports same | ||
| transaction destruction. | ||
| transaction destruction and zero balance on created accounts. | ||
|
|
||
| Parameters | ||
| ---------- | ||
|
|
@@ -448,6 +448,20 @@ def destroy_account(tx_state: TransactionState, address: Address) -> None: | |
| set_account(tx_state, address, None) | ||
|
|
||
|
|
||
| def preserve_account_balance(account: Account) -> None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about creating a |
||
| """ | ||
| Clear nonce and code for an account, but preserve balance. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| account : | ||
| The account to modify. | ||
|
|
||
| """ | ||
| account.nonce = Uint(0) | ||
| account.code_hash = EMPTY_CODE_HASH | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth to double check whether this is sufficient to clear the code. cc @gurukamath as he probably knows the answer.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. This is equivalent to resetting the code. |
||
|
|
||
|
|
||
| def destroy_storage(tx_state: TransactionState, address: Address) -> None: | ||
| """ | ||
| Completely remove the storage at ``address``. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| """Tests for [EIP-8246: Remove SELFDESTRUCT Burn](https://eips.ethereum.org/EIPS/eip-8246).""" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| """Reference spec for [EIP-8246](https://eips.ethereum.org/EIPS/eip-8246).""" | ||
|
|
||
| from dataclasses import dataclass | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class ReferenceSpec: | ||
| """Reference specification.""" | ||
|
|
||
| git_path: str | ||
| version: str | ||
|
|
||
|
|
||
| ref_spec_8246 = ReferenceSpec( | ||
| git_path="EIPS/eip-8246.md", | ||
| version="3b30ff829e5e698f1c6f69427111d194b80af38d", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| """Tests for [EIP-8246: Remove SELFDESTRUCT balance burn](https://eips.ethereum.org/EIPS/eip-8246).""" | ||
|
|
||
| import pytest | ||
| from execution_testing import ( | ||
| Account, | ||
| Address, | ||
| Alloc, | ||
| Block, | ||
| BlockchainTestFiller, | ||
| Bytecode, | ||
| Op, | ||
| Storage, | ||
| Transaction, | ||
| compute_create_address, | ||
| keccak256, | ||
| ) | ||
|
|
||
| from .spec import ref_spec_8246 | ||
|
|
||
| REFERENCE_SPEC_GIT_PATH = ref_spec_8246.git_path | ||
| REFERENCE_SPEC_VERSION = ref_spec_8246.version | ||
|
|
||
| pytestmark = pytest.mark.valid_from("EIP8246") | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("initial_balance", [0, 1]) | ||
| @pytest.mark.parametrize("create_opcode", [Op.CREATE, Op.CREATE2]) | ||
| @pytest.mark.parametrize("post_send_count", [0, 1, 3]) | ||
| @pytest.mark.parametrize( | ||
| "post_send_opcode", [Op.CALL, Op.CALLCODE, Op.SELFDESTRUCT] | ||
| ) | ||
| @pytest.mark.parametrize( | ||
| "initial_storage", | ||
| [ | ||
| pytest.param(False, id="no_storage"), | ||
| pytest.param(True, id="with_storage"), | ||
| ], | ||
| ) | ||
| @pytest.mark.parametrize( | ||
| "transfer_target, transfer_drains_victim", | ||
| [ | ||
| pytest.param(Op.ADDRESS, False, id="self"), | ||
| pytest.param(0x01, True, id="precompile"), | ||
| pytest.param( | ||
| Address(keccak256(b"eip-8246-eoa-target")[-20:]), | ||
| True, | ||
| id="eoa", | ||
| ), | ||
| ], | ||
| ) | ||
| @pytest.mark.parametrize( | ||
| "exit_op, execution_success", | ||
| [ | ||
| pytest.param(Op.STOP, True, id="success"), | ||
| pytest.param(Op.REVERT(0, 0), False, id="revert"), | ||
| pytest.param(Op.MSTORE(2**32, 0), False, id="oog"), | ||
| ], | ||
| ) | ||
| def test_selfdestruct_preserves_balance( | ||
| blockchain_test: BlockchainTestFiller, | ||
| pre: Alloc, | ||
| initial_balance: int, | ||
| post_send_count: int, | ||
| create_opcode: Op, | ||
| post_send_opcode: Op, | ||
| initial_storage: bool, | ||
| transfer_target: Op, | ||
| transfer_drains_victim: bool, | ||
| exit_op: Op, | ||
| execution_success: bool, | ||
| ) -> None: | ||
| """ | ||
| Same-tx SELFDESTRUCT preserves the victim's balance per EIP-8246. | ||
|
|
||
| Test flow: | ||
| selfdestruct_tx | ||
| tx.to = entry_contract | ||
| ββ CALL selfdestruct_contract_factory | ||
| ββ initcode runs: | ||
| [optional] SSTORE(slot, value) | ||
| SELFDESTRUCT(transfer_target) # registers victim | ||
| ββ selfdestruct_contract_factory exits via STOP | REVERT | OOG | ||
| ββ N * post-send to victim (CALL | CALLCODE | donor.SELFDESTRUCT) | ||
|
|
||
| tx finalize | ||
| - victim balance-only per EIP-8246, | ||
| - or NONEXISTENT if EIP-161 cleans up a zero-balance account | ||
|
|
||
| probe_tx | ||
| tx.to = probe_contract | ||
| ββ STORAGE [0] = BALANCE(victim) | ||
| STORAGE [1] = EXTCODEHASH(victim) | ||
| STORAGE [2] = EXTCODESIZE(victim) | ||
| STORAGE [3] = SHA3(EXTCODECOPY(victim, 0, 0, size)) | ||
| """ | ||
| # Selfdestruct target contract template. | ||
| # Optionally initializes storage to test clearing. | ||
| storage_init = Op.SSTORE(0, 1) if initial_storage else Bytecode() | ||
| selfdestruct_initcode = storage_init + Op.SELFDESTRUCT(transfer_target) | ||
|
|
||
| selfdestruct_template = pre.deploy_contract(code=selfdestruct_initcode) | ||
|
|
||
| # Build selfdestruct target contract via CREATE/CREATE2 | ||
| salt = 0 | ||
| if create_opcode == Op.CREATE2: | ||
| create_call = create_opcode( | ||
| value=initial_balance, | ||
| size=len(selfdestruct_initcode), | ||
| salt=salt, | ||
| ) | ||
| else: | ||
| create_call = create_opcode( | ||
| value=initial_balance, | ||
| size=len(selfdestruct_initcode), | ||
| ) | ||
|
|
||
| # Selfdestruct target contract factory | ||
| # Exits via STOP/REVERT/OOG for different scenario | ||
| selfdestruct_contract_factory = pre.deploy_contract( | ||
| code=Op.EXTCODECOPY( | ||
| address=selfdestruct_template, size=len(selfdestruct_initcode) | ||
| ) | ||
| + Op.POP(create_call) | ||
| + exit_op | ||
| ) | ||
|
|
||
| victim = compute_create_address( | ||
| address=selfdestruct_contract_factory, | ||
| opcode=create_opcode, | ||
| nonce=1, | ||
| salt=salt, | ||
| initcode=selfdestruct_initcode, | ||
| ) | ||
|
|
||
| # Post value sending to the victim | ||
| # Ensure the ether transfer is not burned after eip-8246. | ||
| post_send_value = 1 | ||
| if post_send_opcode == Op.SELFDESTRUCT: | ||
| donor = pre.deploy_contract(code=Op.SELFDESTRUCT(victim)) | ||
| post_send = Op.POP( | ||
| Op.CALL(gas=Op.GAS, address=donor, value=post_send_value) | ||
| ) | ||
| else: | ||
| post_send = Op.POP( | ||
| post_send_opcode(gas=Op.GAS, address=victim, value=post_send_value) | ||
| ) | ||
|
|
||
| entry_contract = pre.deploy_contract( | ||
| code=Op.POP( | ||
| Op.CALL( | ||
| gas=Op.GAS, | ||
| address=selfdestruct_contract_factory, | ||
| value=initial_balance, | ||
| ) | ||
| ) | ||
| + post_send * post_send_count | ||
| ) | ||
|
|
||
| total_balance = initial_balance + post_send_count * post_send_value | ||
|
|
||
| sender = pre.fund_eoa() | ||
| selfdestruct_tx = Transaction( | ||
| sender=sender, | ||
| to=entry_contract, | ||
| value=total_balance, | ||
| gas_limit=5_000_000, | ||
| ) | ||
|
|
||
| # Balance verification | ||
| # retained: | ||
| # selfdestruct-to-self retains balance | ||
| # selfdestruct-to-others drains balance if not revert / OOG | ||
| # delivered: post-sends count except for CALLCODE | ||
| retained = 0 if transfer_drains_victim else initial_balance | ||
| delivered = ( | ||
| 0 | ||
| if post_send_opcode == Op.CALLCODE | ||
| else post_send_count * post_send_value | ||
| ) | ||
|
|
||
| expected_balance = retained + delivered if execution_success else delivered | ||
| victim_alive = expected_balance > 0 | ||
|
|
||
| probe_storage = Storage() | ||
| probe_code = ( | ||
| Op.SSTORE( | ||
| probe_storage.store_next(expected_balance), | ||
| Op.BALANCE(victim), | ||
| ) | ||
| + Op.SSTORE( | ||
| probe_storage.store_next(keccak256(b"") if victim_alive else 0), | ||
| Op.EXTCODEHASH(victim), | ||
| ) | ||
| + Op.SSTORE( | ||
| probe_storage.store_next(0), | ||
| Op.EXTCODESIZE(victim), | ||
| ) | ||
| + Op.EXTCODECOPY(victim, 0, 0, len(selfdestruct_initcode)) | ||
| + Op.SSTORE( | ||
| probe_storage.store_next( | ||
| keccak256(b"\x00" * len(selfdestruct_initcode)) | ||
| ), | ||
| Op.SHA3(0, len(selfdestruct_initcode)), | ||
| ) | ||
| + Op.STOP | ||
| ) | ||
|
|
||
| probe_contract = pre.deploy_contract( | ||
| code=probe_code, storage=probe_storage.canary() | ||
| ) | ||
|
|
||
| probe_tx = Transaction( | ||
| sender=sender, | ||
| to=probe_contract, | ||
| gas_limit=200_000, | ||
| ) | ||
|
|
||
| blockchain_test( | ||
| pre=pre, | ||
| post={ | ||
| victim: ( | ||
| Account.NONEXISTENT | ||
| if not victim_alive | ||
| else Account( | ||
| balance=expected_balance, nonce=0, code=b"", storage={} | ||
| ) | ||
| ), | ||
| probe_contract: Account(storage=probe_storage), | ||
| }, | ||
| blocks=[Block(txs=[selfdestruct_tx, probe_tx])], | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If modify_state ends up calling destroy_account (zero-balance case), destroy_storage runs twice. Not a huge issue but something to be kept in mind