fix(blockchain): reject attestation whose head is off the finalized branch (leanSpec #1179)

[MegaRedHand]

Root cause

leanSpec PR #1179 (commit 9033157c7, merged 2026-07-04) added a HEAD_NOT_DESCENDANT_OF_FINALIZED rejection rule to attestation validation. Fork choice only ever descends from the latest finalized block, so a vote whose head sits on a branch orphaned by finalization carries no fork-choice weight. Without this rule, a vote that finalization pruning has already dropped is re-admitted when the same aggregate is re-gossiped, instead of being rejected.

ethlambda's shared attestation validation (validate_attestation_data, used by both on_gossip_attestation and on_gossip_aggregated_attestation_core) ran availability, topology, consistency, ancestry, and timing checks but never verified that the head descends from latest_finalized.

Fix

After the target-ancestor-of-head check, verify store.latest_finalized() is an ancestor of the head checkpoint, reusing the existing checkpoint_is_ancestor helper and the already-fetched head header. Adds a new StoreError::HeadNotDescendantOfFinalized variant (with head root/slot and finalized root/slot for context).

This mirrors leanSpec #1179's semantics and insertion point exactly. It is sound because ethlambda re-derives the store's finalized checkpoint from the head on each head update, so finalized is always an ancestor of the head.

Tests

• Fixes the previously failing fork-choice spec test test_re_gossip_of_pruned_orphaned_vote_is_rejected.
• Adds unit test validate_attestation_rejects_head_off_finalized_branch (admissible while finalized sits below the fork point; rejected once a sibling finalizes past it).

test result: ok. 517 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

(make test, full workspace, release)

Notes

• A follow-up commit will add test-runner/driver hardening.

[github-actions]

🤖 Kimi Code Review

Review: PR #510 - Finalized-Ancestor Check for Attestation Validation

Overall: Correct and well-tested. This implements a critical consensus safety check per leanSpec #1179.

Consensus Correctness

crates/blockchain/src/store.rs:227-241
The logic correctly implements the fork choice invariant that attestations must descend from the latest finalized block. The check placement after target→head ancestry but before slot validation is appropriate.

Minor clarification: Ensure checkpoint_is_ancestor handles the edge case where head == finalized (returns true). The current error message "does not descend from" implies strict descent, but consensus-wise a block should be considered a descendant of itself for this check.

Code Quality

crates/blockchain/src/store.rs:954-963
The new StoreError variant uses descriptive named fields. The thiserror formatting is valid.

Suggestion: Consider adding the attestation's source checkpoint to the error context for debugging re-org scenarios, though the current fields are sufficient.

Test Coverage

crates/blockchain/src/store.rs:1558-1630
The test validate_attestation_rejects_head_off_finalized_branch correctly validates:

1. Admissibility while finalized sits below the fork point
2. Rejection after finalization moves past the fork

Nit: The test uses update_checkpoints with ForkCheckpoints::new(block_2, None, Some(finalized)). Verify that block_2 (the block root) and finalized (the checkpoint) are correctly distinguished—finalized.root should equal block_2 in this test setup, which appears correct.

Security Assessment

This check prevents a stale attestation replay attack where validators could re-gossip attestations for orphaned branches after finalization pruning has dropped them. Without this check, an attacker could accumulate weight on dead branches, potentially confusing the fork choice of syncing nodes. This is a necessary security boundary.

Performance

The ancestry check walks back from head to finalized. Since Ethereum finalizes every epoch (32 slots) and the store prunes orphaned branches, the traversal depth is bounded (~32 slots typical, ~8192 slots worst-case). This is acceptable for attestation validation hot path.

Verdict

Approve with minor suggestion. The implementation is spec-compliant and secure. Consider documenting in the function-level docs that this check ensures finalized_checkpoint <= head_checkpoint in the ancestor relation.

// Optional documentation addition at line 227:
/// Ensures the attestation votes on a chain descending from the latest finalized
/// checkpoint, preventing stale votes from affecting fork choice after finalization.

---

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

[greptile-apps]

Greptile Summary

This PR rejects attestations whose head is no longer on the finalized branch. The main changes are:

• Adds a finalized-ancestor check to shared attestation validation.
• Adds a HeadNotDescendantOfFinalized store error with head and finalized context.
• Adds a regression test for re-gossiped votes on an orphaned branch.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
crates/blockchain/src/store.rs	Adds the finalized-ancestor attestation check, the new rejection error, and focused regression coverage for orphaned-branch votes.

_{Reviews (1): Last reviewed commit: "fix(blockchain): reject attestation whos..." | Re-trigger Greptile}

[github-actions]

🤖 Codex Code Review

1. crates/blockchain/src/store.rs:230 fixes only the re-gossip path, not the already-admitted stale-vote case. When finalization advances, update_checkpoints() prunes payloads by target.slot <= finalized_slot and signatures by data.slot <= finalized_slot only, so an attestation that was valid before finalization but now points to a head on an orphaned branch can remain in known_payloads / new_payloads / gossip_signatures if its target/attestation slot is still above the finalized slot. Those entries are still surfaced by extract_latest_*_attestations() and can keep polluting fork-choice inputs and aggregation reuse. If the intent of leanSpec #1179 is “once orphaned by finalization, this vote is no longer admissible,” the prune path needs the same finalized-ancestor predicate or a revalidation sweep on checkpoint advance. See store.rs, store.rs, store.rs, store.rs, store.rs, store.rs.

Briefly: the new ancestor check itself looks sound, and the added regression test is useful, but it only covers target.slot == finalized.slot. I’d add a case where the orphaned vote’s target.slot and head.slot are both still above the finalized slot, since that is the gap the current prune logic leaves open.

I couldn’t run the Rust test target here because cargo/rustup failed to create temp files under the current filesystem restrictions.

---

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

[github-actions]

🤖 Claude Code Review

Review: fix(blockchain): reject attestation whose head is off the finalized branch (leanSpec #1179)

Summary

This is a correct, well-targeted fix. It adds the missing HEAD_NOT_DESCENDANT_OF_FINALIZED check from leanSpec #1179 to the shared validate_attestation_data, closing the gap where a re-gossiped vote whose head was pruned by finalization could be re-admitted. I traced the surrounding invariants (Store::latest_finalized(), update_head's re-derivation logic, update_checkpoints'/ForkCheckpoints' None-is-no-op semantics) and the soundness argument in the code comment holds: latest_finalized() is always monotonic and, on every path that updates it, derived to be an ancestor of the current head. The new unit test correctly exercises both the admissible and rejected cases with a genuine orphan-branch fork structure.

Findings

1. Doc comment not updated (minor, maintainability)
crates/blockchain/src/store.rs:162-171 — the function-level doc comment enumerates 7 checks ("Ensures the vote respects the basic laws of time and topology: 1. ... 7. ..."), but the new finalized-ancestor check (a real 8th check) isn't added to that list. Since this list is the primary reference for what validate_attestation_data guarantees, it should be updated so it doesn't silently drift out of sync with the code (as it just did once already).

2. Redundant ancestor walk (minor, performance)
crates/blockchain/src/store.rs:236-244 — the new check walks from head_header all the way down to finalized.slot, which duplicates the tail of the walk already performed a few lines above for checkpoint_is_ancestor(store, &data.target, &data.head, &head_header) (line 226). Since source.slot <= target.slot <= head.slot is already established by this point (topology checks at lines 188/191) and transitivity holds along a single parent chain, this could instead check checkpoint_is_ancestor(store, &finalized, &data.target, &target_header) (or even against source_header, if finalized.slot is typically ≤ source.slot) using the already-fetched header, shortening the average-case walk. Not a correctness issue — just extra work on a hot path (every gossiped attestation/aggregate) that's easy to trim.

Things verified as correct (no action needed)

• Check placement (after ancestry, before the head-slot/time checks) matches the described leanSpec insertion point.
• checkpoint_is_ancestor correctly returns false for slot-equal-but-root-mismatched checkpoints and short-circuits without a walk when ancestor.slot >= descendant.slot.
• No exhaustive match over StoreError elsewhere in the codebase, so adding the HeadNotDescendantOfFinalized variant is a non-breaking, additive change.
• BlockHeaders are never pruned (only LiveChain/signatures are), so the ancestor walk down to finalized remains valid even across pruning boundaries.
• The new test (validate_attestation_rejects_head_off_finalized_branch) builds a genuine sibling-branch scenario and checks both the pre-finalization-admissible and post-finalization-rejected transitions, with a precise matches! assertion on the new error variant's fields.

Nothing here blocks merging; both findings are polish-level.

---

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