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

Medium Risk
Touches rollback/snapshot/WAL/receipt retention semantics and concurrent pruning paths; mis-wiring ExternalPruning or collector registration could cause silent unbounded growth or over-pruning, though defaults preserve current behavior until the collector is enabled.

Overview
Implements gc.PrunableStore on BlockDB, littidx ReceiptDB, State WAL, and FlatKV so a single StorageGarbageCollector can coordinate retention across Giga storage instead of each store pruning in isolation.

The collector still takes the minimum head and pruning boundaries each cycle, but PruneBelow is only invoked when ExternalPruning() is true. Self-pruning stores (e.g. FlatKV with ExternalPruning off, littidx without a collector) keep voting on boundaries while their local pruners run; ExternalPruning ties “collector owns pruning” to standing down internal pruners so two retention drivers cannot race.

BlockDB: LittBlockConfig → BlockDBConfig (Retention → RetentionTime, new RetentionWindow), Litt table ledger → blocks with startup guard for legacy ledger dirs, new litt_block_gc.go.

ReceiptDB (littidx): GC adapter, ExternalPruning / runsLocalPruner (local pruner off when collector manages), gcFilter so body reclamation follows the block floor (flat littRetentionTime TTL instead of KeepRecent×2s), pebble backend rejects ExternalPruning. Default KeepRecent is 10000 for direct config builders; nodes still override via min-retain-blocks.

FlatKV: store_gc.go (snapshot-aware GetPruningBoundary, PruneBelow on snapshots), FlatKVConfig.ExternalPruning disables pruneSnapshots and tryTruncateWAL.

State WAL: GC surface with atomic lastCompletedBlock; seiwal.WAL.PruneBefore documented as safe from the collector goroutine.

Config/golden/test updates only; wiring the collector in production is explicitly out of scope (defaults leave ExternalPruning false).

^{Reviewed by Cursor Bugbot for commit 7ccdf29. 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 85.64815% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.01%. Comparing base (a115971) to head (7ccdf29).

Files with missing lines	Patch %	Lines
sei-db/state_db/sc/flatkv/store_gc.go	86.44%	6 Missing and 2 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   61.61%   61.01%   -0.60%     
==========================================
  Files        2348     2279      -69     
  Lines      200852   193282    -7570     
==========================================
- Hits       123755   117932    -5823     
+ Misses      66044    64852    -1192     
+ Partials    11053    10498     -555     

Flag	Coverage Δ
sei-chain-pr	60.51% <84.21%> (?)
sei-db	70.41% <ø> (ø)
sei-db-state-db	?
sei-db-state-db-pr	76.30% <87.25%> (?)

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% <100.00%> (ø)
sei-db/ledger_db/block/blocksim/blocksim.go	9.02% <100.00%> (ø)
sei-db/ledger_db/receipt/receipt_store.go	71.00% <100.00%> (+4.33%)	⬆️
sei-db/management/gc/storage_garbage_collector.go	100.00% <100.00%> (+1.78%)	⬆️
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%)	⬆️
sei-db/state_db/sc/flatkv/snapshot.go	67.80% <66.66%> (+0.43%)	⬆️
... and 8 more

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

[claude]

🟡 littReceiptStore's litt-table TTL (litt_receipt_store.go, set as cfg.KeepRecent * littTTLPerBlock) is derived from KeepRecent alone and ignores RollbackWindow, while under ExternalPruning the collector's visible retention floor is RollbackWindow + GetRetentionWindow() (litt_receipt_gc.go:52-57 returns keepRecent verbatim). Once a StorageGarbageCollector is wired in with a non-trivial RollbackWindow and a small KeepRecent, litt's TTL-driven segment GC can physically reclaim receipt bytes for blocks still at or above earliestVersion, causing GetReceiptFromStore to silently return not-found for receipts the collector still considers retained. Fix by deriving the TTL from RollbackWindow + KeepRecent (or an independent wall-clock failsafe, as blockDB does) rather than KeepRecent alone.

Extended reasoning...

The bug. newLittReceiptStore (litt_receipt_store.go) sets the litt table's body TTL once at construction:

if cfg.KeepRecent > 0 {
    if err := receipts.SetTTL(time.Duration(cfg.KeepRecent) * littTTLPerBlock); err != nil {

littTTLPerBlock is a fixed 2s/block constant. This derivation depends only on KeepRecent and is applied unconditionally regardless of cfg.ExternalPruning.

Meanwhile, this PR's littReceiptStore.GetRetentionWindow (litt_receipt_gc.go:52-57) returns keepRecent verbatim as the extra retention the StorageGarbageCollector should keep beyond the shared RollbackWindow. Combined with GetPruningBoundary's unconditional cutLine passthrough, the collector's logical retention floor once wired in is head - RollbackWindow - keepRecent (via pruneBlocksBelow advancing earliestVersion to that cut line). So the store's contract is to keep RollbackWindow + keepRecent blocks servable, but its physical reclaim mechanism (the litt TTL) only retains bodies for roughly keepRecent blocks worth of wall-clock time.

Why nothing today prevents the gap. litt's TTL-based segment GC reclaims whole segments once their age exceeds the TTL, independent of the tag-index earliestVersion floor. The existing code comment ("visible retention never exceeds that floor regardless of GC timing") only holds when the TTL window is deeper than the logical retention floor — true before this PR, when the only retention driver was KeepRecent itself. This PR adds a second, deeper retention driver (RollbackWindow) to the logical floor without adding a matching term to the TTL, breaking that invariant.

Concretely: at Giga's ~7ms block time, a KeepRecent*2s TTL covers roughly keepRecent * 285 blocks. With the default RollbackWindow = 1000 and, say, KeepRecent = 3 under ExternalPruning, the logical floor sits 1000 + 3 = 1003 blocks behind head, but the physical TTL only protects the most recent 3 * 285 ≈ 855 blocks. Blocks between roughly 855 and 1003 blocks behind head are still at/above earliestVersion (the collector still considers them retained and servable) but their bodies may already have been physically reclaimed by litt's TTL GC.

Step-by-step proof of the read failure:

1. A StorageGarbageCollector with RollbackWindow = 1000 manages a littReceiptStore configured with KeepRecent = 3 and ExternalPruning = true.
2. GetRetentionWindow() returns 3; each prune cycle advances earliestVersion to head - 1000 - 3 = head - 1003.
3. litt's TTL GC, running independently, reclaims segments for bodies older than 3 * 2s = 6s of wall clock — roughly 855 blocks behind head at Giga speed.
4. A client calls GetReceiptFromStore for a tx in block head - 900 (still ≥ earliestVersion = head - 1003, so the collector's contract says it must be servable).
5. s.receipts.Get(txHash[:]) returns exists = false because litt's TTL GC already reclaimed that segment — this check runs before the belowRetentionFloor check even executes.
6. The method returns ErrNotFound for a receipt that, per the store's own advertised retention contract, should still be present.

Fix. Derive the litt TTL from RollbackWindow + KeepRecent under ExternalPruning (or, more robustly, use an independent wall-clock failsafe decoupled from the block-count window, the way blockDB keeps Retention and RetentionWindow as fully separate knobs).

Severity note. All three independent verifiers rated this nit, and I agree: per the PR description, no code constructs a StorageGarbageCollector yet, so ExternalPruning is never actually driven by a live collector on any current path — the gap is real but currently unreachable. It also affects only auxiliary, non-consensus RPC receipt data, which already tolerates a missing body as a not-found response. It's worth fixing while this code is fresh, but it doesn't block merge.

[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] A non-zero default here is a fleet-wide retention decision, not a blockDB-local one. Because the collector prunes every store to the shared minimum (storage_garbage_collector.go calls this out explicitly: "receiptDB retention 100_000 also keeps SC/SS snapshots that far back"), defaulting RetentionWindow to 10 000 means that once wiring lands, FlatKV's snapshots, ReceiptDB and the state WAL all retain 10 000 blocks beyond RollbackWindow — purely because blockDB's default says so. Every other store added in this PR reports 0.

Suggest defaulting to 0 (matching the "no extra beyond the shared window" contract) and making 10 000 an explicit opt-in at the call site, or documenting here why 10 000 is the right floor for the whole fleet.

Related: AutobahnBlockDBConfig in sei-tendermint/config/autobahn.go grew no override for RetentionWindow even though it has one for Retention, so a tendermint-side operator gets this default with no way to tune it.

[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.

[seidroid]

[nit] pruned is initialized and incremented but never read — each deletion already logs individually at line 91. Either drop it, or use it for a single summary log line at the end (which would also be cheaper than one Info per snapshot when a deep backlog is cleared).

[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.

[claude]

🟡 ReceiptStoreConfig.ExternalPruning is tagged mapstructure:"external-pruning", making it addressable from app.toml, but its own doc comment says it is "not read from the receipt-store config" (like KeepRecent) and the accompanying test explicitly asserts it's tagged mapstructure:"-". Please tag it mapstructure:"-" to match the documented contract and the sibling KeepRecent field.

Extended reasoning...

ReceiptStoreConfig.ExternalPruning (sei-db/config/receipt_config.go:74) is declared with mapstructure:"external-pruning", a real, addressable config key. This directly contradicts three independent statements of the intended contract that this same PR introduces:

1. The field's own doc comment: "Like KeepRecent this is not read from the receipt-store config. It is set by whatever constructs the collector..."
2. Its sibling field KeepRecent, which the comment explicitly compares itself to, and which is correctly tagged mapstructure:"-".
3. TestManifestNamesEveryField in receipt_config_fuzz_test.go, which excludes ExternalPruning from key-manifest coverage with the reasoning "ExternalPruning is tagged mapstructure:"-"" — a factually false statement about the code as written.

All three signals point to the same intended tag (mapstructure:"-"), so this reads as a copy/paste or oversight rather than a deliberate choice.

Why the test doesn't catch it: CheckManifestCoversEveryField (testutil/configtest/golden.go) keys off the Go field name via spec.Path, not the actual mapstructure tag value. So the test's exclusion of ExternalPruning passes regardless of what the tag says — the prose justifying the exclusion is unchecked and has silently drifted from the code.

Current impact — none, but the guard is silently degraded. ReadReceiptConfig populates ReceiptStoreConfig field-by-field via explicit opts.Get(flag) calls, and there is no flagRSExternalPruning constant and no generic mapstructure/viper Unmarshal of this struct anywhere in the tree. So a hand-written external-pruning = true under [receipt-store] in app.toml is not read today, and no live exploit exists via the current app.toml loading path.

Why it's still worth fixing. The entire point of using mapstructure:"-" here (per the field's own comment) is defense-in-depth against any future or alternative config-binding mechanism (e.g. a generic mapstructure decode, a config migration tool, or a refactor of ReadReceiptConfig to a reflective bind) accidentally mapping an operator-supplied key onto a field that must only be set by whatever constructs the StorageGarbageCollector. Enabling ExternalPruning without a running collector attached would silently stand down the store's own KeepRecent pruner with nothing to replace it — unbounded growth with no error and no log, which is precisely the failure mode the surrounding comment (and the analogous FlatKV ExternalPruning field raised separately by seidroid[bot]) warns about. Leaving the tag as external-pruning removes that safety net for no benefit, since nothing currently relies on the key being addressable.

Step-by-step proof of the mismatch:

1. Read the field declaration: ExternalPruning bool mapstructure:"external-pruning"`` — this is a real, non-"-" tag.
2. Read the doc comment directly above it: "Like KeepRecent this is not read from the receipt-store config."
3. Read KeepRecent's tag a few lines up: mapstructure:"-" — confirming what "like KeepRecent" should mean for the tag.
4. Read the test comment in receipt_config_fuzz_test.go: "ExternalPruning is tagged mapstructure:"-"" — this statement is false for the code as written.
5. Conclusion: the field's tag does not match its own documentation, its sibling's tag, or the test's stated justification. All three independently point to mapstructure:"-" being correct.

Fix: change the tag to mapstructure:"-", matching KeepRecent and restoring the documented contract. This is a one-token change with no other code path depending on the current tag value.

[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 9, 2026, 10:31 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.}

[cursor]

Self-pruning FlatKV strands older snapshots

Medium Severity

With ExternalPruning left false (the default), FlatKV still votes via GetPruningBoundary, which returns only the newest snapshot at or below cutLine. StateWAL always accepts collector prunes, so the WAL can be cut to that newer snapshot while SnapshotKeepRecent still keeps older ones. Those older snapshots stay on disk but can no longer replay, so rollback to them fails. The hybrid path this PR documents as safe therefore breaks SnapshotKeepRecent depth as soon as a collector is wired without flipping the flag.

Additional Locations (1)

• sei-db/state_db/statewal/state_wal_gc.go#L39-L42

 

^{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.