From 92ddfeed524a59bfe5241a2969d06ac81b7484d5 Mon Sep 17 00:00:00 2001 From: Bob Bobber Date: Mon, 28 Sep 2026 01:05:23 -0400 Subject: [PATCH 1/2] ibd: stop header prefix retries when first header fails --- .../src/ibd/events/ibd_memory_tests.rs | 84 ++++++++++++++++++- crates/rbitcoin-net/src/ibd/events/mod.rs | 48 ++++++----- 2 files changed, 112 insertions(+), 20 deletions(-) diff --git a/crates/rbitcoin-net/src/ibd/events/ibd_memory_tests.rs b/crates/rbitcoin-net/src/ibd/events/ibd_memory_tests.rs index c522eceda..51bc7bbe2 100644 --- a/crates/rbitcoin-net/src/ibd/events/ibd_memory_tests.rs +++ b/crates/rbitcoin-net/src/ibd/events/ibd_memory_tests.rs @@ -1,7 +1,7 @@ //! Header-path and body-intake bounds (findings C03, C05). use super::super::state::IbdWorkState; -use super::{apply_block_framed, on_headers_batch}; +use super::{accepted_prefix_after_failure, apply_block_framed, on_headers_batch}; use bitcoin::absolute::LockTime; use bitcoin::block::{Header, Version}; use bitcoin::consensus::encode::serialize; @@ -267,6 +267,88 @@ fn unmapped_stored_run_walks_once_not_per_header() { let _ = std::fs::remove_dir_all(dir); } +#[test] +fn failed_first_prefix_stops_search() { + let mut probes = Vec::new(); + let (lo, values): (usize, Vec) = accepted_prefix_after_failure(2000, |mid| { + probes.push(mid); + Err(crate::error::NetError::Protocol("first header rejected")) + }); + assert_eq!(lo, 0); + assert!(values.is_empty()); + assert_eq!( + probes, + vec![1], + "do not retry longer prefixes after first header fails" + ); + + let mut probes = Vec::new(); + let (lo, values) = accepted_prefix_after_failure(2000, |mid| { + probes.push(mid); + if mid <= 731 { + Ok((1..=mid).collect::>()) + } else { + Err(crate::error::NetError::Protocol("rejected tail")) + } + }); + assert_eq!(lo, 731); + assert_eq!(values.len(), lo); + assert_eq!(values[0], 1); + assert_eq!(values[lo - 1], 731); + assert_eq!(probes[0], 1); +} + +#[test] +fn unknown_first_header_does_not_retry_longer_prefixes() { + let (dir, hub) = tmp_hub(); + hub.ensure_genesis().unwrap(); + let unknown = BlockHash::from_byte_array([0x7d; 32]); + let mut chain = Vec::new(); + let mut prev = unknown; + for height in 1..=20 { + let header = mine(prev, 1_500_050_000 + height * 600, height).header; + prev = header.block_hash(); + chain.push(header); + } + let mut st = IbdWorkState::new(Vec::new(), hub.tip_hash(), hub.tip_height()); + on_headers_batch(&mut st, &hub, chain.clone()); + assert!(st.header_fks.is_empty()); + for header in &chain { + assert!(!st.hash_height.contains_key(&header.block_hash())); + } + let _ = std::fs::remove_dir_all(dir); +} + +#[test] +fn rejected_tail_keeps_stored_prefix() { + const CAP: u32 = 4; + let (dir, hub) = tmp_hub(); + hub.ensure_genesis().unwrap(); + let gen = hub.tip_hash().unwrap(); + let mut chain = Vec::new(); + let mut prev = gen; + for height in 1..=CAP + 8 { + let header = mine(prev, 1_500_030_000 + height * 600, height).header; + prev = header.block_hash(); + chain.push(header); + } + hub.ensure_headers_batch(&chain).unwrap(); + hub.set_stored_height_walk_cap(CAP); + let mut st = IbdWorkState::new(Vec::new(), hub.tip_hash(), hub.tip_height()); + let mut rejected = mine(prev, 1_500_030_000 + (CAP + 9) * 600, CAP + 9).header; + rejected.time = 0; + let mut batch = chain[CAP as usize..].to_vec(); + batch.push(rejected); + let _ = hub.take_stored_height_walk_steps(); + on_headers_batch(&mut st, &hub, batch); + assert!(hub.take_stored_height_walk_steps() >= u64::from(CAP)); + for header in &chain[CAP as usize..] { + assert!(st.header_fks.contains_key(&header.block_hash())); + } + assert!(!st.header_fks.contains_key(&rejected.block_hash())); + let _ = std::fs::remove_dir_all(dir); +} + /// `stored_header_height` stops after a cap (10,000 in production). A stored /// run that opens above that cap must walk once; later headers stay /// unresolved instead of each walking to the cap. The hub cap is lowered so diff --git a/crates/rbitcoin-net/src/ibd/events/mod.rs b/crates/rbitcoin-net/src/ibd/events/mod.rs index 7e898a9a7..4c3ad3e92 100644 --- a/crates/rbitcoin-net/src/ibd/events/mod.rs +++ b/crates/rbitcoin-net/src/ibd/events/mod.rs @@ -213,6 +213,32 @@ fn try_enqueue_ordered_header( false } +fn accepted_prefix_after_failure( + len: usize, + mut probe: impl FnMut(usize) -> Result, NetError>, +) -> (usize, Vec) { + let Ok(mut lo_fks) = probe(1) else { + return (0, Vec::new()); + }; + let mut lo = 1usize; + let mut hi = len; + for _ in 0..len { + let width = hi - lo; + if width <= 1 { + break; + } + let mid = lo + width / 2; + match probe(mid) { + Ok(fks) => { + lo = mid; + lo_fks = fks; + } + Err(_) => hi = mid, + } + } + (lo, lo_fks) +} + /// Longest prefix [`ChainHub::ensure_headers_batch`] accepts. /// /// A rejected tail is not stored and must not update path or explore state. @@ -230,25 +256,9 @@ fn ensure_accepted_prefix( if headers.len() == 1 { return Vec::new(); } - let mut lo = 0usize; - let mut lo_fks = Vec::new(); - let mut hi = headers.len(); - // A midpoint that does not shrink the window is not a longer prefix. - // The batch length caps a stuck step so it returns this `lo`. - for _ in 0..headers.len() { - let width = hi - lo; - if width <= 1 { - break; - } - let mid = lo + width / 2; - match hub.ensure_headers_batch(&headers[..mid]) { - Ok(fks) => { - lo = mid; - lo_fks = fks; - } - Err(_) => hi = mid, - } - } + let (lo, lo_fks) = accepted_prefix_after_failure(headers.len(), |mid| { + hub.ensure_headers_batch(&headers[..mid]) + }); headers[..lo].iter().copied().zip(lo_fks).collect() } From 63bfa5ea4ae988d6e69f0f6ed67d0da63b7d80fd Mon Sep 17 00:00:00 2001 From: Brandon Black Date: Mon, 28 Sep 2026 07:38:32 -0700 Subject: [PATCH 2/2] ibd: note the first-header prefix stop in the changelog A headers batch that cannot connect used to search longer prefixes after the full batch failed. The release note records that the search now stops when the first header is rejected. Co-authored-by: Cursor --- changelog.d/ibd-header-prefix-first-failure.md | 7 +++++++ crates/rbitcoin-net/src/ibd/events/mod.rs | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 changelog.d/ibd-header-prefix-first-failure.md diff --git a/changelog.d/ibd-header-prefix-first-failure.md b/changelog.d/ibd-header-prefix-first-failure.md new file mode 100644 index 000000000..b4a20aec7 --- /dev/null +++ b/changelog.d/ibd-header-prefix-first-failure.md @@ -0,0 +1,7 @@ +Fixed + +- **A headers batch whose first header does not connect skips the longer + prefix search.** After the full batch failed, IBD tried about ten longer + prefixes even when the first header could not be stored. The search now + stops after that first header fails. A connected first header still + binary-searches the rest of the batch. diff --git a/crates/rbitcoin-net/src/ibd/events/mod.rs b/crates/rbitcoin-net/src/ibd/events/mod.rs index 4c3ad3e92..1c43dae65 100644 --- a/crates/rbitcoin-net/src/ibd/events/mod.rs +++ b/crates/rbitcoin-net/src/ibd/events/mod.rs @@ -213,6 +213,9 @@ fn try_enqueue_ordered_header( false } +/// After the full batch fails, probe one header before searching. +/// +/// A rejected first header means no longer prefix can be stored. fn accepted_prefix_after_failure( len: usize, mut probe: impl FnMut(usize) -> Result, NetError>, @@ -242,7 +245,9 @@ fn accepted_prefix_after_failure( /// Longest prefix [`ChainHub::ensure_headers_batch`] accepts. /// /// A rejected tail is not stored and must not update path or explore state. -/// The success path is one batch. A failing tail binary-searches the prefix. +/// The success path is one batch. A failing batch probes the first header +/// and stops when that header is rejected. Otherwise the rest of the prefix +/// is binary-searched. fn ensure_accepted_prefix( hub: &ChainHub, headers: &[bitcoin::block::Header],