execution/stagedsync: extract minIBS post-apply path into a dedicated method#21212
Merged
AskAlexSharov merged 1 commit intoMay 19, 2026
Merged
Conversation
… method Pure structural cleanup of finalizeTxSimple — no semantic change. Pulls the ~55 lines that construct a minimal IntraBlockState to feed postApplyMessageFunc (AuRa system calls / EIP-7708 burn-log emission via LogSelfDestructedAccounts) into its own method, runPostApplyMessageOnMinIBS. The extraction has two purposes: * Names and isolates the load-bearing IBS dependency in finalizeTxSimple's post-execution path. The new godoc enumerates the three IBS deps the method carries today (GetRemovedAccountsWithBalance lookup, AddLog/GetLogs log buffering, AddBalance fee-credit bookkeeping) so the IBS-removal PR that follows has a clearly-flagged swap point. * Shrinks finalizeTxSimple's body from 257 → 205 lines, removing the inline ibs.New / ApplyVersionedWrites / AddBalance / GetLogs sequence from the function the parallel-exec stack reads daily. Behavior is identical: the new method runs the same engine post-apply hook with the same IBS construction, in the same control flow position. CollectorWrites / allWrites construction, fee-balance math, receipt creation, and return values are untouched. Verified under EXEC3_PARALLEL=true: * make lint clean * execution/stagedsync, execution/state, execution/tests, execution/engineapi, execution/execmodule, rpc/jsonrpc test-short green * TestEIP7708BurnLogWhenCoinbaseSelfDestructs green * TestEngineApiBAL family green * TestLegacyBlockchain/ValidBlocks/bcEIP3675 green Stacks on #21211. Sets up the IBS-removal PR in the #21138 sequence.
This was referenced May 15, 2026
AskAlexSharov
approved these changes
May 19, 2026
Member
|
@mh0lt this PR targets some temporary branch, not |
mh0lt
added a commit
that referenced
this pull request
May 23, 2026
…ectChange
journal.go defined two journal entry types for the same logical
"createObject" event with non-symmetric dirtied() returns:
func (ch createObjectChange) dirtied() (accounts.Address, bool) { return ch.account, true }
func (ch resetObjectChange) dirtied() (accounts.Address, bool) { return accounts.NilAddress, false }
createObject in intra_block_state.go picks between them on
`previous == nil` — first-creation goes through createObjectChange,
recreation (e.g. SD-revival via GetOrNewStateObject) goes through
resetObjectChange. Both represent the same operation ("a stateObject
was placed at this address"); they differ only in revert behaviour.
The asymmetry bites parallel-exec when tx1 selfdestructs an address
and tx2 hits CreateAccount or GetOrNewStateObject on the same address:
* Serial: tx1's writer already DeleteAccount'd the addr, so
getStateObject returns nil → createObject(addr, nil)
appends createObjectChange → marks journal.dirties.
* Parallel: versionedRead returns the contract's base-state account
and reads tx1's SelfDestructPath=true; createObject
synthesises a non-nil previous with selfdestructed=true
→ appends resetObjectChange → with the old return,
does NOT mark journal.dirties.
At MakeWriteSet the worker IBS computes isDirty from journal.dirties.
With no mark, updateAccount falls through both DELETE and UPDATE
branches → LightCollector.UpdateAccountData never fires →
result.CollectorWrites is missing the empty-account write
(test_double_kill / EEST EIP-6780 family on the parallel shard).
Symmetric tracking restores the dirty mark for the recreate path
without changing first-create behaviour. Verified on
TestEngineApiBAL*, TestEIP7708BurnLog*, TestDeleteRecreate* under
EXEC3_PARALLEL=true.
## Known regression — see #21217
TestSelfDestructReceive fails under EXEC3_PARALLEL=true after this
change with a wrong-trie-root for block 1. The validator's
stateObject reconstruction for an SD-then-revived address emits
different field values (`nonce=1, codehash=emptyHash`) than the
unfixed canonical (`nonce=0, codehash=<nil>`). The empty-touch /
CreateAccount paths the fix addresses don't have this issue; the
AddBalance(non-zero) on an SD'd address does.
Filed as #21217 with full repro and the two failed narrow-fix
attempts (unconditional symmetry; conditional SD-revival +
SelfDestructPath=false re-emit). Lands as the last commit in this
stack so the broader structural direction is visible together; the
TestSelfDestructReceive regression is the explicit "more work
needed" marker before final merge.
Stacks on #21212.
mh0lt
added a commit
that referenced
this pull request
May 23, 2026
…ectChange
journal.go defined two journal entry types for the same logical
"createObject" event with non-symmetric dirtied() returns:
func (ch createObjectChange) dirtied() (accounts.Address, bool) { return ch.account, true }
func (ch resetObjectChange) dirtied() (accounts.Address, bool) { return accounts.NilAddress, false }
createObject in intra_block_state.go picks between them on
`previous == nil` — first-creation goes through createObjectChange,
recreation (e.g. SD-revival via GetOrNewStateObject) goes through
resetObjectChange. Both represent the same operation ("a stateObject
was placed at this address"); they differ only in revert behaviour.
The asymmetry bites parallel-exec when tx1 selfdestructs an address
and tx2 hits CreateAccount or GetOrNewStateObject on the same address:
* Serial: tx1's writer already DeleteAccount'd the addr, so
getStateObject returns nil → createObject(addr, nil)
appends createObjectChange → marks journal.dirties.
* Parallel: versionedRead returns the contract's base-state account
and reads tx1's SelfDestructPath=true; createObject
synthesises a non-nil previous with selfdestructed=true
→ appends resetObjectChange → with the old return,
does NOT mark journal.dirties.
At MakeWriteSet the worker IBS computes isDirty from journal.dirties.
With no mark, updateAccount falls through both DELETE and UPDATE
branches → LightCollector.UpdateAccountData never fires →
result.CollectorWrites is missing the empty-account write
(test_double_kill / EEST EIP-6780 family on the parallel shard).
Symmetric tracking restores the dirty mark for the recreate path
without changing first-create behaviour. Verified on
TestEngineApiBAL*, TestEIP7708BurnLog*, TestDeleteRecreate* under
EXEC3_PARALLEL=true.
## Known regression — see #21217
TestSelfDestructReceive fails under EXEC3_PARALLEL=true after this
change with a wrong-trie-root for block 1. The validator's
stateObject reconstruction for an SD-then-revived address emits
different field values (`nonce=1, codehash=emptyHash`) than the
unfixed canonical (`nonce=0, codehash=<nil>`). The empty-touch /
CreateAccount paths the fix addresses don't have this issue; the
AddBalance(non-zero) on an SD'd address does.
Filed as #21217 with full repro and the two failed narrow-fix
attempts (unconditional symmetry; conditional SD-revival +
SelfDestructPath=false re-emit). Lands as the last commit in this
stack so the broader structural direction is visible together; the
TestSelfDestructReceive regression is the explicit "more work
needed" marker before final merge.
Stacks on #21212.
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
3 participants
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.
Stack
Depends on: #21211 — first cut of #21138's heuristic / IBS-dependency removal sequence (dead-finalize cleanup). Must merge first.
Summary
Pure structural cleanup of
finalizeTxSimple. Pulls the ~55 lines that construct a minimalIntraBlockStateto feedpostApplyMessageFunc(AuRa system calls / EIP-7708 burn-log emission viaLogSelfDestructedAccounts) into its own method,runPostApplyMessageOnMinIBS.No semantic change. Behavior identical: same engine post-apply hook, same IBS construction, same control flow position.
Why
This is preparation for the IBS-removal PR that follows in the #21138 sequence. The end-state #21138 drives toward is one
finalizeTxfunction, no IBS used in the parallel-exec code path outside workers. Today the live IBS dependency infinalizeTxSimpleis entangled with surrounding fee-credit logic that has nothing to do with IBS — pulling them apart here, without changing behavior, makes the IBS-removal PR small enough to review confidently.The extracted method's godoc enumerates the three IBS dependencies it carries today, so the swap point is clearly flagged:
ibs.GetRemovedAccountsWithBalance()lookup (consumed insideLogSelfDestructedAccounts).ibs.AddLog→ibs.GetLogslog-buffer round-trip (so EIP-7708 burn logs reach the receipt).ibs.AddBalancefee-credit bookkeeping (so the SD'd coinbase carriesFeeTippedat the timeLogSelfDestructedAccountsinspects it).finalizeTxSimpleshrinks from 257 → 205 lines; the IBS-using block becomes a single method call.Test plan
make lintcleanmake test-shortgreen underEXEC3_PARALLEL=trueacross:execution/stagedsync,execution/state,execution/tests,execution/engineapi,execution/execmodule,rpc/jsonrpcTestEIP7708BurnLogWhenCoinbaseSelfDestructsgreenTestEngineApiBAL*family greenTestLegacyBlockchain/ValidBlocks/bcEIP3675greenWhat's next in the sequence
runPostApplyMessageOnMinIBS. Add explicit SD-with-balance computation onExecutionResult, changeLogSelfDestructedAccountsto return[]*types.Loginstead of usingibs.AddLog, and remove theibs.AddBalancebookkeeping (which exists only to informLogSelfDestructedAccounts).finalizeTxSimplebecomes IBS-free.normalizeWriteSetwithfilterWritesByVersionMap; replacecalcState.ApplyWriteswithVersionedWrites.TouchUpdates; move EIP-7002/7251 syscalls into the worker pool (separate PR — requires interface change toEngine.Finalize/SysCallContract).Related