db, execution: harden StateCache read-fills against stale overwrites and unwinds

db/kv/kv_interface.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"slices"
 	"strings"
 	"sync"
@@ -435,6 +436,10 @@ type Putter interface {
 // This type represents a step in time across the chain history or an amount of steps.
 type Step uint64
 
+// NoStepBound marks the absence of a per-key step bound: every step is
+// servable. Distinct from a bound at step 0, which admits only step 0.
+const NoStepBound = Step(math.MaxUint64)
+
 // Returns the txNum of the first tx in the step.
 func (s Step) ToTxNum(stepSize uint64) uint64 { return uint64(s) * stepSize }
 
@@ -560,6 +565,9 @@ func WithFlushCallback(domain Domain, cb func(k []byte, v []byte, step Step, txN
 type TemporalMemBatch interface {
 	DomainPut(domain Domain, k string, v []byte, txNum uint64, preval []byte) error
 	DomainDel(domain Domain, k string, txNum uint64, preval []byte) error
+	// GetLatest returns the key's latest in-mem value. On a miss, step carries
+	// the key's in-flight-unwind bound — NoStepBound when none — so callers
+	// can bound their fall-through read.
 	GetLatest(domain Domain, key []byte) (v []byte, step Step, ok bool)
 	GetDiffset(tx RwTx, blockHash common.Hash, blockNumber uint64) ([DomainLen][]DomainEntryDiff, bool, error)
 	Merge(other TemporalMemBatch) error

db/state/execctx/domain_shared.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"math"
 	"runtime"
 	"sync"
 	"sync/atomic"
@@ -1082,18 +1081,6 @@ func (sd *SharedDomains) Commit(ctx context.Context, tx kv.RwTx, validate ...fun
 	return nil
 }
 
-// DetachBranchCache makes this SharedDomains ignore the aggregator-scope
-// BranchCache: commitment branch reads go straight to sd.mem/overlay/MDBX and
-// no read populates the shared cache. Used for fork-validation SDs, which read
-// transient fork state — sharing the canonical BranchCache let them read stale
-// committed branches (wrong trie root → INVALID payload) and pollute the cache
-// with fork-transient branches. Only sd.branchCache (the read/populate path,
-// domain_shared GetLatest) is consulted, so nil-ing it fully detaches; the
-// canonical SDs keep their warm cache.
-func (sd *SharedDomains) DetachBranchCache() {
-	sd.branchCache = nil
-}
-
 // TemporalDomain satisfaction. Collects no read metrics — see
 // temporalGetter.GetLatest for why there is no process-wide accumulator.
 func (sd *SharedDomains) GetLatest(domain kv.Domain, tx kv.TemporalTx, k []byte) (v []byte, step kv.Step, err error) {
@@ -1109,6 +1096,16 @@ func (sd *SharedDomains) GetLatestContext(ctx context.Context, domain kv.Domain,
 	return sd.getLatestMetered(domain, tx, k, kvmetrics.MetricsFromContext(ctx))
 }
 
+// servableUnderBound gates a cached entry against an in-flight unwind's
+// per-key maxStep: a hit above the bound would diverge from the bounded read
+// the cache-disabled path takes (the epoch floor usually drops such entries
+// already; the gate keeps the two paths identical regardless). Callers convert
+// their unit first — the StateCache stamps txNums (divide by step size), the
+// BranchCache stores step indices (no divide).
+func servableUnderBound(cStep, maxStep kv.Step) bool {
+	return cStep <= maxStep
+}
+
 // getLatestMetered is the read implementation. wm is the caller's lock-free
 // per-task/per-worker metrics accumulator (nil disables metrics for the call).
 // No global metrics lock is taken on this hot path — accumulators are combined
@@ -1127,7 +1124,7 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k
 			wm = sd.reqMetrics
 		}
 	}
-	maxStep := kv.Step(math.MaxUint64)
+	maxStep := kv.NoStepBound
 
 	// Check mem batch first - it has the current transaction's uncommitted state.
 	// No need to populate stateCache here — mem is checked first on every read,
@@ -1138,7 +1135,7 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k
 		}
 		return v, step, nil
 	} else {
-		if step > 0 {
+		if step < maxStep {
 			maxStep = step
 		}
 	}
@@ -1151,7 +1148,7 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k
 			}
 			return v, step, nil
 		} else {
-			if step > 0 && step < maxStep {
+			if step < maxStep {
 				maxStep = step
 			}
 		}
@@ -1171,16 +1168,11 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k
 	// that haven't been flushed to DB yet. Early return keeps correctness AND performance.
 	if sd.stateCache != nil {
 		v, cTxNum, ok := sd.stateCache.GetWithTxNum(domain, k)
+		// The cache stamps txNums — divide to get the step the entry reflects.
+		// An empty value is stamped with the domain's progress at fill time, so
+		// its cStep is progress-derived, not the step of any deletion.
 		cStep := kv.Step(cTxNum / sd.StepSize())
-		// Respect maxStep, mirroring the BranchCache gate below. sd.mem /
-		// sd.parent.mem lowered maxStep above when an in-flight unwind re-bound
-		// this key to an earlier step (the per-key unwindChangeset signal). A
-		// cached entry from a higher step would diverge from the (maxStep-bounded)
-		// DB read the cache-disabled path takes, so treat it as a miss and fall
-		// through; the Put below refreshes it. For direct domains the (txNum,epoch)
-		// floor in Get usually already drops such entries — this keeps the two
-		// read paths identical regardless.
-		if ok && cStep > maxStep {
+		if ok && !servableUnderBound(cStep, maxStep) {
 			ok = false
 		}
 		if dbg.KVReadLevelledMetrics {
@@ -1191,7 +1183,11 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k
 			}
 		}
 		if ok {
-			if dbg.AssertStateCache {
+			// The divergence assert is skipped while the mem overlay bounds this
+			// key (in-flight unwind): MDBX still holds the not-yet-deleted dying
+			// rows inside the bound, so the "authoritative" read can return
+			// dead-fork bytes and blame the cache for a legitimate hit.
+			if dbg.AssertStateCache && maxStep == kv.NoStepBound {
 				// Fetch authoritative value from the backing tx and panic on any divergence.
 				// sd.mem and sd.parent.mem were already checked above and missed, so the
 				// backing tx is the single source of truth for this key at this point.
@@ -1225,18 +1221,11 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k
 	// with a newer MDBX state.
 	if domain == kv.CommitmentDomain && sd.branchCache != nil {
 		if cv, cStepU64, ok := sd.branchCache.Get(k); ok {
-			// Respect maxStep. sd.mem / sd.parent.mem lowered maxStep above when an
-			// unwound key reports its restored step via unwindChangeset (the per-key
-			// in-mem unwind signal). The cache's global epoch/floor is coarser than
-			// that per-key signal, so a cached entry below the floor can still belong
-			// to a step the unwind re-bound away. Serving it then diverges from the
-			// (maxStep-bounded) DB read the cache-disabled path takes. Fall through to
-			// the bounded read in that case; the Put below refreshes the entry.
 			// Get returns the on-disk step index directly — do NOT divide by
 			// StepSize (that double-division collapsed cStep to ~0, defeating the
 			// gate).
 			cStep := kv.Step(cStepU64)
-			if cStep <= maxStep {
+			if servableUnderBound(cStep, maxStep) {
 				return cv, cStep, nil
 			}
 		}
@@ -1257,26 +1246,30 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k
 		return nil, 0, fmt.Errorf("storage %x read error: %w", k, err)
 	}
 
-	// Populate state cache on successful read. Stamp with an upper bound on the
-	// value's write txNum (the read only gives us the file/step it came from):
-	// the last txNum of that step. An unwind below this bound can't leave the
-	// value stale, so frozen-step reads stay warm across unwinds while
-	// recent-step reads are dropped.
+	// Populate the cache with if-absent semantics: a read-fill never carries
+	// newer information than a flush-apply, so it must not overwrite one
+	// (e.g. an embedded-RPC read straddling an FCU commit). Stamp with the
+	// last txNum of the step the value came from — an upper bound on its
+	// write txNum — so an unwind below it can't leave the entry stale. A
+	// negative carries no step; stamp it with the domain's progress at
+	// observation time so any unwind drops it.
 	if sd.stateCache != nil {
 		readTxNum := (uint64(step)+1)*sd.StepSize() - 1
 		if domain == kv.CodeDomain {
 			if len(v) > 0 {
 				// This SD getter is the single place that populates the code cache
 				// on a read. Key the content-addressed entry by the code's OWN hash,
 				// keccak(v) — NEVER a separately-read account codeHash, which under
-				// parallel exec can be a skewed/cross-account value and would poison
-				// the shared codeHash->code map for every account sharing that hash.
-				// keccak(v) makes every cached entry self-consistent, so a skewed
-				// account read can never produce a bad entry.
-				sd.stateCache.PutCodeWithHash(k, v, crypto.Keccak256(v), readTxNum)
+				// parallel exec can be a skewed or cross-account value and would
+				// poison the shared codeHash→code map for every account sharing
+				// that hash.
+				sd.stateCache.PutCodeWithHashIfAbsent(k, v, crypto.Keccak256(v), readTxNum)
 			}
 		} else {
-			sd.stateCache.Put(domain, k, v, readTxNum)
+			if len(v) == 0 && sd.stateCache.GetCache(domain) != nil {
+				readTxNum = tx.Debug().DomainProgress(domain)
+			}
+			sd.stateCache.PutIfAbsent(domain, k, v, readTxNum)
 		}
 	}
 	// Only cache a branch when the read's txN is known: a txN=0 entry would

db/state/execctx/statecache_readfill_bench_test.go

@@ -0,0 +1,99 @@
+// Copyright 2026 The Erigon Authors
+// This file is part of Erigon.
+//
+// Erigon is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Erigon is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with Erigon. If not, see <http://www.gnu.org/licenses/>.
+
+package execctx_test
+
+import (
+	"encoding/binary"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/erigontech/erigon/common/log/v3"
+	"github.com/erigontech/erigon/db/kv"
+	"github.com/erigontech/erigon/db/state/execctx"
+)
+
+// benchSeedDb commits one account so the domain tables are non-empty for
+// cold-negative probes.
+func benchSeedDb(b *testing.B) kv.TemporalRwDB {
+	b.Helper()
+	const stepSize = uint64(16)
+	ctx := b.Context()
+	db := newTestDb(b, stepSize)
+
+	rwTx, err := db.BeginTemporalRw(ctx)
+	require.NoError(b, err)
+	defer rwTx.Rollback()
+	sd, err := execctx.NewSharedDomains(ctx, rwTx, log.New())
+	require.NoError(b, err)
+	defer sd.Close()
+	written := make([]byte, 20)
+	written[0] = 0x01
+	sd.SetTxNum(100)
+	require.NoError(b, sd.DomainPut(kv.AccountsDomain, rwTx, written, encAccount(7), 100, nil))
+	require.NoError(b, sd.Commit(ctx, rwTx))
+	return db
+}
+
+// BenchmarkDomainProgress isolates the negative-stamp source: one
+// files.EndTxNum read plus an MDBX LastKey on the domain's keys table.
+func BenchmarkDomainProgress(b *testing.B) {
+	db := benchSeedDb(b)
+	roTx, err := db.BeginTemporalRo(b.Context())
+	require.NoError(b, err)
+	defer roTx.Rollback()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_ = roTx.Debug().DomainProgress(kv.AccountsDomain)
+	}
+}
+
+// benchColdNegativeReads drives the full cold-negative SD read: the whole
+// miss stack, plus — when a cache is wired — the progress stamp and the
+// if-absent fill.
+func benchColdNegativeReads(b *testing.B, withCache bool) {
+	db := benchSeedDb(b)
+	ctx := b.Context()
+	roTx, err := db.BeginTemporalRo(ctx)
+	require.NoError(b, err)
+	defer roTx.Rollback()
+	sd, err := execctx.NewSharedDomains(ctx, roTx, log.New())
+	require.NoError(b, err)
+	defer sd.Close()
+	if withCache {
+		sd.SetStateCacheForTest(newSmallStateCache())
+	}
+
+	key := make([]byte, 20)
+	key[0] = 0x02
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		binary.BigEndian.PutUint64(key[12:], uint64(i)+1)
+		v, _, err := sd.GetLatest(kv.AccountsDomain, roTx, key)
+		if err != nil {
+			b.Fatal(err)
+		}
+		if len(v) != 0 {
+			b.Fatalf("expected a negative, got %x", v)
+		}
+	}
+}
+
+func BenchmarkGetLatestColdNegative(b *testing.B) { benchColdNegativeReads(b, true) }
+
+// The baseline the stamp+fill cost adds to.
+func BenchmarkGetLatestColdNegativeNoCache(b *testing.B) { benchColdNegativeReads(b, false) }