Skip to content

fix: reject attestations whose head is off the finalized chain#1502

Open
vuonghuuhung wants to merge 2 commits into
ReamLabs:masterfrom
vuonghuuhung:fix/lean-fork-choice-head-descends-from-finalized
Open

fix: reject attestations whose head is off the finalized chain#1502
vuonghuuhung wants to merge 2 commits into
ReamLabs:masterfrom
vuonghuuhung:fix/lean-fork-choice-head-descends-from-finalized

Conversation

@vuonghuuhung

Copy link
Copy Markdown
Contributor

What was wrong?

Address leanSpec #1179 (issue #1176)

TL;DR:

  • validate_attestation now rejects an attestation whose head does not descend from latest_finalized, mirroring the predicate prune_stale_attestation_data already uses.

Before this change, admission only checked ancestry between source, target, and head against each other, never against the store's latest_finalized. So a vote could be admitted, later dropped by pruning once its head landed on a finalized-orphaned branch, and then be re-admitted unchanged if the same vote was re-gossiped, undoing the prune and forcing repeated signature verification on data that carries zero fork-choice weight (its head is unreachable from the justified root once finalized has moved past it). Not a consensus-safety bug, but a pool-inflation / prune-idempotence gap.

How was it fixed?

  • validate_attestation in crates/common/fork_choice/lean/src/store.rs now checks checkpoint_is_ancestor(latest_finalized, head) alongside the existing source→target and target→head ancestry checks. A head equal to the finalized checkpoint still passes, so genesis-adjacent votes aren't wrongly rejected.
  • Two tests added:
    • test_validate_attestation_rejects_head_on_finalized_orphaned_branch: builds a canonical branch and a sibling orphan branch off the same parent, finalizes onto the canonical branch, and asserts a vote whose head is on the orphan branch is now rejected.
    • test_validate_attestation_accepts_head_descending_from_finalized: same setup, asserts a vote whose head descends from the canonical/finalized branch is still accepted.
  • Added short doc comments mirroring leanSpec's own documentation of the two invariant items that don't need code changes:
    • latest_justified (table doc): highest-slot justified checkpoint; the head walk starts here.
    • latest_finalized (table doc) and prune_stale_attestation_data (fn doc): finalized is re-derived from the head every update, so it's reorg-mutable, always an ancestor of the head, never monotone, never a safety guarantee on its own, pruning against it is only sound because of that re-derivation.

What's to notice

  • leanSpec's PR bundles four items (M-1 through M-4); only M-2 has an actual code change (typed RejectionReason + the ancestry check). M-1, M-3 and M-4 are documentation-only in leanSpec's diff too, ported here as doc comments.
  • ream has no typed rejection-reason enum in this crate (validate_attestation uses anyhow::ensure! with string messages throughout), so no new error type was introduced.

To-Do

@vuonghuuhung vuonghuuhung force-pushed the fix/lean-fork-choice-head-descends-from-finalized branch from 99b6c50 to 2c4474a Compare July 7, 2026 08:21
@vuonghuuhung vuonghuuhung marked this pull request as ready for review July 9, 2026 04:09
@perfogic

perfogic commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

let fix this one
image

@vuonghuuhung vuonghuuhung force-pushed the fix/lean-fork-choice-head-descends-from-finalized branch from 2c4474a to 5032cab Compare July 9, 2026 09: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