Misc Amsterdam eip fixes#4251
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Amsterdam EIP-7708 / EIP-7928 integration mismatches by adjusting EVM log emission semantics, tightening BAL (block-level access list) canonical serialization/hashing rules, and preventing synthetic system-call BAL touches from leaking into block BAL outputs.
Changes:
- Update EIP-7708 transfer/selfdestruct log emission behavior (including precompile log ordering) and adjust EIP-7928 CALL gas OOG boundary behavior.
- Canonicalize BAL serialization (quantity-like normalization, canonical sorting, system-address conditional inclusion) and preserve prior tx balance history on net-zero cleanup.
- Add tests asserting BAL canonical serialization and that fixture BAL hashes to
header.blockAccessListHash.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vm/test/tester/executionSpecBlockchain.test.ts | Adds an assertion that fixture BAL hash matches the block header blockAccessListHash. |
| packages/vm/src/requests.ts | Snapshots/restores the SYSTEM_ADDRESS BAL entry around EIP-7002/EIP-7251 system runCall() to avoid leaking synthetic touches. |
| packages/util/test/bal.spec.ts | Adds BAL unit tests for canonical quantity-like encoding and conditional SYSTEM_ADDRESS omission/inclusion. |
| packages/util/src/bal.ts | Implements BAL canonical ordering/normalization updates, net-zero balance cleanup refinement, and conditional SYSTEM_ADDRESS inclusion. |
| packages/evm/src/opcodes/gas.ts | Adjusts EIP-7928 CALL gas checking so new-account gas can OOG after committing target access. |
| packages/evm/src/interpreter.ts | Ensures EIP-7708 selfdestruct-to-self log can emit even when EIP-6780 prevents balance zeroing. |
| packages/evm/src/evm.ts | Emits EIP-7708 transfer logs for all non-DELEGATECALL value transfers and prepends logs for precompile calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const results = await vm.evm.runCall({ | ||
| caller: systemAddress, | ||
| gasLimit: vm.common.param('systemCallGasLimit'), | ||
| to: consolidationsAddress, | ||
| }) | ||
| restoreSystemAccessEntry(vm, systemAddressHex, balSnapshot) |
There was a problem hiding this comment.
Same issue as above: if vm.evm.runCall() throws, the BAL snapshot for SYSTEM_ADDRESS won't be restored. Use a try/finally to ensure restoreSystemAccessEntry() always executes.
| const results = await vm.evm.runCall({ | |
| caller: systemAddress, | |
| gasLimit: vm.common.param('systemCallGasLimit'), | |
| to: consolidationsAddress, | |
| }) | |
| restoreSystemAccessEntry(vm, systemAddressHex, balSnapshot) | |
| let results | |
| try { | |
| results = await vm.evm.runCall({ | |
| caller: systemAddress, | |
| gasLimit: vm.common.param('systemCallGasLimit'), | |
| to: consolidationsAddress, | |
| }) | |
| } finally { | |
| restoreSystemAccessEntry(vm, systemAddressHex, balSnapshot) | |
| } |
| const storageChanges = ( | ||
| Object.entries(data.storageChanges) as [BALStorageKeyHex, BALRawStorageChange[]][] | ||
| ) | ||
| .sort((a, b) => a[0].localeCompare(b[0])) | ||
| .map(([slot, changes]) => [normalizeHexForRLP(slot), changes.sort((a, b) => a[0] - b[0])]) | ||
| .sort((a, b) => compareLexicographicHexOrBytes(a[0], b[0])) | ||
| .map(([slot, changes]) => [ | ||
| normalizeHexForRLP(slot), | ||
| changes | ||
| .sort((a, b) => a[0] - b[0]) | ||
| .map( | ||
| ([index, value]) => | ||
| [index, normalizeBytesForRLPQuantity(value)] as BALRawStorageChange, | ||
| ), | ||
| ]) |
There was a problem hiding this comment.
storageChanges are sorted before slot normalization (normalizeHexForRLP(slot)), but normalization strips leading zeros. If JSON/inputs contain padded slots (e.g. 0x00ff), the pre-normalization order can differ from the canonical post-normalization order, producing a non-canonical RLP/hash. Sort using the normalized slot bytes (e.g. normalize first, then sort on the normalized value) to ensure deterministic canonical ordering.
| const systemAddressHex = systemAddress.toString() | ||
| const balSnapshot = cloneSystemAccessEntry(vm, systemAddressHex) | ||
| const results = await vm.evm.runCall({ | ||
| caller: systemAddress, | ||
| gasLimit: vm.common.param('systemCallGasLimit'), | ||
| to: withdrawalsAddress, | ||
| }) | ||
| restoreSystemAccessEntry(vm, systemAddressHex, balSnapshot) |
There was a problem hiding this comment.
restoreSystemAccessEntry() is not guaranteed to run if vm.evm.runCall() throws (e.g., an invalid precompile can throw from runPrecompile). This can leak the system address BAL entry into subsequent processing. Wrap the runCall() in a try { ... } finally { restoreSystemAccessEntry(...) } so restoration happens even on exceptions.
gabrocheleau
left a comment
gabrocheleau
left a comment
There was a problem hiding this comment.
Hey @ScottyPoi ! nice work.
Would you be able to fix the build issue? The issue is related to the request.ts file.
Also I'm noticing some AI copilot comments on the PR related to try/catch & restoration. Would be great if you can evaluate if those are relevant or not. Would then give this a manual review and merge.
Thanks
ScottyPoi
force-pushed
the
amsterdam-fixes
branch
from
1ee96b5 to
b628707
Compare
ScottyPoi
force-pushed
the
amsterdam-fixes
branch
from
b628707 to
77f2214
Compare
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 |
* evm: align 7708 logs * util: normalize BAL output * vm: ignore system call BAL * test(vm): check fixture BAL hash * fix linting --------- Co-authored-by: ScottyPoi <scott.simpson@ethereum.org> Co-authored-by: Gabriel Rocheleau <18757482+gabrocheleau@users.noreply.github.com>
This PR fixes several Amsterdam EIP-7708 / EIP-7928 integration issues across EVM, VM, and BAL serialization.
Fixes failling test fixtures from
v500_mixed_with_other_eipsAlign EIP-7708 log emission with the execution-spec fixtures:
Fix block-level access list serialization and cleanup:
0x/0x0/0x00/ odd-length values)SYSTEM_ADDRESSonly when it has real BAL contentExclude synthetic BAL writes introduced by post-block system calls:
SYSTEM_ADDRESSBAL entry around EIP-7002 / EIP-7251 systemrunCall()execution so internal caller bookkeeping does not leak into the block BALAdd coverage:
blockAccessListHash