laantungir/fips
1
0
Fork 0
mirror of https://github.com/jmcorgan/fips.git synced 2026-07-30 19:46:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix transit node coord_cache expiry breaking multi-hop routing

Browse Source 
Transit nodes cache destination coordinates when they forward
SessionSetup messages (via try_warm_coord_cache). These coord_cache
entries have a 5-minute TTL, after which they expire. Once expired,
the transit node can no longer forward data packets for that
destination — find_next_hop returns None and the node sends
CoordsRequired errors back to the source. This creates a permanent
routing failure for any multi-hop path after 5 minutes of the initial
session establishment, even if traffic is actively flowing.

The root cause was that find_next_hop used coord_cache.get(), a
read-only lookup that checks expiry but never extends it. Active
forwarding did not keep the cache warm. Meanwhile, get_and_touch()
existed but only updated last_used without extending expires_at.

Fix:
- find_next_hop now calls coord_cache.get_and_touch() instead of get()
- get_and_touch now calls entry.refresh() instead of entry.touch(),
  which extends expires_at by the default TTL on each access
- find_next_hop signature changed from &self to &mut self to allow
  the mutable cache access

This ensures that as long as traffic flows through a transit node,
the coord_cache entries stay warm and routing continues to work.
Entries still expire after 5 minutes of inactivity as designed.
This commit is contained in:
Johnathan Corgan
2026-02-16 12:41:45 +00:00
parent 930f139787
commit 5987cbfb69
3 changed files with 64 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/cache/coord_cache.rs
Vendored
+3 -3
View File
@@ -111,7 +111,7 @@ impl CoordCache {
})
}
/// Look up coordinates and touch (update last_used).
/// Look up coordinates and refresh (update last_used and extend TTL).
pub fn get_and_touch(
&mut self,
addr: &NodeAddr,
@@ -125,9 +125,9 @@ impl CoordCache {
return None;
}
// Touch and return
// Refresh TTL and return
if let Some(entry) = self.entries.get_mut(addr) {
entry.touch(current_time_ms);
entry.refresh(current_time_ms, self.default_ttl_ms);
Some(entry.coords())
} else {
None
src/node/mod.rs
+5 -5
View File
@@ -896,7 +896,7 @@ impl Node {
/// fallback). Without coordinates, the node cannot make loop-free
/// forwarding decisions. The caller should signal `CoordsRequired` back
/// to the source when `None` is returned for a non-local destination.
pub fn find_next_hop(&self, dest_node_addr: &NodeAddr) -> Option<&ActivePeer> {
pub fn find_next_hop(&mut self, dest_node_addr: &NodeAddr) -> Option<&ActivePeer> {
// 1. Local delivery
if dest_node_addr == self.node_addr() {
return None;
@@ -915,17 +915,17 @@ impl Node {
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_millis() as u64)
.unwrap_or(0);
let dest_coords = self.coord_cache.get(dest_node_addr, now_ms)
.or_else(|| self.route_cache.get(dest_node_addr).map(|c| c.coords()))?;
let dest_coords = self.coord_cache.get_and_touch(dest_node_addr, now_ms)
.or_else(|| self.route_cache.get(dest_node_addr).map(|c| c.coords()))?.clone();
// 3. Bloom filter candidates — requires dest_coords for loop-free selection
let candidates: Vec<&ActivePeer> = self.destination_in_filters(dest_node_addr);
if !candidates.is_empty() {
return self.select_best_candidate(&candidates, dest_coords);
return self.select_best_candidate(&candidates, &dest_coords);
}
// 4. Greedy tree routing fallback
let next_hop_id = self.tree_state.find_next_hop(dest_coords)?;
let next_hop_id = self.tree_state.find_next_hop(&dest_coords)?;
self.peers.get(&next_hop_id).filter(|p| p.can_send())
}
src/node/tests/routing.rs
+56 -11
View File
@@ -16,7 +16,7 @@ use std::collections::HashSet;
#[test]
fn test_routing_local_delivery() {
let node = make_node();
let mut node = make_node();
let my_addr = *node.node_addr();
assert!(node.find_next_hop(&my_addr).is_none());
}
@@ -43,7 +43,7 @@ fn test_routing_direct_peer() {
#[test]
fn test_routing_unknown_destination() {
let node = make_node();
let mut node = make_node();
let unknown = make_node_addr(99);
assert!(node.find_next_hop(&unknown).is_none());
}
@@ -222,6 +222,51 @@ fn test_routing_tree_no_coords_in_cache() {
assert!(node.find_next_hop(&dest).is_none());
}
// === Active routing refreshes coord_cache TTL ===
#[test]
fn test_routing_refreshes_coord_cache_ttl() {
let mut node = make_node();
let transport_id = TransportId::new(1);
let my_addr = *node.node_addr();
// Create a peer
let link_id = LinkId::new(1);
let (conn, id) = make_completed_connection(&mut node, link_id, transport_id, 1000);
let peer_addr = *id.node_addr();
node.add_connection(conn).unwrap();
node.promote_connection(link_id, id, 2000).unwrap();
// Set up tree coordinates
let dest = make_node_addr(99);
let dest_coords =
TreeCoordinate::from_addrs(vec![dest, peer_addr, my_addr]).unwrap();
node.tree_state_mut().update_peer(
ParentDeclaration::new(peer_addr, my_addr, 1, 1000),
TreeCoordinate::from_addrs(vec![peer_addr, my_addr]).unwrap(),
);
// Insert with a short TTL (10s) — enough to survive until find_next_hop runs
let now_ms = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_millis() as u64)
.unwrap_or(0);
let short_ttl = 10_000; // 10 seconds
node.coord_cache_mut().insert_with_ttl(dest, dest_coords, now_ms, short_ttl);
let original_expiry = node.coord_cache().get_entry(&dest).unwrap().expires_at();
// find_next_hop should succeed and refresh TTL to now + default_ttl (300s)
assert!(node.find_next_hop(&dest).is_some());
// The refresh should have extended expires_at beyond the original
let new_expiry = node.coord_cache().get_entry(&dest).unwrap().expires_at();
assert!(
new_expiry > original_expiry,
"find_next_hop should refresh the coord_cache TTL: original={}, new={}",
original_expiry, new_expiry,
);
}
// === Bloom filter without coords → no route (loop prevention) ===
#[test]
@@ -369,9 +414,10 @@ async fn test_routing_chain_topology() {
// Node 0 should be able to route toward node 3.
// The next hop should be node 1 (only peer of node 0).
let node1_addr = *nodes[1].node.node_addr();
let node2_addr = *nodes[2].node.node_addr();
let hop = nodes[0].node.find_next_hop(&node3_addr);
assert!(hop.is_some(), "Node 0 should find route to node 3");
let node1_addr = *nodes[1].node.node_addr();
assert_eq!(
hop.unwrap().node_addr(),
&node1_addr,
@@ -381,7 +427,6 @@ async fn test_routing_chain_topology() {
// Node 3 should route toward node 0 via node 2.
let hop = nodes[3].node.find_next_hop(&node0_addr);
assert!(hop.is_some(), "Node 3 should find route to node 0");
let node2_addr = *nodes[2].node.node_addr();
assert_eq!(
hop.unwrap().node_addr(),
&node2_addr,
@@ -466,7 +511,7 @@ fn build_addr_index(nodes: &[TestNode]) -> std::collections::HashMap<NodeAddr, u
/// the result to the next node. Terminates on delivery, routing failure,
/// or loop detection.
fn simulate_forwarding(
nodes: &[TestNode],
nodes: &mut [TestNode],
addr_index: &std::collections::HashMap<NodeAddr, usize>,
src: usize,
dst: usize,
@@ -586,7 +631,7 @@ async fn test_routing_reachability_100_nodes() {
total_pairs += 1;
match simulate_forwarding(&nodes, &addr_index, src, dst) {
match simulate_forwarding(&mut nodes, &addr_index, src, dst) {
ForwardResult::Delivered(hops) => {
total_hops += hops;
if hops > max_hops {
@@ -705,7 +750,7 @@ async fn test_routing_stops_after_peer_removal() {
// Verify routing works before removal: node 0 → node 3
let addr_index = build_addr_index(&nodes);
match simulate_forwarding(&nodes, &addr_index, 0, 3) {
match simulate_forwarding(&mut nodes, &addr_index, 0, 3) {
ForwardResult::Delivered(_) => {}
other => panic!("Expected delivery before removal, got {:?}", other),
}
@@ -746,7 +791,7 @@ async fn test_routing_stops_after_peer_removal() {
// still attempt forwarding — but the self-distance check prevents loops.
// Either NoRoute or Loop-with-stale-coords is acceptable here; what
// matters is that delivery does NOT succeed.
match simulate_forwarding(&nodes, &addr_index, 0, 3) {
match simulate_forwarding(&mut nodes, &addr_index, 0, 3) {
ForwardResult::NoRoute { .. } => {} // Expected: can't reach node 3
ForwardResult::Loop { .. } => {} // Also acceptable: stale coords cause loop detection
ForwardResult::Delivered(hops) => {
@@ -755,7 +800,7 @@ async fn test_routing_stops_after_peer_removal() {
}
// But routing within the same component still works: node 2 → node 3
match simulate_forwarding(&nodes, &addr_index, 2, 3) {
match simulate_forwarding(&mut nodes, &addr_index, 2, 3) {
ForwardResult::Delivered(_) => {}
other => panic!("Expected delivery within component, got {:?}", other),
}
@@ -898,7 +943,7 @@ async fn test_routing_source_only_coords_100_nodes() {
.coord_cache_mut()
.insert(*dest_addr, dest_coords.clone(), now_ms);
match simulate_forwarding(&nodes, &addr_index, src, dst) {
match simulate_forwarding(&mut nodes, &addr_index, src, dst) {
ForwardResult::Delivered(_) => source_only_delivered += 1,
ForwardResult::NoRoute { .. } => source_only_failed += 1,
ForwardResult::Loop { .. } => {
@@ -940,7 +985,7 @@ async fn test_routing_source_only_coords_100_nodes() {
let mut full_cache_failures = 0usize;
for &(src, dst) in &sample_pairs {
match simulate_forwarding(&nodes, &addr_index, src, dst) {
match simulate_forwarding(&mut nodes, &addr_index, src, dst) {
ForwardResult::Delivered(_) => {}
_ => full_cache_failures += 1,
}