Implement new Giga GarbageCollector interface

[yzang2019]

Summary

Retention for Giga's storage components is currently decided by each store's own
pruner, independently and with no knowledge of the others. This PR implements
gc.PrunableStore for all four of them so a single StorageGarbageCollector
can manage the pruning logic for the whole fleet.

Each cycle the collector asks every store for its ingest height and for the
oldest block it must keep in order to serve the shared RollbackWindow, takes
the minimum of those answers, and prunes every store to it. The stores therefore
stay mutually consistent — a rollback target that one store can serve is one they
can all serve, which per-store pruners could not guarantee.

What's included

Store	Change
BlockDB	New litt_block_gc.go; adds a RetentionWindow config field
ReceiptDB	New litt_receipt_gc.go; adds an ExternalPruning config field that gates the existing background pruner; adds a litt GCFilter so reclamation follows the retention floor
StateWAL	New state_wal_gc.go; PruneBelow calls seiwal directly
FlatKV (SC)	New store_gc.go; snapshot-aware pruning boundary; adds an ExternalPruning config field

Notable design points

ExternalPruning() bool on PrunableStore

Two stores keep a pruner of their own — FlatKV (SnapshotKeepRecent) and
ReceiptDB (KeepRecent) — because both still run without a collector: FlatKV in
the seidb tools and bench paths, ReceiptDB on any node with
rs-backend = "littidx". Both pruners active at once is unsafe: the local one
would delete the very data the collector is holding to serve the rollback window.
Standing the local one down with nothing to replace it is equally unsafe, and
fails silently — the retention floor simply stops advancing.

ExternalPruning makes both combinations unrepresentable rather than merely
discouraged. Each store answers it from the same config.ExternalPruning field
its own pruner reads to stand down, so "the collector prunes this store" and "the
store does not prune itself" are one fact instead of two settings that can
disagree. Stores with no pruner of their own return true unconditionally.

A store that reports false is still asked for its boundary and still holds the
shared minimum down — it just never receives PruneBelow. Dropping it from the
vote instead would prune the WAL out from under the snapshots it replays from.

Neither ExternalPruning field is reachable from app.toml (both are
mapstructure:"-"). Enabling one is only correct when the store is registered
with a running collector, which is a property of how the process was wired and
not something an operator can assert. Where it is checkable it is checked:
newReceiptBackend rejects pebbledb + ExternalPruning, because that backend
is not a gc.PrunableStore and would end up with no pruner at all.

The retention floor gates reclamation, and the TTL is only an age failsafe

Both litt-backed stores now pair their TTL with a GCFilter, so a record is
reclaimed only when it is both below the store's retention floor and
older than the TTL. Previously ReceiptDB had no filter, which made the TTL the
sole reclamation mechanism, and it was sized as KeepRecent × 2s. Under
ExternalPruning the enforced retention is RollbackWindow + KeepRecent, so a
TTL sized for KeepRecent alone expired receipt bodies for blocks the collector
still considered live — ErrNotFound inside the rollback window, which is the
exact cross-store guarantee the collector exists to provide.

With the filter in place the TTL no longer has to know how many blocks anything
is, so littTTLPerBlock is gone and both stores take a flat duration
(RetentionTime / littRetentionTime), defaulting to 1 hour. Visible retention
follows the floor and reclamation can no longer lead it.

Retention semantics

With R = a store's GetRetentionWindow and F = LatestBlock - RollbackWindow - R,
collection guarantees, per managed store:

1. Nothing needed to roll back to any block in [LatestBlock - RollbackWindow, LatestBlock] is deleted.
2. No data at or above F is deleted — so even after rolling back to
   LatestBlock - RollbackWindow, the most recent R blocks are still readable.
3. Data below F is eventually deleted, each store reclaiming on its own schedule.

Guarantee 2 is why pruning is to the shared minimum rather than to each store's
own boundary: a retained snapshot is only restorable if the blocks that follow it
survive in the contiguous stores. This is recorded on
StorageGarbageCollectorConfig.RollbackWindow and, in block terms, on
BlockDBConfig.RetentionWindow.

GetRetentionWindow reports extra retention beyond the shared
RollbackWindow, with InfiniteRetentionWindow (-1) meaning never prune.
Note that ReceiptStoreConfig.KeepRecent == 0 already means "keep everything",
which is the opposite of what 0 means to the collector, so it is mapped to
InfiniteRetentionWindow rather than passed through.

StateWAL answers 0 unconditionally, and has no retention config of its own. Its
depth is not its to declare: it is a replay source, and SC/SS already express how
far back it must reach by answering their oldest live snapshot as a boundary. A
window here would be additive on top of the shared minimum, retaining every
managed store further back rather than the WAL alone — a fleet-wide decision
wearing a per-store name, which is what RollbackWindow already is.

Snapshot stores

FlatKV restores only at a snapshot boundary and replays the WAL forward from
there, so what it must retain is not a block range but the newest snapshot at or
below the target. GetPruningBoundary reports that, which is what holds the WAL
back for it.

Config changes

• littblock.BlockDBConfig.Retention → RetentionTime, default 24h → 1h.
  It is an age floor, not a retention policy; how much history BlockDB keeps is
  RetentionWindow. The AutobahnBlockDBConfig.Retention override keeps its
  name, since its retention JSON key is a persisted config format.
• littblock.DefaultConfig now leaves RetentionWindow at 0 (was 10000).
  It is an input to a minimum shared across every managed store, so a non-zero
  default here would have held ReceiptDB, the state WAL and the SC snapshots
  10k blocks further back on BlockDB's say-so. Every other store reports 0; a
  deployment wanting deeper block history sets it at the call site.
• New ExternalPruning on ReceiptStoreConfig and FlatKVConfig, both
  mapstructure:"-" and both defaulting to false.

ReceiptStoreConfig.KeepRecent is deliberately left at 0 (keep everything).
Nothing here couples it to RetentionWindow, because the two fields disagree
about 0: BlockDB folds only negatives to InfiniteRetentionWindow, so 0
there is the most aggressive setting, while ReceiptDB folds <= 0 to infinite,
so 0 there means never prune. KeepRecent cannot express "nothing beyond the
rollback window" at all. Reconciling the two is left to the wiring PR, along
with what the shared window should actually be.

Also in this PR

• Renames littblock.LittBlockConfig to BlockDBConfig (ripples into sei-tendermint).
• Renames the BlockDB table from ledger to blocks. No migration is needed —
  BlockDB is not deployed on any network and no such data exists. Because the
  table name is persisted layout rather than an identifier, NewBlockDB would
  otherwise open a fresh empty table beside the old data, and an empty store is
  indistinguishable from a correct one until something asks for history. A
  refuseLegacyTable check at open turns that into a startup error naming the
  directory, for dev/CI/devnet homes written before the rename; it can be deleted
  once no such directory remains.
• Documents seiwal.WAL.PruneBefore as safe to call off the WAL owner's
  goroutine, unlike most of the interface, which is what lets the collector prune
  the WAL from its own goroutine.

Not in this PR

Not moving all stores to use StorageGarbageCollector yet. We expect to
construct it in a future PR when we decide to unify the pruning for mainnet. Both
ExternalPruning fields deliberately default to false: no behavior change by
default.

That wiring PR is also where the remaining guard belongs. The receipt path can
reject its unsupported combination at startup, but nothing on the FlatKV path can
validate that a collector exists — "a collector exists" is not knowable from that
package. Keeping the field unreachable from config is what stands in for the
check until then.

Also worth knowing at wiring time: enabling ExternalPruning changes the shape of
snapshot retention rather than just its depth. Snapshot count becomes roughly
RollbackWindow / SnapshotInterval instead of SnapshotKeepRecent + 1.

Not managing SS yet in this PR since SS doesn't have snapshot capability yet.

Testing

• A *_gc_test.go suite per store, plus collector coverage for the self-pruning
  path (a store reporting false keeps its vote but receives no PruneBelow).
• litt_receipt_gcfilter_internal_test.go covers the filter as a predicate and
  end-to-end: blocks below the floor are reclaimed by a real litt GC pass, those
  at or above are retained. It fails if the filter is removed.
• litt_receipt_pruner_internal_test.go pins when the local pruner runs, as a
  pure predicate rather than a timing assertion.
• litt_block_legacy_table_test.go covers the pre-rename refusal, including that
  a refused open leaves the directory exactly as it found it.
• Config defaults that moved are re-recorded in testdata/*.golden, so each new
  value lands in a diff.
• Run under -race.

[cursor]

PR Summary

High Risk
Changes rollback and retention coordination across WAL, snapshots, blocks, and receipts; mis-wiring ExternalPruning or boundary math could prune data still needed for rollback, though defaults leave current per-store pruning behavior unchanged until a future collector wiring PR.

Overview
Implements gc.PrunableStore on BlockDB, littidx ReceiptDB, StateWAL, and FlatKV so a single StorageGarbageCollector can compute a fleet-wide prune height from each store’s head, retention window, and pruning boundary, then prune only stores that report ExternalPruning().

The collector still asks self-pruning stores for boundaries (they hold the shared minimum down) but skips PruneBelow on them; prune logs mark those as selfPruned. ExternalPruning on Receipt and FlatKV config (not in app.toml, default false) is the single switch that stands down local pruners (KeepRecent background pruner, SnapshotKeepRecent / tryTruncateWAL) when the collector owns retention. Pebble receipt + ExternalPruning is rejected at open.

Receipt (littidx) adds a litt GCFilter so bodies reclaim only below the retention floor (plus TTL); TTL becomes a flat littRetentionTime instead of KeepRecent × 2s. BlockDB renames LittBlockConfig → BlockDBConfig, Retention → RetentionTime (default 1h), adds RetentionWindow (default 0), renames the on-disk table ledger → blocks with refuseLegacyTable at open, and always participates externally. FlatKV prunes snapshots via PruneBelow when external; seiwal.WAL.PruneBefore is documented as safe from the collector goroutine.

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

[claude]

Beyond the inline finding, I also checked two other candidate issues: the BlockDB litt table rename from "ledger" to "blocks" (no live production data exists under the old table name for this not-yet-deployed Giga store, so this isn't an on-disk compatibility break), and littidx ReceiptDB losing its background pruner with no ExternalPruning fallback (expected per this PR's description — the collector wiring lands in a follow-up PR, consistent with this repo's staged-rollout convention).

Extended reasoning...

This PR is large and touches critical, not-yet-wired pruning logic across four Giga storage stores. One nit-level bug was already found and posted inline (statewal.New skips config.Validate). Beyond that, I reviewed two additional candidate concerns raised by finder agents and ruled both out: the BlockDB table rename is safe because the Giga block store has no production data to migrate yet, and the ReceiptDB pruner removal is explicitly called out in the PR description as intentional pending the follow-up collector-wiring PR.

[codecov]

Codecov Report

❌ Patch coverage is 84.41558% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.74%. Comparing base (ab08efb) to head (c07fa75).

Files with missing lines	Patch %	Lines
sei-db/state_db/sc/flatkv/store_gc.go	82.66%	9 Missing and 4 partials ⚠️
sei-db/ledger_db/receipt/litt_receipt_store.go	85.71%	3 Missing and 2 partials ⚠️
sei-db/ledger_db/block/littblock/litt_block_gc.go	77.77%	4 Missing ⚠️
sei-db/state_db/statewal/state_wal_gc.go	71.42%	4 Missing ⚠️
sei-db/ledger_db/block/littblock/litt_block_db.go	85.00%	3 Missing ⚠️
...-db/ledger_db/block/littblock/litt_block_config.go	81.81%	1 Missing and 1 partial ⚠️
sei-db/ledger_db/receipt/litt_receipt_gc.go	88.23%	2 Missing ⚠️
sei-tendermint/config/autobahn.go	50.00%	2 Missing ⚠️
sei-db/state_db/sc/flatkv/snapshot.go	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3868      +/-   ##
==========================================
- Coverage   59.45%   58.74%   -0.71%     
==========================================
  Files        2319     2250      -69     
  Lines      198379   190825    -7554     
==========================================
- Hits       117946   112108    -5838     
+ Misses      69235    68078    -1157     
+ Partials    11198    10639     -559     

Flag	Coverage Δ
sei-chain-pr	65.63% <84.07%> (?)
sei-db	70.41% <ø> (-0.22%)	⬇️
sei-db-state-db	?
sei-db-state-db-pr	76.26% <84.74%> (?)

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

Files with missing lines	Coverage Δ
sei-db/config/giga_config.go	88.88% <ø> (ø)
sei-db/config/receipt_config.go	91.48% <ø> (ø)
sei-db/ledger_db/block/blocksim/blocksim.go	9.02% <100.00%> (ø)
sei-db/ledger_db/receipt/receipt_store.go	67.00% <100.00%> (+0.33%)	⬆️
sei-db/management/gc/storage_garbage_collector.go	98.29% <100.00%> (+0.07%)	⬆️
.../management/gc/storage_garbage_collector_config.go	100.00% <ø> (ø)
sei-db/seiwal/seiwal.go	71.42% <ø> (ø)
sei-db/state_db/sc/flatkv/config/config.go	72.30% <ø> (ø)
sei-db/state_db/sc/flatkv/store.go	78.93% <100.00%> (ø)
sei-db/state_db/statewal/state_wal_impl.go	83.20% <100.00%> (+0.27%)	⬆️
... and 9 more

... and 74 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.

[claude]

Beyond the inline finding, I also checked the littidx TTL depth under ExternalPruning: the litt TTL is set from KeepRecent independent of the collector's RollbackWindow, so it could in principle expire receipt bodies before the collector's retention floor — but with no StorageGarbageCollector constructed anywhere yet and ExternalPruning unreachable from app.toml, this can't produce a live gap today. Not a blocker.

Extended reasoning...

Checked whether littidx's TTL (derived from KeepRecent, littTTLPerBlock) could expire receipt bodies earlier than a StorageGarbageCollector's RollbackWindow would require once ExternalPruning is wired up. Confirmed this is currently unreachable: ExternalPruning is mapstructure:"-" (no app.toml key) and no code in this PR or elsewhere constructs a StorageGarbageCollector, so the mismatch has no live path today. Recording this so a future PR that wires up the collector re-checks the TTL depth against RollbackWindow before enabling ExternalPruning in production.

[seidroid]

Well-documented, well-tested implementation of gc.PrunableStore across the four Giga stores, with the collector-side ExternalPruning plumbing looking correct. One blocking issue: the LittDB block table is silently renamed from "ledger" to "blocks", which is a persisted on-disk identifier with no migration, and is not mentioned anywhere in the PR description.

Findings: 1 blocking | 8 non-blocking | 4 posted inline

Blockers

• None at the file/PR level.
• 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

• Cursor's second-opinion pass produced no output (cursor-review.md is empty), so this review reflects only Claude's and Codex's findings. Codex reported a single finding (the block table rename), which I independently confirmed and raised inline.
• sei-db/state_db/sc/flatkv/store.go: InitializeDataDirectories and applyPebbleMetricsConfig are moved verbatim from the top of the file to the bottom (and var _ Store = (*CommitStore)(nil) moved up) — ~80 lines of pure code motion with no behavior change and no relation to GC. Worth dropping to keep the diff reviewable.
• No end-to-end test constructs a real StorageGarbageCollector over the four concrete stores: each store is tested against the interface in isolation, and the collector only against mocks. The load-bearing interaction the PR exists for — FlatKV's snapshot boundary holding the state WAL back — is therefore only covered by mockStore. Understood that wiring is deferred, but a single integration test over the real four would pin the invariant the mocks assert.
• The preconditions in store_gc.go and state_wal_gc.go ("FlatKV must be managed alongside the state WAL, and vice versa") are documented but unenforced. The converse combination is the sharper one: self-pruning FlatKV + collector-managed state WAL leaves tryTruncateWAL live, and it can truncate the WAL above the collector's shared minimum, dropping the replay range SS still needs. Consider a construction-time check wherever the collector is eventually wired.
• statewal and flatkv each got a race-detector test for the GC surface against a concurrent writer (TestGCConcurrentWithWriter, TestGCConcurrentWithCommitter); blockDB did not. It is fully mutex-guarded so it is likely fine, but note that GetLatestBlock and PruneBelow both take s.mu, the same mutex WriteBlock/WriteQC hold — so the collector's goroutine can now block behind block writes. A matching parallel test would pin both the safety and the intent.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] This field is mapstructure-addressable while its exact counterpart on ReceiptStoreConfig is mapstructure:"-", and the receipt-side comment gives the reason: enabling it is "only correct when this store is registered with a running StorageGarbageCollector, which is a property of how the process was wired and not something an operator can assert from app.toml. Exposing a key for it would let a node stand its pruner down with nothing to replace it, and the resulting unbounded growth is silent."

That argument applies at least as strongly here — this flag stands down two mechanisms (pruneSnapshots and tryTruncateWAL), so a stray external-pruning = true under [state-commit.flatkvconfig] leaves both snapshots and the state WAL with nothing bounding them. The toml_test.go addition keeps it out of the generated template, but mapstructure still binds a hand-written key.

Two options: tag it mapstructure:"-" to match the receipt field, or, if it is meant to stay operator-settable, pin the new read in testutil/configtest per the repo's configuration-reads guidance.

Separately: the receipt path rejects the unsupported combination at startup (newReceiptBackend errors on pebble + ExternalPruning), but nothing on the FlatKV path validates that a collector exists. Worth an equivalent guard wherever the collector gets constructed.

[claude]

I didn't find any new issues in this pass, but this is a large, critical-path storage change — four different store implementations (BlockDB, ReceiptDB, StateWAL, FlatKV) are being given a shared PrunableStore/ExternalPruning contract that a follow-up StorageGarbageCollector will drive — so it's worth a human look rather than a bot approval.

What was reviewed: the ExternalPruning/GetRetentionWindow/GetPruningBoundary implementations across all four stores and the collector's cutLine/pruneHeight/self-pruning-store logic in storage_garbage_collector.go; confirmed the empty-store PruneBelow issue flagged earlier in this thread has been fixed (pruneBlocksBelow now no-ops when latestVersion <= 0); and checked FlatKV's ExternalPruning mapstructure:"-" omission — it's intentionally unexposed in app.toml like its receipt-store sibling, not a missing guard.

Extended reasoning...

This PR introduces gc.PrunableStore implementations for BlockDB, ReceiptDB, StateWAL, and FlatKV, plus the StorageGarbageCollector decision logic that will eventually drive pruning across all of them, and renames the public littblock.LittBlockConfig type (rippling into sei-tendermint). No collector is wired up yet in this PR — that is deliberately left for a follow-up — so the immediate behavioral risk is low, but the contracts being established here (ExternalPruning, GetRetentionWindow, GetPruningBoundary, the cutLine/pruneHeight minimum) are exactly what future pruning correctness will depend on.

No security-sensitive surface (auth/crypto/permissions) is touched; the risk here is data-retention correctness (accidentally pruning data a store still needs, or leaking unbounded growth), not exploitability.

Scrutiny should be high: this is core sei-db storage-lifecycle code, not app/consensus logic, but a mistake in the shared-minimum logic or in an ExternalPruning stand-down could silently either delete data needed for rollback or grow disk unbounded once the collector is wired up in the next PR. The PR is also large (29 files) and threads a subtle invariant (self-pruning stores still vote but never receive PruneBelow) through four independent implementations, which increases the chance of a one-store inconsistency.

The thread already contains substantive back-and-forth (cursor[bot] and my own prior comments) covering an empty-store PruneBelow bug (now fixed), a BlockDB table rename the author confirmed is safe (no existing data), and two still-open nits (statewal.New not calling config.Validate(), and the receipt store's litt TTL not accounting for RollbackWindow once ExternalPruning is wired up) that were assessed as non-blocking since no current code path can reach them. Given the size, the number of new invariants being introduced, and that not every open thread has been explicitly resolved, this is a good candidate for a human pass before merge.

[claude]

This PR implements a new fleet-wide StorageGarbageCollector and wires gc.PrunableStore into BlockDB, ReceiptDB, StateWAL, and FlatKV — no bugs were found this run, but given the scope and criticality of this change I think it's worth a human look before merging.

Beyond the bug hunting system's findings, this run also checked: FlatKV/state-WAL/BlockDB/ReceiptDB GetPruningBoundary/GetLatestBlock contracts for off-by-one and empty-store edge cases, the self-pruning-vs-collector ExternalPruning handoff in all four stores, and the two candidate issues the bug hunting system ruled out this run (receiptDB's boundary vs. its local pruner floor, and blockDB's ExternalPruning vs. its RetainHeight-driven pruner) — both were not real bugs. I also confirmed the concerns from earlier review rounds (the empty-store PruneBelow gap in littReceiptStore, the stale .LittBlockConfig/.BlockDBConfig comment, and the unreachable RetentionWindow validation in statewal) have since been addressed in later commits.

Extended reasoning...

Overview

This PR adds a shared StorageGarbageCollector (sei-db/management/gc) and implements gc.PrunableStore for four storage components — littblock.BlockDB, the littidx ReceiptDB, StateWAL, and FlatKV's CommitStore — so a single collector can coordinate pruning/retention across the whole Giga storage fleet instead of each store pruning independently. It also renames littblock.LittBlockConfig to BlockDBConfig (rippling into sei-tendermint) and renames the underlying litt table from ledger to blocks. None of this is wired into a running collector yet per the PR description — that is deferred to a follow-up PR.

Security risks

No injection/auth/crypto surface here. The real risk category is data-durability/consensus-adjacent correctness: a wrong pruning boundary or retention-window calculation could silently delete blocks, receipts, WAL entries, or state snapshots still needed to serve a rollback, or (in the WAL table-rename case) strand already-persisted data under an old table name. These failure modes are typically silent (unbounded disk growth or missing data discovered much later) rather than loud, which raises the value of a careful human pass even absent a concrete bug today.

Level of scrutiny

High. This is new infrastructure for coordinating retention across BlockDB, ReceiptDB, StateWAL and FlatKV — all storage paths that back consensus and RPC serving for Giga. Cursor's own summary flagged it "High Risk" for the same reason. The code is not live yet (no construction site calls NewStorageGarbageCollector), which lowers the blast radius today, but the interface contracts being established here (GetPruningBoundary, GetRetentionWindow, ExternalPruning) will govern real pruning behavior once wired up in the next PR, so getting them right now matters.

Other factors

Test coverage is extensive (a *_gc_test.go suite per store plus collector-level tests, per the PR description), and this run's independent look, plus the bug hunting system's, did not surface a live bug. Several issues raised in earlier review rounds on this PR (an empty-store PruneBelow gap in the receipt store, a stale comment, and a validation method that was never called) have since been fixed or the underlying field removed entirely, which is a good signal the author is engaging with feedback. Given the size (29 files) and the criticality of the code paths involved, I'd still like a human maintainer to sign off.

[claude]

No new bugs found this run beyond the existing inline findings on this PR. Beyond those, I checked whether CommitStore.reopenWAL/Rollback swapping in a new stateWALImpl would orphan a collector-held gc.PrunableStore reference to the old (now-closed) WAL — it would, but no code in this PR (or elsewhere in the tree) constructs a StorageGarbageCollector yet, so this is a wiring concern for the follow-up PR that actually registers these stores, not a bug in what's merged here.

Extended reasoning...

The candidate issue was: stateWALImpl's identity as a gc.PrunableStore is invalidated whenever CommitStore.reopenWAL or CommitStore.Rollback swaps s.wal for a freshly constructed statewal.New(...) instance. If a StorageGarbageCollector held a reference to the old stateWALImpl object, that reference would silently go stale (PruneBelow on it would just error 'state WAL is closed', and the real live WAL would never be pruned again). This is refuted as a bug in the current PR because no code anywhere in the tree constructs a StorageGarbageCollector with these stores yet — the PR description explicitly defers that wiring to a follow-up PR. Whoever writes that wiring will need to either re-register the store after a reopen, or expose a stable handle that survives it; that is a real design point to track at wiring time, but not something the current diff gets wrong.

[cody-littley]

Might be useful to call out the following invariants:

Storage garbage collection guarantees the following invariants:

1. Garbage collection will not delete any data that is necessary to roll back to any block 
   between LatestBlock and (LatestBlock - RollbackWindow), inclusive.
2. Garbage collection will not delete block DB data that is before 
   (LatestBlock - RollbackWindow - RetentionWindow). This ensures that even if the 
   system rolls back to block (LatestBlock - RollbackWindow), it is still possible to read any
   block from the last RetentionWindow blocks.
3. Garbage collection will eventually delete block data older than 
   (LatestBlock - RollbackWindow - RetentionWindow).

[cody-littley]

I suggest we paste this block of invariants at each RetentionWindow or RollbackWindow config (with wording adjusted a little, the above is specific to block storage).

[cody-littley]

LGTM. Technically Flush() is also legal to call from anywhere, but it's ok for the godocs to be more restrictive than the code when it comes to threading model.

[cody-littley]

The following would probably simplify this code a lot:

• Create a method getSnapshotBlocks() ([]uint64, error) that returns a slice of block numbers in sorted order for all the snapshots on disk.
• Create a method deleteSnapshot(block uint64) error that deletes a single snapshot with the given block number

This lets us avoid the lambda function, and splits apart the logic for traversing the directory structure and the logic for deciding which blocks to keep and which ones to drop.

[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	Aug 10, 2026, 2:33 PM

[seidroid]

A carefully staged, unusually well-documented and well-tested implementation of gc.PrunableStore across the four Giga stores, with a genuine correctness gap: the littidx receipt TTL is still derived from KeepRecent alone, so under ExternalPruning receipt bodies inside the collector's shared RollbackWindow can expire while the retention floor still claims them servable. Also flagged: the two new ExternalPruning config fields carry live-looking mapstructure keys that contradict their own docs and a characterization-test comment.

Findings: 2 blocking | 10 non-blocking | 7 posted inline

Blockers

• Cursor's second-opinion pass produced no output (cursor-review.md is empty), so this synthesis merges only Claude's and Codex's findings.
• 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

• Nothing in the tree constructs a StorageGarbageCollector or sets either ExternalPruning field, so every path added here is unreachable in a running node today. That is stated in the PR description and is fine as staged work, but it means the interaction between the four stores is only exercised by mocks in storage_garbage_collector_test.go — there is no test where the real FlatKV, StateWAL, ReceiptDB and BlockDB vote in one cycle. Worth adding when the collector is wired, since the cross-store invariant (SC's snapshot boundary holding the WAL back) is the whole point of the design and is currently pinned only against mockStore.
• flatkv/config.Config.ExternalPruning has no equivalent of newReceiptBackend's "reject the combination we cannot honor" guard — the doc says so explicitly and defers it to wherever the collector is constructed. Please make sure that follow-up actually lands a "ExternalPruning set but this store is not registered with a running collector" check; the failure mode (snapshots and the state WAL both unbounded) is silent and expensive, and the flag alone stands down two mechanisms.
• sei-db/state_db/sc/flatkv/store.go: relocating InitializeDataDirectories/applyPebbleMetricsConfig to the bottom of the file and moving var _ Store = (*CommitStore)(nil) up is pure code motion unrelated to the GC work. It adds ~76 lines of diff noise and makes git blame on those functions point at this PR. Consider dropping it or splitting it out.
• TestReceiptLocalPrunerAdvancesFloorWithoutCollector pays a real 1–2s wall-clock wait against a jittered ticker with a 6s require.Eventually budget. The comment justifies it and TestRunsLocalPruner covers the decision without waiting, so this is only a note: it is the kind of test that becomes the flake on a loaded CI runner.
• 6 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[blocker] The litt TTL is the one thing that does not follow the collector's window. (Also raised by Codex as P1.)

newLittReceiptStore still sets receipts.SetTTL(KeepRecent * littTTLPerBlock) (line 162), i.e. the body TTL covers roughly KeepRecent blocks of wall clock. But under ExternalPruning the retention the collector enforces is RollbackWindow + GetRetentionWindow() = RollbackWindow + KeepRecent blocks.

Concretely, with ExternalPruning = true, KeepRecent = 100_000 and RollbackWindow = 100_000: the collector holds the floor at head - 200_000 and every other store retains to match, so a rollback to head - 150_000 is supposed to be servable — but litt has already expired the receipt bodies for everything older than ~100_000 × 2s. GetReceiptFromStore returns ErrNotFound for blocks the retention floor says are live, which is exactly the cross-store consistency guarantee the collector exists to provide.

The type doc here says "KeepRecent is what this store asks for either way: the pruner's window when it runs, the collector's retention window when it does not, and the litt TTL in both cases" — the third clause is the bug. The TTL needs to cover RollbackWindow + KeepRecent when external pruning is on, which means RollbackWindow has to reach this constructor (or the TTL has to be disabled under ExternalPruning and reclamation left entirely to the collector).

Latent today since nothing wires a collector, but the mapping between KeepRecent, GetRetentionWindow and the TTL is defined here, so this is where it should be resolved rather than in the wiring PR.

[seidroid]

[suggestion] The doc directly above says "Like KeepRecent this is not read from the receipt-store config", but unlike KeepRecent (line 52, mapstructure:"-") this field carries a live-looking key. ReadReceiptConfig only does explicit opts.Get lookups so nothing decodes it today — but the tag is the only structural expression of the invariant, and it currently says the opposite of the prose. Suggest mapstructure:"-" to match KeepRecent; testdata/receipt-store.golden records the field either way (as it does for KeepRecent), so the change is inert for the characterization suite.

[seidroid]

[suggestion] This comment states as fact that ExternalPruning is tagged mapstructure:"-", but receipt_config.go:74 tags it mapstructure:"external-pruning". Per testutil/configtest/AGENTS.md these manifest exclusions are the recorded contract a replacement implementation reads, so a comment that misdescribes the tag is the specific drift the suite is meant to prevent. Fix the tag (preferred) or the comment.

[seidroid]

[suggestion] Same as the receipt-side field: the doc says "Keeping the field unreachable from config is what replaces that guard", but mapstructure:"external-pruning" is a real key name. GetConfig reads FlatKV via explicit v.IsSet("state-commit.flatkv.*") calls and this one is not among them, so it is unreachable in practice — but toml_test.go:61 only asserts the key is absent from the template, which would keep passing if a future viper.Unmarshal path picked the struct up. Given the doc's own point that this one flag stands down both pruneSnapshots and tryTruncateWAL, mapstructure:"-" makes the claim structural rather than incidental.

[seidroid]

[nit] pruned is incremented at line 90 but never read — the per-snapshot logger.Info below already carries the information. Either drop the counter or use it (e.g. a single summary log with the count, which would also be quieter than one line per snapshot when RollbackWindow / SnapshotInterval is large).

[seidroid]

[suggestion] This default is a fleet-wide policy in a per-store field. As the RetentionWindow doc two dozen lines up says, the value is an input to a shared minimum — so 10000 here pushes pruneHeight 10k blocks deeper for ReceiptDB, the state WAL and the SC snapshots too, not just for BlockDB. Two things worth reconsidering: (a) whether the default should be 0 and the extra depth expressed once as RollbackWindow in StorageGarbageCollectorConfig, and (b) that AutobahnBlockDBConfig.LittBlockConfig exposes Retention but not RetentionWindow, so once wired there is no way to tune this from tendermint config.

[seidroid]

[suggestion] The comment you added here states the hazard precisely — a rename makes NewBlockDB open a fresh empty table while the old data sits under <root>/ledger/ "neither served nor reclaimed" — and then the rename is performed anyway, relying on "BlockDB is not deployed on any network yet" from the PR description. That holds for mainnet, but any dev/CI/devnet home directory carrying a ledger/ table silently comes up empty rather than failing, which is the worst shape for the one class of environment where it can happen.

Since the check is cheap and the comment already argues for it: os.Stat(<root>/ledger) at open and refuse to start (or log loudly) if it exists. That turns a silent empty store into a one-line operator action, and can be deleted once no such directories remain.

[seidroid]

Well-constructed implementation of gc.PrunableStore across BlockDB/ReceiptDB/StateWAL/FlatKV, with the ExternalPruning single-source-of-truth pattern correctly enforced at the collector (the choke point) and thorough per-store tests. I found no blocking correctness bug; the notes below are default/behavior changes that aren't called out in the PR description, plus some diff noise and deferred-wiring risks.

Findings: 0 blocking | 11 non-blocking | 4 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Verification notes: I traced the concurrency claim added to seiwal.WAL.PruneBefore. Both real implementations (walImpl.PruneBefore → sendToWriter, serializingWAL.PruneBefore → submit) funnel through a channel with a shutdown-priority select, so the off-goroutine carve-out is genuinely honored. walsim.legacyWALShim.PruneBefore mutates a plain counter without a lock, but it satisfies walStore, not seiwal.WAL, so it is not bound by the new contract. Similarly confirmed CommitStore.GetLatestBlock's RLock matches Commit writing committedVersion under s.mu.Lock() (store_write.go:46,116), and that earliestVersion in the receipt store is monotonic in every writer, which is what the new gcFilter monotonicity requirement depends on.
• Stale comment: litt_block_db.go:397 still reads "gcFilter marks a key in the shared ledger table" after the table rename to blocks. Not on a changed line, so easy to miss.
• Nothing in the tree constructs a StorageGarbageCollector over these stores (GigaStorageConfig.PruningConfig is still unwired), so every new ExternalPruning stand-down path ships dark — pruneSnapshots, tryTruncateWAL, and startPruning all keep running in every real deployment. That is explicitly the stated scope, but it means the four-store end-to-end cycle has no coverage beyond mocks. Worth a follow-up integration test at the wiring PR.
• AutobahnBlockDBConfig (sei-tendermint/config/autobahn.go) exposes Retention and GCPeriod but no override for the new RetentionWindow, so sei-tendermint-configured block DBs are pinned to the 10000 default. Harmless today (no collector), but the knob will be needed at wiring time.
• TestReceiptLocalPrunerAdvancesFloorWithoutCollector spends up to 6 real seconds waiting on a 1–2s jittered ticker (~3 ticks of headroom). Under -race + coverage on a loaded CI shard that is a plausible flake source; the comment acknowledges the wait is deliberate, but consider making the prune interval injectable so the wait can shrink.
• refuseLegacyTable only distinguishes exists / not-exists; a file named ledger in a root would produce the "pre-rename table" error even though it is not a table directory. Cosmetic, and the operator action (move it aside) is the same.
• Second-opinion passes: Codex reported no material issues. cursor-review.md is empty — that pass produced no output, so it contributed nothing to this synthesis.
• 4 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] This silently drops the TTL failsafe from 24h to 1h (the setup_test.go assertion is updated to match, so it is deliberate), but the PR description doesn't mention it among the BlockDB changes.

RetentionTime is documented right above as the guard that "even an over-eager watermark cannot delete data younger than" — shrinking it 24× shrinks exactly that safety margin, and it applies today to autobahn/devnet block DBs that have no collector at all. Please call it out in the description, or keep 24h until the collector actually owns the watermark.

[seidroid]

[suggestion] I confirmed the safety claim for seid: readReceiptStoreConfig (app/receipt_store_config.go:27) overwrites KeepRecent unconditionally from min-retain-blocks, and KeepRecent is mapstructure:"-" so the app.toml template is unaffected. So no node changes behavior here.

The one caller worth flagging is DefaultGigaStorageConfig (sei-db/config/giga_config.go:58), which now hands a receipt store KeepRecent = 10000 instead of "keep everything". It has no production caller today, but it is the Giga wiring path this PR is building toward — a node constructed from it would serve eth_getTransactionReceipt for only ~10k blocks, and nothing in that path re-derives KeepRecent from min-retain-blocks. Consider setting it explicitly in DefaultGigaStorageConfig (or pinning it in giga_config_test.go) so the wiring PR can't inherit this default by accident.

[seidroid]

[suggestion] The doc is candid that there is no guard here and that the collector-exists check "belongs wherever the collector is eventually constructed" — agreed, but note this is the one place in the PR where the invariant is documentation only, and it is the highest-stakes one: this single flag stands down both pruneSnapshots and tryTruncateWAL, and the failure mode (nothing bounds snapshots or the state WAL) is silent and unbounded.

Per AGENTS.md's "guard at the choke point," the cleanest shape at wiring time is for NewStorageGarbageCollector to be the only thing that can turn this on — e.g. the collector constructor sets ExternalPruning on the configs it takes ownership of, rather than accepting configs that already claim it. Worth recording as the intended follow-up so the next author doesn't add a second setting that can disagree.

[seidroid]

[nit] Relocating InitializeDataDirectories / applyPebbleMetricsConfig from the top of the file to the bottom (and moving this interface assertion up in their place) is ~80 lines of pure churn unrelated to the GC work, and it makes this file's diff read as if it changed substantively when it didn't. Behavior is identical, so this is fine to keep — but a separate commit, or leaving it out, would make the PR easier to review.

Superseded: latest AI review found no blocking issues.

Superseded: latest AI review found no blocking issues.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 7ccdf29. Configure here.}

[seidroid]

Solid, unusually well-documented change: all four Giga stores implement gc.PrunableStore, the ExternalPruning flag genuinely makes "collector prunes me" and "my own pruner is off" a single fact, and the new GCFilter correctly makes the retention floor a precondition for litt reclamation. No blocking correctness or security issues found; the notes below are a coverage gap, a forward-looking guard gap, and some default/doc drift.

Findings: 0 blocking | 7 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Cursor's second-opinion pass (cursor-review.md) is empty — that review produced no output. Codex (codex-review.md) reported no material issues, which matches my own read.
• sei-tendermint/config/autobahn.go:51 still documents Absent ⇒ littblock.DefaultConfig Retention; the field it resolves to is now RetentionTime. Outside the diff hunk, so noting it here.
• Wiring-PR hazard worth recording now: the collector holds a specific store instance, but CommitStore.reopenWAL() (sei-db/state_db/sc/flatkv/store.go:852) replaces s.wal with a freshly opened statewal.StateWAL on the state-sync/import path. A collector registered against the old stateWALImpl would then be pruning a closed WAL (returning errors every cycle) while the live one grows unbounded. The PrunableStore doc on stateWALImpl states the SC/SS co-management precondition but not this instance-identity one.
• Verified the claims I could check independently: readReceiptStoreConfig (app/receipt_store_config.go:27) does overwrite KeepRecent unconditionally, and DefaultGigaStorageConfig has no non-test callers — so the DefaultReceiptKeepRecent 0 → 10000 change really is confined to tools/tests as the description says. The tx-hash read path (GetReceiptFromStore) does enforce belowRetentionFloor, so shortening the litt TTL to a flat hour changes only physical reclamation, not visible retention.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] This guard closes the pebbledb hole, but the littidx path can still reach the same silent-unbounded-growth state the guard exists to prevent: with ExternalPruning = true and KeepRecent == 0, runsLocalPruner() is false and GetRetentionWindow() folds to InfiniteRetentionWindow, so getCutLine returns 0, the store is never asked for a boundary, and never receives PruneBelow. Nothing prunes it.

That isn't an exotic combination — KeepRecent comes from min-retain-blocks, which defaults to 0, so it's the shape a wiring PR gets unless it remembers to set both. Not a live bug (nothing sets ExternalPruning yet), but it's exactly the combination the design section says should be unrepresentable rather than merely discouraged, and this switch is the choke point every receipt store passes through.

Suggest either rejecting ExternalPruning && KeepRecent <= 0 here alongside the pebble case, or — if "external pruning + keep everything" is meant to be legal — logging it once at open so an operator can tell it apart from a store whose floor has simply not moved yet.

[seidroid]

[suggestion] TestGCExternalPruningStandsDownSnapshotPruner pins the pruneSnapshots half of this flag, but nothing pins this half. Per the ExternalPruning doc in config/config.go, this is the higher-stakes of the two — dropping the || s.config.ExternalPruning here (or reordering the nil check) leaves the state WAL with both truncators off if the collector isn't registered, and the failure is silent and expensive.

A table-driven test in the shape of the snapshot-pruner one would do it: two CommitStores differing only in ExternalPruning, a WAL with a known stored range, tryTruncateWAL(), then assert the range moved in one case and not the other. snapshot_test.go:1510 already exercises tryTruncateWAL directly, so the harness exists.

[seidroid]

[nit] Worth calling out that this is a live default change, not just a rename: AutobahnBlockDBConfig.Retention is optional, so any autobahn node without an explicit retention in its config file goes from a 24h TTL failsafe to 1h. Benign as far as I can tell (the watermark is the visible-retention gate and reads below it are already refused, so this only makes reclamation of already-released data 24× more prompt), but the PR description presents the TTL rework under the ReceiptDB heading and this one reads as incidental to the rename.

Separately: RetentionWindow: 10000 on the next line has no path from AutobahnBlockDBConfig — the only two overrides are retention and gc_period — so whenever BlockDB is registered with a collector, autobahn nodes will be stuck on the hardcoded window until that config grows a key for it.

[seidroid]

A well-documented, test-heavy implementation of gc.PrunableStore across BlockDB, ReceiptDB, StateWAL and FlatKV; the collector integration, the ExternalPruning gate, and the new receipt GCFilter all hold up under inspection, and both new config fields default to false so there is no behavior change on any existing path. No blocking defects found — the notes below are a config trap the follow-up wiring PR will hit, a doc/implementation mismatch on the new PruneBefore concurrency contract, and one test-coverage gap.

Findings: 0 blocking | 11 non-blocking | 5 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Cursor's second-opinion file (cursor-review.md) is empty — that pass produced no output, so this review merges only Claude's and Codex's findings.
• Test gap: nothing pins that the real receipt store attaches the GC filter. litt_receipt_gcfilter_internal_test.go builds its own litt table (and says so), and the store-level tests only assert the read-time floor (ErrNotFound), which is independent of GCFilter. Deleting tableConfig.GCFilter = s.gcFilter in newLittReceiptStore leaves the whole suite green while restoring exactly the TTL-leads-the-floor bug this PR exists to close. Consider a store-level test with tiny TargetSegmentFileSize/GCPeriod that drives a real reclamation through NewReceiptStore.
• Wiring-PR risk worth restating loudly (the PR body acknowledges it): FlatKVConfig.ExternalPruning stands down two mechanisms — pruneSnapshots and tryTruncateWAL — and nothing in flatkv can verify the state WAL was actually registered with a collector. Enabling that one flag without registering statewal leaves the state WAL with nothing bounding it at all. The receipt path got a startup refusal (newReceiptBackend rejecting pebbledb + ExternalPruning); the FlatKV path has none, so this is the single most expensive mistake available at wiring time.
• sei-db/state_db/sc/flatkv/store.go: moving InitializeDataDirectories/applyPebbleMetricsConfig to the bottom of the file and var _ Store = (*CommitStore)(nil) to the top is pure code motion unrelated to this PR's purpose. It is behavior-free (so it satisfies AGENTS.md's refactor rule), but it adds ~75 lines of unrelated diff noise to an already 2,257-line change.
• littReceiptStore.SetEarliestVersion (exported on the ReceiptStore interface) now has a materially stronger effect than before: advancing the floor releases receipt bodies to litt's GC for permanent reclamation, where previously it only masked reads. There is no production caller today, but a future one that advances the floor optimistically (e.g. a state-sync restore) would permanently delete bodies rather than temporarily hide them. Worth a line in that method's doc.
• Naming consistency nit: LittBlockConfig → BlockDBConfig and Retention → RetentionTime were renamed, but AutobahnBlockDBConfig.LittBlockConfig(dir) and its Retention field in sei-tendermint/config/autobahn.go keep the old names, so the tendermint-facing knob no longer matches the field it sets.
• 5 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] ExternalPruning = true with the default KeepRecent = 0 yields no pruner at all — the exact silent, unbounded failure ExternalPruning is documented to make unrepresentable.

Trace: keepRecent <= 0 returns InfiniteRetentionWindow, and per gc/api.go that forces cutLine == 0, so the collector never asks this store for a boundary and never calls PruneBelow. Meanwhile runsLocalPruner() is already false (it requires keepRecent > 0). Both drivers are off and the tag index grows without bound.

The PR body defers reconciling KeepRecent's and RetentionWindow's disagreement about 0 to the wiring PR, which is reasonable — but the trap is cheap to close here, mirroring the guard that already exists one file over: reject ExternalPruning && KeepRecent <= 0 in newLittReceiptStore, the same way newReceiptBackend rejects pebbledb + ExternalPruning. That keeps the guard at the choke point every construction passes through instead of leaving it as a fact the wiring PR has to remember. No production impact today (the field is mapstructure:"-" and defaults false).

[seidroid]

[suggestion] The new contract says PruneBefore may be called "concurrently with any method including Close", but a prune racing a close is silently dropped while returning nil: submit can win its senderCtx check and enqueue serPrune behind serClose, and serializerLoop returns on serClose without draining the channel (seiwal_serializing.go:343-348). walImpl has the same shape.

Codex flagged this as High; I'd rate it lower — a dropped prune only defers reclamation (the WAL stays consistent, and the next cycle or the next process re-issues it), and the doc two lines below already says which records a prune reclaims is approximate and arbitrarily delayable. Reply channels for serFlush/serBounds/serIterator are unblocked by Close's s.cancel(nil), so nothing hangs.

Still worth one clause here: say that a prune racing Close may be dropped and that nil therefore does not promise the prune was scheduled. Otherwise the contract reads stronger than the two implementations deliver, and a future implementer will take it literally. TestGCPruneBelowBeforeClose only covers the ordered (non-racing) case.

[seidroid]

[nit] Codex flags this branch as violating the rollback guarantee (WAL pruned below a height this store cannot restore to). I checked and disagree that it's a correctness bug: this case is reachable only when the oldest snapshot is above cutLine, so there is nothing at or below cutLine for a lower answer to protect, and the WAL blocks below cutLine are unreachable anyway — replay is forward-only from a snapshot that doesn't exist. Returning CannotServeRollback here would stall every store's pruning indefinitely whenever snapshot retention is shallower than RollbackWindow, which is the worse failure. The doc comment above already reasons this through correctly.

What is missing is a signal. The store silently cannot serve the configured rollback window, and today the only place that surfaces is a failed rollback. A rate-limited logger.Warn naming cutLine and the oldest snapshot would turn a snapshot-retention shortfall into something operators can see before they need it — the CannotServeRollback path right below already logs, and this path is arguably the one more likely to persist unnoticed.

[seidroid]

[suggestion] This one line is the whole reason the retention floor gates reclamation, and no test covers it — the internal filter test builds its own litt table and explicitly notes it "cannot prove that the constructor attaches the filter". Removing this line leaves the entire suite green while making the TTL the sole reclaimer again, which is the bug the PR exists to fix. Worth a store-level test that drives a real reclamation through NewReceiptStore with tiny segment sizing.

[seidroid]

[nit] The BlockDB age failsafe drops 24h → 1h here (the receipt side goes the other way, from KeepRecent × 2s up to a flat 1h). The PR body states the new default, so this is disclosed — just confirming intent, since this value is the backstop against a watermark bug and it shrinks 24×. Autobahn devnets pick it up via DefaultConfig; AutobahnBlockDBConfig.Retention still overrides it, and the assertion in sei-tendermint/node/setup_test.go was updated to match.