refactor(storage): add Store::current_slot

[MegaRedHand]

🗒️ Description / Motivation

Extracted from #561. Stacked on #569, which is what makes it possible: with the interval grid in ethlambda-types, ethlambda-storage can own the accessor.

Deriving the slot from the store clock was open-coded at three call sites in the blockchain crate, each repeating store.time() / INTERVALS_PER_SLOT along with its own expect message.

Note

Targets refactor/interval-constants-in-types. Merge #569 first and this rebases onto main on its own.

What Changed

• crates/storage/src/store.rs — new Store::current_slot(); the Store::time() doc comment now points at it for the slot case rather than spelling out the division.
• crates/blockchain/src/store.rs — two call sites (on_tick, on_block_core).
• crates/blockchain/src/lib.rs — one call site (the NewAttestation handler).

Correctness / Behavior Guarantees

Identical behavior. All three sites already panicked on a failed time() read (unwrap / expect), so current_slot()'s expect("store time exists") preserves the failure mode; only the panic message differs at two of them.

Uses of INTERVALS_PER_SLOT that are not "derive the current slot from the clock" are deliberately untouched: the % INTERVALS_PER_SLOT interval derivation, data.slot.saturating_mul(INTERVALS_PER_SLOT), the tick skip-check, and the set_time(n * INTERVALS_PER_SLOT) calls in tests.

Tests Added / Run

No new tests — no behavior change to cover.

make fmt
make lint
make leanSpec/fixtures     # fixtures were absent in the worktree
cargo test --workspace --profile release-fast --no-fail-fast

Related Issues / PRs

• Stacked on refactor(types): move the interval grid into ethlambda-types #569
• Extracted from feat(consensus): two-tier heartbeat fork choice on a 4-interval slot #561

✅ Verification Checklist

• Ran make fmt — clean
• Ran make lint (clippy with -D warnings) — clean
• Ran make test (cargo test --workspace --profile release-fast) — all passing

[greptile-apps]

Greptile Summary

This PR centralizes conversion of the persisted store clock into the current slot without changing runtime behavior.

• Adds Store::current_slot() using the shared INTERVALS_PER_SLOT constant.
• Replaces three open-coded slot calculations in tick processing, block import, and attestation handling.
• Updates the Store::time() documentation to direct slot callers to the new accessor.

Confidence Score: 5/5

The PR appears safe to merge with no actionable behavioral defects identified.

The new accessor performs the same metadata read and integer division as all replaced expressions, and it is available through existing unconditional crate dependencies.

Important Files Changed

Filename	Overview
crates/storage/src/store.rs	Adds an unconditional public accessor that preserves the existing time-read, panic, and slot-division semantics.
crates/blockchain/src/store.rs	Replaces two equivalent current-slot calculations in tick and block-import paths; only diagnostic panic text changes.
crates/blockchain/src/lib.rs	Replaces the attestation handler’s calculation with the exactly equivalent storage accessor.

_{Reviews (1): Last reviewed commit: "refactor(storage): add Store::current_sl..." | Re-trigger Greptile}

[github-actions]

🤖 Kimi Code Review

This is a clean refactoring that properly centralizes slot calculation logic. The changes are correct and improve maintainability.

Approval with minor observations:

1. Code Quality (crates/storage/src/store.rs:832) — Good encapsulation of the time / INTERVALS_PER_SLOT calculation. The documentation update correctly guides callers to use the new helper.

2. Performance Note (crates/blockchain/src/store.rs:348-349) — Not introduced by this PR, but on_tick now performs three separate storage reads for time (one in set_time, one in current_slot, one for interval). Consider caching store.time() in a local variable to reduce redundant DB/metadata lookups:

   let new_time = store.time().unwrap() + 1;
   store.set_time(new_time).expect("set_time should succeed");
   let slot = new_time / INTERVALS_PER_SLOT; // or keep using current_slot() if preferred

   This is minor since metadata reads are typically cached in memory, but worth noting if this path is hot.

3. Panic Consistency — The change from expect("DB read should succeed") to expect("store time exists") (in on_block_core) is actually an improvement; the new message better reflects the invariant being asserted.

4. Consensus Correctness — The slot horizon check (current_slot + 1) and attestation timing logic remain semantically identical. The INTERVALS_PER_SLOT division is the correct spec-derived calculation.

Verdict: LGTM. The refactoring reduces duplication without changing consensus-critical behavior.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Claude Code Review

This is a clean, correctly scoped refactor. Verified the behavior claim holds — time() internally calls get_metadata, which already panics (via .expect) on any read failure, so current_slot()'s own .expect("store time exists") is unreachable in practice; the three call sites' panic behavior is preserved as described.

Findings

Stale doc comment now duplicates the formula current_slot was added to centralize (crates/blockchain/src/store.rs:328-331)

The doc comment directly above on_tick still spells out slot = store.time() / INTERVALS_PER_SLOT, even though the function body right below it now calls store.current_slot() (line 351). Since the PR's stated purpose is eliminating the open-coded division at call sites, this comment is now the last place restating the formula outside Store::current_slot/Store::time's own doc. Worth updating to say "see Store::current_slot" instead, so there's a single source of truth for the derivation.

Everything else is solid:

• Store::current_slot() (crates/storage/src/store.rs:832-835) is a straightforward, correctly-placed accessor — good spot next to time()/set_time().
• The time() doc comment update at crates/storage/src/store.rs:819-822 appropriately delegates to the new method rather than re-deriving the formula.
• All three call-site replacements (lib.rs:1416, store.rs:351, store.rs:640) are behavior-preserving; the two expect/unwrap panic-message differences are cosmetic and called out honestly in the PR description.
• Import of INTERVALS_PER_SLOT in crates/storage/src/store.rs is now actually used, and the pre-existing uses of INTERVALS_PER_SLOT for interval derivation, tick skip-check, and test set_time calls were correctly left untouched — the PR didn't overreach into unrelated INTERVALS_PER_SLOT usages.
• No SSZ, fork-choice, or attestation-validation logic touched; scope is exactly what's advertised.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt