Increase STATES_TO_KEEP from 900 to 3000

[pablodeymo]

Motivation

Increase state retention window from ~1 hour (900 slots) to ~3.3 hours (3000 slots) at 4-second slot times. This reduces the gap between state retention and block retention (BLOCKS_TO_KEEP=21600), making state-header mismatches less likely during finalization stalls.

Changes

Single constant change in crates/storage/src/store.rs:

// Before
const STATES_TO_KEEP: usize = 900;

// After
const STATES_TO_KEEP: usize = 3_000;

How to Test

make test — all tests pass.

Closes #239

[github-actions]

🤖 Kimi Code Review

Review Summary

The PR increases STATES_TO_KEEP from 900 to 3,000 states (extending state history retention from ~1 hour to ~3.3 hours at 4-second slots). This appears to be a simple configuration change, but there are several considerations:

Issues Found

1. Memory Usage Impact (line 86-87)

   • Increasing state retention by 3.33x will significantly increase memory usage. Each beacon state is large (hundreds of MB with full validator sets). This could cause OOM issues on resource-constrained nodes.
   • Suggestion: Add a runtime configuration option or compile-time feature flag to allow operators to tune this based on their hardware constraints.

2. Missing Documentation Update (line 85)

   • The comment on line 85 still references the old value (900) in the calculation: (12000 / 4 = 3000) should be (12_000 / 4 = 3_000) to match the new constant and Rust numeric literal conventions.
   • Fix: Update the comment to use consistent numeric formatting.

3. Potential Sync Issues (related to the change)

   • With 3x more states retained, initial sync times may increase for new nodes joining the network, as they'll need to process more historical states during backfill.
   • Consideration: Document this trade-off in release notes.

4. State Pruning Logic (not shown in diff)

   • Ensure the state pruning logic correctly handles the transition when this constant changes. Nodes upgrading mid-sync might have edge cases where they temporarily retain more states than intended.

The change itself is straightforward and the assertion on lines 88-91 correctly ensures BLOCKS_TO_KEEP >= STATES_TO_KEEP remains valid.

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

🤖 Codex Code Review

Findings (ordered by severity):

1. Low - stale test comment after retention change
   In store.rs:1489, the comment still says “905 headers total / top 900 kept,” which is now outdated with STATES_TO_KEEP = 3000.
   Suggestion: rewrite the comment to reference STATES_TO_KEEP symbolically (as done in assertions) to avoid drift.

2. Info - increased resource footprint should be explicitly justified
   The change in store.rs:86 and store.rs:87 raises retained states from 900 to 3000 (3.33x). Pruning remains bounded and logic appears correct, but this increases worst-case DB size and prune-scan work.
   Suggestion: add a brief rationale in PR description or code comment about expected storage/IO impact and why 3000 is the right bound.

Overall assessment:

• No consensus-critical logic changes (fork choice, attestation validation, justification/finalization, state transition, XMSS, SSZ) are touched by this PR.
• No new correctness or security bug is evident in the changed code path.

Validation note:

• I could not run tests in this environment because the Rust toolchain download is blocked (no network), so this review is static-analysis only.

---

Automated review by OpenAI Codex · custom prompt

[greptile-apps]

Greptile Summary

This PR increases STATES_TO_KEEP from 900 to 3_000 in crates/storage/src/store.rs, extending the in-memory/on-disk state retention window from ~1 hour to ~3.3 hours at 4-second slot times. The goal is to reduce state-header mismatches during finalization stalls by narrowing the gap between state retention (~3.3 h) and block retention (~6 h / BLOCKS_TO_KEEP=21600).

Key observations:

• The compile-time assertion BLOCKS_TO_KEEP >= STATES_TO_KEEP (21 600 ≥ 3 000) still holds — no regression there.
• All pruning logic and test assertions use the STATES_TO_KEEP constant directly, so they remain correct automatically.
• The doc-comment on the constant was updated to reflect the new math (12000 / 4 = 3000, ~3.3 h) — values check out.
• One explanatory test comment at line 1489 still hard-codes the old numbers (905, 900, 902) and should be updated to match the new constant (3005, 3000, 3002).
• The change will increase peak state storage by ~3.3×; this is intentional and accepted as a trade-off for better resilience during finalization stalls.

Confidence Score: 4/5

• Safe to merge — the change is a single constant bump with a correct compile-time guard; only a stale test comment needs updating.
• The logic is straightforward: one constant is changed, the compile-time assertion confirms the invariant, and all tests use the constant symbolically rather than by hard-coded value. The only issue is a cosmetic stale comment in a test, which does not affect correctness or runtime behaviour.
• The stale numbers in the fallback_pruning_removes_old_states_and_blocks test comment (lines 1489–1491 of crates/storage/src/store.rs) should be updated, but this is non-blocking.

Important Files Changed

Filename	Overview
crates/storage/src/store.rs	Increases STATES_TO_KEEP from 900 to 3_000 and updates the doc-comment; the compile-time assertion BLOCKS_TO_KEEP >= STATES_TO_KEEP still holds. One test comment (lines 1489–1491) retains the old hardcoded numbers and should be updated to match the new constant.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[New block/state arrives] --> B{States count > STATES_TO_KEEP\n3_000}
    B -- No --> C[Keep all states — no pruning]
    B -- Yes --> D[Sort states by slot descending]
    D --> E[Skip top 3_000 newest states]
    E --> F{Is state root in\nprotected_roots?\nfinalized / justified}
    F -- Yes --> G[Preserve state]
    F -- No --> H[Delete state from storage]
    G --> I[Pruning complete]
    H --> I

Loading

Comments Outside Diff (1)

1. crates/storage/src/store.rs, line 1489-1491 (link)

   Stale hardcoded numbers in test comment

   After increasing STATES_TO_KEEP from 900 to 3_000, this comment still references the old concrete values (905, 900, 902). The assertion below correctly uses STATES_TO_KEEP + 2, but the explanatory comment is now misleading — it describes a scenario with 905 total entries and a 900-slot window, when the actual values are 3005 and 3000.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: crates/storage/src/store.rs
   Line: 1489-1491
   
   Comment:
   **Stale hardcoded numbers in test comment**
   
   After increasing `STATES_TO_KEEP` from `900` to `3_000`, this comment still references the old concrete values (`905`, `900`, `902`). The assertion below correctly uses `STATES_TO_KEEP + 2`, but the explanatory comment is now misleading — it describes a scenario with 905 total entries and a 900-slot window, when the actual values are 3005 and 3000.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: crates/storage/src/store.rs
Line: 1489-1491

Comment:
**Stale hardcoded numbers in test comment**

After increasing `STATES_TO_KEEP` from `900` to `3_000`, this comment still references the old concrete values (`905`, `900`, `902`). The assertion below correctly uses `STATES_TO_KEEP + 2`, but the explanatory comment is now misleading — it describes a scenario with 905 total entries and a 900-slot window, when the actual values are 3005 and 3000.

```suggestion
        // 3005 headers total. Top 3000 by slot are kept in the retention window,
        // leaving 5 candidates. 2 are protected (finalized + justified),
        // so 3 are pruned → 3005 - 3 = 3002 states remaining.
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 637b131}

[github-actions]

🤖 Claude Code Review

Here is the review:

---

PR Review: Increase STATES_TO_KEEP from 900 to 3000

Summary

Minimal single-constant change. The motivation is sound: reducing the mismatch between block retention (~1 day) and state retention (~1 hour) helps avoid state-header mismatches during finalization stalls.

Correctness

The compile-time invariant at store.rs:89–92 still holds (BLOCKS_TO_KEEP 21,600 ≥ STATES_TO_KEEP 3,000), and the pruning logic uses the constant correctly throughout.

Stale Test Comment

store.rs:1489–1491 has a hardcoded comment that now refers to the old value:

// 905 headers total. Top 900 by slot are kept in the retention window,
// leaving 5 candidates. 2 are protected (finalized + justified),
// so 3 are pruned → 905 - 3 = 902 states remaining.

The test logic is correct (it uses STATES_TO_KEEP symbolically), but the English explanation still says 900. It should read 3000 and 3005 respectively:

// 3005 headers total. Top 3000 by slot are kept in the retention window,
// leaving 5 candidates. 2 are protected (finalized + justified),
// so 3 are pruned → 3005 - 3 = 3002 states remaining.

Disk / Memory Impact

States include historical_block_hashes, justified_slots, justification_roots, and justification_validators, which grow with chain age. On a small testnet (9 validators, early chain) each serialized state is modest in size, so 3.33× more stored states (3,000 vs 900) is acceptable. On a long-running or larger-validator-set chain this triples disk usage for the States table; worth a note in a follow-up if the production validator set grows significantly.

Code Quality

• Comment math is correct: 12000 / 4 = 3000 slots, 12000s / 3600 ≈ 3.3h.
• Underscore separator on 3_000 is consistent with 21_600.

Verdict

Approve with the minor comment fix noted above.

---

Automated review by Claude (Anthropic) · custom prompt

[MegaRedHand]

LGTM. Left a comment about a typo in a comment