From dc9334e7256a4a3651e262a986302833460438bd Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 15:13:41 +0000 Subject: [PATCH] proto/stp: hoist the parent flap/hold-down veto to the shell for a clock-free classify core evaluate_parent no longer reads the injected clock: it returns a ParentEval of Mandatory, Discretionary, or None, and the flap/hold-down veto is applied at the edge via a new TreeState::is_switch_suppressed(now_ms), gating only the discretionary arm. classify_announce/classify_periodic take the pre-computed switch_suppressed bool instead of now_ms, so the whole classify ladder is clock-free; the shell callers (tree announce/periodic re-eval, MMP first-RTT re-eval, handle_parent_lost) compute the veto verdict. The no-coords parent case stays discretionary (veto-gated) exactly as before. Behavior unchanged; the veto tests now assert the moved responsibility. --- src/node/handlers/mmp.rs | 18 ++++-- src/node/tree.rs | 16 ++++- src/proto/stp/core.rs | 81 +++++++++++++++++------ src/proto/stp/mod.rs | 2 +- src/proto/stp/state.rs | 88 +++++++++++++++---------- src/proto/stp/tests/limits.rs | 40 ++++++++---- src/proto/stp/tests/state.rs | 117 +++++++++++++++++++++++++--------- 7 files changed, 260 insertions(+), 102 deletions(-) diff --git a/src/node/handlers/mmp.rs b/src/node/handlers/mmp.rs index 39ea9c7..e0866af 100644 --- a/src/node/handlers/mmp.rs +++ b/src/node/handlers/mmp.rs @@ -13,6 +13,7 @@ use crate::proto::mmp::{ LinkReportKind, LinkReportSnapshot, MmpAction, PeerLivenessSnapshot, ReceiverReport, RrLog, SenderReport, }; +use crate::proto::stp::ParentEval; use std::time::{Duration, Instant}; use tracing::{debug, info, trace, warn}; @@ -198,11 +199,18 @@ impl Node { .map(|d| d.as_secs()) .unwrap_or(0); let mono_now_ms = crate::time::mono_ms(); - if let Some(new_parent) = self.tree_state.evaluate_parent( - &peer_costs, - &std::collections::BTreeSet::new(), - mono_now_ms, - ) { + // Compute the flap-dampening / hold-down veto at the edge; a mandatory + // switch bypasses it, a discretionary one is taken only if not suppressed. + let switch_suppressed = self.tree_state.is_switch_suppressed(mono_now_ms); + let new_parent = match self + .tree_state + .evaluate_parent(&peer_costs, &std::collections::BTreeSet::new()) + { + ParentEval::Mandatory(p) => Some(p), + ParentEval::Discretionary(p) if !switch_suppressed => Some(p), + ParentEval::Discretionary(_) | ParentEval::None => None, + }; + if let Some(new_parent) = new_parent { let new_seq = self.tree_state.my_declaration().sequence() + 1; let flap_dampened = self.tree_state diff --git a/src/node/tree.rs b/src/node/tree.rs index d7e7fc3..f14c4b3 100644 --- a/src/node/tree.rs +++ b/src/node/tree.rs @@ -329,8 +329,17 @@ impl Node { // 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::time::mono_ms(); + // Compute the flap-dampening / hold-down veto at the edge; the classify core + // is clock-free and consumes only this pre-computed verdict. + let switch_suppressed = self.tree_state.is_switch_suppressed(mono_now_ms); - match Stp::classify_announce(&self.tree_state, *from, &peer_costs, &skip, mono_now_ms) { + match Stp::classify_announce( + &self.tree_state, + *from, + &peer_costs, + &skip, + switch_suppressed, + ) { TreeDecision::Switch { new_parent, new_seq, @@ -583,8 +592,11 @@ impl Node { // Monotonic ms for the flap-dampening / hold-down timers, read once and // threaded into classify + the state mutators. let mono_now_ms = crate::time::mono_ms(); + // Compute the flap-dampening / hold-down veto at the edge; the classify core + // is clock-free and consumes only this pre-computed verdict. + let switch_suppressed = self.tree_state.is_switch_suppressed(mono_now_ms); - match Stp::classify_periodic(&self.tree_state, &peer_costs, &skip, mono_now_ms) { + match Stp::classify_periodic(&self.tree_state, &peer_costs, &skip, switch_suppressed) { TreeDecision::Switch { new_parent, new_seq, diff --git a/src/proto/stp/core.rs b/src/proto/stp/core.rs index 8a93b62..669d000 100644 --- a/src/proto/stp/core.rs +++ b/src/proto/stp/core.rs @@ -3,10 +3,13 @@ //! 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`. +//! shell-passed facts (`peer_costs`, `skip`, and the pre-computed `switch_suppressed` +//! verdict of the flap-dampening / hold-down veto) and returns a [`TreeDecision`] +//! the shell drives. No I/O, no tracing, no keys, no mutation — and no clock: the +//! classify core is clock-free. The shell reads the monotonic clock at the edge, +//! computes the veto verdict via `TreeState::is_switch_suppressed`, and passes the +//! resulting bool in; `evaluate_parent` distinguishes mandatory from discretionary +//! switches and the ladder applies the veto only to the discretionary arm. //! //! [`TreeState`] lives beside this module in `proto/stp/state.rs`. @@ -76,22 +79,55 @@ pub(crate) enum TreeDecision { NoChange, } +/// Outcome of `TreeState::evaluate_parent`: whether a parent switch is warranted +/// and, if so, whether it is mandatory (bypasses the flap/hold-down veto) or +/// discretionary (the shell applies the veto before switching). +pub(crate) enum ParentEval { + /// Path-breaking / root-correcting switch — always taken, veto bypassed. + Mandatory(NodeAddr), + /// Improvement switch — taken only if the shell's veto is not active. + Discretionary(NodeAddr), + /// No switch warranted. + None, +} + +impl ParentEval { + /// The switch target if one is warranted IGNORING the veto (i.e. treating a + /// discretionary candidate as taken). Convenience for tests/paths that do not + /// engage the veto. Returns `None` only for `ParentEval::None`. + #[cfg(test)] + pub(crate) fn switch_target(&self) -> Option { + match self { + ParentEval::Mandatory(a) | ParentEval::Discretionary(a) => Some(*a), + ParentEval::None => Option::None, + } + } +} + 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. + /// (per-peer link cost), `skip` (non-full/leaf peers excluded from parent + /// candidacy — empty on master, `non_full_peers()` on next), and the + /// pre-computed `switch_suppressed` veto verdict (the shell reads the clock and + /// calls `TreeState::is_switch_suppressed`; a discretionary switch is skipped + /// while it is true, a mandatory switch ignores it). 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, + switch_suppressed: bool, ) -> TreeDecision { - if let Some(new_parent) = tree.evaluate_parent(peer_costs, skip, now_ms) { + let switch = match tree.evaluate_parent(peer_costs, skip) { + ParentEval::Mandatory(p) => Some(p), + ParentEval::Discretionary(p) if !switch_suppressed => Some(p), + ParentEval::Discretionary(_) | ParentEval::None => None, + }; + if let Some(new_parent) = switch { let new_seq = tree.my_declaration().sequence() + 1; TreeDecision::Switch { new_parent, @@ -120,20 +156,27 @@ impl Stp { /// 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. + /// Pure: reads the current `tree` plus the shell-passed `peer_costs`, `skip` + /// (empty on master, `non_full_peers()` on next), and the pre-computed + /// `switch_suppressed` veto verdict (see `classify_announce`; a discretionary + /// switch is skipped while true, a mandatory switch ignores it). 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, + switch_suppressed: bool, ) -> TreeDecision { - if let Some(new_parent) = tree.evaluate_parent(peer_costs, skip, now_ms) { + let switch = match tree.evaluate_parent(peer_costs, skip) { + ParentEval::Mandatory(p) => Some(p), + ParentEval::Discretionary(p) if !switch_suppressed => Some(p), + ParentEval::Discretionary(_) | ParentEval::None => None, + }; + if let Some(new_parent) = switch { let new_seq = tree.my_declaration().sequence() + 1; TreeDecision::Switch { new_parent, diff --git a/src/proto/stp/mod.rs b/src/proto/stp/mod.rs index 37f7c89..d0fb51d 100644 --- a/src/proto/stp/mod.rs +++ b/src/proto/stp/mod.rs @@ -32,7 +32,7 @@ use thiserror::Error; use crate::{IdentityError, NodeAddr}; pub use coordinate::{CoordEntry, TreeCoordinate}; -pub(crate) use core::{Stp, TreeDecision}; +pub(crate) use core::{ParentEval, Stp, TreeDecision}; pub use state::{ParentDeclaration, TreeState}; pub use wire::TreeAnnounce; pub(crate) use wire::{ diff --git a/src/proto/stp/state.rs b/src/proto/stp/state.rs index 817cc4c..9d6aeba 100644 --- a/src/proto/stp/state.rs +++ b/src/proto/stp/state.rs @@ -3,6 +3,7 @@ use alloc::collections::{BTreeMap, BTreeSet}; use core::fmt; +use super::core::ParentEval; use super::limits::FlapDampener; use super::{CoordEntry, TreeCoordinate}; use crate::NodeAddr; @@ -316,28 +317,38 @@ impl TreeState { self.flap.is_flap_dampened(now_ms) } + /// Whether a *discretionary* parent switch should currently be suppressed by the + /// flap-dampening / hold-down veto. `now_ms` is the injected monotonic time. + /// Mandatory switches ignore this. Read at the shell edge; the classify core is + /// clock-free and takes the resulting bool. + pub fn is_switch_suppressed(&self, now_ms: u64) -> bool { + self.flap.is_hold_down_active(now_ms) || self.flap.is_flap_dampened(now_ms) + } + /// Evaluate whether to switch parents based on current peer tree state. /// /// Uses effective_depth (depth + link_cost) for parent comparison. /// `peer_costs` maps each peer's NodeAddr to its link cost (from local /// MMP measurements). Missing entries default to 1.0 (optimistic). /// - /// Returns `Some(peer_node_addr)` if a parent switch is recommended, - /// or `None` if the current parent is adequate. + /// Returns a [`ParentEval`] describing whether a parent switch is warranted: + /// `Mandatory` (path-breaking / root-correcting — always taken), `Discretionary` + /// (an improvement the caller applies only if its veto is inactive), or `None`. + /// + /// This core is clock-free: it no longer applies the flap-dampening / hold-down + /// veto. The caller reads the clock, computes the veto verdict via + /// [`is_switch_suppressed`](Self::is_switch_suppressed), and suppresses the + /// `Discretionary` arm at the edge; `Mandatory` switches bypass the veto. /// /// `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( + pub(crate) fn evaluate_parent( &self, peer_costs: &BTreeMap, skip_peers: &BTreeSet, - now_ms: u64, - ) -> Option { + ) -> ParentEval { if self.peer_ancestry.is_empty() { - return None; + return ParentEval::None; } // Find the smallest root visible across all peers @@ -356,7 +367,10 @@ impl TreeState { }); } - let smallest_root = smallest_root?; + let smallest_root = match smallest_root { + Some(r) => r, + None => return ParentEval::None, + }; // If our own NodeAddr is smaller than (or equal to) the smallest visible // root, we are the network's smallest node and must be root. Returning @@ -366,7 +380,7 @@ impl TreeState { // peer's larger root at the tail — violating "advertised root = min path // entry" and getting rejected by recipients' `validate_semantics`. if self.my_node_addr <= smallest_root { - return None; + return ParentEval::None; } // Among peers that reach the smallest root, find the lowest effective_depth. @@ -404,14 +418,17 @@ impl TreeState { } } - let (best_peer_id, best_eff_depth) = best_peer?; + let (best_peer_id, best_eff_depth) = match best_peer { + Some(b) => b, + None => return ParentEval::None, + }; // If already using this peer as parent, no switch needed if *self.my_declaration.parent_id() == best_peer_id && !self.is_root() { - return None; + return ParentEval::None; } - // --- Mandatory switches (bypass hold-down and hysteresis) --- + // --- Mandatory switches (bypass the shell's hold-down / flap veto) --- // If our current parent is gone from peer_ancestry, our path is broken — always switch if !self.is_root() @@ -419,32 +436,24 @@ impl TreeState { .peer_ancestry .contains_key(self.my_declaration.parent_id()) { - return Some(best_peer_id); + return ParentEval::Mandatory(best_peer_id); } // Switching roots (smaller root found) → always switch if smallest_root < self.root || (self.is_root() && smallest_root < self.my_node_addr) { - return Some(best_peer_id); + return ParentEval::Mandatory(best_peer_id); } // We're root but shouldn't be (peers have a smaller root) — always switch if self.is_root() { - return Some(best_peer_id); + return ParentEval::Mandatory(best_peer_id); } - // --- Hold-down: suppress non-mandatory re-evaluation after recent switch --- - - if self.flap.is_hold_down_active(now_ms) { - return None; - } - - // --- Flap dampening: suppress after excessive parent switches --- - - if self.flap.is_flap_dampened(now_ms) { - return None; - } - - // --- Same root, cost-aware comparison with hysteresis --- + // --- Discretionary switches (the caller applies the veto before taking) --- + // + // Same root, cost-aware comparison with hysteresis. Everything below is + // veto-gated at the edge: the shell suppresses these `Discretionary` results + // while hold-down / flap-dampening is active. // Current parent's effective_depth. // If peer_costs is non-empty but current parent has no entry, @@ -461,15 +470,17 @@ impl TreeState { let current_parent_coords = self.peer_ancestry.get(self.my_declaration.parent_id()); let current_parent_eff = match current_parent_coords { Some(coords) => coords.depth() as f64 + current_parent_cost, - None => return Some(best_peer_id), // Parent has no coords — treat as lost + // Parent has no coords — treat as lost. This sat BELOW the veto, so it + // is veto-gated: Discretionary, not Mandatory. + None => return ParentEval::Discretionary(best_peer_id), }; // Apply hysteresis: only switch if candidate is significantly better if best_eff_depth < current_parent_eff * (1.0 - self.parent_hysteresis) { - return Some(best_peer_id); + return ParentEval::Discretionary(best_peer_id); } - None + ParentEval::None } /// Handle loss of current parent. @@ -488,8 +499,15 @@ impl TreeState { now_secs: u64, now_ms: u64, ) -> bool { - // Try to find an alternative parent - if let Some(new_parent) = self.evaluate_parent(peer_costs, &BTreeSet::new(), now_ms) { + // Try to find an alternative parent. The veto is computed at the edge and + // applied only to a discretionary result; a mandatory switch bypasses it. + let suppressed = self.is_switch_suppressed(now_ms); + let alt = match self.evaluate_parent(peer_costs, &BTreeSet::new()) { + ParentEval::Mandatory(p) => Some(p), + ParentEval::Discretionary(p) if !suppressed => Some(p), + ParentEval::Discretionary(_) | ParentEval::None => None, + }; + if let Some(new_parent) = alt { let new_seq = self.my_declaration.sequence() + 1; self.set_parent(new_parent, new_seq, now_secs, now_ms); self.recompute_coords(); diff --git a/src/proto/stp/tests/limits.rs b/src/proto/stp/tests/limits.rs index e6d8d94..078fd18 100644 --- a/src/proto/stp/tests/limits.rs +++ b/src/proto/stp/tests/limits.rs @@ -3,7 +3,7 @@ use std::collections::{BTreeMap, BTreeSet}; use super::util::{make_coords, make_costs, make_node_addr}; -use crate::proto::stp::{ParentDeclaration, TreeState}; +use crate::proto::stp::{ParentDeclaration, ParentEval, TreeState}; #[test] fn test_flap_dampening_engages_after_threshold() { @@ -43,11 +43,16 @@ fn test_flap_dampening_engages_after_threshold() { 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 + // The flap-dampening veto now lives at the shell edge, not inside + // evaluate_parent. The clock-free core still finds peer_b as a discretionary + // candidate (peer_b much better than peer_a); the shell suppresses it because + // dampening is active — same net "no switch" as before. 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 + assert!(state.is_switch_suppressed(3000)); // veto active at the edge + assert!(matches!( + state.evaluate_parent(&costs, &BTreeSet::new()), + ParentEval::Discretionary(p) if p == peer_b + )); } #[test] @@ -82,8 +87,13 @@ fn test_flap_dampening_allows_mandatory_switches() { // 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 + // Dampening is still active at the edge, but a parent-lost switch is mandatory + // and bypasses the veto: the core returns Mandatory and the switch is taken. + assert!(state.is_switch_suppressed(3000)); // veto still active + assert!(matches!( + state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new()), + ParentEval::Mandatory(p) if p == peer_b + )); } #[test] @@ -119,9 +129,13 @@ fn test_flap_dampening_expires() { // With 0-second duration, dampening should have already expired assert!(!state.is_flap_dampened(3000)); - // evaluate_parent should work normally now + // Dampening has expired: the veto is no longer active at the edge, so the + // discretionary switch to peer_b resumes and is taken. let costs = make_costs(&[(1, 10.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &BTreeSet::new(), 3000); + assert!(!state.is_switch_suppressed(3000)); // veto cleared + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer_b)); // not suppressed } @@ -156,9 +170,13 @@ fn test_flap_dampening_below_threshold() { assert!(!state.is_flap_dampened(3000)); - // evaluate_parent should still work normally + // Dampening never engaged, so the veto is inactive at the edge and the + // discretionary switch to peer_b is taken normally. let costs = make_costs(&[(1, 10.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &BTreeSet::new(), 3000); + assert!(!state.is_switch_suppressed(3000)); // veto never engaged + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer_b)); // not suppressed } diff --git a/src/proto/stp/tests/state.rs b/src/proto/stp/tests/state.rs index 3966ecc..70bcaa9 100644 --- a/src/proto/stp/tests/state.rs +++ b/src/proto/stp/tests/state.rs @@ -3,7 +3,7 @@ use std::collections::{BTreeMap, BTreeSet}; use super::util::{add_peer, make_coords, make_costs, make_node_addr, make_tree_state}; -use crate::proto::stp::{ParentDeclaration, TreeCoordinate, TreeState}; +use crate::proto::stp::{ParentDeclaration, ParentEval, TreeCoordinate, TreeState}; // ===== ParentDeclaration Tests ===== @@ -214,7 +214,9 @@ fn test_evaluate_parent_picks_smallest_root() { make_coords(&[7, 2]), ); - let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer3)); } @@ -240,7 +242,9 @@ fn test_evaluate_parent_prefers_shallowest_depth() { make_coords(&[2, 3, 4, 0]), ); - let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer1)); } @@ -258,7 +262,9 @@ fn test_evaluate_parent_stays_root_when_smallest() { ); assert_eq!( - state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), + state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(), None ); } @@ -283,7 +289,9 @@ fn test_evaluate_parent_no_switch_when_already_best() { // Now evaluate — should return None since peer1 is already our parent assert_eq!( - state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), + state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(), None ); } @@ -294,7 +302,9 @@ fn test_evaluate_parent_no_peers() { let state = TreeState::new(my_node, 1000); assert_eq!( - state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), + state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(), None ); } @@ -329,7 +339,9 @@ fn test_evaluate_parent_depth_threshold() { make_coords(&[3, 0]), ); - let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer3)); } @@ -351,7 +363,9 @@ fn test_evaluate_parent_rejects_loop_candidate() { // Should return None — the only candidate creates a loop assert_eq!( - state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000), + state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(), None ); } @@ -378,7 +392,9 @@ fn test_evaluate_parent_picks_loop_free_over_loopy() { make_coords(&[2, 3, 4, 0]), ); - let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer2)); } @@ -685,7 +701,9 @@ 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, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer_b)); } @@ -711,7 +729,9 @@ 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, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer1)); } @@ -736,7 +756,9 @@ fn test_effective_depth_tiebreak_by_node_addr() { ); let costs = make_costs(&[(1, 1.0), (2, 1.0)]); - let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer1)); // smaller NodeAddr } @@ -769,7 +791,9 @@ fn test_hysteresis_prevents_marginal_switch() { state.recompute_coords(); let costs = make_costs(&[(1, 2.5), (2, 2.2)]); - let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, None); // marginal improvement blocked by hysteresis } @@ -802,7 +826,9 @@ fn test_hysteresis_allows_significant_switch() { state.recompute_coords(); let costs = make_costs(&[(1, 6.0), (2, 1.01)]); - let result = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer_b)); } @@ -828,7 +854,9 @@ fn test_cold_start_default_cost() { ); // Empty cost map — all peers get default 1.0 - let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer1)); // shallowest wins } @@ -859,8 +887,14 @@ fn test_hold_down_suppresses_reeval() { // 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, &BTreeSet::new(), 1000); - assert_eq!(result, None); // suppressed by hold-down + // The hold-down veto now lives at the shell edge, not inside evaluate_parent. + // The clock-free core still finds peer_b as a discretionary candidate; the shell + // suppresses it because hold-down is active — same net "no switch" as before. + assert!(state.is_switch_suppressed(1000)); // hold-down active at the edge + assert!(matches!( + state.evaluate_parent(&costs, &BTreeSet::new()), + ParentEval::Discretionary(p) if p == peer_b + )); } #[test] @@ -889,8 +923,13 @@ fn test_mandatory_switch_bypasses_hold_down() { // Remove peer_a (parent lost) — should bypass hold-down state.remove_peer(&peer_a); - let result = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new(), 1000); - assert_eq!(result, Some(peer_b)); // mandatory switch + // Hold-down is active at the edge, but a parent-lost switch is mandatory and + // bypasses the veto: the core returns Mandatory and the switch is taken anyway. + assert!(state.is_switch_suppressed(1000)); // hold-down active + assert!(matches!( + state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new()), + ParentEval::Mandatory(p) if p == peer_b + )); } #[test] @@ -931,12 +970,16 @@ 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(&BTreeMap::new(), &BTreeSet::new(), 1000); + let result_no_cost = state + .evaluate_parent(&BTreeMap::new(), &BTreeSet::new()) + .switch_target(); 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, &BTreeSet::new(), 1000); + let result_with_cost = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result_with_cost, Some(peer1)); // Now test the critical case: node 5 currently has LoRa parent (peer 2). @@ -945,14 +988,18 @@ fn test_heterogeneous_7node_avoids_bottleneck() { state.recompute_coords(); assert_eq!(state.my_coords().depth(), 2); // depth 2 through LoRa peer - let result_switch = state.evaluate_parent(&costs, &BTreeSet::new(), 1000); + let result_switch = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); 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, &BTreeSet::new(), 1000); + let result_hyst = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result_hyst, Some(peer1)); } @@ -988,14 +1035,18 @@ 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, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&initial_costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer_a)); 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, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&initial_costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, None); // Peer A's link degrades significantly (LoRa-like latency + loss) @@ -1003,7 +1054,9 @@ 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, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(°raded_costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer_b)); } @@ -1036,7 +1089,9 @@ 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, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, None); } @@ -1058,7 +1113,9 @@ 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, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, Some(peer_a)); state.set_parent(peer_a, 1, 1000, 1000); @@ -1066,6 +1123,8 @@ fn test_single_peer_no_reeval_benefit() { // Even with terrible cost, no switch (no alternative) let bad_costs = make_costs(&[(1, 50.0)]); - let result = state.evaluate_parent(&bad_costs, &BTreeSet::new(), 1000); + let result = state + .evaluate_parent(&bad_costs, &BTreeSet::new()) + .switch_target(); assert_eq!(result, None); }