From 67e8611147c9082e868f64b708bd84ab7c5e0537 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 13:54:46 +0200 Subject: [PATCH 1/7] db/state/execctx: gate SD read-fills, skip the divergence assert under per-key bounds, stamp negatives with progress Split from #22159 (StateCache review findings #22120). - Read-fills use if-absent semantics: a read-fill never carries newer information than a flush-apply, so it must not overwrite a live authoritative entry (e.g. an embedded-RPC read straddling an FCU commit). Code fills go through PutCodeWithHashIfAbsent, keyed by keccak(v). - Negative results (missing account, empty slot) are stamped with the domain's progress at observation time instead of a synthetic step-zero bound, so any unwind drops them instead of letting them outlive the fact. - The ASSERT_STATE_CACHE divergence assert is skipped while the mem overlay bounds the key (in-flight unwind): MDBX still holds the not-yet-deleted dying row inside the bound, so the authoritative read can return dead-fork bytes and blame the cache for a legitimate hit. - The state-cache and branch-cache maxStep gates share servableUnderBound, with their different units explicit per call site. - Dead DetachBranchCache is deleted: it advertised a fork-validation guard that was never wired; the actual mechanism is the epoch-bumping sd.Unwind. --- db/state/execctx/domain_shared.go | 74 +++--- .../execctx/statecache_readfill_bench_test.go | 99 ++++++++ db/state/execctx/statecache_readfill_test.go | 211 ++++++++++++++++++ 3 files changed, 343 insertions(+), 41 deletions(-) create mode 100644 db/state/execctx/statecache_readfill_bench_test.go create mode 100644 db/state/execctx/statecache_readfill_test.go diff --git a/db/state/execctx/domain_shared.go b/db/state/execctx/domain_shared.go index ed76fc775aa..3ac1177de33 100644 --- a/db/state/execctx/domain_shared.go +++ b/db/state/execctx/domain_shared.go @@ -1082,18 +1082,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 +1097,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 @@ -1171,16 +1169,9 @@ 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. 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 +1182,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.Step(math.MaxUint64) { // 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 +1220,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,11 +1245,13 @@ 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 { @@ -1269,14 +1259,16 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k // 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 diff --git a/db/state/execctx/statecache_readfill_bench_test.go b/db/state/execctx/statecache_readfill_bench_test.go new file mode 100644 index 00000000000..a5868cf2933 --- /dev/null +++ b/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 . + +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) } diff --git a/db/state/execctx/statecache_readfill_test.go b/db/state/execctx/statecache_readfill_test.go new file mode 100644 index 00000000000..7209a1cac21 --- /dev/null +++ b/db/state/execctx/statecache_readfill_test.go @@ -0,0 +1,211 @@ +// 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 . + +package execctx_test + +import ( + "encoding/binary" + "testing" + + "github.com/c2h5oh/datasize" + "github.com/holiman/uint256" + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/dbg" + "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/db/state/execctx" + "github.com/erigontech/erigon/execution/cache" + "github.com/erigontech/erigon/execution/types/accounts" +) + +func encAccount(nonce uint64) []byte { + a := accounts.Account{Nonce: nonce, Balance: *uint256.NewInt(nonce * 1000)} + return accounts.SerialiseV3(&a) +} + +// twoStepRows commits two versions of one account key so MDBX holds rows at +// step 0 (txNum 5, v1) and step 1 (txNum 20, v2), and returns a delete-only +// unwind diff for the step-1 row — the legacy-changeset shape that makes the +// mem overlay publish a per-key maxStep bound while MDBX still holds the +// dying row. +func twoStepRows(t *testing.T, db kv.TemporalRwDB, sc *cache.StateCache) (key, v1, v2 []byte, diffs [kv.DomainLen][]kv.DomainEntryDiff) { + t.Helper() + ctx := t.Context() + key = make([]byte, 20) + key[0] = 0xaa + v1, v2 = encAccount(1), encAccount(2) + + rwTx, err := db.BeginTemporalRw(ctx) + require.NoError(t, err) + defer rwTx.Rollback() + + sd, err := execctx.NewSharedDomains(ctx, rwTx, log.New()) + require.NoError(t, err) + defer sd.Close() + sd.SetStateCacheForTest(sc) + + sd.SetTxNum(5) + require.NoError(t, sd.DomainPut(kv.AccountsDomain, rwTx, key, v1, 5, nil)) + sd.SetTxNum(20) + require.NoError(t, sd.DomainPut(kv.AccountsDomain, rwTx, key, v2, 20, v1)) + require.NoError(t, sd.Commit(ctx, rwTx)) + + stepBytes := make([]byte, 8) + binary.BigEndian.PutUint64(stepBytes, ^uint64(1)) + diffs[kv.AccountsDomain] = []kv.DomainEntryDiff{{Key: string(key) + string(stepBytes), Value: nil}} + return key, v1, v2, diffs +} + +func newSmallStateCache() *cache.StateCache { + b := 1 * datasize.MB + return cache.NewStateCache(b, b, b, b) +} + +// During an in-flight unwind the mem overlay bounds reads of an affected key +// by maxStep while MDBX still holds the not-yet-deleted dying row inside that +// bound. A cache hit legitimately below the unwind floor then diverges from +// the maxStep-bounded DB read, and the ASSERT_STATE_CACHE comparison must not +// blame the cache for it. +func TestAssertStateCache_NoFalsePanicDuringInFlightUnwind(t *testing.T) { + if testing.Short() { + t.Skip() + } + // Mutates dbg.AssertStateCache — must not run in parallel with tests that + // read it on the SD read path. + + const stepSize = uint64(16) + ctx := t.Context() + db := newTestDb(t, stepSize) + sc := newSmallStateCache() + key, v1, _, diffs := twoStepRows(t, db, sc) + + roTx, err := db.BeginTemporalRo(ctx) + require.NoError(t, err) + defer roTx.Rollback() + + sd, err := execctx.NewSharedDomains(ctx, roTx, log.New()) + require.NoError(t, err) + defer sd.Close() + sd.SetStateCacheForTest(sc) + + // A live cache entry below the unwind floor: the restored (correct) value. + sc.Put(kv.AccountsDomain, key, v1, 5) + sd.Unwind(10, &diffs) // in-flight: mem publishes maxStep=1; MDBX still holds the step-1 row + + old := dbg.AssertStateCache + dbg.AssertStateCache = true + t.Cleanup(func() { dbg.AssertStateCache = old }) + + var v []byte + require.NotPanics(t, func() { + v, _, err = sd.GetLatest(kv.AccountsDomain, roTx, key) + }, "assert must not fire on a legitimately-bounded cache hit during an in-flight unwind") + require.NoError(t, err) + require.Equal(t, v1, v, "the cache serves the restored value") +} + +// The read-fill after a fall-through read must not replace a live cache +// entry: it never carries newer information than a flush-apply, and during an +// in-flight unwind the bounded DB read can even return the not-yet-deleted +// dying row. +func TestReadFill_DoesNotClobberLiveEntry(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Parallel() + + const stepSize = uint64(16) + ctx := t.Context() + db := newTestDb(t, stepSize) + sc := newSmallStateCache() + key, _, v2, diffs := twoStepRows(t, db, sc) + + roTx, err := db.BeginTemporalRo(ctx) + require.NoError(t, err) + defer roTx.Rollback() + + sd, err := execctx.NewSharedDomains(ctx, roTx, log.New()) + require.NoError(t, err) + defer sd.Close() + sd.SetStateCacheForTest(sc) + + sd.Unwind(10, &diffs) + // A live (current-epoch) entry above the read bound: the maxStep gate turns + // the hit into a miss, so the read falls through to the bounded DB read. + v3 := encAccount(3) + sc.Put(kv.AccountsDomain, key, v3, 40) + + v, _, err := sd.GetLatest(kv.AccountsDomain, roTx, key) + require.NoError(t, err) + require.Equal(t, v2, v, "fall-through read serves the maxStep-bounded DB row") + + got, ok := sc.Get(kv.AccountsDomain, key) + require.True(t, ok) + require.Equal(t, v3, got, "read-fill must not clobber the live entry") +} + +// Negative results (missing account) must be stamped with the domain's +// progress at observation time, not a synthetic step-0 bound that survives +// every unwind. +func TestReadFill_NegativeStampedWithProgress(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Parallel() + + const stepSize = uint64(16) + ctx := t.Context() + db := newTestDb(t, stepSize) + sc := newSmallStateCache() + + rwTx, err := db.BeginTemporalRw(ctx) + require.NoError(t, err) + defer rwTx.Rollback() + + sd, err := execctx.NewSharedDomains(ctx, rwTx, log.New()) + require.NoError(t, err) + defer sd.Close() + sd.SetStateCacheForTest(sc) + + written := make([]byte, 20) + written[0] = 0x01 + sd.SetTxNum(100) + require.NoError(t, sd.DomainPut(kv.AccountsDomain, rwTx, written, encAccount(7), 100, nil)) + require.NoError(t, sd.Commit(ctx, rwTx)) + + roTx, err := db.BeginTemporalRo(ctx) + require.NoError(t, err) + defer roTx.Rollback() + sd2, err := execctx.NewSharedDomains(ctx, roTx, log.New()) + require.NoError(t, err) + defer sd2.Close() + sd2.SetStateCacheForTest(sc) + + missing := make([]byte, 20) + missing[0] = 0x02 + v, _, err := sd2.GetLatest(kv.AccountsDomain, roTx, missing) + require.NoError(t, err) + require.Empty(t, v) + _, ok := sc.Get(kv.AccountsDomain, missing) + require.True(t, ok, "the negative result must be cached") + + // The domain's progress is 100 (the committed write), so any unwind at or + // below it must drop the negative instead of letting it outlive the fact. + sc.Unwind(50) + _, ok = sc.Get(kv.AccountsDomain, missing) + require.False(t, ok, "a negative observed at progress 100 must not survive an unwind to 50") +} From 1da3a896c202f6dff56679d7a75bb4768e426172 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 16:53:14 +0200 Subject: [PATCH 2/7] execution/exec: stamp warmBody negative read-fills with domain progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A total-miss negative carries no step, so the step-derived stamp pinned it near txNum 0 where no unwind could drop it. Stamp it with the domain's progress at observation time, as the SD read-fill does — covering the second fill site into the process-global StateCache. --- execution/exec/blocks_read_ahead.go | 23 ++++++++++++++++++----- execution/exec/blocks_read_ahead_test.go | 22 +++++++++++++++++++++- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/execution/exec/blocks_read_ahead.go b/execution/exec/blocks_read_ahead.go index 57eb3a6fdb0..8e4a17eb44e 100644 --- a/execution/exec/blocks_read_ahead.go +++ b/execution/exec/blocks_read_ahead.go @@ -84,7 +84,13 @@ func (bra *BlockReadAheader) SetStateCache(sc *cache.StateCache) { type cachePopulatingGetter struct { g kv.TemporalGetter sc *cache.StateCache - stepSize uint64 // for the read txNum upper bound (last txNum of the read's step) + progress func(kv.Domain) uint64 // domain progress source for stamping negative fills + stepSize uint64 // for the read txNum upper bound (last txNum of the read's step) +} + +func newCachePopulatingGetter(tx kv.TemporalTx, sc *cache.StateCache) *cachePopulatingGetter { + debug := tx.Debug() + return &cachePopulatingGetter{g: tx, sc: sc, progress: debug.DomainProgress, stepSize: debug.StepSize()} } func (cpg *cachePopulatingGetter) GetLatest(name kv.Domain, k []byte) ([]byte, kv.Step, error) { @@ -108,8 +114,15 @@ func (cpg *cachePopulatingGetter) GetLatest(name kv.Domain, k []byte) ([]byte, k // skip the file accessor stack. Mirrors revm's CacheAccount // { account: None, status: LoadedNotExisting } pattern. // Stamp with an upper bound on the value's write txNum (last txNum - // of the step it came from) so unwind invalidation is correct. - cpg.sc.PutIfAbsent(name, k, v, (uint64(step)+1)*cpg.stepSize-1) + // of the step it came from) so unwind invalidation is correct. A + // negative carries no step — stamp it with the domain's progress + // at observation time so any unwind drops it (as the SD read-fill + // does). + readTxNum := (uint64(step)+1)*cpg.stepSize - 1 + if len(v) == 0 && name != kv.CodeDomain && cpg.sc.GetCache(name) != nil { + readTxNum = cpg.progress(name) + } + cpg.sc.PutIfAbsent(name, k, v, readTxNum) } } return v, step, err @@ -229,7 +242,7 @@ func (bra *BlockReadAheader) warmBody(ctx context.Context, db kv.RoDB, header *t } var getter kv.TemporalGetter = ttx if bra.stateCache != nil { - getter = &cachePopulatingGetter{g: ttx, sc: bra.stateCache, stepSize: ttx.Debug().StepSize()} + getter = newCachePopulatingGetter(ttx, bra.stateCache) } stateReader := state.NewReaderV3(getter) @@ -300,7 +313,7 @@ func (bra *BlockReadAheader) warmBody(ctx context.Context, db kv.RoDB, header *t var getter kv.TemporalGetter = ttx var cpg *cachePopulatingGetter if bra.stateCache != nil { - cpg = &cachePopulatingGetter{g: ttx, sc: bra.stateCache, stepSize: ttx.Debug().StepSize()} + cpg = newCachePopulatingGetter(ttx, bra.stateCache) getter = cpg } stateReader := state.NewReaderV3(getter) diff --git a/execution/exec/blocks_read_ahead_test.go b/execution/exec/blocks_read_ahead_test.go index f3a88e4d15c..3566bd6bb1e 100644 --- a/execution/exec/blocks_read_ahead_test.go +++ b/execution/exec/blocks_read_ahead_test.go @@ -116,10 +116,30 @@ func TestCachePopulatingGetterWarmsColdKeys(t *testing.T) { // Negative results (missing account, empty slot) are cached as nil hits. sc = newTestStateCache() - cpg = &cachePopulatingGetter{g: stubTemporalGetter{v: nil}, sc: sc, stepSize: 1_562_500} + cpg = &cachePopulatingGetter{g: stubTemporalGetter{v: nil}, sc: sc, stepSize: 1_562_500, progress: func(kv.Domain) uint64 { return 100 }} _, _, err = cpg.GetLatest(kv.AccountsDomain, key) require.NoError(t, err) got, ok = sc.Get(kv.AccountsDomain, key) require.True(t, ok) require.Empty(t, got) } + +// A negative (missing account, empty slot) carries no write step, so a +// step-derived stamp pins it at the start of history where no unwind can drop +// it. It must be stamped with the domain's progress at observation time — +// any unwind at or below that progress then invalidates it (mirroring the SD +// read-fill). +func TestCachePopulatingGetterNegativeDroppedByUnwind(t *testing.T) { + key := []byte("\x11\x22\x33\x44\x55\x66\x77\x88\x99\xaa\xbb\xcc\xdd\xee\xff\x00\x11\x22\x33\x44") + sc := newTestStateCache() + cpg := &cachePopulatingGetter{g: stubTemporalGetter{v: nil}, sc: sc, stepSize: 16, progress: func(kv.Domain) uint64 { return 100 }} + + _, _, err := cpg.GetLatest(kv.AccountsDomain, key) + require.NoError(t, err) + _, ok := sc.Get(kv.AccountsDomain, key) + require.True(t, ok, "the negative result must be cached") + + sc.Unwind(50) + _, ok = sc.Get(kv.AccountsDomain, key) + require.False(t, ok, "a negative must not survive an unwind below the progress at which it was observed") +} From b1680448623ad0ae1a0e4b9af223edc3ad780253 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 17:14:36 +0200 Subject: [PATCH 3/7] db/state/execctx: document progress-derived step on cached empty-value hits --- db/state/execctx/domain_shared.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/state/execctx/domain_shared.go b/db/state/execctx/domain_shared.go index 3ac1177de33..f7bf0f9d99e 100644 --- a/db/state/execctx/domain_shared.go +++ b/db/state/execctx/domain_shared.go @@ -1170,6 +1170,8 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k 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()) if ok && !servableUnderBound(cStep, maxStep) { ok = false From 7eb085931c74790537cdfdbc015c136a5041b04e Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 17:19:40 +0200 Subject: [PATCH 4/7] db/state/execctx: name the no-bound maxStep sentinel --- db/state/execctx/domain_shared.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/db/state/execctx/domain_shared.go b/db/state/execctx/domain_shared.go index f7bf0f9d99e..1186e01970b 100644 --- a/db/state/execctx/domain_shared.go +++ b/db/state/execctx/domain_shared.go @@ -1097,6 +1097,10 @@ func (sd *SharedDomains) GetLatestContext(ctx context.Context, domain kv.Domain, return sd.getLatestMetered(domain, tx, k, kvmetrics.MetricsFromContext(ctx)) } +// noStepBound is the maxStep of a read the mem overlay published no per-key +// in-flight-unwind bound for: every step is servable. +const noStepBound = kv.Step(math.MaxUint64) + // 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 @@ -1125,7 +1129,7 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k wm = sd.reqMetrics } } - maxStep := kv.Step(math.MaxUint64) + maxStep := 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, @@ -1188,7 +1192,7 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k // 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.Step(math.MaxUint64) { + if dbg.AssertStateCache && maxStep == 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. From 891bc0d880d59d51760e7f5ad53c0d28eb5ed481 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 17:34:00 +0200 Subject: [PATCH 5/7] execution/exec: fix stale method name in cachePopulatingGetter doc --- execution/exec/blocks_read_ahead.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/execution/exec/blocks_read_ahead.go b/execution/exec/blocks_read_ahead.go index 8e4a17eb44e..f3f7582107c 100644 --- a/execution/exec/blocks_read_ahead.go +++ b/execution/exec/blocks_read_ahead.go @@ -79,8 +79,8 @@ func (bra *BlockReadAheader) SetStateCache(sc *cache.StateCache) { // stack cost on the EVM's first touch of any prefetched address. // // For the CodeDomain the wrapper also populates the codeHashToCode -// (codeHash→bytes) + size-cache layers via PutCodeWithHash, keyed by the -// code's own keccak hash so every cached pair is self-consistent. +// (codeHash→bytes) + size-cache layers via PutCodeWithHashIfAbsent, keyed by +// the code's own keccak hash so every cached pair is self-consistent. type cachePopulatingGetter struct { g kv.TemporalGetter sc *cache.StateCache From c69bbc34ca67f59f32dab7fdc066bfb99c239787 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 20:31:19 +0200 Subject: [PATCH 6/7] db/kv, db/state, db/state/execctx: preserve the per-key unwind bound at step 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A delete-shape unwind entry at step 0 was indistinguishable from a plain miss, so no bound was published and the ASSERT_STATE_CACHE guard compared a correct cached negative against the not-yet-deleted dying row. Plain misses now return kv.NoStepBound, letting a step-0 bound through — a young chain's whole state lives in step 0. --- db/kv/kv_interface.go | 8 +++ db/state/execctx/domain_shared.go | 13 ++--- db/state/execctx/statecache_readfill_test.go | 56 ++++++++++++++++++++ db/state/temporal_mem_batch.go | 6 ++- 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/db/kv/kv_interface.go b/db/kv/kv_interface.go index 4048d7dfd1e..95973a79970 100644 --- a/db/kv/kv_interface.go +++ b/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 diff --git a/db/state/execctx/domain_shared.go b/db/state/execctx/domain_shared.go index 1186e01970b..fa0d207d55a 100644 --- a/db/state/execctx/domain_shared.go +++ b/db/state/execctx/domain_shared.go @@ -21,7 +21,6 @@ import ( "context" "errors" "fmt" - "math" "runtime" "sync" "sync/atomic" @@ -1097,10 +1096,6 @@ func (sd *SharedDomains) GetLatestContext(ctx context.Context, domain kv.Domain, return sd.getLatestMetered(domain, tx, k, kvmetrics.MetricsFromContext(ctx)) } -// noStepBound is the maxStep of a read the mem overlay published no per-key -// in-flight-unwind bound for: every step is servable. -const noStepBound = kv.Step(math.MaxUint64) - // 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 @@ -1129,7 +1124,7 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k wm = sd.reqMetrics } } - maxStep := noStepBound + 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, @@ -1140,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 } } @@ -1153,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 } } @@ -1192,7 +1187,7 @@ func (sd *SharedDomains) getLatestMetered(domain kv.Domain, tx kv.TemporalTx, k // 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 == noStepBound { + 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. diff --git a/db/state/execctx/statecache_readfill_test.go b/db/state/execctx/statecache_readfill_test.go index 7209a1cac21..107b35f9914 100644 --- a/db/state/execctx/statecache_readfill_test.go +++ b/db/state/execctx/statecache_readfill_test.go @@ -118,6 +118,62 @@ func TestAssertStateCache_NoFalsePanicDuringInFlightUnwind(t *testing.T) { require.Equal(t, v1, v, "the cache serves the restored value") } +// Same invariant with the unwound key bound at step 0 — a young chain's whole +// state lives there. A step-0 bound must not read as "no bound": the assert +// must stay silenced while MDBX still holds the dying step-0 row, and the +// cache serves the correct negative (the key was created inside the unwound +// range, so the delete-shape diff restores nothing). +func TestAssertStateCache_NoFalsePanicDuringInFlightUnwindStepZero(t *testing.T) { + // Mutates dbg.AssertStateCache — must not run in parallel with tests that + // read it on the SD read path. + + const stepSize = uint64(16) + ctx := t.Context() + db := newTestDb(t, stepSize) + sc := newSmallStateCache() + + key := make([]byte, 20) + key[0] = 0xbb + v1 := encAccount(1) + + rwTx, err := db.BeginTemporalRw(ctx) + require.NoError(t, err) + defer rwTx.Rollback() + sd, err := execctx.NewSharedDomains(ctx, rwTx, log.New()) + require.NoError(t, err) + defer sd.Close() + sd.SetTxNum(5) + require.NoError(t, sd.DomainPut(kv.AccountsDomain, rwTx, key, v1, 5, nil)) + require.NoError(t, sd.Commit(ctx, rwTx)) + + stepBytes := make([]byte, 8) + binary.BigEndian.PutUint64(stepBytes, ^uint64(0)) + var diffs [kv.DomainLen][]kv.DomainEntryDiff + diffs[kv.AccountsDomain] = []kv.DomainEntryDiff{{Key: string(key) + string(stepBytes), Value: nil}} + + roTx, err := db.BeginTemporalRo(ctx) + require.NoError(t, err) + defer roTx.Rollback() + sd2, err := execctx.NewSharedDomains(ctx, roTx, log.New()) + require.NoError(t, err) + defer sd2.Close() + sd2.SetStateCacheForTest(sc) + + sc.Put(kv.AccountsDomain, key, nil, 2) + sd2.Unwind(3, &diffs) + + old := dbg.AssertStateCache + dbg.AssertStateCache = true + t.Cleanup(func() { dbg.AssertStateCache = old }) + + var v []byte + require.NotPanics(t, func() { + v, _, err = sd2.GetLatest(kv.AccountsDomain, roTx, key) + }, "assert must not fire when the in-flight unwind bound is at step 0") + require.NoError(t, err) + require.Empty(t, v, "the cache serves the correct negative") +} + // The read-fill after a fall-through read must not replace a live cache // entry: it never carries newer information than a flush-apply, and during an // in-flight unwind the bounded DB read can even return the not-yet-deleted diff --git a/db/state/temporal_mem_batch.go b/db/state/temporal_mem_batch.go index dd56cabb688..5f28a4a0c2d 100644 --- a/db/state/temporal_mem_batch.go +++ b/db/state/temporal_mem_batch.go @@ -236,7 +236,11 @@ func (sd *TemporalMemBatch) getLatest(domain kv.Domain, key []byte) (v []byte, s } } - return nil, 0, false + // kv.NoStepBound distinguishes "no in-flight unwind bounds this key" + // from a bound at step 0 (a delete-shape entry above whose keyStep is + // 0), so young chains — whose whole state lives in step 0 — still get + // the per-key signal. + return nil, kv.NoStepBound, false } keyS := common.ToStringZeroCopy(key) From af1698b50d77679a9cf3ec7398f824b467e53f2a Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 20:31:43 +0200 Subject: [PATCH 7/7] db/state/execctx: run read-fill regressions in the short suite At ~20-30 ms each they are not long-running, and they carry the PR's core behavioral coverage. --- db/state/execctx/statecache_readfill_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/db/state/execctx/statecache_readfill_test.go b/db/state/execctx/statecache_readfill_test.go index 107b35f9914..95d63fbafd7 100644 --- a/db/state/execctx/statecache_readfill_test.go +++ b/db/state/execctx/statecache_readfill_test.go @@ -81,9 +81,6 @@ func newSmallStateCache() *cache.StateCache { // the maxStep-bounded DB read, and the ASSERT_STATE_CACHE comparison must not // blame the cache for it. func TestAssertStateCache_NoFalsePanicDuringInFlightUnwind(t *testing.T) { - if testing.Short() { - t.Skip() - } // Mutates dbg.AssertStateCache — must not run in parallel with tests that // read it on the SD read path. @@ -179,9 +176,6 @@ func TestAssertStateCache_NoFalsePanicDuringInFlightUnwindStepZero(t *testing.T) // in-flight unwind the bounded DB read can even return the not-yet-deleted // dying row. func TestReadFill_DoesNotClobberLiveEntry(t *testing.T) { - if testing.Short() { - t.Skip() - } t.Parallel() const stepSize = uint64(16) @@ -218,9 +212,6 @@ func TestReadFill_DoesNotClobberLiveEntry(t *testing.T) { // progress at observation time, not a synthetic step-0 bound that survives // every unwind. func TestReadFill_NegativeStampedWithProgress(t *testing.T) { - if testing.Short() { - t.Skip() - } t.Parallel() const stepSize = uint64(16)