Skip to content

Commit d17601b

Browse files
committed
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.
1 parent 0ffdbdb commit d17601b

1 file changed

Lines changed: 106 additions & 8 deletions

File tree

  • crates/gitlawb-node/src/p2p

‎crates/gitlawb-node/src/p2p/mod.rs‎

Lines changed: 106 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ pub const REF_UPDATES_TOPIC: &str = "gitlawb/ref-updates/v1";
3333
// Identify is an untrusted address source. Keep enough entries for normal
3434
// multi-homed peers while bounding both one peer and the whole routing table.
3535
const IDENTIFY_ADDRESS_LIMIT: usize = 8;
36+
// Bound canonicalization and sorting too, including rejected/duplicate entries.
37+
const IDENTIFY_REPORT_ADDRESS_LIMIT: usize = 64;
3638
const IDENTIFY_NEW_ADDRESS_LIMIT: usize = 8;
3739
const IDENTIFY_GLOBAL_ADDRESS_LIMIT: usize = 1024;
3840
const IDENTIFY_ADDRESS_WINDOW: Duration = Duration::from_secs(60);
@@ -201,6 +203,14 @@ impl IdentifyAddressBook {
201203
if self.peers.contains_key(&peer_id) {
202204
changes.removed.extend(self.expire_peer(peer_id, now));
203205
}
206+
let reported = addresses
207+
.iter()
208+
.take(IDENTIFY_REPORT_ADDRESS_LIMIT)
209+
.filter_map(|address| address.clone().with_p2p(peer_id).ok())
210+
.collect::<HashSet<_>>();
211+
if reported.is_empty() && !self.peers.contains_key(&peer_id) {
212+
return changes;
213+
}
204214
self.peers
205215
.entry(peer_id)
206216
.or_insert_with(|| IdentifyPeerAddresses {
@@ -216,21 +226,22 @@ impl IdentifyAddressBook {
216226
}
217227
}
218228

219-
let reported = addresses
220-
.iter()
221-
.filter_map(|address| address.clone().with_p2p(peer_id).ok())
222-
.collect::<HashSet<_>>();
223-
224-
// Refresh every address in the report before admitting anything new.
225-
// This prevents an oversized report from evicting an address merely
226-
// because it appeared later in the input.
229+
// Refresh every retained address in the bounded report before admission.
230+
let mut refreshed = Vec::new();
227231
if let Some(state) = self.peers.get_mut(&peer_id) {
228232
for existing in &mut state.addresses {
229233
if reported.contains(&existing.address) {
230234
existing.expires_at = now + IDENTIFY_ADDRESS_TTL;
235+
refreshed.push(existing.address.clone());
231236
}
232237
}
233238
}
239+
// Global eviction follows refresh recency, so an active address does
240+
// not lose its slot just because it was originally admitted first.
241+
for address in refreshed {
242+
self.remove_insertion_token(peer_id, &address);
243+
self.insertion_order.push_back((peer_id, address));
244+
}
234245

235246
let mut new_addresses = self
236247
.peers
@@ -735,6 +746,93 @@ mod tests {
735746
.unwrap()
736747
}
737748

749+
#[test]
750+
fn identify_empty_reports_do_not_retain_peer_entries() {
751+
let now = Instant::now();
752+
let mut book = IdentifyAddressBook::default();
753+
let foreign = identify_address(PeerId::random(), 1, 10_000);
754+
for _ in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT + 1 {
755+
let peer = PeerId::random();
756+
assert_eq!(
757+
book.update(peer, now, &[]),
758+
IdentifyAddressChanges::default()
759+
);
760+
assert_eq!(
761+
book.update(peer, now, std::slice::from_ref(&foreign)),
762+
IdentifyAddressChanges::default()
763+
);
764+
}
765+
assert!(book.peers.is_empty());
766+
assert!(book.insertion_order.is_empty());
767+
assert_eq!(book.address_count, 0);
768+
}
769+
770+
#[test]
771+
fn identify_report_processing_stops_at_the_input_limit() {
772+
let peer = PeerId::random();
773+
let now = Instant::now();
774+
let foreign = identify_address(PeerId::random(), 1, 10_000);
775+
let accepted = identify_address(peer, 2, 10_001);
776+
let ignored = identify_address(peer, 3, 10_002);
777+
let mut report = vec![foreign; IDENTIFY_REPORT_ADDRESS_LIMIT - 1];
778+
report.push(accepted.clone());
779+
report.push(ignored);
780+
let mut book = IdentifyAddressBook::default();
781+
let changes = book.update(peer, now, &report);
782+
assert_eq!(changes.added, vec![accepted]);
783+
assert!(changes.removed.is_empty());
784+
assert_eq!(book.address_count, 1);
785+
}
786+
787+
#[test]
788+
fn identify_addresses_grow_without_eviction_below_the_peer_cap() {
789+
let peer = PeerId::random();
790+
let now = Instant::now();
791+
let first = identify_address(peer, 1, 10_000);
792+
let second = identify_address(peer, 2, 10_001);
793+
let mut book = IdentifyAddressBook::default();
794+
book.update(peer, now, std::slice::from_ref(&first));
795+
let changes = book.update(
796+
peer,
797+
now + IDENTIFY_ADDRESS_WINDOW,
798+
std::slice::from_ref(&second),
799+
);
800+
assert_eq!(changes.added, vec![second]);
801+
assert!(changes.removed.is_empty());
802+
assert_eq!(book.peers[&peer].addresses.len(), 2);
803+
assert_eq!(book.address_count, 2);
804+
}
805+
806+
#[test]
807+
fn identify_global_eviction_preserves_refreshed_addresses() {
808+
let now = Instant::now();
809+
let mut book = IdentifyAddressBook::default();
810+
let peers = (0..IDENTIFY_GLOBAL_ADDRESS_LIMIT)
811+
.map(|index| {
812+
let peer = PeerId::random();
813+
let address = identify_address(peer, 1, 10_000 + index as u16);
814+
book.update(peer, now, std::slice::from_ref(&address));
815+
(peer, address)
816+
})
817+
.collect::<Vec<_>>();
818+
let (peer, refreshed) = &peers[0];
819+
let new_address = identify_address(*peer, 2, 30_000);
820+
let changes = book.update(
821+
*peer,
822+
now + IDENTIFY_ADDRESS_WINDOW,
823+
&[new_address.clone(), refreshed.clone()],
824+
);
825+
assert_eq!(changes.added, vec![new_address]);
826+
assert_eq!(changes.removed, vec![peers[1].clone()]);
827+
assert!(book.peers[peer]
828+
.addresses
829+
.iter()
830+
.any(|a| &a.address == refreshed));
831+
assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT);
832+
assert_eq!(book.insertion_order.len(), book.address_count);
833+
assert_eq!(book.peers.len(), IDENTIFY_GLOBAL_ADDRESS_LIMIT - 1);
834+
}
835+
738836
#[test]
739837
fn identify_addresses_are_capped_with_fifo_eviction() {
740838
let peer_id = PeerId::random();

0 commit comments

Comments
 (0)