Skip to content

refactor(blockchain): reference INTERVALS_PER_SLOT in the tick panic message - #572

Merged
MegaRedHand merged 3 commits into
mainfrom
refactor/interval-count-in-panic-message
Aug 7, 2026
Merged

refactor(blockchain): reference INTERVALS_PER_SLOT in the tick panic message#572
MegaRedHand merged 3 commits into
mainfrom
refactor/interval-count-in-panic-message

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

🗒️ Description / Motivation

Extracted from #561, where the interval count changes and this message would otherwise have gone stale.

SlotInterval::from_intervals_since_genesis hardcoded the interval count in its unreachable! string, so changing the grid would leave a wrong number in a panic message — the one place you least want to be misled.

What Changed

  • crates/blockchain/src/lib.rs — the panic message captures INTERVALS_PER_SLOT instead of spelling out the number.

Correctness / Behavior Guarantees

No behavior change; the arm is unreachable by construction (% INTERVALS_PER_SLOT). Inline format-arg capture of a const inside unreachable! was confirmed to compile, not assumed.

Two doc comments in the same file still spell out 800ms / 5 intervals in prose. Left alone as out of scope for this PR.

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

✅ 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

…message

The message hardcoded the interval count, so changing the grid would leave a
stale number in a panic string.
@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR keeps SlotInterval’s unreachable panic message synchronized with the configured interval count.

  • Replaces the hardcoded interval count with INTERVALS_PER_SLOT.
  • Does not alter interval matching or runtime control flow.

Confidence Score: 5/5

The PR appears safe to merge with no actionable issues identified.

The change only updates diagnostic text to capture an in-scope integer constant using formatting supported by the repository’s pinned Rust toolchain, without changing control flow.

Important Files Changed

Filename Overview
crates/blockchain/src/lib.rs Safely replaces a hardcoded panic-message value with the existing in-scope interval constant.

Reviews (1): Last reviewed commit: "refactor(blockchain): reference INTERVAL..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

crates/blockchain/src/lib.rs:109

This is a positive change that improves maintainability. Using the INTERVALS_PER_SLOT constant in the panic message prevents the error string from becoming stale if the interval count ever changes.

Verification needed:
Ensure INTERVALS_PER_SLOT is defined in scope (presumably with value 5 given the match arms 0-4) and implements Display. If this constant is imported from another module, verify the import is present.

No security or correctness issues introduced. The unreachable! branch remains appropriate for this exhaustive match guard.


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

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

The change is correct and safe — no findings. It's a minimal, well-justified fix that keeps the panic message in sync with INTERVALS_PER_SLOT, with no behavioral change since that match arm is unreachable by construction.


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

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

No findings.

The only change at crates/blockchain/src/lib.rs:109 replaces a hardcoded panic string with INTERVALS_PER_SLOT, which is defined in scope at crates/blockchain/src/lib.rs:74. That does not affect fork choice, attestation handling, justification/finalization, state transition, XMSS verification, SSZ behavior, or performance; it is just a maintainability improvement that keeps the diagnostic aligned with the constant.

Residual gap: I could not run cargo check in this environment because rustup failed to create temp files under /home/runner/.rustup (read-only filesystem), so this review is based on source inspection rather than a local build.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@MegaRedHand
MegaRedHand merged commit 4209de9 into main Aug 7, 2026
2 checks passed
@MegaRedHand
MegaRedHand deleted the refactor/interval-count-in-panic-message branch August 7, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants