Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion changelog.d/one-headers-ask.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Changed

- **During IBD there is one headers question: the header after the candidate.** The locator also names a lighter fork still being weighed. A refused low-work batch is not paged, and a connecting peer does not start a second headers sync. An unknown block inv from outside the header slot does not retire the reserved peer. After IBD, connect-time headers sync is unchanged.
- **During IBD one peer is asked for the next header that is not stored.** While the queue has room the locator starts at the stored tip, then the candidate and a lighter fork. A full queue asks from the candidate. A refused low-work batch is not paged. Only one connecting peer starts headers sync. An unknown block inv from outside the header slot does not retire the reserved peer. After IBD, connect-time headers sync is unchanged.
3 changes: 3 additions & 0 deletions changelog.d/queue-refill.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed

- **An empty header queue during IBD asks from the stored tip.** A reply that continues that tip is written onto the queue even when it stops short of the next checkpoint, so block download can resume when a restart kept the walk checkpoint and not the headers in between. A reply that misses the checkpoint does not occupy the gap, and a later reply that meets it replaces the short guess. A `headers_done` latch that never proved a walk opens again when a peer is ahead. A proved walk already at the confirmed tip does not. A connecting peer still gets the one initial headers sync.
28 changes: 22 additions & 6 deletions crates/rbitcoin-net/src/ibd/exit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,24 @@ pub(crate) fn empty_path_header_fan(st: &IbdWorkState, tip_h: u32, alive: usize)

/// Clear `headers_done` so empty-path `getheaders` can resume.
///
/// Latch stops the tip storm. Unlatch only for a 1–2 block connecting-header
/// hole. A far-ahead `max_peer_height` (less-work fork / bogus LastBlock) is
/// not a most-work tip — do not unlatch and chase it.
/// A proved walk at the confirmed tip does not chase a far
/// `version.start_height`, including one advertised by a connected peer.
/// One or two missing connecting headers still open. A proved walk above
/// the confirmed tip, or a latch that never proved a walk, opens for any
/// peer lead.
#[inline]
pub(crate) fn should_unlatch_headers_done(st: &IbdWorkState, tip_h: u32) -> bool {
if !st.headers_done || !st.ordered.is_empty() {
return false;
}
let lag = header_lag_behind_peers(st, tip_h);
lag > 0 && lag <= 2 && !has_tip_plus_one(st, tip_h)
if lag == 0 || has_tip_plus_one(st, tip_h) {
return false;
}
if st.header_walk.proved_at_or_below(tip_h) {
return lag <= 2;
}
!super::header_walk::headers_complete(st, tip_h)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[bug] !headers_complete is broader than "a latch that never proved a walk." headers_complete is false when the walk tip is above the confirmed tip (the hole this patch should reopen) and also when any non-quiet live peer has peer_height above a proved walk that is already at the confirmed tip (wants_lookahead). header_lag_behind_peers still uses max_peer_height, so a far version.start_height unlatches. The new should_unlatch_headers_done test never inserts a slot, and prove_through does not set a tip hash, so wants_lookahead stays false and the signet assertion does not cover a connected peer. After the unlatch, header_poll will not LatchDone while empty_path_header_fan sees lag > 2. A full 2000-header reply returns before settle_walk_peer can insert walk_quiet, so headers_complete stays false. headers_done is only set again by an empty reply or LatchDone. ibd_caught_up then stays false (headers_complete false and lag > 2 requires headers_done), and the main loop keeps enqueueing HeaderCmd::Ask on the empty-path cadence. A proved walk above the confirmed tip with a real hole should still unlatch; that part is not this bug. A proved walk at the confirmed tip must not.

Suggestion: Unlatch a proved walk only when its tip is above the confirmed tip or above max_ordered_height (the missing download path). When the walk is proved and tip_height() <= tip_h, keep the old rule: do not clear headers_done for lag > 2. Add a slot whose peer_height is far above that tip to should_unlatch_headers_done and expect no unlatch. Re-latch, or leave the latch set, when a full window fails to extend the candidate so catch-up can still finish.

}

/// Full `seed_work_path_from_store` (O(header_count) walk) while empty-lagging.
Expand Down Expand Up @@ -503,6 +511,7 @@ mod tests {
// Empty-EOF at the most-work path: extra advertised height is not a
// tip we chase (less-work high-height fork / bogus LastBlock).
signet.headers_done = true;
signet.header_walk.prove_through(2000);
assert!(
ibd_caught_up(&signet, 2000),
"headers_done at path HWM completes even if start_height is far ahead"
Expand Down Expand Up @@ -541,7 +550,8 @@ mod tests {
assert!(!super::should_unlatch_headers_done(&at, 100));
}

/// Unlatch only a 1–2 block connecting-header hole. lag>2 is not a chase.
/// An unproved empty queue unlatches for any real peer lead. A proved
/// walk does not chase a far start height.
#[test]
fn should_unlatch_headers_done() {
use super::super::state::InflightReq;
Expand All @@ -557,10 +567,16 @@ mod tests {
.insert(BlockHash::from_byte_array([i; 32]), InflightReq::new(0));
}
assert_eq!(header_lag_behind_peers(&st, 100), 5);
assert!(
super::should_unlatch_headers_done(&st, 100),
"an unproved latch asks again when a peer is more than two blocks ahead"
);
st.header_walk.prove_through(100);
assert!(
!super::should_unlatch_headers_done(&st, 100),
"lag>2 with no connecting hole is not a most-work tip we unlatch for"
"a proved walk does not chase a far start height"
);
st.header_walk = super::super::header_walk::HeaderWalk::default();

st.max_peer_height = 100;
assert_eq!(header_lag_behind_peers(&st, 100), 0);
Expand Down
Loading
Loading