execution/cache: prevent dead-fork StateCache fills across unwind

[yperbasis]

Fixes #22463.

Summary

StateCache stores latest committed state. Unwind already made resident dead-fork entries stale, but readers could add those values again from an old or transient view: a transaction could survive or first bind during unwind, staged unwind rows still existed in the backing database, and read-ahead could fill concurrently.

This PR closes those windows by binding fill authority to both the durable PlainStateVersion and the lifetime of the original ReadView. Reads constrained by a staged unwind cannot fill, cache changes are published only after the database commit, and read-ahead cannot cross the unwind transition.

Snapshot and immutable-file publication are a separate coherence boundary. #23028 still requires #23047 or an equivalent publication hook and is not addressed here. Bounded speculative-unwind fills in the separate commitment BranchCache pre-exist this PR and are tracked in #23253.

Review guide

Suggested order:

1. execution/cache/view.go and state_cache.go: fill admission and publication.
2. db/state/execctx/domain_shared.go: transaction identity, bounded reads, and commit/unwind integration.
3. db/kv/membatchwithdb/memory_mutation.go and db/state/temporal_mem_batch.go: PlainStateVersion ownership and monotonicity.
4. execution/exec/blocks_read_ahead.go and execution/execmodule: read-ahead exclusion and lifecycle.

Focused regression tests sit beside each area.

Correctness invariants

Marker	Protects
PlainStateVersion	The durable state visible to a transaction
readViewEpoch	Whether a ReadView predates the latest unwind or state discontinuity
Per-cache entry epoch and unwind floor	Whether a stored value belongs to the retained fork

Once the cache has a durable state version, an admission-gated state fill is accepted only if the view has the published state version and current epoch, publication is not in progress, its exact domain frontier is not behind the cache, and the read has no staged-unwind step bound. Content-addressed code-size fills do not need these state-view checks.

An ineligible view may still read cache hits; only its fill authority is revoked. WithFrontier preserves the original epoch, so rebinding cannot renew an old view. Stored entries remain O(1) to invalidate and are discarded lazily. The three markers stay separate because durable state, reader, and stored-entry lifetimes change at different boundaries.

Commit and unwind flow

1. Staging an unwind revokes existing views, invalidates stored entries, and records the lowest staged boundary. Bounded reads cannot fill.
2. Flush advances PlainStateVersion exactly once with the domain writes and collects cache updates without publishing them.
3. The database transaction commits.
4. Cache publication applies the complete batch. It repeats unwind invalidation at the durable boundary, rejects delayed or out-of-order versions, preserves entries after a continuous forward commit, and clears them when continuity is unknown.

During publication, reads and view binding remain available, but fills are disabled. A view bound during publication remains fill-inert until explicitly rebound.

MemoryMutation resolves untouched sequences from its backing transaction and flushes only changed sequence keys, so it cannot replay an older state version. Pre-commit notifications receive the projected version explicitly rather than deriving it from overlay sequence writes. Notification ordering itself is unchanged and remains tracked in #23240.

One semaphore permit covers read-ahead warmup and unwind exclusion. A warmup acquires it without blocking, so work requested while another warmup or an unwind owns or waits for the permit is skipped rather than queued. Unwind callers acquire it with their context and abort before staging if cancellation wins. updateForkChoice and SetHead hold the permit through unwind and publication; ValidateChain acquires it only when it stages an unwind. Every FCU currently excludes warmup, including FCUs that do not unwind; narrowing that scope is tracked in #23003.

Performance

• The cache-hit path is unchanged.
• View(nil) adds one atomic load. Binding a fill-enabled ReadView also takes admissionMu.RLock to check publication and state-version eligibility; getters retain that view instead of paying the binding cost per key.
• Normal getters reuse the SharedDomains transaction's memoized state version. A different transaction resolves its version at initial binding. If that resolution temporarily fails, later cache misses retry it; each retry is local to that miss.
• Unwind invalidation remains O(1), with no cache scan or diff replay.
• Each accepted warmup performs one uncontended semaphore acquisition. Rejected warmups do not start a goroutine, and the gate is never touched per key.

Validation

Regression tests cover old and newly bound views across every unwind phase, bounded state and code-hash reads, delayed publications, memory-overlay state versions, read-ahead exclusion and cancellation, and valid forward fills.

[Copilot]

Pull request overview

This pull request hardens execution/cache against reorg/unwind edge-cases where reads from pre-unwind MVCC snapshots (or bounded “in-flight unwind” reads) could previously repopulate the shared StateCache with dead-fork values. It does so by adding a StateCache-wide read-view epoch that revokes fill authority (not read ability) from older ReadViews after an unwind, and by skipping cache fills when a read is step-bounded by the in-memory overlay.

Changes:

• Add readViewEpoch to StateCache and snapshot it into each ReadView; unwind advances the epoch, and admission-gated fills reject older epochs.
• Preserve the original epoch when binding a frontier later (ReadView.WithFrontier), preventing older views from becoming “current” by re-binding.
• Skip read-fill (and derived addr→codeHash seeding) when the mem overlay indicates a bounded read (maxStep != kv.NoStepBound), avoiding caching transient “dying row” results during staged unwinds.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
execution/cache/view.go	Extends ReadView with readViewEpoch, adds WithFrontier, and threads the epoch through fill/seed paths.
execution/cache/state_cache.go	Introduces StateCache.readViewEpoch, gates fill admission on epoch equality, and advances the epoch on unwind.
execution/cache/cache.go	Updates package-level documentation to reflect read-view epoch semantics during unwinds.
execution/cache/cache_test.go	Adds/adjusts unit tests covering refill behavior across unwind and ensuring “ahead of apply” readers can still fill.
db/state/execctx/domain_shared.go	Skips fills on bounded reads; uses WithFrontier to bind a frontier without changing the original view epoch; skips derived code-hash seeding when bounded.
db/state/execctx/statecache_readfill_test.go	Adds tests ensuring bounded in-flight unwind reads do not populate StateCache or derived addr→codeHash mappings.
db/state/execctx/statecache_rpc_integration_test.go	Adds an integration test ensuring an embedded RPC view opened pre-unwind cannot refill an unwound account into StateCache.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

db/state/execctx/statecache_readfill_test.go:329

• Close this cache at test cleanup. StateCache.Close releases reservations from the process-global cache budget; without it, this parallel test leaves those reservations active and can affect cache sizing in later tests.

	sc := newSmallStateCache()

db/state/execctx/statecache_readfill_test.go:302

• Close this cache at test cleanup. StateCache.Close releases reservations from the process-global cache budget; without it, this parallel test leaves those reservations active and can affect cache sizing in later tests.

This issue also appears on line 329 of the same file.

	sc := newSmallStateCache()

[AskAlexSharov]

Review of the current head, limited to correctness bugs and memory issues.

1. branchCache fill is not covered by the staged-unwind guard

db/state/execctx/domain_shared.go:1508

The new guard was added to the StateCache fill (maxStep == kv.NoStepBound, line 1492), but the sibling branchCache.Put a few lines below is on the same read path and stays unguarded. branchCache is aggregator-scoped (line 299), so a bounded commitment read during a staged unwind inserts a staged-view value into a cache shared with the canonical generation.

SharedDomains.Unwind (line 849) only drops branch entries, which is safe in either direction. Inserting a value read under a step bound is not symmetric: the bound is step-granular, the unwind point is not, and nothing re-invalidates the entry if validation is rolled back — the durable rows were never unwound, but the cache now holds the bounded value with a txN below the floor, so it survives the epoch bump. That is the failure class the StateCache guard closes. Commit does not repeat a branch-cache invalidation at the durable boundary either; it only Puts/Invalidates the flushed commitment keys.

2. SuspendWarmup blocks uncancellably

execution/exec/blocks_read_ahead.go:146

SuspendWarmup is a bare warmupGate.Lock(). It replaced drainReadAhead, which called WaitForWarmup(ctx) and returned on ctx.Done(). If a warmBody worker is stuck on a slow MDBX read or a pinned snapshot, updateForkChoice (forkchoice.go:363), SetHead (set_head.go:61) and the validation path park forever while holding the exec semaphore, with no shutdown escape. The engine API stops answering instead of timing out.

3. A queued warmup can outlive shutdown's bounded wait

execution/exec/blocks_read_ahead.go:127

warmWg.Go registers the warmup, and only then does the goroutine park in withWarmupPermit on warmupGate.RLock(). At shutdown WaitForWarmup(warmCtx) (5s timeout) returns while that goroutine is still parked. When the suspension is released, warmBody runs db.BeginRo(ctx) after chainDB.Close() has begun — the waitTxsAllDoneOnClose hang WaitForWarmup exists to prevent. Before, the WaitGroup only covered warmups that were already running and would finish on their own.

4. publishing is cleared without a defer

execution/cache/state_cache.go:491 / :531

beginPublication sets publishing = true; only the final finishPublication resets it, on the normal path. If applyPrepared panics and the panic is recovered up the stack, publishing stays true for the rest of the process: eligibleFrontierLocked returns nil for every View, and fillIfFresh / fillCodeIfFresh / seedAddrCodeHash all short-circuit. The cache silently degrades to apply-only with nothing surfaced. applierMu is released by its own defer, so it is not even detectably wedged.

5. publish clones every value a second time, at the commit memory peak

execution/cache/state_cache.go:513

SharedDomains.Commit already deep-copies each flushed tuple into pendingState (append([]byte(nil), k...) / ...v...). publish then allocates a parallel make([]preparedStateUpdate, len(updates)) and prepareStateUpdate does bytes.Clone(update.Value) again. On a mainnet flush that holds two full copies of the flushed value bytes plus a second 40-byte-per-entry slice live at once, exactly at the commit peak. The old Applier.Apply path cloned once, inside putOrDelete.

The clone also runs before beginPublication, so a rejected publication pays for the whole copy. Moving the committedStateVersion <= sourceStateVersion check into beginPublication made this strictly worse: that early return used to precede the clone loop.

6. baseStateVersion is never refreshed, so Commit is silently single-use

db/state/execctx/domain_shared.go:1132

After a successful Commit the durable version is base+1 but sd.baseStateVersion is still base. A second Commit on the same SharedDomains fails stateVersionsForCommit with state version changed since SharedDomains was created, and ProjectedStateVersion() keeps returning base+1 — which dispatchNotificationsFromOverlay feeds to accumulator.SetStateID. Same after BlockOverlay().UpdateTxn(newerRoTx), which rebinds reads to a newer durable snapshot without touching baseStateVersion. Every current caller happens to rotate the SD, so the constraint is invisible until someone reuses one; the Commit docstring documents only the tx requirement.

7. Detached overlay reports every untouched sequence as 0

db/kv/membatchwithdb/memory_mutation.go:217

Dropping initSequences means the overlay no longer snapshots kv.Sequence. The readTx refactor fixed read views, but on a detached overlay readTx == nil and readSequenceLocked returns 0, nil. rawdb.WriteRawBodyIfNotExists would then take baseTxnID 0 from IncrementSequence(kv.EthTx, n) and ResetSequence on top of it; Flush copies kv.Sequence verbatim into the destination tx, rewinding the durable EthTx sequence. Latent — DetachDB has no production caller — but the previous code was safe by construction.

---

Smaller, same pass:

• execution/execmodule/forkchoice.go:363 and set_head.go:61 still take the exclusive gate for the whole call, so read-ahead warming is off during every FCU, not only the ones that unwind — SetHead holds it across BeginTemporalRw, i.e. while waiting for the MDBX writer lock. ValidateChain got the lazy ensureReadAheadSuspended treatment; these two did not.
• db/state/execctx/domain_shared.go:1660 — the hasUnwindBound branch calls getLatestMetered, whose first two steps are the sd.mem / sd.parent.mem lookups the lines just above already did and discarded, including both latestStateLock.RLocks. Passing the computed bound down would avoid two locked lookups per bounded account probe on the EVM hot path.
• db/state/execctx/domain_shared.go:193 — cacheFrontierFor boxes twice (sdFrontier into the embedded Frontier, then frontierWithStateVersion into the return), and when generationTx.ViewID() != sd.baseViewID it calls rawdb.GetStateVersion(generationTx), a cursor seek on kv.Sequence. Both per-miss rebind paths (lines 1500 and 1713) hit it, so every cache miss there pays a DB read the old code did not.

Two earlier findings are addressed at this head and need no action: the publication-rejection path now keeps the unwind authoritative via unwindLocked, and read views carry their own readTx.

[yperbasis]

Thanks for the detailed pass. Disposition on the current head (ba03dd895b):

• Point 1 is valid, but the bounded speculative-unwind fill is in the separate aggregator-scoped BranchCache and already exists on main. I moved it to execution/commitment: reject BranchCache fills from staged-unwind reads #23253, with the abandoned-validation and successful-unwind cases required as regression tests. This PR keeps its scope on StateCache fill readmission.
• Points 2 and 3 are fixed by eb41519663. One semaphore permit now represents either an active warmup or unwind suspension. Warmups use TryAcquire before starting a goroutine, so they skip instead of queueing. Suspension uses the caller's context, and FCU, SetHead, and ValidateChain abort before unwind if acquisition is cancelled. WaitForWarmup uses the same permit.
• Point 4 was considered and prototyped, then deliberately left out. A cache Put has no expected production panic path, and the StateStep recovery does not surround post-commit StateCache.Publish. Recovering only the cache would not make the broader post-commit operation recoverable. The proposed cleanup added a new exceptional state transition for a synthetic failure mode, so I do not think it belongs in this already large correctness PR.
• Point 5 is valid performance work and is tracked in execution/cache: avoid duplicate work when publishing StateCache batches #23226. It needs an explicit ownership contract and measurements rather than removing a defensive copy locally.
• Point 6 is now explicit in the SharedDomains.Commit contract: commit is terminal for that SharedDomains; callers continue with a new instance on a fresh transaction. Current production callers already rotate it this way.
• Point 7 is fixed by ba03dd895b. A bare detached overlay now returns an error when an untouched sequence needs the missing backing transaction, so IncrementSequence cannot silently start at zero. Explicit overlay sequence writes still work, and NewReadView(tx) remains the supported way to resolve untouched sequences. The regression test was red before the change and also verifies that a failed increment does not create a zero-based value.

For the smaller items:

• Narrowing FCU read-ahead exclusion to actual unwinds remains the performance-only follow-up in execution/execmodule: drain read-ahead only before an actual unwind #23003.
• The repeated mem lookup occurs only after a staged-unwind bound is observed. I left it alone because plumbing the bound through another read path adds complexity to a rare transition path, not the normal EVM hot path.
• 7386067f4f distinguishes retryable bindings from stale rejected views, so stale RPC transactions no longer repeat the state-version read on every miss. The SD's own transaction also uses its memoized version. Remaining direct-read/view-reuse work is tracked in execution/cache: avoid rebuilding cache views on direct reads #23145.

The two earlier items you noted remain fixed: rejected PublishUnwind calls still perform the durable invalidation (4c3a6b31c8), and overlay read views carry their own plain read transaction (452aab4700).

Verification for the latest changes: full db/kv/membatchwithdb tests under -race, repeated make lint, and make erigon integration.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.