fix(evm): bound store-cache depth amplification from stacked EVM snapshots

[codchen]

Problem

Each successful EVM call frame stacks a new nested CacheMultiStore layer in DBImpl.Snapshot(). The sei go-ethereum fork only calls RevertToSnapshot on failure — the success-path DiscardSnapshot is a commented-out TODO, and vm.StateDB exposes no discard hook — so geth cannot tell sei-chain a frame succeeded and the layers are never released.

After N sequential successful calls the current context sits atop ~N nested cachekv layers. A Cosmos read at that depth then walks every layer:

• The distribution rewards() precompile (reachable via CALL/STATICCALL) → DelegationTotalRewards → stakingKeeper.IterateDelegations (an iterator through the merge-iterator chain) plus ~8–10 per-delegation point reads (Validator, IncrementValidatorPeriod, GetDelegatorStartingInfo, GetValidatorHistoricalRewards).
• Each read recurses through all N layers. Done at growing depth across the attacker's call loop, a linear number of reads becomes O(N²) wall-clock while gas stays linear.

Staking/distribution route through the regular nested cachekv (not the giga store), and those layers are empty for EVM txs — yet the merge iterators and getFromCache still traverse them.

Fix

Entirely opt-in, so all non-EVM code paths are unchanged (they never call Freeze()):

• cachekv / giga store — add frozen+dirty state, Freeze()/Unfreeze(), and a memoized readThroughParent() that skips runs of frozen empty layers on the Get path. iterator() returns the parent iterator directly when the layer is empty in-range (closing the unused cache iterator), collapsing the cacheMergeIterator chain. Reads become O(1) in depth.
• cachemulti — propagate Freeze()/Unfreeze() to sub-stores via a shared flag so lazily-materialized stores inherit the state.
• DBImpl.Snapshot (x/evm/state and the giga fork) freezes the just-superseded layer — never the base layer (snapshottedCtxs[0], the flush target read directly by GetCommittedState). RevertToSnapshot unfreezes the re-exposed layer, since a writable top must never be treated as a skippable frozen layer.

Why it's correct

A layer only receives writes while it is the newest layer; once superseded it is frozen and, if empty, stays empty. Skips are gated on frozen && !dirty, so a re-exposed-and-written layer (revert → write → snapshot) is always consulted again. Within one DBImpl, RevertToSnapshot also discards every layer above the target (so no live skip memo can go stale) and now unfreezes the target. The concurrent statedb.Copy() trace path shares layers, but is safe because the EVM never reverts below a tx's starting layer, and the unfreeze gate covers the immediate-parent case.

Verification

• New tests (frozen_skip_test.go in cachekv and giga store): read/iterate equivalence through deep empty stacks, shadowing/deletes through frozen layers (must not skip), the exact revert → write → snapshot sequence, unfreeze bypassing a stale skip memo, and Write() reset.
• Benchmark: deep-stack Get goes from O(depth) (251 ns → 3.9 µs → 16.2 µs at depth 16/256/1024) to a flat ~134 ns — ~120× faster at depth 1024.
• Passing with -race: cachekv, cachemulti, both giga stores, x/evm/state, giga/deps/xevm/state, x/evm/keeper, the distribution + staking precompiles, and the full giga integration suite. gofmt, goimports, go vet clean.

🤖 Generated with Claude Code

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Jul 7, 2026, 4:34 AM

[codecov]

Codecov Report

❌ Patch coverage is 92.07921% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.38%. Comparing base (2937bbb) to head (b2d9282).
⚠️ Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
sei-cosmos/store/cachekv/store.go	87.09%	3 Missing and 1 partial ⚠️
sei-cosmos/store/cachemulti/store.go	88.23%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3713      +/-   ##
==========================================
- Coverage   59.30%   58.38%   -0.93%     
==========================================
  Files        2274     2187      -87     
  Lines      188706   178980    -9726     
==========================================
- Hits       111917   104500    -7417     
+ Misses      66716    65206    -1510     
+ Partials    10073     9274     -799     

Flag	Coverage Δ
sei-chain-pr	76.21% <92.07%> (?)
sei-db	70.62% <ø> (+0.21%)	⬆️
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
giga/deps/store/cachekv.go	67.56% <100.00%> (+7.56%)	⬆️
giga/deps/xevm/state/state.go	96.96% <100.00%> (+0.14%)	⬆️
x/evm/state/state.go	96.96% <100.00%> (+0.14%)	⬆️
sei-cosmos/store/cachekv/store.go	87.44% <87.09%> (-0.26%)	⬇️
sei-cosmos/store/cachemulti/store.go	76.30% <88.23%> (+2.72%)	⬆️

... and 88 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cursor]

PR Summary

Medium Risk
Touches core KV cache read/iterator paths and EVM snapshot lifecycle; behavior is gated on explicit Freeze() and frozen+!dirty, with broad test coverage, but incorrect skip logic could corrupt read semantics.

Overview
Adds an opt-in Freeze / Unfreeze path so deep stacks of empty EVM snapshot cache layers no longer make every Get and iterator walk O(depth) (and many reads O(N²) at call depth N).

cachekv and giga cachekv track frozen, dirty, and a memoized readThroughParent() so cache misses skip consecutive frozen empty layers. Iterators use the same skip target and return the parent iterator directly when the layer has no in-range writes/deletes, collapsing merge-iterator chains. cachemulti propagates freeze state via a shared flag so lazily materialized substores inherit it.

DBImpl.Snapshot freezes the superseded CacheMultiStore (never the base flush layer); RevertToSnapshot unfreezes the re-exposed writable top. Same wiring in x/evm/state and giga/deps/xevm/state.

New tests cover equivalence, shadowing, revert→write→snapshot, stale memo after unfreeze, and deep-stack benchmarks.

^{Reviewed by Cursor Bugbot for commit b2d9282. Bugbot is set up for automated code reviews on this repo. Configure here.}

[seidroid]

A carefully-scoped, opt-in optimization that freezes superseded EVM snapshot cache layers so deep reads skip empty layers, fixing O(N²) read amplification; the core logic is correct and well-tested for the single-DBImpl case, with a few non-blocking concerns around the cross-Copy stale-memo edge and the breadth of the iterator fast path. No blockers found.

Findings: 0 blocking | 6 non-blocking | 2 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Cross-Copy() deep-ancestor staleness: readThroughParent's memo is only bypassed when the immediate parent's live frozen/dirty bit changes. In the statedb.Copy() path (e.g. block_trace_profiled.go), layers are shared between DBImpls. If one DBImpl reverts to and writes into a deep shared ancestor A while another (copy) still sees its immediate parent as frozen-empty, the copy's memoized skip past A would return stale data (miss A's new writes). The PR's own comment/description acknowledges this and argues safety ('the EVM never reverts below a tx's starting layer; trace copies are read-only'), but none of the new tests exercise the two-DBImpl shared-chain deep-revert case — only the immediate-parent case (TestUnfreezeBypassesStaleSkipMemo). Recommend adding a test that reverts/writes a non-immediate ancestor shared via Copy, or documenting/asserting the 'no revert below tx start' invariant more concretely.
• Asymmetry: the giga store (giga/deps/store/cachekv.go) gains the frozen-skip only on the Get path (readThroughParent) but its iterator() was not given the fast-path/skip treatment that sei-cosmos/store/cachekv/store.go received, so deep giga iteration remains O(depth). This is correctness-safe (giga backs evm/bank and is point-read heavy) but worth noting so the asymmetry is intentional, not an oversight.
• Second-opinion passes produced no findings: codex-review.md states 'No material issues found in the PR diff.' and cursor-review.md is empty. Nothing to merge from either; noting for completeness.
• Minor: cachemulti.Store.Freeze()/Unfreeze() iterate cms.stores under mu.RLock() but iterate cms.gigaStores with no lock. This appears safe today (gigaStores is populated at construction and only mutated by SetGigaKVStores, not concurrently with Freeze), but the inconsistency is worth a comment given the file's emphasis on the OCC scheduler calling these concurrently.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] This gate only checks the immediate parent's live frozen/dirty bit. Once a memo is stored (line 106), it can skip multiple deeper frozen-empty ancestors; if one of those non-immediate ancestors is later unfrozen and written (possible when layers are shared across DBImpls via statedb.Copy()), a store whose immediate parent is still frozen-empty will keep using the stale memo and miss those writes. Within a single DBImpl this can't happen (revert discards everything above the target), but the shared-Copy case relies on the unproven 'EVM never reverts below a tx's starting layer' invariant. Consider adding a test that exercises a deep revert+write on a Copy-shared chain, or invalidating the memo more defensively.

[seidroid]

[suggestion] This !cache.Valid() fast path returns the raw parent iterator (instead of wrapping in a CacheMergeIterator) for every cachekv layer whose in-range cache is empty — not just frozen EVM layers. Unlike the Get path (gated on frozen), this is a global behavioral change: callers now receive the parent store's iterator type directly, so anything relying on the merge wrapper's Domain() or on the iterator identity/type for an empty-in-range layer will see different behavior. It looks behavior-preserving for iteration order/keys/values, but the change is broader than the PR's 'entirely opt-in' framing suggests — worth calling out explicitly.

[claude]

I didn't find any bugs, but this changes core cachekv/cachemulti store read/iterator paths plus the EVM Snapshot/RevertToSnapshot lifecycle in both x/evm/state and the giga fork, so it warrants a human look before merging.

What was reviewed

• The Freeze/Unfreeze/readThroughParent skip logic in both cachekv copies and the iterator fast-path (parent-return when the cache is empty in-range).
• cachemulti frozen-flag propagation to lazily-materialized substores (shared *atomic.Bool in Store value copies).
• The Snapshot (freeze superseded layer, skip base at index 0) and RevertToSnapshot (unfreeze re-exposed top) wiring in both x/evm/state/state.go and giga/deps/xevm/state/state.go.
• The concurrent-Copy() trace path: shared snapshottedCtxs layers between original and traced copy, and the description's claim that the immediate-parent unfreeze gate keeps memoized skip pointers correct.

Worth a human look because the invariant ("a frozen layer never gains writes as long as it stays superseded") is load-bearing across Snapshot/Revert/Copy and any future call site that stacks or re-exposes cache layers.

Extended reasoning...

Overview

This PR adds an opt-in Freeze/Unfreeze mechanism to cachekv (both sei-cosmos/store/cachekv and giga/deps/store) and propagates it through cachemulti. It uses that mechanism in DBImpl.Snapshot/RevertToSnapshot (both the main and giga forks) to short-circuit reads through empty superseded snapshot layers so that a Cosmos read at EVM call depth N doesn't walk N layers. The read path adds a memoized readThroughParent() that skips runs of frozen+empty layers; the iterator path returns the parent iterator directly when the cache has no in-range writes/deletes. Test coverage is broad (equivalence, shadowing, revert→write→snapshot, stale-memo-after-unfreeze, Write() reset, deep-stack benchmark).

Security risks

Not an auth/crypto/permissions surface. The main risk is correctness: a wrongly-taken skip would silently drop writes from reads and change state root computation, which would be app-hash-breaking. The PR is labeled non-app-hash-breaking, consistent with the design (skips are only taken when frozen && !dirty, and dirty is set on any Set/Delete). The concurrent-Copy() interaction is the subtlest case — the design gates on the immediate parent's live frozen bit, but shared ancestors between original and copy still need the "EVM never reverts below a tx's starting layer" invariant to hold for the whole memoized skip chain, which I could not fully verify from the diff alone.

Level of scrutiny

High. cachekv and cachemulti are core Cosmos SDK store primitives — every read and iteration in the app funnels through them, and every module (not just EVM) is affected any time freeze flags get propagated. Even though the code path is opt-in, mistakes in the concurrency/memoization logic would corrupt read semantics under load, and would likely not surface in unit tests. The Snapshot/Revert wiring in DBImpl is on the EVM tx execution hot path.

Other factors

• Complexity: seven files, ~500 lines of net change touching atomics, memoization, iterator plumbing, and shared-flag propagation.
• The PR description is thorough and the invariants are documented in-code, which helps a human review land, but the reasoning about the Copy() trace path and re-exposed shared layers is the kind of thing that benefits from a second pair of eyes.
• Bugbot (external tool) also flagged this as "Medium Risk"; that's not a blocker but is consistent with my read.
• CI passes; codecov reports 92% patch coverage; buf is green.