Skip to content

execution/state: state-layer upgrade (1/3, split of #23967) - #24132

Open
mh0lt wants to merge 1 commit into
mainfrom
mh/state-upgrade
Open

mh0lt wants to merge 1 commit into
mainfrom
mh/state-upgrade

Conversation

@mh0lt

@mh0lt mh0lt commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

PR 1 of 3 — split of #23967 (functional split proposed here).

State-layer correctness surfaced through parallel-exec testing, split out as the base of a stacked series:

  • versionMap read/write/validate; cross-path read dependencies
  • EIP-161 empty-account delete self-encoded on the write side
  • account origin-seed discipline; self-destruct / revival lifecycle
  • prev-block layering; removal of read-time account synthesis
  • removes the read-side WriteSet.Normalize path, the block cache, the state-object arena, and writeset-merge helpers

Stacked: this PR intentionally does not build standalone — main's executor/protocol still reference the pre-upgrade state API. The executor's atomic switch to the write-side FinalizedWrites lands in PR2 (#TBD). The execution/state package and its tests are self-consistent. Review as the first of the stack; merge together with PR2.

Part of the split of #23967.

Write-side state correctness surfaced through parallel-exec testing, as the base of a
stacked series (PR2 = parallel executor + normalize removal stacks on this). versionMap
read/write/validate, EIP-161 delete self-encoded write-side, origin-seed discipline,
self-destruct/revival lifecycle, cross-path read dependencies, prev-block layering, and
removal of read-time account synthesis. Also removes the read-side WriteSet.Normalize
path, the block cache, the state-object arena, and writeset-merge helpers.

Stacked: main's executor/protocol reference the pre-upgrade state API until PR2 lands the
write-side FinalizedWrites switch atomically. The state package and its tests are
self-consistent.
@AskAlexSharov

Copy link
Copy Markdown
Collaborator

Correctness

execution/state/versionedio.go:1752 — nil deref. The if account == nil { return nil, nil } guard was dropped from versionedStateReader.ReadAccountDataForDebug; the sibling ReadAccountData still has its if account != nil. ReaderV3.ReadAccountDataForDebug returns (nil, nil) for an absent account, so *account panics. Reached from intra_block_state.go:977 whenever tracingHooks.OnBalanceChange is set and the address is new — any first transfer to a fresh account under a tracer. That caller already handles a nil account, so restoring the guard is the whole fix.

execution/state/account_view.go:50 + intra_block_state.go:2502 — a failed state read is written out as an account delete. ensureBase stores the reader error in baseErr, and nothing ever reads that field. On error baseAcc stays nil, so GetBalance/GetNonce/GetCodeHash all answer empty. encodeExistingEmptyRemovals builds exactly this view: take a live account whose write set holds only storage slots and whose AddressPath origin was not seeded this tx; if its ReadAccountData fails transiently, it passes the !bal.IsZero() || nonce != 0 || !codeHash.IsEmpty() test at line 2524 and gets deleteAddr + SetSelfDestruct(true) + SetBalance(0). Apply then removes a live account's balance/nonce/code, and no error surfaces anywhere. Propagate baseErr out of the accessors, or at minimum bail out of encodeExistingEmptyRemovals on it.

execution/state/prev_block_layer.go:25eip8246=false is wrong for the reader #24133 mounts on it. The comment argues the hardcoded false is safe because only "a direct whole-account reader with no IBS above it" needs the reconstruction. layered_domain_reader.go:71 (readLayered, kv.AccountsDomain) is that reader: #24133 wires it as commitDomainReader (exec3_parallel.go:388), serving the commitment calculator with no IBS above it. On Amsterdam+, a prev block that self-destructs an account preserving a non-zero balance writes SelfDestruct+Incarnation at the destruct tx but no AddressPath, so AccountLifecycleAt returns LifecycleAbsent — the Address probe uses >= destroyedAt, the Balance/Nonce/CodeHash probes use >. With eip8246=false, ReadAccountData returns nil, readLayered reports not-found, and commitment for the next block drops the leaf while ApplyWrites (rw_v3.go, sdPreservedBalance) keeps the account. Wrong state root for the window where the prev block is finished but not yet committed.

execution/state/intra_block_state.go:3202ResetVersionedIO no longer clears stateReadErr. It is now cleared only in Reset(), so on the IBS-reusing paths (block assembler, blockgen, bal/rederive) one recorded read error sticks for every later tx in the block. Related: intra_block_state.go has no recordStateReadError call site left at all — serial GetCodeSize, versionedAccountBase's committed fallback and getStateObject's committed read each lost theirs, and read_paths.go:887 seedOrigin never had one. A read error reaching Exist()/Empty() on the parallel path then returns as a plain tx error with Operational=false, and the executor re-executes the task instead of reporting an infrastructure failure.

Minor

  • execution/state/versionmap_writeview.go:104SelfDestructs() yields Val:false on a non-resolved cell, while every other accessor in the file falls back to the key-set value. Not reachable today (both publication sites seal before building the view), but it is one ordering change away from a missed account delete.
  • Left behind by the Normalize removal, still dead at execution: parallel executor + normalize removal (2/3, split of #23967) #24133's head: versionedio.go SetAccountFieldFromMap / SetAccountFieldZero / SetAccountFieldFromAccount / WriteSet.Filter, and versionmap.go VersionMap.StorageKeys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants