diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d428bff..8fa09394 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 matching the wildcard UDP receive path instead of issuing one `recv(2)` syscall per queued datagram. +- Routing next-hop selection now visits borrowed peers and coordinates instead + of allocating candidate snapshots for each forwarded packet. + - The Ethernet transport's per-interface `discovery` flag was renamed to `listen` (`transports.ethernet.*`) to match the symmetric `announce` (transmit) / `listen` (receive) neighbor-beacon vocabulary. The old diff --git a/benches/routing_next_hop.rs b/benches/routing_next_hop.rs index 76e3e53f..e858304b 100644 --- a/benches/routing_next_hop.rs +++ b/benches/routing_next_hop.rs @@ -1,23 +1,18 @@ -//! Micro-benchmark quantifying the per-forwarded-packet heap-allocation cost -//! of the routing next-hop candidate-assembly path. +//! Micro-benchmark quantifying the per-forwarded-packet cost of routing +//! next-hop selection before and after candidate assembly was fused. //! -//! `find_next_hop` runs once per forwarded data packet. Its sans-IO core -//! assembles a `Vec` by enumerating every peer through the -//! `RoutingView` seam: `peer_addrs()` materializes a `Vec` of all -//! peers, the survivors are snapshotted (each cloning its `TreeCoordinate`), -//! and the result is collected into a second `Vec`. This bench measures that -//! per-call allocation against a fused zero-alloc reference that iterates the -//! peer map directly and borrows coordinates instead of cloning. +//! `find_next_hop` runs once per forwarded data packet. The former path +//! materialized a `Vec`, cloned each surviving `TreeCoordinate` into +//! a second `Vec`, then selected from that snapshot. The current +//! path visits borrowed peers, filters and selects inline, and borrows +//! coordinates. This bench retains the former implementation as a baseline +//! and measures both paths in the same process. //! -//! Visibility caveat: the production `routing_candidates` / `select_best_candidate` -//! / `RoutingView` / `Candidate` are `pub(crate)` (src/proto/routing/core.rs) -//! and are not re-exported at the crate root, so an external bench crate cannot -//! name them. Rather than change production visibility, this file reproduces -//! that path verbatim over the real public `NodeAddr` / `TreeCoordinate` / -//! `CoordEntry` / `BloomFilter` types with the same iterator chain and the same -//! `HashMap`-backed view the shell uses (src/node/mod.rs NodeRoutingView). The -//! allocation behavior is therefore identical to production by construction; -//! only the symbol identity differs. +//! Visibility caveat: the production selector and `RoutingView` are +//! `pub(crate)` and are not re-exported at the crate root, so an external bench +//! crate cannot name them. Rather than widen production visibility, this file +//! mirrors both implementations over the real public routing value types and +//! the same `HashMap`-backed shape used by `NodeRoutingView`. use std::alloc::{GlobalAlloc, Layout, System}; use std::collections::HashMap; @@ -65,9 +60,9 @@ const REACH_DENOMINATOR: usize = 2; const COORD_DEPTH: usize = 8; // --------------------------------------------------------------------------- -// Reproduction of the pub(crate) routing seam (src/proto/routing/core.rs). +// Former allocating routing seam, retained as the before baseline. // --------------------------------------------------------------------------- -trait RoutingView { +trait FormerRoutingView { fn peer_addrs(&self) -> Vec; fn peer_may_reach(&self, peer: &NodeAddr, dest: &NodeAddr) -> bool; fn peer_can_send(&self, peer: &NodeAddr) -> bool; @@ -75,20 +70,20 @@ trait RoutingView { fn peer_coords(&self, peer: &NodeAddr) -> Option; } -struct Candidate { +struct FormerCandidate { addr: NodeAddr, can_send: bool, link_cost: f64, coords: Option, } -/// Verbatim from `routing::routing_candidates` (core.rs). Allocates the -/// `peer_addrs` Vec, clones each survivor's coords, and collects into a Vec. -fn routing_candidates(rv: &impl RoutingView, dest: &NodeAddr) -> Vec { +/// Former `routing::routing_candidates`. Allocates the `peer_addrs` Vec, +/// clones each survivor's coords, and collects into a second Vec. +fn former_routing_candidates(rv: &impl FormerRoutingView, dest: &NodeAddr) -> Vec { rv.peer_addrs() .into_iter() .filter(|peer| rv.peer_may_reach(peer, dest)) - .map(|peer| Candidate { + .map(|peer| FormerCandidate { can_send: rv.peer_can_send(&peer), link_cost: rv.peer_link_cost(&peer), coords: rv.peer_coords(&peer), @@ -97,14 +92,14 @@ fn routing_candidates(rv: &impl RoutingView, dest: &NodeAddr) -> Vec .collect() } -/// Verbatim from `routing::select_best_candidate` (core.rs). Pure, no alloc. -fn select_best_candidate( - candidates: &[Candidate], +/// Former `routing::select_best_candidate`. Pure itself; consumes the snapshot. +fn former_select_best_candidate( + candidates: &[FormerCandidate], dest_coords: &TreeCoordinate, my_coords: &TreeCoordinate, ) -> Option { let my_distance = my_coords.distance_to(dest_coords); - let mut best: Option<(&Candidate, f64, usize)> = None; + let mut best: Option<(&FormerCandidate, f64, usize)> = None; for candidate in candidates { if !candidate.can_send { continue; @@ -149,7 +144,7 @@ struct BenchView { coords: HashMap, } -impl RoutingView for BenchView { +impl FormerRoutingView for BenchView { fn peer_addrs(&self) -> Vec { self.peers.keys().copied().collect() } @@ -167,10 +162,10 @@ impl RoutingView for BenchView { } } -/// Zero-alloc reference: what an iterator/visitor seam would do. Iterates the -/// peer map directly, fuses the may_reach + can_send filters, borrows coords -/// instead of cloning, and tracks the best hop inline. No Vec, no coord clone. -fn resolve_next_hop_zeroalloc( +/// Current borrowed selector. Mirrors the production visitor seam after +/// monomorphization: peer facts come from the map entry, coordinates are +/// borrowed from the tree-state map, and the winner is tracked inline. +fn select_best_candidate_current( view: &BenchView, dest: &NodeAddr, dest_coords: &TreeCoordinate, @@ -310,19 +305,19 @@ fn report_allocs() { println!("\n=== allocations per call (heap alloc ops: alloc+alloc_zeroed+realloc) ==="); println!( "{:>6} {:>10} {:>16} {:>16}", - "peers", "survivors", "current/call", "zero-alloc/call" + "peers", "survivors", "former/call", "current/call" ); for &n in &PEER_COUNTS { let s = Scenario::new(n); let survivors = s.survivors(); + let former = count_allocs(ITERS, || { + let candidates = former_routing_candidates(&s.view, &s.dest); + former_select_best_candidate(&candidates, &s.dest_coords, &s.my_coords) + }); let current = count_allocs(ITERS, || { - let cands = routing_candidates(&s.view, &s.dest); - select_best_candidate(&cands, &s.dest_coords, &s.my_coords) + select_best_candidate_current(&s.view, &s.dest, &s.dest_coords, &s.my_coords) }); - let zero = count_allocs(ITERS, || { - resolve_next_hop_zeroalloc(&s.view, &s.dest, &s.dest_coords, &s.my_coords) - }); - println!("{n:>6} {survivors:>10} {current:>16.2} {zero:>16.2}"); + println!("{n:>6} {survivors:>10} {former:>16.2} {current:>16.2}"); } println!(); } @@ -333,19 +328,19 @@ fn bench_next_hop(c: &mut Criterion) { let mut group = c.benchmark_group("find_next_hop"); for &n in &PEER_COUNTS { let scenario = Scenario::new(n); - group.bench_with_input(BenchmarkId::new("current_alloc", n), &n, |b, _| { + group.bench_with_input(BenchmarkId::new("former_allocating", n), &n, |b, _| { b.iter(|| { - let cands = routing_candidates(&scenario.view, &scenario.dest); - black_box(select_best_candidate( - &cands, + let candidates = former_routing_candidates(&scenario.view, &scenario.dest); + black_box(former_select_best_candidate( + &candidates, &scenario.dest_coords, &scenario.my_coords, )) }); }); - group.bench_with_input(BenchmarkId::new("zero_alloc_ref", n), &n, |b, _| { + group.bench_with_input(BenchmarkId::new("current_borrowed", n), &n, |b, _| { b.iter(|| { - black_box(resolve_next_hop_zeroalloc( + black_box(select_best_candidate_current( &scenario.view, &scenario.dest, &scenario.dest_coords, diff --git a/src/node/mod.rs b/src/node/mod.rs index 3a66f5ae..20bda80c 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -2780,26 +2780,24 @@ impl Node { // 3. Bloom filter candidates — requires dest_coords for loop-free selection. // If no candidate is strictly closer, fall through to tree routing. - // The sans-IO core assembles the candidate snapshot over the - // `RoutingView` seam (enumerate peers, apply the bloom `may_reach` - // filter, snapshot each), then picks the winner; the shell supplies - // only the raw per-peer reads. - let candidates = { + // The sans-IO core enumerates borrowed peers over the `RoutingView` + // seam, applies the bloom/send/progress filters, and tracks the + // winner inline; the shell supplies only raw per-peer reads. + let next_hop = { let view = NodeRoutingView { coord_cache: &self.coord_cache, peers: &self.peers, tree_state: &self.tree_state, congested: false, }; - routing::routing_candidates(&view, dest_node_addr) - }; - if !candidates.is_empty() - && let Some(next_hop) = routing::select_best_candidate( - &candidates, + routing::select_best_candidate( + &view, + dest_node_addr, &dest_coords, self.tree_state.my_coords(), ) - { + }; + if let Some(next_hop) = next_hop { return self.peers.get(&next_hop); } @@ -3113,17 +3111,16 @@ impl Node { /// Shell-side [`routing::RoutingView`] seam over live `Node` state — the sole /// routing read adapter the shell retains. It hands the sans-IO routing core -/// raw per-peer reads (enumeration plus `may_reach` / `can_send` / `link_cost` -/// / `coords`) so the candidate assembly, selection, and error synthesis all -/// live in `proto::routing::core`; no routing decision or assembly logic -/// remains here. +/// borrowed peers plus raw `may_reach` / `can_send` / `link_cost` / `coords` +/// reads so selection and error synthesis live in `proto::routing::core`; no +/// routing decision logic remains here. /// /// Field-narrowed to `coord_cache` + `peers` + `tree_state` (never `&Node` /// whole) so it borrows disjointly from `&mut self.routing` on the /// forward/synth path, where the handler also holds the mutable `Router`. /// /// Two call sites: -/// - `find_next_hop` builds it to assemble bloom candidates via the `peer_*` +/// - `find_next_hop` builds it to select a bloom candidate via the `peer_*` /// reads; it never queries `is_congested`, so it leaves `congested` false. /// - `handle_session_datagram` builds it for `Router::route` / `synth_*`, /// which read `is_congested` (precomputed once for the resolved next hop) @@ -3136,6 +3133,11 @@ pub(in crate::node) struct NodeRoutingView<'a> { } impl routing::RoutingView for NodeRoutingView<'_> { + type Peer<'a> + = (&'a NodeAddr, &'a ActivePeer) + where + Self: 'a; + fn is_congested(&self, _next_hop: &NodeAddr) -> bool { self.congested } @@ -3144,26 +3146,30 @@ impl routing::RoutingView for NodeRoutingView<'_> { self.coord_cache.get(dest, now_ms).cloned() } - fn peer_addrs(&self) -> Vec { - self.peers.keys().copied().collect() + fn for_each_peer<'a>(&'a self, mut visitor: impl FnMut(Self::Peer<'a>)) { + for peer in self.peers { + visitor(peer); + } } - fn peer_may_reach(&self, peer: &NodeAddr, dest: &NodeAddr) -> bool { - self.peers.get(peer).is_some_and(|p| p.may_reach(dest)) + fn peer_addr<'a>(&'a self, peer: Self::Peer<'a>) -> NodeAddr { + *peer.0 } - fn peer_can_send(&self, peer: &NodeAddr) -> bool { - self.peers.get(peer).is_some_and(|p| p.can_send()) + fn peer_may_reach<'a>(&'a self, peer: Self::Peer<'a>, dest: &NodeAddr) -> bool { + peer.1.may_reach(dest) } - fn peer_link_cost(&self, peer: &NodeAddr) -> f64 { - self.peers - .get(peer) - .map_or(f64::INFINITY, |p| p.link_cost()) + fn peer_can_send<'a>(&'a self, peer: Self::Peer<'a>) -> bool { + peer.1.can_send() } - fn peer_coords(&self, peer: &NodeAddr) -> Option { - self.tree_state.peer_coords(peer).cloned() + fn peer_link_cost<'a>(&'a self, peer: Self::Peer<'a>) -> f64 { + peer.1.link_cost() + } + + fn peer_coords<'a>(&'a self, peer: Self::Peer<'a>) -> Option<&'a TreeCoordinate> { + self.tree_state.peer_coords(peer.0) } } diff --git a/src/proto/routing/core.rs b/src/proto/routing/core.rs index efe334e6..32c1f604 100644 --- a/src/proto/routing/core.rs +++ b/src/proto/routing/core.rs @@ -7,14 +7,12 @@ //! actual encrypted sends, metrics, and logging). No I/O, no clock, no //! metrics, no logging here. //! -//! This module also holds the pure candidate assembly ([`routing_candidates`]) -//! and the hop-selection / route-classification helpers -//! ([`select_best_candidate`], [`classify_forward`]). The assembly enumerates -//! peers through the [`RoutingView`] seam, applies the bloom `may_reach` -//! filter, and snapshots each survivor into a [`Candidate`]; the shell hands -//! over only raw per-peer reads (enumeration plus `may_reach` / `can_send` / -//! `link_cost` / `coords`). Selection and classification then consume the -//! assembled set. All routing narrowing and decision logic lives here. +//! This module also holds the pure hop-selection / route-classification helpers +//! ([`select_best_candidate`], [`classify_forward`]). Selection enumerates +//! borrowed peers through the [`RoutingView`] seam, applies the bloom +//! `may_reach` and send/progress filters, and tracks the winner inline. The +//! shell hands over only raw per-peer reads; all routing narrowing and decision +//! logic lives here. use super::state::Router; use super::wire::{CoordsRequired, MtuExceeded, PathBroken}; @@ -28,6 +26,12 @@ use crate::{NodeAddr, TreeCoordinate}; /// `proto` free of any dependency on `node` and lets the core be unit-tested /// with a mock. pub(crate) trait RoutingView { + /// Borrowed peer handle exposed while enumerating this view. The concrete + /// shell type stays opaque to the routing core. + type Peer<'a>: Copy + where + Self: 'a; + /// Is the outgoing link toward `next_hop` congested (ECN local signal)? fn is_congested(&self, next_hop: &NodeAddr) -> bool; /// Cached destination coordinates for `dest`, if any (read-only lookup). @@ -36,19 +40,19 @@ pub(crate) trait RoutingView { /// [`Router::synth_routing_error`]. fn cached_coords(&self, dest: &NodeAddr, now_ms: u64) -> Option; - /// Node addresses of every currently-active peer — the raw enumeration the - /// candidate assembly iterates. No filtering or ordering is applied here; - /// [`routing_candidates`] applies the bloom `may_reach` narrowing in core. - fn peer_addrs(&self) -> Vec; + /// Visit every currently-active peer without filtering or ordering. + fn for_each_peer<'a>(&'a self, visitor: impl FnMut(Self::Peer<'a>)); + /// Node address of a borrowed peer. + fn peer_addr<'a>(&'a self, peer: Self::Peer<'a>) -> NodeAddr; /// Does `peer`'s bloom filter indicate it may reach `dest`? The raw - /// per-peer predicate the core assembly filters candidates on. - fn peer_may_reach(&self, peer: &NodeAddr, dest: &NodeAddr) -> bool; + /// per-peer predicate the core filters candidates on. + fn peer_may_reach<'a>(&'a self, peer: Self::Peer<'a>, dest: &NodeAddr) -> bool; /// Can `peer`'s session currently carry a forward? - fn peer_can_send(&self, peer: &NodeAddr) -> bool; + fn peer_can_send<'a>(&'a self, peer: Self::Peer<'a>) -> bool; /// `peer`'s outgoing link cost (lower is preferred). - fn peer_link_cost(&self, peer: &NodeAddr) -> f64; + fn peer_link_cost<'a>(&'a self, peer: Self::Peer<'a>) -> f64; /// `peer`'s tree coordinates, if known. - fn peer_coords(&self, peer: &NodeAddr) -> Option; + fn peer_coords<'a>(&'a self, peer: Self::Peer<'a>) -> Option<&'a TreeCoordinate>; } /// A next hop the shell resolved for a transit forward: the peer address and @@ -269,101 +273,61 @@ pub(crate) enum RouteClass { DirectPeer, } -/// A bloom-filter routing candidate, snapshotted by [`routing_candidates`] -/// from the per-peer reads the [`RoutingView`] seam exposes. +/// Select the best next hop from the active peers that may reach `dest`. /// -/// The assembly applies the bloom `may_reach` narrowing before building each -/// snapshot, so [`select_best_candidate`] is a pure consumer of an -/// already-narrowed set and names no shell peer type. -pub(crate) struct Candidate { - /// The candidate peer's node address. - pub addr: NodeAddr, - /// Whether the peer's session can currently carry a forward. - pub can_send: bool, - /// The outgoing link cost (lower is preferred). - pub link_cost: f64, - /// The candidate's tree coordinates, if known. - pub coords: Option, -} - -/// Assemble the bloom-filter routing candidates toward `dest`. -/// -/// Enumerates every peer through the [`RoutingView`] seam, applies the bloom -/// `may_reach` filter, and snapshots each surviving peer's send-eligibility, -/// link cost, and tree coordinates into a [`Candidate`]. Pure over the seam's -/// primitive reads — the shell hands over raw per-peer data only, so all -/// narrowing and snapshotting happens here and [`select_best_candidate`] -/// consumes an already-assembled set. Candidate order follows the seam's -/// enumeration, which the selection ordering renders immaterial (it breaks -/// ties deterministically on `node_addr`). -pub(crate) fn routing_candidates(rv: &impl RoutingView, dest: &NodeAddr) -> Vec { - rv.peer_addrs() - .into_iter() - .filter(|peer| rv.peer_may_reach(peer, dest)) - .map(|peer| Candidate { - can_send: rv.peer_can_send(&peer), - link_cost: rv.peer_link_cost(&peer), - coords: rv.peer_coords(&peer), - addr: peer, - }) - .collect() -} - -/// Select the best next hop from a set of bloom-filter candidates. -/// -/// Uses each candidate's tree-coordinate distance to the destination as the -/// primary metric (after link cost). Only peers strictly closer to the -/// destination than we are (`my_coords`) are eligible — the self-distance -/// check that prevents routing loops. +/// Enumerates borrowed peers through [`RoutingView`], applies the bloom and +/// send-eligibility filters, and tracks the best hop inline without allocating +/// candidate vectors or cloning coordinates. Only peers strictly closer to the +/// destination than we are (`my_coords`) are eligible — the self-distance check +/// that prevents routing loops. /// /// Ordering: `(link_cost, distance_to_dest, node_addr)`. Returns the winning /// peer's address, or `None` when no candidate is send-ready and strictly /// closer to the destination than us. pub(crate) fn select_best_candidate( - candidates: &[Candidate], + rv: &impl RoutingView, + dest: &NodeAddr, dest_coords: &TreeCoordinate, my_coords: &TreeCoordinate, ) -> Option { let my_distance = my_coords.distance_to(dest_coords); - let mut best: Option<(&Candidate, f64, usize)> = None; + let mut best: Option<(NodeAddr, f64, usize)> = None; - for candidate in candidates { - if !candidate.can_send { - continue; + rv.for_each_peer(|peer| { + if !rv.peer_may_reach(peer, dest) || !rv.peer_can_send(peer) { + return; } - let cost = candidate.link_cost; + let addr = rv.peer_addr(peer); + let cost = rv.peer_link_cost(peer); - let dist = candidate - .coords - .as_ref() + let dist = rv + .peer_coords(peer) .map(|pc| pc.distance_to(dest_coords)) .unwrap_or(usize::MAX); // Self-distance check: only consider peers strictly closer // to the destination than we are (prevents routing loops) if dist >= my_distance { - continue; + return; } let dominated = match &best { None => true, - Some((_, best_cost, best_dist)) => { + Some((best_addr, best_cost, best_dist)) => { cost < *best_cost || (cost == *best_cost && dist < *best_dist) - || (cost == *best_cost - && dist == *best_dist - && candidate.addr < best.as_ref().unwrap().0.addr) + || (cost == *best_cost && dist == *best_dist && addr < *best_addr) } }; if dominated { - best = Some((candidate, cost, dist)); + best = Some((addr, cost, dist)); } - } + }); - best.map(|(candidate, _, _)| candidate.addr) + best.map(|(addr, _, _)| addr) } /// Classify a transit forward by route class from tree coordinates. diff --git a/src/proto/routing/mod.rs b/src/proto/routing/mod.rs index 96b17730..b9fa4bd8 100644 --- a/src/proto/routing/mod.rs +++ b/src/proto/routing/mod.rs @@ -7,10 +7,10 @@ //! - `core.rs` — the `RoutingView` read-seam trait, the `NextHop` / //! `RouteOutcome` types, `Router::route`, the pure transit-forward //! decision (local-vs-forward, transit TTL, path-MTU min-fold, ECN CE), and the -//! pure candidate assembly + hop-selection / route-classification helpers -//! (`Candidate`, `RouteClass`, `routing_candidates`, `select_best_candidate`, -//! `classify_forward`). The assembly reads raw per-peer data through the -//! `RoutingView` seam; the shell keeps only the seam impl. +//! pure hop-selection / route-classification helpers (`RouteClass`, +//! `select_best_candidate`, `classify_forward`). Selection reads borrowed +//! per-peer data through the `RoutingView` seam; the shell keeps only the seam +//! impl. //! - `state.rs` — `Router`, the routing-subsystem state owned by `Node`. //! - `limits.rs` — the routing error-signal rate limiter. //! - `wire.rs` — the routing error-signal PDUs (`CoordsRequired`, @@ -27,7 +27,7 @@ mod tests; pub(crate) use core::{ DropReason, NextHop, RouteAction, RouteClass, RouteOutcome, RoutingView, classify_forward, - routing_candidates, select_best_candidate, + select_best_candidate, }; pub(crate) use limits::RoutingErrorRateLimiter; pub(crate) use state::Router; diff --git a/src/proto/routing/tests/core.rs b/src/proto/routing/tests/core.rs index a4f075c7..c6be0dad 100644 --- a/src/proto/routing/tests/core.rs +++ b/src/proto/routing/tests/core.rs @@ -1,13 +1,13 @@ //! Tests for the sans-IO routing decision core. use super::util::{MockPeer, MockRoutingView, make_coords, make_datagram_ref, make_next_hop}; -use crate::TreeCoordinate; use crate::proto::link::SessionDatagramRef; use crate::proto::routing::RoutingSignalType; use crate::proto::routing::{ - DropReason, RouteAction, RouteOutcome, Router, RoutingView, routing_candidates, + DropReason, RouteAction, RouteOutcome, Router, RoutingView, select_best_candidate, }; use crate::testutil::make_node_addr; +use crate::{NodeAddr, TreeCoordinate}; /// Decode a forwarded byte buffer (which carries the leading msg_type byte) /// back into a borrowed view so tests can inspect the re-encoded header. @@ -15,6 +15,32 @@ fn decode_forward(bytes: &[u8]) -> SessionDatagramRef<'_> { SessionDatagramRef::decode(&bytes[1..]).expect("forwarded datagram re-decodes") } +fn choose_candidate( + rv: &MockRoutingView, + dest: &NodeAddr, + dest_coords: &TreeCoordinate, + my_coords: &TreeCoordinate, +) -> Option { + select_best_candidate(rv, dest, dest_coords, my_coords) +} + +fn mock_peer( + addr: u8, + dest: NodeAddr, + may_reach: bool, + can_send: bool, + link_cost: f64, + coords: Option<&[u8]>, +) -> MockPeer { + MockPeer { + addr: make_node_addr(addr), + reach: may_reach.then_some(dest).into_iter().collect(), + can_send, + link_cost, + coords: coords.map(make_coords), + } +} + /// A transit datagram that arrived already exhausted is dropped and charged /// to `TtlExhausted`. A next hop is supplied so the drop is evidence of the /// TTL gate rather than of an absent route. @@ -253,44 +279,98 @@ fn cached_coords_reads_the_view_table() { } #[test] -fn routing_candidates_filters_by_may_reach_and_snapshots() { +fn candidate_selection_is_independent_of_peer_enumeration_order() { let dest = make_node_addr(0x50); - let reacher = make_node_addr(0x60); - let non_reacher = make_node_addr(0x61); - let reacher_coords = make_coords(&[0x01, 0x60]); + let root = 0x00; + let my_coords = make_coords(&[0x10, root]); + let dest_coords = make_coords(&[0x50, root]); + let lower_addr = mock_peer(0x20, dest, true, true, 1.0, Some(&[root])); + let higher_addr = mock_peer(0x30, dest, true, true, 1.0, Some(&[root])); + + let forward = MockRoutingView { + peers: vec![lower_addr.clone(), higher_addr.clone()], + ..MockRoutingView::new(false) + }; + let reverse = MockRoutingView { + peers: vec![higher_addr, lower_addr], + ..MockRoutingView::new(false) + }; + + assert_eq!( + choose_candidate(&forward, &dest, &dest_coords, &my_coords), + Some(make_node_addr(0x20)) + ); + assert_eq!( + choose_candidate(&reverse, &dest, &dest_coords, &my_coords), + Some(make_node_addr(0x20)) + ); +} + +#[test] +fn candidate_selection_filters_bloom_unsendable_and_missing_coords() { + let dest = make_node_addr(0x50); + let root = 0x00; + let my_coords = make_coords(&[0x10, root]); + let dest_coords = make_coords(&[0x50, root]); + let eligible = make_node_addr(0x60); let rv = MockRoutingView { peers: vec![ - MockPeer { - addr: reacher, - reach: vec![dest], - can_send: true, - link_cost: 2.5, - coords: Some(reacher_coords.clone()), - }, - MockPeer { - // Bloom filter does not contain dest — narrowed out in core. - addr: non_reacher, - reach: Vec::new(), - can_send: true, - link_cost: 1.0, - coords: None, - }, + mock_peer(0x01, dest, true, false, 0.0, Some(&[0x50, root])), + mock_peer(0x02, dest, true, true, 0.0, None), + mock_peer(0x03, dest, false, true, 0.0, Some(&[0x50, root])), + mock_peer(0x60, dest, true, true, 10.0, Some(&[root])), ], ..MockRoutingView::new(false) }; - let candidates = routing_candidates(&rv, &dest); + assert_eq!( + choose_candidate(&rv, &dest, &dest_coords, &my_coords), + Some(eligible) + ); +} + +#[test] +fn candidate_must_be_strictly_closer_than_self() { + let dest = make_node_addr(0x50); + let root = 0x00; + let my_coords = make_coords(&[0x10, root]); + let dest_coords = make_coords(&[0x50, root]); + let rv = MockRoutingView { + peers: vec![ + // A sibling is exactly as far from dest as this node. + mock_peer(0x20, dest, true, true, 1.0, Some(&[0x20, root])), + // This descendant of a sibling is farther from dest. + mock_peer(0x21, dest, true, true, 0.5, Some(&[0x21, 0x20, root])), + ], + ..MockRoutingView::new(false) + }; + + assert_eq!(choose_candidate(&rv, &dest, &dest_coords, &my_coords), None); +} + +#[test] +fn candidate_ordering_is_cost_then_distance_then_address() { + let dest = make_node_addr(0x50); + let root = 0x00; + let my_coords = make_coords(&[0x10, 0x11, root]); + let dest_coords = make_coords(&[0x50, root]); + let rv = MockRoutingView { + peers: vec![ + // Lowest address loses because distance precedes address. + mock_peer(0x01, dest, true, true, 1.0, Some(&[root])), + // Closest peer loses because cost is the primary key. + mock_peer(0x02, dest, true, true, 1.0, Some(&[0x50, root])), + mock_peer(0x04, dest, true, true, 0.5, Some(&[root])), + // Same cost and distance: lower address wins. + mock_peer(0x03, dest, true, true, 0.5, Some(&[root])), + ], + ..MockRoutingView::new(false) + }; assert_eq!( - candidates.len(), - 1, - "only peers whose bloom may_reach the dest survive assembly" + choose_candidate(&rv, &dest, &dest_coords, &my_coords), + Some(make_node_addr(0x03)) ); - let c = &candidates[0]; - assert_eq!(c.addr, reacher); - assert!(c.can_send); - assert_eq!(c.link_cost, 2.5); - assert_eq!(c.coords, Some(reacher_coords)); } /// Extract the error-PDU msg_type byte from an encoded routing-error action. diff --git a/src/proto/routing/tests/util.rs b/src/proto/routing/tests/util.rs index 7f96818f..25a844fe 100644 --- a/src/proto/routing/tests/util.rs +++ b/src/proto/routing/tests/util.rs @@ -7,6 +7,7 @@ use crate::{NodeAddr, TreeCoordinate}; /// A mock peer for the candidate-assembly seam: the set of destinations its /// bloom filter reaches, its send state, link cost, and tree coordinates. +#[derive(Clone)] pub(super) struct MockPeer { pub(super) addr: NodeAddr, pub(super) reach: Vec, @@ -31,13 +32,14 @@ impl MockRoutingView { peers: Vec::new(), } } - - fn peer(&self, addr: &NodeAddr) -> Option<&MockPeer> { - self.peers.iter().find(|p| p.addr == *addr) - } } impl RoutingView for MockRoutingView { + type Peer<'a> + = &'a MockPeer + where + Self: 'a; + fn is_congested(&self, _next_hop: &NodeAddr) -> bool { self.congested } @@ -47,20 +49,25 @@ impl RoutingView for MockRoutingView { .find(|(addr, _)| addr == dest) .map(|(_, coords)| coords.clone()) } - fn peer_addrs(&self) -> Vec { - self.peers.iter().map(|p| p.addr).collect() + fn for_each_peer<'a>(&'a self, mut visitor: impl FnMut(Self::Peer<'a>)) { + for peer in &self.peers { + visitor(peer); + } } - fn peer_may_reach(&self, peer: &NodeAddr, dest: &NodeAddr) -> bool { - self.peer(peer).is_some_and(|p| p.reach.contains(dest)) + fn peer_addr<'a>(&'a self, peer: Self::Peer<'a>) -> NodeAddr { + peer.addr } - fn peer_can_send(&self, peer: &NodeAddr) -> bool { - self.peer(peer).is_some_and(|p| p.can_send) + fn peer_may_reach<'a>(&'a self, peer: Self::Peer<'a>, dest: &NodeAddr) -> bool { + peer.reach.contains(dest) } - fn peer_link_cost(&self, peer: &NodeAddr) -> f64 { - self.peer(peer).map_or(f64::INFINITY, |p| p.link_cost) + fn peer_can_send<'a>(&'a self, peer: Self::Peer<'a>) -> bool { + peer.can_send } - fn peer_coords(&self, peer: &NodeAddr) -> Option { - self.peer(peer).and_then(|p| p.coords.clone()) + fn peer_link_cost<'a>(&'a self, peer: Self::Peer<'a>) -> f64 { + peer.link_cost + } + fn peer_coords<'a>(&'a self, peer: Self::Peer<'a>) -> Option<&'a TreeCoordinate> { + peer.coords.as_ref() } }