execution, db: bind StateCache fills to transaction views and reject stale fills

[yperbasis]

Closes #22356 — the forward stale-fill direction: a fill from a read view older than the latest apply is rejected. The reverse direction around unwind is tracked in #22463 and stays open (details in the scope section).

What StateCache is, who touches it, when

StateCache is one process-global in-memory cache of the latest committed values of accounts, storage and code. It exists to skip the file-accessor/MDBX stack on repeated GetLatest reads. It is not a snapshot: per key it holds one value — the newest committed value known to the process, whether applied by execution or filled from a read. It is on by default; USE_STATE_CACHE=false constructs no cache at all (readers go straight to the backing tx), and STATE_CACHE_FILLS=false disables every reader write (apply-only mode) — the kill switch and the A/B lever for measuring what fills contribute.

Actor	Reads	Writes	When
Canonical execution (SharedDomains.Commit)	via GetLatest	apply: put committed updates, physically delete deletions	after the RwTx commit succeeds — sd.Commit walks the pending updates into the cache only once tx.Commit returns
Embedded RPC (execmodule.CacheView → SharedDomains.AsGetter)	via GetLatest	fill: on a miss, offer the value read from its own tx	during any request at latest
Read-ahead warmup (blocks_read_ahead)	—	fill: same, to pre-warm keys for exec	while exec runs

A fill is a cache write performed on behalf of a database reader (fill-on-miss); an apply is the authoritative post-commit write from SharedDomains.Commit. A read view is what a read-only temporal tx sees (its MDBX read tx + pinned files view); its frontier is the exclusive txNum end of what it can see — a view with frontier N sees txNums < N. This sharing — including the RPC fills — is main's code today (wiring: node/eth/backend.go hands execmodule.Cache to the embedded rpcdaemon as its kvcache; CacheView.Get → SharedDomains.AsGetter → getLatestMetered, which consults and fills the shared cache). The PR does not add the sharing; it adds the missing ordering.

Problem

1. Reader begins a read-only tx; its frontier is N.
2. Exec at txNum M >= N deletes key K and commits; the post-commit apply empties K's slot.
3. Reader reads K: cache miss → reads its own older tx → gets the pre-delete value → its fill re-inserts it (the slot is empty, so PutIfAbsent lands).
4. Anyone's next GetLatest(K) serves the deleted value from the cache.

Each step is consistent inside its own tx. The bug is the unordered hand-off between two txs through the shared cache — no per-tx consistency property can prevent it.

sequenceDiagram
    participant Reader as Reader with frontier N
    participant Cache as shared StateCache
    participant Exec as Exec commit
    Exec->>Cache: apply at txNum M >= N. delete K, appliedEnd = M+1
    Reader->>Cache: GetLatest(K). miss, the slot is empty
    Reader->>Reader: falls back to its own older tx. pre-delete value
    Reader->>Cache: fill(K, pre-delete value)
    Note over Cache: main. PutIfAbsent lands, K is resurrected
    Note over Cache: this PR. rejected, frontier N < appliedEnd

Loading

Solution

1. API: no reads or writes on the cache object, only handles

StateCache has no data methods (like db). Access goes through two handles (like tx):

• ReadView — reads plus admission-gated fills, bound to one tx's read view; it must not outlive the tx. SharedDomains.AsGetter*(tx) builds one per getter; the plain GetLatest wrappers use a frontier-less view and bind a frontier only on the miss path. The view resolves its frontier internally, so a fill can no longer be paired with another tx's frontier.
• Applier — Apply / Unwind / Clear. Used by the authoritative SharedDomains commit/unwind paths (every SD holds a handle; the operations serialize under the admission lock).

2. Fill admission

The cache tracks, per domain, the exclusive end of what has been applied (appliedEnd), guarded by one RWMutex:

• Applier.Apply (write lock) advances appliedEnd and mutates the cache in the same critical section. Applying txNum X records appliedEnd = X+1.
• ReadView.Fill (read lock) rechecks its view's frontier and inserts before releasing: admitted iff frontier >= appliedEnd. Fills are put-if-absent, so they never replace a live authoritative entry.

Ordering proof, two cases: if the fill takes the lock first, the later apply overwrites or deletes it; if the apply takes the lock first, the stale fill is rejected. Equality is safe because a view whose frontier equals the applied end has seen the apply. Checking freshness only at view creation would not be enough — an apply can land between the check and the fill — which is why admission happens at fill time, under the same lock applies take.

3. Exact frontier

DomainVisibleEnd reports the exact exclusive frontier of a tx's domain read view: a file covering [0,N) reports N; a hot-DB view containing txNum N reports N+1. Views without an exact frontier answer ok=false and never fill — remote and history-disabled backends, and dependency-clamped values views (their reads mix ages, and the clamped-away state can appear later without any apply); such reads still work, they just skip cache population. Two memos, one per path: read-only temporal txs memoize the frontier per domain in a tx-local cache, reset only by ForceReopenUnderlyingFilesTx (which can only extend it); the writable path memoizes in SharedDomains, reset at flush and on ViewID change. A compile-time assert guards each memo's bitmask against domain-count growth.

Positive entries are stamped with their step-derived txNum bound. A negative entry is stamped with the last txNum its read view included, max(frontier-1, 0): since Unwind(N) treats N as the first rolled-back txNum, the negative survives Unwind(N) and is invalidated by Unwind(N-1).

Admission safety needs one more invariant: a view's frontier never decreases in a process that fills the cache (the DB component is frozen at tx begin; a files reopen only extends it). This is enforced, not just documented: wiring a fill-enabled StateCache forbids visibility lowering on that aggregator, and recalcVisibleFiles panics on the one transition that breaks admission — a cached state domain's visible end decreasing — whichever entry point causes it. Raising visibility (unaligning a lagging entity) stays allowed, and apply-only caches (STATE_CACHE_FILLS=false) skip the forbid: with no fills there is nothing for a lowered frontier to poison.

4. Physical deletion

Apply physically deletes account, storage and code entries; on the authoritative side absence is represented by absence, with no deletion markers (a marker can be evicted, re-opening the fill window). Reader-side negative entries do exist — they are ordinary admission-gated fills, stamped as below. An account update invalidates the derived address→codeHash mapping; an account deletion also removes the address's code binding; an account-only deletion does not advance the code-domain frontier (that would suppress valid code fills for unrelated contracts). On authoritative code apply, bytes are cloned before hashing so cached code and its codeHash cannot diverge. SeedAddrCodeHash accepts only view-sourced account records (a cache-sourced record can lag the latest apply and would carry pre-apply state past the gate); a read error does not seed the zero-hash sentinel.

What this PR deliberately does not do

• Reads are not snapshot-isolated: a ReadView hit can be newer than the view's tx. That is the same direction the Overlay already serves (embedded RPC at latest reads exec's newest state on purpose), and a single-version LRU cannot give stable per-view reads without becoming a second kvcache — node/shards already provides that model for the remote daemon. The cache's contract, in the forward direction, is monotonicity: content never regresses behind the applied frontier; unwinds invalidate by epoch and floor.
• The reverse direction around unwind — a view opened before an unwind refilling a value from the discarded fork — is tracked separately in execution/cache: stale-fill admission is one-sided — pre-unwind read views refill dead-fork values, and unwound keys bypass the flush cache-apply #22463.

Performance

• Cache hits are unchanged: no frontier query, no admission lock.
• The memoized frontier read is ~2.4 ns/op, 0 allocs (Apple M2 Max); the memo adds +64 B/op to a read-only temporal tx (BenchmarkBeginTemporalRo/WithBlockSnaps; absolute numbers drift with unrelated changes on main).
• Getter paths (exec workers, RPC) build their frontier-carrying ReadView once per getter; frontier-less views elsewhere (GetCode/GetCodeSize fast paths) are by-value and allocation-free, and a frontier is bound — one allocation — only on cold fill/seed paths. A cold negative fill through the plain GetLatest wrappers is ~0.3 µs/op, 48 B/op, 2 allocs: one allocation is the fill itself, one is binding the frontier on the miss path — both amortize against the backing read they follow (the no-cache baseline of the same read is ~0.1 µs).

Testing

Production-path reproductions through embedded RPC (execmodule.Cache.View over a published SharedDomains overlay and a real temporal DB): account, storage and code resurrection from an old RPC read view, and code-of-a-deleted-account refill (pinning both the SD-level paired deletion and the cache-level accounts-frontier check on code fills). Plus: admission at the exact frontier boundary and a 20,000-round concurrent apply/delete vs fill interleaving; physical deletion and derived code-mapping invalidation; account-only deletion not suppressing unrelated code fills; the address→codeHash mapping seeding only from view-sourced records; exact file/DB frontier calculation and memo re-derivation after files-view reopen and after flush; read-ahead freshness, no-exact-frontier backends never filling; negatives surviving Unwind(N) but not Unwind(N-1); the admission frontier surviving Clear; the STATE_CACHE_FILLS=false apply-only switch; USE_STATE_CACHE=false constructing no cache; node close releasing every cache-budget reservation (a full EngineApiTester lifecycle); and the visibility-lowering assert. All cross-package tests exercise the public ReadView/Applier API. The three resurrection tests use no API introduced by this PR: ported unchanged to main, all three fail with the resurrected values served — they pin the bug, not the refactor.

Verified with go test (and -race) on execution/cache, execution/exec, db/state/execctx, db/kv/temporal, full-tree build, benchmarks with -benchmem, and repeated clean make lint runs.

How to review

Suggested order:

1. execution/cache/cache.go (package doc) and execution/cache/view.go — the contract and the API: Frontier, ReadView, Applier. Small files; everything else implements them.
2. execution/cache/state_cache.go — admission internals: appliedEnd, fillIfFresh, apply.
3. db/state/execctx/domain_shared.go — wiring: getters hold a ReadView, commit/unwind hold the Applier; getLatestMetered shows the full read chain (mem → cache → backing tx → fill).
4. execution/exec/blocks_read_ahead.go — the read-ahead fill path.
5. db/state/execctx/statecache_rpc_integration_test.go — the resurrection reproduced end-to-end through the embedded-RPC path, and the fix pinned.

[Copilot]

Pull request overview

This PR hardens StateCache against stale snapshot read-fills that could resurrect canonically deleted accounts/storage/code by linearizing cache fill admission with committed cache mutations.

Changes:

• Adds an admission RWMutex and per-domain appliedProgress to execution/cache.StateCache, plus an Apply API that advances progress and mutates the cache atomically.
• Updates production read-fill paths (SharedDomains.getLatestMetered, SharedDomains.codeHashForAddr, and read-ahead warmup) to use Put*IfFresh APIs that recheck snapshot freshness under the admission lock.
• Adds unit + integration tests covering stale-fill rejection after deletes, concurrent apply/fill interleavings, unwind/clear lifecycle, and embedded-RPC reproduction scenarios.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
execution/exec/blocks_read_ahead.go	Wires read-ahead warmup fills through freshness-checked cache APIs using a snapshot-progress oracle.
execution/exec/blocks_read_ahead_test.go	Extends tests to cover negative stamping/unwind behavior, nil-progress behavior, and stale-snapshot fill rejection.
execution/cache/state_cache.go	Introduces admission locking + applied progress tracking, removes stale-fill windows, and adds atomic Apply w/ physical deletion semantics.
execution/cache/code_cache.go	Serializes addr-binding deletion with existing writer mutex to keep deletion coherent with concurrent bind writers.
execution/cache/cache_test.go	Adds tests for applied-progress lifecycle, stale snapshot fill rejection after delete, and concurrent apply/delete vs fill ordering.
db/state/execctx/statecache_rpc_integration_test.go	Adds embedded-RPC integration reproduction test ensuring deleted state cannot be resurrected via stale snapshot fills.
db/state/execctx/domain_shared.go	Switches commit-time cache mutations to StateCache.Apply and read-fill paths to Put*IfFresh (freshness recheck under admission lock).
db/state/execctx/codehash_routing_test.go	Updates to new PutAddrCodeHashIfFresh API.

---

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

execution/cache/state_cache.go:282

• fillCodeIfFresh clones code bytes under admissionMu.RLock() via bytes.Clone(value) in the Put call. Contract code can be relatively large, so this can materially increase contention with Apply (write lock). Clone (and derive codeHash from the clone) before taking the lock, then do the freshness check + Put while holding the lock.

	codeHash := crypto.Keccak256(value)
	c.admissionMu.RLock()
	defer c.admissionMu.RUnlock()
	if visibleEnd < c.appliedEnd[kv.CodeDomain] || accountsVisibleEnd < c.appliedEnd[kv.AccountsDomain] {
		return
	}
	codeCache.PutWithCodeHashIfAbsent(key, bytes.Clone(value), codeHash, readTxNum)
}

execution/cache/state_cache.go:262

• fillIfFresh holds admissionMu while cloning the filled value (bytes.Clone(value)). For large values this extends the read-side critical section and can block Apply (which needs the write lock). Cloning can be done before taking admissionMu so the freshness check + PutIfAbsent stay serialized but the lock hold time is reduced.

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

	c.admissionMu.RLock()
	defer c.admissionMu.RUnlock()
	if visibleEnd < c.appliedEnd[domain] {
		return
	}
	if len(value) == 0 {
		readTxNum = 0
		if visibleEnd > 0 {
			readTxNum = visibleEnd - 1
		}
	}
	cache.PutIfAbsent(key, bytes.Clone(value), readTxNum)

[AskAlexSharov]

The core design holds. I checked the admission argument by hand rather than reading it off the description, and I could not break it in the forward direction. Below are one likely perf regression, one assert that guards the wrong quantity, and a few enforcement gaps.

One thing worth stating explicitly because it is load-bearing and easy to lose in a later refactor: admission works per domain, and it has to. appliedEnd[D] advances only from the txN of domain-D updates, and D's frontier is max(D history-II files end, lastTxNumInDB(D)+1) — both derived from the same per-domain write events. So a flush touching only accounts leaves appliedEnd[Storage] and the storage frontier equal, and storage fills keep being admitted. A single global frontier would have silently killed fills for every quiet domain.

1. Applier.Apply takes the write lock once per key

Commit stashes every flushed tuple for Accounts/Storage/Code into pending (domain_shared.go:1066-1090), then loops cacheApplier.Apply(...). Each call does admissionMu.Lock() / Unlock() (state_cache.go:304). A batch flush carries the whole batch, so this is hundreds of thousands to millions of write-lock round trips where main had lock-free Put / Delete — and each one is a barrier against concurrent RPC and read-ahead RLocks.

Suggest a batched ApplyAll(pending) that takes the lock once and calls noteApplied per domain at the end. One lock over the whole batch is strictly stronger than per-key, so the ordering proof survives unchanged. Worth timing doms.Commit on a real datadir before merge — the micro-benchmarks in the description do not reach this path.

2. The recalcVisibleFiles assert guards a different quantity than admission reads

DomainVisibleEnd returns at.d[name].ht.iit.visibleEnd(tx) — the history inverted-index visible end (aggregator.go:2631). The forbid-lowering check compares prev.d[d].files / next.d[d].files, the domain values visible end (aggregator.go:1879).

In the common case both come from the same toTxNum ceiling, so they move together. But the assert cannot fire on a lowering of the quantity fill admission actually depends on. Either check dhii[d] as well, or make DomainVisibleEnd return min(domain end, ii end) so the guarded value bounds the reported one.

3. "no cache is ever advanced past durable MDBX state" holds for applies, not fills

flushMem resets the frontier memo on return (domain_shared.go:1003), then Commit runs runValidate and the adaptive-pin preload against the in-flight tx. A read through this SD in that window sees flushed-but-uncommitted values and a frontier that already covers them, so it fills them. If tx.Commit() then fails, pending is discarded but those fills stay.

Narrow — a failed commit is fatal anyway — but the comment claims more than the code delivers.

4. The new Flush-vs-Commit contract is documented but not enforced

The doc now says an SD with a cache must route every flush through Commit. Per the repo comment policy, prefer code that enforces the invariant over a comment that describes it: return an error from Flush when sd.stateCache != nil. No current caller violates it (the SD at stages.go:720 has no cache), which is exactly when the check is cheap to add and will not bite anyone.

5. GuardAggregatorForCache duck-types where the type system would do

*temporal.DB already has Agg() any (kv_temporal.go:91). Declaring it on kv.TemporalRwDB turns both panics into compile errors and removes the "remember to call this at every wiring site" discipline.

Related: the guard runs in the ExecModule constructor, while the real wiring point is SetStateCache. Four of the five SetStateCache call sites depend on the constructor having run first.

6. Latent nil deref through Debug()

MemoryMutation.Debug() returns nil when m.db == nil (memory_mutation.go:1029), and sdFrontier.DomainVisibleEnd calls tx.Debug().DomainVisibleEnd(...) unguarded. Not reachable today — the only mem-batch-backed SD, filterSd in builder/exec.go:132, has no cache — but the PR makes Debug() load-bearing on a fill path where it was not before.

7. Question: do parallel-exec worker fills still get admitted?

Worker.chainTx is a read-only tx opened on the worker's first task and rolled back only when Run() exits (exec/state.go:525-535, 382-386). Its frontier is frozen for the worker's lifetime, while appliedEnd advances at every doms.Commit. If a worker outlives a commit, every fill through AsGetterMetered(chainTx, ...) is rejected from then on, and the cache becomes apply-only for the parallel path.

That would be correct but expensive, and no test or benchmark here would show it. A fills-admitted / fills-rejected counter over a real sync would settle it.

8. Comment volume

272 added comment lines against 516 added non-test code lines; view.go alone is 72 comment lines to 101 code lines. The ordering proof, the two-memo rationale and the perf numbers are already in the PR description, which is where the repo comment policy wants them.

Keep as-is

• CodeCache.Delete taking addrBindMu closes a real check-and-bind race against putCodeLocked.
• Adding DeleteAddrCodeHash to the code-domain deletion apply fixes a gap main has today.
• Cloning code before hashing in apply is correct and not obvious until you notice the caller may reuse its buffer.
• The var _ [32 - 2*int(kv.DomainLen)]struct{} bitmask assert is the right shape (DomainLen=6, room up to 16).
• The three resurrection tests use no API introduced here, so the "port to main and they fail" claim is checkable.

[Copilot]

Pull request overview

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

[sudeepdino008]

since we're moving the flush+commit online for simplicity; do we need this PR and other rpc<>StateCache fixes now?

[yperbasis]

since we're moving the flush+commit online for simplicity; do we need this PR and other rpc<>StateCache fixes now?

Current main (and release/3.6) with default settings have the bug (Issue #22356).

[taratorio]

since we're moving the flush+commit online for simplicity; do we need this PR and other rpc<>StateCache fixes now?

Current main (and release/3.6) with default settings have the bug (Issue #22356).

@yperbasis Then add a feature flag and disable usage of global shared domains and caching in RPC daemon. It’s 100% unnecessary and doesn’t improve anything. In fact it makes things worse with a premature design that will have to be rebuilt from ground up.

[yperbasis]

@yperbasis Then add a feature flag and disable usage of global shared domains and caching in RPC daemon. It’s 100% unnecessary and doesn’t improve anything. In fact it makes things worse with a premature design that will have to be rebuilt from ground up.

@taratorio Fair enough. I've filed Issue #23082