net: learn an addr-fetch getaddr reply, then disconnect

changelog.d/addrfetch-learns-reply.md

@@ -0,0 +1,8 @@
+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 (`addrv2` or
+  v1 `addr`) is learned and not relayed, and more than one address ends the
+  session with a plain disconnect.

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(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)?,
@@ -3082,19 +3087,55 @@ 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(
     follow: &mut PeerFollowState,
     session: Option<&crate::peers::LivePeer>,
-    n: usize,
+    list: &[(u32, bitcoin::p2p::address::Address)],
 ) -> Result<(), NetError> {
-    if session.is_some_and(|s| s.conn_type == crate::peers::PeerConnType::AddrFetch && n > 1) {
-        punish_disconnect(&mut follow.ban_score, session);
+    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 `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();
+    }
+}
+
 #[doc(hidden)]
 pub fn on_addrv2(
     follow: &mut PeerFollowState,
@@ -3111,8 +3152,8 @@ 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);
+    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 {

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.

crates/rbitcoin-net/tests/peer/mod.rs

@@ -5075,3 +5075,123 @@ 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, 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};
+    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 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,
+            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, typ)
+    };
+    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: routable(i),
+            port: 18444,
+        })
+        .collect();
+    push(
+        &hub,
+        &out_tx,
+        &mut follow,
+        Some(seed.as_ref()),
+        NetworkMessage::AddrV2(reply),
+    )
+    .await;
+    let learned = book_len() - 1;
+    assert!(
+        learned >= MAX_ADDR_TO_SEND * 9 / 10,
+        "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");
+    assert!(
+        relayed.try_recv().is_err(),
+        "an addr-fetch reply is not relayed"
+    );
+
+    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![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());
+}