execution/state: skip the self-destruct storage walk when the address has no account - #23604
execution/state: skip the self-destruct storage walk when the address has no account#23604AskAlexSharov wants to merge 4 commits into
Conversation
… has no account A self-destruct emits DELETE for every committed storage slot, which means a prefix walk over the storage domain. That walk seeks the .bt index of every storage .kv file, so it costs the same whether the address owns a thousand slots or none. An address with no committed account owns none: storage is only written for an account that exists, and deleting an account wipes its storage prefix. Probing the account first is served by the per-file existence filters, so the walk is skipped for a contract created and destroyed inside one batch, which never has a committed account. That is the whole of the traffic this was found on -- early 2019 gas-token burns destroy 42-44 storage-less children per transaction, and on mainnet blocks 7319406-7320491 every one of those walks came back empty: sdDomainCalls == sdDomainEmpty in all 167 logged cascades, 1.45s of a 12.65s execution phase. Same argument and the same assert as the create-time wipe, so the four copies of the walk collapse into one helper.
|
CI found a real defect, not a flake.
Self-destruct in one tx, recreate at the same address in a later tx of the same block. The guard's premise — no committed account implies no committed storage — holds for state committed across batches, but not inside one: once the self-destruct has removed the account, a later tx's Normalize sees no account and skips the walk, so the storage-delete cascade the trie needs is never emitted, and the root goes wrong. Do not merge as-is. The probe has to distinguish 'never had an account' from 'account was removed earlier in this batch'. |
|
Root cause found: the self-destruct apply path does not wipe the storage prefix when the block-state cache is active.
blockCache.DeleteAccount(addr, txNum)
...
if pureDelete { continue } // no DomainDelPrefixagainst the So between a self-destruct and the end of the block: the account reads empty while the domain still holds the pre-block storage. That is the window the probe cannot see, and it is why only the The discriminator exists, though: That needs the cache threaded into |
… in this block The account probe alone was wrong. With a block cache active the self-destruct apply path routes the delete through BlockStateCache.DeleteAccount and returns before DomainDelPrefix, so the storage prefix only reaches the domain at the block-end flush. Until then the account reads absent while its pre-block storage is still there and still owed a trie delete, and skipping the walk lost that cascade -- EEST test_recreate_self_destructed_contract_different_txs returned INVALID. A destroy is recorded as a present-but-nil current entry, which separates 'destroyed here' from 'never existed', so the skip now needs both. A nil cache keeps the plain probe: that path wipes the prefix inline, so the window does not exist. Gas-token children are created and destroyed inside one transaction and are never carried into the block cache as a destroy of prior state, so the case this targets still skips.
Erigon records history per transaction, so an RPC re-exec of a later transaction must see a destroyed contract's storage already gone while a read at the destroying txNum still sees it. Those history records exist only because the cascade enumerates the committed slots, and nothing covered that: the existing cascade tests stop at Normalize's output, and a trie-root check compares only the block's final state, where the prefix wipe hides a missing per-txn delete. Drives Normalize through Apply and Flush, then reads GetAsOf on both sides of the destroying txNum. Forcing CommittedStorageKeys to always skip turns it red.
1 participant
Self-destruct emits DELETE for every committed storage slot, so it walks the storage-domain prefix for the address. That walk seeks the
.btindex of every storage.kvfile, so it costs the same whether the address owns a thousand slots or none.An address with no committed account owns none — storage is only written for an account that exists, and deleting an account wipes its storage prefix. Probing the account first is served by the per-file existence filters, so the walk is skipped for a contract created and destroyed inside one batch, which never has a committed account.
That is exactly the traffic this was found on. Early-2019 gas-token burns destroy 42–44 storage-less children per transaction. On mainnet blocks 7319406–7320491 (
integration stage_exec --limit=3000on n5):slow WriteSet.Normalizewarnings, andsdDomainCalls == sdDomainEmptyin all 167 parseable ones, without exception — every walk returned nothing,sdSlots=0throughoutsdTookis ~98% of Normalize (took=9.71ms sdTook=9.53ms sdDomainTook=9.52ms sdVMTook=7µs), 480–675 µs per callSame argument and the same
assertNoCommittedStorageguard as #23506, which did this on the create path. The four copies of the walk collapse into one helper.Draft until it is measured on n5 — the A/B needs this merged into the branch carrying the
sdCascadeStatsinstrumentation.