fix(stf): invalidate blocks with out-of-range justified-slot votes (leanSpec #1023)

[MegaRedHand]

Problem

justified_slots_ops::is_slot_justified read a slot past the tracked justification window as "not justified" via unwrap_or(false). After process_block_header that window covers up to block.slot - 1, so a vote whose source or target slot is at or beyond the importing block's own slot was silently skipped and the block accepted.

leanSpec raises JUSTIFIED_SLOT_OUT_OF_RANGE there (spec/forks/lstar/containers/state.py, leanSpec #1023) and the block is invalid. A crafted block would therefore split ethlambda from spec-strict clients. Honest ethlambda proposers never emit one, since the builder mirrors is_valid_vote.

Change

File	Change
state_transition/src/lib.rs	New Error::JustifiedSlotOutOfRange { slot, finalized_slot, tracked_length }; is_valid_vote → Result<bool, Error>; process_attestations propagates with ?
state_transition/src/justified_slots_ops.rs	is_slot_justified → Result<bool, Error>; slots.get(idx).unwrap_or(false) becomes .ok_or(Error::JustifiedSlotOutOfRange { .. })
blockchain/src/block_builder.rs	Both entry_passes_filters call sites explicitly .unwrap_or(false), preserving current behavior

Why the builder stays lenient

ProjectedState::from_head_state clones head_state.justified_slots, whose window stops at head.slot - 1, but the extended chain view it filters against covers up to candidate.slot - 1. A vote targeting the head block's own slot is therefore routinely untracked in the builder. Propagating the error would have filtered out the dominant attestation shape and killed block packing.

entry_passes_filters pre-filters gossip candidates rather than deciding validity, so "untracked ⇒ not justified yet" is the correct reading there; the state transition stays the authority.

Note on the existing spec fixture

test_source_slot_beyond_tracked_range_rejects_block (leanSpec #1100) already ships in the fixture bundle and already passed before this change, but vacuously. The filler zeroes stateRoot on an invalid test's final block, so ethlambda rejected on StateRootMismatch long before reaching the real check. The runner only asserts result.is_err(); it deserializes rejectionReason but never compares it.

Instrumenting the runner to print actual error vs expected reason confirms the fix now rejects for the right reason:

JUSTIFIED_SLOT_OUT_OF_RANGE => justified slot 2 is outside the tracked range
                               (finalized_boundary=0, tracked_length=1)

The same instrumentation showed two other fixtures still passing vacuously through StateRootMismatch rather than their own check: TOO_MANY_ATTESTATION_DATA and BLOCK_SLOT_MISMATCH. Not addressed here. Making the runner assert on rejectionReason would close the whole class and is worth a follow-up.

Testing

• make fmt, make lint (clippy -D warnings) clean.
• Full workspace suite green, 0 failures: 122 forkchoice, 74 STF, 3 signature spectests, plus unit tests.
• No new tests added, per review scope. The spec fixture covers the behavior once the runner checks reasons.

[github-actions]

🤖 Kimi Code Review

Overall Assessment: Correct and well-justified consensus logic. The PR properly distinguishes between strict state transition validation (where out-of-range slots invalidate the block) and conservative gossip pre-filtering (where unknown slots default to "not justified").

Specific Comments:

crates/blockchain/state_transition/src/justified_slots_ops.rs

1. Line 28-36: The Result<bool, Error> return type is the correct way to distinguish "implicitly justified" (finalized), "explicitly not justified" (tracked), and "out of range" (error). This prevents the previous ambiguity where out-of-range slots were silently treated as justified.

2. Line 16 (existing code): The cast as usize in relative_index could theoretically truncate on 32-bit architectures if target_slot - finalized_slot - 1 > u32::MAX. While Ethereum slots are currently ~10M and the justified window is small (likely <10k slots), consider using try_into().ok()? or a bounds check for absolute correctness if this code might run on exotic architectures.

crates/blockchain/state_transition/src/lib.rs

3. Line 320: The use of ? to propagate JustifiedSlotOutOfRange from is_valid_vote to process_attestations (and thus invalidate the block) correctly implements the spec requirement that votes referencing slots beyond the tracked window make the block invalid.

4. Lines 400-409: The documentation comment accurately explains the distinction between "drop the vote" (soft failure) and "invalidate the block" (hard failure) semantics.

crates/blockchain/src/block_builder.rs

5. Lines 468-469 & 488: The .unwrap_or(false) handling for gossip validation is correct and safe:

   • For source checks (line 468): Unknown/untracked slots default to "not justified", causing the attestation to be rejected conservatively. This is safe for gossip pre-filtering.
   • For target checks (line 488): Unknown/untracked slots default to "not justified", allowing the attestation to remain eligible for inclusion (since we want to justify new slots).

6. Lines 460-464 & 479-482: The comments clearly explain the windowing semantics and the rationale for divergent behavior between the projection (gossip) and the STF.

Minor Suggestions:

• Code clarity: In block_builder.rs, consider replacing .unwrap_or(false) with an explicit match or if let to make the "unknown = not justified" logic more visible, though the current comments mitigate this.
• Documentation: The reference to leanSpec #1023 is

---

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

[github-actions]

🤖 Codex Code Review

Findings

1. crates/blockchain/src/block_builder.rs and crates/blockchain/src/block_builder.rs: unwrap_or(false) now swallows every is_slot_justified error, not just the newly introduced JustifiedSlotOutOfRange. Today that helper only returns one error variant, so behavior happens to match the comment, but the new Result API makes error meaning consensus-relevant. If another failure mode is added later, the proposer path will silently reinterpret it as a soft filter decision and keep building on a bad local state instead of failing loudly. This should match explicitly on Err(Error::JustifiedSlotOutOfRange { .. }) and treat any other error as unexpected.

The rest of the change looks coherent: the STF-side transition from “ignore” to hard-invalid for out-of-window justification slots is internally consistent with the process_block_header window invariant, and I don’t see a memory-safety or obvious performance regression in the touched paths.

I could not run the Rust tests in this environment. cargo test needs to fetch dependencies (leansig from GitHub / crates.io), but network access is blocked here. A direct regression test for the new JustifiedSlotOutOfRange invalidation path would still be worth adding in crates/blockchain/state_transition/src/lib.rs.

---

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

[github-actions]

🤖 Claude Code Review

Good — errors propagate transparently via #[from], so no exhaustive match sites need updating. This confirms the new variant is wired correctly through the whole call chain.

I have enough to write the review now.

Review: fix(stf) — invalidate blocks with out-of-range justified-slot votes (leanSpec #1023)

Overall: Solid, well-scoped fix. The core bug (an out-of-range justification lookup silently read as "not justified" via unwrap_or(false), letting a spec-invalid block be accepted) is real, and the fix correctly threads the new error through is_slot_justified → is_valid_vote → process_attestations using ?, while deliberately keeping the block-builder's pre-filter lenient with .unwrap_or(false). The asymmetry between the two call sites is explained clearly in both code comments and the PR description, and it's the right call — the builder's projection window legitimately differs from the STF's.

Correctness

• justified_slots_ops.rs:28-42 — logic is correct: slots ≤ finalized_slot still return Ok(true) unconditionally (implicitly justified), only in-range-but-past-tracked-bitlist slots now produce Err. This matches the documented semantics.
• lib.rs:411-450 — the short-circuiting order (source check before target check, both via ?) preserves the original control flow exactly: if the source is in-range-but-unjustified, the function returns Ok(false) without ever touching the target's bitlist, so no OOB error is raised for the target in that case. This mirrors the presumed Python spec behavior (if not is_justified(source): return False short-circuits identically), not a new bug — worth flagging in case reviewers assume both slots are always range-checked per vote.
• store.rs:993 confirms ethlambda_state_transition::Error is threaded via #[from], so the new JustifiedSlotOutOfRange variant needs no additional match-arm updates elsewhere in the codebase — good, no missed call sites.

Minor nits (non-blocking)

• lib.rs:52 — the #[error(...)] message uses finalized_boundary={finalized_slot} while the struct field is named finalized_slot. Harmless, but slightly inconsistent naming between the display string and the field; consider renaming one or the other for grep-ability when this error shows up in logs.
• No new unit test was added directly for is_slot_justified's new Err branch (PR notes this is intentional, relying on the spec fixture). Given this is a consensus-splitting bug, a small in-crate unit test asserting is_slot_justified returns Err(JustifiedSlotOutOfRange { .. }) for a slot beyond finalized_slot + tracked_length would be cheap insurance independent of the fixture bundle's filler behavior (which the PR itself notes has masked wrong-reason failures before, e.g. TOO_MANY_ATTESTATION_DATA/BLOCK_SLOT_MISMATCH).
• The doc comments on entry_passes_filters (block_builder.rs:460-465, 482-483) and is_slot_justified (justified_slots_ops.rs:20-27) are excellent — they explain why the two call sites diverge, which is exactly the kind of non-obvious invariant worth documenting.

Security

This is a legitimate consensus-safety fix: an attacker could previously craft a block with a vote whose source/target slot lies at or beyond the block's own slot to have it silently accepted by ethlambda while spec-strict clients reject it — a fork-inducing eclipse/split vector. The fix closes that gap without over-correcting the builder's own leniency (which would have broken block packing on the dominant attestation shape, per the PR's explanation).

No other concerns — the diff is minimal, well-targeted, and consistent with the crate's existing Result/thiserror error-handling conventions.

---

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

[greptile-apps]

Out-of-range target aborts proposal

When a pooled attestation has an in-range source but targets the proposed block's slot or later, .unwrap_or(false) treats the out-of-range target as eligible. The selected attestation then raises JustifiedSlotOutOfRange in the final state transition, aborting block construction and causing the proposer to miss the slot.

Knowledge Base Used: Blockchain core: fork choice, state transition, block building, sync

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/blockchain/src/block_builder.rs
Line: 490

Comment:
**Out-of-range target aborts proposal**

When a pooled attestation has an in-range source but targets the proposed block's slot or later, `.unwrap_or(false)` treats the out-of-range target as eligible. The selected attestation then raises `JustifiedSlotOutOfRange` in the final state transition, aborting block construction and causing the proposer to miss the slot.

**Knowledge Base Used:** [Blockchain core: fork choice, state transition, block building, sync](https://app.greptile.com/lambdaclass/-/custom-context/knowledge-base/lambdaclass/ethlambda/-/docs/blockchain-core.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Greptile Summary

This PR makes out-of-range justified-slot lookups explicit and block-invalidating in the state transition while preserving lenient lookup handling in the block builder.

• Adds the JustifiedSlotOutOfRange state-transition error.
• Changes vote validation and justified-slot lookup APIs to propagate lookup failures.
• Adapts builder filtering to interpret untracked slots as not justified.

Confidence Score: 4/5

This PR should not merge until the builder excludes votes that the changed state transition will reject, preventing malformed pooled attestations from causing missed proposals.

An out-of-range target is still considered eligible during proposal selection but now raises an error during the final state transition, aborting construction of the entire block.

Files Needing Attention: crates/blockchain/src/block_builder.rs

Important Files Changed

Filename	Overview
crates/blockchain/src/block_builder.rs	Preserves lenient target filtering, which can now select an attestation that aborts the changed final STF.
crates/blockchain/state_transition/src/justified_slots_ops.rs	Replaces the out-of-range false fallback with a domain error while retaining implicit justification for finalized slots.
crates/blockchain/state_transition/src/lib.rs	Propagates out-of-range source and target lookups so imported blocks containing such votes are rejected.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Pool[Pooled attestation] --> Filter[Builder target filter]
  Filter -->|out of range becomes false| Selected[Candidate selected]
  Selected --> STF[Final state transition]
  STF -->|JustifiedSlotOutOfRange| Abort[Block build aborts]

Loading

Prompt To Fix All With AI

### Issue 1
crates/blockchain/src/block_builder.rs:490
**Out-of-range target aborts proposal**

When a pooled attestation has an in-range source but targets the proposed block's slot or later, `.unwrap_or(false)` treats the out-of-range target as eligible. The selected attestation then raises `JustifiedSlotOutOfRange` in the final state transition, aborting block construction and causing the proposer to miss the slot.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(stf): invalidate blocks with out-of-..." | Re-trigger Greptile}