execution/state: mirror createObjectChange dirty-tracking on resetObjectChange (#21138)#21220
execution/state: mirror createObjectChange dirty-tracking on resetObjectChange (#21138)#21220mh0lt wants to merge 1 commit into
Conversation
…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.
|
@yperbasis — both blockers from this PR's review are now cleared and the change is ready for re-review:
FYI: I've also cherry-picked this commit onto #21017's branch ( If this PR can clear review in the next day or so, great. Otherwise the fix lands via #21017 and we can close this as resolved-elsewhere. |
mh0lt
force-pushed
the
mh/21138-pr3-dirtied-symmetric
branch
from
799d6ad to
a7169ca
Compare
|
Force-pushed the rebased branch (now |
Pull request was closed
3 participants
Stack
Depends on: #21212 — #21138 PR 2 (minIBS extraction). Must merge first.
Summary
Third in the #21138 heuristic / IBS-dependency removal stack. Restores symmetric dirty-tracking between
createObjectChange.dirtied()andresetObjectChange.dirtied()so parallel-exec workers don't drop the post-SD-revival writeset.`journal.go` defined the two journal entry types for the same logical "createObject" event with non-symmetric `dirtied()` returns:
```go
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; the asymmetry was an oversight.
Manifests under parallel-exec when tx1 SD's an address and tx2 hits `CreateAccount` / `GetOrNewStateObject` on the same address:
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).
Diagnosis credit: original investigation by Mark Holt on PR #21207 thread.
Known regression — tracked in #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=`). The empty-touch / `CreateAccount` paths this fix addresses don't have this issue; the `AddBalance(non-zero)` on an SD'd address does.
Filed as #21217 with full repro and two failed narrow-fix attempts (unconditional symmetry — this PR; conditional SD-revival + `SelfDestructPath=false` re-emit — also regresses). The right narrower fix needs more diagnosis on the validator's read path.
Per coordination with the original diagnosis author, this PR lands as the last commit in the #21138 cleanup stack so the broader structural direction is visible together; the `TestSelfDestructReceive` regression is the explicit "more work needed" marker before final merge.
What's verified
Under `EXEC3_PARALLEL=true` on the fix branch:
What's next
Related