Skip to content

fix: don't bound source-target distance #431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pablodeymo merged 2 commits into from
Jun 11, 2026
+2 −19
Merged

fix: don't bound source-target distance #431

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e34c591
fix: don't bound source-target distance
MegaRedHand Jun 11, 2026
ab4868c
docs: update function documentation
MegaRedHand Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 2 additions & 19 deletions crates/blockchain/src/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,12 @@ fn update_safe_target(store: &mut Store) {
store.set_safe_target(safe_target);
}

/// Maximum number of parent links [`checkpoint_is_ancestor`] walks before
/// giving up. Bounds the per-attestation work on the gossip path; a vote whose
/// source/target sits more than this many slots below the descendant is
/// conservatively rejected.
const MAX_ANCESTRY_WALK_SLOTS: u64 = 128;

/// Return whether `ancestor` lies on `descendant`'s parent chain.
///
/// `descendant_header` is the descendant's already-fetched header, so the walk
/// starts from its parent without re-reading it. Walks parent links down to the
/// ancestor's slot, up to [`MAX_ANCESTRY_WALK_SLOTS`] steps. Empty (skipped)
/// slots on the path are traversed transparently since they carry no block.
/// Conservative: a block missing from the store, or exceeding the walk cap,
/// yields `false`.
/// ancestor's slot. Empty (skipped) slots on the path are traversed transparently
/// since they carry no block. A block missing from the store yields `false`.
fn checkpoint_is_ancestor(
store: &Store,
Comment on lines 119 to 126

@greptile-apps greptile-apps Bot Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The doc comment still refers to MAX_ANCESTRY_WALK_SLOTS and the walk-cap behavior ("up to [MAX_ANCESTRY_WALK_SLOTS] steps", "exceeding the walk cap, yields false"), both of which were removed by this PR. The dangling link will also produce a rustdoc warning ([MAX_ANCESTRY_WALK_SLOTS] resolves to nothing).

Suggested change
/// Return whether `ancestor` lies on `descendant`'s parent chain.
///
/// `descendant_header` is the descendant's already-fetched header, so the walk
/// starts from its parent without re-reading it. Walks parent links down to the
/// ancestor's slot, up to [`MAX_ANCESTRY_WALK_SLOTS`] steps. Empty (skipped)
/// slots on the path are traversed transparently since they carry no block.
/// Conservative: a block missing from the store, or exceeding the walk cap,
/// yields `false`.
/// ancestor's slot. Empty (skipped) slots on the path are traversed transparently
/// since they carry no block. A block missing from the store yields `false`.
fn checkpoint_is_ancestor(
store: &Store,
/// Return whether `ancestor` lies on `descendant`'s parent chain.
///
/// `descendant_header` is the descendant's already-fetched header, so the walk
/// starts from its parent without re-reading it. Walks parent links down to the
/// ancestor's slot. Empty (skipped) slots on the path are traversed
/// transparently since they carry no block.
/// Conservative: a block missing from the store yields `false`.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: crates/blockchain/src/store.rs
Line: 119-126

Comment:
The doc comment still refers to `MAX_ANCESTRY_WALK_SLOTS` and the walk-cap behavior ("up to `[MAX_ANCESTRY_WALK_SLOTS]` steps", "exceeding the walk cap, yields `false`"), both of which were removed by this PR. The dangling link will also produce a `rustdoc` warning (`[MAX_ANCESTRY_WALK_SLOTS]` resolves to nothing).

```suggestion
/// Return whether `ancestor` lies on `descendant`'s parent chain.
///
/// `descendant_header` is the descendant's already-fetched header, so the walk
/// starts from its parent without re-reading it. Walks parent links down to the
/// ancestor's slot. Empty (skipped) slots on the path are traversed
/// transparently since they carry no block.
/// Conservative: a block missing from the store yields `false`.
```

How can I resolve this? If you propose a fix, please make it concise.

ancestor: &Checkpoint,
Expand All @@ -144,22 +136,13 @@ fn checkpoint_is_ancestor(

// The descendant header is already in hand, so begin the walk at its parent.
let mut current_root = descendant_header.parent_root;
let mut steps: u64 = 0;
while let Some(current_header) = store.get_block_header(&current_root) {
if current_header.slot == ancestor.slot {
return current_root == ancestor.root;
}
if current_header.slot < ancestor.slot {
return false;
}
steps += 1;
if steps >= MAX_ANCESTRY_WALK_SLOTS {
warn!(
cap = MAX_ANCESTRY_WALK_SLOTS,
"Attestation ancestry walk exceeded cap, rejecting"
);
return false;
}
current_root = current_header.parent_root;
}

Expand Down