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/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..1c43dae65 100644 --- a/crates/rbitcoin-net/src/ibd/events/mod.rs +++ b/crates/rbitcoin-net/src/ibd/events/mod.rs @@ -213,10 +213,41 @@ 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>, +) -> (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. -/// 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], @@ -230,25 +261,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() }