feat(test): OOG and success selfdestruct tests to all precompiles #1954

fselmo · 2025-12-29T22:58:35Z

🗒️ Description

Additional selfdestruct to precompile test cases:

parametrize with all precompile opcodes
include success case and parametrize with oog_before_state_access = [True, False] cases

It doesn't seem like we have good coverage for selfdestruct to precompiles. Some static tests have this for 0x01 and some static tests for 0x01, 0x02, and 0x03 but I believe that's it. I haven't dug too deep into those static test cases but I think it is safe to assume we want to include wider coverage for all fork precompiles with selfdestruct cases.

In this PR, I added Tangerine (Tangerine Whistle in EELS) support and included the selfdestruct OOG cases in a new EIP-150 directory in our tests. We did not have support for tangerine whistle turned on so this turns that on and this seemed most appropriate to add here as this is when the gas was introduced for selfdestruct.
For the success cases, the first precompiles were introduced in Homestead, so I added these test cases there.

🔗 Related Issues or PRs

Related to a Nethermind bug in BAL devnet 1, relevant PR here

✅ Checklist

All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
```
uvx tox -e static
```
All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
All: Considered updating the online docs in the ./docs/ directory.
All: Set appropriate labels for the changes (only maintainers can apply labels).
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.

Cute Animal Picture

codecov · 2025-12-29T23:40:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (eips/amsterdam/eip-7928@ba0eec1). Learn more about missing BASE report.

Additional details and impacted files

@@                    Coverage Diff                     @@
##             eips/amsterdam/eip-7928    #1954   +/-   ##
==========================================================
  Coverage                           ?   86.25%           
==========================================================
  Files                              ?      538           
  Lines                              ?    34561           
  Branches                           ?     3224           
==========================================================
  Hits                               ?    29809           
  Misses                             ?     4165           
  Partials                           ?      587

Flag	Coverage Δ
unittests	`86.25% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fselmo · 2025-12-30T16:21:38Z

I think we should move these to an earlier fork as it looks like we may not have coverage for this. I see some coverage in the static tests but not for all opcodes. It would be good to have this in python either way and use the pytest.mark.with_all_precompiles to test every added precompile for every fork as they are added.

I'll work on porting this to an earlier fork.

jochem-brouwer · 2025-12-30T20:53:19Z

Does this initialize the precompiles as empty and does it attempt to create the accounts via SELFDESTRUCT?

This is definitely a case which should be added, but just wanted to note here that due to this bug:

(from the yellow paper) there is this weird behavior with precompiles, creating the account, the precompile going out-of-gas and then the account cleanup either cleaning up the account (or not).

This bug caused that later devnets and all live chains just funded the addresses with 1 wei, such that the account is always there (and not the "0 balance, 0 nonce, no code, no storage", but still in trie case, which now is not possible anymore since it has a balance)

I wanted to share this because this might be the reason why the coverage report is low. I think (although this should be an EIP in that case) that the general consensus is to "just fund the precompiles" with 1 wei at genesis to get rid of this behavior, and that it is thus either unspecified or untested behavior.

fselmo · 2025-12-30T21:14:33Z

Does this initialize the precompiles as empty and does it attempt to create the accounts via SELFDESTRUCT?

pytest.mark.with_all_precompiles will parametrize for all precompile addresses that are mapped for the fork you are filling for, but it doesn't add anything to the pre. All the EVM knows is that this is the beneficiary for the self-destruct. Is there a missing case? 🤔

jochem-brouwer · 2025-12-30T22:01:20Z

pytest.mark.with_all_precompiles will parametrize for all precompile addresses that are mapped for the fork you are filling for, but it doesn't add anything to the pre. All the EVM knows is that this is the beneficiary for the self-destruct. Is there a missing case? 🤔

Ah sorry, I think my comment is confusing. The YP case is, IIRC, a case which is only possible if a precompile account is empty (no balance, 0 nonce, no code, no storage). I am not sure but I thought some tests pre-fill precompiles with a balance to make them non-empty, and wanted to explicitly mention this here. But I think this is already the case. So no missing cases found here.

jochem-brouwer

Not a review of the tests, just a small point regarding fork names and docs 😄 👍

packages/testing/src/execution_testing/forks/forks/forks.py

packages/testing/src/execution_testing/forks/__init__.py

jochem-brouwer · 2025-12-30T22:06:38Z

tests/amsterdam/eip7928_block_level_access_lists/test_cases.md

 | `test_bal_lexicographic_address_ordering` | Ensure BAL enforces strict lexicographic byte-wise ordering | Pre-fund three addresses with specific byte patterns: `addr_low = 0x0000...0001`, `addr_mid = 0x0000...0100`, `addr_high = 0x0100...0000`. Contract touches them in reverse order: `BALANCE(addr_high), BALANCE(addr_low), BALANCE(addr_mid)`. Additionally, include two endian-trap addresses that are byte-reversals of each other: `addr_endian_low = 0x0100000000000000000000000000000000000002`, `addr_endian_high = 0x0200000000000000000000000000000000000001`. Note: `reverse(addr_endian_low) = addr_endian_high`. Correct lexicographic order: `addr_endian_low < addr_endian_high` (0x01 < 0x02 at byte 0). If implementation incorrectly reverses bytes before comparing, it would get `addr_endian_low > addr_endian_high` (wrong). | BAL account list **MUST** be sorted lexicographically by address bytes: `addr_low` < `addr_mid` < `addr_high` < `addr_endian_low` < `addr_endian_high`, regardless of access order. The endian-trap addresses specifically catch byte-reversal bugs where addresses are compared with wrong byte order. Complements `test_bal_invalid_account_order` which tests rejection; this tests correct generation. | ✅ Completed |
 | `test_bal_transient_storage_not_tracked` | Ensure BAL excludes EIP-1153 transient storage operations | Contract executes: `TSTORE(0x01, 0x42)` (transient write), `TLOAD(0x01)` (transient read), `SSTORE(0x02, result)` (persistent write using transient value). | BAL **MUST** include slot 0x02 in `storage_changes` (persistent storage was modified). BAL **MUST NOT** include slot 0x01 in `storage_reads` or `storage_changes` (transient storage is not persisted, not needed for stateless execution). This verifies TSTORE/TLOAD don't pollute BAL. | ✅ Completed |
-| `test_bal_selfdestruct_to_precompile` | Ensure BAL captures SELFDESTRUCT with precompile as beneficiary | Caller triggers victim contract (balance=100) to execute `SELFDESTRUCT(0x0000...0001)` (ecrecover precompile). Precompile starts with balance=0. | BAL **MUST** include: (1) Contract with `balance_changes` (100→0, loses balance to selfdestruct). (2) Precompile address 0x01 with `balance_changes` (0→100, receives selfdestruct balance). Precompile **MUST NOT** have `code_changes` or `nonce_changes`. This complements `test_bal_withdrawal_to_precompiles` (withdrawal) and `test_bal_precompile_funded` (tx value). | ✅ Completed |
+| `test_bal_selfdestruct_to_precompile_and_oog` | Ensure BAL captures SELFDESTRUCT to precompile at different gas boundaries | Victim executes `SELFDESTRUCT(precompile)`. Parameterized by all precompiles and three scenarios: (1) Success, (2) OOG before state access, (3) OOG after state access. | Success: victim and precompile have `balance_changes`. OOG before state access: precompile **NOT** in BAL. OOG after state access: precompile in BAL with empty changes. | ✅ Completed |


(3) is only possible in this case if we send value, right?

If we have >= selfdestruct (5000) cost, we reach into the accessed_addresses for beneficiary to see if it's alive or not and so we mark it warm. It's a bit of a pedantic case where it may be marked "warm" even though we don't need to touch it (if 0 transfer). But as far as the access list is concerned, whether we touch it in state or not with optimizations, this means it's already warmed at this point and is in BAL. The EIP mentions this specifically in the Scope and Inclusion section, under "addresses accessed without state changes, including":

Beneficiary addresses for SELFDESTRUCT

So the gas checks are: base cost (5000) + warm/cold access + create account.

Just iterating this for myself but there are thus 4 boundaries:

Not enough gas to pay base cost

Not enough gas to pay in case target is cold (to warm it up) (If target is warm, this check costs 0)

Not enough gas to pay for creating the account

Success

So one of the cases is missing here 🤔

Also linking EIP-2929: the warm/cold check is there even if the value of SELFDESTRUCT is 0 https://eips.ethereum.org/EIPS/eip-2929#selfdestruct-changes

Yes! This is missing some cases for sure thank you. I was thinking about this from a BAL perspective as this was a BAL test ported to earlier forks (pre state access and post state access). But since we are missing coverage for this, we should be more thorough on these boundaries in general with these tests. Thanks!

…um#1954

- For successful tests, start at Homestead where precompiles were introduced (EIPs 196, 197, 198). - For OOG tests, start at Tangerine where operation gas costs were introduced (EIP 150).

…um#1954

… space)

feat(test): OOG and success selfdestruct tests to all precompiles

ed3590b

fselmo mentioned this pull request Dec 29, 2025

OOG if not enough gas for self-destruct NethermindEth/nethermind#10064

Open

16 tasks

fselmo marked this pull request as ready for review December 29, 2025 23:01

fselmo force-pushed the feat/additional-bal-selfdestruct-tests branch from 250a01c to a4faa41 Compare December 30, 2025 21:04

jochem-brouwer reviewed Dec 30, 2025

View reviewed changes

fselmo added a commit to fselmo/execution-specs that referenced this pull request Dec 30, 2025

refactor(test): Tangerine -> Tangerine whistle; comments on PR ethere…

a39caa1

…um#1954

fselmo added 3 commits December 30, 2025 16:40

feat(test): Expand fork range for selfdestruct to precompile tests

6ce0d98

- For successful tests, start at Homestead where precompiles were introduced (EIPs 196, 197, 198). - For OOG tests, start at Tangerine where operation gas costs were introduced (EIP 150).

refactor(test): Tangerine -> Tangerine whistle; comments on PR ethere…

908a792

…um#1954

fix(test): remove unnecessary isolation for enginex

88cb64f

fselmo force-pushed the feat/additional-bal-selfdestruct-tests branch from e89a230 to 88cb64f Compare December 30, 2025 23:40

fix(tool): Fix EvmOneTransitionTool parsing for TangerineWhistle (add…

dfac8a9

… space)

feat(test): OOG and success selfdestruct tests to all precompiles #1954

Are you sure you want to change the base?

feat(test): OOG and success selfdestruct tests to all precompiles #1954

Conversation

fselmo commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Description

🔗 Related Issues or PRs

✅ Checklist

Cute Animal Picture

Uh oh!

codecov bot commented Dec 29, 2025

Codecov Report

Uh oh!

fselmo commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jochem-brouwer commented Dec 30, 2025

Uh oh!

fselmo commented Dec 30, 2025

Uh oh!

jochem-brouwer commented Dec 30, 2025

Uh oh!

jochem-brouwer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jochem-brouwer Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

fselmo Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jochem-brouwer Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

jochem-brouwer Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

fselmo Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fselmo commented Dec 29, 2025 •

edited

Loading

fselmo commented Dec 30, 2025 •

edited

Loading

fselmo Dec 30, 2025 •

edited

Loading

fselmo Dec 30, 2025 •

edited

Loading