From f374370e5cf2f16c0bfba4f2e5a6c8dc2b12d2be Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Mon, 16 Feb 2026 22:50:39 +0000 Subject: [PATCH] Cache architecture: identity fix, cache merge, parent-change flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Identity cache: remove TTL-based expiry (60s TTL broke active sessions after expiry since handle_tun_outbound checks identity_cache before session table). Replace with LRU-only eviction bounded by configurable identity_size (default 10K). Lookup now touches timestamp for LRU freshness. Cache merge: unify coord_cache and route_cache into single coordinate cache. Both stored NodeAddr→TreeCoordinate; the layer distinction was conceptual, not functional. Discovery-sourced entries now get the same TTL+refresh treatment as session-sourced entries. Simplifies find_next_hop() to single cache lookup. Parent-change flush: clear coord_cache after recompute_coords() in both parent-switch paths of handle_tree_announce(). Stale coordinates after tree reconvergence cause dead-end routing that's more expensive than re-discovery. Tested: 493 unit tests passed, clippy clean, Docker mesh 20/20, Docker chain 6/6. --- src/cache/mod.rs | 7 +- src/cache/route_cache.rs | 364 --------------------------------- src/config/node.rs | 15 +- src/lib.rs | 2 +- src/node/handlers/discovery.rs | 8 +- src/node/handlers/session.rs | 5 +- src/node/mod.rs | 57 ++---- src/node/tests/discovery.rs | 51 +++-- src/node/tests/routing.rs | 21 +- src/node/tests/session.rs | 53 ++--- src/node/tree.rs | 4 +- 11 files changed, 108 insertions(+), 479 deletions(-) delete mode 100644 src/cache/route_cache.rs diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 32f3ae9..4144126 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -1,18 +1,15 @@ //! Caching Entities //! -//! Coordinate and route caching for FIPS routing. The CoordCache stores -//! address-to-coordinate mappings populated by session setup, while -//! RouteCache stores coordinates learned from discovery queries. +//! Coordinate caching for FIPS routing. The CoordCache stores +//! address-to-coordinate mappings populated by session setup and discovery. mod coord_cache; mod entry; -mod route_cache; use thiserror::Error; pub use coord_cache::{CoordCache, DEFAULT_COORD_CACHE_SIZE, DEFAULT_COORD_CACHE_TTL_MS}; pub use entry::CacheEntry; -pub use route_cache::{CachedCoords, RouteCache, DEFAULT_ROUTE_CACHE_SIZE}; /// Errors related to cache operations. #[derive(Debug, Error)] diff --git a/src/cache/route_cache.rs b/src/cache/route_cache.rs deleted file mode 100644 index 0d8533b..0000000 --- a/src/cache/route_cache.rs +++ /dev/null @@ -1,364 +0,0 @@ -//! Route cache for discovered destinations. -//! -//! Separate from CoordCache, this stores routes learned from the discovery -//! protocol (LookupRequest/LookupResponse) rather than session establishment. - -use std::collections::HashMap; - -use crate::tree::TreeCoordinate; -use crate::NodeAddr; - -/// Default maximum entries in route cache. -pub const DEFAULT_ROUTE_CACHE_SIZE: usize = 10_000; - -/// A cached route from discovery. -#[derive(Clone, Debug)] -pub struct CachedCoords { - /// The coordinates discovered. - coords: TreeCoordinate, - /// When this was discovered (Unix milliseconds). - discovered_at: u64, - /// Last time we used this route (Unix milliseconds). - last_used: u64, -} - -impl CachedCoords { - /// Create a new cached route. - pub fn new(coords: TreeCoordinate, discovered_at: u64) -> Self { - Self { - coords, - discovered_at, - last_used: discovered_at, - } - } - - /// Get the coordinates. - pub fn coords(&self) -> &TreeCoordinate { - &self.coords - } - - /// Get the discovery timestamp. - pub fn discovered_at(&self) -> u64 { - self.discovered_at - } - - /// Get the last used timestamp. - pub fn last_used(&self) -> u64 { - self.last_used - } - - /// Touch (update last_used). - pub fn touch(&mut self, current_time_ms: u64) { - self.last_used = current_time_ms; - } - - /// Age since discovery. - pub fn age(&self, current_time_ms: u64) -> u64 { - current_time_ms.saturating_sub(self.discovered_at) - } - - /// Idle time since last use. - pub fn idle_time(&self, current_time_ms: u64) -> u64 { - current_time_ms.saturating_sub(self.last_used) - } - - /// Update coordinates (re-discovered). - pub fn update(&mut self, coords: TreeCoordinate, current_time_ms: u64) { - self.coords = coords; - self.discovered_at = current_time_ms; - self.last_used = current_time_ms; - } -} - -/// Route cache for discovered destinations. -/// -/// Separate from CoordCache, this stores routes learned from the discovery -/// protocol (LookupRequest/LookupResponse) rather than session establishment. -/// Keyed by NodeAddr. -#[derive(Clone, Debug)] -pub struct RouteCache { - /// NodeAddr -> discovered coordinates. - entries: HashMap, - /// Maximum entries. - max_entries: usize, -} - -impl RouteCache { - /// Create a new route cache. - pub fn new(max_entries: usize) -> Self { - Self { - entries: HashMap::with_capacity(max_entries.min(1000)), - max_entries, - } - } - - /// Create with default capacity. - pub fn with_defaults() -> Self { - Self::new(DEFAULT_ROUTE_CACHE_SIZE) - } - - /// Get the maximum capacity. - pub fn max_entries(&self) -> usize { - self.max_entries - } - - /// Insert a discovered route. - pub fn insert(&mut self, node_addr: NodeAddr, coords: TreeCoordinate, current_time_ms: u64) { - // Update existing - if let Some(entry) = self.entries.get_mut(&node_addr) { - entry.update(coords, current_time_ms); - return; - } - - // Evict if full - if self.entries.len() >= self.max_entries { - self.evict_lru(current_time_ms); - } - - self.entries - .insert(node_addr, CachedCoords::new(coords, current_time_ms)); - } - - /// Look up a route (without touching). - pub fn get(&self, node_addr: &NodeAddr) -> Option<&CachedCoords> { - self.entries.get(node_addr) - } - - /// Look up and touch. - pub fn get_and_touch( - &mut self, - node_addr: &NodeAddr, - current_time_ms: u64, - ) -> Option<&TreeCoordinate> { - if let Some(entry) = self.entries.get_mut(node_addr) { - entry.touch(current_time_ms); - Some(entry.coords()) - } else { - None - } - } - - /// Remove a route (e.g., after route failure). - pub fn invalidate(&mut self, node_addr: &NodeAddr) -> Option { - self.entries.remove(node_addr) - } - - /// Check if a node is cached. - pub fn contains(&self, node_addr: &NodeAddr) -> bool { - self.entries.contains_key(node_addr) - } - - /// Number of cached routes. - pub fn len(&self) -> usize { - self.entries.len() - } - - /// Check if empty. - pub fn is_empty(&self) -> bool { - self.entries.is_empty() - } - - /// Clear all routes. - pub fn clear(&mut self) { - self.entries.clear(); - } - - /// Evict routes older than a threshold. - pub fn evict_older_than(&mut self, max_age_ms: u64, current_time_ms: u64) -> usize { - let before = self.entries.len(); - self.entries - .retain(|_, entry| entry.age(current_time_ms) < max_age_ms); - before - self.entries.len() - } - - fn evict_lru(&mut self, current_time_ms: u64) { - let lru_id = self - .entries - .iter() - .max_by_key(|(_, e)| e.idle_time(current_time_ms)) - .map(|(k, _)| *k); - - if let Some(id) = lru_id { - self.entries.remove(&id); - } - } -} - -impl Default for RouteCache { - fn default() -> Self { - Self::with_defaults() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn make_node_addr(val: u8) -> NodeAddr { - let mut bytes = [0u8; 16]; - bytes[0] = val; - NodeAddr::from_bytes(bytes) - } - - fn make_coords(ids: &[u8]) -> TreeCoordinate { - TreeCoordinate::from_addrs(ids.iter().map(|&v| make_node_addr(v)).collect()).unwrap() - } - - #[test] - fn test_cached_coords() { - let coords = make_coords(&[1, 0]); - let mut cached = CachedCoords::new(coords.clone(), 1000); - - assert_eq!(cached.coords(), &coords); - assert_eq!(cached.discovered_at(), 1000); - assert_eq!(cached.last_used(), 1000); - - cached.touch(1500); - assert_eq!(cached.last_used(), 1500); - assert_eq!(cached.idle_time(1600), 100); - assert_eq!(cached.age(1600), 600); - } - - #[test] - fn test_route_cache_basic() { - let mut cache = RouteCache::new(100); - let node = make_node_addr(1); - let coords = make_coords(&[1, 0]); - - cache.insert(node, coords.clone(), 0); - - assert!(cache.contains(&node)); - assert_eq!(cache.get(&node).unwrap().coords(), &coords); - } - - #[test] - fn test_route_cache_invalidate() { - let mut cache = RouteCache::new(100); - let node = make_node_addr(1); - let coords = make_coords(&[1, 0]); - - cache.insert(node, coords, 0); - assert!(cache.contains(&node)); - - cache.invalidate(&node); - assert!(!cache.contains(&node)); - } - - #[test] - fn test_route_cache_lru_eviction() { - let mut cache = RouteCache::new(2); - - let node1 = make_node_addr(1); - let node2 = make_node_addr(2); - let node3 = make_node_addr(3); - - cache.insert(node1, make_coords(&[1, 0]), 0); - cache.insert(node2, make_coords(&[2, 0]), 100); - - // Touch node2 - let _ = cache.get_and_touch(&node2, 200); - - // Insert node3 - cache.insert(node3, make_coords(&[3, 0]), 300); - - // node1 should be evicted - assert!(!cache.contains(&node1)); - assert!(cache.contains(&node2)); - assert!(cache.contains(&node3)); - } - - #[test] - fn test_route_cache_evict_older_than() { - let mut cache = RouteCache::new(100); - - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 500); - cache.insert(make_node_addr(3), make_coords(&[3, 0]), 1000); - - let evicted = cache.evict_older_than(600, 1000); - - assert_eq!(evicted, 1); // node1 is > 600ms old - assert_eq!(cache.len(), 2); - } - - #[test] - fn test_route_cache_update() { - let mut cache = RouteCache::new(100); - let node = make_node_addr(1); - - cache.insert(node, make_coords(&[1, 0]), 0); - cache.insert(node, make_coords(&[1, 2, 0]), 500); - - assert_eq!(cache.len(), 1); - let cached = cache.get(&node).unwrap(); - assert_eq!(cached.coords().depth(), 2); - assert_eq!(cached.discovered_at(), 500); - } - - #[test] - fn test_cached_coords_update() { - let mut cached = CachedCoords::new(make_coords(&[1, 0]), 1000); - - let new_coords = make_coords(&[1, 2, 0]); - cached.update(new_coords.clone(), 2000); - - assert_eq!(cached.coords(), &new_coords); - assert_eq!(cached.discovered_at(), 2000); - assert_eq!(cached.last_used(), 2000); - } - - #[test] - fn test_route_cache_get_and_touch() { - let mut cache = RouteCache::new(100); - let node = make_node_addr(1); - let coords = make_coords(&[1, 0]); - - cache.insert(node, coords.clone(), 0); - - let result = cache.get_and_touch(&node, 500); - assert_eq!(result, Some(&coords)); - - // Verify last_used was updated - let entry = cache.get(&node).unwrap(); - assert_eq!(entry.last_used(), 500); - } - - #[test] - fn test_route_cache_get_and_touch_missing() { - let mut cache = RouteCache::new(100); - let result = cache.get_and_touch(&make_node_addr(99), 0); - assert!(result.is_none()); - } - - #[test] - fn test_route_cache_clear_and_is_empty() { - let mut cache = RouteCache::new(100); - - assert!(cache.is_empty()); - - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); - - assert!(!cache.is_empty()); - assert_eq!(cache.len(), 2); - - cache.clear(); - assert!(cache.is_empty()); - assert_eq!(cache.len(), 0); - } - - #[test] - fn test_route_cache_default() { - let cache = RouteCache::default(); - - assert_eq!(cache.max_entries(), DEFAULT_ROUTE_CACHE_SIZE); - assert!(cache.is_empty()); - } - - #[test] - fn test_route_cache_invalidate_missing() { - let mut cache = RouteCache::new(100); - let result = cache.invalidate(&make_node_addr(99)); - assert!(result.is_none()); - } -} diff --git a/src/config/node.rs b/src/config/node.rs index 289d77e..c809923 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -116,12 +116,9 @@ pub struct CacheConfig { /// Coord cache entry TTL in seconds (`node.cache.coord_ttl_secs`). #[serde(default = "CacheConfig::default_coord_ttl_secs")] pub coord_ttl_secs: u64, - /// Max entries in route cache (`node.cache.route_size`). - #[serde(default = "CacheConfig::default_route_size")] - pub route_size: usize, - /// Identity cache entry TTL in seconds (`node.cache.identity_ttl_secs`). - #[serde(default = "CacheConfig::default_identity_ttl_secs")] - pub identity_ttl_secs: u64, + /// Max entries in identity cache (`node.cache.identity_size`). + #[serde(default = "CacheConfig::default_identity_size")] + pub identity_size: usize, } impl Default for CacheConfig { @@ -129,8 +126,7 @@ impl Default for CacheConfig { Self { coord_size: 50_000, coord_ttl_secs: 300, - route_size: 10_000, - identity_ttl_secs: 60, + identity_size: 10_000, } } } @@ -138,8 +134,7 @@ impl Default for CacheConfig { impl CacheConfig { fn default_coord_size() -> usize { 50_000 } fn default_coord_ttl_secs() -> u64 { 300 } - fn default_route_size() -> usize { 10_000 } - fn default_identity_ttl_secs() -> u64 { 60 } + fn default_identity_size() -> usize { 10_000 } } /// Discovery protocol (`node.discovery.*`). diff --git a/src/lib.rs b/src/lib.rs index b09fbe7..4e91a56 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,7 +48,7 @@ pub use protocol::{ }; // Re-export cache types -pub use cache::{CacheEntry, CacheError, CacheStats, CachedCoords, CoordCache, RouteCache}; +pub use cache::{CacheEntry, CacheError, CacheStats, CoordCache}; // Re-export peer types pub use peer::{ diff --git a/src/node/handlers/discovery.rs b/src/node/handlers/discovery.rs index 21eba5e..d29e558 100644 --- a/src/node/handlers/discovery.rs +++ b/src/node/handlers/discovery.rs @@ -92,7 +92,7 @@ impl Node { /// Processing steps: /// 1. Decode and validate /// 2. Check recent_requests to determine if we originated or are forwarding - /// 3. If originator: cache target_coords in route_cache + /// 3. If originator: cache target_coords in coord_cache /// 4. If transit: reverse-path forward to from_peer pub(in crate::node) async fn handle_lookup_response( &mut self, @@ -139,7 +139,7 @@ impl Node { ); let target = response.target; - self.route_cache.insert( + self.coord_cache.insert( target, response.target_coords, now_ms, @@ -149,7 +149,7 @@ impl Node { self.pending_lookups.remove(&target); // If we have pending TUN packets for this target, retry session - // initiation. The route_cache now has coords, so find_next_hop() + // initiation. The coord_cache now has coords, so find_next_hop() // should succeed. if self.pending_tun_packets.contains_key(&target) { self.retry_session_after_discovery(target).await; @@ -261,7 +261,7 @@ impl Node { /// Creates a LookupRequest and floods it to all peers. The originator /// does NOT record the request_id in recent_requests, so when the /// response arrives, it's recognized as "our request" and the - /// target's coordinates are cached in route_cache. + /// target's coordinates are cached in coord_cache. pub(in crate::node) async fn initiate_lookup(&mut self, target: &NodeAddr, ttl: u8) { let origin = *self.node_addr(); let origin_coords = self.tree_state().my_coords().clone(); diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 7851707..c6e84a0 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -497,9 +497,6 @@ impl Node { if let Some(coords) = self.coord_cache.get(dest, now_ms) { return coords.clone(); } - if let Some(cached) = self.route_cache.get(dest) { - return cached.coords().clone(); - } // Fallback: use our own coordinates. The SessionSetup dest_coords // field cannot be empty (wire format requires ≥1 entry). Using our // own coords is safe — transit routers will still cache them, and @@ -675,7 +672,7 @@ impl Node { /// Retry session initiation after discovery provided coordinates. /// /// Called when a LookupResponse arrives and we have pending TUN packets - /// for the discovered target. The route_cache now has coords, so + /// for the discovered target. The coord_cache now has coords, so /// `find_next_hop()` should succeed and the SessionSetup can be sent. pub(in crate::node) async fn retry_session_after_discovery(&mut self, dest_addr: NodeAddr) { // Look up the destination's public key from the identity cache diff --git a/src/node/mod.rs b/src/node/mod.rs index bbb38ff..0045817 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -16,7 +16,7 @@ mod tree; mod tests; use crate::bloom::BloomState; -use crate::cache::{CoordCache, RouteCache}; +use crate::cache::CoordCache; use crate::utils::index::IndexAllocator; use crate::node::session::SessionEntry; use crate::peer::{ActivePeer, PeerConnection}; @@ -215,10 +215,8 @@ pub struct Node { bloom_state: BloomState, // === Routing === - /// Address -> coordinates cache (from session setup). + /// Address -> coordinates cache (from session setup and discovery). coord_cache: CoordCache, - /// Discovered routes (from discovery protocol). - route_cache: RouteCache, /// Recent discovery requests (dedup + reverse-path forwarding). /// Maps request_id → RecentRequest. recent_requests: HashMap, @@ -360,7 +358,6 @@ impl Node { config.node.cache.coord_size, config.node.cache.coord_ttl_secs * 1000, ); - let route_cache = RouteCache::new(config.node.cache.route_size); let rl = &config.node.rate_limit; let msg1_rate_limiter = HandshakeRateLimiter::with_params( rate_limit::TokenBucket::with_params(rl.handshake_burst, rl.handshake_rate), @@ -379,7 +376,6 @@ impl Node { tree_state, bloom_state, coord_cache, - route_cache, recent_requests: HashMap::new(), transports: HashMap::new(), links: HashMap::new(), @@ -438,7 +434,6 @@ impl Node { config.node.cache.coord_size, config.node.cache.coord_ttl_secs * 1000, ); - let route_cache = RouteCache::new(config.node.cache.route_size); let rl = &config.node.rate_limit; let msg1_rate_limiter = HandshakeRateLimiter::with_params( rate_limit::TokenBucket::with_params(rl.handshake_burst, rl.handshake_rate), @@ -457,7 +452,6 @@ impl Node { tree_state, bloom_state, coord_cache, - route_cache, recent_requests: HashMap::new(), transports: HashMap::new(), links: HashMap::new(), @@ -639,18 +633,6 @@ impl Node { &mut self.coord_cache } - // === Route Cache === - - /// Get the route cache (discovery protocol). - pub fn route_cache(&self) -> &RouteCache { - &self.route_cache - } - - /// Get mutable route cache. - pub fn route_cache_mut(&mut self) -> &mut RouteCache { - &mut self.route_cache - } - // === TUN Interface === /// Get the TUN state. @@ -891,19 +873,22 @@ impl Node { let mut prefix = [0u8; 15]; prefix.copy_from_slice(&node_addr.as_bytes()[0..15]); self.identity_cache.insert(prefix, (node_addr, pubkey, Self::now_ms())); + // LRU eviction + let max = self.config.node.cache.identity_size; + if self.identity_cache.len() > max + && let Some(oldest_key) = self.identity_cache.iter() + .min_by_key(|(_, (_, _, ts))| *ts) + .map(|(k, _)| *k) + { + self.identity_cache.remove(&oldest_key); + } } /// Look up a destination by FipsAddress prefix (bytes 1-15 of the IPv6 address). - /// Returns None if the entry has expired (lazy expiry). pub(crate) fn lookup_by_fips_prefix(&mut self, prefix: &[u8; 15]) -> Option<(NodeAddr, secp256k1::PublicKey)> { - let ttl_ms = self.config.node.cache.identity_ttl_secs * 1000; - let now_ms = Self::now_ms(); - if let Some(&(addr, pk, registered_at)) = self.identity_cache.get(prefix) { - if now_ms.saturating_sub(registered_at) > ttl_ms { - self.identity_cache.remove(prefix); - return None; - } - Some((addr, pk)) + if let Some(entry) = self.identity_cache.get_mut(prefix) { + entry.2 = Self::now_ms(); // LRU touch + Some((entry.0, entry.1)) } else { None } @@ -931,10 +916,10 @@ impl Node { /// 5. No route → `None` /// /// Both the bloom filter and tree routing paths require cached destination - /// coordinates (checked in `coord_cache` first, then `route_cache` as - /// 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. + /// coordinates (checked in `coord_cache`). 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(&mut self, dest_node_addr: &NodeAddr) -> Option<&ActivePeer> { // 1. Local delivery if dest_node_addr == self.node_addr() { @@ -948,14 +933,12 @@ impl Node { return Some(peer); } - // Look up destination coords (required by both bloom and tree paths). - // Try coord_cache first (session-based), then route_cache (discovery-based). + // Look up cached destination coordinates (required by both bloom and tree paths). let now_ms = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - 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(); + let dest_coords = self.coord_cache.get_and_touch(dest_node_addr, now_ms)?.clone(); // 3. Bloom filter candidates — requires dest_coords for loop-free selection let candidates: Vec<&ActivePeer> = self.destination_in_filters(dest_node_addr); diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index 1f334e9..c76156d 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -110,7 +110,7 @@ async fn test_response_decode_error() { let from = make_node_addr(0xAA); node.handle_lookup_response(&from, &[0x00; 10]).await; // No panic, no route cached - assert!(node.route_cache.is_empty()); + assert!(node.coord_cache().is_empty()); } #[tokio::test] @@ -134,10 +134,13 @@ async fn test_response_originator_caches_route() { node.handle_lookup_response(&from, payload).await; - // Route should be cached - assert!(node.route_cache.contains(&target)); - let cached = node.route_cache.get(&target).unwrap(); - assert_eq!(cached.coords(), &coords); + // Route should be cached in coord_cache + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); + assert!(node.coord_cache().contains(&target, now_ms)); + assert_eq!(node.coord_cache().get(&target, now_ms).unwrap(), &coords); } #[tokio::test] @@ -169,8 +172,12 @@ async fn test_response_transit_needs_recent_request() { // (will fail silently since 0xDD is not an actual peer) node.handle_lookup_response(&from, payload).await; - // Should NOT cache in route_cache (we're transit, not originator) - assert!(!node.route_cache.contains(&target)); + // Should NOT cache in coord_cache (we're transit, not originator) + let now_ms2 = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); + assert!(!node.coord_cache().contains(&target, now_ms2)); } // ============================================================================ @@ -275,8 +282,12 @@ async fn test_request_target_found_generates_response() { } // Node0 should have cached node1's route (it originated the request) + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); assert!( - nodes[0].node.route_cache.contains(&node1_addr), + nodes[0].node.coord_cache().contains(&node1_addr, now_ms), "Node 0 should have cached node 1's route from LookupResponse" ); @@ -317,8 +328,12 @@ async fn test_request_three_node_chain() { ); // Node0 should have cached node2's route + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); assert!( - nodes[0].node.route_cache.contains(&node2_addr), + nodes[0].node.coord_cache().contains(&node2_addr, now_ms), "Node 0 should have cached node 2's route through 3-node chain" ); @@ -436,13 +451,17 @@ async fn test_discovery_100_nodes() { } } - // Verify: each originator should have the target's coords in route_cache + // Verify: each originator should have the target's coords in coord_cache + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); let mut resolved = 0usize; let mut failed = 0usize; let mut failed_pairs: Vec<(usize, usize)> = Vec::new(); for &(src, dst) in &lookup_pairs { - if nodes[src].node.route_cache.contains(&all_addrs[dst]) { + if nodes[src].node.coord_cache().contains(&all_addrs[dst], now_ms) { resolved += 1; } else { failed += 1; @@ -463,12 +482,12 @@ async fn test_discovery_100_nodes() { resolved as f64 / total_lookups as f64 * 100.0 ); - // Report route_cache stats across all nodes - let total_cached: usize = nodes.iter().map(|tn| tn.node.route_cache.len()).sum(); - let min_cached = nodes.iter().map(|tn| tn.node.route_cache.len()).min().unwrap(); - let max_cached = nodes.iter().map(|tn| tn.node.route_cache.len()).max().unwrap(); + // Report coord_cache stats across all nodes + let total_cached: usize = nodes.iter().map(|tn| tn.node.coord_cache().len()).sum(); + let min_cached = nodes.iter().map(|tn| tn.node.coord_cache().len()).min().unwrap(); + let max_cached = nodes.iter().map(|tn| tn.node.coord_cache().len()).max().unwrap(); eprintln!( - " Route cache entries: total={} min={} max={} avg={:.1}", + " Coord cache entries: total={} min={} max={} avg={:.1}", total_cached, min_cached, max_cached, diff --git a/src/node/tests/routing.rs b/src/node/tests/routing.rs index f078f77..86c14ac 100644 --- a/src/node/tests/routing.rs +++ b/src/node/tests/routing.rs @@ -303,14 +303,13 @@ fn test_routing_bloom_hit_without_coords_returns_none() { assert!(node.find_next_hop(&dest).is_none()); } -// === Route cache fallback === +// === Discovery-populated coord_cache === #[test] -fn test_routing_route_cache_fallback() { - // Verify that find_next_hop() falls back to route_cache when - // coord_cache has no entry. This is the key change that enables - // discovery-based routing: initiate_lookup() populates route_cache, - // and find_next_hop() now consults it. +fn test_routing_discovery_coord_cache() { + // Verify that find_next_hop() uses coord_cache entries populated by + // discovery. initiate_lookup() populates coord_cache, and + // find_next_hop() consults it. let mut node = make_node(); let transport_id = TransportId::new(1); let my_addr = *node.node_addr(); @@ -346,15 +345,15 @@ fn test_routing_route_cache_fallback() { .unwrap_or(0); assert!(node.coord_cache().get(&dest, now_ms).is_none()); - // Without route_cache entry, should return None (same as before) + // Without coord_cache entry, should return None assert!(node.find_next_hop(&dest).is_none()); - // Now populate route_cache (as discovery would do) - node.route_cache_mut().insert(dest, dest_coords, now_ms); + // Now populate coord_cache (as discovery would do) + node.coord_cache_mut().insert(dest, dest_coords, now_ms); - // find_next_hop should succeed via route_cache fallback + // find_next_hop should succeed via coord_cache let result = node.find_next_hop(&dest); - assert!(result.is_some(), "Should route via route_cache fallback"); + assert!(result.is_some(), "Should route via coord_cache"); assert_eq!( result.unwrap().node_addr(), &peer_addr, diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index d9a533d..106e6e5 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -732,12 +732,6 @@ async fn test_session_100_nodes() { let min_coord = *coord_cache_sizes.iter().min().unwrap(); let max_coord = *coord_cache_sizes.iter().max().unwrap(); - let route_cache_sizes: Vec = nodes - .iter() - .map(|tn| tn.node.route_cache().len()) - .collect(); - let total_route_entries: usize = route_cache_sizes.iter().sum(); - // === Report === eprintln!("\n === Session 100-Node Test ==="); @@ -813,7 +807,6 @@ async fn test_session_100_nodes() { max_coord, total_coord_entries as f64 / NUM_NODES as f64 ); - eprintln!(" Route cache: total={}", total_route_entries); eprintln!("\n --- Timing ---"); eprintln!( @@ -1307,37 +1300,45 @@ fn test_purge_idle_sessions_disabled_when_zero() { } // ============================================================================ -// Unit tests: Identity cache expiry +// Unit tests: Identity cache // ============================================================================ #[test] -fn test_identity_cache_expiry() { +fn test_identity_cache_lru_eviction() { let mut node = make_node(); - // Use a short TTL (1s) and insert with an old timestamp - node.config.node.cache.identity_ttl_secs = 1; + node.config.node.cache.identity_size = 2; - let remote = Identity::generate(); - let remote_addr = *remote.node_addr(); + let id1 = Identity::generate(); + let id2 = Identity::generate(); + let id3 = Identity::generate(); - let mut prefix = [0u8; 15]; - prefix.copy_from_slice(&remote_addr.as_bytes()[0..15]); + // Insert first two with explicit timestamps to ensure deterministic ordering + let mut prefix1 = [0u8; 15]; + prefix1.copy_from_slice(&id1.node_addr().as_bytes()[0..15]); + node.identity_cache.insert(prefix1, (*id1.node_addr(), id1.pubkey_full(), 1000)); - // Insert directly with a timestamp far in the past (time 0) - node.identity_cache.insert(prefix, (remote_addr, remote.pubkey_full(), 0)); + let mut prefix2 = [0u8; 15]; + prefix2.copy_from_slice(&id2.node_addr().as_bytes()[0..15]); + node.identity_cache.insert(prefix2, (*id2.node_addr(), id2.pubkey_full(), 2000)); - // Lookup should find the entry expired (registered_at=0, TTL=1s, now >> 1s) - let result = node.lookup_by_fips_prefix(&prefix); - assert!(result.is_none(), "Expired identity should return None"); + assert_eq!(node.identity_cache_len(), 2); - // Entry should have been removed from cache - assert!(!node.identity_cache.contains_key(&prefix), - "Expired entry should be removed from cache"); + // Adding a third should evict the oldest (id1, timestamp 1000) + node.register_identity(*id3.node_addr(), id3.pubkey_full()); + assert_eq!(node.identity_cache_len(), 2); + + assert!(node.lookup_by_fips_prefix(&prefix1).is_none(), + "Oldest entry should have been evicted"); + + let mut prefix3 = [0u8; 15]; + prefix3.copy_from_slice(&id3.node_addr().as_bytes()[0..15]); + assert!(node.lookup_by_fips_prefix(&prefix3).is_some(), + "Newest entry should be present"); } #[test] -fn test_identity_cache_survives_before_ttl() { +fn test_identity_cache_lookup() { let mut node = make_node(); - // Default 60s TTL — just registered, should be available let remote = Identity::generate(); let remote_addr = *remote.node_addr(); @@ -1348,7 +1349,7 @@ fn test_identity_cache_survives_before_ttl() { prefix.copy_from_slice(&remote_addr.as_bytes()[0..15]); let result = node.lookup_by_fips_prefix(&prefix); - assert!(result.is_some(), "Fresh identity should be available"); + assert!(result.is_some(), "Registered identity should be available"); let (addr, pk) = result.unwrap(); assert_eq!(addr, remote_addr); diff --git a/src/node/tree.rs b/src/node/tree.rs index 986aeb7..24ad693 100644 --- a/src/node/tree.rs +++ b/src/node/tree.rs @@ -205,13 +205,14 @@ impl Node { return; } self.tree_state.recompute_coords(); + self.coord_cache.clear(); info!( new_parent = %new_parent, new_seq = new_seq, new_root = %self.tree_state.root(), depth = self.tree_state.my_coords().depth(), - "Parent switched, announcing to all peers" + "Parent switched, flushed coord cache, announcing to all peers" ); self.send_tree_announce_to_all().await; @@ -236,6 +237,7 @@ impl Node { return; } self.tree_state.recompute_coords(); + self.coord_cache.clear(); let new_root = *self.tree_state.root(); let new_depth = self.tree_state.my_coords().depth();