diff --git a/src/cache/coord_cache.rs b/src/cache/coord_cache.rs index a0c914a..e832915 100644 --- a/src/cache/coord_cache.rs +++ b/src/cache/coord_cache.rs @@ -9,7 +9,7 @@ use std::collections::HashMap; use super::CacheStats; use super::entry::CacheEntry; use crate::NodeAddr; -use crate::tree::TreeCoordinate; +use crate::proto::stp::TreeCoordinate; /// Default maximum entries in coordinate cache. pub const DEFAULT_COORD_CACHE_SIZE: usize = 50_000; diff --git a/src/cache/entry.rs b/src/cache/entry.rs index ec8f6f6..69c30ff 100644 --- a/src/cache/entry.rs +++ b/src/cache/entry.rs @@ -1,6 +1,6 @@ //! Cache entry with TTL and LRU tracking. -use crate::tree::TreeCoordinate; +use crate::proto::stp::TreeCoordinate; /// A cached coordinate entry. #[derive(Clone, Debug)] diff --git a/src/lib.rs b/src/lib.rs index 9917609..d7d5d33 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,7 +27,6 @@ pub mod protocol; #[cfg(test)] pub(crate) mod testutil; pub mod transport; -pub mod tree; pub mod upper; pub mod utils; pub mod version; @@ -45,8 +44,8 @@ pub use upper::config::{DnsConfig, TunConfig}; // Re-export discovery types pub use discovery::{BootstrapHandoffResult, EstablishedTraversal}; -// Re-export tree types -pub use tree::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError, TreeState}; +// Re-export tree types (relocated from tree:: to proto::stp) +pub use proto::stp::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError, TreeState}; // Re-export bloom filter types pub use bloom::{BloomError, BloomFilter, BloomState}; @@ -62,9 +61,12 @@ pub use transport::{ // Re-export protocol types pub use protocol::{ FilterAnnounce, LinkMessageType, ProtocolError, SessionAck, SessionDatagram, SessionFlags, - SessionMessageType, SessionSetup, TreeAnnounce, + SessionMessageType, SessionSetup, }; +// Re-export STP wire types (relocated from protocol:: to proto::stp) +pub use proto::stp::TreeAnnounce; + // Re-export discovery wire types (relocated from protocol:: to proto::discovery) pub use proto::discovery::{LookupRequest, LookupResponse}; diff --git a/src/node/handlers/mmp.rs b/src/node/handlers/mmp.rs index 4ba64c3..51b927f 100644 --- a/src/node/handlers/mmp.rs +++ b/src/node/handlers/mmp.rs @@ -7,6 +7,7 @@ use crate::NodeAddr; use crate::node::Node; use crate::node::reject::{MmpReject, RejectReason, TreeReject}; +use crate::node::tree::sign_declaration; use crate::proto::mmp::{ BackoffUpdate, LinkReportKind, LinkReportSnapshot, MmpAction, MmpSessionState, PathMtuNotification, PeerLivenessSnapshot, ReceiverReport, RrLog, SendResult, SenderReport, @@ -186,26 +187,36 @@ impl Node { // Trigger re-evaluation so the node doesn't wait for the next // periodic tick or TreeAnnounce. if first_rtt { - let peer_costs: std::collections::HashMap = self + let peer_costs: std::collections::BTreeMap = self .peers .iter() .filter(|(_, p)| p.has_srtt()) .map(|(a, p)| (*a, p.link_cost())) .collect(); + // Wall-clock seconds for the escaping declaration timestamp; + // monotonic ms for the flap-dampening / hold-down timers. + let now_secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let mono_now_ms = crate::mmp::mono_ms(); let skip = self.non_full_peers(); - if let Some(new_parent) = self.tree_state.evaluate_parent(&peer_costs, &skip) { + if let Some(new_parent) = + self.tree_state + .evaluate_parent(&peer_costs, &skip, mono_now_ms) + { let new_seq = self.tree_state.my_declaration().sequence() + 1; - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); - let flap_dampened = self.tree_state.set_parent(new_parent, new_seq, timestamp); + let flap_dampened = + self.tree_state + .set_parent(new_parent, new_seq, now_secs, mono_now_ms); self.tree_state.recompute_coords(); // Clone identity once: sign_declaration borrows &mut tree_state while // the identity() accessor borrows all of &self, so an owned copy avoids // the split-borrow conflict on this infrequent parent-switch path. let our_identity = self.identity().clone(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { warn!(error = %e, "Failed to sign declaration after first-RTT parent eval"); self.metrics() .tree @@ -234,10 +245,12 @@ impl Node { let all_peers: Vec = self.peers.keys().copied().collect(); self.bloom_state.mark_all_updates_needed(all_peers); } else if !self.tree_state.is_root() && self.tree_state.should_be_root() { - self.tree_state.become_root(); + self.tree_state.become_root(now_secs); // Clone identity once (see the parent-switch branch above for why). let our_identity = self.identity().clone(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { warn!(error = %e, "Failed to sign self-root declaration after first-RTT"); self.metrics() .tree diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 72ccb73..025cc81 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -49,8 +49,8 @@ struct PipelinedSend<'a> { timestamp: u32, fsp_flags: u8, inner_plaintext: &'a [u8], - my_coords: Option<&'a crate::tree::TreeCoordinate>, - dest_coords: Option<&'a crate::tree::TreeCoordinate>, + my_coords: Option<&'a crate::proto::stp::TreeCoordinate>, + dest_coords: Option<&'a crate::proto::stp::TreeCoordinate>, } impl Node { @@ -2152,7 +2152,10 @@ impl Node { /// Returns our own coordinates as a fallback (the SessionSetup will /// carry src_coords for return path routing; empty dest_coords /// would fail wire encoding since TreeCoordinate requires ≥1 entry). - pub(in crate::node) fn get_dest_coords(&self, dest: &NodeAddr) -> crate::tree::TreeCoordinate { + pub(in crate::node) fn get_dest_coords( + &self, + dest: &NodeAddr, + ) -> crate::proto::stp::TreeCoordinate { let now_ms = Self::now_ms(); if let Some(coords) = self.coord_cache.get(dest, now_ms) { return coords.clone(); diff --git a/src/node/mod.rs b/src/node/mod.rs index bc1d5c3..823176f 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -62,6 +62,7 @@ use crate::proto::fmp::Fmp; use crate::proto::fmp::NodeProfile; use crate::proto::mmp::Mmp; use crate::proto::routing::{self, Router, RoutingErrorRateLimiter}; +use crate::proto::stp::TreeState; #[cfg(unix)] use crate::transport::ethernet::EthernetTransport; use crate::transport::nym::NymTransport; @@ -72,14 +73,13 @@ use crate::transport::{ ConnectionState, Link, LinkId, PacketRx, PacketTx, TransportAddr, TransportError, TransportHandle, TransportId, }; -use crate::tree::TreeState; use crate::upper::hosts::HostMap; use crate::upper::icmp_rate_limit::IcmpRateLimiter; use crate::upper::tun::{TunError, TunOutboundRx, TunState, TunTx}; use crate::utils::index::IndexAllocator; use crate::{Config, ConfigError, Identity, IdentityError, NodeAddr, PeerIdentity, TreeCoordinate}; use rand::Rng; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; use std::fmt; use std::sync::Arc; use std::thread::JoinHandle; @@ -583,7 +583,11 @@ impl Node { }; // Initialize tree state with signed self-declaration - let mut tree_state = TreeState::new(node_addr); + let tree_now_secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let mut tree_state = TreeState::new(node_addr, tree_now_secs); tree_state.set_parent_hysteresis(config.node.tree.parent_hysteresis); tree_state.set_hold_down(config.node.tree.hold_down_secs); tree_state.set_flap_dampening( @@ -591,8 +595,7 @@ impl Node { config.node.tree.flap_window_secs, config.node.tree.flap_dampening_secs, ); - tree_state - .sign_declaration(&identity) + tree::sign_declaration(tree_state.my_declaration_mut(), &identity) .expect("signing own declaration should never fail"); let coord_cache = CoordCache::new( @@ -746,7 +749,11 @@ impl Node { }; // Initialize tree state with signed self-declaration - let mut tree_state = TreeState::new(node_addr); + let tree_now_secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let mut tree_state = TreeState::new(node_addr, tree_now_secs); tree_state.set_parent_hysteresis(config.node.tree.parent_hysteresis); tree_state.set_hold_down(config.node.tree.hold_down_secs); tree_state.set_flap_dampening( @@ -754,8 +761,7 @@ impl Node { config.node.tree.flap_window_secs, config.node.tree.flap_dampening_secs, ); - tree_state - .sign_declaration(&identity) + tree::sign_declaration(tree_state.my_declaration_mut(), &identity) .expect("signing own declaration should never fail"); let mut bloom_state = BloomState::new(node_addr); @@ -1272,7 +1278,7 @@ impl Node { /// Collect the set of peers that are not full nodes (non-routing/leaf). /// /// Used by tree and routing functions to skip non-transit peers. - fn non_full_peers(&self) -> std::collections::HashSet { + fn non_full_peers(&self) -> BTreeSet { self.peers .iter() .filter(|(_, p)| p.peer_profile() != NodeProfile::Full) @@ -1604,7 +1610,7 @@ impl Node { /// Resolution order: this node when it is root, then the root as a live /// authenticated peer (cryptographically attested npub), then the /// identity-cache, else `None`. - pub(crate) fn resolve_root_npub(&self, tree: &crate::tree::TreeState) -> Option { + pub(crate) fn resolve_root_npub(&self, tree: &crate::proto::stp::TreeState) -> Option { if tree.is_root() { return Some(self.npub()); } diff --git a/src/node/session_wire.rs b/src/node/session_wire.rs index 4e20336..4acf4c4 100644 --- a/src/node/session_wire.rs +++ b/src/node/session_wire.rs @@ -32,8 +32,8 @@ //! | 0x2 | - | Handshake msg2 | SessionAck (Noise XX msg2) | //! | 0x3 | - | Handshake msg3 | SessionMsg3 (Noise XX msg3) | +use crate::proto::stp::TreeCoordinate; use crate::protocol::{ProtocolError, decode_optional_coords}; -use crate::tree::TreeCoordinate; // ============================================================================ // Constants diff --git a/src/node/tests/bloom.rs b/src/node/tests/bloom.rs index a1ac716..48d090a 100644 --- a/src/node/tests/bloom.rs +++ b/src/node/tests/bloom.rs @@ -457,7 +457,7 @@ async fn test_bloom_filter_split_horizon() { fn compute_mesh_size_counts_each_peer_filter_once() { use crate::bloom::BloomFilter; use crate::peer::ActivePeer; - use crate::tree::ParentDeclaration; + use crate::proto::stp::ParentDeclaration; let mut node = make_node(); let my_addr = *node.tree_state().my_node_addr(); @@ -498,8 +498,8 @@ fn compute_mesh_size_counts_each_peer_filter_once() { // Seed parent ancestry first so recompute_coords can extend it and // flip is_root() to false; child ancestry is for completeness. - let parent_ancestry = crate::tree::TreeCoordinate::root_with_meta(parent_addr, 1, 1); - let child_ancestry = crate::tree::TreeCoordinate::root_with_meta(child_addr, 1, 1); + let parent_ancestry = crate::proto::stp::TreeCoordinate::root_with_meta(parent_addr, 1, 1); + let child_ancestry = crate::proto::stp::TreeCoordinate::root_with_meta(child_addr, 1, 1); // Inject the stale-cache scenario: peer_declaration(P) still names // US (M) as P's parent (the pre-switch advert that the cache hasn't // refreshed yet). Q is a legitimate child also naming M as parent. @@ -513,7 +513,7 @@ fn compute_mesh_size_counts_each_peer_filter_once() { .update_peer(child_decl, child_ancestry); // Switch our parent to P and recompute coords so root flips off self. - node.tree_state_mut().set_parent(parent_addr, 2, 1); + node.tree_state_mut().set_parent(parent_addr, 2, 1, 1); node.tree_state_mut().recompute_coords(); assert!( !node.tree_state().is_root(), diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index dafe6db..30ef768 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -6,7 +6,7 @@ use super::*; use crate::proto::discovery::{LookupRequest, LookupResponse, RecentRequest}; -use crate::tree::TreeCoordinate; +use crate::proto::stp::TreeCoordinate; use spanning_tree::{ cleanup_nodes, generate_random_edges, lock_large_network_test, process_available_packets, run_tree_test, run_tree_test_with_mtus, verify_tree_convergence, @@ -1204,7 +1204,7 @@ async fn test_check_pending_lookups_default_sequence_unreachable() { // as its parent. `is_tree_peer` checks both directions — the child // direction (peer.parent_id == self.node_addr) is what we exercise. let our_addr = *node.node_addr(); - let peer_decl = crate::tree::ParentDeclaration::new(peer_addr, our_addr, 1, 0); + let peer_decl = crate::proto::stp::ParentDeclaration::new(peer_addr, our_addr, 1, 0); let peer_coords = TreeCoordinate::from_addrs(vec![peer_addr, our_addr]).unwrap(); node.tree_state_mut().update_peer(peer_decl, peer_coords); assert!(node.is_tree_peer(&peer_addr), "peer must be a tree peer"); diff --git a/src/node/tests/forwarding.rs b/src/node/tests/forwarding.rs index a90f620..a88b9f4 100644 --- a/src/node/tests/forwarding.rs +++ b/src/node/tests/forwarding.rs @@ -6,8 +6,8 @@ use super::*; use crate::node::session_wire::{FSP_FLAG_CP, build_fsp_header}; +use crate::proto::stp::TreeCoordinate; use crate::protocol::{SessionAck, SessionDatagram, SessionSetup, encode_coords}; -use crate::tree::TreeCoordinate; use spanning_tree::{ TestNode, cleanup_nodes, process_available_packets, run_tree_test, verify_tree_convergence, }; diff --git a/src/node/tests/mmp_chartests.rs b/src/node/tests/mmp_chartests.rs index 482eb1f..63137fa 100644 --- a/src/node/tests/mmp_chartests.rs +++ b/src/node/tests/mmp_chartests.rs @@ -33,7 +33,7 @@ use crate::node::session::{EndToEndState, SessionEntry}; use crate::noise::HandshakeState; use crate::peer::ActivePeer; use crate::proto::mmp::{MmpMode, ReceiverReport}; -use crate::tree::{ParentDeclaration, TreeCoordinate}; +use crate::proto::stp::{ParentDeclaration, TreeCoordinate}; // =========================================================================== // Helpers diff --git a/src/node/tests/routing.rs b/src/node/tests/routing.rs index 9425a79..48fa688 100644 --- a/src/node/tests/routing.rs +++ b/src/node/tests/routing.rs @@ -5,7 +5,7 @@ use super::*; use crate::bloom::BloomFilter; -use crate::tree::{ParentDeclaration, TreeCoordinate}; +use crate::proto::stp::{ParentDeclaration, TreeCoordinate}; use spanning_tree::{ TestNode, cleanup_nodes, drain_all_packets, generate_random_edges, initiate_handshake, lock_large_network_test, make_test_node, run_tree_test, verify_tree_convergence, @@ -826,7 +826,7 @@ async fn test_routing_stops_after_peer_removal() { .map(|d| d.as_millis() as u64) .unwrap_or(0); - let all_coords: Vec<(NodeAddr, crate::tree::TreeCoordinate)> = nodes + let all_coords: Vec<(NodeAddr, crate::proto::stp::TreeCoordinate)> = nodes .iter() .map(|tn| { ( @@ -1007,7 +1007,7 @@ async fn test_routing_source_only_coords_100_nodes() { .unwrap_or(0); // Collect all coords for injection - let all_coords: Vec<(NodeAddr, crate::tree::TreeCoordinate)> = nodes + let all_coords: Vec<(NodeAddr, crate::proto::stp::TreeCoordinate)> = nodes .iter() .map(|tn| { ( @@ -1361,7 +1361,7 @@ fn test_parent_loss_reparent_invalidates_coord_cache() { TreeCoordinate::from_addrs(vec![alt, root]).unwrap(), ); // Adopt `parent`; our coords become [my_addr, parent, root], root = `root`. - node.tree_state_mut().set_parent(parent, 1, 1000); + node.tree_state_mut().set_parent(parent, 1, 1000, 1000); node.tree_state_mut().recompute_coords(); assert!(!node.tree_state().is_root()); assert_eq!(node.tree_state().root(), &root); @@ -1416,7 +1416,7 @@ fn test_parent_loss_selfroot_invalidates_coord_cache() { ParentDeclaration::new(parent, old_root, 1, 1000), TreeCoordinate::from_addrs(vec![parent, old_root]).unwrap(), ); - node.tree_state_mut().set_parent(parent, 1, 1000); + node.tree_state_mut().set_parent(parent, 1, 1000, 1000); node.tree_state_mut().recompute_coords(); assert!(!node.tree_state().is_root()); diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 3478215..73c7863 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -18,7 +18,7 @@ fn populate_all_coord_caches(nodes: &mut [TestNode]) { .unwrap() .as_millis() as u64; - let all_coords: Vec<(NodeAddr, crate::tree::TreeCoordinate)> = nodes + let all_coords: Vec<(NodeAddr, crate::proto::stp::TreeCoordinate)> = nodes .iter() .map(|tn| { ( diff --git a/src/node/tests/spanning_tree.rs b/src/node/tests/spanning_tree.rs index 10877da..22e26fa 100644 --- a/src/node/tests/spanning_tree.rs +++ b/src/node/tests/spanning_tree.rs @@ -5,9 +5,10 @@ //! reused by bloom filter tests. use super::*; -use crate::protocol::TreeAnnounce; +use crate::node::tree::sign_declaration; +use crate::proto::stp::TreeAnnounce; +use crate::proto::stp::{CoordEntry, ParentDeclaration, TreeCoordinate}; use crate::transport::loopback::{LoopbackRegistry, LoopbackTransport, new_registry}; -use crate::tree::{CoordEntry, ParentDeclaration, TreeCoordinate}; static LARGE_NETWORK_TEST_LOCK: std::sync::LazyLock> = std::sync::LazyLock::new(|| tokio::sync::Mutex::new(())); @@ -914,7 +915,7 @@ async fn test_rejects_tree_announce_with_inconsistent_root() { // sequence/timestamp so the announce would be acceptable on freshness // grounds if its ancestry semantics were valid. let mut declaration = ParentDeclaration::new(a_addr, fake_parent, 99, 12345); - declaration.sign(nodes[0].node.identity()).unwrap(); + sign_declaration(&mut declaration, nodes[0].node.identity()).unwrap(); let announce = TreeAnnounce::new( declaration, @@ -993,18 +994,15 @@ async fn test_tree_announce_repushed_on_root_disagreement() { // it had never processed the root's attaching announce. The child's // advertised root is now itself, disagreeing with the root's view, and its // own periodic re-evaluation cannot recover it (single peer). - nodes[child_idx].node.tree_state_mut().become_root(); + nodes[child_idx].node.tree_state_mut().become_root(1000); nodes[child_idx] .node .tree_state_mut() .remove_peer(&root_addr); { let identity = nodes[child_idx].node.identity().clone(); - nodes[child_idx] - .node - .tree_state_mut() - .sign_declaration(&identity) - .unwrap(); + let decl_mut = nodes[child_idx].node.tree_state_mut().my_declaration_mut(); + sign_declaration(decl_mut, &identity).unwrap(); } assert!(nodes[child_idx].node.tree_state().is_root()); @@ -1052,3 +1050,369 @@ async fn test_tree_announce_repushed_on_root_disagreement() { cleanup_nodes(&mut nodes).await; } + +// ===== Direct handler characterization tests ===== +// +// These drive `handle_tree_announce` directly to pin the individual +// classification and validation arms that the aggregate convergence suite +// only exercises indirectly: the four validation rejects (addr-mismatch, +// sig-fail, stale, unknown-peer) and the self-root / loop-drop / +// same-parent ancestry-update transitions. `run_tree_test(2, ..)` supplies +// two handshaked peers (so the sender's pubkey is known); the transition +// tests then force the receiver's local tree state into the precise shape +// each arm requires — the same `tree_state_mut()` seam +// `test_tree_announce_repushed_on_root_disagreement` uses above. +// +// `make_node_addr(0)` is the all-zero address, strictly smaller than every +// randomly-generated real node address, so it is used as a synthetic global +// root that keeps forced ancestries valid (advertised root = path minimum). + +/// A TreeAnnounce whose declared `node_addr` does not match the sending peer +/// must be rejected (addr-mismatch) before any state mutation. The addr-match +/// gate precedes signature verification, so a harvested-but-unrelated +/// signature is enough to reach it. +#[tokio::test] +async fn test_tree_announce_rejects_addr_mismatch() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + let a_addr = *nodes[0].node.node_addr(); + + let sig = nodes[0].node.identity().sign(&[0u8; 48]).to_byte_array(); + let bogus = make_node_addr(200); + let declaration = ParentDeclaration::with_signature(bogus, bogus, 5, 2000, sig); + let announce = TreeAnnounce::new( + declaration, + TreeCoordinate::from_addrs(vec![bogus]).unwrap(), + ); + let encoded = announce.encode().unwrap(); + + let mismatch_before = nodes[1].node.metrics().tree.addr_mismatch.get(); + let accepted_before = nodes[1].node.metrics().tree.accepted.get(); + let root_before = *nodes[1].node.tree_state().root(); + + // Sender is the known peer a_addr, but the declaration claims `bogus`. + nodes[1] + .node + .handle_tree_announce(&a_addr, &encoded[1..]) + .await; + + assert_eq!( + nodes[1].node.metrics().tree.addr_mismatch.get(), + mismatch_before + 1 + ); + assert_eq!(nodes[1].node.metrics().tree.accepted.get(), accepted_before); + assert_eq!(*nodes[1].node.tree_state().root(), root_before); + + cleanup_nodes(&mut nodes).await; +} + +/// A TreeAnnounce whose declared node_addr matches the sender but whose +/// signature does not verify under the sender's pubkey must be rejected +/// (sig-fail) without mutating tree state. +#[tokio::test] +async fn test_tree_announce_rejects_bad_signature() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + let a_addr = *nodes[0].node.node_addr(); + + // A valid A-signature, but over a *different* declaration, so verifying it + // against the forged declaration's signing bytes fails. + let mut signed_other = ParentDeclaration::new(a_addr, a_addr, 99, 88); + sign_declaration(&mut signed_other, nodes[0].node.identity()).unwrap(); + let sig = *signed_other.signature().unwrap(); + let forged = ParentDeclaration::with_signature(a_addr, a_addr, 5, 2000, sig); + let announce = TreeAnnounce::new(forged, TreeCoordinate::from_addrs(vec![a_addr]).unwrap()); + let encoded = announce.encode().unwrap(); + + let sig_failed_before = nodes[1].node.metrics().tree.sig_failed.get(); + let accepted_before = nodes[1].node.metrics().tree.accepted.get(); + + nodes[1] + .node + .handle_tree_announce(&a_addr, &encoded[1..]) + .await; + + assert_eq!( + nodes[1].node.metrics().tree.sig_failed.get(), + sig_failed_before + 1 + ); + assert_eq!(nodes[1].node.metrics().tree.accepted.get(), accepted_before); + + cleanup_nodes(&mut nodes).await; +} + +/// Replaying a peer's already-known declaration verbatim (same sequence) is +/// not fresher, so `update_peer` reports no change and the announce is counted +/// stale and ignored rather than accepted. +#[tokio::test] +async fn test_tree_announce_stale_ignored() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + let a_addr = *nodes[0].node.node_addr(); + + let stored_decl = nodes[1] + .node + .tree_state() + .peer_declaration(&a_addr) + .expect("node 1 should hold A's declaration after convergence") + .clone(); + let stored_coords = nodes[1] + .node + .tree_state() + .peer_coords(&a_addr) + .expect("node 1 should hold A's coordinates after convergence") + .clone(); + let announce = TreeAnnounce::new(stored_decl, stored_coords); + let encoded = announce.encode().unwrap(); + + let stale_before = nodes[1].node.metrics().tree.stale.get(); + let accepted_before = nodes[1].node.metrics().tree.accepted.get(); + + nodes[1] + .node + .handle_tree_announce(&a_addr, &encoded[1..]) + .await; + + assert_eq!(nodes[1].node.metrics().tree.stale.get(), stale_before + 1); + assert_eq!(nodes[1].node.metrics().tree.accepted.get(), accepted_before); + + cleanup_nodes(&mut nodes).await; +} + +/// A TreeAnnounce from a node that is not a known peer must be rejected +/// (unknown-peer) at the pubkey-lookup gate. +#[tokio::test] +async fn test_tree_announce_rejects_unknown_peer() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + + let unknown = make_node_addr(201); + let sig = nodes[0].node.identity().sign(&[0u8; 48]).to_byte_array(); + let declaration = ParentDeclaration::with_signature(unknown, unknown, 1, 1000, sig); + let announce = TreeAnnounce::new( + declaration, + TreeCoordinate::from_addrs(vec![unknown]).unwrap(), + ); + let encoded = announce.encode().unwrap(); + + let unknown_before = nodes[1].node.metrics().tree.unknown_peer.get(); + + nodes[1] + .node + .handle_tree_announce(&unknown, &encoded[1..]) + .await; + + assert_eq!( + nodes[1].node.metrics().tree.unknown_peer.get(), + unknown_before + 1 + ); + + cleanup_nodes(&mut nodes).await; +} + +/// A non-root node whose only visible root is larger than its own address must +/// self-promote to root. P (the smaller-addr node) is forced into a child of +/// its larger peer L rooted at a synthetic smaller root; when L then announces +/// itself as its own (larger) root, P's smallest visible root becomes L, so P +/// promotes itself. +#[tokio::test] +async fn test_tree_announce_self_root_promotion() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + + // P must be the smaller-addr node (the one that should win root); L larger. + let (p_idx, l_idx) = if nodes[0].node.node_addr() < nodes[1].node.node_addr() { + (0, 1) + } else { + (1, 0) + }; + let p_addr = *nodes[p_idx].node.node_addr(); + let l_addr = *nodes[l_idx].node.node_addr(); + let fake_root = make_node_addr(0); + + // Force P into a non-root child of L rooted at fake_root: coords + // [P, L, fake_root]. P > fake_root, so recompute keeps it attached. + { + let ts = nodes[p_idx].node.tree_state_mut(); + ts.remove_peer(&l_addr); + ts.update_peer( + ParentDeclaration::new(l_addr, fake_root, 1, 1000), + TreeCoordinate::from_addrs(vec![l_addr, fake_root]).unwrap(), + ); + ts.set_parent(l_addr, 1, 1000, 1000); + ts.recompute_coords(); + } + { + let identity = nodes[p_idx].node.identity().clone(); + let decl_mut = nodes[p_idx].node.tree_state_mut().my_declaration_mut(); + sign_declaration(decl_mut, &identity).unwrap(); + } + assert!(!nodes[p_idx].node.tree_state().is_root()); + assert_eq!(*nodes[p_idx].node.tree_state().root(), fake_root); + + // L announces a fresh self-root (root = L > P). + let mut decl = ParentDeclaration::self_root(l_addr, 5, 2000); + sign_declaration(&mut decl, nodes[l_idx].node.identity()).unwrap(); + let announce = TreeAnnounce::new(decl, TreeCoordinate::from_addrs(vec![l_addr]).unwrap()); + let encoded = announce.encode().unwrap(); + + let switched_before = nodes[p_idx].node.metrics().tree.parent_switched.get(); + nodes[p_idx] + .node + .handle_tree_announce(&l_addr, &encoded[1..]) + .await; + + assert!( + nodes[p_idx].node.tree_state().is_root(), + "P should self-promote to root when its only visible root is larger" + ); + assert_eq!(*nodes[p_idx].node.tree_state().root(), p_addr); + assert_eq!( + nodes[p_idx].node.metrics().tree.parent_switched.get(), + switched_before + 1 + ); + + cleanup_nodes(&mut nodes).await; +} + +/// When our current parent's freshly announced ancestry comes to contain us, a +/// loop has formed and the parent must be dropped. P is forced into a child of +/// Q rooted at a synthetic root; Q then announces an ancestry [Q, P, root] that +/// runs back through P, so P detects the loop and (having no alternative) falls +/// back to self-root. +#[tokio::test] +async fn test_tree_announce_loop_detection_drops_parent() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + + let (p_idx, q_idx) = (0, 1); + let p_addr = *nodes[p_idx].node.node_addr(); + let q_addr = *nodes[q_idx].node.node_addr(); + let root = make_node_addr(0); + + // Force P into a child of Q rooted at `root`: coords [P, Q, root]. + { + let ts = nodes[p_idx].node.tree_state_mut(); + ts.remove_peer(&q_addr); + ts.update_peer( + ParentDeclaration::new(q_addr, root, 1, 1000), + TreeCoordinate::from_addrs(vec![q_addr, root]).unwrap(), + ); + ts.set_parent(q_addr, 1, 1000, 1000); + ts.recompute_coords(); + } + { + let identity = nodes[p_idx].node.identity().clone(); + let decl_mut = nodes[p_idx].node.tree_state_mut().my_declaration_mut(); + sign_declaration(decl_mut, &identity).unwrap(); + } + assert!(!nodes[p_idx].node.tree_state().is_root()); + assert_eq!( + nodes[p_idx].node.tree_state().my_declaration().parent_id(), + &q_addr + ); + + // Q announces an ancestry that now runs through P (declaring P as its own + // parent): [Q, P, root]. Adopting it would form a loop. + let mut decl = ParentDeclaration::new(q_addr, p_addr, 5, 2000); + sign_declaration(&mut decl, nodes[q_idx].node.identity()).unwrap(); + let announce = TreeAnnounce::new( + decl, + TreeCoordinate::from_addrs(vec![q_addr, p_addr, root]).unwrap(), + ); + let encoded = announce.encode().unwrap(); + + let loop_before = nodes[p_idx].node.metrics().tree.loop_detected.get(); + nodes[p_idx] + .node + .handle_tree_announce(&q_addr, &encoded[1..]) + .await; + + assert_eq!( + nodes[p_idx].node.metrics().tree.loop_detected.get(), + loop_before + 1 + ); + // No alternative parent remains, so P falls back to self-root. + assert!(nodes[p_idx].node.tree_state().is_root()); + assert_eq!(*nodes[p_idx].node.tree_state().root(), p_addr); + + cleanup_nodes(&mut nodes).await; +} + +/// When our parent keeps the same root and depth but swaps a mid-chain +/// ancestor, we keep the parent yet must recompute our coordinates and +/// re-announce (the `old_addrs != new_addrs` gate). P is forced into +/// [P, Q, mid, root]; Q re-announces [Q, new_mid, root], leaving root and depth +/// unchanged while replacing the interior ancestor. +#[tokio::test] +async fn test_tree_announce_same_parent_ancestry_update() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + + let (p_idx, q_idx) = (0, 1); + let q_addr = *nodes[q_idx].node.node_addr(); + let root = make_node_addr(0); + let mid = make_node_addr(1); + let new_mid = make_node_addr(2); + + // Force P into a child of Q rooted at `root` via `mid`: [P, Q, mid, root]. + { + let ts = nodes[p_idx].node.tree_state_mut(); + ts.remove_peer(&q_addr); + ts.update_peer( + ParentDeclaration::new(q_addr, mid, 1, 1000), + TreeCoordinate::from_addrs(vec![q_addr, mid, root]).unwrap(), + ); + ts.set_parent(q_addr, 1, 1000, 1000); + ts.recompute_coords(); + } + { + let identity = nodes[p_idx].node.identity().clone(); + let decl_mut = nodes[p_idx].node.tree_state_mut().my_declaration_mut(); + sign_declaration(decl_mut, &identity).unwrap(); + } + let depth_before = nodes[p_idx].node.tree_state().my_coords().depth(); + assert_eq!( + nodes[p_idx].node.tree_state().my_declaration().parent_id(), + &q_addr + ); + + // Q keeps root and depth but swaps its mid-chain ancestor mid -> new_mid. + let mut decl = ParentDeclaration::new(q_addr, new_mid, 5, 2000); + sign_declaration(&mut decl, nodes[q_idx].node.identity()).unwrap(); + let announce = TreeAnnounce::new( + decl, + TreeCoordinate::from_addrs(vec![q_addr, new_mid, root]).unwrap(), + ); + let encoded = announce.encode().unwrap(); + + let ancestry_before = nodes[p_idx].node.metrics().tree.ancestry_changed.get(); + nodes[p_idx] + .node + .handle_tree_announce(&q_addr, &encoded[1..]) + .await; + + assert_eq!( + nodes[p_idx].node.metrics().tree.ancestry_changed.get(), + ancestry_before + 1 + ); + // Same parent, same depth, but the recomputed path now runs through new_mid. + assert_eq!( + nodes[p_idx].node.tree_state().my_declaration().parent_id(), + &q_addr + ); + assert_eq!( + nodes[p_idx].node.tree_state().my_coords().depth(), + depth_before + ); + let path: Vec = nodes[p_idx] + .node + .tree_state() + .my_coords() + .node_addrs() + .copied() + .collect(); + assert!( + path.contains(&new_mid), + "recomputed path should include the swapped-in ancestor" + ); + assert!( + !path.contains(&mid), + "old mid-chain ancestor should be gone from the recomputed path" + ); + + cleanup_nodes(&mut nodes).await; +} diff --git a/src/node/tree.rs b/src/node/tree.rs index 1216bdd..2f3e8c4 100644 --- a/src/node/tree.rs +++ b/src/node/tree.rs @@ -3,15 +3,63 @@ //! Handles building, sending, and receiving TreeAnnounce messages, //! including periodic root refresh and rate-limited propagation. -use std::collections::HashMap; +use std::collections::BTreeMap; -use crate::NodeAddr; -use crate::protocol::TreeAnnounce; +use secp256k1::XOnlyPublicKey; +use secp256k1::schnorr::Signature; + +use crate::proto::stp::{ParentDeclaration, Stp, TreeAnnounce, TreeDecision, TreeError}; +use crate::{Identity, NodeAddr}; use super::reject::TreeReject; use super::{Node, NodeError}; use tracing::{debug, info, trace, warn}; +/// Sign a node's own tree declaration, writing the 64-byte signature back into +/// it. The key-crypto boundary (§6): `proto::stp` owns the declaration data and +/// the pure `signing_bytes()` serialization; the shell owns the `secp256k1` +/// sign (`Identity::sign` hashes with SHA-256 internally). Mirrors discovery's +/// shell-side proof signing. +pub(super) fn sign_declaration( + decl: &mut ParentDeclaration, + identity: &Identity, +) -> Result<(), TreeError> { + if identity.node_addr() != decl.node_addr() { + return Err(TreeError::InvalidSignature(*decl.node_addr())); + } + let signature = identity.sign(&decl.signing_bytes()); + decl.set_signature(signature.to_byte_array()); + Ok(()) +} + +/// Verify a peer's tree declaration signature against their pubkey. The shell +/// side of the key-crypto boundary (§6): runs the `sha2` hash + `secp256k1` +/// schnorr verification over the in-core declaration's `signing_bytes()`. +pub(super) fn verify_declaration( + decl: &ParentDeclaration, + pubkey: &XOnlyPublicKey, +) -> Result<(), TreeError> { + let sig_bytes = decl + .signature() + .ok_or(TreeError::InvalidSignature(*decl.node_addr()))?; + let signature = Signature::from_slice(sig_bytes) + .map_err(|_| TreeError::InvalidSignature(*decl.node_addr()))?; + + let secp = secp256k1::Secp256k1::verification_only(); + let hash = signing_hash(decl); + + secp.verify_schnorr(&signature, &hash, pubkey) + .map_err(|_| TreeError::InvalidSignature(*decl.node_addr())) +} + +/// Compute the SHA-256 hash of a declaration's signing bytes (shell side, §6). +fn signing_hash(decl: &ParentDeclaration) -> [u8; 32] { + use sha2::{Digest, Sha256}; + let mut hasher = Sha256::new(); + hasher.update(decl.signing_bytes()); + hasher.finalize().into() +} + impl Node { /// Build a TreeAnnounce from our current tree state. fn build_tree_announce(&self) -> Result { @@ -167,7 +215,7 @@ impl Node { return; } - if let Err(e) = announce.declaration.verify(&pubkey) { + if let Err(e) = verify_declaration(&announce.declaration, &pubkey) { self.metrics().tree.sig_failed.inc(); warn!( from = %self.peer_display_name(from), @@ -249,7 +297,7 @@ impl Node { // receive path and is naturally bounded by the per-peer 500 ms // tree-announce rate limiter, so it does not storm during normal // convergence (it stops as soon as the peer adopts our root). - if *announce.ancestry.root_id() > *self.tree_state.root() + if Stp::should_echo(announce.ancestry.root_id(), self.tree_state.root()) && let Err(e) = self.send_tree_announce_to_peer(from).await { debug!( @@ -271,110 +319,128 @@ impl Node { // Re-evaluate parent selection with current link costs. // Exclude peers without MMP RTT data — they are not yet eligible // as parent candidates (prevents oscillation from optimistic defaults). - let peer_costs: HashMap = self + let peer_costs: BTreeMap = self .peers .iter() .filter(|(_, peer)| peer.has_srtt()) .map(|(addr, peer)| (*addr, peer.link_cost())) .collect(); let skip = self.non_full_peers(); - if let Some(new_parent) = self.tree_state.evaluate_parent(&peer_costs, &skip) { - let new_seq = self.tree_state.my_declaration().sequence() + 1; - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); - // Clone identity up front to avoid a split borrow against the - // &mut self.tree_state / &mut self.coord_cache calls below (cold path). - let our_identity = self.identity().clone(); - let flap_dampened = self.tree_state.set_parent(new_parent, new_seq, timestamp); - // recompute_coords may demote to self_root if the new path would be - // invalid; sign AFTER recompute so the signature covers the final - // declaration. - self.tree_state.recompute_coords(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { - warn!(error = %e, "Failed to sign declaration after parent switch"); - self.metrics() - .tree - .record_reject(TreeReject::OutboundSignFailed); - return; + // Monotonic ms for the flap-dampening / hold-down timers (distinct from + // the wall-clock `now_ms` above used for the peer's tree position). Read + // once and threaded into classify + the state mutators. + let mono_now_ms = crate::mmp::mono_ms(); + + match Stp::classify_announce(&self.tree_state, *from, &peer_costs, &skip, mono_now_ms) { + TreeDecision::Switch { + new_parent, + new_seq, + } => { + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + + // Clone identity up front to avoid a split borrow against the + // &mut self.tree_state / &mut self.coord_cache calls below (cold path). + let our_identity = self.identity().clone(); + let flap_dampened = + self.tree_state + .set_parent(new_parent, new_seq, timestamp, mono_now_ms); + // recompute_coords may demote to self_root if the new path would be + // invalid; sign AFTER recompute so the signature covers the final + // declaration. + self.tree_state.recompute_coords(); + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { + warn!(error = %e, "Failed to sign declaration after parent switch"); + self.metrics() + .tree + .record_reject(TreeReject::OutboundSignFailed); + return; + } + // Surgical invalidation — see CoordCache::invalidate_via_node doc. + self.coord_cache + .invalidate_via_node(our_identity.node_addr()); + self.reset_discovery_backoff(); + + self.metrics().tree.parent_switched.inc(); + self.metrics().tree.parent_switches.inc(); + + info!( + new_parent = %self.peer_display_name(&new_parent), + new_seq = new_seq, + new_root = %self.tree_state.root(), + depth = self.tree_state.my_coords().depth(), + "Parent switched, invalidated downstream coord cache entries, announcing to all peers" + ); + if flap_dampened { + self.metrics().tree.flap_dampened.inc(); + warn!("Flap dampening engaged: excessive parent switches detected"); + } + + self.send_tree_announce_to_all().await; + + // Tree structure changed — trigger bloom filter exchange with all peers + let all_peers: Vec = self.peers.keys().copied().collect(); + self.bloom_state.mark_all_updates_needed(all_peers); } - // Surgical invalidation — see CoordCache::invalidate_via_node doc. - self.coord_cache - .invalidate_via_node(our_identity.node_addr()); - self.reset_discovery_backoff(); - - self.metrics().tree.parent_switched.inc(); - self.metrics().tree.parent_switches.inc(); - - info!( - new_parent = %self.peer_display_name(&new_parent), - new_seq = new_seq, - new_root = %self.tree_state.root(), - depth = self.tree_state.my_coords().depth(), - "Parent switched, invalidated downstream coord cache entries, announcing to all peers" - ); - if flap_dampened { - self.metrics().tree.flap_dampened.inc(); - warn!("Flap dampening engaged: excessive parent switches detected"); + TreeDecision::SelfRoot => { + // Self is the smallest visible NodeAddr — promote to root rather + // than continuing to advertise a stale ancestry rooted elsewhere. + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + // Clone identity up front to avoid a split borrow against the + // &mut self.tree_state / &mut self.coord_cache calls below (cold path). + let our_identity = self.identity().clone(); + self.tree_state.become_root(timestamp); + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { + warn!(error = %e, "Failed to sign self-root declaration"); + self.metrics() + .tree + .record_reject(TreeReject::OutboundSignFailed); + return; + } + // Surgical invalidation — see CoordCache::invalidate_other_roots doc. + self.coord_cache + .invalidate_other_roots(our_identity.node_addr()); + self.reset_discovery_backoff(); + self.metrics().tree.parent_switched.inc(); + self.metrics().tree.parent_switches.inc(); + info!( + new_root = %self.tree_state.root(), + "Self-promoted to root: smallest visible NodeAddr" + ); + self.send_tree_announce_to_all().await; + let all_peers: Vec = self.peers.keys().copied().collect(); + self.bloom_state.mark_all_updates_needed(all_peers); } - - self.send_tree_announce_to_all().await; - - // Tree structure changed — trigger bloom filter exchange with all peers - let all_peers: Vec = self.peers.keys().copied().collect(); - self.bloom_state.mark_all_updates_needed(all_peers); - } else if !self.tree_state.is_root() && self.tree_state.should_be_root() { - // Self is the smallest visible NodeAddr — promote to root rather - // than continuing to advertise a stale ancestry rooted elsewhere. - // Clone identity up front to avoid a split borrow against the - // &mut self.tree_state / &mut self.coord_cache calls below (cold path). - let our_identity = self.identity().clone(); - self.tree_state.become_root(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { - warn!(error = %e, "Failed to sign self-root declaration"); - self.metrics() - .tree - .record_reject(TreeReject::OutboundSignFailed); - return; - } - // Surgical invalidation — see CoordCache::invalidate_other_roots doc. - self.coord_cache - .invalidate_other_roots(our_identity.node_addr()); - self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); - self.metrics().tree.parent_switches.inc(); - info!( - new_root = %self.tree_state.root(), - "Self-promoted to root: smallest visible NodeAddr" - ); - self.send_tree_announce_to_all().await; - let all_peers: Vec = self.peers.keys().copied().collect(); - self.bloom_state.mark_all_updates_needed(all_peers); - } else if !self.tree_state.is_root() - && *self.tree_state.my_declaration().parent_id() == *from - { - // Check for loop: if parent's ancestry now contains us, drop parent - if let Some(parent_coords) = self.tree_state.peer_coords(from) - && parent_coords.contains(self.identity().node_addr()) - { + TreeDecision::LoopDrop => { self.metrics().tree.loop_detected.inc(); warn!( parent = %self.peer_display_name(from), "Parent ancestry contains us — loop detected, dropping parent" ); - let peer_costs: HashMap = self - .peers - .iter() - .filter(|(_, peer)| peer.has_srtt()) - .map(|(addr, peer)| (*addr, peer.link_cost())) - .collect(); - if self.tree_state.handle_parent_lost(&peer_costs) { + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + if self + .tree_state + .handle_parent_lost(&peer_costs, timestamp, mono_now_ms) + { // Clone identity up front to avoid a split borrow against the // &mut self.tree_state / &mut self.coord_cache calls below (cold path). let our_identity = self.identity().clone(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { warn!(error = %e, "Failed to sign declaration after loop detection"); self.metrics() .tree @@ -390,74 +456,82 @@ impl Node { self.reset_discovery_backoff(); self.send_tree_announce_to_all().await; } - return; } + TreeDecision::AncestryUpdate { parent, new_seq } => { + // Our parent's ancestry changed but we're keeping the same parent. + // Recompute our own coordinates (which derive from parent's ancestry) + // and re-announce so downstream nodes stay current. + // + // Compare the full address path (not just root + depth) so that a + // mid-chain ancestor swap also triggers re-announce. A reroute that + // replaces an interior ancestor without changing the root or the + // path length leaves both `root` and `depth` unchanged but still + // alters our coords; downstream peers must learn the new path or + // they will route into a phantom intermediate that no longer + // exists on our parent's tree. + let old_root = *self.tree_state.root(); + let old_depth = self.tree_state.my_coords().depth(); + let old_addrs: Vec = + self.tree_state.my_coords().node_addrs().copied().collect(); - // Our parent's ancestry changed but we're keeping the same parent. - // Recompute our own coordinates (which derive from parent's ancestry) - // and re-announce so downstream nodes stay current. - // - // Compare the full address path (not just root + depth) so that a - // mid-chain ancestor swap also triggers re-announce. A reroute that - // replaces an interior ancestor without changing the root or the - // path length leaves both `root` and `depth` unchanged but still - // alters our coords; downstream peers must learn the new path or - // they will route into a phantom intermediate that no longer - // exists on our parent's tree. - let old_root = *self.tree_state.root(); - let old_depth = self.tree_state.my_coords().depth(); - let old_addrs: Vec = - self.tree_state.my_coords().node_addrs().copied().collect(); + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); - let new_seq = self.tree_state.my_declaration().sequence() + 1; - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); + // Clone identity up front to avoid a split borrow against the + // &mut self.tree_state / &mut self.coord_cache calls below (cold path). + let our_identity = self.identity().clone(); + self.tree_state + .set_parent(parent, new_seq, timestamp, mono_now_ms); + self.tree_state.recompute_coords(); + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { + warn!(error = %e, "Failed to sign declaration after parent update"); + self.metrics() + .tree + .record_reject(TreeReject::OutboundSignFailed); + return; + } + // Surgical invalidation — see CoordCache::invalidate_via_node doc. + self.coord_cache + .invalidate_via_node(our_identity.node_addr()); + self.reset_discovery_backoff(); - // Clone identity up front to avoid a split borrow against the - // &mut self.tree_state / &mut self.coord_cache calls below (cold path). - let our_identity = self.identity().clone(); - self.tree_state.set_parent(*from, new_seq, timestamp); - self.tree_state.recompute_coords(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { - warn!(error = %e, "Failed to sign declaration after parent update"); - self.metrics() - .tree - .record_reject(TreeReject::OutboundSignFailed); - return; + let new_addrs: Vec = + self.tree_state.my_coords().node_addrs().copied().collect(); + + if old_addrs != new_addrs { + self.metrics().tree.ancestry_changed.inc(); + info!( + parent = %self.peer_display_name(from), + old_root = %old_root, + new_root = %self.tree_state.root(), + old_depth = old_depth, + new_depth = self.tree_state.my_coords().depth(), + "Parent ancestry changed, re-announcing" + ); + self.send_tree_announce_to_all().await; + + // Bloom contents do not depend on path structure, only on + // identity sets. Our parent_id is unchanged in this branch, + // so our tree-peer set is unchanged and our outgoing filter + // content is unchanged. Use mark_changed_peers, which + // checks for actual content delta against last_sent_filters, + // instead of mark_all_updates_needed, which marks + // unconditionally regardless of whether content changed. + let peer_addrs: Vec = self.peers.keys().copied().collect(); + let peer_filters = self.peer_inbound_filters(); + self.bloom_state + .mark_changed_peers(from, &peer_addrs, &peer_filters); + } } - // Surgical invalidation — see CoordCache::invalidate_via_node doc. - self.coord_cache - .invalidate_via_node(our_identity.node_addr()); - self.reset_discovery_backoff(); - - let new_addrs: Vec = - self.tree_state.my_coords().node_addrs().copied().collect(); - - if old_addrs != new_addrs { - self.metrics().tree.ancestry_changed.inc(); - info!( - parent = %self.peer_display_name(from), - old_root = %old_root, - new_root = %self.tree_state.root(), - old_depth = old_depth, - new_depth = self.tree_state.my_coords().depth(), - "Parent ancestry changed, re-announcing" - ); - self.send_tree_announce_to_all().await; - - // Bloom contents do not depend on path structure, only on - // identity sets. Our parent_id is unchanged in this branch, - // so our tree-peer set is unchanged and our outgoing filter - // content is unchanged. Use mark_changed_peers, which - // checks for actual content delta against last_sent_filters, - // instead of mark_all_updates_needed, which marks - // unconditionally regardless of whether content changed. - let peer_addrs: Vec = self.peers.keys().copied().collect(); - let peer_filters = self.peer_inbound_filters(); - self.bloom_state - .mark_changed_peers(from, &peer_addrs, &peer_filters); + TreeDecision::NoChange => {} + // classify_announce never yields PeriodicRebroadcast (the periodic + // path's no-change tail) nor ParentLost (the removal drive's outcome). + TreeDecision::PeriodicRebroadcast | TreeDecision::ParentLost => { + unreachable!("classify_announce yields neither PeriodicRebroadcast nor ParentLost") } } } @@ -498,100 +572,132 @@ impl Node { self.last_parent_reeval = Some(now); - let peer_costs: HashMap = self + let peer_costs: BTreeMap = self .peers .iter() .filter(|(_, peer)| peer.has_srtt()) .map(|(addr, peer)| (*addr, peer.link_cost())) .collect(); - let skip = self.non_full_peers(); - if let Some(new_parent) = self.tree_state.evaluate_parent(&peer_costs, &skip) { - let new_seq = self.tree_state.my_declaration().sequence() + 1; - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); - // Clone identity up front to avoid a split borrow against the - // &mut self.tree_state / &mut self.coord_cache calls below (cold path). - let our_identity = self.identity().clone(); - let flap_dampened = self.tree_state.set_parent(new_parent, new_seq, timestamp); - self.tree_state.recompute_coords(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { - warn!(error = %e, "Failed to sign declaration after periodic parent re-eval"); - self.metrics() - .tree - .record_reject(TreeReject::OutboundSignFailed); - return; + // Monotonic ms for the flap-dampening / hold-down timers, read once and + // threaded into classify + the state mutators. + let mono_now_ms = crate::mmp::mono_ms(); + + match Stp::classify_periodic(&self.tree_state, &peer_costs, &skip, mono_now_ms) { + TreeDecision::Switch { + new_parent, + new_seq, + } => { + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + + // Clone identity up front to avoid a split borrow against the + // &mut self.tree_state / &mut self.coord_cache calls below (cold path). + let our_identity = self.identity().clone(); + let flap_dampened = + self.tree_state + .set_parent(new_parent, new_seq, timestamp, mono_now_ms); + self.tree_state.recompute_coords(); + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { + warn!(error = %e, "Failed to sign declaration after periodic parent re-eval"); + self.metrics() + .tree + .record_reject(TreeReject::OutboundSignFailed); + return; + } + // Surgical invalidation — see CoordCache::invalidate_via_node doc. + self.coord_cache + .invalidate_via_node(our_identity.node_addr()); + self.reset_discovery_backoff(); + + self.metrics().tree.parent_switched.inc(); + self.metrics().tree.parent_switches.inc(); + + info!( + new_parent = %self.peer_display_name(&new_parent), + new_seq = new_seq, + new_root = %self.tree_state.root(), + depth = self.tree_state.my_coords().depth(), + trigger = "periodic", + "Parent switched via periodic cost re-evaluation" + ); + if flap_dampened { + self.metrics().tree.flap_dampened.inc(); + warn!("Flap dampening engaged: excessive parent switches detected"); + } + + self.send_tree_announce_to_all().await; + + let all_peers: Vec = self.peers.keys().copied().collect(); + self.bloom_state.mark_all_updates_needed(all_peers); } - // Surgical invalidation — see CoordCache::invalidate_via_node doc. - self.coord_cache - .invalidate_via_node(our_identity.node_addr()); - self.reset_discovery_backoff(); - - self.metrics().tree.parent_switched.inc(); - self.metrics().tree.parent_switches.inc(); - - info!( - new_parent = %self.peer_display_name(&new_parent), - new_seq = new_seq, - new_root = %self.tree_state.root(), - depth = self.tree_state.my_coords().depth(), - trigger = "periodic", - "Parent switched via periodic cost re-evaluation" - ); - if flap_dampened { - self.metrics().tree.flap_dampened.inc(); - warn!("Flap dampening engaged: excessive parent switches detected"); + TreeDecision::SelfRoot => { + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + // Clone identity up front to avoid a split borrow against the + // &mut self.tree_state / &mut self.coord_cache calls below (cold path). + let our_identity = self.identity().clone(); + self.tree_state.become_root(timestamp); + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { + warn!(error = %e, "Failed to sign self-root declaration in periodic reeval"); + self.metrics() + .tree + .record_reject(TreeReject::OutboundSignFailed); + return; + } + // Surgical invalidation — see CoordCache::invalidate_other_roots doc. + self.coord_cache + .invalidate_other_roots(our_identity.node_addr()); + self.reset_discovery_backoff(); + self.metrics().tree.parent_switched.inc(); + self.metrics().tree.parent_switches.inc(); + info!( + new_root = %self.tree_state.root(), + trigger = "periodic", + "Self-promoted to root in periodic reeval: smallest visible NodeAddr" + ); + self.send_tree_announce_to_all().await; + let all_peers: Vec = self.peers.keys().copied().collect(); + self.bloom_state.mark_all_updates_needed(all_peers); } - - self.send_tree_announce_to_all().await; - - let all_peers: Vec = self.peers.keys().copied().collect(); - self.bloom_state.mark_all_updates_needed(all_peers); - } else if !self.tree_state.is_root() && self.tree_state.should_be_root() { - // Clone identity up front to avoid a split borrow against the - // &mut self.tree_state / &mut self.coord_cache calls below (cold path). - let our_identity = self.identity().clone(); - self.tree_state.become_root(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { - warn!(error = %e, "Failed to sign self-root declaration in periodic reeval"); - self.metrics() - .tree - .record_reject(TreeReject::OutboundSignFailed); - return; + TreeDecision::PeriodicRebroadcast => { + // Periodic re-broadcast on no-change: makes TreeAnnounce + // distribution eventually-consistent. Receivers coalesce + // by sequence via ParentDeclaration::is_fresher_than and + // short-circuit at the `if !updated` gate in + // handle_tree_announce; the per-peer 500 ms rate-limiter + // never blocks at this 60 s cadence. Closes the cross-init + // in-flight loss recovery gap where the swap window can + // strand one side's announce on a session-index the other + // side cannot decrypt. + trace!( + seq = self.tree_state.my_declaration().sequence(), + root = %self.tree_state.root(), + "Periodic TreeAnnounce re-broadcast (no state change)" + ); + self.send_tree_announce_to_all().await; + } + // classify_periodic never yields these: a periodic tick has no + // announcing peer, so the same-parent loop-drop / ancestry-update + // arms cannot arise, the no-change tail is PeriodicRebroadcast, and + // ParentLost is the removal drive's outcome. + TreeDecision::LoopDrop + | TreeDecision::AncestryUpdate { .. } + | TreeDecision::ParentLost + | TreeDecision::NoChange => { + unreachable!( + "classify_periodic yields only Switch / SelfRoot / PeriodicRebroadcast" + ) } - // Surgical invalidation — see CoordCache::invalidate_other_roots doc. - self.coord_cache - .invalidate_other_roots(our_identity.node_addr()); - self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); - self.metrics().tree.parent_switches.inc(); - info!( - new_root = %self.tree_state.root(), - trigger = "periodic", - "Self-promoted to root in periodic reeval: smallest visible NodeAddr" - ); - self.send_tree_announce_to_all().await; - let all_peers: Vec = self.peers.keys().copied().collect(); - self.bloom_state.mark_all_updates_needed(all_peers); - } else { - // Periodic re-broadcast on no-change: makes TreeAnnounce - // distribution eventually-consistent. Receivers coalesce - // by sequence via ParentDeclaration::is_fresher_than and - // short-circuit at the `if !updated` gate in - // handle_tree_announce; the per-peer 500 ms rate-limiter - // never blocks at this 60 s cadence. Closes the cross-init - // in-flight loss recovery gap where the swap window can - // strand one side's announce on a session-index the other - // side cannot decrypt. - trace!( - seq = self.tree_state.my_declaration().sequence(), - root = %self.tree_state.root(), - "Periodic TreeAnnounce re-broadcast (no state change)" - ); - self.send_tree_announce_to_all().await; } } @@ -607,20 +713,50 @@ impl Node { self.tree_state.remove_peer(node_addr); - if was_parent { - self.metrics().tree.parent_losses.inc(); - let peer_costs: HashMap = self - .peers - .iter() - .filter(|(_, peer)| peer.has_srtt()) - .map(|(addr, peer)| (*addr, peer.link_cost())) - .collect(); - let changed = self.tree_state.handle_parent_lost(&peer_costs); - if changed { + if !was_parent { + return false; + } + + // The removed peer was our parent. `parent_losses` counts the loss + // itself (independent of whether we recover), so it is stamped here — + // before the recovery mutation — exactly as before. + self.metrics().tree.parent_losses.inc(); + + let peer_costs: BTreeMap = self + .peers + .iter() + .filter(|(_, peer)| peer.has_srtt()) + .map(|(addr, peer)| (*addr, peer.link_cost())) + .collect(); + + // Wall-clock seconds stamped onto the new declaration; monotonic ms for + // the parent re-evaluation's flap timers. + let now_secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let mono_now_ms = crate::mmp::mono_ms(); + + // Removal is not a pure classify: `handle_parent_lost` is a &mut mutator + // whose returned `changed` bool IS the decision. Drive it and map the + // outcome onto the TreeDecision vocabulary. + let decision = if self + .tree_state + .handle_parent_lost(&peer_costs, now_secs, mono_now_ms) + { + TreeDecision::ParentLost + } else { + TreeDecision::NoChange + }; + + match decision { + TreeDecision::ParentLost => { // Re-sign the new declaration. Clone identity to avoid a split // borrow against the &mut self.tree_state receiver (cold path). let our_identity = self.identity().clone(); - if let Err(e) = self.tree_state.sign_declaration(&our_identity) { + if let Err(e) = + sign_declaration(self.tree_state.my_declaration_mut(), &our_identity) + { warn!(error = %e, "Failed to sign declaration after parent loss"); self.metrics() .tree @@ -641,10 +777,11 @@ impl Node { is_root = self.tree_state.is_root(), "Tree state updated after parent loss" ); + true } - changed - } else { - false + TreeDecision::NoChange => false, + // The removal drive constructs only ParentLost / NoChange above. + _ => unreachable!("removal drive yields only ParentLost / NoChange"), } } } diff --git a/src/peer/active.rs b/src/peer/active.rs index 332f72d..ad756ca 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -9,8 +9,8 @@ use crate::node::REKEY_JITTER_SECS; use crate::noise::{HandshakeState as NoiseHandshakeState, NoiseError, NoiseSession}; use crate::proto::fmp::{NegotiationPayload, NodeProfile}; use crate::proto::mmp::MmpPeerState; +use crate::proto::stp::{ParentDeclaration, TreeCoordinate}; use crate::transport::{LinkId, LinkStats, TransportAddr, TransportId}; -use crate::tree::{ParentDeclaration, TreeCoordinate}; use crate::utils::index::SessionIndex; use crate::{FipsAddress, NodeAddr, PeerIdentity}; use rand::RngExt; diff --git a/src/proto/discovery/wire.rs b/src/proto/discovery/wire.rs index 6fc40c5..854f19a 100644 --- a/src/proto/discovery/wire.rs +++ b/src/proto/discovery/wire.rs @@ -2,9 +2,9 @@ use crate::NodeAddr; use crate::proto::fmp::TlvEntry; +use crate::proto::stp::TreeCoordinate; use crate::protocol::ProtocolError; use crate::protocol::session::{decode_coords, encode_coords}; -use crate::tree::TreeCoordinate; use secp256k1::schnorr::Signature; /// Request to discover a node's coordinates. diff --git a/src/proto/mod.rs b/src/proto/mod.rs index 0552f78..1afa3c1 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -7,3 +7,4 @@ pub(crate) mod discovery; pub(crate) mod fmp; pub(crate) mod mmp; pub(crate) mod routing; +pub(crate) mod stp; diff --git a/src/proto/routing/wire.rs b/src/proto/routing/wire.rs index 864d131..c12a913 100644 --- a/src/proto/routing/wire.rs +++ b/src/proto/routing/wire.rs @@ -9,11 +9,11 @@ //! downward here (a `proto -> protocol` dependency is allowed). use crate::NodeAddr; +use crate::proto::stp::TreeCoordinate; use crate::protocol::ProtocolError; use crate::protocol::session::{ SessionMessageType, decode_optional_coords, encode_coords, encode_empty_coords, }; -use crate::tree::TreeCoordinate; /// Link-layer error signal indicating router cache miss. /// diff --git a/src/tree/coordinate.rs b/src/proto/stp/coordinate.rs similarity index 99% rename from src/tree/coordinate.rs rename to src/proto/stp/coordinate.rs index 5858c6b..c293dd4 100644 --- a/src/tree/coordinate.rs +++ b/src/proto/stp/coordinate.rs @@ -1,6 +1,6 @@ //! Tree coordinates and distance calculations. -use std::fmt; +use core::fmt; use super::TreeError; use crate::NodeAddr; diff --git a/src/proto/stp/core.rs b/src/proto/stp/core.rs new file mode 100644 index 0000000..8a93b62 --- /dev/null +++ b/src/proto/stp/core.rs @@ -0,0 +1,157 @@ +//! Pure spanning-tree classification over an in-core [`TreeState`]. +//! +//! The post-`update_peer` parent-switch / self-root / loop-drop / ancestry-update +//! ladder that the async `node::tree` handler inlines is extracted here as a pure +//! decision: [`Stp::classify_announce`] reads a consistent `TreeState` plus the +//! shell-passed facts (`peer_costs`, `skip`, and the monotonic `now_ms` the +//! ranking's hold-down / flap-dampening suppression checks read) and returns a +//! [`TreeDecision`] the shell drives. No I/O, no tracing, no keys, no mutation; +//! the injected `now_ms` is the only time input, threaded to `evaluate_parent`. +//! +//! [`TreeState`] lives beside this module in `proto/stp/state.rs`. + +use alloc::collections::{BTreeMap, BTreeSet}; + +use super::TreeState; +use crate::NodeAddr; + +/// Empty namespace anchor for the STP classify ladder (like `Mmp`/`Fmp`). +/// +/// The tree state lives on the (soon in-core) [`TreeState`], not here; `Stp` +/// exists only to namespace the pure classification functions. +pub(crate) struct Stp; + +/// A structured spanning-tree transition the shell matches and drives. +/// +/// The core emits only the structural decision; every effect (sign, coord-cache +/// invalidation, discovery-backoff reset, metrics, fan-out send, bloom mark) runs +/// shell-side while driving the returned variant. The `classify_announce` and +/// `classify_periodic` ladders produce these variants; the removal (`ParentLost`) +/// variant arrives with its own stage. +pub(crate) enum TreeDecision { + /// `evaluate_parent` picked a (different) parent: switch to it. Shell: + /// `set_parent(new_parent, new_seq, ts)` -> flap_dampened; `recompute_coords`; + /// sign; `invalidate_via_node`; reset_backoff; + /// metrics(parent_switched/parent_switches[, flap_dampened]); send_all; + /// bloom.mark_all_updates_needed. + Switch { new_parent: NodeAddr, new_seq: u64 }, + + /// `evaluate_parent` None && !is_root && should_be_root: self-promote to root. + /// Shell: `become_root`; sign; `invalidate_other_roots`; reset_backoff; + /// metrics(parent_switched/parent_switches); send_all; + /// bloom.mark_all_updates_needed. + SelfRoot, + + /// parent == from && parent's ancestry now contains us: drop parent. Shell: + /// metrics(loop_detected); `handle_parent_lost` -> if changed { sign; + /// `invalidate_via_node` + `invalidate_other_roots`; reset_backoff; send_all }. + LoopDrop, + + /// parent == from, no loop: keep parent, recompute, re-announce iff coords + /// changed. Shell: capture `old_addrs` before mutation; `set_parent(parent, + /// new_seq, ts)`; recompute; sign; `invalidate_via_node`; reset_backoff; THEN + /// if `old_addrs != new_addrs` { metrics(ancestry_changed); send_all; + /// bloom.mark_changed_peers }. + AncestryUpdate { parent: NodeAddr, new_seq: u64 }, + + /// Periodic no-change: re-broadcast our current declaration for eventual + /// consistency, no state change. Produced only by `classify_periodic` when + /// neither a parent switch nor a self-root promotion is warranted. Shell: + /// send_all (no sign, no invalidate, no metrics, no bloom). + PeriodicRebroadcast, + + /// Removal path: the removed peer was our parent and `handle_parent_lost` + /// changed our tree state (reparented onto an alternative or self-rooted). + /// Produced only by the removal drive in + /// `node::tree::handle_peer_removal_tree_cleanup`, never by `classify_announce` + /// or `classify_periodic`. Unlike the other variants this is not a pure + /// classification: `handle_parent_lost` is a `&mut` mutator whose returned + /// `changed` bool IS the decision, so the shell mutates first and reads the + /// outcome. Shell: sign; `invalidate_via_node` + `invalidate_other_roots`; + /// (the `parent_losses` metric is stamped before the mutation, and the caller + /// announces). + ParentLost, + + /// No tree transition warranted. + NoChange, +} + +impl Stp { + /// Classify an inbound, already-validated TreeAnnounce into a [`TreeDecision`]. + /// + /// Pure: reads the post-`update_peer` `tree` plus the shell-passed `peer_costs` + /// (per-peer link cost) and `skip` (non-full/leaf peers excluded from parent + /// candidacy — empty on master, `non_full_peers()` on next). Mirrors the inline + /// ladder in `node::tree::handle_tree_announce`: parent-switch, else self-root, + /// else (same-parent) loop-drop or ancestry-update, else no change. + pub(crate) fn classify_announce( + tree: &TreeState, + from: NodeAddr, + peer_costs: &BTreeMap, + skip: &BTreeSet, + now_ms: u64, + ) -> TreeDecision { + if let Some(new_parent) = tree.evaluate_parent(peer_costs, skip, now_ms) { + let new_seq = tree.my_declaration().sequence() + 1; + TreeDecision::Switch { + new_parent, + new_seq, + } + } else if !tree.is_root() && tree.should_be_root() { + TreeDecision::SelfRoot + } else if !tree.is_root() && *tree.my_declaration().parent_id() == from { + // Same parent: loop if parent's ancestry now contains us, else the + // parent's ancestry changed and we recompute + (maybe) re-announce. + if let Some(parent_coords) = tree.peer_coords(&from) + && parent_coords.contains(tree.my_node_addr()) + { + TreeDecision::LoopDrop + } else { + let new_seq = tree.my_declaration().sequence() + 1; + TreeDecision::AncestryUpdate { + parent: from, + new_seq, + } + } + } else { + TreeDecision::NoChange + } + } + + /// Classify a periodic parent re-evaluation into a [`TreeDecision`]. + /// + /// Pure: reads the current `tree` plus the shell-passed `peer_costs` and `skip` + /// (empty on master, `non_full_peers()` on next). Mirrors the periodic ladder in + /// `node::tree::check_periodic_parent_reeval`: parent-switch, else self-root, + /// else re-broadcast for eventual consistency. Unlike `classify_announce`, the + /// periodic path has no same-parent loop-drop / ancestry-update arms — a periodic + /// tick has no announcing peer, so those cases never arise; the no-change tail is + /// a re-broadcast rather than a true no-op. + pub(crate) fn classify_periodic( + tree: &TreeState, + peer_costs: &BTreeMap, + skip: &BTreeSet, + now_ms: u64, + ) -> TreeDecision { + if let Some(new_parent) = tree.evaluate_parent(peer_costs, skip, now_ms) { + let new_seq = tree.my_declaration().sequence() + 1; + TreeDecision::Switch { + new_parent, + new_seq, + } + } else if !tree.is_root() && tree.should_be_root() { + TreeDecision::SelfRoot + } else { + TreeDecision::PeriodicRebroadcast + } + } + + /// Whether to echo our current position back to an announcing peer. + /// + /// Root election is smallest-NodeAddr-wins, so a peer advertising a strictly + /// worse (higher) root than ours has a stale/pre-attachment view and can attach + /// through us; only the better-rooted side echoes. + pub(crate) fn should_echo(announce_root: &NodeAddr, our_root: &NodeAddr) -> bool { + announce_root > our_root + } +} diff --git a/src/proto/stp/limits.rs b/src/proto/stp/limits.rs new file mode 100644 index 0000000..9fc36ca --- /dev/null +++ b/src/proto/stp/limits.rs @@ -0,0 +1,123 @@ +//! Parent-switch flap dampening and hold-down. +//! +//! Suppresses excessive parent churn in the spanning tree via two +//! complementary mechanisms, both owned by [`TreeState`](super::TreeState): +//! +//! - **Hold-down**: after a parent switch, non-mandatory re-evaluation is +//! suppressed for a configurable window (`0` = disabled). Mandatory +//! switches (parent lost, smaller root found) bypass it. +//! - **Flap dampening**: if more than `flap_threshold` switches occur within +//! `flap_window`, further non-mandatory switches are suppressed for +//! `flap_dampening_duration`. +//! +//! The monotonic clock is injected: every timing method takes a `now_ms: u64` +//! (monotonic milliseconds, read by the shell from `crate::mmp::mono_ms`), and +//! all timers/durations are stored as plain `u64` milliseconds. Configuration +//! setters accept seconds (matching the node config) and store the value scaled +//! to milliseconds so the comparisons stay in one unit. + +/// Flap-dampening / hold-down state for a node's parent selection. +/// +/// Groups the flap-detection timers behind a single struct so the tree +/// ranking in [`TreeState`](super::TreeState) drives them through a small +/// method surface, mirroring the routing / discovery limiter idiom. All +/// timers are monotonic milliseconds; the shell injects `now_ms`. +pub(crate) struct FlapDampener { + /// Hold-down period after a parent switch, in ms (0 = disabled). + hold_down: u64, + /// Monotonic ms of last parent switch (for hold-down enforcement). + last_parent_switch: Option, + /// Number of parent switches in the current flap window. + flap_count: u32, + /// Monotonic ms of the start of the current flap counting window. + flap_window_start: Option, + /// If dampened, suppressed until this monotonic ms. + flap_dampening_until: Option, + /// Flap threshold: max switches before dampening engages. + flap_threshold: u32, + /// Flap window duration, in ms. + flap_window: u64, + /// Dampening duration when threshold exceeded, in ms. + flap_dampening_duration: u64, +} + +impl FlapDampener { + /// Create with the default flap parameters (hold-down disabled). + pub(crate) fn new() -> Self { + Self { + hold_down: 0, + last_parent_switch: None, + flap_count: 0, + flap_window_start: None, + flap_dampening_until: None, + flap_threshold: 4, + flap_window: 60_000, + flap_dampening_duration: 120_000, + } + } + + /// Set the hold-down duration after parent switches (seconds). + pub(crate) fn set_hold_down(&mut self, secs: u64) { + self.hold_down = secs.saturating_mul(1000); + } + + /// Configure flap dampening parameters (durations in seconds). + pub(crate) fn set_flap_dampening( + &mut self, + threshold: u32, + window_secs: u64, + dampening_secs: u64, + ) { + self.flap_threshold = threshold; + self.flap_window = window_secs.saturating_mul(1000); + self.flap_dampening_duration = dampening_secs.saturating_mul(1000); + } + + /// Stamp the time of a parent switch (called on every `set_parent`, + /// whether or not the parent actually changed). `now_ms` is the injected + /// monotonic time in milliseconds. + pub(crate) fn mark_switch(&mut self, now_ms: u64) { + self.last_parent_switch = Some(now_ms); + } + + /// Record a parent switch for flap detection. + /// Returns true if dampening was just engaged. `now_ms` is the injected + /// monotonic time in milliseconds. + pub(crate) fn record_parent_switch(&mut self, now_ms: u64) -> bool { + // Reset window if expired or not started + match self.flap_window_start { + Some(start) if now_ms.saturating_sub(start) < self.flap_window => { + self.flap_count += 1; + } + _ => { + self.flap_window_start = Some(now_ms); + self.flap_count = 1; + } + } + + // Check threshold + if self.flap_count >= self.flap_threshold && self.flap_dampening_until.is_none() { + self.flap_dampening_until = Some(now_ms + self.flap_dampening_duration); + return true; + } + false + } + + /// Check if flap dampening is currently active. `now_ms` is the injected + /// monotonic time in milliseconds. + pub(crate) fn is_flap_dampened(&self, now_ms: u64) -> bool { + match self.flap_dampening_until { + Some(until) => now_ms < until, + None => false, + } + } + + /// Whether hold-down currently suppresses a non-mandatory re-evaluation. + /// `now_ms` is the injected monotonic time in milliseconds. + pub(crate) fn is_hold_down_active(&self, now_ms: u64) -> bool { + self.hold_down != 0 + && self + .last_parent_switch + .is_some_and(|last| now_ms.saturating_sub(last) < self.hold_down) + } +} diff --git a/src/tree/mod.rs b/src/proto/stp/mod.rs similarity index 56% rename from src/tree/mod.rs rename to src/proto/stp/mod.rs index 21a1d72..7ea582d 100644 --- a/src/tree/mod.rs +++ b/src/proto/stp/mod.rs @@ -1,20 +1,40 @@ -//! Spanning Tree Protocol Entities +//! Sans-IO spanning tree protocol (STP) state. //! -//! Tree coordinates and parent declarations for the FIPS spanning tree. -//! The spanning tree provides a routing topology where each node maintains -//! a path to a common root, enabling greedy distance-based routing. +//! The non-async STP surface has been migrated out of the async node shell: +//! `TreeState` ranking/election, `TreeCoordinate` algebra, `ParentDeclaration` +//! data, and the flap-dampening limiter all live here. The async I/O handlers +//! remain in `node::tree`. The STP wire codec lives in `wire.rs` (the +//! `TreeAnnounce` struct + `validate_semantics`), per the +//! wire-migrates-with-subsystem policy. It imports the shared `ProtocolError` +//! and `LinkMessageType` downward from `crate::protocol`. +//! +//! - `core.rs` — the pure classify ladder (`Stp::classify_announce` / +//! `classify_periodic` / `should_echo`) over an in-core `TreeState`. +//! - `state.rs` — `TreeState` + `ParentDeclaration` data + the `&self` +//! ranking/election methods (`evaluate_parent`, `should_be_root`, +//! `find_next_hop`). +//! - `coordinate.rs` — `TreeCoordinate` / `CoordEntry`. +//! - `limits.rs` — the flap-dampening / hold-down state machine. +//! - `wire.rs` — `TreeAnnounce` + `validate_semantics` (the one std-tethered +//! file). mod coordinate; -mod declaration; +mod core; +mod limits; mod state; +mod wire; + +#[cfg(test)] +mod tests; use thiserror::Error; use crate::{IdentityError, NodeAddr}; pub use coordinate::{CoordEntry, TreeCoordinate}; -pub use declaration::ParentDeclaration; -pub use state::TreeState; +pub(crate) use core::{Stp, TreeDecision}; +pub use state::{ParentDeclaration, TreeState}; +pub use wire::TreeAnnounce; /// Errors related to spanning tree operations. #[derive(Debug, Error)] @@ -65,6 +85,3 @@ pub enum TreeError { #[error("identity error: {0}")] Identity(#[from] IdentityError), } - -#[cfg(test)] -mod tests; diff --git a/src/tree/state.rs b/src/proto/stp/state.rs similarity index 69% rename from src/tree/state.rs rename to src/proto/stp/state.rs index 11d5218..817cc4c 100644 --- a/src/tree/state.rs +++ b/src/proto/stp/state.rs @@ -1,11 +1,11 @@ //! Local spanning tree state for a node. -use std::collections::HashMap; -use std::fmt; -use std::time::{Duration, Instant}; +use alloc::collections::{BTreeMap, BTreeSet}; +use core::fmt; -use super::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError}; -use crate::{Identity, NodeAddr}; +use super::limits::FlapDampener; +use super::{CoordEntry, TreeCoordinate}; +use crate::NodeAddr; /// Local spanning tree state for a node. /// @@ -22,39 +22,23 @@ pub struct TreeState { /// The current elected root (smallest reachable node_addr). pub(super) root: NodeAddr, /// Each peer's most recent parent declaration. - peer_declarations: HashMap, + peer_declarations: BTreeMap, /// Each peer's full ancestry to root. - peer_ancestry: HashMap, + peer_ancestry: BTreeMap, /// Hysteresis factor for cost-based parent re-selection (0.0-1.0). parent_hysteresis: f64, - /// Hold-down period after parent switch (0 = disabled). - hold_down: Duration, - /// Timestamp of last parent switch (for hold-down enforcement). - last_parent_switch: Option, - /// Number of parent switches in current flap window. - flap_count: u32, - /// Start of the current flap counting window. - flap_window_start: Option, - /// If dampened, suppressed until this instant. - flap_dampening_until: Option, - /// Flap threshold: max switches before dampening engages. - flap_threshold: u32, - /// Flap window duration. - flap_window: Duration, - /// Dampening duration when threshold exceeded. - flap_dampening_duration: Duration, + /// Flap-dampening / hold-down state machine. + flap: FlapDampener, } impl TreeState { /// Create initial tree state for a node (as root candidate). /// /// The node starts as its own root until it learns of a smaller node_addr. - /// Initial sequence is 1 per protocol spec; timestamp is current Unix time. - pub fn new(my_node_addr: NodeAddr) -> Self { - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); + /// Initial sequence is 1 per protocol spec; `now_secs` is the injected + /// wall-clock Unix time in seconds stamped onto the initial declaration. + pub fn new(my_node_addr: NodeAddr, now_secs: u64) -> Self { + let timestamp = now_secs; let my_declaration = ParentDeclaration::self_root(my_node_addr, 1, timestamp); let my_coords = TreeCoordinate::root_with_meta(my_node_addr, 1, timestamp); @@ -63,17 +47,10 @@ impl TreeState { my_declaration, my_coords, root: my_node_addr, - peer_declarations: HashMap::new(), - peer_ancestry: HashMap::new(), + peer_declarations: BTreeMap::new(), + peer_ancestry: BTreeMap::new(), parent_hysteresis: 0.0, - hold_down: Duration::ZERO, - last_parent_switch: None, - flap_count: 0, - flap_window_start: None, - flap_dampening_until: None, - flap_threshold: 4, - flap_window: Duration::from_secs(60), - flap_dampening_duration: Duration::from_secs(120), + flap: FlapDampener::new(), } } @@ -164,15 +141,25 @@ impl TreeState { /// Call this when switching parents. Updates the declaration and coordinates. /// Returns true if flap dampening was just engaged due to this switch. /// Only records a flap when the parent actually changes. - pub fn set_parent(&mut self, parent_id: NodeAddr, sequence: u64, timestamp: u64) -> bool { + /// + /// `timestamp` is the escaping wall-clock Unix seconds stamped onto the new + /// declaration; `now_ms` is the monotonic milliseconds driving the + /// flap-dampening timers (the two clock bases must not be crossed). + pub fn set_parent( + &mut self, + parent_id: NodeAddr, + sequence: u64, + timestamp: u64, + now_ms: u64, + ) -> bool { let parent_changed = self.is_root() || *self.my_declaration.parent_id() != parent_id; self.my_declaration = ParentDeclaration::new(self.my_node_addr, parent_id, sequence, timestamp); - self.last_parent_switch = Some(Instant::now()); + self.flap.mark_switch(now_ms); // Record switch for flap detection only when parent actually changes; // coordinates will be recomputed when ancestry is available if parent_changed { - self.record_parent_switch() + self.flap.record_parent_switch(now_ms) } else { false } @@ -239,13 +226,12 @@ impl TreeState { /// Promote self to root with an incremented sequence number. /// - /// Caller must `sign_declaration` afterwards before sending the result. - pub fn become_root(&mut self) { + /// `now_secs` is the injected wall-clock Unix time in seconds stamped onto + /// the new self-root declaration. Caller must `sign_declaration` afterwards + /// before sending the result. + pub fn become_root(&mut self, now_secs: u64) { let new_seq = self.my_declaration.sequence() + 1; - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); + let timestamp = now_secs; self.my_declaration = ParentDeclaration::self_root(self.my_node_addr, new_seq, timestamp); self.recompute_coords(); } @@ -274,7 +260,7 @@ impl TreeState { pub fn find_next_hop( &self, dest_coords: &TreeCoordinate, - skip_peers: &std::collections::HashSet, + skip_peers: &BTreeSet, ) -> Option { if self.my_coords.root_id() != dest_coords.root_id() { return None; @@ -315,46 +301,19 @@ impl TreeState { /// Set the hold-down duration after parent switches. pub fn set_hold_down(&mut self, secs: u64) { - self.hold_down = Duration::from_secs(secs); + self.flap.set_hold_down(secs); } /// Configure flap dampening parameters. pub fn set_flap_dampening(&mut self, threshold: u32, window_secs: u64, dampening_secs: u64) { - self.flap_threshold = threshold; - self.flap_window = Duration::from_secs(window_secs); - self.flap_dampening_duration = Duration::from_secs(dampening_secs); + self.flap + .set_flap_dampening(threshold, window_secs, dampening_secs); } - /// Record a parent switch for flap detection. - /// Returns true if dampening was just engaged. - pub fn record_parent_switch(&mut self) -> bool { - let now = Instant::now(); - - // Reset window if expired or not started - match self.flap_window_start { - Some(start) if now.duration_since(start) < self.flap_window => { - self.flap_count += 1; - } - _ => { - self.flap_window_start = Some(now); - self.flap_count = 1; - } - } - - // Check threshold - if self.flap_count >= self.flap_threshold && self.flap_dampening_until.is_none() { - self.flap_dampening_until = Some(now + self.flap_dampening_duration); - return true; - } - false - } - - /// Check if flap dampening is currently active. - pub fn is_flap_dampened(&self) -> bool { - match self.flap_dampening_until { - Some(until) => Instant::now() < until, - None => false, - } + /// Check if flap dampening is currently active. `now_ms` is the injected + /// monotonic time in milliseconds. + pub fn is_flap_dampened(&self, now_ms: u64) -> bool { + self.flap.is_flap_dampened(now_ms) } /// Evaluate whether to switch parents based on current peer tree state. @@ -368,10 +327,14 @@ impl TreeState { /// /// `skip_peers` contains peers that should not be considered as parent /// candidates (e.g., non-routing and leaf nodes that don't forward transit). + /// + /// `now_ms` is the injected monotonic time in milliseconds, used only for + /// the hold-down / flap-dampening suppression checks below. pub fn evaluate_parent( &self, - peer_costs: &HashMap, - skip_peers: &std::collections::HashSet, + peer_costs: &BTreeMap, + skip_peers: &BTreeSet, + now_ms: u64, ) -> Option { if self.peer_ancestry.is_empty() { return None; @@ -471,17 +434,13 @@ impl TreeState { // --- Hold-down: suppress non-mandatory re-evaluation after recent switch --- - if !self.hold_down.is_zero() - && self - .last_parent_switch - .is_some_and(|last| last.elapsed() < self.hold_down) - { + if self.flap.is_hold_down_active(now_ms) { return None; } // --- Flap dampening: suppress after excessive parent switches --- - if self.is_flap_dampened() { + if self.flap.is_flap_dampened(now_ms) { return None; } @@ -519,37 +478,38 @@ impl TreeState { /// If none available, becomes its own root (increments sequence). /// /// Returns `true` if the tree state changed (caller should re-announce). - pub fn handle_parent_lost(&mut self, peer_costs: &HashMap) -> bool { + /// + /// `now_secs` is the injected wall-clock Unix seconds stamped onto the new + /// declaration; `now_ms` is the monotonic milliseconds driving the parent + /// re-evaluation's flap timers (the two clock bases must not be crossed). + pub fn handle_parent_lost( + &mut self, + peer_costs: &BTreeMap, + now_secs: u64, + now_ms: u64, + ) -> bool { // Try to find an alternative parent - if let Some(new_parent) = - self.evaluate_parent(peer_costs, &std::collections::HashSet::new()) - { - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); + if let Some(new_parent) = self.evaluate_parent(peer_costs, &BTreeSet::new(), now_ms) { let new_seq = self.my_declaration.sequence() + 1; - self.set_parent(new_parent, new_seq, timestamp); + self.set_parent(new_parent, new_seq, now_secs, now_ms); self.recompute_coords(); return true; } // No alternative: become own root - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); let new_seq = self.my_declaration.sequence() + 1; - self.my_declaration = ParentDeclaration::self_root(self.my_node_addr, new_seq, timestamp); + self.my_declaration = ParentDeclaration::self_root(self.my_node_addr, new_seq, now_secs); self.recompute_coords(); true } - /// Sign this node's declaration with the given identity. + /// Mutable access to this node's declaration. /// - /// The identity's node_addr must match this TreeState's node_addr. - pub fn sign_declaration(&mut self, identity: &Identity) -> Result<(), TreeError> { - self.my_declaration.sign(identity) + /// Exposed so the shell can write back a signature after signing: the + /// declaration data + `signing_bytes()` live in-core, but the key-crypto + /// (the schnorr sign) is a shell-driven boundary (§6), mirroring discovery. + pub(crate) fn my_declaration_mut(&mut self) -> &mut ParentDeclaration { + &mut self.my_declaration } /// Check if this node's declaration is signed. @@ -569,3 +529,142 @@ impl fmt::Debug for TreeState { .finish() } } + +/// A node's declaration of its parent in the spanning tree. +/// +/// Each node periodically announces its parent selection. The declaration +/// includes a monotonic sequence number for freshness and a signature +/// for authenticity. When `parent_id == node_addr`, the node declares itself +/// as a root candidate. +#[derive(Clone)] +pub struct ParentDeclaration { + /// The node making this declaration. + node_addr: NodeAddr, + /// The selected parent (equals node_addr if self-declaring as root). + parent_id: NodeAddr, + /// Monotonically increasing sequence number. + sequence: u64, + /// Timestamp when this declaration was created (Unix seconds). + timestamp: u64, + /// Raw 64-byte Schnorr signature over the declaration fields. Stored as + /// opaque bytes so the in-core type carries no signature-crypto dependency; + /// the shell computes/verifies it over `signing_bytes()` (§6). + signature: Option<[u8; 64]>, +} + +impl ParentDeclaration { + /// Create a new unsigned parent declaration. + /// + /// The declaration must be signed before transmission using `set_signature()`. + pub fn new(node_addr: NodeAddr, parent_id: NodeAddr, sequence: u64, timestamp: u64) -> Self { + Self { + node_addr, + parent_id, + sequence, + timestamp, + signature: None, + } + } + + /// Create a self-declaration (node is root candidate). + pub fn self_root(node_addr: NodeAddr, sequence: u64, timestamp: u64) -> Self { + Self::new(node_addr, node_addr, sequence, timestamp) + } + + /// Create a declaration with a pre-computed signature. + pub fn with_signature( + node_addr: NodeAddr, + parent_id: NodeAddr, + sequence: u64, + timestamp: u64, + signature: [u8; 64], + ) -> Self { + Self { + node_addr, + parent_id, + sequence, + timestamp, + signature: Some(signature), + } + } + + /// Get the declaring node's ID. + pub fn node_addr(&self) -> &NodeAddr { + &self.node_addr + } + + /// Get the parent node's ID. + pub fn parent_id(&self) -> &NodeAddr { + &self.parent_id + } + + /// Get the sequence number. + pub fn sequence(&self) -> u64 { + self.sequence + } + + /// Get the timestamp. + pub fn timestamp(&self) -> u64 { + self.timestamp + } + + /// Get the raw 64-byte signature, if set. + pub fn signature(&self) -> Option<&[u8; 64]> { + self.signature.as_ref() + } + + /// Set the raw 64-byte signature after signing. + pub fn set_signature(&mut self, signature: [u8; 64]) { + self.signature = Some(signature); + } + + /// Check if this is a root declaration (parent == self). + pub fn is_root(&self) -> bool { + self.node_addr == self.parent_id + } + + /// Check if this declaration is signed. + pub fn is_signed(&self) -> bool { + self.signature.is_some() + } + + /// Get the bytes that should be signed. + /// + /// Format: node_addr (16) || parent_id (16) || sequence (8) || timestamp (8) + pub fn signing_bytes(&self) -> Vec { + let mut bytes = Vec::with_capacity(48); + bytes.extend_from_slice(self.node_addr.as_bytes()); + bytes.extend_from_slice(self.parent_id.as_bytes()); + bytes.extend_from_slice(&self.sequence.to_le_bytes()); + bytes.extend_from_slice(&self.timestamp.to_le_bytes()); + bytes + } + + /// Check if this declaration is fresher than another. + pub fn is_fresher_than(&self, other: &ParentDeclaration) -> bool { + self.sequence > other.sequence + } +} + +impl fmt::Debug for ParentDeclaration { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ParentDeclaration") + .field("node_addr", &self.node_addr) + .field("parent_id", &self.parent_id) + .field("sequence", &self.sequence) + .field("is_root", &self.is_root()) + .field("signed", &self.is_signed()) + .finish() + } +} + +impl PartialEq for ParentDeclaration { + fn eq(&self, other: &Self) -> bool { + self.node_addr == other.node_addr + && self.parent_id == other.parent_id + && self.sequence == other.sequence + && self.timestamp == other.timestamp + } +} + +impl Eq for ParentDeclaration {} diff --git a/src/proto/stp/tests/coordinate.rs b/src/proto/stp/tests/coordinate.rs new file mode 100644 index 0000000..772fda2 --- /dev/null +++ b/src/proto/stp/tests/coordinate.rs @@ -0,0 +1,161 @@ +//! TreeCoordinate unit tests. + +use super::util::{make_coords, make_node_addr}; +use crate::proto::stp::{CoordEntry, TreeCoordinate, TreeError}; + +#[test] +fn test_tree_coordinate_root() { + let root_id = make_node_addr(1); + let coord = TreeCoordinate::root(root_id); + + assert!(coord.is_root()); + assert_eq!(coord.depth(), 0); + assert_eq!(coord.node_addr(), &root_id); + assert_eq!(coord.root_id(), &root_id); + assert_eq!(coord.parent_id(), &root_id); +} + +#[test] +fn test_tree_coordinate_path() { + let node = make_node_addr(1); + let parent = make_node_addr(2); + let root = make_node_addr(3); + + let coord = make_coords(&[1, 2, 3]); + + assert!(!coord.is_root()); + assert_eq!(coord.depth(), 2); + assert_eq!(coord.node_addr(), &node); + assert_eq!(coord.parent_id(), &parent); + assert_eq!(coord.root_id(), &root); +} + +#[test] +fn test_tree_coordinate_empty_fails() { + let result = TreeCoordinate::from_addrs(vec![]); + assert!(matches!(result, Err(TreeError::EmptyCoordinate))); +} + +#[test] +fn test_tree_coordinate_entries_metadata() { + let node = make_node_addr(1); + let root = make_node_addr(0); + + let coord = TreeCoordinate::new(vec![ + CoordEntry::new(node, 5, 1000), + CoordEntry::new(root, 1, 500), + ]) + .unwrap(); + + assert_eq!(coord.entries()[0].sequence, 5); + assert_eq!(coord.entries()[0].timestamp, 1000); + assert_eq!(coord.entries()[1].sequence, 1); + assert_eq!(coord.entries()[1].timestamp, 500); +} + +#[test] +fn test_tree_distance_same_node() { + let node = make_node_addr(1); + let coord = TreeCoordinate::root(node); + + assert_eq!(coord.distance_to(&coord), 0); +} + +#[test] +fn test_tree_distance_siblings() { + let coord_a = make_coords(&[1, 0]); + let coord_b = make_coords(&[2, 0]); + + // a -> root -> b = 2 hops + assert_eq!(coord_a.distance_to(&coord_b), 2); +} + +#[test] +fn test_tree_distance_ancestor() { + let coord_parent = make_coords(&[1, 0]); + let coord_child = make_coords(&[2, 1, 0]); + + // child -> parent = 1 hop + assert_eq!(coord_child.distance_to(&coord_parent), 1); +} + +#[test] +fn test_tree_distance_cousins() { + // Tree structure: + // root(0) + // / \ + // a(1) b(2) + // / \ + // c(3) d(4) + let coord_c = make_coords(&[3, 1, 0]); + let coord_d = make_coords(&[4, 2, 0]); + + // c -> a -> root -> b -> d = 4 hops + assert_eq!(coord_c.distance_to(&coord_d), 4); +} + +#[test] +fn test_tree_distance_different_roots() { + let coord1 = TreeCoordinate::root(make_node_addr(1)); + let coord2 = TreeCoordinate::root(make_node_addr(2)); + + assert_eq!(coord1.distance_to(&coord2), usize::MAX); +} + +#[test] +fn test_has_ancestor() { + let root = make_node_addr(0); + let parent = make_node_addr(1); + let child = make_node_addr(2); + + let coord = make_coords(&[2, 1, 0]); + + assert!(coord.has_ancestor(&parent)); + assert!(coord.has_ancestor(&root)); + assert!(!coord.has_ancestor(&child)); // self is not an ancestor +} + +#[test] +fn test_contains() { + let root = make_node_addr(0); + let parent = make_node_addr(1); + let child = make_node_addr(2); + let other = make_node_addr(99); + + let coord = make_coords(&[2, 1, 0]); + + assert!(coord.contains(&child)); + assert!(coord.contains(&parent)); + assert!(coord.contains(&root)); + assert!(!coord.contains(&other)); +} + +#[test] +fn test_ancestor_at() { + let root = make_node_addr(0); + let parent = make_node_addr(1); + let child = make_node_addr(2); + + let coord = make_coords(&[2, 1, 0]); + + assert_eq!(coord.ancestor_at(0), Some(&child)); + assert_eq!(coord.ancestor_at(1), Some(&parent)); + assert_eq!(coord.ancestor_at(2), Some(&root)); + assert_eq!(coord.ancestor_at(3), None); +} + +#[test] +fn test_lca() { + let root = make_node_addr(0); + let a = make_node_addr(1); + + // c under a, d under b, both under root + let coord_c = make_coords(&[3, 1, 0]); + let coord_d = make_coords(&[4, 2, 0]); + + assert_eq!(coord_c.lca(&coord_d), Some(&root)); + + // c and a share ancestry through a and root + let coord_a = make_coords(&[1, 0]); + assert_eq!(coord_c.lca(&coord_a), Some(&a)); +} diff --git a/src/proto/stp/tests/limits.rs b/src/proto/stp/tests/limits.rs new file mode 100644 index 0000000..e6d8d94 --- /dev/null +++ b/src/proto/stp/tests/limits.rs @@ -0,0 +1,235 @@ +//! Flap dampening / hold-down unit tests. + +use std::collections::{BTreeMap, BTreeSet}; + +use super::util::{make_coords, make_costs, make_node_addr}; +use crate::proto::stp::{ParentDeclaration, TreeState}; + +#[test] +fn test_flap_dampening_engages_after_threshold() { + // Create TreeState with flap_threshold=3, window=60s, dampening=3600s (long) + let my_node = make_node_addr(5); + let mut state = TreeState::new(my_node, 1000); + state.set_flap_dampening(3, 60, 3600); + state.set_hold_down(0); // disable hold-down for this test + + let peer_a = make_node_addr(1); + let peer_b = make_node_addr(2); + let root = make_node_addr(0); + + state.update_peer( + ParentDeclaration::new(peer_a, root, 1, 1000), + make_coords(&[1, 0]), + ); + state.update_peer( + ParentDeclaration::new(peer_b, root, 1, 1000), + make_coords(&[2, 0]), + ); + + // Switch 1: initial parent selection (root -> peer_a) + assert!(!state.is_flap_dampened(3000)); + state.set_parent(peer_a, 1, 1000, 1000); + state.recompute_coords(); + assert!(!state.is_flap_dampened(3000)); + + // Switch 2: peer_a -> peer_b + state.set_parent(peer_b, 2, 2000, 2000); + state.recompute_coords(); + assert!(!state.is_flap_dampened(3000)); + + // Switch 3: peer_b -> peer_a — threshold reached, dampening engages + let dampened = state.set_parent(peer_a, 3, 3000, 3000); + state.recompute_coords(); + assert!(dampened); + assert!(state.is_flap_dampened(3000)); + + // evaluate_parent should return None for non-mandatory switches + // Make peer_b much better than peer_a + let costs = make_costs(&[(1, 10.0), (2, 1.0)]); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 3000); + assert_eq!(result, None); // suppressed by flap dampening +} + +#[test] +fn test_flap_dampening_allows_mandatory_switches() { + // Engage dampening, then verify mandatory switches still work + let my_node = make_node_addr(5); + let mut state = TreeState::new(my_node, 1000); + state.set_flap_dampening(3, 60, 3600); + state.set_hold_down(0); + + let peer_a = make_node_addr(1); + let peer_b = make_node_addr(2); + let root = make_node_addr(0); + + state.update_peer( + ParentDeclaration::new(peer_a, root, 1, 1000), + make_coords(&[1, 0]), + ); + state.update_peer( + ParentDeclaration::new(peer_b, root, 1, 1000), + make_coords(&[2, 0]), + ); + + // Trigger dampening with 3 switches + state.set_parent(peer_a, 1, 1000, 1000); + state.recompute_coords(); + state.set_parent(peer_b, 2, 2000, 2000); + state.recompute_coords(); + state.set_parent(peer_a, 3, 3000, 3000); + state.recompute_coords(); + assert!(state.is_flap_dampened(3000)); + + // Remove current parent (peer_a) — this is a mandatory switch + state.remove_peer(&peer_a); + let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 3000); + assert_eq!(result, Some(peer_b)); // mandatory switch bypasses dampening +} + +#[test] +fn test_flap_dampening_expires() { + // Test with 0-second dampening duration to verify expiry logic + let my_node = make_node_addr(5); + let mut state = TreeState::new(my_node, 1000); + state.set_flap_dampening(3, 60, 0); // 0-second dampening + state.set_hold_down(0); + + let peer_a = make_node_addr(1); + let peer_b = make_node_addr(2); + let root = make_node_addr(0); + + state.update_peer( + ParentDeclaration::new(peer_a, root, 1, 1000), + make_coords(&[1, 0]), + ); + state.update_peer( + ParentDeclaration::new(peer_b, root, 1, 1000), + make_coords(&[2, 0]), + ); + + // Trigger dampening + state.set_parent(peer_a, 1, 1000, 1000); + state.recompute_coords(); + state.set_parent(peer_b, 2, 2000, 2000); + state.recompute_coords(); + let dampened = state.set_parent(peer_a, 3, 3000, 3000); + state.recompute_coords(); + assert!(dampened); // dampening was engaged + + // With 0-second duration, dampening should have already expired + assert!(!state.is_flap_dampened(3000)); + + // evaluate_parent should work normally now + let costs = make_costs(&[(1, 10.0), (2, 1.0)]); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 3000); + assert_eq!(result, Some(peer_b)); // not suppressed +} + +#[test] +fn test_flap_dampening_below_threshold() { + // Fewer switches than threshold should NOT engage dampening + let my_node = make_node_addr(5); + let mut state = TreeState::new(my_node, 1000); + state.set_flap_dampening(4, 60, 3600); // threshold=4 + state.set_hold_down(0); + + let peer_a = make_node_addr(1); + let peer_b = make_node_addr(2); + let root = make_node_addr(0); + + state.update_peer( + ParentDeclaration::new(peer_a, root, 1, 1000), + make_coords(&[1, 0]), + ); + state.update_peer( + ParentDeclaration::new(peer_b, root, 1, 1000), + make_coords(&[2, 0]), + ); + + // Only 3 switches (below threshold of 4) + state.set_parent(peer_a, 1, 1000, 1000); + state.recompute_coords(); + state.set_parent(peer_b, 2, 2000, 2000); + state.recompute_coords(); + state.set_parent(peer_a, 3, 3000, 3000); + state.recompute_coords(); + + assert!(!state.is_flap_dampened(3000)); + + // evaluate_parent should still work normally + let costs = make_costs(&[(1, 10.0), (2, 1.0)]); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 3000); + assert_eq!(result, Some(peer_b)); // not suppressed +} + +#[test] +fn test_flap_dampening_window_reset() { + // Test that the flap window resets after expiry. + // Use a 0-second window so it immediately expires between switch groups. + let my_node = make_node_addr(5); + let mut state = TreeState::new(my_node, 1000); + // threshold=3, window=0s (expires immediately), dampening=3600s + state.set_flap_dampening(3, 0, 3600); + state.set_hold_down(0); + + let peer_a = make_node_addr(1); + let peer_b = make_node_addr(2); + let root = make_node_addr(0); + + state.update_peer( + ParentDeclaration::new(peer_a, root, 1, 1000), + make_coords(&[1, 0]), + ); + state.update_peer( + ParentDeclaration::new(peer_b, root, 1, 1000), + make_coords(&[2, 0]), + ); + + // Each switch resets the window (0s window means every switch starts fresh). + // So we never accumulate enough to reach threshold=3. + state.set_parent(peer_a, 1, 1000, 1000); + state.recompute_coords(); + // Window expired, counter resets on next switch + state.set_parent(peer_b, 2, 2000, 2000); + state.recompute_coords(); + // Window expired, counter resets on next switch + state.set_parent(peer_a, 3, 3000, 3000); + state.recompute_coords(); + + // Dampening should NOT have engaged because each switch reset the window + assert!(!state.is_flap_dampened(3000)); +} + +#[test] +fn test_flap_dampening_same_parent_no_count() { + // Re-declaring the same parent should not count as a flap + let my_node = make_node_addr(5); + let mut state = TreeState::new(my_node, 1000); + state.set_flap_dampening(3, 60, 3600); + state.set_hold_down(0); + + let peer_a = make_node_addr(1); + let root = make_node_addr(0); + + state.update_peer( + ParentDeclaration::new(peer_a, root, 1, 1000), + make_coords(&[1, 0]), + ); + + // Initial parent selection + state.set_parent(peer_a, 1, 1000, 1000); + state.recompute_coords(); + + // Re-declare same parent multiple times (e.g., parent ancestry changed) + state.set_parent(peer_a, 2, 2000, 2000); + state.recompute_coords(); + state.set_parent(peer_a, 3, 3000, 3000); + state.recompute_coords(); + state.set_parent(peer_a, 4, 4000, 4000); + state.recompute_coords(); + state.set_parent(peer_a, 5, 5000, 5000); + state.recompute_coords(); + + // Should NOT be dampened since only the first was a real switch + assert!(!state.is_flap_dampened(3000)); +} diff --git a/src/proto/stp/tests/mod.rs b/src/proto/stp/tests/mod.rs new file mode 100644 index 0000000..15de4ca --- /dev/null +++ b/src/proto/stp/tests/mod.rs @@ -0,0 +1,6 @@ +//! STP primitive unit tests. Shared helpers live in `util`. + +mod coordinate; +mod limits; +mod state; +mod util; diff --git a/src/tree/tests.rs b/src/proto/stp/tests/state.rs similarity index 64% rename from src/tree/tests.rs rename to src/proto/stp/tests/state.rs index 21fddff..05dc612 100644 --- a/src/tree/tests.rs +++ b/src/proto/stp/tests/state.rs @@ -1,175 +1,9 @@ -use std::collections::{HashMap, HashSet}; +//! ParentDeclaration, TreeState, parent-selection, and find_next_hop unit tests. -use super::*; +use std::collections::{BTreeMap, BTreeSet}; -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() -} - -// ===== TreeCoordinate Tests ===== - -#[test] -fn test_tree_coordinate_root() { - let root_id = make_node_addr(1); - let coord = TreeCoordinate::root(root_id); - - assert!(coord.is_root()); - assert_eq!(coord.depth(), 0); - assert_eq!(coord.node_addr(), &root_id); - assert_eq!(coord.root_id(), &root_id); - assert_eq!(coord.parent_id(), &root_id); -} - -#[test] -fn test_tree_coordinate_path() { - let node = make_node_addr(1); - let parent = make_node_addr(2); - let root = make_node_addr(3); - - let coord = make_coords(&[1, 2, 3]); - - assert!(!coord.is_root()); - assert_eq!(coord.depth(), 2); - assert_eq!(coord.node_addr(), &node); - assert_eq!(coord.parent_id(), &parent); - assert_eq!(coord.root_id(), &root); -} - -#[test] -fn test_tree_coordinate_empty_fails() { - let result = TreeCoordinate::from_addrs(vec![]); - assert!(matches!(result, Err(TreeError::EmptyCoordinate))); -} - -#[test] -fn test_tree_coordinate_entries_metadata() { - let node = make_node_addr(1); - let root = make_node_addr(0); - - let coord = TreeCoordinate::new(vec![ - CoordEntry::new(node, 5, 1000), - CoordEntry::new(root, 1, 500), - ]) - .unwrap(); - - assert_eq!(coord.entries()[0].sequence, 5); - assert_eq!(coord.entries()[0].timestamp, 1000); - assert_eq!(coord.entries()[1].sequence, 1); - assert_eq!(coord.entries()[1].timestamp, 500); -} - -#[test] -fn test_tree_distance_same_node() { - let node = make_node_addr(1); - let coord = TreeCoordinate::root(node); - - assert_eq!(coord.distance_to(&coord), 0); -} - -#[test] -fn test_tree_distance_siblings() { - let coord_a = make_coords(&[1, 0]); - let coord_b = make_coords(&[2, 0]); - - // a -> root -> b = 2 hops - assert_eq!(coord_a.distance_to(&coord_b), 2); -} - -#[test] -fn test_tree_distance_ancestor() { - let coord_parent = make_coords(&[1, 0]); - let coord_child = make_coords(&[2, 1, 0]); - - // child -> parent = 1 hop - assert_eq!(coord_child.distance_to(&coord_parent), 1); -} - -#[test] -fn test_tree_distance_cousins() { - // Tree structure: - // root(0) - // / \ - // a(1) b(2) - // / \ - // c(3) d(4) - let coord_c = make_coords(&[3, 1, 0]); - let coord_d = make_coords(&[4, 2, 0]); - - // c -> a -> root -> b -> d = 4 hops - assert_eq!(coord_c.distance_to(&coord_d), 4); -} - -#[test] -fn test_tree_distance_different_roots() { - let coord1 = TreeCoordinate::root(make_node_addr(1)); - let coord2 = TreeCoordinate::root(make_node_addr(2)); - - assert_eq!(coord1.distance_to(&coord2), usize::MAX); -} - -#[test] -fn test_has_ancestor() { - let root = make_node_addr(0); - let parent = make_node_addr(1); - let child = make_node_addr(2); - - let coord = make_coords(&[2, 1, 0]); - - assert!(coord.has_ancestor(&parent)); - assert!(coord.has_ancestor(&root)); - assert!(!coord.has_ancestor(&child)); // self is not an ancestor -} - -#[test] -fn test_contains() { - let root = make_node_addr(0); - let parent = make_node_addr(1); - let child = make_node_addr(2); - let other = make_node_addr(99); - - let coord = make_coords(&[2, 1, 0]); - - assert!(coord.contains(&child)); - assert!(coord.contains(&parent)); - assert!(coord.contains(&root)); - assert!(!coord.contains(&other)); -} - -#[test] -fn test_ancestor_at() { - let root = make_node_addr(0); - let parent = make_node_addr(1); - let child = make_node_addr(2); - - let coord = make_coords(&[2, 1, 0]); - - assert_eq!(coord.ancestor_at(0), Some(&child)); - assert_eq!(coord.ancestor_at(1), Some(&parent)); - assert_eq!(coord.ancestor_at(2), Some(&root)); - assert_eq!(coord.ancestor_at(3), None); -} - -#[test] -fn test_lca() { - let root = make_node_addr(0); - let a = make_node_addr(1); - - // c under a, d under b, both under root - let coord_c = make_coords(&[3, 1, 0]); - let coord_d = make_coords(&[4, 2, 0]); - - assert_eq!(coord_c.lca(&coord_d), Some(&root)); - - // c and a share ancestry through a and root - let coord_a = make_coords(&[1, 0]); - assert_eq!(coord_c.lca(&coord_a), Some(&a)); -} +use super::util::{add_peer, make_coords, make_costs, make_node_addr, make_tree_state}; +use crate::proto::stp::{ParentDeclaration, TreeCoordinate, TreeState}; // ===== ParentDeclaration Tests ===== @@ -247,7 +81,7 @@ fn test_parent_declaration_equality() { #[test] fn test_tree_state_new() { let node = make_node_addr(1); - let state = TreeState::new(node); + let state = TreeState::new(node, 1000); assert_eq!(state.my_node_addr(), &node); assert!(state.is_root()); @@ -259,7 +93,7 @@ fn test_tree_state_new() { #[test] fn test_tree_state_update_peer() { let my_node = make_node_addr(0); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer = make_node_addr(1); let root = make_node_addr(2); @@ -284,7 +118,7 @@ fn test_tree_state_update_peer() { #[test] fn test_tree_state_remove_peer() { let my_node = make_node_addr(0); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer = make_node_addr(1); let root = make_node_addr(2); @@ -303,7 +137,7 @@ fn test_tree_state_remove_peer() { #[test] fn test_tree_state_distance_to_peer() { let my_node = make_node_addr(0); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer = make_node_addr(1); @@ -319,7 +153,7 @@ fn test_tree_state_distance_to_peer() { let shared_root = make_node_addr(99); // Update my state to have shared root - state.set_parent(shared_root, 1, 1000); + state.set_parent(shared_root, 1, 1000, 1000); let my_new_coords = make_coords(&[0, 99]); // Manually set coords for test (normally done by recompute_coords) state.my_coords = my_new_coords; @@ -337,7 +171,7 @@ fn test_tree_state_distance_to_peer() { #[test] fn test_tree_state_peer_ids() { let my_node = make_node_addr(0); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let peer2 = make_node_addr(2); @@ -366,7 +200,7 @@ fn test_evaluate_parent_picks_smallest_root() { // Peer 7's path: [7, 2] (root=2) // Should pick peer 3 because root 1 < root 2. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer3 = make_node_addr(3); let peer7 = make_node_addr(7); @@ -380,7 +214,7 @@ fn test_evaluate_parent_picks_smallest_root() { make_coords(&[7, 2]), ); - let result = state.evaluate_parent(&HashMap::new(), &HashSet::new()); + let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); assert_eq!(result, Some(peer3)); } @@ -389,7 +223,7 @@ fn test_evaluate_parent_prefers_shallowest_depth() { // Node 5, root=0 (shared). Peer 1 at depth 1, peer 2 at depth 3. // Both reach root 0. Should pick peer 1 (shallowest). let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let peer2 = make_node_addr(2); @@ -406,7 +240,7 @@ fn test_evaluate_parent_prefers_shallowest_depth() { make_coords(&[2, 3, 4, 0]), ); - let result = state.evaluate_parent(&HashMap::new(), &HashSet::new()); + let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); assert_eq!(result, Some(peer1)); } @@ -414,7 +248,7 @@ fn test_evaluate_parent_prefers_shallowest_depth() { fn test_evaluate_parent_stays_root_when_smallest() { // Node 0 (smallest possible) should stay root even if peers exist. let my_node = make_node_addr(0); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); // Peer 1 has root 0 (us) — shouldn't trigger switch @@ -424,7 +258,7 @@ fn test_evaluate_parent_stays_root_when_smallest() { ); assert_eq!( - state.evaluate_parent(&HashMap::new(), &HashSet::new()), + state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), None ); } @@ -433,7 +267,7 @@ fn test_evaluate_parent_stays_root_when_smallest() { fn test_evaluate_parent_no_switch_when_already_best() { // Node 5, already using peer 1 as parent. No better option. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let root = make_node_addr(0); @@ -444,12 +278,12 @@ fn test_evaluate_parent_no_switch_when_already_best() { ); // Switch to peer1 as parent first - state.set_parent(peer1, 1, 1000); + state.set_parent(peer1, 1, 1000, 1000); state.recompute_coords(); // Now evaluate — should return None since peer1 is already our parent assert_eq!( - state.evaluate_parent(&HashMap::new(), &HashSet::new()), + state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), None ); } @@ -457,10 +291,10 @@ fn test_evaluate_parent_no_switch_when_already_best() { #[test] fn test_evaluate_parent_no_peers() { let my_node = make_node_addr(5); - let state = TreeState::new(my_node); + let state = TreeState::new(my_node, 1000); assert_eq!( - state.evaluate_parent(&HashMap::new(), &HashSet::new()), + state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), None ); } @@ -472,7 +306,7 @@ fn test_evaluate_parent_depth_threshold() { // Peer 3 offers depth 1 (improvement of 3, exceeds threshold). // Should switch to peer 3. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer2 = make_node_addr(2); let peer3 = make_node_addr(3); @@ -485,7 +319,7 @@ fn test_evaluate_parent_depth_threshold() { ); // Set peer2 as our parent, making us depth 4 - state.set_parent(peer2, 1, 1000); + state.set_parent(peer2, 1, 1000, 1000); state.recompute_coords(); assert_eq!(state.my_coords().depth(), 4); @@ -495,7 +329,7 @@ fn test_evaluate_parent_depth_threshold() { make_coords(&[3, 0]), ); - let result = state.evaluate_parent(&HashMap::new(), &HashSet::new()); + let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); assert_eq!(result, Some(peer3)); } @@ -504,7 +338,7 @@ fn test_evaluate_parent_rejects_loop_candidate() { // Node 5 with peer 1 whose ancestry contains node 5 — selecting // peer 1 would create a coordinate loop. evaluate_parent must skip it. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let _root = make_node_addr(0); @@ -517,7 +351,7 @@ fn test_evaluate_parent_rejects_loop_candidate() { // Should return None — the only candidate creates a loop assert_eq!( - state.evaluate_parent(&HashMap::new(), &HashSet::new()), + state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), None ); } @@ -527,7 +361,7 @@ fn test_evaluate_parent_picks_loop_free_over_loopy() { // Two peers reach the same root. Peer 1's ancestry contains us (loop), // peer 2's does not. Should pick peer 2 even though peer 1 is shallower. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let peer2 = make_node_addr(2); @@ -544,14 +378,14 @@ fn test_evaluate_parent_picks_loop_free_over_loopy() { make_coords(&[2, 3, 4, 0]), ); - let result = state.evaluate_parent(&HashMap::new(), &HashSet::new()); + let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); assert_eq!(result, Some(peer2)); } #[test] fn test_handle_parent_lost_finds_alternative() { let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let peer2 = make_node_addr(2); @@ -567,12 +401,12 @@ fn test_handle_parent_lost_finds_alternative() { ); // Set peer1 as parent - state.set_parent(peer1, 1, 1000); + state.set_parent(peer1, 1, 1000, 1000); state.recompute_coords(); // Remove peer1 (parent lost) state.remove_peer(&peer1); - let changed = state.handle_parent_lost(&HashMap::new()); + let changed = state.handle_parent_lost(&BTreeMap::new(), 1000, 1000); assert!(changed); // Should have switched to peer2 @@ -591,7 +425,7 @@ fn test_handle_parent_lost_becomes_root_when_self_smaller_than_remaining() { // bug seen in production where ubuntu-dev (3847a4..) advertised itself // as root while its path still contained mac (312c79..). let my_node = make_node_addr(1); // our addr is the smallest - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let smaller = make_node_addr(0); let bigger1 = make_node_addr(2); @@ -613,14 +447,14 @@ fn test_handle_parent_lost_becomes_root_when_self_smaller_than_remaining() { make_coords(&[3]), ); - state.set_parent(smaller, 2, 2000); + state.set_parent(smaller, 2, 2000, 2000); state.recompute_coords(); assert_eq!(state.my_coords().entries().len(), 2); assert_eq!(state.root(), &smaller); // Smaller peer disconnects. state.remove_peer(&smaller); - let changed = state.handle_parent_lost(&HashMap::new()); + let changed = state.handle_parent_lost(&BTreeMap::new(), 1000, 1000); assert!(changed); // Must become root (we're the smallest visible), NOT pick bigger1/bigger2. @@ -644,7 +478,7 @@ fn test_recompute_coords_demotes_when_self_smaller_than_parent_root() { // path), recompute_coords must produce a valid ancestry by demoting to // self-root rather than emit [self, peer, peer_root] with last > min. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let bigger_peer = make_node_addr(7); state.update_peer( @@ -652,7 +486,7 @@ fn test_recompute_coords_demotes_when_self_smaller_than_parent_root() { make_coords(&[7]), ); - state.set_parent(bigger_peer, 2, 2000); + state.set_parent(bigger_peer, 2, 2000, 2000); state.recompute_coords(); assert!(state.is_root(), "recompute_coords demoted to self-root"); @@ -666,7 +500,7 @@ fn test_recompute_coords_demotes_when_self_smaller_than_parent_root() { #[test] fn test_handle_parent_lost_becomes_root() { let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let root = make_node_addr(0); @@ -677,13 +511,13 @@ fn test_handle_parent_lost_becomes_root() { ); // Set peer1 as parent - state.set_parent(peer1, 1, 1000); + state.set_parent(peer1, 1, 1000, 1000); state.recompute_coords(); let seq_before = state.my_declaration().sequence(); // Remove peer1 (only parent) state.remove_peer(&peer1); - let changed = state.handle_parent_lost(&HashMap::new()); + let changed = state.handle_parent_lost(&BTreeMap::new(), 1000, 1000); assert!(changed); assert!(state.is_root()); @@ -693,26 +527,6 @@ fn test_handle_parent_lost_becomes_root() { // === find_next_hop tests === -/// Build a TreeState with our own coordinates set. -fn make_tree_state(my_addr: u8, coord_path: &[u8]) -> TreeState { - let my_node = make_node_addr(my_addr); - let mut state = TreeState::new(my_node); - let coords = make_coords(coord_path); - state.root = *coords.root_id(); - state.my_coords = coords; - state -} - -/// Add a peer with given coordinates to the tree state. -fn add_peer(state: &mut TreeState, peer_addr: u8, coord_path: &[u8]) { - let peer = make_node_addr(peer_addr); - let parent = make_node_addr(coord_path[1]); - state.update_peer( - ParentDeclaration::new(peer, parent, 1, 1000), - make_coords(coord_path), - ); -} - #[test] fn test_find_next_hop_chain() { // Chain: 0 (root) <- 5 (us) <- 1 <- 2 @@ -724,7 +538,7 @@ fn test_find_next_hop_chain() { let dest = make_coords(&[2, 1, 5, 0]); assert_eq!( - state.find_next_hop(&dest, &HashSet::new()), + state.find_next_hop(&dest, &BTreeSet::new()), Some(make_node_addr(2)) ); } @@ -739,7 +553,7 @@ fn test_find_next_hop_chain_indirect() { let dest = make_coords(&[2, 1, 5, 0]); assert_eq!( - state.find_next_hop(&dest, &HashSet::new()), + state.find_next_hop(&dest, &BTreeSet::new()), Some(make_node_addr(1)) ); } @@ -753,7 +567,7 @@ fn test_find_next_hop_toward_root() { let dest = make_coords(&[0]); assert_eq!( - state.find_next_hop(&dest, &HashSet::new()), + state.find_next_hop(&dest, &BTreeSet::new()), Some(make_node_addr(1)) ); } @@ -771,7 +585,7 @@ fn test_find_next_hop_sibling() { let dest = make_coords(&[3, 0]); assert_eq!( - state.find_next_hop(&dest, &HashSet::new()), + state.find_next_hop(&dest, &BTreeSet::new()), Some(make_node_addr(3)) ); } @@ -790,7 +604,7 @@ fn test_find_next_hop_tie_breaking() { // Peer 3 distance: 2 (up to root, down to 4) // Peer 2 distance: 2 (up to root, down to 4) // All equal to our distance — no peer is strictly closer. - assert_eq!(state.find_next_hop(&dest, &HashSet::new()), None); + assert_eq!(state.find_next_hop(&dest, &BTreeSet::new()), None); } #[test] @@ -800,14 +614,14 @@ fn test_find_next_hop_different_root() { // Destination in a different tree (root = 9) let dest = make_coords(&[3, 9]); - assert_eq!(state.find_next_hop(&dest, &HashSet::new()), None); + assert_eq!(state.find_next_hop(&dest, &BTreeSet::new()), None); } #[test] fn test_find_next_hop_no_peers() { let state = make_tree_state(5, &[5, 0]); let dest = make_coords(&[3, 0]); - assert_eq!(state.find_next_hop(&dest, &HashSet::new()), None); + assert_eq!(state.find_next_hop(&dest, &BTreeSet::new()), None); } #[test] @@ -822,7 +636,7 @@ fn test_find_next_hop_local_minimum() { add_peer(&mut state, 8, &[8, 5, 0]); let dest = make_coords(&[3, 0]); - assert_eq!(state.find_next_hop(&dest, &HashSet::new()), None); + assert_eq!(state.find_next_hop(&dest, &BTreeSet::new()), None); } #[test] @@ -839,21 +653,13 @@ fn test_find_next_hop_best_of_multiple() { let dest = make_coords(&[7, 3, 1, 0]); assert_eq!( - state.find_next_hop(&dest, &HashSet::new()), + state.find_next_hop(&dest, &BTreeSet::new()), Some(make_node_addr(3)) ); } // === Cost-based parent selection tests === -/// Build a peer_costs map from (addr_byte, cost) pairs. -fn make_costs(entries: &[(u8, f64)]) -> HashMap { - entries - .iter() - .map(|&(addr, cost)| (make_node_addr(addr), cost)) - .collect() -} - #[test] fn test_effective_depth_selects_lower_cost_deeper_peer() { // Peer A at depth 1 with high cost (LoRa), peer B at depth 2 with low cost (fiber). @@ -861,7 +667,7 @@ fn test_effective_depth_selects_lower_cost_deeper_peer() { // effective_depth(B) = 2 + 1.01 = 3.01 // Should select B despite being deeper. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer_a = make_node_addr(1); let peer_b = make_node_addr(2); @@ -879,7 +685,7 @@ fn test_effective_depth_selects_lower_cost_deeper_peer() { ); let costs = make_costs(&[(1, 6.0), (2, 1.01)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, Some(peer_b)); } @@ -887,7 +693,7 @@ fn test_effective_depth_selects_lower_cost_deeper_peer() { fn test_effective_depth_equal_cost_degenerates_to_depth() { // Both peers at cost 1.0 (default). Should pick shallowest, same as v1. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let peer2 = make_node_addr(2); @@ -905,7 +711,7 @@ fn test_effective_depth_equal_cost_degenerates_to_depth() { ); let costs = make_costs(&[(1, 1.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, Some(peer1)); } @@ -913,7 +719,7 @@ fn test_effective_depth_equal_cost_degenerates_to_depth() { fn test_effective_depth_tiebreak_by_node_addr() { // Two peers with identical effective_depth. Smaller NodeAddr wins. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let peer2 = make_node_addr(2); @@ -930,7 +736,7 @@ fn test_effective_depth_tiebreak_by_node_addr() { ); let costs = make_costs(&[(1, 1.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, Some(peer1)); // smaller NodeAddr } @@ -940,7 +746,7 @@ fn test_hysteresis_prevents_marginal_switch() { // With 20% hysteresis, threshold = 3.5 * 0.8 = 2.8. // 3.2 > 2.8, so no switch. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); state.set_parent_hysteresis(0.2); let peer_a = make_node_addr(1); // current parent @@ -959,11 +765,11 @@ fn test_hysteresis_prevents_marginal_switch() { ); // Set peer_a as current parent - state.set_parent(peer_a, 1, 1000); + state.set_parent(peer_a, 1, 1000, 1000); state.recompute_coords(); let costs = make_costs(&[(1, 2.5), (2, 2.2)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, None); // marginal improvement blocked by hysteresis } @@ -973,7 +779,7 @@ fn test_hysteresis_allows_significant_switch() { // With 20% hysteresis, threshold = 7.0 * 0.8 = 5.6. // 3.01 < 5.6, so switch occurs. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); state.set_parent_hysteresis(0.2); let peer_a = make_node_addr(1); // current parent (LoRa) @@ -992,11 +798,11 @@ fn test_hysteresis_allows_significant_switch() { ); // Set peer_a as current parent - state.set_parent(peer_a, 1, 1000); + state.set_parent(peer_a, 1, 1000, 1000); state.recompute_coords(); let costs = make_costs(&[(1, 6.0), (2, 1.01)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, Some(peer_b)); } @@ -1005,7 +811,7 @@ fn test_cold_start_default_cost() { // Peer with no cost entry in map gets default 1.0. // This degenerates to depth-only selection. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); let peer2 = make_node_addr(2); @@ -1022,7 +828,7 @@ fn test_cold_start_default_cost() { ); // Empty cost map — all peers get default 1.0 - let result = state.evaluate_parent(&HashMap::new(), &HashSet::new()); + let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); assert_eq!(result, Some(peer1)); // shallowest wins } @@ -1030,7 +836,7 @@ fn test_cold_start_default_cost() { fn test_hold_down_suppresses_reeval() { // After a parent switch, re-evaluation returns None during hold-down. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); state.set_hold_down(60); // 60s hold-down let peer_a = make_node_addr(1); @@ -1047,13 +853,13 @@ fn test_hold_down_suppresses_reeval() { ); // Switch to peer_a (sets last_parent_switch) - state.set_parent(peer_a, 1, 1000); + state.set_parent(peer_a, 1, 1000, 1000); state.recompute_coords(); // Peer_b now offers better cost, but hold-down suppresses let costs = make_costs(&[(1, 5.0), (2, 1.0)]); state.set_parent_hysteresis(0.0); // no hysteresis, only hold-down - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, None); // suppressed by hold-down } @@ -1061,7 +867,7 @@ fn test_hold_down_suppresses_reeval() { fn test_mandatory_switch_bypasses_hold_down() { // Parent loss during hold-down still triggers switch. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); state.set_hold_down(60); // 60s hold-down let peer_a = make_node_addr(1); @@ -1078,12 +884,12 @@ fn test_mandatory_switch_bypasses_hold_down() { ); // Switch to peer_a - state.set_parent(peer_a, 1, 1000); + state.set_parent(peer_a, 1, 1000, 1000); state.recompute_coords(); // Remove peer_a (parent lost) — should bypass hold-down state.remove_peer(&peer_a); - let result = state.evaluate_parent(&HashMap::new(), &HashSet::new()); + let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); assert_eq!(result, Some(peer_b)); // mandatory switch } @@ -1109,7 +915,7 @@ fn test_heterogeneous_7node_avoids_bottleneck() { // Test from node 5's perspective let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer1 = make_node_addr(1); // fiber peer at depth 1 let peer2 = make_node_addr(2); // LoRa peer at depth 1 @@ -1125,28 +931,28 @@ fn test_heterogeneous_7node_avoids_bottleneck() { ); // Without costs (all 1.0): picks peer 1 (smaller addr) — correct by luck - let result_no_cost = state.evaluate_parent(&HashMap::new(), &HashSet::new()); + let result_no_cost = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); assert_eq!(result_no_cost, Some(peer1)); // With costs: fiber (1.01) vs LoRa (6.0) — fiber wins definitively let costs = make_costs(&[(1, 1.01), (2, 6.0)]); - let result_with_cost = state.evaluate_parent(&costs, &HashSet::new()); + let result_with_cost = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result_with_cost, Some(peer1)); // Now test the critical case: node 5 currently has LoRa parent (peer 2). // Even without hysteresis, it should want to switch to fiber (peer 1). - state.set_parent(peer2, 1, 1000); + state.set_parent(peer2, 1, 1000, 1000); state.recompute_coords(); assert_eq!(state.my_coords().depth(), 2); // depth 2 through LoRa peer - let result_switch = state.evaluate_parent(&costs, &HashSet::new()); + let result_switch = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result_switch, Some(peer1)); // switches away from LoRa bottleneck // With hysteresis enabled, still switches because the cost difference is large state.set_parent_hysteresis(0.2); // current_parent_eff = 1 + 6.0 = 7.0, best_eff = 1 + 1.01 = 2.01 // threshold = 7.0 * 0.8 = 5.6, 2.01 < 5.6 → switch - let result_hyst = state.evaluate_parent(&costs, &HashSet::new()); + let result_hyst = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result_hyst, Some(peer1)); } @@ -1164,7 +970,7 @@ fn test_cost_degradation_triggers_switch() { // (both fiber). After stabilization, peer A's link degrades (becomes // LoRa-like). Re-evaluation with updated costs should trigger a switch. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); state.set_parent_hysteresis(0.2); let peer_a = make_node_addr(1); @@ -1182,14 +988,14 @@ fn test_cost_degradation_triggers_switch() { // Initial: both fiber-like costs. Node picks peer_a (smaller addr). let initial_costs = make_costs(&[(1, 1.05), (2, 1.08)]); - let result = state.evaluate_parent(&initial_costs, &HashSet::new()); + let result = state.evaluate_parent(&initial_costs, &BTreeSet::new(), 1000); assert_eq!(result, Some(peer_a)); - state.set_parent(peer_a, 1, 1000); + state.set_parent(peer_a, 1, 1000, 1000); state.recompute_coords(); // Verify stable: no switch with same costs - let result = state.evaluate_parent(&initial_costs, &HashSet::new()); + let result = state.evaluate_parent(&initial_costs, &BTreeSet::new(), 1000); assert_eq!(result, None); // Peer A's link degrades significantly (LoRa-like latency + loss) @@ -1197,7 +1003,7 @@ fn test_cost_degradation_triggers_switch() { // best_eff = 1 + 1.08 = 2.08 // threshold = 7.0 * 0.8 = 5.6, 2.08 < 5.6 → switch let degraded_costs = make_costs(&[(1, 6.0), (2, 1.08)]); - let result = state.evaluate_parent(°raded_costs, &HashSet::new()); + let result = state.evaluate_parent(°raded_costs, &BTreeSet::new(), 1000); assert_eq!(result, Some(peer_b)); } @@ -1206,7 +1012,7 @@ fn test_cost_improvement_within_hysteresis_no_switch() { // Node 5 has parent peer_a. Peer_b's cost improves slightly but // stays within the hysteresis band. Re-evaluation should not switch. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); state.set_parent_hysteresis(0.2); let peer_a = make_node_addr(1); @@ -1222,7 +1028,7 @@ fn test_cost_improvement_within_hysteresis_no_switch() { make_coords(&[2, 0]), ); - state.set_parent(peer_a, 1, 1000); + state.set_parent(peer_a, 1, 1000, 1000); state.recompute_coords(); // Peer B slightly better: cost 1.5 vs peer A cost 2.0 @@ -1230,7 +1036,7 @@ fn test_cost_improvement_within_hysteresis_no_switch() { // best_eff = 1 + 1.5 = 2.5 // threshold = 3.0 * 0.8 = 2.4, 2.5 > 2.4 → no switch let costs = make_costs(&[(1, 2.0), (2, 1.5)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, None); } @@ -1240,7 +1046,7 @@ fn test_single_peer_no_reeval_benefit() { // but once it's our parent, re-evaluation returns None regardless // of cost changes (no alternative exists). let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let peer_a = make_node_addr(1); let root = make_node_addr(0); @@ -1252,251 +1058,18 @@ fn test_single_peer_no_reeval_benefit() { // Initial selection: picks the only peer let costs = make_costs(&[(1, 1.05)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); + let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); assert_eq!(result, Some(peer_a)); - state.set_parent(peer_a, 1, 1000); + state.set_parent(peer_a, 1, 1000, 1000); state.recompute_coords(); // Even with terrible cost, no switch (no alternative) let bad_costs = make_costs(&[(1, 50.0)]); - let result = state.evaluate_parent(&bad_costs, &HashSet::new()); + let result = state.evaluate_parent(&bad_costs, &BTreeSet::new(), 1000); assert_eq!(result, None); } -// ===================================================================== -// Flap dampening tests -// ===================================================================== - -#[test] -fn test_flap_dampening_engages_after_threshold() { - // Create TreeState with flap_threshold=3, window=60s, dampening=3600s (long) - let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); - state.set_flap_dampening(3, 60, 3600); - state.set_hold_down(0); // disable hold-down for this test - - let peer_a = make_node_addr(1); - let peer_b = make_node_addr(2); - let root = make_node_addr(0); - - state.update_peer( - ParentDeclaration::new(peer_a, root, 1, 1000), - make_coords(&[1, 0]), - ); - state.update_peer( - ParentDeclaration::new(peer_b, root, 1, 1000), - make_coords(&[2, 0]), - ); - - // Switch 1: initial parent selection (root -> peer_a) - assert!(!state.is_flap_dampened()); - state.set_parent(peer_a, 1, 1000); - state.recompute_coords(); - assert!(!state.is_flap_dampened()); - - // Switch 2: peer_a -> peer_b - state.set_parent(peer_b, 2, 2000); - state.recompute_coords(); - assert!(!state.is_flap_dampened()); - - // Switch 3: peer_b -> peer_a — threshold reached, dampening engages - let dampened = state.set_parent(peer_a, 3, 3000); - state.recompute_coords(); - assert!(dampened); - assert!(state.is_flap_dampened()); - - // evaluate_parent should return None for non-mandatory switches - // Make peer_b much better than peer_a - let costs = make_costs(&[(1, 10.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); - assert_eq!(result, None); // suppressed by flap dampening -} - -#[test] -fn test_flap_dampening_allows_mandatory_switches() { - // Engage dampening, then verify mandatory switches still work - let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); - state.set_flap_dampening(3, 60, 3600); - state.set_hold_down(0); - - let peer_a = make_node_addr(1); - let peer_b = make_node_addr(2); - let root = make_node_addr(0); - - state.update_peer( - ParentDeclaration::new(peer_a, root, 1, 1000), - make_coords(&[1, 0]), - ); - state.update_peer( - ParentDeclaration::new(peer_b, root, 1, 1000), - make_coords(&[2, 0]), - ); - - // Trigger dampening with 3 switches - state.set_parent(peer_a, 1, 1000); - state.recompute_coords(); - state.set_parent(peer_b, 2, 2000); - state.recompute_coords(); - state.set_parent(peer_a, 3, 3000); - state.recompute_coords(); - assert!(state.is_flap_dampened()); - - // Remove current parent (peer_a) — this is a mandatory switch - state.remove_peer(&peer_a); - let result = state.evaluate_parent(&HashMap::new(), &HashSet::new()); - assert_eq!(result, Some(peer_b)); // mandatory switch bypasses dampening -} - -#[test] -fn test_flap_dampening_expires() { - // Test with 0-second dampening duration to verify expiry logic - let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); - state.set_flap_dampening(3, 60, 0); // 0-second dampening - state.set_hold_down(0); - - let peer_a = make_node_addr(1); - let peer_b = make_node_addr(2); - let root = make_node_addr(0); - - state.update_peer( - ParentDeclaration::new(peer_a, root, 1, 1000), - make_coords(&[1, 0]), - ); - state.update_peer( - ParentDeclaration::new(peer_b, root, 1, 1000), - make_coords(&[2, 0]), - ); - - // Trigger dampening - state.set_parent(peer_a, 1, 1000); - state.recompute_coords(); - state.set_parent(peer_b, 2, 2000); - state.recompute_coords(); - let dampened = state.set_parent(peer_a, 3, 3000); - state.recompute_coords(); - assert!(dampened); // dampening was engaged - - // With 0-second duration, dampening should have already expired - assert!(!state.is_flap_dampened()); - - // evaluate_parent should work normally now - let costs = make_costs(&[(1, 10.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); - assert_eq!(result, Some(peer_b)); // not suppressed -} - -#[test] -fn test_flap_dampening_below_threshold() { - // Fewer switches than threshold should NOT engage dampening - let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); - state.set_flap_dampening(4, 60, 3600); // threshold=4 - state.set_hold_down(0); - - let peer_a = make_node_addr(1); - let peer_b = make_node_addr(2); - let root = make_node_addr(0); - - state.update_peer( - ParentDeclaration::new(peer_a, root, 1, 1000), - make_coords(&[1, 0]), - ); - state.update_peer( - ParentDeclaration::new(peer_b, root, 1, 1000), - make_coords(&[2, 0]), - ); - - // Only 3 switches (below threshold of 4) - state.set_parent(peer_a, 1, 1000); - state.recompute_coords(); - state.set_parent(peer_b, 2, 2000); - state.recompute_coords(); - state.set_parent(peer_a, 3, 3000); - state.recompute_coords(); - - assert!(!state.is_flap_dampened()); - - // evaluate_parent should still work normally - let costs = make_costs(&[(1, 10.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &HashSet::new()); - assert_eq!(result, Some(peer_b)); // not suppressed -} - -#[test] -fn test_flap_dampening_window_reset() { - // Test that the flap window resets after expiry. - // Use a 0-second window so it immediately expires between switch groups. - let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); - // threshold=3, window=0s (expires immediately), dampening=3600s - state.set_flap_dampening(3, 0, 3600); - state.set_hold_down(0); - - let peer_a = make_node_addr(1); - let peer_b = make_node_addr(2); - let root = make_node_addr(0); - - state.update_peer( - ParentDeclaration::new(peer_a, root, 1, 1000), - make_coords(&[1, 0]), - ); - state.update_peer( - ParentDeclaration::new(peer_b, root, 1, 1000), - make_coords(&[2, 0]), - ); - - // Each switch resets the window (0s window means every switch starts fresh). - // So we never accumulate enough to reach threshold=3. - state.set_parent(peer_a, 1, 1000); - state.recompute_coords(); - // Window expired, counter resets on next switch - state.set_parent(peer_b, 2, 2000); - state.recompute_coords(); - // Window expired, counter resets on next switch - state.set_parent(peer_a, 3, 3000); - state.recompute_coords(); - - // Dampening should NOT have engaged because each switch reset the window - assert!(!state.is_flap_dampened()); -} - -#[test] -fn test_flap_dampening_same_parent_no_count() { - // Re-declaring the same parent should not count as a flap - let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); - state.set_flap_dampening(3, 60, 3600); - state.set_hold_down(0); - - let peer_a = make_node_addr(1); - let root = make_node_addr(0); - - state.update_peer( - ParentDeclaration::new(peer_a, root, 1, 1000), - make_coords(&[1, 0]), - ); - - // Initial parent selection - state.set_parent(peer_a, 1, 1000); - state.recompute_coords(); - - // Re-declare same parent multiple times (e.g., parent ancestry changed) - state.set_parent(peer_a, 2, 2000); - state.recompute_coords(); - state.set_parent(peer_a, 3, 3000); - state.recompute_coords(); - state.set_parent(peer_a, 4, 4000); - state.recompute_coords(); - state.set_parent(peer_a, 5, 5000); - state.recompute_coords(); - - // Should NOT be dampened since only the first was a real switch - assert!(!state.is_flap_dampened()); -} - // === Skip peers (non-routing/leaf profile exclusion) === #[test] @@ -1504,7 +1077,7 @@ fn test_evaluate_parent_skips_non_full_peer() { // Two peers reaching the same root (0). Peer 3 is shallower (depth 1) // but in skip set. Peer 7 is deeper (depth 2) but not skipped. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let root = make_node_addr(0); let peer3 = make_node_addr(3); @@ -1521,9 +1094,9 @@ fn test_evaluate_parent_skips_non_full_peer() { make_coords(&[7, 2, 0]), ); - let mut skip = HashSet::new(); + let mut skip = BTreeSet::new(); skip.insert(peer3); - let result = state.evaluate_parent(&HashMap::new(), &skip); + let result = state.evaluate_parent(&BTreeMap::new(), &skip, 1000); assert_eq!(result, Some(peer7)); } @@ -1531,7 +1104,7 @@ fn test_evaluate_parent_skips_non_full_peer() { fn test_evaluate_parent_all_skipped_returns_none() { // Only peer is in skip set — no valid parent. let my_node = make_node_addr(5); - let mut state = TreeState::new(my_node); + let mut state = TreeState::new(my_node, 1000); let root = make_node_addr(0); let peer3 = make_node_addr(3); @@ -1540,9 +1113,9 @@ fn test_evaluate_parent_all_skipped_returns_none() { make_coords(&[3, 0]), ); - let mut skip = HashSet::new(); + let mut skip = BTreeSet::new(); skip.insert(peer3); - assert_eq!(state.evaluate_parent(&HashMap::new(), &skip), None); + assert_eq!(state.evaluate_parent(&BTreeMap::new(), &skip, 1000), None); } #[test] @@ -1555,7 +1128,7 @@ fn test_find_next_hop_skips_non_full_peer() { add_peer(&mut state, 2, &[2, 1, 5, 0]); let dest = make_coords(&[2, 1, 5, 0]); - let mut skip = HashSet::new(); + let mut skip = BTreeSet::new(); skip.insert(make_node_addr(2)); assert_eq!(state.find_next_hop(&dest, &skip), Some(make_node_addr(1))); } @@ -1567,7 +1140,7 @@ fn test_find_next_hop_all_closer_skipped() { add_peer(&mut state, 1, &[1, 5, 0]); let dest = make_coords(&[2, 1, 5, 0]); - let mut skip = HashSet::new(); + let mut skip = BTreeSet::new(); skip.insert(make_node_addr(1)); assert_eq!(state.find_next_hop(&dest, &skip), None); } diff --git a/src/proto/stp/tests/util.rs b/src/proto/stp/tests/util.rs new file mode 100644 index 0000000..58ee826 --- /dev/null +++ b/src/proto/stp/tests/util.rs @@ -0,0 +1,44 @@ +//! Shared test helpers for the STP primitive unit tests. + +use std::collections::BTreeMap; + +use crate::NodeAddr; +use crate::proto::stp::{ParentDeclaration, TreeCoordinate, TreeState}; + +pub(super) fn make_node_addr(val: u8) -> NodeAddr { + let mut bytes = [0u8; 16]; + bytes[0] = val; + NodeAddr::from_bytes(bytes) +} + +pub(super) fn make_coords(ids: &[u8]) -> TreeCoordinate { + TreeCoordinate::from_addrs(ids.iter().map(|&v| make_node_addr(v)).collect()).unwrap() +} + +/// Build a TreeState with our own coordinates set. +pub(super) fn make_tree_state(my_addr: u8, coord_path: &[u8]) -> TreeState { + let my_node = make_node_addr(my_addr); + let mut state = TreeState::new(my_node, 1000); + let coords = make_coords(coord_path); + state.root = *coords.root_id(); + state.my_coords = coords; + state +} + +/// Add a peer with given coordinates to the tree state. +pub(super) fn add_peer(state: &mut TreeState, peer_addr: u8, coord_path: &[u8]) { + let peer = make_node_addr(peer_addr); + let parent = make_node_addr(coord_path[1]); + state.update_peer( + ParentDeclaration::new(peer, parent, 1, 1000), + make_coords(coord_path), + ); +} + +/// Build a peer_costs map from (addr_byte, cost) pairs. +pub(super) fn make_costs(entries: &[(u8, f64)]) -> BTreeMap { + entries + .iter() + .map(|&(addr, cost)| (make_node_addr(addr), cost)) + .collect() +} diff --git a/src/protocol/tree.rs b/src/proto/stp/wire.rs similarity index 93% rename from src/protocol/tree.rs rename to src/proto/stp/wire.rs index 4255c81..9c572e9 100644 --- a/src/protocol/tree.rs +++ b/src/proto/stp/wire.rs @@ -1,9 +1,9 @@ //! TreeAnnounce message: spanning tree state propagation. -use super::error::ProtocolError; -use super::link::LinkMessageType; +use super::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError}; use crate::NodeAddr; -use crate::tree::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError}; +use crate::protocol::LinkMessageType; +use crate::protocol::ProtocolError; use secp256k1::schnorr::Signature; /// Spanning tree announcement carrying parent declaration and ancestry. @@ -218,8 +218,11 @@ impl TreeAnnounce { let sig_bytes: [u8; 64] = payload[pos..pos + 64] .try_into() .map_err(|_| ProtocolError::Malformed("bad signature".into()))?; - let signature = - Signature::from_slice(&sig_bytes).map_err(|_| ProtocolError::InvalidSignature)?; + // Validate the signature parses as a well-formed schnorr signature (the + // codec's only crypto touch, §11 w2); store the raw bytes so the in-core + // declaration carries no `secp256k1` dependency. Actual verification is a + // shell concern (§6). + Signature::from_slice(&sig_bytes).map_err(|_| ProtocolError::InvalidSignature)?; // The first entry's node_addr is the declaring node if entries.is_empty() { @@ -230,7 +233,7 @@ impl TreeAnnounce { let node_addr = entries[0].node_addr; let declaration = - ParentDeclaration::with_signature(node_addr, parent, sequence, timestamp, signature); + ParentDeclaration::with_signature(node_addr, parent, sequence, timestamp, sig_bytes); let ancestry = TreeCoordinate::new(entries) .map_err(|e| ProtocolError::Malformed(format!("bad ancestry: {}", e)))?; @@ -245,6 +248,7 @@ impl TreeAnnounce { #[cfg(test)] mod tests { use super::*; + use crate::identity::Identity; fn make_node_addr(val: u8) -> NodeAddr { let mut bytes = [0u8; 16]; @@ -252,6 +256,14 @@ mod tests { NodeAddr::from_bytes(bytes) } + /// Sign a declaration in place. In production the shell owns the key-crypto + /// (§6); this test-local helper keeps the sign/verify boundary out of the + /// in-core `state.rs` while letting the codec tests build signed messages. + fn sign_decl(decl: &mut ParentDeclaration, identity: &Identity) { + let sig = identity.sign(&decl.signing_bytes()); + decl.set_signature(sig.to_byte_array()); + } + fn make_coords(ids: &[u8]) -> TreeCoordinate { TreeCoordinate::from_addrs(ids.iter().map(|&v| make_node_addr(v)).collect()).unwrap() } @@ -278,7 +290,7 @@ mod tests { // Root declaration: parent == self let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 5000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); // Root ancestry: just the root itself let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 5000)]).unwrap(); @@ -317,7 +329,7 @@ mod tests { let root = make_node_addr(4); let mut decl = ParentDeclaration::new(node_addr, parent, 5, 10000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![ CoordEntry::new(node_addr, 5, 10000), @@ -366,7 +378,7 @@ mod tests { let node_addr = *identity.node_addr(); let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 1000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 1000)]).unwrap(); let announce = TreeAnnounce::new(decl, ancestry); @@ -408,7 +420,7 @@ mod tests { let node_addr = *identity.node_addr(); let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 1000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 1000)]).unwrap(); let announce = TreeAnnounce::new(decl, ancestry); @@ -453,7 +465,7 @@ mod tests { let root = make_node_addr(1); let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![ CoordEntry::new(node_addr, 5, 1000), @@ -477,7 +489,7 @@ mod tests { let advertised_root = make_node_addr(1); let mut decl = ParentDeclaration::new(node_addr, smaller, 5, 1000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![ CoordEntry::new(node_addr, 5, 1000), @@ -507,7 +519,7 @@ mod tests { let ancestry_parent = make_node_addr(3); let mut decl = ParentDeclaration::new(node_addr, declared_parent, 5, 1000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![ CoordEntry::new(node_addr, 5, 1000), @@ -537,7 +549,7 @@ mod tests { let parent = make_node_addr(2); let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![ CoordEntry::new(ancestry_sender, 5, 1000), @@ -565,7 +577,7 @@ mod tests { let node_addr = *identity.node_addr(); let mut decl = ParentDeclaration::self_root(node_addr, 5, 1000); - decl.sign(&identity).unwrap(); + sign_decl(&mut decl, &identity); let ancestry = TreeCoordinate::new(vec![ CoordEntry::new(node_addr, 5, 1000), diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index fb3995d..3b6538f 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -24,7 +24,6 @@ mod error; mod filter; mod link; pub(crate) mod session; -mod tree; // Re-export all public types at protocol:: level pub use error::ProtocolError; @@ -37,7 +36,6 @@ pub use session::{ SessionSetup, }; pub(crate) use session::{coords_wire_size, decode_optional_coords, encode_coords}; -pub use tree::TreeAnnounce; /// Protocol version for message compatibility. pub const PROTOCOL_VERSION: u8 = 1; diff --git a/src/protocol/session.rs b/src/protocol/session.rs index c12ad73..fc586f0 100644 --- a/src/protocol/session.rs +++ b/src/protocol/session.rs @@ -2,7 +2,7 @@ use super::ProtocolError; use crate::NodeAddr; -use crate::tree::TreeCoordinate; +use crate::proto::stp::TreeCoordinate; use std::fmt; // ============================================================================ diff --git a/src/tree/declaration.rs b/src/tree/declaration.rs deleted file mode 100644 index c66b987..0000000 --- a/src/tree/declaration.rs +++ /dev/null @@ -1,182 +0,0 @@ -//! Parent declarations for the spanning tree. - -use secp256k1::XOnlyPublicKey; -use secp256k1::schnorr::Signature; -use std::fmt; - -use super::TreeError; -use crate::{Identity, NodeAddr}; - -/// A node's declaration of its parent in the spanning tree. -/// -/// Each node periodically announces its parent selection. The declaration -/// includes a monotonic sequence number for freshness and a signature -/// for authenticity. When `parent_id == node_addr`, the node declares itself -/// as a root candidate. -#[derive(Clone)] -pub struct ParentDeclaration { - /// The node making this declaration. - node_addr: NodeAddr, - /// The selected parent (equals node_addr if self-declaring as root). - parent_id: NodeAddr, - /// Monotonically increasing sequence number. - sequence: u64, - /// Timestamp when this declaration was created (Unix seconds). - timestamp: u64, - /// Schnorr signature over the declaration fields. - signature: Option, -} - -impl ParentDeclaration { - /// Create a new unsigned parent declaration. - /// - /// The declaration must be signed before transmission using `set_signature()`. - pub fn new(node_addr: NodeAddr, parent_id: NodeAddr, sequence: u64, timestamp: u64) -> Self { - Self { - node_addr, - parent_id, - sequence, - timestamp, - signature: None, - } - } - - /// Create a self-declaration (node is root candidate). - pub fn self_root(node_addr: NodeAddr, sequence: u64, timestamp: u64) -> Self { - Self::new(node_addr, node_addr, sequence, timestamp) - } - - /// Create a declaration with a pre-computed signature. - pub fn with_signature( - node_addr: NodeAddr, - parent_id: NodeAddr, - sequence: u64, - timestamp: u64, - signature: Signature, - ) -> Self { - Self { - node_addr, - parent_id, - sequence, - timestamp, - signature: Some(signature), - } - } - - /// Get the declaring node's ID. - pub fn node_addr(&self) -> &NodeAddr { - &self.node_addr - } - - /// Get the parent node's ID. - pub fn parent_id(&self) -> &NodeAddr { - &self.parent_id - } - - /// Get the sequence number. - pub fn sequence(&self) -> u64 { - self.sequence - } - - /// Get the timestamp. - pub fn timestamp(&self) -> u64 { - self.timestamp - } - - /// Get the signature, if set. - pub fn signature(&self) -> Option<&Signature> { - self.signature.as_ref() - } - - /// Set the signature after signing. - pub fn set_signature(&mut self, signature: Signature) { - self.signature = Some(signature); - } - - /// Sign this declaration with the given identity. - /// - /// The identity's node_addr must match this declaration's node_addr. - /// Returns an error if the node_addrs don't match. - pub fn sign(&mut self, identity: &Identity) -> Result<(), TreeError> { - if identity.node_addr() != &self.node_addr { - return Err(TreeError::InvalidSignature(self.node_addr)); - } - let signature = identity.sign(&self.signing_bytes()); - self.signature = Some(signature); - Ok(()) - } - - /// Check if this is a root declaration (parent == self). - pub fn is_root(&self) -> bool { - self.node_addr == self.parent_id - } - - /// Check if this declaration is signed. - pub fn is_signed(&self) -> bool { - self.signature.is_some() - } - - /// Get the bytes that should be signed. - /// - /// Format: node_addr (16) || parent_id (16) || sequence (8) || timestamp (8) - pub fn signing_bytes(&self) -> Vec { - let mut bytes = Vec::with_capacity(48); - bytes.extend_from_slice(self.node_addr.as_bytes()); - bytes.extend_from_slice(self.parent_id.as_bytes()); - bytes.extend_from_slice(&self.sequence.to_le_bytes()); - bytes.extend_from_slice(&self.timestamp.to_le_bytes()); - bytes - } - - /// Verify the signature on this declaration. - /// - /// Returns Ok(()) if the signature is valid, or an error otherwise. - pub fn verify(&self, pubkey: &XOnlyPublicKey) -> Result<(), TreeError> { - let signature = self - .signature - .as_ref() - .ok_or(TreeError::InvalidSignature(self.node_addr))?; - - let secp = secp256k1::Secp256k1::verification_only(); - let hash = self.signing_hash(); - - secp.verify_schnorr(signature, &hash, pubkey) - .map_err(|_| TreeError::InvalidSignature(self.node_addr)) - } - - /// Compute the SHA-256 hash of the signing bytes. - fn signing_hash(&self) -> [u8; 32] { - use sha2::{Digest, Sha256}; - let mut hasher = Sha256::new(); - hasher.update(self.signing_bytes()); - hasher.finalize().into() - } - - /// Check if this declaration is fresher than another. - pub fn is_fresher_than(&self, other: &ParentDeclaration) -> bool { - self.sequence > other.sequence - } -} - -impl fmt::Debug for ParentDeclaration { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("ParentDeclaration") - .field("node_addr", &self.node_addr) - .field("parent_id", &self.parent_id) - .field("sequence", &self.sequence) - .field("is_root", &self.is_root()) - .field("signed", &self.is_signed()) - .finish() - } -} - -impl PartialEq for ParentDeclaration { - fn eq(&self, other: &Self) -> bool { - self.node_addr == other.node_addr - && self.parent_id == other.parent_id - && self.sequence == other.sequence - && self.timestamp == other.timestamp - } -} - -impl Eq for ParentDeclaration {}