✨ feat(tests): EIP-7928 SELFDESTRUCT test

[raxhvl]

🗒️ Description

Test case effects of SELFDESTRUCT on EIP-7928.

cc: @nerolation bal_changes for the self-destructing contract is captured too.

Additional changes

• Validate not just ordering but duplicate entries in the BAL coming from EELS / t8n
• Restrict extra keys in the models used for BALs so we fail on unexpected kwargs used

✅ Checklist

• All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:

  uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint

• All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
• All: Set appropriate labels for the changes (only maintainers can apply labels).

[fselmo]

Lgtm overall, thanks for getting this in 👌🏼. I left some comments to think about but also, I don't expect selfdestruct tests to grow too large. I don't think these deserve their own file for this reason, please correct me if I'm wrong though. Perhaps we should move these into the positive (err... non-invalid) test cases for now instead of their own file. Thoughts?

[raxhvl]

re: creating a separate file for this test

This is purely done to avoid conflicts while I work on these PRs in parallel (I have done the same in other PRs too). Once all PRs are merged, we could group them. Let me know if you want me to move them now.

[fselmo]

This is purely done to avoid conflicts while I work on these PRs in parallel (I have done the same in other PRs too). Once all PRs are merged, we could group them. Let me know if you want me to move them now.

Oh I see. I wouldn't think it's difficult to rebase if these are standalone cases. It would be better to PR them to the appropriate place, but if that's messy I suppose that's fine. Don't let this be a blocker at all.

[raxhvl]

Just so you are in the loop @fselmo

Self destructs will now include an address with postNonce=0 and newCode=0x. The rationale is that setting post nonce to 0 is effectively an account reset operation, and no other operation except SELFDESTRUCT should be able to do it.

[fselmo]

@nerolation @raxhvl this should be updated in the EIP specifications, right? Anyone working on that?

[nerolation]

Yeah on it!

[fselmo]

LGTM! We should move the test into test_block_access_lists.py before merging since self destruct cases shouldn't need a standalone file. I also updated the eels resolutions to point to the latest specs commit for Toni's changes that were merged in.

[nerolation]

Will fix it - thanks!

[fselmo]

@raxhvl... @nerolation and I worked on self destruct back and forth some at the end of last week to get the specs ironed out and I added the specific case of the pre-funded self-destruct account where we want to record the balance change as 0 in the end. I cherry picked this here.

I also pointed to the latest commit in the spec for the resolver. All the tests are currently filling here now.

I think this is good to go? Let me know if we're missing anything 👀

[raxhvl]

Thanks @fselmo I made the following changes

1. parameterize pre_funded separately from self_destruct_in_same_tx to get better test coverage for both cases
2. Extend the self destruct bytecode to include storage operation (read and write)

@nerolation Here is the impact on specs

1. If the account has been prefunded before self destruct tx (sending ether to a create 2 tx before deploying it) then self destruct will include a balance_change -> 0

[fselmo]

Depends on fselmo/execution-specs#13

I commented on the issue here, the tests here were not expecting the right things. This does fill appropriately now (fixed here. I think this PR is good to be merged.

[spencer-tb]

LGTM! Couple small comments. No blockers. Feel free to merge :D