execution/state, execution/stagedsync: retain AuRa SystemAddress in all EIP-161 empty-removal paths#21916
execution/state, execution/stagedsync: retain AuRa SystemAddress in all EIP-161 empty-removal paths#21916yperbasis wants to merge 1 commit into
Conversation
…ll EIP-161 empty-removal paths The AuRa hack — never EIP-161-remove the empty SystemAddress (0xff…fe), to match the reference implementation — lived only in the serial executor's updateAccount. Extract it into a shared EIP161EmptyRemoval helper and apply it at every empty-account-removal decision site: - updateAccount (serial executor) — refactored onto the helper - printAccount (debug mirror) - StateV3.applyVersionedWrites balance-increase apply (rw_v3.go) - calcFees coinbase and burnt fee accounts (parallel executor) - normalizeWriteSet, per touched account (parallel executor) The parallel-executor sites (calcFees, normalizeWriteSet, applyVersionedWrites) are EIP-7928/BAL paths that AuRa does not exercise today, so the change is correctness-by-construction / consistency there; the serial path is the consensus-relevant one for AuRa chains. Adds a table test for the helper and a normalizeWriteSet test pinning that an empty SystemAddress is retained under AuRa and removed otherwise.
There was a problem hiding this comment.
Pull request overview
This PR centralizes the AuRa-specific “never EIP-161-remove the empty SystemAddress (0xff…fe)” rule into a shared EIP161EmptyRemoval(...) helper and applies it consistently across serial execution, debug printing, state apply, and parallel-executor fee/write normalization paths to prevent AuRa state-root divergence.
Changes:
- Added
EIP161EmptyRemoval(spuriousDragon, isAura, addr)helper and refactored serialupdateAccount+ debugprintAccountto use it. - Applied the helper to additional empty-account deletion decision points (
StateV3.applyVersionedWrites, parallelcalcFees,normalizeWriteSet). - Added targeted tests for the helper and for
normalizeWriteSetretainingparams.SystemAddressunder AuRa.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| execution/state/versionedio_test.go | Adds table test for EIP161EmptyRemoval. |
| execution/state/rw_v3.go | Uses EIP161EmptyRemoval when deciding whether balance-increase results in an empty-account delete. |
| execution/state/intra_block_state.go | Introduces EIP161EmptyRemoval helper; threads AuRa flag into printAccount; refactors deletion gating. |
| execution/stagedsync/exec3_parallel.go | Applies helper in parallel fee calculation and empty-account deletion in normalizeWriteSet; extends normalizeWriteSet signature with isAura. |
| execution/stagedsync/exec3_finalize_test.go | Updates existing normalizeWriteSet tests and adds AuRa SystemAddress retention test. |
| execution/stagedsync/calc_state_test.go | Updates normalizeWriteSet call sites in pipeline tests to pass isAura. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| emptyAddrs := make(map[accounts.Address]bool) | ||
| if emptyRemoval { | ||
| for addr, s := range acctStates { | ||
| if s.hasBal && s.hasNonce && s.hasCode && | ||
| s.balance.IsZero() && s.nonce == 0 && s.codeHash.IsEmpty() { | ||
| emptyAddrs[addr] = true | ||
| } | ||
| for addr, s := range acctStates { | ||
| if state.EIP161EmptyRemoval(emptyRemoval, isAura, addr) && | ||
| s.hasBal && s.hasNonce && s.hasCode && | ||
| s.balance.IsZero() && s.nonce == 0 && s.codeHash.IsEmpty() { |
| // EIP161EmptyRemoval reports whether an empty account at addr is removed under | ||
| // EIP-161 (SpuriousDragon). AuRa retains its SystemAddress even when empty, to | ||
| // match the reference implementation. | ||
| func EIP161EmptyRemoval(spuriousDragon, isAura bool, addr accounts.Address) bool { | ||
| return spuriousDragon && (!isAura || addr != params.SystemAddress) | ||
| } |
|
Cross-link: I just opened #21930 (and backport #21931 to release/3.5) which fixes a different but related EIP-161-in-parallel bug — chiado wrong-trie-root at block 0 from the AuRa The two will conflict line-for-line at that callsite. Both PRs are also AuRa-specific empty-account-handling fixes in the parallel path, so the natural end state looks like a single helper covering both carve-outs: EIP161EmptyRemoval(spuriousDragon, isAura, addr, blockNum)
// your AuRa SystemAddress carve-out (this PR)
// + AuRa block-0 ZeroAddress carve-out (#21930)I'm also locally investigating an apparently-related gnosis tip-tracking wedge (deterministic |
|
Update from local test: this PR fixes the gnosis-tip dep-0 wedge I mentioned above. Setup: I had a release/3.5 gnosis-tip-tracking instance wedged for 6 days at block 46,710,713, deterministically failing every retry with: at I snapshotted the wedged chaindata (hardlinked snapshots/, ~42G real cost), built 909 blocks applied past the wedge point in a single clean cycle, zero errors. That includes block 46710713 (the dep-0 site) and the next 906. So your hypothesis that "AuRa doesn't exercise the parallel-exec sites today" is conservative — gnosis tip does hit I'll close #21930/#21931 and propose a consolidated PR that extends your |
|
Update: since #21930 merged this morning, I've rebased this PR's commit onto current main and opened it as the next-layer fix in the family, with the verification evidence from my wedged-datadir test:
I think this PR (#21916) can be closed in favour of #21937 — same code, but now with the gnosis-tip-wedge verification attached and rebased onto post-#21930 main. Happy for you to close it or for it to land as-is — either way, the change is the same. |
|
Closing in favour of #21937 |
…ll EIP-161 empty-removal paths (extend erigontech#21930 family) (erigontech#21937) Cherry-pick of erigontech#21916 (yperbasis/aura-retain-system-address) onto current main. The original ZeroAddress-at-genesis fix (erigontech#21930) is already merged; this PR adds the matching SystemAddress carve-out, completing the AuRa EIP-161 family. Fixes the gnosis tip-tracking dep-0 abort wedge (verified locally — see verification below). ## Background Two AuRa-specific EIP-161 bugs in parallel exec share the same callsite (`normalizeWriteSet`) and the same family: 1. **erigontech#21930 (already merged):** Block-0 ZeroAddress at genesis. Mirrors serial's `rules = &chain.Rules{}` clobber at `txtask.Execute` so the AuRa `CreateAccount(ZeroAddress, false)` placeholder is retained in parallel. Gates `emptyRemoval` with `be.blockNum != 0` at the callsite. 2. **This PR (cherry-pick of erigontech#21916):** AuRa SystemAddress carve-out everywhere. Extracts `EIP161EmptyRemoval(spuriousDragon, isAura, addr)` helper that exempts the SystemAddress under AuRa; applies it at `updateAccount`, `printAccount`, `calcFees` (coinbase + burnt), `applyVersionedWrites`, and `normalizeWriteSet`. Both layers are now compatible: the block-0 gate sets `emptyRemoval=false` at genesis for any chain (preserves all empty allocs); the helper inside `normalizeWriteSet` then carves out SystemAddress on AuRa for blocks where `emptyRemoval=true`. ## Verification - Existing tests: `TestNormalize|TestFlushToUpdates|TestApplyWrites|TestSD|TestEIP161EmptyRemoval` all green on this branch + lint clean + `make erigon` clean. - **Gnosis tip-tracking wedge — fixed.** I had a release/3.5 gnosis instance wedged for 6 days at block 46,710,713 with deterministic `DependencyTxIndex=0` abort on the AuRa system-init at `forkchoice.go:520 → exec3_parallel.go:301`. I snapshotted the wedged chaindata (~42G real cost via hardlinked snapshots/), built yperbasis's branch at `e3409b6`, and ran it against the snapshot. Result: `[4/6 Execution] parallel executed blk=46711620 blks=909` — **909 blocks applied past the wedge point in one clean cycle, zero errors**. The dep-0 abort was the AuRa system-init writing an empty SystemAddress through `calcFees`/`normalizeWriteSet` (no AuRa carve-out, unlike serial's `updateAccount`), which poisoned OCC dependency tracking for the next block's system-init in the same batch. ## Co-authored by Andrew Ashikhmin / @yperbasis — author of the original erigontech#21916 commit, cherry-picked verbatim. ## Backport A `[r3.5]` cherry-pick PR will follow. ## Supersedes - erigontech#21916 (this is the same diff, rebased onto current main on top of erigontech#21930) Co-authored-by: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com>
What
The AuRa SystemAddress non-removal hack — never EIP-161-remove the empty
SystemAddress(0xff…fe), to match the reference AuRa implementation — previously lived only in the serial executor'supdateAccount. This extracts it into a sharedEIP161EmptyRemoval(spuriousDragon, isAura, addr)helper and applies it at every empty-account-removal decision site:updateAccountprintAccountStateV3.applyVersionedWrites(balance increases)calcFeescoinbase + burntnormalizeWriteSet(per touched account)Why
updateAccountwas the only site that exempted the SystemAddress. Any other path that converts an empty account to a delete would diverge from the reference AuRa state root if it processed an empty SystemAddress — e.g. a block-reward credit that nets to empty, or the coinbase set toSystemAddressduring an AuRa system call.The three parallel-executor sites (
calcFees,normalizeWriteSet,applyVersionedWrites) are EIP-7928/BAL paths that AuRa does not exercise today, so the change is correctness-by-construction / consistency there; the serial path is the consensus-relevant one for AuRa chains.Tests
TestEIP161EmptyRemoval— table test for the helper.TestNormalizeWriteSet_AuraSystemAddressRetained— pins that an empty SystemAddress is retained under AuRa and removed otherwise (written red-first: it fails on the retain assertion before the suppression is added).#21915 reworks the same EIP-161 gating lines: it introduces
IsEIP161Enabled()(=IsSpuriousDragon && !DisabledEIPs(161)) and swapsIsSpuriousDragon → IsEIP161Enabled()in theupdateAccount/printAccountcallsites,rw_v3.go,calcFees, and thenormalizeWriteSetcall. The two changes are complementary but conflict line-for-line, so whichever lands second will need a rebase.The intended unified end state is for
EIP161EmptyRemovalto take the EIP-161-enabled bool with callers passingchainRules.IsEIP161Enabled(), so the predicate composes both theDisabledEIPsgating (#21915) and the AuRa SystemAddress exemption (this PR). Kept as a separate draft pending coordination with #21915.