Skip to content

Commit 306e2c9

Browse files
authored
db/state: skip DomainDel(AccountsDomain) when prev value is already nil (erigontech#21939)
Fixes erigontech#21685. ## Summary A second `DomainDel(AccountsDomain, addr)` against an already-deleted account currently writes a duplicate nil-history entry; serial's IBS short-circuits in this case so the duplicate is parallel-only. Per @sudeepdino008's analysis on erigontech#21685, mainnet block 2,713,395's two DAO-cleanup txs each emit `SelfDestructPath=true` against the same ~200 already-empty accounts when run under parallel exec — both flushes write nil entries where serial writes only one. Final domain value is the same (nil) in both modes, so reads are correct, but the on-disk `accounts.0-256.v/.ef`, `commitment.0-256.v/.ef`, `rcache.0-256.v` history files diverge from r3.4. Fix: mirror the existing `kv.CodeDomain` skip a few lines below — when `prevVal == nil` after the GetLatest fetch, skip the AccountsDomain delete entirely. Storage prefix wipe and CodeDomain delete that would have run downstream would have been no-ops anyway. ## Test - `TestSharedDomain_DomainDel_IdempotentForAlreadyDeletedAccount` — puts an account at txNum=0, deletes at txNum=1, re-deletes at txNum=2; asserts the history index records exactly `[0, 1]`, not `[0, 1, 2]`. - Red-first verified: without the fix, the test fails with `[0, 1, 2]`. - Existing `db/state/execctx` test suite still green. ## Test plan - [x] New test green; red-first behaviour confirmed. - [x] `make lint` clean (x2, lint is non-deterministic). - [x] `make erigon` clean. - [ ] Mainnet exec-from-0 on the deterministic-snapshot harness: verify the three history-file hashes (`accounts.0-256.v/.ef`, `commitment.0-256.v/.ef`, `rcache.0-256.v`) match r3.4 baseline after this fix. - [ ] CI green. ## Backport A `[r3.5]` cherry-pick PR will follow.
1 parent 7dd7942 commit 306e2c9

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

execution/state/parallel_fixes_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,55 @@ func TestBlockStateCacheWriteAccountUpdatesCurrent(t *testing.T) {
397397
assert.Equal(t, enc2, current, "GetCurrentAccount should return the latest write")
398398
}
399399

400+
// Pins that a second DeleteAccount in the same block is a writeLog no-op —
401+
// Flush must emit exactly one DomainDel per address (matching serial's IBS
402+
// short-circuit). Without this dedup the redundant nil-history entry feeds
403+
// into commitment-cache step keys and produces a non-deterministic state
404+
// root between parallel-exec nodes validating each other's blocks.
405+
func TestBlockStateCacheDeleteAccount_IdempotentInBlock(t *testing.T) {
406+
cache := NewBlockStateCache()
407+
addr := accounts.InternAddress([20]byte{0x77})
408+
409+
cache.DeleteAccount(addr, 1)
410+
cache.DeleteAccount(addr, 2)
411+
412+
deletes := 0
413+
for i := range cache.writeLog {
414+
if cache.writeLog[i].kind == bcOpDeleteAccount && cache.writeLog[i].addr == addr {
415+
deletes++
416+
}
417+
}
418+
assert.Equal(t, 1, deletes, "second DeleteAccount in the same block must not append a second writeLog entry")
419+
420+
enc, present := cache.GetCurrentAccount(addr)
421+
assert.True(t, present, "current view must still report addr as present-and-empty")
422+
assert.Nil(t, enc, "current view value must remain nil")
423+
}
424+
425+
// Pins the recreate-then-redelete pattern: an intervening WriteAccount
426+
// resets the dedup so the next DeleteAccount IS recorded — only the
427+
// "no write in between" duplicate is collapsed.
428+
func TestBlockStateCacheDeleteAccount_RecreateThenDeleteRecords(t *testing.T) {
429+
cache := NewBlockStateCache()
430+
addr := accounts.InternAddress([20]byte{0x88})
431+
432+
acc := accounts.NewAccount()
433+
acc.Balance = *uint256.NewInt(1)
434+
enc := accounts.SerialiseV3(&acc)
435+
436+
cache.DeleteAccount(addr, 1)
437+
cache.WriteAccount(addr, enc, 2)
438+
cache.DeleteAccount(addr, 3)
439+
440+
deletes := 0
441+
for i := range cache.writeLog {
442+
if cache.writeLog[i].kind == bcOpDeleteAccount && cache.writeLog[i].addr == addr {
443+
deletes++
444+
}
445+
}
446+
assert.Equal(t, 2, deletes, "delete after recreate must be recorded; only no-op duplicates are collapsed")
447+
}
448+
400449
// TestSelfDestructKeepsDirtyStorageReadableSameTx verifies that after an
401450
// account self-destructs (versionMap active), a subsequent same-tx GetState
402451
// still returns the dirty value written before the SELFDESTRUCT. Pre-Cancun

execution/state/rw_v3.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,6 +1205,12 @@ func (c *BlockStateCache) DeleteAccount(addr accounts.Address, txNum uint64) {
12051205
// Mark deleted in the current view so subsequent in-block reads / puts
12061206
// see the destruction immediately. nil (not absent) so GetCurrentAccount
12071207
// reports "present but empty" rather than falling back to committed state.
1208+
if v, present := c.currentAccounts[addr]; present && v == nil {
1209+
// Already deleted by an earlier tx in this block — match serial's
1210+
// IBS short-circuit so Flush emits a single DomainDel per address.
1211+
c.mu.Unlock()
1212+
return
1213+
}
12081214
c.currentAccounts[addr] = nil
12091215
delete(c.currentCode, addr)
12101216
delete(c.currentStorage, addr)

0 commit comments

Comments
 (0)