From 269e8c208a27ce36fc8ea0413a8827d307921523 Mon Sep 17 00:00:00 2001 From: russeree Date: Tue, 20 Oct 2026 08:31:03 -0700 Subject: [PATCH 1/4] net: learn an addr-fetch getaddr reply, then disconnect An addr-fetch session (the --proxy DNS seeds, -seednode) that got more than one address back called punish_disconnect and returned before learn_addrv2, so the reply never reached AddrMan and the seed took a full ban score. The getaddr also never credited the token bucket, so even a learned reply would have kept one address. Follow Core ProcessMessage ADDR/ADDRV2: sending getaddr adds MAX_ADDR_TO_SEND tokens, the reply is learned, and the addr-fetch session then ends with request_disconnect and no misbehavior. Co-Authored-By: Claude Opus 5.5 --- changelog.d/addrfetch-learns-reply.md | 7 +++ crates/rbitcoin-net/src/peer.rs | 37 ++++++----- crates/rbitcoin-net/src/peers.rs | 9 +++ crates/rbitcoin-net/tests/peer/mod.rs | 88 +++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 14 deletions(-) create mode 100644 changelog.d/addrfetch-learns-reply.md diff --git a/changelog.d/addrfetch-learns-reply.md b/changelog.d/addrfetch-learns-reply.md new file mode 100644 index 0000000000..d1574d59c5 --- /dev/null +++ b/changelog.d/addrfetch-learns-reply.md @@ -0,0 +1,7 @@ +Fixed + +- **An addr-fetch session keeps the addresses it asked for.** The `getaddr` + reply from an addr-fetch peer (the `--proxy` DNS seeds, `-seednode`) was + dropped before AddrMan and scored as misbehavior. As in Core, the + `getaddr` now buys `MAX_ADDR_TO_SEND` more tokens, the reply is learned, + and the session ends with a plain disconnect. diff --git a/crates/rbitcoin-net/src/peer.rs b/crates/rbitcoin-net/src/peer.rs index ed5175e98d..3220839671 100644 --- a/crates/rbitcoin-net/src/peer.rs +++ b/crates/rbitcoin-net/src/peer.rs @@ -455,6 +455,10 @@ impl AddrTokenBucket { self.tokens -= take as f64; take } + + pub(crate) fn credit(&mut self, n: usize) { + self.tokens += n as f64; + } } #[doc(hidden)] @@ -2148,6 +2152,7 @@ pub fn maybe_queue_addrfetch_getaddr( if session.conn_type != crate::peers::PeerConnType::AddrFetch { return false; } + session.credit_getaddr_reply(); let _ = queue_out(out, NetworkMessage::GetAddr); true } @@ -2971,7 +2976,7 @@ pub fn handle_peer_inventory_msg( return Ok(()); } match payload { - NetworkMessage::Addr(list) => on_addr_list(follow, session, list.len())?, + NetworkMessage::Addr(list) => on_addr_list(session, list.len())?, NetworkMessage::AddrV2(list) => on_addrv2(follow, session, list)?, NetworkMessage::GetHeaders(gh) => on_getheaders(hub, out_tx, follow, session, gh)?, NetworkMessage::GetBlocks(gb) => on_getblocks(hub, out_tx, follow, session, gb)?, @@ -3083,18 +3088,23 @@ pub fn on_sendaddrv2(follow: &mut PeerFollowState, session: Option<&crate::peers } #[doc(hidden)] -pub fn on_addr_list( - follow: &mut PeerFollowState, - session: Option<&crate::peers::LivePeer>, - n: usize, -) -> Result<(), NetError> { - if session.is_some_and(|s| s.conn_type == crate::peers::PeerConnType::AddrFetch && n > 1) { - punish_disconnect(&mut follow.ban_score, session); - return Ok(()); - } +pub fn on_addr_list(session: Option<&crate::peers::LivePeer>, n: usize) -> Result<(), NetError> { + end_addrfetch_on_reply(session, n); Ok(()) } +/// Core: an addr-fetch peer that answered with more than one address is +/// done. It is disconnected, not punished. +fn end_addrfetch_on_reply(session: Option<&crate::peers::LivePeer>, n: usize) -> bool { + match session { + Some(s) if s.conn_type == crate::peers::PeerConnType::AddrFetch && n > 1 => { + s.request_disconnect(); + true + } + _ => false, + } +} + #[doc(hidden)] pub fn on_addrv2( follow: &mut PeerFollowState, @@ -3111,16 +3121,15 @@ pub fn on_addrv2( misbehaving(&mut follow.ban_score, session); return Ok(()); } - if session.is_some_and(|s| s.conn_type == crate::peers::PeerConnType::AddrFetch && n > 1) { - punish_disconnect(&mut follow.ban_score, session); - return Ok(()); - } if let Some(s) = session { if let Some(ph) = s.peer_hub() { // Node time, as the bucket's start (`connected_at`) is. let now_ms = s.clock_now().saturating_mul(1000); let allow = s.take_addr_tokens(list.len(), now_ms); ph.learn_addrv2(&list[..allow]); + if end_addrfetch_on_reply(session, n) { + return Ok(()); + } let mut neighbors: Vec<_> = ph .live_peers() .into_iter() diff --git a/crates/rbitcoin-net/src/peers.rs b/crates/rbitcoin-net/src/peers.rs index 429ee268b6..e97c893779 100644 --- a/crates/rbitcoin-net/src/peers.rs +++ b/crates/rbitcoin-net/src/peers.rs @@ -983,6 +983,15 @@ impl LivePeer { *self.out_tx.lock().unwrap_or_else(|e| e.into_inner()) = Some(tx); } + /// Core `m_addr_token_bucket += MAX_ADDR_TO_SEND` on `getaddr`: the + /// reply may exceed the bucket cap. + pub(crate) fn credit_getaddr_reply(&self) { + self.addr_bucket + .lock() + .unwrap_or_else(|e| e.into_inner()) + .credit(crate::peer::MAX_ADDR_TO_SEND); + } + /// How many of the `want` addresses in one message this peer may have /// processed (learned and relayed). The rest count as rate limited. A /// peer with the `addr` permission spends tokens but is never limited. diff --git a/crates/rbitcoin-net/tests/peer/mod.rs b/crates/rbitcoin-net/tests/peer/mod.rs index 7a561c7ccb..e531ce3890 100644 --- a/crates/rbitcoin-net/tests/peer/mod.rs +++ b/crates/rbitcoin-net/tests/peer/mod.rs @@ -5075,3 +5075,91 @@ fn compact_filter_ranges_past_core_limits_disconnect() { } mod log_text_tests; + +/// Core `ProcessMessage` ADDR/ADDRV2: the `getaddr` an addr-fetch session +/// sends buys `MAX_ADDR_TO_SEND` more addresses. The reply is learned, then +/// the session ends without misbehavior. +#[tokio::test] +async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { + use bitcoin::p2p::address::{AddrV2, AddrV2Message, Address}; + use bitcoin::p2p::message_network::VersionMessage; + use bitcoin::p2p::ServiceFlags; + use std::sync::atomic::Ordering; + + let (dir, hub) = rbitcoin_net::chain::tiny_regtest_hub_labeled("addrfetch-reply"); + let peers = rbitcoin_net::peers::PeerHub::new(); + let book = std::sync::Arc::new(std::sync::Mutex::new(rbitcoin_net::seeds::AddrMan::new())); + peers.set_addrman(std::sync::Arc::clone(&book)); + let seed_at = |port: u16| { + let addr = std::net::SocketAddr::from(([127, 0, 0, 1], port)); + let ver = VersionMessage { + version: bitcoin::p2p::PROTOCOL_VERSION, + services: ServiceFlags::NETWORK | ServiceFlags::WITNESS, + timestamp: 0, + receiver: Address::new(&addr, ServiceFlags::NONE), + sender: Address::new(&addr, ServiceFlags::NONE), + nonce: 0, + user_agent: "/rbitcoin:test/".into(), + start_height: 0, + relay: true, + }; + peers.register( + addr, + addr, + &ver, + false, + rbitcoin_net::peers::PeerConnType::AddrFetch, + ) + }; + + let seed = seed_at(18460); + let (out_tx, _out_rx) = mpsc::unbounded_channel(); + assert!(maybe_queue_addrfetch_getaddr(&out_tx, &seed)); + let time = seed.clock_now() as u32; + let reply = (0..MAX_ADDR_TO_SEND as u32) + .map(|i| AddrV2Message { + time, + services: ServiceFlags::NETWORK | ServiceFlags::WITNESS, + addr: AddrV2::Ipv4(std::net::Ipv4Addr::from(0x0b00_0001 + (i << 16))), + port: 18444, + }) + .collect(); + let mut follow = PeerFollowState::new(); + push( + &hub, + &out_tx, + &mut follow, + Some(seed.as_ref()), + NetworkMessage::AddrV2(reply), + ) + .await; + let learned = book.lock().unwrap().len(); + assert!( + learned > 1, + "the getaddr reply is learned past one token: {learned}" + ); + assert!( + seed.stop.load(Ordering::SeqCst), + "the reply ends addr-fetch" + ); + assert_eq!(follow.ban_score, 0, "a getaddr reply is not misbehavior"); + + let v1_seed = seed_at(18461); + let addr = std::net::SocketAddr::from(([11, 0, 0, 1], 18444)); + let v1 = bitcoin::p2p::address::Address::new(&addr, ServiceFlags::NETWORK); + let mut follow = PeerFollowState::new(); + push( + &hub, + &out_tx, + &mut follow, + Some(v1_seed.as_ref()), + NetworkMessage::Addr(vec![(time, v1.clone()), (time, v1)]), + ) + .await; + assert!( + v1_seed.stop.load(Ordering::SeqCst), + "a v1 reply ends addr-fetch" + ); + assert_eq!(follow.ban_score, 0, "a v1 getaddr reply is not misbehavior"); + let _ = std::fs::remove_dir_all(dir.path()); +} From 507bda95355372d8469017fdcac44f27aa3a50be Mon Sep 17 00:00:00 2001 From: russeree Date: Tue, 20 Oct 2026 10:00:12 -0700 Subject: [PATCH 2/4] net: learn a v1 addr-fetch reply and never relay one Review follow-up. A v1 addr reply to an addr-fetch getaddr was only a disconnect and never reached AddrMan; it is now converted and learned. An addr-fetch session no longer relays what it learns: Core sets m_getaddr_sent and does not relay a getaddr reply, and a run of one-address addrv2 messages fell through to the neighbor relay. learn_addrfetch_reply is the one path for both. The test checks at least 900 of the 1000 addresses are learned, that a wants_addrv2 neighbor gets nothing relayed, that a one-address self-announce is learned without ending the session, and that the v1 reply reaches the book. Co-Authored-By: Claude Opus 5.5 --- changelog.d/addrfetch-learns-reply.md | 5 +- crates/rbitcoin-net/src/peer.rs | 62 +++++++++++++++++------ crates/rbitcoin-net/tests/peer/mod.rs | 72 +++++++++++++++++++-------- 3 files changed, 102 insertions(+), 37 deletions(-) diff --git a/changelog.d/addrfetch-learns-reply.md b/changelog.d/addrfetch-learns-reply.md index d1574d59c5..c430388e06 100644 --- a/changelog.d/addrfetch-learns-reply.md +++ b/changelog.d/addrfetch-learns-reply.md @@ -3,5 +3,6 @@ Fixed - **An addr-fetch session keeps the addresses it asked for.** The `getaddr` reply from an addr-fetch peer (the `--proxy` DNS seeds, `-seednode`) was dropped before AddrMan and scored as misbehavior. As in Core, the - `getaddr` now buys `MAX_ADDR_TO_SEND` more tokens, the reply is learned, - and the session ends with a plain disconnect. + `getaddr` now buys `MAX_ADDR_TO_SEND` more tokens, the reply (`addrv2` or + v1 `addr`) is learned and not relayed, and more than one address ends the + session with a plain disconnect. diff --git a/crates/rbitcoin-net/src/peer.rs b/crates/rbitcoin-net/src/peer.rs index 3220839671..3a13a93e09 100644 --- a/crates/rbitcoin-net/src/peer.rs +++ b/crates/rbitcoin-net/src/peer.rs @@ -2976,7 +2976,7 @@ pub fn handle_peer_inventory_msg( return Ok(()); } match payload { - NetworkMessage::Addr(list) => on_addr_list(session, list.len())?, + NetworkMessage::Addr(list) => on_addr_list(follow, session, list)?, NetworkMessage::AddrV2(list) => on_addrv2(follow, session, list)?, NetworkMessage::GetHeaders(gh) => on_getheaders(hub, out_tx, follow, session, gh)?, NetworkMessage::GetBlocks(gb) => on_getblocks(hub, out_tx, follow, session, gb)?, @@ -3087,21 +3087,52 @@ pub fn on_sendaddrv2(follow: &mut PeerFollowState, session: Option<&crate::peers punish_disconnect(&mut follow.ban_score, session); } +/// v1 `addr` is learned only as an addr-fetch session's `getaddr` reply. #[doc(hidden)] -pub fn on_addr_list(session: Option<&crate::peers::LivePeer>, n: usize) -> Result<(), NetError> { - end_addrfetch_on_reply(session, n); +pub fn on_addr_list( + follow: &mut PeerFollowState, + session: Option<&crate::peers::LivePeer>, + list: &[(u32, bitcoin::p2p::address::Address)], +) -> Result<(), NetError> { + let Some(s) = session.filter(|s| s.conn_type == crate::peers::PeerConnType::AddrFetch) else { + return Ok(()); + }; + if list.len() > MAX_ADDR_TO_SEND { + misbehaving(&mut follow.ban_score, session); + return Ok(()); + } + let v2: Vec<_> = list + .iter() + .filter_map(|(time, a)| { + let sock = a.socket_addr().ok()?; + Some(bitcoin::p2p::address::AddrV2Message { + time: *time, + services: a.services, + addr: crate::NetAddr::from_socket(sock).to_addrv2(), + port: sock.port(), + }) + }) + .collect(); + learn_addrfetch_reply(s, &v2, list.len()); Ok(()) } -/// Core: an addr-fetch peer that answered with more than one address is -/// done. It is disconnected, not punished. -fn end_addrfetch_on_reply(session: Option<&crate::peers::LivePeer>, n: usize) -> bool { - match session { - Some(s) if s.conn_type == crate::peers::PeerConnType::AddrFetch && n > 1 => { - s.request_disconnect(); - true - } - _ => false, +/// Core `ProcessMessage` ADDR/ADDRV2 on an addr-fetch session: the reply to +/// the `getaddr` it sent is learned and not relayed (`m_getaddr_sent`). +/// More than one address ends the session, without misbehavior. +fn learn_addrfetch_reply( + s: &crate::peers::LivePeer, + list: &[bitcoin::p2p::address::AddrV2Message], + n: usize, +) { + if let Some(ph) = s.peer_hub() { + // Node time, as the bucket's start (`connected_at`) is. + let now_ms = s.clock_now().saturating_mul(1000); + let allow = s.take_addr_tokens(list.len(), now_ms); + ph.learn_addrv2(&list[..allow]); + } + if n > 1 { + s.request_disconnect(); } } @@ -3121,15 +3152,16 @@ pub fn on_addrv2( misbehaving(&mut follow.ban_score, session); return Ok(()); } + if let Some(s) = session.filter(|s| s.conn_type == crate::peers::PeerConnType::AddrFetch) { + learn_addrfetch_reply(s, list, n); + return Ok(()); + } if let Some(s) = session { if let Some(ph) = s.peer_hub() { // Node time, as the bucket's start (`connected_at`) is. let now_ms = s.clock_now().saturating_mul(1000); let allow = s.take_addr_tokens(list.len(), now_ms); ph.learn_addrv2(&list[..allow]); - if end_addrfetch_on_reply(session, n) { - return Ok(()); - } let mut neighbors: Vec<_> = ph .live_peers() .into_iter() diff --git a/crates/rbitcoin-net/tests/peer/mod.rs b/crates/rbitcoin-net/tests/peer/mod.rs index e531ce3890..223eb56257 100644 --- a/crates/rbitcoin-net/tests/peer/mod.rs +++ b/crates/rbitcoin-net/tests/peer/mod.rs @@ -5077,8 +5077,9 @@ fn compact_filter_ranges_past_core_limits_disconnect() { mod log_text_tests; /// Core `ProcessMessage` ADDR/ADDRV2: the `getaddr` an addr-fetch session -/// sends buys `MAX_ADDR_TO_SEND` more addresses. The reply is learned, then -/// the session ends without misbehavior. +/// sends buys `MAX_ADDR_TO_SEND` more addresses. The reply, v1 or v2, is +/// learned and not relayed (`m_getaddr_sent`). More than one address ends the +/// session without misbehavior; a one-address self-announce does not. #[tokio::test] async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { use bitcoin::p2p::address::{AddrV2, AddrV2Message, Address}; @@ -5090,7 +5091,7 @@ async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { let peers = rbitcoin_net::peers::PeerHub::new(); let book = std::sync::Arc::new(std::sync::Mutex::new(rbitcoin_net::seeds::AddrMan::new())); peers.set_addrman(std::sync::Arc::clone(&book)); - let seed_at = |port: u16| { + let register = |port: u16, typ: rbitcoin_net::peers::PeerConnType| { let addr = std::net::SocketAddr::from(([127, 0, 0, 1], port)); let ver = VersionMessage { version: bitcoin::p2p::PROTOCOL_VERSION, @@ -5103,28 +5104,46 @@ async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { start_height: 0, relay: true, }; - peers.register( - addr, - addr, - &ver, - false, - rbitcoin_net::peers::PeerConnType::AddrFetch, - ) + peers.register(addr, addr, &ver, false, typ) }; - - let seed = seed_at(18460); + let neighbor = register(18459, rbitcoin_net::peers::PeerConnType::OutboundFullRelay); + neighbor.set_wants_addrv2(); + let (relay_tx, mut relayed) = mpsc::unbounded_channel(); + neighbor.attach_out(relay_tx); + let book_len = || book.lock().unwrap().len(); + let routable = |i: u32| AddrV2::Ipv4(std::net::Ipv4Addr::from(0x0b00_0001 + (i << 16))); + + let seed = register(18460, rbitcoin_net::peers::PeerConnType::AddrFetch); let (out_tx, _out_rx) = mpsc::unbounded_channel(); assert!(maybe_queue_addrfetch_getaddr(&out_tx, &seed)); let time = seed.clock_now() as u32; + let mut follow = PeerFollowState::new(); + push( + &hub, + &out_tx, + &mut follow, + Some(seed.as_ref()), + NetworkMessage::AddrV2(vec![AddrV2Message { + time, + services: ServiceFlags::NETWORK, + addr: AddrV2::Ipv4(std::net::Ipv4Addr::new(9, 9, 9, 9)), + port: 8333, + }]), + ) + .await; + assert_eq!(book_len(), 1, "a self-announce is learned"); + assert!( + !seed.stop.load(Ordering::SeqCst), + "one address is not the reply" + ); let reply = (0..MAX_ADDR_TO_SEND as u32) .map(|i| AddrV2Message { time, services: ServiceFlags::NETWORK | ServiceFlags::WITNESS, - addr: AddrV2::Ipv4(std::net::Ipv4Addr::from(0x0b00_0001 + (i << 16))), + addr: routable(i), port: 18444, }) .collect(); - let mut follow = PeerFollowState::new(); push( &hub, &out_tx, @@ -5133,9 +5152,9 @@ async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { NetworkMessage::AddrV2(reply), ) .await; - let learned = book.lock().unwrap().len(); + let learned = book_len() - 1; assert!( - learned > 1, + learned >= MAX_ADDR_TO_SEND * 9 / 10, "the getaddr reply is learned past one token: {learned}" ); assert!( @@ -5143,23 +5162,36 @@ async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { "the reply ends addr-fetch" ); assert_eq!(follow.ban_score, 0, "a getaddr reply is not misbehavior"); + assert!( + relayed.try_recv().is_err(), + "an addr-fetch reply is not relayed" + ); - let v1_seed = seed_at(18461); - let addr = std::net::SocketAddr::from(([11, 0, 0, 1], 18444)); - let v1 = bitcoin::p2p::address::Address::new(&addr, ServiceFlags::NETWORK); + let v1_seed = register(18461, rbitcoin_net::peers::PeerConnType::AddrFetch); + assert!(maybe_queue_addrfetch_getaddr(&out_tx, &v1_seed)); + let before = book_len(); + let v1 = |last: u8| { + let addr = std::net::SocketAddr::from(([12, 0, 0, last], 18444)); + (time, Address::new(&addr, ServiceFlags::NETWORK)) + }; let mut follow = PeerFollowState::new(); push( &hub, &out_tx, &mut follow, Some(v1_seed.as_ref()), - NetworkMessage::Addr(vec![(time, v1.clone()), (time, v1)]), + NetworkMessage::Addr(vec![v1(1), v1(2)]), ) .await; + assert!(book_len() > before, "a v1 getaddr reply is learned"); assert!( v1_seed.stop.load(Ordering::SeqCst), "a v1 reply ends addr-fetch" ); assert_eq!(follow.ban_score, 0, "a v1 getaddr reply is not misbehavior"); + assert!( + relayed.try_recv().is_err(), + "a v1 addr-fetch reply is not relayed" + ); let _ = std::fs::remove_dir_all(dir.path()); } From 9462002928c66be36a8abb46adc279c5bd957327 Mon Sep 17 00:00:00 2001 From: russeree Date: Tue, 20 Oct 2026 08:31:03 -0700 Subject: [PATCH 3/4] node: wait for a --proxy seed answer before catch-up With --proxy and an empty book, startup queues the DNS seeds as SOCKS addr-fetch dials and then reads its dial targets right away. No seed can answer in that time, so catch-up stayed incomplete and the node shut down about 60 ms in, saving no peers. Wait until the shared book holds an answer and no addr-fetch session is still live (a seed announces itself before its getaddr reply). Re-queue the seeds every 60 s until then. Shutdown ends the wait. Co-Authored-By: Claude Opus 5.5 --- changelog.d/proxy-seed-bootstrap-wait.md | 7 ++ crates/rbitcoin-node/src/run.rs | 58 ++++++++++++++++ crates/rbitcoin-node/tests/run.rs | 84 ++++++++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 changelog.d/proxy-seed-bootstrap-wait.md diff --git a/changelog.d/proxy-seed-bootstrap-wait.md b/changelog.d/proxy-seed-bootstrap-wait.md new file mode 100644 index 0000000000..1976f4ab14 --- /dev/null +++ b/changelog.d/proxy-seed-bootstrap-wait.md @@ -0,0 +1,7 @@ +Fixed + +- **A fresh `--proxy` datadir waits for its seeds.** Catch-up read the dial + targets before any SOCKS seed addr-fetch had answered, so a node with an + empty book shut down about 60 ms after start. It now waits until a seed + has answered and its session has ended, and re-queues the seeds every + 60 s until then. diff --git a/crates/rbitcoin-node/src/run.rs b/crates/rbitcoin-node/src/run.rs index 7a923672be..ba774470e8 100644 --- a/crates/rbitcoin-node/src/run.rs +++ b/crates/rbitcoin-node/src/run.rs @@ -685,6 +685,24 @@ pub async fn run_p2p(config: NodeConfig) -> Result<(), NodeError> { node.user_agent().to_string(), ); } + if proxy_seeds_queued && addrman.is_empty() { + info!("ibd: waiting for a SOCKS seed addrfetch answer…"); + if let Some(book) = await_proxy_seed_book( + &node.peers, + config.network, + &shared_peers, + &shutdown, + PROXY_SEED_RETRY, + ) + .await + { + addrman = book; + info!( + "ibd: seed addrfetch filled the book (book={})", + addrman.len() + ); + } + } let max_out = config.listen.max_outbound.max(1) as usize; let candidate_n = max_out.saturating_mul(2).clamp(16, 48); @@ -1589,6 +1607,46 @@ pub fn queue_proxy_seed_addrfetch(peers: &Arc, network: N n } +/// A SOCKS seed answer that lost its circuit is not retried by the dial +/// queue, so the wait re-queues the seeds on this period. +const PROXY_SEED_RETRY: Duration = Duration::from_secs(60); + +/// The shared book once a `--proxy` seed addr-fetch answer fills it, or +/// `None` on shutdown. Re-queues the seeds every `retry` until then. A seed +/// announces itself before its `getaddr` reply, so the answer is complete +/// only when no addr-fetch session is still live. +#[doc(hidden)] +pub async fn await_proxy_seed_book( + peers: &Arc, + network: Network, + shared_peers: &Arc>, + shutdown: &Shutdown, + retry: Duration, +) -> Option { + let mut poll = tokio::time::interval(Duration::from_millis(250)); + let mut requeue = tokio::time::interval_at(tokio::time::Instant::now() + retry, retry); + loop { + tokio::select! { + biased; + _ = shutdown.cancelled() => return None, + _ = requeue.tick() => { + let n = queue_proxy_seed_addrfetch(peers, network); + info!("ibd: no seed answer yet — re-queued {n} seed hostnames via SOCKS addrfetch"); + } + _ = poll.tick() => { + let answering = peers + .snapshot() + .iter() + .any(|p| p.conn_type == PeerConnType::AddrFetch); + let book = shared_peers.lock().unwrap_or_else(|e| e.into_inner()); + if !book.is_empty() && !answering { + return Some(book.clone()); + } + } + } + } +} + /// IBD horizon after `sync_cancellable` (or no peers to dial). #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum CatchUp { diff --git a/crates/rbitcoin-node/tests/run.rs b/crates/rbitcoin-node/tests/run.rs index 93fd978245..050f71a8b5 100644 --- a/crates/rbitcoin-node/tests/run.rs +++ b/crates/rbitcoin-node/tests/run.rs @@ -694,3 +694,87 @@ async fn max_run_deadline_wakes_without_the_perf_tick() { assert!(matches!(wake, TipFollowWake::Stop)); assert!(start.elapsed() < Duration::from_millis(200)); } + +/// A fresh `--proxy` datadir has no targets until a seed answers the +/// addr-fetch. Catch-up waits for that answer and re-queues the seeds. +#[tokio::test] +async fn proxy_seed_wait_requeues_until_an_answer_fills_the_book() { + let peers = rbitcoin_net::PeerHub::new(); + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); + peers.set_dialer(tx); + let shared = Arc::new(std::sync::Mutex::new(AddrMan::new())); + let answered: rbitcoin_net::NetAddr = "1.2.3.4:38333".parse().unwrap(); + let seed = { + let shared = Arc::clone(&shared); + tokio::spawn(async move { + let req = rx.recv().await.expect("seed addr-fetch re-queued"); + assert_eq!(req.typ, PeerConnType::AddrFetch); + shared.lock().unwrap().add_addr(answered); + }) + }; + let sd = Shutdown::new(); + let book = tokio::time::timeout( + Duration::from_secs(5), + await_proxy_seed_book( + &peers, + rbitcoin_primitives::Network::Signet, + &shared, + &sd, + Duration::from_millis(50), + ), + ) + .await + .expect("the wait ends once a seed answers") + .expect("no shutdown was requested"); + seed.await.unwrap(); + assert_eq!(book.len(), 1); + + let at = std::net::SocketAddr::from(([127, 0, 0, 1], 38333)); + let ver = bitcoin::p2p::message_network::VersionMessage { + version: bitcoin::p2p::PROTOCOL_VERSION, + services: bitcoin::p2p::ServiceFlags::NETWORK, + timestamp: 0, + receiver: bitcoin::p2p::Address::new(&at, bitcoin::p2p::ServiceFlags::NONE), + sender: bitcoin::p2p::Address::new(&at, bitcoin::p2p::ServiceFlags::NONE), + nonce: 0, + user_agent: "/seed:0/".into(), + start_height: 0, + relay: true, + }; + let answering = peers.register(at, at, &ver, false, PeerConnType::AddrFetch); + let held = tokio::time::timeout( + Duration::from_millis(600), + await_proxy_seed_book( + &peers, + rbitcoin_primitives::Network::Signet, + &shared, + &sd, + Duration::from_secs(60), + ), + ) + .await; + assert!(held.is_err(), "a seed still answering holds the wait"); + peers.unregister(answering.id); + let book = await_proxy_seed_book( + &peers, + rbitcoin_primitives::Network::Signet, + &shared, + &sd, + Duration::from_secs(60), + ) + .await + .expect("the answer is complete once the seed session ends"); + assert_eq!(book.len(), 1); + + let empty = Arc::new(std::sync::Mutex::new(AddrMan::new())); + sd.request(); + let book = await_proxy_seed_book( + &peers, + rbitcoin_primitives::Network::Signet, + &empty, + &sd, + Duration::from_secs(60), + ) + .await; + assert!(book.is_none(), "shutdown ends the wait with no book"); +} From fb9ddd59cbc957dc908c3d62c9fd6ee34ff6a464 Mon Sep 17 00:00:00 2001 From: russeree Date: Tue, 20 Oct 2026 10:09:06 -0700 Subject: [PATCH 4/4] node: bound the --proxy seed wait and count learned replies Review follow-up. The wait had no deadline, so a proxy that never answered held run_p2p before catch-up and RPC for good. It now gives up after PROXY_SEED_WAIT (180 s) and the empty book takes the existing awaiting_dialable path. It also ended on "book non-empty and no addr-fetch session live", which a stored one-address self-announce can satisfy. PeerHub now counts addr-fetch getaddr replies learned (more than one address, bumped only in learn_addrfetch_reply), and the wait ends when that count rises. Seeds are re-queued only while no addr-fetch session is live. Stacked on the addr-fetch learn fix (#1101): without it no seed reply is learned and the count never rises. Co-Authored-By: Claude Opus 5.5 --- changelog.d/proxy-seed-bootstrap-wait.md | 7 +- crates/rbitcoin-net/src/peer.rs | 3 + crates/rbitcoin-net/src/peers.rs | 13 +++ crates/rbitcoin-net/tests/peer/mod.rs | 7 ++ crates/rbitcoin-node/src/run.rs | 44 +++++++--- crates/rbitcoin-node/tests/run.rs | 105 +++++++++++++---------- 6 files changed, 120 insertions(+), 59 deletions(-) diff --git a/changelog.d/proxy-seed-bootstrap-wait.md b/changelog.d/proxy-seed-bootstrap-wait.md index 1976f4ab14..46fc5abb4f 100644 --- a/changelog.d/proxy-seed-bootstrap-wait.md +++ b/changelog.d/proxy-seed-bootstrap-wait.md @@ -2,6 +2,7 @@ Fixed - **A fresh `--proxy` datadir waits for its seeds.** Catch-up read the dial targets before any SOCKS seed addr-fetch had answered, so a node with an - empty book shut down about 60 ms after start. It now waits until a seed - has answered and its session has ended, and re-queues the seeds every - 60 s until then. + empty book shut down about 60 ms after start. It now waits up to 180 s + for a seed's `getaddr` reply to be learned (a self-announce does not + count) and re-queues the seeds every 60 s while no addr-fetch session is + live. With no reply, the empty book takes the incomplete path as before. diff --git a/crates/rbitcoin-net/src/peer.rs b/crates/rbitcoin-net/src/peer.rs index 3a13a93e09..4921ae234a 100644 --- a/crates/rbitcoin-net/src/peer.rs +++ b/crates/rbitcoin-net/src/peer.rs @@ -3130,6 +3130,9 @@ fn learn_addrfetch_reply( let now_ms = s.clock_now().saturating_mul(1000); let allow = s.take_addr_tokens(list.len(), now_ms); ph.learn_addrv2(&list[..allow]); + if n > 1 { + ph.note_addrfetch_reply(); + } } if n > 1 { s.request_disconnect(); diff --git a/crates/rbitcoin-net/src/peers.rs b/crates/rbitcoin-net/src/peers.rs index e97c893779..e54e7b4f66 100644 --- a/crates/rbitcoin-net/src/peers.rs +++ b/crates/rbitcoin-net/src/peers.rs @@ -1466,6 +1466,9 @@ pub struct PeerHub { /// Outbound dials still before session registration, by target. The /// redial pass skips these, so a slow connect is not stacked every 2 s. pub dials_in_flight: Arc>>, + /// Addr-fetch `getaddr` replies (more than one address) learned into the + /// book. A self-announce does not count. + addrfetch_replies: AtomicU64, /// Network P2P port used when a remembered host omits `:port`. `0` = unset. pub connect_default_port: AtomicU16, pub dial_tx: Mutex>>, @@ -1588,6 +1591,7 @@ impl PeerHub { connect_hosts: Mutex::new(Vec::new()), connect_addrs: Mutex::new(Vec::new()), dials_in_flight: Arc::new(Mutex::new(HashMap::new())), + addrfetch_replies: AtomicU64::new(0), connect_default_port: AtomicU16::new(0), dial_tx: Mutex::new(None), hb_selected: Mutex::new(Vec::new()), @@ -1816,6 +1820,15 @@ impl PeerHub { /// /// Copies the I2P reachability bit onto that book. `learn_addrv2` stores /// only rows the book will dial. + /// How many addr-fetch `getaddr` replies have been learned so far. + pub fn addrfetch_replies(&self) -> u64 { + self.addrfetch_replies.load(Ordering::Acquire) + } + + pub(crate) fn note_addrfetch_reply(&self) { + self.addrfetch_replies.fetch_add(1, Ordering::Release); + } + pub fn set_addrman(&self, am: std::sync::Arc>) { let on = self.i2p_reachable.load(Ordering::Relaxed); am.lock() diff --git a/crates/rbitcoin-net/tests/peer/mod.rs b/crates/rbitcoin-net/tests/peer/mod.rs index 223eb56257..e8cb02b011 100644 --- a/crates/rbitcoin-net/tests/peer/mod.rs +++ b/crates/rbitcoin-net/tests/peer/mod.rs @@ -5132,6 +5132,11 @@ async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { ) .await; assert_eq!(book_len(), 1, "a self-announce is learned"); + assert_eq!( + peers.addrfetch_replies(), + 0, + "a self-announce is not the reply" + ); assert!( !seed.stop.load(Ordering::SeqCst), "one address is not the reply" @@ -5162,6 +5167,7 @@ async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { "the reply ends addr-fetch" ); assert_eq!(follow.ban_score, 0, "a getaddr reply is not misbehavior"); + assert_eq!(peers.addrfetch_replies(), 1); assert!( relayed.try_recv().is_err(), "an addr-fetch reply is not relayed" @@ -5189,6 +5195,7 @@ async fn addrfetch_learns_the_getaddr_reply_then_disconnects() { "a v1 reply ends addr-fetch" ); assert_eq!(follow.ban_score, 0, "a v1 getaddr reply is not misbehavior"); + assert_eq!(peers.addrfetch_replies(), 2); assert!( relayed.try_recv().is_err(), "a v1 addr-fetch reply is not relayed" diff --git a/crates/rbitcoin-node/src/run.rs b/crates/rbitcoin-node/src/run.rs index ba774470e8..fe72fe536c 100644 --- a/crates/rbitcoin-node/src/run.rs +++ b/crates/rbitcoin-node/src/run.rs @@ -692,7 +692,9 @@ pub async fn run_p2p(config: NodeConfig) -> Result<(), NodeError> { config.network, &shared_peers, &shutdown, + || node.peers.addrfetch_replies(), PROXY_SEED_RETRY, + PROXY_SEED_WAIT, ) .await { @@ -1611,36 +1613,56 @@ pub fn queue_proxy_seed_addrfetch(peers: &Arc, network: N /// queue, so the wait re-queues the seeds on this period. const PROXY_SEED_RETRY: Duration = Duration::from_secs(60); -/// The shared book once a `--proxy` seed addr-fetch answer fills it, or -/// `None` on shutdown. Re-queues the seeds every `retry` until then. A seed -/// announces itself before its `getaddr` reply, so the answer is complete -/// only when no addr-fetch session is still live. +/// How long catch-up waits for a `--proxy` seed answer. Past it the empty +/// book falls through to the `awaiting_dialable` path. +const PROXY_SEED_WAIT: Duration = Duration::from_secs(180); + +/// The shared book once an addr-fetch `getaddr` reply has been learned +/// since the wait began (`replies` counts them; a self-announce does not), +/// or `None` on shutdown or after `wait`. Re-queues the seeds every `retry` +/// while no addr-fetch session is live. #[doc(hidden)] pub async fn await_proxy_seed_book( peers: &Arc, network: Network, shared_peers: &Arc>, shutdown: &Shutdown, + replies: impl Fn() -> u64, retry: Duration, + wait: Duration, ) -> Option { + let start = replies(); + let deadline = tokio::time::sleep(wait); + tokio::pin!(deadline); let mut poll = tokio::time::interval(Duration::from_millis(250)); let mut requeue = tokio::time::interval_at(tokio::time::Instant::now() + retry, retry); loop { tokio::select! { biased; _ = shutdown.cancelled() => return None, - _ = requeue.tick() => { - let n = queue_proxy_seed_addrfetch(peers, network); - info!("ibd: no seed answer yet — re-queued {n} seed hostnames via SOCKS addrfetch"); + _ = &mut deadline => { + warn!( + "ibd: no SOCKS seed addrfetch answer in {}s — catch-up stays open", + wait.as_secs() + ); + return None; } - _ = poll.tick() => { + _ = requeue.tick() => { let answering = peers .snapshot() .iter() .any(|p| p.conn_type == PeerConnType::AddrFetch); - let book = shared_peers.lock().unwrap_or_else(|e| e.into_inner()); - if !book.is_empty() && !answering { - return Some(book.clone()); + if !answering { + let n = queue_proxy_seed_addrfetch(peers, network); + info!("ibd: no seed answer yet — re-queued {n} seed hostnames via SOCKS addrfetch"); + } + } + _ = poll.tick() => { + if replies() > start { + let book = shared_peers.lock().unwrap_or_else(|e| e.into_inner()); + if !book.is_empty() { + return Some(book.clone()); + } } } } diff --git a/crates/rbitcoin-node/tests/run.rs b/crates/rbitcoin-node/tests/run.rs index 050f71a8b5..cbc30bf545 100644 --- a/crates/rbitcoin-node/tests/run.rs +++ b/crates/rbitcoin-node/tests/run.rs @@ -695,39 +695,24 @@ async fn max_run_deadline_wakes_without_the_perf_tick() { assert!(start.elapsed() < Duration::from_millis(200)); } -/// A fresh `--proxy` datadir has no targets until a seed answers the -/// addr-fetch. Catch-up waits for that answer and re-queues the seeds. +/// A fresh `--proxy` datadir has no targets until a seed's `getaddr` +/// reply is learned. The wait re-queues the seeds while no addr-fetch +/// session is live, ignores a book filled only by a self-announce, and +/// ends on that reply, shutdown, or its deadline. #[tokio::test] -async fn proxy_seed_wait_requeues_until_an_answer_fills_the_book() { +async fn proxy_seed_wait_ends_on_a_learned_reply_shutdown_or_deadline() { + use std::sync::atomic::{AtomicU64, Ordering}; + let peers = rbitcoin_net::PeerHub::new(); let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); peers.set_dialer(tx); + let signet = rbitcoin_primitives::Network::Signet; let shared = Arc::new(std::sync::Mutex::new(AddrMan::new())); - let answered: rbitcoin_net::NetAddr = "1.2.3.4:38333".parse().unwrap(); - let seed = { - let shared = Arc::clone(&shared); - tokio::spawn(async move { - let req = rx.recv().await.expect("seed addr-fetch re-queued"); - assert_eq!(req.typ, PeerConnType::AddrFetch); - shared.lock().unwrap().add_addr(answered); - }) - }; + let announced: rbitcoin_net::NetAddr = "1.2.3.4:38333".parse().unwrap(); + shared.lock().unwrap().add_addr(announced); + let replies = Arc::new(AtomicU64::new(0)); let sd = Shutdown::new(); - let book = tokio::time::timeout( - Duration::from_secs(5), - await_proxy_seed_book( - &peers, - rbitcoin_primitives::Network::Signet, - &shared, - &sd, - Duration::from_millis(50), - ), - ) - .await - .expect("the wait ends once a seed answers") - .expect("no shutdown was requested"); - seed.await.unwrap(); - assert_eq!(book.len(), 1); + let short = Duration::from_millis(50); let at = std::net::SocketAddr::from(([127, 0, 0, 1], 38333)); let ver = bitcoin::p2p::message_network::VersionMessage { @@ -742,37 +727,67 @@ async fn proxy_seed_wait_requeues_until_an_answer_fills_the_book() { relay: true, }; let answering = peers.register(at, at, &ver, false, PeerConnType::AddrFetch); - let held = tokio::time::timeout( - Duration::from_millis(600), + let counted = Arc::clone(&replies); + let timed_out = await_proxy_seed_book( + &peers, + signet, + &shared, + &sd, + move || counted.load(Ordering::SeqCst), + short, + Duration::from_millis(400), + ) + .await; + assert!( + timed_out.is_none(), + "a self-announce alone is not the reply" + ); + assert!( + rx.try_recv().is_err(), + "a live addr-fetch session is not re-queued" + ); + peers.unregister(answering.id); + + let seed = { + let (shared, replies) = (Arc::clone(&shared), Arc::clone(&replies)); + tokio::spawn(async move { + let req = rx.recv().await.expect("seed addr-fetch re-queued"); + assert_eq!(req.typ, PeerConnType::AddrFetch); + shared + .lock() + .unwrap() + .add_addr("5.6.7.8:38333".parse().unwrap()); + replies.fetch_add(1, Ordering::SeqCst); + }) + }; + let counted = Arc::clone(&replies); + let book = tokio::time::timeout( + Duration::from_secs(5), await_proxy_seed_book( &peers, - rbitcoin_primitives::Network::Signet, + signet, &shared, &sd, + move || counted.load(Ordering::SeqCst), + short, Duration::from_secs(60), ), ) - .await; - assert!(held.is_err(), "a seed still answering holds the wait"); - peers.unregister(answering.id); - let book = await_proxy_seed_book( - &peers, - rbitcoin_primitives::Network::Signet, - &shared, - &sd, - Duration::from_secs(60), - ) .await - .expect("the answer is complete once the seed session ends"); - assert_eq!(book.len(), 1); + .expect("the wait ends once a reply is learned") + .expect("no shutdown and no deadline"); + seed.await.unwrap(); + assert_eq!(book.len(), 2); - let empty = Arc::new(std::sync::Mutex::new(AddrMan::new())); sd.request(); + let counted = Arc::clone(&replies); let book = await_proxy_seed_book( &peers, - rbitcoin_primitives::Network::Signet, - &empty, + signet, + &shared, &sd, + move || counted.load(Ordering::SeqCst), + Duration::from_secs(60), Duration::from_secs(60), ) .await;