Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes EIP-7708 execution-spec failures related to SELFDESTRUCT finalization logs in the Amsterdam fork by deferring log emission to transaction cleanup time and recomputing the bloom filter afterward.
Changes:
- Changed selfdestruct tracking from
Set<PrefixedHexString>toMap<PrefixedHexString, PrefixedHexString>(address → beneficiary) across the EVM package - Added finalization log emission in
processSelfdestructs()to capture residual balance at cleanup time - Moved bloom filter generation to after selfdestruct finalization so receipts include finalization logs
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/evm/src/types.ts | New SelfdestructMap type; updated selfdestruct field types |
| packages/evm/src/message.ts | Updated selfdestruct field type to SelfdestructMap |
| packages/evm/src/interpreter.ts | Changed selfdestruct from Set to Map, storing beneficiary address |
| packages/evm/src/evm.ts | Updated Set→Map defaults for selfdestruct |
| packages/evm/src/eip7708.ts | New createEIP7708SelfdestructLog helper |
| packages/evm/src/index.ts | Exported new type and eip7708 module |
| packages/vm/src/runTx.ts | Finalization logs in processSelfdestructs, bloom moved after |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Bundle Size Analysis
Values are minified+gzipped bundles of each package entry. Workspace deps are bundled; external deps are excluded. Generated by bundle-size workflow |
ScottyPoi
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes the remaining Amsterdam EIP-7708 execution-spec failures around
SELFDESTRUCTfinalization logs.The bug was that we treated
SELFDESTRUCTlogging as an opcode-time concern only. That works for immediate balance transfers, but it is wrong for the Amsterdam cases where a contract selfdestructs, then later in the same transaction receives more ETH or priority-fee proceeds. In those cases, the spec expects a finalization log based on the contract’s residual balance at tx cleanup time, not just the balance that existed when the opcode executed.There were two related problems:
We emitted
SELFDESTRUCT-related EIP-7708 logs too early.We built the tx bloom before selfdestruct finalization was processed.
receiptsRootandlogsBloommismatches in the execution-spec fixtures.Fixes:
1. Preserve selfdestruct metadata across execution
I changed selfdestruct tracking in the EVM from a bare
Setto a map keyed by selfdestructed address.That fixes an information loss problem: nested calls and creates now propagate selfdestruct context explicitly instead of only remembering that “some account selfdestructed”. This makes finalization handling deterministic and gives the VM enough information to reason about selfdestructed accounts during tx cleanup.
2. Emit finalization logs during tx cleanup
In
runTx,processSelfdestructs()now:This matches the Amsterdam spec behavior: finalization logs are based on the post-execution/post-fee balance that actually exists when the account is cleaned up.
3. Recompute the tx bloom after finalization logs exist
I moved tx bloom generation to happen after
processSelfdestructs().That fixes the header mismatches directly, because the receipt bloom and receipt trie now include the finalization logs that were previously missing from the receipt payload.