From 88ce191d403c65b36ca5442b7d5aac7e4d796481 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Mon, 7 Sep 2026 02:44:49 -0400 Subject: [PATCH 1/8] fix(node): bound Identify-derived Kademlia addresses. --- crates/gitlawb-node/src/p2p/mod.rs | 387 ++++++++++++++++++++++++++++- 1 file changed, 384 insertions(+), 3 deletions(-) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index 80e28a4a..0c553031 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -7,10 +7,10 @@ //! The node's PeerId is derived from its Ed25519 identity keypair, //! so the gitlawb DID and libp2p PeerId share the same key. -use std::collections::{hash_map::DefaultHasher, HashMap}; +use std::collections::{hash_map::DefaultHasher, HashMap, HashSet, VecDeque}; use std::hash::{Hash, Hasher}; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; use anyhow::Result; use chrono::Utc; @@ -30,6 +30,15 @@ use crate::db::{Db, ReceivedRefUpdate}; /// Topic for ref-update notifications published after every push. pub const REF_UPDATES_TOPIC: &str = "gitlawb/ref-updates/v1"; +// Identify is an untrusted address source. Keep enough entries for normal +// multi-homed peers while bounding both one peer and the whole routing table. +const IDENTIFY_ADDRESS_LIMIT: usize = 8; +const IDENTIFY_NEW_ADDRESS_LIMIT: usize = 8; +const IDENTIFY_GLOBAL_ADDRESS_LIMIT: usize = 1024; +const IDENTIFY_ADDRESS_WINDOW: Duration = Duration::from_secs(60); +const IDENTIFY_ADDRESS_TTL: Duration = Duration::from_secs(30 * 60); +const IDENTIFY_ADDRESS_CLEANUP_INTERVAL: Duration = Duration::from_secs(60); + /// A ref-update event published to Gossipsub when a push lands. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct RefUpdateEvent { @@ -155,6 +164,197 @@ fn did_to_kad_key(did: &str) -> kad::RecordKey { kad::RecordKey::new(&format!("/gitlawb/did/{did}").as_bytes()) } +#[derive(Debug)] +struct IdentifyAddress { + address: Multiaddr, + expires_at: Instant, +} + +#[derive(Debug)] +struct IdentifyPeerAddresses { + addresses: VecDeque, + window_started: Instant, + new_addresses: usize, +} + +#[derive(Debug, Default, PartialEq, Eq)] +struct IdentifyAddressChanges { + added: Vec, + removed: Vec<(PeerId, Multiaddr)>, +} + +#[derive(Debug, Default)] +struct IdentifyAddressBook { + peers: HashMap, + insertion_order: VecDeque<(PeerId, Multiaddr)>, + address_count: usize, +} + +impl IdentifyAddressBook { + fn update( + &mut self, + peer_id: PeerId, + now: Instant, + addresses: &[Multiaddr], + ) -> IdentifyAddressChanges { + let mut changes = IdentifyAddressChanges::default(); + if self.peers.contains_key(&peer_id) { + changes.removed.extend(self.expire_peer(peer_id, now)); + } + self.peers + .entry(peer_id) + .or_insert_with(|| IdentifyPeerAddresses { + addresses: VecDeque::new(), + window_started: now, + new_addresses: 0, + }); + + if let Some(state) = self.peers.get_mut(&peer_id) { + if now.saturating_duration_since(state.window_started) >= IDENTIFY_ADDRESS_WINDOW { + state.window_started = now; + state.new_addresses = 0; + } + } + + for address in addresses { + let Some(address) = address.clone().with_p2p(peer_id).ok() else { + continue; + }; + + if let Some(existing) = self.peers.get_mut(&peer_id).and_then(|state| { + state + .addresses + .iter_mut() + .find(|existing| existing.address == address) + }) { + existing.expires_at = now + IDENTIFY_ADDRESS_TTL; + continue; + } + + if self + .peers + .get(&peer_id) + .is_some_and(|state| state.new_addresses >= IDENTIFY_NEW_ADDRESS_LIMIT) + { + continue; + } + + if self.address_count >= IDENTIFY_GLOBAL_ADDRESS_LIMIT { + let Some(evicted) = self.evict_oldest(peer_id) else { + continue; + }; + changes.removed.push(evicted); + } + + let evicted = self.peers.get_mut(&peer_id).and_then(|state| { + state.new_addresses += 1; + if state.addresses.len() >= IDENTIFY_ADDRESS_LIMIT { + state.addresses.pop_front() + } else { + None + } + }); + if let Some(evicted) = evicted { + self.remove_insertion_token(peer_id, &evicted.address); + self.address_count -= 1; + changes.removed.push((peer_id, evicted.address)); + } + + if let Some(state) = self.peers.get_mut(&peer_id) { + state.addresses.push_back(IdentifyAddress { + address: address.clone(), + expires_at: now + IDENTIFY_ADDRESS_TTL, + }); + self.insertion_order.push_back((peer_id, address.clone())); + self.address_count += 1; + changes.added.push(address); + } + } + + changes + } + + fn expire(&mut self, now: Instant) -> Vec<(PeerId, Multiaddr)> { + let mut removed = Vec::new(); + let peers = self.peers.keys().copied().collect::>(); + for peer_id in peers { + removed.extend(self.expire_peer(peer_id, now)); + } + removed + } + + fn expire_peer(&mut self, peer_id: PeerId, now: Instant) -> Vec<(PeerId, Multiaddr)> { + let expired = self + .peers + .get_mut(&peer_id) + .map(|state| Self::expire_state(state, now)) + .unwrap_or_default(); + for address in &expired { + self.remove_insertion_token(peer_id, address); + self.address_count -= 1; + } + if self + .peers + .get(&peer_id) + .is_some_and(|state| state.addresses.is_empty()) + { + self.peers.remove(&peer_id); + } + expired + .into_iter() + .map(|address| (peer_id, address)) + .collect() + } + + fn expire_state(state: &mut IdentifyPeerAddresses, now: Instant) -> Vec { + let mut expired = Vec::new(); + let mut retained = VecDeque::with_capacity(state.addresses.len()); + while let Some(address) = state.addresses.pop_front() { + if address.expires_at <= now { + expired.push(address.address); + } else { + retained.push_back(address); + } + } + state.addresses = retained; + expired + } + + fn evict_oldest(&mut self, preserve_peer: PeerId) -> Option<(PeerId, Multiaddr)> { + while let Some((peer_id, address)) = self.insertion_order.pop_front() { + let Some(state) = self.peers.get_mut(&peer_id) else { + continue; + }; + let Some(index) = state + .addresses + .iter() + .position(|tracked| tracked.address == address) + else { + continue; + }; + let evicted = state.addresses.remove(index)?.address; + self.address_count -= 1; + if state.addresses.is_empty() && peer_id != preserve_peer { + self.peers.remove(&peer_id); + } + return Some((peer_id, evicted)); + } + None + } + + fn remove_insertion_token(&mut self, peer_id: PeerId, address: &Multiaddr) { + if let Some(index) = self + .insertion_order + .iter() + .position(|(token_peer, token_address)| { + token_peer == &peer_id && token_address == address + }) + { + self.insertion_order.remove(index); + } + } +} + /// Combined libp2p behaviour. #[derive(NetworkBehaviour)] #[behaviour(prelude = "libp2p_swarm::derive_prelude")] @@ -275,12 +475,30 @@ pub async fn start( // Track in-flight GetRecord queries → reply channels let mut pending_get_did: HashMap>> = HashMap::new(); + // Keep Identify-derived entries across disconnects so a bootstrap address + // learned through Identify remains usable for redial. The TTL cleanup below + // removes entries that are no longer refreshed without touching explicit + // AddKnownPeer addresses. + let mut identify_addresses = IdentifyAddressBook::default(); + let mut explicit_addresses: HashMap> = HashMap::new(); + let mut identify_cleanup = tokio::time::interval(IDENTIFY_ADDRESS_CLEANUP_INTERVAL); + identify_cleanup.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); // Start the event loop as a background task tokio::spawn(async move { let mut shutdown_rx = shutdown_rx; loop { tokio::select! { + _ = identify_cleanup.tick() => { + for (peer_id, address) in identify_addresses.expire(Instant::now()) { + if !explicit_addresses + .get(&peer_id) + .is_some_and(|addresses| addresses.contains(&address)) + { + swarm.behaviour_mut().kademlia.remove_address(&peer_id, &address); + } + } + } // Graceful shutdown: exit the swarm loop when the // process-wide signal flips. This drops the Swarm // which closes all libp2p connections cleanly. @@ -366,7 +584,23 @@ pub async fn start( identify::Event::Received { peer_id, info, .. } )) => { debug!(peer = %peer_id, "identify received"); - for addr in info.listen_addrs { + let changes = identify_addresses.update( + peer_id, + Instant::now(), + &info.listen_addrs, + ); + for (removed_peer, addr) in changes.removed { + if !explicit_addresses + .get(&removed_peer) + .is_some_and(|addresses| addresses.contains(&addr)) + { + swarm + .behaviour_mut() + .kademlia + .remove_address(&removed_peer, &addr); + } + } + for addr in changes.added { swarm.behaviour_mut().kademlia.add_address(&peer_id, addr); } } @@ -392,6 +626,13 @@ pub async fn start( } } P2pCommand::AddKnownPeer { peer_id, addr } => { + let Ok(addr) = addr.with_p2p(peer_id) else { + continue; + }; + explicit_addresses + .entry(peer_id) + .or_default() + .insert(addr.clone()); swarm.behaviour_mut().kademlia.add_address(&peer_id, addr); } P2pCommand::Dial(addr) => { @@ -443,6 +684,146 @@ pub async fn start( mod tests { use super::*; + fn identify_address(peer_id: PeerId, octet: u8, port: u16) -> Multiaddr { + format!("/ip4/192.0.2.{octet}/udp/{port}/quic-v1") + .parse::() + .unwrap() + .with_p2p(peer_id) + .unwrap() + } + + fn identify_address_without_peer(octet: u8, port: u16) -> Multiaddr { + format!("/ip4/192.0.2.{octet}/udp/{port}/quic-v1") + .parse::() + .unwrap() + } + + #[test] + fn identify_addresses_are_capped_with_fifo_eviction() { + let peer_id = PeerId::random(); + let now = Instant::now(); + let initial = (0..IDENTIFY_ADDRESS_LIMIT) + .map(|index| identify_address(peer_id, index as u8 + 1, 10_000 + index as u16)) + .collect::>(); + let replacement = identify_address(peer_id, 200, 20_000); + let mut book = IdentifyAddressBook::default(); + + let first = book.update(peer_id, now, &initial); + assert_eq!(first.added, initial); + assert!(first.removed.is_empty()); + + let second = book.update( + peer_id, + now + IDENTIFY_ADDRESS_WINDOW, + std::slice::from_ref(&replacement), + ); + assert_eq!(second.added, vec![replacement]); + assert_eq!(second.removed, vec![(peer_id, initial[0].clone())]); + } + + #[test] + fn identify_addresses_rate_limit_new_entries_across_updates() { + let peer_id = PeerId::random(); + let now = Instant::now(); + let initial = (0..IDENTIFY_NEW_ADDRESS_LIMIT) + .map(|index| identify_address(peer_id, index as u8 + 1, 10_000 + index as u16)) + .collect::>(); + let replacement = identify_address(peer_id, 200, 20_000); + let mut book = IdentifyAddressBook::default(); + + assert_eq!(book.update(peer_id, now, &initial).added, initial); + assert!(book + .update(peer_id, now, std::slice::from_ref(&replacement)) + .added + .is_empty()); + assert_eq!( + book.update( + peer_id, + now + IDENTIFY_ADDRESS_WINDOW, + std::slice::from_ref(&replacement), + ) + .added, + vec![replacement] + ); + } + + #[test] + fn identify_addresses_have_a_global_budget_across_peers() { + let now = Instant::now(); + let mut book = IdentifyAddressBook::default(); + let mut first = None; + + for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT { + let peer_id = PeerId::random(); + let address = identify_address(peer_id, (index % 254) as u8 + 1, 10_000 + index as u16); + if first.is_none() { + first = Some((peer_id, address.clone())); + } + assert!(book.update(peer_id, now, &[address]).removed.is_empty()); + } + + let (peer_id, first_address) = first.unwrap(); + let address = identify_address(peer_id, 250, 30_000); + let changes = book.update( + peer_id, + now + IDENTIFY_ADDRESS_WINDOW, + std::slice::from_ref(&address), + ); + assert_eq!(changes.added, vec![address]); + assert_eq!(changes.removed, vec![(peer_id, first_address)]); + } + + #[test] + fn identify_addresses_refresh_and_expire() { + let peer_id = PeerId::random(); + let now = Instant::now(); + let address = identify_address(peer_id, 1, 10_000); + let mut book = IdentifyAddressBook::default(); + + assert_eq!( + book.update(peer_id, now, std::slice::from_ref(&address)) + .added, + vec![address.clone()] + ); + let refreshed = now + IDENTIFY_ADDRESS_TTL - Duration::from_secs(1); + let refresh = book.update(peer_id, refreshed, std::slice::from_ref(&address)); + assert!(refresh.added.is_empty()); + assert!(refresh.removed.is_empty()); + assert!(book + .expire(now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)) + .is_empty()); + assert_eq!( + book.expire(refreshed + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)), + vec![(peer_id, address)] + ); + } + + #[test] + fn identify_addresses_reject_foreign_peer_suffixes() { + let peer_id = PeerId::random(); + let other_peer_id = PeerId::random(); + let address = identify_address(other_peer_id, 1, 10_000); + let mut book = IdentifyAddressBook::default(); + + assert!(book + .update(peer_id, Instant::now(), &[address]) + .added + .is_empty()); + } + + #[test] + fn identify_addresses_canonicalize_the_peer_suffix_once() { + let peer_id = PeerId::random(); + let raw = identify_address_without_peer(1, 10_000); + let canonical = identify_address(peer_id, 1, 10_000); + let mut book = IdentifyAddressBook::default(); + + assert_eq!( + book.update(peer_id, Instant::now(), &[raw]).added, + vec![canonical] + ); + } + #[test] fn ref_update_event_round_trip_with_owner_did() { let event = RefUpdateEvent { From 618d60739a9cdecf3da50da89be8f72b6a120928 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Mon, 7 Sep 2026 02:56:16 -0400 Subject: [PATCH 2/8] fix(node): preserve refreshed Identify addresses. --- crates/gitlawb-node/src/p2p/mod.rs | 115 ++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 25 deletions(-) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index 0c553031..4f6e79a5 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -216,48 +216,84 @@ impl IdentifyAddressBook { } } - for address in addresses { - let Some(address) = address.clone().with_p2p(peer_id).ok() else { - continue; - }; + let reported = addresses + .iter() + .filter_map(|address| address.clone().with_p2p(peer_id).ok()) + .collect::>(); - if let Some(existing) = self.peers.get_mut(&peer_id).and_then(|state| { - state - .addresses - .iter_mut() - .find(|existing| existing.address == address) - }) { - existing.expires_at = now + IDENTIFY_ADDRESS_TTL; - continue; + // Refresh every address in the report before admitting anything new. + // This prevents an oversized report from evicting an address merely + // because it appeared later in the input. + if let Some(state) = self.peers.get_mut(&peer_id) { + for existing in &mut state.addresses { + if reported.contains(&existing.address) { + existing.expires_at = now + IDENTIFY_ADDRESS_TTL; + } } + } + + let mut new_addresses = self + .peers + .get(&peer_id) + .map(|state| { + reported + .iter() + .filter(|address| { + !state + .addresses + .iter() + .any(|existing| &existing.address == *address) + }) + .cloned() + .collect::>() + }) + .unwrap_or_else(|| reported.iter().cloned().collect()); + // HashSet iteration is deliberately unordered. Sort new candidates so + // an oversized Identify report has deterministic admission behavior. + new_addresses.sort(); + for address in new_addresses { if self .peers .get(&peer_id) .is_some_and(|state| state.new_addresses >= IDENTIFY_NEW_ADDRESS_LIMIT) { - continue; - } - - if self.address_count >= IDENTIFY_GLOBAL_ADDRESS_LIMIT { - let Some(evicted) = self.evict_oldest(peer_id) else { - continue; - }; - changes.removed.push(evicted); + break; } let evicted = self.peers.get_mut(&peer_id).and_then(|state| { - state.new_addresses += 1; - if state.addresses.len() >= IDENTIFY_ADDRESS_LIMIT { - state.addresses.pop_front() - } else { - None + if state.addresses.len() < IDENTIFY_ADDRESS_LIMIT { + return None; } + let index = state + .addresses + .iter() + .position(|existing| !reported.contains(&existing.address))?; + state.addresses.remove(index) }); if let Some(evicted) = evicted { self.remove_insertion_token(peer_id, &evicted.address); self.address_count -= 1; changes.removed.push((peer_id, evicted.address)); + } else if self + .peers + .get(&peer_id) + .is_some_and(|state| state.addresses.len() >= IDENTIFY_ADDRESS_LIMIT) + { + // Every retained address was refreshed in this report. Keep + // that stable subset and drop surplus new candidates. + break; + } + + if self.address_count >= IDENTIFY_GLOBAL_ADDRESS_LIMIT { + let Some(evicted) = self.evict_oldest(peer_id) else { + break; + }; + changes.removed.push(evicted); + } + + if let Some(state) = self.peers.get_mut(&peer_id) { + state.new_addresses += 1; } if let Some(state) = self.peers.get_mut(&peer_id) { @@ -721,6 +757,35 @@ mod tests { assert_eq!(second.removed, vec![(peer_id, initial[0].clone())]); } + #[test] + fn identify_oversized_reports_keep_the_same_refreshed_subset() { + let peer_id = PeerId::random(); + let now = Instant::now(); + let mut report = (0..IDENTIFY_ADDRESS_LIMIT + 2) + .map(|index| identify_address(peer_id, index as u8 + 1, 10_000 + index as u16)) + .collect::>(); + let mut book = IdentifyAddressBook::default(); + + let first = book.update(peer_id, now, &report); + assert_eq!(first.added.len(), IDENTIFY_ADDRESS_LIMIT); + assert_eq!(first.removed.len(), 0); + let retained = first.added.clone(); + + report.reverse(); + let second = book.update(peer_id, now + IDENTIFY_ADDRESS_WINDOW, &report); + assert!(second.added.is_empty()); + assert!(second.removed.is_empty()); + let current = book + .peers + .get(&peer_id) + .unwrap() + .addresses + .iter() + .map(|entry| entry.address.clone()) + .collect::>(); + assert_eq!(current, retained); + } + #[test] fn identify_addresses_rate_limit_new_entries_across_updates() { let peer_id = PeerId::random(); From 0ffdbdb4e3a252c168e7c89795d0b97a507ab0d1 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Tue, 8 Sep 2026 20:32:58 -0400 Subject: [PATCH 3/8] fix(node): Log rejected known-peer addresses for diagnosis. --- crates/gitlawb-node/src/p2p/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index 4f6e79a5..bfb33081 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -662,7 +662,8 @@ pub async fn start( } } P2pCommand::AddKnownPeer { peer_id, addr } => { - let Ok(addr) = addr.with_p2p(peer_id) else { + let Ok(addr) = addr.clone().with_p2p(peer_id) else { + warn!(%peer_id, %addr, "dropping known-peer address with a foreign peer suffix"); continue; }; explicit_addresses From d17601ba6d2fb8d8aad7e53621fb4a9532fbbac6 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Wed, 9 Sep 2026 01:29:07 -0400 Subject: [PATCH 4/8] fix(node): bound Identify report processing and refresh eviction Avoid retaining peers with empty or rejected address reports, bound canonicalization to a fixed input prefix, and move refreshed addresses behind stale addresses in global eviction order. Add regressions for each bound and below-cap growth. --- crates/gitlawb-node/src/p2p/mod.rs | 114 +++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 8 deletions(-) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index bfb33081..525195f1 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -33,6 +33,8 @@ pub const REF_UPDATES_TOPIC: &str = "gitlawb/ref-updates/v1"; // Identify is an untrusted address source. Keep enough entries for normal // multi-homed peers while bounding both one peer and the whole routing table. const IDENTIFY_ADDRESS_LIMIT: usize = 8; +// Bound canonicalization and sorting too, including rejected/duplicate entries. +const IDENTIFY_REPORT_ADDRESS_LIMIT: usize = 64; const IDENTIFY_NEW_ADDRESS_LIMIT: usize = 8; const IDENTIFY_GLOBAL_ADDRESS_LIMIT: usize = 1024; const IDENTIFY_ADDRESS_WINDOW: Duration = Duration::from_secs(60); @@ -201,6 +203,14 @@ impl IdentifyAddressBook { if self.peers.contains_key(&peer_id) { changes.removed.extend(self.expire_peer(peer_id, now)); } + let reported = addresses + .iter() + .take(IDENTIFY_REPORT_ADDRESS_LIMIT) + .filter_map(|address| address.clone().with_p2p(peer_id).ok()) + .collect::>(); + if reported.is_empty() && !self.peers.contains_key(&peer_id) { + return changes; + } self.peers .entry(peer_id) .or_insert_with(|| IdentifyPeerAddresses { @@ -216,21 +226,22 @@ impl IdentifyAddressBook { } } - let reported = addresses - .iter() - .filter_map(|address| address.clone().with_p2p(peer_id).ok()) - .collect::>(); - - // Refresh every address in the report before admitting anything new. - // This prevents an oversized report from evicting an address merely - // because it appeared later in the input. + // Refresh every retained address in the bounded report before admission. + let mut refreshed = Vec::new(); if let Some(state) = self.peers.get_mut(&peer_id) { for existing in &mut state.addresses { if reported.contains(&existing.address) { existing.expires_at = now + IDENTIFY_ADDRESS_TTL; + refreshed.push(existing.address.clone()); } } } + // Global eviction follows refresh recency, so an active address does + // not lose its slot just because it was originally admitted first. + for address in refreshed { + self.remove_insertion_token(peer_id, &address); + self.insertion_order.push_back((peer_id, address)); + } let mut new_addresses = self .peers @@ -735,6 +746,93 @@ mod tests { .unwrap() } + #[test] + fn identify_empty_reports_do_not_retain_peer_entries() { + let now = Instant::now(); + let mut book = IdentifyAddressBook::default(); + let foreign = identify_address(PeerId::random(), 1, 10_000); + for _ in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT + 1 { + let peer = PeerId::random(); + assert_eq!( + book.update(peer, now, &[]), + IdentifyAddressChanges::default() + ); + assert_eq!( + book.update(peer, now, std::slice::from_ref(&foreign)), + IdentifyAddressChanges::default() + ); + } + assert!(book.peers.is_empty()); + assert!(book.insertion_order.is_empty()); + assert_eq!(book.address_count, 0); + } + + #[test] + fn identify_report_processing_stops_at_the_input_limit() { + let peer = PeerId::random(); + let now = Instant::now(); + let foreign = identify_address(PeerId::random(), 1, 10_000); + let accepted = identify_address(peer, 2, 10_001); + let ignored = identify_address(peer, 3, 10_002); + let mut report = vec![foreign; IDENTIFY_REPORT_ADDRESS_LIMIT - 1]; + report.push(accepted.clone()); + report.push(ignored); + let mut book = IdentifyAddressBook::default(); + let changes = book.update(peer, now, &report); + assert_eq!(changes.added, vec![accepted]); + assert!(changes.removed.is_empty()); + assert_eq!(book.address_count, 1); + } + + #[test] + fn identify_addresses_grow_without_eviction_below_the_peer_cap() { + let peer = PeerId::random(); + let now = Instant::now(); + let first = identify_address(peer, 1, 10_000); + let second = identify_address(peer, 2, 10_001); + let mut book = IdentifyAddressBook::default(); + book.update(peer, now, std::slice::from_ref(&first)); + let changes = book.update( + peer, + now + IDENTIFY_ADDRESS_WINDOW, + std::slice::from_ref(&second), + ); + assert_eq!(changes.added, vec![second]); + assert!(changes.removed.is_empty()); + assert_eq!(book.peers[&peer].addresses.len(), 2); + assert_eq!(book.address_count, 2); + } + + #[test] + fn identify_global_eviction_preserves_refreshed_addresses() { + let now = Instant::now(); + let mut book = IdentifyAddressBook::default(); + let peers = (0..IDENTIFY_GLOBAL_ADDRESS_LIMIT) + .map(|index| { + let peer = PeerId::random(); + let address = identify_address(peer, 1, 10_000 + index as u16); + book.update(peer, now, std::slice::from_ref(&address)); + (peer, address) + }) + .collect::>(); + let (peer, refreshed) = &peers[0]; + let new_address = identify_address(*peer, 2, 30_000); + let changes = book.update( + *peer, + now + IDENTIFY_ADDRESS_WINDOW, + &[new_address.clone(), refreshed.clone()], + ); + assert_eq!(changes.added, vec![new_address]); + assert_eq!(changes.removed, vec![peers[1].clone()]); + assert!(book.peers[peer] + .addresses + .iter() + .any(|a| &a.address == refreshed)); + assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); + assert_eq!(book.insertion_order.len(), book.address_count); + assert_eq!(book.peers.len(), IDENTIFY_GLOBAL_ADDRESS_LIMIT - 1); + } + #[test] fn identify_addresses_are_capped_with_fifo_eviction() { let peer_id = PeerId::random(); From 31f79ed2875cc667b740d46893e7f9ce6c872e13 Mon Sep 17 00:00:00 2001 From: Marlowe <321669285+cairn-intern@users.noreply.github.com> Date: Thu, 24 Sep 2026 07:58:34 -0400 Subject: [PATCH 5/8] fix(node): address CodeRabbit majors on Identify address bounds - Reject Identify address admissions once the global address budget is full instead of evicting another peer's entry. Cross-peer eviction let a flood of new identities push honest peers' addresses out of the book and, when the evicted address was their last one, out of the Kademlia routing table. Per-peer eviction of the reporting peer's own addresses is unchanged. - Re-add refreshed Identify addresses to Kademlia, not just newly added ones. Kademlia may have dropped a retained address after a failed dial or never kept it for a full k-bucket, and a TTL refresh alone would never re-admit it. --- crates/gitlawb-node/src/p2p/mod.rs | 87 ++++++++++++++++-------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index 525195f1..f59bf55b 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -182,6 +182,7 @@ struct IdentifyPeerAddresses { #[derive(Debug, Default, PartialEq, Eq)] struct IdentifyAddressChanges { added: Vec, + refreshed: Vec, removed: Vec<(PeerId, Multiaddr)>, } @@ -236,11 +237,14 @@ impl IdentifyAddressBook { } } } - // Global eviction follows refresh recency, so an active address does - // not lose its slot just because it was originally admitted first. + // Record refreshed addresses so the Identify handler can re-admit + // them to Kademlia. Kademlia may have dropped a retained address + // after a failed dial or never kept it for a full k-bucket, and a + // TTL refresh alone would never re-add it. for address in refreshed { self.remove_insertion_token(peer_id, &address); - self.insertion_order.push_back((peer_id, address)); + self.insertion_order.push_back((peer_id, address.clone())); + changes.refreshed.push(address); } let mut new_addresses = self @@ -297,10 +301,13 @@ impl IdentifyAddressBook { } if self.address_count >= IDENTIFY_GLOBAL_ADDRESS_LIMIT { - let Some(evicted) = self.evict_oldest(peer_id) else { - break; - }; - changes.removed.push(evicted); + // Reject the admission instead of evicting another peer's + // entry: cross-peer eviction lets a flood of new identities + // push honest peers' addresses out of the book and, when the + // evicted address is their last one, out of the Kademlia + // routing table. Per-peer eviction above still applies to the + // reporting peer's own addresses. + break; } if let Some(state) = self.peers.get_mut(&peer_id) { @@ -367,28 +374,6 @@ impl IdentifyAddressBook { expired } - fn evict_oldest(&mut self, preserve_peer: PeerId) -> Option<(PeerId, Multiaddr)> { - while let Some((peer_id, address)) = self.insertion_order.pop_front() { - let Some(state) = self.peers.get_mut(&peer_id) else { - continue; - }; - let Some(index) = state - .addresses - .iter() - .position(|tracked| tracked.address == address) - else { - continue; - }; - let evicted = state.addresses.remove(index)?.address; - self.address_count -= 1; - if state.addresses.is_empty() && peer_id != preserve_peer { - self.peers.remove(&peer_id); - } - return Some((peer_id, evicted)); - } - None - } - fn remove_insertion_token(&mut self, peer_id: PeerId, address: &Multiaddr) { if let Some(index) = self .insertion_order @@ -647,7 +632,7 @@ pub async fn start( .remove_address(&removed_peer, &addr); } } - for addr in changes.added { + for addr in changes.refreshed.into_iter().chain(changes.added) { swarm.behaviour_mut().kademlia.add_address(&peer_id, addr); } } @@ -804,7 +789,7 @@ mod tests { } #[test] - fn identify_global_eviction_preserves_refreshed_addresses() { + fn identify_global_budget_rejects_new_admissions() { let now = Instant::now(); let mut book = IdentifyAddressBook::default(); let peers = (0..IDENTIFY_GLOBAL_ADDRESS_LIMIT) @@ -822,15 +807,23 @@ mod tests { now + IDENTIFY_ADDRESS_WINDOW, &[new_address.clone(), refreshed.clone()], ); - assert_eq!(changes.added, vec![new_address]); - assert_eq!(changes.removed, vec![peers[1].clone()]); - assert!(book.peers[peer] - .addresses - .iter() - .any(|a| &a.address == refreshed)); + // The budget is full: the new address is rejected rather than + // evicting another peer's entry, and the refreshed address keeps its + // slot and is reported for re-admission to Kademlia. + assert!(changes.added.is_empty()); + assert!(changes.removed.is_empty()); + assert_eq!(changes.refreshed, vec![(*refreshed).clone()]); + assert_eq!(book.peers.len(), IDENTIFY_GLOBAL_ADDRESS_LIMIT); assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); assert_eq!(book.insertion_order.len(), book.address_count); - assert_eq!(book.peers.len(), IDENTIFY_GLOBAL_ADDRESS_LIMIT - 1); + assert_eq!( + book.peers[peer] + .addresses + .iter() + .map(|a| &a.address) + .collect::>(), + vec![refreshed] + ); } #[test] @@ -933,8 +926,19 @@ mod tests { now + IDENTIFY_ADDRESS_WINDOW, std::slice::from_ref(&address), ); - assert_eq!(changes.added, vec![address]); - assert_eq!(changes.removed, vec![(peer_id, first_address)]); + // The global budget is full, so the admission is rejected and the + // peer's existing entry is left alone. + assert!(changes.added.is_empty()); + assert!(changes.removed.is_empty()); + assert_eq!( + book.peers[&peer_id] + .addresses + .iter() + .map(|a| &a.address) + .collect::>(), + vec![&first_address] + ); + assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); } #[test] @@ -953,6 +957,9 @@ mod tests { let refresh = book.update(peer_id, refreshed, std::slice::from_ref(&address)); assert!(refresh.added.is_empty()); assert!(refresh.removed.is_empty()); + // Refreshed addresses are reported so the Identify handler re-adds + // them to Kademlia even though they were not newly admitted. + assert_eq!(refresh.refreshed, vec![address.clone()]); assert!(book .expire(now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)) .is_empty()); From 1ef3f1995f56467f924e7f4d581b68f4aceb560c Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Fri, 25 Sep 2026 00:44:00 -0400 Subject: [PATCH 6/8] fix(node): address #462 review findings from beardthelion - remove the write-only insertion_order bookkeeping (field, remove_insertion_token, and its call sites) and collapse the repeated peers.get/get_mut lookups in IdentifyAddressBook::update into a single entry binding - drop a peer row at the end of update() when every candidate was rejected, so zero-address rows no longer linger until the expiry tick - add a regression test proving per-peer eviction still applies at full global budget (fails if the global check is hoisted above eviction) - extract the explicit-address retention predicate into the tested is_explicit_address helper used by both Kademlia removal sites --- crates/gitlawb-node/src/p2p/mod.rs | 204 ++++++++++++++++------------- 1 file changed, 113 insertions(+), 91 deletions(-) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index f59bf55b..46302937 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -166,6 +166,19 @@ fn did_to_kad_key(did: &str) -> kad::RecordKey { kad::RecordKey::new(&format!("/gitlawb/did/{did}").as_bytes()) } +/// Returns true when `address` was explicitly configured for `peer` through +/// AddKnownPeer. Explicit addresses are retained in Kademlia even when an +/// overlapping Identify lease expires or is evicted. +fn is_explicit_address( + explicit_addresses: &HashMap>, + peer: &PeerId, + address: &Multiaddr, +) -> bool { + explicit_addresses + .get(peer) + .is_some_and(|addresses| addresses.contains(address)) +} + #[derive(Debug)] struct IdentifyAddress { address: Multiaddr, @@ -189,7 +202,6 @@ struct IdentifyAddressChanges { #[derive(Debug, Default)] struct IdentifyAddressBook { peers: HashMap, - insertion_order: VecDeque<(PeerId, Multiaddr)>, address_count: usize, } @@ -212,7 +224,8 @@ impl IdentifyAddressBook { if reported.is_empty() && !self.peers.contains_key(&peer_id) { return changes; } - self.peers + let state = self + .peers .entry(peer_id) .or_insert_with(|| IdentifyPeerAddresses { addresses: VecDeque::new(), @@ -220,81 +233,55 @@ impl IdentifyAddressBook { new_addresses: 0, }); - if let Some(state) = self.peers.get_mut(&peer_id) { - if now.saturating_duration_since(state.window_started) >= IDENTIFY_ADDRESS_WINDOW { - state.window_started = now; - state.new_addresses = 0; - } + if now.saturating_duration_since(state.window_started) >= IDENTIFY_ADDRESS_WINDOW { + state.window_started = now; + state.new_addresses = 0; } // Refresh every retained address in the bounded report before admission. - let mut refreshed = Vec::new(); - if let Some(state) = self.peers.get_mut(&peer_id) { - for existing in &mut state.addresses { - if reported.contains(&existing.address) { - existing.expires_at = now + IDENTIFY_ADDRESS_TTL; - refreshed.push(existing.address.clone()); - } - } - } - // Record refreshed addresses so the Identify handler can re-admit + // Refreshed addresses are reported so the Identify handler can re-admit // them to Kademlia. Kademlia may have dropped a retained address // after a failed dial or never kept it for a full k-bucket, and a // TTL refresh alone would never re-add it. - for address in refreshed { - self.remove_insertion_token(peer_id, &address); - self.insertion_order.push_back((peer_id, address.clone())); - changes.refreshed.push(address); + for existing in &mut state.addresses { + if reported.contains(&existing.address) { + existing.expires_at = now + IDENTIFY_ADDRESS_TTL; + changes.refreshed.push(existing.address.clone()); + } } - let mut new_addresses = self - .peers - .get(&peer_id) - .map(|state| { - reported + let mut new_addresses: Vec = reported + .iter() + .filter(|address| { + !state + .addresses .iter() - .filter(|address| { - !state - .addresses - .iter() - .any(|existing| &existing.address == *address) - }) - .cloned() - .collect::>() + .any(|existing| &existing.address == *address) }) - .unwrap_or_else(|| reported.iter().cloned().collect()); + .cloned() + .collect(); // HashSet iteration is deliberately unordered. Sort new candidates so // an oversized Identify report has deterministic admission behavior. new_addresses.sort(); for address in new_addresses { - if self - .peers - .get(&peer_id) - .is_some_and(|state| state.new_addresses >= IDENTIFY_NEW_ADDRESS_LIMIT) - { + if state.new_addresses >= IDENTIFY_NEW_ADDRESS_LIMIT { break; } - let evicted = self.peers.get_mut(&peer_id).and_then(|state| { - if state.addresses.len() < IDENTIFY_ADDRESS_LIMIT { - return None; - } - let index = state + let evicted = if state.addresses.len() < IDENTIFY_ADDRESS_LIMIT { + None + } else { + state .addresses .iter() - .position(|existing| !reported.contains(&existing.address))?; - state.addresses.remove(index) - }); + .position(|existing| !reported.contains(&existing.address)) + .and_then(|index| state.addresses.remove(index)) + }; if let Some(evicted) = evicted { - self.remove_insertion_token(peer_id, &evicted.address); self.address_count -= 1; changes.removed.push((peer_id, evicted.address)); - } else if self - .peers - .get(&peer_id) - .is_some_and(|state| state.addresses.len() >= IDENTIFY_ADDRESS_LIMIT) - { + } else if state.addresses.len() >= IDENTIFY_ADDRESS_LIMIT { // Every retained address was refreshed in this report. Keep // that stable subset and drop surplus new candidates. break; @@ -310,19 +297,20 @@ impl IdentifyAddressBook { break; } - if let Some(state) = self.peers.get_mut(&peer_id) { - state.new_addresses += 1; - } + state.new_addresses += 1; + state.addresses.push_back(IdentifyAddress { + address: address.clone(), + expires_at: now + IDENTIFY_ADDRESS_TTL, + }); + self.address_count += 1; + changes.added.push(address); + } - if let Some(state) = self.peers.get_mut(&peer_id) { - state.addresses.push_back(IdentifyAddress { - address: address.clone(), - expires_at: now + IDENTIFY_ADDRESS_TTL, - }); - self.insertion_order.push_back((peer_id, address.clone())); - self.address_count += 1; - changes.added.push(address); - } + // Do not keep a row when every candidate was rejected: a zero-address + // row is invisible to address_count and would otherwise linger until + // the expiry tick. This matches expire_peer, which drops empty rows. + if state.addresses.is_empty() { + self.peers.remove(&peer_id); } changes @@ -343,10 +331,7 @@ impl IdentifyAddressBook { .get_mut(&peer_id) .map(|state| Self::expire_state(state, now)) .unwrap_or_default(); - for address in &expired { - self.remove_insertion_token(peer_id, address); - self.address_count -= 1; - } + self.address_count -= expired.len(); if self .peers .get(&peer_id) @@ -373,18 +358,6 @@ impl IdentifyAddressBook { state.addresses = retained; expired } - - fn remove_insertion_token(&mut self, peer_id: PeerId, address: &Multiaddr) { - if let Some(index) = self - .insertion_order - .iter() - .position(|(token_peer, token_address)| { - token_peer == &peer_id && token_address == address - }) - { - self.insertion_order.remove(index); - } - } } /// Combined libp2p behaviour. @@ -523,10 +496,7 @@ pub async fn start( tokio::select! { _ = identify_cleanup.tick() => { for (peer_id, address) in identify_addresses.expire(Instant::now()) { - if !explicit_addresses - .get(&peer_id) - .is_some_and(|addresses| addresses.contains(&address)) - { + if !is_explicit_address(&explicit_addresses, &peer_id, &address) { swarm.behaviour_mut().kademlia.remove_address(&peer_id, &address); } } @@ -622,9 +592,7 @@ pub async fn start( &info.listen_addrs, ); for (removed_peer, addr) in changes.removed { - if !explicit_addresses - .get(&removed_peer) - .is_some_and(|addresses| addresses.contains(&addr)) + if !is_explicit_address(&explicit_addresses, &removed_peer, &addr) { swarm .behaviour_mut() @@ -748,7 +716,6 @@ mod tests { ); } assert!(book.peers.is_empty()); - assert!(book.insertion_order.is_empty()); assert_eq!(book.address_count, 0); } @@ -815,7 +782,6 @@ mod tests { assert_eq!(changes.refreshed, vec![(*refreshed).clone()]); assert_eq!(book.peers.len(), IDENTIFY_GLOBAL_ADDRESS_LIMIT); assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); - assert_eq!(book.insertion_order.len(), book.address_count); assert_eq!( book.peers[peer] .addresses @@ -826,6 +792,62 @@ mod tests { ); } + #[test] + fn identify_per_peer_eviction_applies_at_full_global_budget() { + let now = Instant::now(); + let mut book = IdentifyAddressBook::default(); + // The victim peer holds a full per-peer row while the rest of the + // global budget is filled by one-address peers. + let victim = PeerId::random(); + let victim_addresses: Vec = (0..IDENTIFY_ADDRESS_LIMIT) + .map(|index| identify_address(victim, index as u8 + 1, 10_000 + index as u16)) + .collect(); + book.update(victim, now, &victim_addresses); + for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT - IDENTIFY_ADDRESS_LIMIT { + let peer = PeerId::random(); + let address = identify_address(peer, 200, 20_000 + index as u16); + book.update(peer, now, &[address]); + } + assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); + + // The victim reports a ninth address after the rate window rolls over. + // Per-peer eviction must still apply at global saturation: the oldest + // non-reported address is evicted to make room, and the global count + // is unchanged. This ordering only holds because eviction runs before + // the global-budget check; hoisting that check above the eviction + // block makes this test fail. + let ninth = identify_address(victim, 100, 30_000); + let changes = book.update( + victim, + now + IDENTIFY_ADDRESS_WINDOW, + std::slice::from_ref(&ninth), + ); + assert_eq!(changes.added, vec![ninth]); + assert_eq!(changes.removed, vec![(victim, victim_addresses[0].clone())]); + assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); + assert_eq!(book.peers[&victim].addresses.len(), IDENTIFY_ADDRESS_LIMIT); + } + + #[test] + fn explicit_address_helper_detects_retained_addresses() { + let peer = PeerId::random(); + let other = PeerId::random(); + let address = identify_address(peer, 1, 10_000); + let mut explicit: HashMap> = HashMap::new(); + // Unknown peer: nothing is explicit. + assert!(!is_explicit_address(&explicit, &peer, &address)); + explicit.entry(peer).or_default().insert(address.clone()); + assert!(is_explicit_address(&explicit, &peer, &address)); + // The same address under a different peer is not retained. + assert!(!is_explicit_address(&explicit, &other, &address)); + // A different address for the same peer is not retained. + assert!(!is_explicit_address( + &explicit, + &peer, + &identify_address(peer, 2, 10_001) + )); + } + #[test] fn identify_addresses_are_capped_with_fifo_eviction() { let peer_id = PeerId::random(); From ab53afef3ffe8227d54bac4b8df8a75bc547fc0d Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Wed, 30 Sep 2026 02:55:17 -0400 Subject: [PATCH 7/8] fix(node): pin row-removal and expiry guards in Identify address book Address beardthelion's round-two P3 findings on #462: - Add test that a fully-rejected fresh peer at full global budget leaves no peers row (pins the empty-row drop in update()). - Add test that update() past IDENTIFY_ADDRESS_TTL reports the stale address in changes.removed (pins expiry reporting). - Add test that expire() drops rows emptied by expiry (pins the sibling drop in expire_peer). - Distinguish per-peer from cross-peer eviction in the saturation test: insert a sentinel peer before the victim and assert its address survives, so a global-oldest eviction policy would fail. --- crates/gitlawb-node/src/p2p/mod.rs | 77 +++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index 46302937..52ccebd2 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -719,6 +719,64 @@ mod tests { assert_eq!(book.address_count, 0); } + #[test] + fn identify_fully_rejected_peer_leaves_no_row_at_full_budget() { + let now = Instant::now(); + let mut book = IdentifyAddressBook::default(); + // Fill the global budget with one-address peers. + for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT { + let peer = PeerId::random(); + let address = identify_address(peer, 1, 10_000 + index as u16); + book.update(peer, now, std::slice::from_ref(&address)); + } + assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); + // A fresh peer reporting at saturation has every candidate rejected + // by the global-budget check. The row created by or_insert_with must + // be dropped: a zero-address row is invisible to address_count and + // would otherwise linger until the expiry tick. + let fresh = PeerId::random(); + let fresh_address = identify_address(fresh, 2, 30_000); + let changes = book.update(fresh, now, std::slice::from_ref(&fresh_address)); + assert!(changes.added.is_empty()); + assert!(!book.peers.contains_key(&fresh)); + assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); + } + + #[test] + fn identify_update_reports_expired_addresses_as_removed() { + let peer = PeerId::random(); + let now = Instant::now(); + let address = identify_address(peer, 1, 10_000); + let mut book = IdentifyAddressBook::default(); + book.update(peer, now, std::slice::from_ref(&address)); + // An update past the TTL expires the stale address through + // expire_peer and reports it in changes.removed so the Identify + // handler can drop it from Kademlia. + let changes = book.update( + peer, + now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1), + std::slice::from_ref(&address), + ); + assert_eq!(changes.removed, vec![(peer, address.clone())]); + assert_eq!(changes.added, vec![address]); + } + + #[test] + fn identify_expire_drops_rows_emptied_by_expiry() { + let peer = PeerId::random(); + let now = Instant::now(); + let address = identify_address(peer, 1, 10_000); + let mut book = IdentifyAddressBook::default(); + book.update(peer, now, std::slice::from_ref(&address)); + assert!(book.peers.contains_key(&peer)); + // Once every address in the row has expired, expire() must drop the + // row itself, matching the empty-row drop in update(). + let removed = book.expire(now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)); + assert_eq!(removed, vec![(peer, address)]); + assert!(!book.peers.contains_key(&peer)); + assert_eq!(book.address_count, 0); + } + #[test] fn identify_report_processing_stops_at_the_input_limit() { let peer = PeerId::random(); @@ -796,6 +854,13 @@ mod tests { fn identify_per_peer_eviction_applies_at_full_global_budget() { let now = Instant::now(); let mut book = IdentifyAddressBook::default(); + // One filler peer goes in before the victim so the evicted address + // is not also the globally-oldest insertion. If saturation evicted + // the globally-oldest entry instead of the victim's own oldest, + // the sentinel's address would be the one removed. + let sentinel = PeerId::random(); + let sentinel_address = identify_address(sentinel, 200, 19_999); + book.update(sentinel, now, std::slice::from_ref(&sentinel_address)); // The victim peer holds a full per-peer row while the rest of the // global budget is filled by one-address peers. let victim = PeerId::random(); @@ -803,7 +868,7 @@ mod tests { .map(|index| identify_address(victim, index as u8 + 1, 10_000 + index as u16)) .collect(); book.update(victim, now, &victim_addresses); - for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT - IDENTIFY_ADDRESS_LIMIT { + for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT - IDENTIFY_ADDRESS_LIMIT - 1 { let peer = PeerId::random(); let address = identify_address(peer, 200, 20_000 + index as u16); book.update(peer, now, &[address]); @@ -826,6 +891,16 @@ mod tests { assert_eq!(changes.removed, vec![(victim, victim_addresses[0].clone())]); assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT); assert_eq!(book.peers[&victim].addresses.len(), IDENTIFY_ADDRESS_LIMIT); + // The sentinel was inserted first; per-peer eviction must leave it + // alone. A global-oldest eviction policy would have removed it. + assert_eq!( + book.peers[&sentinel] + .addresses + .iter() + .map(|a| &a.address) + .collect::>(), + vec![&sentinel_address] + ); } #[test] From c6908977fe0f3f5062090f69a11d0ce7b5721ddb Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Wed, 30 Sep 2026 11:41:34 -0400 Subject: [PATCH 8/8] fix(node): pin refresh predicate negative direction per beardthelion round 3 --- crates/gitlawb-node/src/p2p/mod.rs | 40 ++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/crates/gitlawb-node/src/p2p/mod.rs b/crates/gitlawb-node/src/p2p/mod.rs index 52ccebd2..b9f23e5f 100644 --- a/crates/gitlawb-node/src/p2p/mod.rs +++ b/crates/gitlawb-node/src/p2p/mod.rs @@ -1066,6 +1066,46 @@ mod tests { ); } + #[test] + fn identify_refresh_reports_only_addresses_in_the_report() { + let peer_id = PeerId::random(); + let now = Instant::now(); + let reported = identify_address(peer_id, 1, 10_000); + let unreported = identify_address(peer_id, 2, 20_000); + let mut book = IdentifyAddressBook::default(); + + // Admit both addresses up front so the peer holds two. + assert_eq!( + book.update(peer_id, now, &[reported.clone(), unreported.clone()]) + .added + .len(), + 2 + ); + + // A later report carrying only one of the two addresses must refresh + // just that one: pins the negative direction of the refresh + // predicate, so an address the peer stopped advertising keeps its + // original TTL instead of being refreshed back onto the Kademlia + // re-admit path on every report. + let report_at = now + IDENTIFY_ADDRESS_TTL - Duration::from_secs(1); + let changes = book.update(peer_id, report_at, std::slice::from_ref(&reported)); + assert!(changes.added.is_empty()); + assert!(changes.removed.is_empty()); + assert_eq!(changes.refreshed, vec![reported.clone()]); + + // The unreported address still expires at its original TTL, while + // the refreshed address survives until its refreshed TTL. + assert_eq!( + book.expire(now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)), + vec![(peer_id, unreported)] + ); + assert!(book.expire(report_at + Duration::from_secs(1)).is_empty()); + assert_eq!( + book.expire(report_at + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)), + vec![(peer_id, reported)] + ); + } + #[test] fn identify_addresses_reject_foreign_peer_suffixes() { let peer_id = PeerId::random();