From b53db662c3f54ddd29708f1842fb5cfe9630ee5a Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 14:08:10 +0000 Subject: [PATCH 1/9] proto/bloom: add BloomFilter::fpr() and use it for the false-positive-rate The false-positive-rate primitive (fill ratio raised to the hash count) was inlined at three sites. Hoist it into a BloomFilter::fpr() method and call it from all three; the threshold/policy logic stays at the call sites. Behavior and result bytes are unchanged. --- src/node/bloom.rs | 4 ++-- src/proto/bloom/core.rs | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/node/bloom.rs b/src/node/bloom.rs index 17e67de..a86f729 100644 --- a/src/node/bloom.rs +++ b/src/node/bloom.rs @@ -87,7 +87,7 @@ impl Node { // operator to see one clear message, not spam. let max_fpr = self.config().node.bloom.max_inbound_fpr; let out_fill = sent_filter.fill_ratio(); - let out_fpr = out_fill.powi(sent_filter.hash_count() as i32); + let out_fpr = sent_filter.fpr(); if out_fpr > max_fpr { let now = std::time::Instant::now(); let should_warn = self @@ -213,7 +213,7 @@ impl Node { // to wipe a victim's contribution to aggregation. let max_fpr = self.config().node.bloom.max_inbound_fpr; let fill = announce.filter.fill_ratio(); - let fpr = fill.powi(announce.filter.hash_count() as i32); + let fpr = announce.filter.fpr(); if fpr > max_fpr { self.metrics() .bloom diff --git a/src/proto/bloom/core.rs b/src/proto/bloom/core.rs index 2876faf..6ef9600 100644 --- a/src/proto/bloom/core.rs +++ b/src/proto/bloom/core.rs @@ -139,6 +139,11 @@ impl BloomFilter { self.count_ones() as f64 / self.num_bits as f64 } + /// Current false-positive rate: fill ratio raised to the hash count. + pub fn fpr(&self) -> f64 { + self.fill_ratio().powi(self.hash_count as i32) + } + /// Estimate the number of elements in the filter. /// /// Uses the formula: n = -(m/k) * ln(1 - X/m) @@ -160,7 +165,7 @@ impl BloomFilter { } let fill = x / m; - let fpr = fill.powi(self.hash_count as i32); + let fpr = self.fpr(); if fpr > max_fpr { return None; } From c1ddbf053cafebdde4e930af64981c8cc2f5147f Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 19:00:24 +0000 Subject: [PATCH 2/9] proto/discovery: home the recent-request cap and inject request_id from the shell Two behavior-neutral discovery cleanups: - Move MAX_RECENT_DISCOVERY_REQUESTS out of the node discovery handler into the discovery subsystem limits module and re-export it, keeping the two use sites unchanged. Same value and semantics; only its home moves. - Remove the ambient rand draw from LookupRequest by deleting generate() and having the shell draw the random request_id and pass it into the existing new() constructor. Same per-request u64 draw, now at the shell, leaving the discovery codec free of ambient RNG reads. --- src/node/handlers/discovery.rs | 12 ++++++++---- src/node/tests/discovery.rs | 10 +++++----- src/proto/discovery/limits.rs | 8 ++++++++ src/proto/discovery/mod.rs | 4 +++- src/proto/discovery/tests/wire.rs | 5 +++-- src/proto/discovery/wire.rs | 13 ------------- 6 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/node/handlers/discovery.rs b/src/node/handlers/discovery.rs index 9a66477..3df02a6 100644 --- a/src/node/handlers/discovery.rs +++ b/src/node/handlers/discovery.rs @@ -7,13 +7,13 @@ use crate::node::Node; use crate::node::reject::DiscoveryReject; -use crate::proto::discovery::{DiscoveryAction, LookupRequest, LookupResponse}; +use crate::proto::discovery::{ + DiscoveryAction, LookupRequest, LookupResponse, MAX_RECENT_DISCOVERY_REQUESTS, +}; use crate::transport::{TransportAddr, TransportId}; use crate::{NodeAddr, PeerIdentity}; use tracing::{debug, info, trace, warn}; -const MAX_RECENT_DISCOVERY_REQUESTS: usize = 4096; - /// Shell adapter exposing the live routing tables to the sans-IO discovery /// core's `RoutingView` read seam. Lives in `node` so it can read `Node`'s /// private `peers` map and call the crate-private tree/bloom predicates. @@ -483,7 +483,11 @@ impl Node { let origin = *self.node_addr(); let origin_coords = self.tree_state().my_coords().clone(); - let request = LookupRequest::generate(*target, origin, origin_coords, ttl, 0); + let request_id = { + use rand::RngExt; + rand::rng().random() + }; + let request = LookupRequest::new(request_id, *target, origin, origin_coords, ttl, 0); // Tree-peer bloom-match selection + single encode live in the sans-IO // core. The core keeps the tree-only (no non-tree fallback) behavior; diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index 7d823c8..21eb6fd 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -1022,11 +1022,11 @@ async fn test_open_discovery_sweep_queues_eligible_skips_filtered() { /// (t=1100ms, 3100ms, 7100ms) and unreachable at t=15100ms. /// 2. **Fresh `initiate_lookup` per attempt** — `req_initiated` counter /// increments by exactly one on each retry. The actual `request_id` -/// is generated by `LookupRequest::generate(...)` via `rand::random()` -/// inside `initiate_lookup` and is not stored on the originator -/// side, so per-attempt freshness is verified indirectly: each -/// `req_initiated` increment corresponds to one fresh -/// `LookupRequest::generate` call. +/// is drawn via `rand::rng().random()` at the shell inside +/// `initiate_lookup` and passed to `LookupRequest::new(...)`; it is +/// not stored on the originator side, so per-attempt freshness is +/// verified indirectly: each `req_initiated` increment corresponds +/// to one fresh `initiate_lookup` call. /// 3. **Final-timeout state transitions** — `pending_lookups` entry is /// removed, `discovery.resp_timed_out` counter ticks, queued packet /// is drained, and an ICMPv6 Destination Unreachable frame is diff --git a/src/proto/discovery/limits.rs b/src/proto/discovery/limits.rs index aed6cf6..c3e9dc6 100644 --- a/src/proto/discovery/limits.rs +++ b/src/proto/discovery/limits.rs @@ -16,6 +16,14 @@ use crate::NodeAddr; use alloc::collections::BTreeMap; +// ============================================================================ +// Receive-side: Request dedup cache bound +// ============================================================================ + +/// Maximum number of recent LookupRequests retained for dedup and +/// reverse-path routing before the cache is treated as full. +pub(crate) const MAX_RECENT_DISCOVERY_REQUESTS: usize = 4096; + // ============================================================================ // Originator-side: Discovery Backoff // ============================================================================ diff --git a/src/proto/discovery/mod.rs b/src/proto/discovery/mod.rs index f3c68cd..883b85e 100644 --- a/src/proto/discovery/mod.rs +++ b/src/proto/discovery/mod.rs @@ -26,7 +26,9 @@ pub(crate) use core::{ initiate_gate, on_response_accepted, plan_forward, plan_initiate, plan_response_route, poll_pending, }; -pub(crate) use limits::{DiscoveryBackoff, DiscoveryForwardRateLimiter}; +pub(crate) use limits::{ + DiscoveryBackoff, DiscoveryForwardRateLimiter, MAX_RECENT_DISCOVERY_REQUESTS, +}; #[cfg(test)] pub(crate) use state::RecentRequest; pub(crate) use state::{Discovery, PendingLookup}; diff --git a/src/proto/discovery/tests/wire.rs b/src/proto/discovery/tests/wire.rs index 12e0092..29f5dc1 100644 --- a/src/proto/discovery/tests/wire.rs +++ b/src/proto/discovery/tests/wire.rs @@ -36,8 +36,9 @@ fn test_lookup_request_generate() { let origin = make_node_addr(2); let coords = make_coords(&[2, 0]); - let req1 = LookupRequest::generate(target, origin, coords.clone(), 5, 0); - let req2 = LookupRequest::generate(target, origin, coords, 5, 0); + use rand::RngExt; + let req1 = LookupRequest::new(rand::rng().random(), target, origin, coords.clone(), 5, 0); + let req2 = LookupRequest::new(rand::rng().random(), target, origin, coords, 5, 0); // Random IDs should differ assert_ne!(req1.request_id, req2.request_id); diff --git a/src/proto/discovery/wire.rs b/src/proto/discovery/wire.rs index de7a80a..e61806c 100644 --- a/src/proto/discovery/wire.rs +++ b/src/proto/discovery/wire.rs @@ -48,19 +48,6 @@ impl LookupRequest { } } - /// Generate a new request with a random ID. - pub fn generate( - target: NodeAddr, - origin: NodeAddr, - origin_coords: TreeCoordinate, - ttl: u8, - min_mtu: u16, - ) -> Self { - use rand::RngExt; - let request_id = rand::rng().random(); - Self::new(request_id, target, origin, origin_coords, ttl, min_mtu) - } - /// Decrement TTL for forwarding. /// /// Returns false if TTL was already 0. From 309a91d293dca5c39dcac5c24256283e05fb7fb4 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 19:00:24 +0000 Subject: [PATCH 3/9] proto/mmp: split state into per-role modules and dissolve the vestigial mmp shell Reorganize the MMP subsystem, behavior unchanged throughout: - Restore the pre-migration role split: move SenderState into sender.rs, ReceiverState (with its GapTracker helper) into receiver.rs, MmpMetrics and RrLog into metrics.rs, and PathMtuState into path_mtu.rs, leaving the Mmp aggregate plus the peer/session state in state.rs (1339 to 196 lines). Home the module constants in a new limits.rs and re-export them at the same paths. Pure code-motion with module rewiring; visibility unchanged. - Dissolve the 97-line src/mmp shell, which held only MmpConfig and the monotonic mono_ms() clock: move MmpConfig into config/node.rs (re-exported as crate::config::MmpConfig), move mono_ms() into a new top-level src/time.rs shell time seam, repoint all callers, and delete src/mmp/. Also correct stale src/mmp/ doc-comment path labels to proto/mmp/ where they name the protocol primitives. --- src/config/mod.rs | 4 +- src/config/node.rs | 66 +- src/lib.rs | 2 +- src/mmp/mod.rs | 97 --- src/node/handlers/encrypted.rs | 4 +- src/node/handlers/mmp.rs | 8 +- src/node/handlers/session.rs | 10 +- src/node/session.rs | 2 +- src/node/tests/mmp_chartests.rs | 24 +- src/node/tests/session.rs | 2 +- src/node/tree.rs | 6 +- src/peer/active.rs | 6 +- src/proto/mmp/core.rs | 10 +- src/proto/mmp/limits.rs | 55 ++ src/proto/mmp/metrics.rs | 333 +++++++++ src/proto/mmp/mod.rs | 73 +- src/proto/mmp/path_mtu.rs | 175 +++++ src/proto/mmp/receiver.rs | 447 ++++++++++++ src/proto/mmp/sender.rs | 213 ++++++ src/proto/mmp/state.rs | 1169 +------------------------------ src/proto/mmp/tests/state.rs | 3 +- src/proto/stp/limits.rs | 2 +- src/time.rs | 16 + 23 files changed, 1380 insertions(+), 1347 deletions(-) delete mode 100644 src/mmp/mod.rs create mode 100644 src/proto/mmp/limits.rs create mode 100644 src/proto/mmp/metrics.rs create mode 100644 src/proto/mmp/path_mtu.rs create mode 100644 src/proto/mmp/receiver.rs create mode 100644 src/proto/mmp/sender.rs create mode 100644 src/time.rs diff --git a/src/config/mod.rs b/src/config/mod.rs index 0097a15..211254f 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -34,8 +34,8 @@ use thiserror::Error; pub use gateway::{ConntrackConfig, GatewayConfig, GatewayDnsConfig, PortForward, Proto}; pub use node::{ BloomConfig, BuffersConfig, CacheConfig, ControlConfig, DiscoveryConfig, LimitsConfig, - NodeConfig, NostrDiscoveryConfig, NostrDiscoveryPolicy, RateLimitConfig, RekeyConfig, - RetryConfig, SessionConfig, SessionMmpConfig, TreeConfig, + MmpConfig, NodeConfig, NostrDiscoveryConfig, NostrDiscoveryPolicy, RateLimitConfig, + RekeyConfig, RetryConfig, SessionConfig, SessionMmpConfig, TreeConfig, }; pub use peer::{ConnectPolicy, PeerAddress, PeerConfig}; pub use transport::{ diff --git a/src/config/node.rs b/src/config/node.rs index ae50541..613ff7d 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -7,7 +7,6 @@ use serde::{Deserialize, Serialize}; use super::IdentityConfig; -use crate::mmp::MmpConfig; use crate::proto::mmp::{DEFAULT_LOG_INTERVAL_SECS, DEFAULT_OWD_WINDOW_SIZE, MmpMode}; // ============================================================================ @@ -727,6 +726,41 @@ impl SessionConfig { } } +/// MMP configuration (`node.mmp.*`). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MmpConfig { + /// Operating mode (`node.mmp.mode`). + #[serde(default)] + pub mode: MmpMode, + + /// Periodic operator log interval in seconds (`node.mmp.log_interval_secs`). + #[serde(default = "MmpConfig::default_log_interval_secs")] + pub log_interval_secs: u64, + + /// OWD trend ring buffer size (`node.mmp.owd_window_size`). + #[serde(default = "MmpConfig::default_owd_window_size")] + pub owd_window_size: usize, +} + +impl Default for MmpConfig { + fn default() -> Self { + Self { + mode: MmpMode::default(), + log_interval_secs: DEFAULT_LOG_INTERVAL_SECS, + owd_window_size: DEFAULT_OWD_WINDOW_SIZE, + } + } +} + +impl MmpConfig { + fn default_log_interval_secs() -> u64 { + DEFAULT_LOG_INTERVAL_SECS + } + fn default_owd_window_size() -> usize { + DEFAULT_OWD_WINDOW_SIZE + } +} + /// Session-layer Metrics Measurement Protocol (`node.session_mmp.*`). /// /// Separate from link-layer `node.mmp.*` to allow independent mode/interval @@ -1096,6 +1130,36 @@ impl NodeConfig { mod tests { use super::*; + #[test] + fn test_config_default() { + let config = MmpConfig::default(); + assert_eq!(config.mode, MmpMode::Full); + assert_eq!(config.log_interval_secs, 30); + assert_eq!(config.owd_window_size, 32); + } + + #[test] + fn test_config_yaml_parse() { + let yaml = r#" +mode: lightweight +log_interval_secs: 60 +owd_window_size: 48 +"#; + let config: MmpConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.mode, MmpMode::Lightweight); + assert_eq!(config.log_interval_secs, 60); + assert_eq!(config.owd_window_size, 48); + } + + #[test] + fn test_config_yaml_partial() { + let yaml = "mode: minimal"; + let config: MmpConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.mode, MmpMode::Minimal); + assert_eq!(config.log_interval_secs, DEFAULT_LOG_INTERVAL_SECS); + assert_eq!(config.owd_window_size, DEFAULT_OWD_WINDOW_SIZE); + } + #[test] fn test_ecn_config_defaults() { let c = EcnConfig::default(); diff --git a/src/lib.rs b/src/lib.rs index 9c1ea15..af83669 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,7 +16,6 @@ pub mod discovery; #[cfg(target_os = "linux")] pub mod gateway; pub mod identity; -pub mod mmp; pub mod node; pub mod noise; pub mod peer; @@ -24,6 +23,7 @@ pub mod perf_profile; pub(crate) mod proto; #[cfg(test)] pub(crate) mod testutil; +mod time; pub mod transport; pub mod upper; pub mod utils; diff --git a/src/mmp/mod.rs b/src/mmp/mod.rs deleted file mode 100644 index 5a729bb..0000000 --- a/src/mmp/mod.rs +++ /dev/null @@ -1,97 +0,0 @@ -//! Metrics Measurement Protocol (MMP) — node-side shell. -//! -//! The MMP protocol core (estimators, sender/receiver/metrics/path-MTU state -//! machines, report codecs and the reporting decisions) lives in the sans-IO -//! [`crate::proto::mmp`] module. What remains here is the shell-only surface: -//! the serde node configuration ([`MmpConfig`]) and the monotonic millisecond -//! clock ([`mono_ms`]) the shell reads at the edge and injects into the owned -//! `u64`-ms state. - -use std::sync::LazyLock; -use std::time::Instant; - -use serde::{Deserialize, Serialize}; - -use crate::proto::mmp::{DEFAULT_LOG_INTERVAL_SECS, DEFAULT_OWD_WINDOW_SIZE, MmpMode}; - -/// Monotonic millisecond clock for the MMP time seam. -/// -/// The owned `proto::mmp` state takes injected `u64` milliseconds rather than -/// reading a clock. The shell reads this process-monotonic clock at each edge -/// and passes the value in; only deltas between two `mono_ms()` reads are ever -/// compared, so the (process-relative) epoch is immaterial. -pub(crate) fn mono_ms() -> u64 { - static START: LazyLock = LazyLock::new(Instant::now); - START.elapsed().as_millis() as u64 -} - -/// MMP configuration (`node.mmp.*`). -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct MmpConfig { - /// Operating mode (`node.mmp.mode`). - #[serde(default)] - pub mode: MmpMode, - - /// Periodic operator log interval in seconds (`node.mmp.log_interval_secs`). - #[serde(default = "MmpConfig::default_log_interval_secs")] - pub log_interval_secs: u64, - - /// OWD trend ring buffer size (`node.mmp.owd_window_size`). - #[serde(default = "MmpConfig::default_owd_window_size")] - pub owd_window_size: usize, -} - -impl Default for MmpConfig { - fn default() -> Self { - Self { - mode: MmpMode::default(), - log_interval_secs: DEFAULT_LOG_INTERVAL_SECS, - owd_window_size: DEFAULT_OWD_WINDOW_SIZE, - } - } -} - -impl MmpConfig { - fn default_log_interval_secs() -> u64 { - DEFAULT_LOG_INTERVAL_SECS - } - fn default_owd_window_size() -> usize { - DEFAULT_OWD_WINDOW_SIZE - } -} - -#[cfg(test)] -mod tests { - use super::MmpConfig; - use crate::proto::mmp::{DEFAULT_LOG_INTERVAL_SECS, DEFAULT_OWD_WINDOW_SIZE, MmpMode}; - - #[test] - fn test_config_default() { - let config = MmpConfig::default(); - assert_eq!(config.mode, MmpMode::Full); - assert_eq!(config.log_interval_secs, 30); - assert_eq!(config.owd_window_size, 32); - } - - #[test] - fn test_config_yaml_parse() { - let yaml = r#" -mode: lightweight -log_interval_secs: 60 -owd_window_size: 48 -"#; - let config: MmpConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.mode, MmpMode::Lightweight); - assert_eq!(config.log_interval_secs, 60); - assert_eq!(config.owd_window_size, 48); - } - - #[test] - fn test_config_yaml_partial() { - let yaml = "mode: minimal"; - let config: MmpConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.mode, MmpMode::Minimal); - assert_eq!(config.log_interval_secs, DEFAULT_LOG_INTERVAL_SECS); - assert_eq!(config.owd_window_size, DEFAULT_OWD_WINDOW_SIZE); - } -} diff --git a/src/node/handlers/encrypted.rs b/src/node/handlers/encrypted.rs index 3df8a43..7401f9b 100644 --- a/src/node/handlers/encrypted.rs +++ b/src/node/handlers/encrypted.rs @@ -258,7 +258,7 @@ impl Node { }; // MMP per-frame processing and statistics - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); let ce_flag = header.flags & FLAG_CE != 0; let sp_flag = header.flags & FLAG_SP != 0; @@ -354,7 +354,7 @@ impl Node { } else { return; }; - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); let mut address_changed = false; if let Some(peer) = self.peers.get_mut(node_addr) { peer.reset_decrypt_failures(); diff --git a/src/node/handlers/mmp.rs b/src/node/handlers/mmp.rs index 9362c6c..39ea9c7 100644 --- a/src/node/handlers/mmp.rs +++ b/src/node/handlers/mmp.rs @@ -152,7 +152,7 @@ impl Node { // Process the report: computes RTT from timestamp echo, updates // loss rate, goodput rate, jitter trend, and ETX. - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); let (first_rtt, rr_log) = mmp.metrics .process_receiver_report(&rr, our_timestamp_ms, now_ms); @@ -197,7 +197,7 @@ impl Node { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) .unwrap_or(0); - let mono_now_ms = crate::mmp::mono_ms(); + let mono_now_ms = crate::time::mono_ms(); if let Some(new_parent) = self.tree_state.evaluate_parent( &peer_costs, &std::collections::BTreeSet::new(), @@ -277,7 +277,7 @@ impl Node { /// /// Called from the tick handler. Also emits periodic operator logs. pub(in crate::node) async fn check_mmp_reports(&mut self) { - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); // Build one report-gating snapshot per peer, resolving every timing read // shell-side into a `bool`. `send_sr`/`send_rr` are `true` on the master @@ -427,7 +427,7 @@ impl Node { // Monotonic ms for the MMP receiver's injected-`u64` liveness clock; the // Instant `now` is still used for the shell-owned heartbeat timing and // the session-start fallback (both `ActivePeer` Instants). - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); let heartbeat_interval = Duration::from_secs(self.config().node.heartbeat_interval_secs); let dead_timeout = Duration::from_secs(self.config().node.link_dead_timeout_secs); let dead_timeout_ms = dead_timeout.as_millis() as u64; diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 952f5ef..3abc8e3 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -324,7 +324,7 @@ impl Node { if let Some(entry) = self.sessions.get_mut(src_addr) && let Some(mmp) = entry.mmp_mut() { - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); mmp.receiver .record_recv(header.counter, timestamp, plaintext.len(), ce_flag, now_ms); // Spin bit: advance state machine for correct TX reflection. @@ -946,7 +946,7 @@ impl Node { /// Called from the tick handler. Also emits periodic session MMP logs. /// Uses the collect-then-send pattern to avoid borrowing conflicts. pub(in crate::node) async fn check_session_mmp_reports(&mut self) { - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); // Build one report-gating snapshot per session, resolving every timing // read shell-side into a `bool`. The snapshots own only @@ -1213,7 +1213,7 @@ impl Node { let (_first_rtt, rr_log) = mmp.metrics - .process_receiver_report(&rr, our_timestamp_ms, crate::mmp::mono_ms()); + .process_receiver_report(&rr, our_timestamp_ms, crate::time::mono_ms()); // Re-emit the operator trace the core used to log mid-decision. super::mmp::log_rr_outcome(&rr, our_timestamp_ms, rr_log); @@ -1283,7 +1283,7 @@ impl Node { let old_mtu = mmp.path_mtu.current_mtu(); let changed = mmp .path_mtu - .apply_notification(notif.path_mtu, crate::mmp::mono_ms()); + .apply_notification(notif.path_mtu, crate::time::mono_ms()); let new_mtu = mmp.path_mtu.current_mtu(); if !changed { @@ -1521,7 +1521,7 @@ impl Node { let old_mtu = mmp.path_mtu.current_mtu(); if mmp .path_mtu - .apply_notification(msg.mtu, crate::mmp::mono_ms()) + .apply_notification(msg.mtu, crate::time::mono_ms()) { let new_mtu = mmp.path_mtu.current_mtu(); info!( diff --git a/src/node/session.rs b/src/node/session.rs index 694012e..18bcde2 100644 --- a/src/node/session.rs +++ b/src/node/session.rs @@ -668,7 +668,7 @@ impl SessionEntry { self.rekey_jitter_secs = draw_rekey_jitter(); // Reset MMP counters to avoid metric discontinuity - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); if let Some(mmp) = &mut self.mmp { mmp.reset_for_rekey(now_ms); } diff --git a/src/node/tests/mmp_chartests.rs b/src/node/tests/mmp_chartests.rs index f3e39d6..efd8db8 100644 --- a/src/node/tests/mmp_chartests.rs +++ b/src/node/tests/mmp_chartests.rs @@ -12,12 +12,12 @@ //! //! Assertions capture what the code does today, surprising or not. //! -//! Report-generation is probed through the reused `src/mmp/` primitives +//! Report-generation is probed through the reused `proto/mmp/` primitives //! (`should_send_report` / `should_send_notification`): after a handler tick, //! a *consumed* interval reads as "not due" (the report was built) while an //! *ungated* interval still reads as "due" (the report was suppressed by the //! mode/flag gate). This survives the later refactor because those primitives -//! stay in `src/mmp/` unchanged. +//! stay in `proto/mmp/` unchanged. //! //! Two `#[cfg(test)]` production seams are used, both on `ActivePeer`: //! * `test_init_mmp(mode)` — attach link MMP with a chosen mode to a @@ -55,7 +55,7 @@ fn arm_link_mmp(node: &mut Node, addr: &NodeAddr) { let mmp = node.get_peer_mut(addr).unwrap().mmp_mut().unwrap(); mmp.sender.record_sent(1, 100, 500); mmp.receiver - .record_recv(1, 100, 500, false, crate::mmp::mono_ms()); + .record_recv(1, 100, 500, false, crate::time::mono_ms()); } /// Complete an in-memory Noise IK handshake, returning the initiator session. @@ -109,7 +109,7 @@ fn arm_session_mmp(node: &mut Node, addr: &NodeAddr) { let mmp = node.sessions.get_mut(addr).unwrap().mmp_mut().unwrap(); mmp.sender.record_sent(1, 100, 500); mmp.receiver - .record_recv(1, 100, 500, false, crate::mmp::mono_ms()); + .record_recv(1, 100, 500, false, crate::time::mono_ms()); } // =========================================================================== @@ -127,7 +127,7 @@ async fn mmp_full_mode_builds_sender_and_receiver_reports() { node.check_mmp_reports().await; let mmp = node.get_peer(&addr).unwrap().mmp().unwrap(); - let now = crate::mmp::mono_ms(); + let now = crate::time::mono_ms(); assert!( !mmp.sender.should_send_report(now), "Full mode consumes the sender interval (SenderReport built)" @@ -149,7 +149,7 @@ async fn mmp_lightweight_mode_builds_receiver_report_only() { node.check_mmp_reports().await; let mmp = node.get_peer(&addr).unwrap().mmp().unwrap(); - let now = crate::mmp::mono_ms(); + let now = crate::time::mono_ms(); assert!( mmp.sender.should_send_report(now), "Lightweight mode suppresses the SenderReport (sender interval intact)" @@ -170,7 +170,7 @@ async fn mmp_minimal_mode_builds_nothing() { node.check_mmp_reports().await; let mmp = node.get_peer(&addr).unwrap().mmp().unwrap(); - let now = crate::mmp::mono_ms(); + let now = crate::time::mono_ms(); assert!( mmp.sender.should_send_report(now), "Minimal mode suppresses the SenderReport" @@ -194,7 +194,7 @@ async fn mmp_should_log_marks_logged_once_per_interval() { .unwrap() .mmp() .unwrap() - .should_log(crate::mmp::mono_ms()), + .should_log(crate::time::mono_ms()), "a freshly created peer is due for its first operator log" ); @@ -206,7 +206,7 @@ async fn mmp_should_log_marks_logged_once_per_interval() { .unwrap() .mmp() .unwrap() - .should_log(crate::mmp::mono_ms()), + .should_log(crate::time::mono_ms()), "after one tick the log is marked and not due again within the interval" ); } @@ -226,7 +226,7 @@ async fn session_full_mode_builds_sender_and_receiver_reports() { node.check_session_mmp_reports().await; let mmp = node.get_session(&addr).unwrap().mmp().unwrap(); - let now = crate::mmp::mono_ms(); + let now = crate::time::mono_ms(); assert!( !mmp.sender.should_send_report(now), "Full session consumes the sender interval" @@ -254,7 +254,7 @@ async fn session_minimal_mode_still_sends_path_mtu() { .path_mtu .observe_incoming_mtu(1200); - let now_before = crate::mmp::mono_ms(); + let now_before = crate::time::mono_ms(); assert!( node.get_session(&addr) .unwrap() @@ -268,7 +268,7 @@ async fn session_minimal_mode_still_sends_path_mtu() { node.check_session_mmp_reports().await; let mmp = node.get_session(&addr).unwrap().mmp().unwrap(); - let now = crate::mmp::mono_ms(); + let now = crate::time::mono_ms(); assert!( mmp.sender.should_send_report(now), "Minimal mode suppresses the session SenderReport" diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 18dc267..b45297c 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -1912,7 +1912,7 @@ async fn test_tun_outbound_path_mtu_generates_ptb() { let entry = nodes[0].node.get_session_mut(&node1_addr).unwrap(); let mmp = entry.mmp_mut().unwrap(); mmp.path_mtu - .apply_notification(reduced_mtu, crate::mmp::mono_ms()); + .apply_notification(reduced_mtu, crate::time::mono_ms()); assert_eq!(mmp.path_mtu.current_mtu(), reduced_mtu); } diff --git a/src/node/tree.rs b/src/node/tree.rs index d595584..d7e7fc3 100644 --- a/src/node/tree.rs +++ b/src/node/tree.rs @@ -328,7 +328,7 @@ impl Node { // 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(); + let mono_now_ms = crate::time::mono_ms(); match Stp::classify_announce(&self.tree_state, *from, &peer_costs, &skip, mono_now_ms) { TreeDecision::Switch { @@ -582,7 +582,7 @@ 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::mmp::mono_ms(); + let mono_now_ms = crate::time::mono_ms(); match Stp::classify_periodic(&self.tree_state, &peer_costs, &skip, mono_now_ms) { TreeDecision::Switch { @@ -735,7 +735,7 @@ impl Node { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) .unwrap_or(0); - let mono_now_ms = crate::mmp::mono_ms(); + let mono_now_ms = crate::time::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 diff --git a/src/peer/active.rs b/src/peer/active.rs index 9c9aea4..c88e95a 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -3,7 +3,7 @@ //! Represents a fully authenticated peer after successful Noise handshake. //! ActivePeer holds tree state, Bloom filter, and routing information. -use crate::mmp::MmpConfig; +use crate::config::MmpConfig; use crate::node::REKEY_JITTER_SECS; use crate::noise::{HandshakeState as NoiseHandshakeState, NoiseError, NoiseSession}; use crate::proto::bloom::BloomFilter; @@ -1056,7 +1056,7 @@ impl ActivePeer { self.reset_replay_suppressed(); // Reset MMP counters to avoid metric discontinuity - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); if let Some(mmp) = &mut self.mmp { mmp.reset_for_rekey(now_ms); } @@ -1093,7 +1093,7 @@ impl ActivePeer { self.reset_replay_suppressed(); // Reset MMP counters to avoid metric discontinuity - let now_ms = crate::mmp::mono_ms(); + let now_ms = crate::time::mono_ms(); if let Some(mmp) = &mut self.mmp { mmp.reset_for_rekey(now_ms); } diff --git a/src/proto/mmp/core.rs b/src/proto/mmp/core.rs index d38398a..4d832bb 100644 --- a/src/proto/mmp/core.rs +++ b/src/proto/mmp/core.rs @@ -6,7 +6,7 @@ //! (pre-computing every clock read into plain `bool` snapshot fields, and //! resolving the rekey-suppression predicate shell-side), call the `plan_*` //! decisions, and drive the returned [`MmpAction`]s — the actual sends, -//! registry mutations, `src/mmp/` primitive calls, metrics, and logging. No +//! registry mutations, `proto/mmp/` primitive calls, metrics, and logging. No //! I/O, no clock, no metrics, no logging here. //! //! Unlike routing/discovery (whose cores do live cross-table reads through a @@ -50,7 +50,7 @@ pub(crate) struct PeerLivenessSnapshot { /// /// Every timing read is resolved shell-side into a plain `bool`: `sr_due` / /// `rr_due` are the pre-evaluated `should_send_report` gates on the peer's -/// `src/mmp/` sender/receiver primitives, and `log_due` is the pre-evaluated +/// `proto/mmp/` sender/receiver primitives, and `log_due` is the pre-evaluated /// `should_log` gate. `send_sr`/`send_rr` are the profile provide/want flags: /// on the master (IK) line there is no profile negotiation, so the shell sets /// them to the behavior-neutral constant `true` (ANDing a runtime `true` is a @@ -81,7 +81,7 @@ pub(crate) struct LinkReportSnapshot { /// /// Every timing read is resolved shell-side into a plain `bool`: `sr_due` / /// `rr_due` are the pre-evaluated `should_send_report` gates on the session's -/// `src/mmp/` sender/receiver primitives, `mtu_due` is the pre-evaluated +/// `proto/mmp/` sender/receiver primitives, `mtu_due` is the pre-evaluated /// `path_mtu.should_send_notification` gate, and `log_due` is the pre-evaluated /// `should_log` gate. Unlike the link path there is **no** `send_sr`/`send_rr` /// profile flag — the session handler has never gated on a profile, so no @@ -172,7 +172,7 @@ pub(crate) enum MmpAction { /// Send a heartbeat to `peer`: the shell runs `mark_heartbeat_sent` and the /// encrypted link send. Heartbeat { peer: NodeAddr }, - /// Build (shell: `src/mmp/` `build_report` + `encode`) and send the given + /// Build (shell: `proto/mmp/` `build_report` + `encode`) and send the given /// link report over the encrypted link. The interval-advancing /// `build_report` mutation happens **only** while driving this action, so an /// ungated report never advances its interval. @@ -183,7 +183,7 @@ pub(crate) enum MmpAction { /// Emit the periodic link operator log for `peer` (shell owns the `tracing` /// call and runs `mark_logged`). LogLink { peer: NodeAddr }, - /// Build (shell: `src/mmp/` `build_report`/`build_notification` + the + /// Build (shell: `proto/mmp/` `build_report`/`build_notification` + the /// `Session*`/`PathMtuNotification` codec) and send the given session report /// over the encrypted session. The interval/notification-advancing build /// mutation happens **only** while driving this action. diff --git a/src/proto/mmp/limits.rs b/src/proto/mmp/limits.rs new file mode 100644 index 0000000..0ada036 --- /dev/null +++ b/src/proto/mmp/limits.rs @@ -0,0 +1,55 @@ +//! MMP tuning constants (EWMA parameters, report-interval bounds, defaults). +//! +//! The plain values the owned sender/receiver/metrics state machines and the +//! shell config clamp against. Re-exported from the module root so external +//! callers keep the `crate::proto::mmp::` path. + +// --- EWMA parameters --- + +/// Dual EWMA short-term: α = 1/4. +pub const EWMA_SHORT_ALPHA: f64 = 0.25; + +/// Dual EWMA long-term: α = 1/32. +pub const EWMA_LONG_ALPHA: f64 = 1.0 / 32.0; + +// --- Timing defaults (milliseconds) --- + +/// Default report interval before SRTT is available (cold start). +pub const DEFAULT_COLD_START_INTERVAL_MS: u64 = 200; + +/// Minimum report interval (SRTT clamp floor). +/// +/// Raised from 100ms to 1000ms: parent re-evaluation runs every 60s, +/// so 60 samples/cycle is more than sufficient for EWMA convergence (~10). +/// The cold-start phase uses `DEFAULT_COLD_START_INTERVAL_MS` (200ms) for +/// fast initial SRTT convergence before transitioning to this floor. +pub const MIN_REPORT_INTERVAL_MS: u64 = 1_000; + +/// Maximum report interval (SRTT clamp ceiling). +pub const MAX_REPORT_INTERVAL_MS: u64 = 5_000; + +/// Number of SRTT samples before transitioning from cold-start to normal floor. +/// +/// During cold-start, report intervals use `DEFAULT_COLD_START_INTERVAL_MS` as +/// the floor to gather SRTT samples quickly. After this many updates, the floor +/// switches to `MIN_REPORT_INTERVAL_MS`. +pub const COLD_START_SAMPLES: u32 = 5; + +/// Default OWD ring buffer capacity. +pub const DEFAULT_OWD_WINDOW_SIZE: usize = 32; + +/// Default operator log interval in seconds. +pub const DEFAULT_LOG_INTERVAL_SECS: u64 = 30; + +// --- Session-layer timing defaults --- +// Session reports are routed end-to-end (bandwidth cost on every transit link), +// so intervals are higher than link-layer. + +/// Session-layer minimum report interval. +pub const MIN_SESSION_REPORT_INTERVAL_MS: u64 = 500; + +/// Session-layer maximum report interval. +pub const MAX_SESSION_REPORT_INTERVAL_MS: u64 = 10_000; + +/// Session-layer cold-start report interval (before SRTT is available). +pub const SESSION_COLD_START_INTERVAL_MS: u64 = 1_000; diff --git a/src/proto/mmp/metrics.rs b/src/proto/mmp/metrics.rs new file mode 100644 index 0000000..6663f48 --- /dev/null +++ b/src/proto/mmp/metrics.rs @@ -0,0 +1,333 @@ +//! Derived MMP metrics (sans-IO). +//! +//! Sender-side derived metrics (RTT, loss, goodput, ETX, trends) computed from +//! incoming `ReceiverReport`s, plus the [`RrLog`] observability outcome returned +//! to the shell so the operator `trace!` points can fire there rather than +//! mid-decision. All time inputs are injected `u64` milliseconds. + +use super::algorithms::{DualEwma, SrttEstimator, compute_etx}; +use super::wire::ReceiverReport; + +// ============================================================================ +// ReceiverReport processing outcome (observability hoisted to the shell) +// ============================================================================ + +/// The observability outcome of [`MmpMetrics::process_receiver_report`], returned +/// so the async shell can emit the operator `trace!` points that used to live +/// mid-decision (per the sans-IO rule that migrated code carries no `tracing`). +/// +/// Exactly one variant applies per call; the shell has the incoming report and +/// our timestamp, so this only carries the values the shell cannot otherwise +/// reconstruct (the previous cumulative snapshot / the derived RTT). +pub enum RrLog { + /// No RTT log point fires (the report carried no timestamp echo). + None, + /// The report was ignored as stale/duplicate. Carries the previous + /// cumulative snapshot for the operator trace. + Stale { + prev_highest: u64, + prev_packets: u64, + prev_bytes: u64, + }, + /// A valid RTT sample was taken; carries the derived RTT and the SRTT as it + /// stood **before** this sample was folded in (matching the original log, + /// which read `srtt` before `update`). + RttSample { rtt_ms: u32, srtt_ms: f64 }, + /// The report carried an echo but the derived RTT was invalid (wrapped / + /// non-positive). + InvalidRtt, +} + +// ============================================================================ +// MmpMetrics (derived metrics) +// ============================================================================ + +/// Derived MMP metrics, updated from incoming ReceiverReports. +/// +/// This lives on the sender side: when we receive a ReceiverReport from +/// our peer describing what they observed about our traffic, we process +/// it here to compute RTT, loss, goodput, and trend indicators. +pub struct MmpMetrics { + /// Smoothed RTT from timestamp echo. + pub srtt: SrttEstimator, + + /// Dual EWMA trend detectors. + pub rtt_trend: DualEwma, + pub loss_trend: DualEwma, + pub goodput_trend: DualEwma, + pub jitter_trend: DualEwma, + pub etx_trend: DualEwma, + + /// Forward delivery ratio (what fraction of our frames the peer received). + pub delivery_ratio_forward: f64, + /// Reverse delivery ratio (set when we compute from our own receiver state). + pub delivery_ratio_reverse: f64, + /// ETX computed from bidirectional delivery ratios. + pub etx: f64, + + /// Smoothed goodput in bytes/sec (forward direction: what the peer received from us). + pub goodput_bps: f64, + + // --- State for delta computation --- + /// Previous ReceiverReport's cumulative counters (for computing interval deltas). + prev_rr_cum_packets: u64, + prev_rr_cum_bytes: u64, + prev_rr_highest_counter: u64, + prev_rr_ecn_ce: u32, + prev_rr_reorder: u32, + /// Time (injected `u64` ms) of previous ReceiverReport (for goodput rate). + prev_rr_ms: Option, + /// Whether we have a previous ReceiverReport for delta computation. + has_prev_rr: bool, + + // --- State for reverse delivery ratio delta computation --- + /// Previous reverse-side cumulative packets received (our receiver state). + prev_reverse_packets: u64, + /// Previous reverse-side highest counter (our receiver state). + prev_reverse_highest: u64, + /// Whether we have a previous reverse-side snapshot for delta computation. + has_prev_reverse: bool, +} + +impl MmpMetrics { + /// Reset state derived from ReceiverReport counters for rekey cutover. + /// + /// The new session starts with counter 0, so the prev_rr deltas must + /// be reset to avoid computing bogus loss/goodput from the counter + /// discontinuity. RTT (SRTT) is preserved since it remains valid. + pub fn reset_for_rekey(&mut self) { + self.prev_rr_cum_packets = 0; + self.prev_rr_cum_bytes = 0; + self.prev_rr_highest_counter = 0; + self.prev_rr_ecn_ce = 0; + self.prev_rr_reorder = 0; + self.prev_rr_ms = None; + self.has_prev_rr = false; + self.delivery_ratio_forward = 1.0; + self.prev_reverse_packets = 0; + self.prev_reverse_highest = 0; + self.has_prev_reverse = false; + // Keep srtt, etx, trends, goodput_bps — they'll refresh from data + } + + pub fn new() -> Self { + Self { + srtt: SrttEstimator::new(), + rtt_trend: DualEwma::new(), + loss_trend: DualEwma::new(), + goodput_trend: DualEwma::new(), + jitter_trend: DualEwma::new(), + etx_trend: DualEwma::new(), + delivery_ratio_forward: 1.0, + delivery_ratio_reverse: 1.0, + etx: 1.0, + goodput_bps: 0.0, + prev_rr_cum_packets: 0, + prev_rr_cum_bytes: 0, + prev_rr_highest_counter: 0, + prev_rr_ecn_ce: 0, + prev_rr_reorder: 0, + prev_rr_ms: None, + has_prev_rr: false, + prev_reverse_packets: 0, + prev_reverse_highest: 0, + has_prev_reverse: false, + } + } + + /// Process an incoming ReceiverReport (from the peer about our traffic). + /// + /// `our_timestamp_ms` is the current session-relative time in ms (for RTT). + /// `now_ms` is the injected monotonic time in ms (for goodput rate). + /// + /// Returns `(first_srtt, log)`: `first_srtt` is `true` if this report + /// produced the first SRTT measurement (transition from uninitialized to + /// initialized); `log` is the [`RrLog`] observability outcome the shell + /// emits (the `tracing` that used to fire here now lives shell-side). + pub fn process_receiver_report( + &mut self, + rr: &ReceiverReport, + our_timestamp_ms: u32, + now_ms: u64, + ) -> (bool, RrLog) { + let had_srtt = self.srtt.initialized(); + + if self.has_prev_rr { + let counters_regressed = rr.highest_counter < self.prev_rr_highest_counter + || rr.cumulative_packets_recv < self.prev_rr_cum_packets + || rr.cumulative_bytes_recv < self.prev_rr_cum_bytes + || rr.ecn_ce_count < self.prev_rr_ecn_ce + || rr.cumulative_reorder_count < self.prev_rr_reorder; + let duplicate_counters = rr.highest_counter == self.prev_rr_highest_counter + && rr.cumulative_packets_recv == self.prev_rr_cum_packets + && rr.cumulative_bytes_recv == self.prev_rr_cum_bytes + && rr.ecn_ce_count == self.prev_rr_ecn_ce + && rr.cumulative_reorder_count == self.prev_rr_reorder; + // Safe to drop: reports are only built after interval data, so + // a fresh report always advances at least one cumulative counter. + if counters_regressed || duplicate_counters { + return ( + false, + RrLog::Stale { + prev_highest: self.prev_rr_highest_counter, + prev_packets: self.prev_rr_cum_packets, + prev_bytes: self.prev_rr_cum_bytes, + }, + ); + } + } + + let mut log = RrLog::None; + + // --- RTT from timestamp echo --- + // RTT = now - echoed_timestamp - dwell_time + if rr.timestamp_echo > 0 { + let echo_ms = rr.timestamp_echo; + let dwell_ms = u32::from(rr.dwell_time); + let rtt_sample_ms = echo_ms + .checked_add(dwell_ms) + .and_then(|send_done_ms| our_timestamp_ms.checked_sub(send_done_ms)); + + match rtt_sample_ms { + Some(rtt_ms) if rtt_ms > 0 => { + let rtt_us = (rtt_ms as i64) * 1000; + // Capture SRTT before folding in this sample (the original + // trace read `srtt` before `update`). + let srtt_ms = self.srtt.srtt_us() as f64 / 1000.0; + log = RrLog::RttSample { rtt_ms, srtt_ms }; + self.srtt.update(rtt_us); + self.rtt_trend.update(rtt_us as f64); + } + _ => { + log = RrLog::InvalidRtt; + } + } + } + + // --- Loss rate from cumulative counters --- + // Delta: frames the peer should have received vs. actually received + if self.has_prev_rr { + let counter_span = rr + .highest_counter + .saturating_sub(self.prev_rr_highest_counter); + let packets_delta = rr + .cumulative_packets_recv + .saturating_sub(self.prev_rr_cum_packets); + + if counter_span > 0 { + let delivery = (packets_delta as f64) / (counter_span as f64); + self.delivery_ratio_forward = delivery.clamp(0.0, 1.0); + let loss_rate = 1.0 - self.delivery_ratio_forward; + self.loss_trend.update(loss_rate); + self.etx = compute_etx(self.delivery_ratio_forward, self.delivery_ratio_reverse); + self.etx_trend.update(self.etx); + } + } + + // --- Goodput from cumulative bytes + time delta --- + if self.has_prev_rr { + let bytes_delta = rr + .cumulative_bytes_recv + .saturating_sub(self.prev_rr_cum_bytes); + self.goodput_trend.update(bytes_delta as f64); + + // Compute bytes/sec if we have a time reference + if let Some(prev_ms) = self.prev_rr_ms { + let secs = (now_ms.saturating_sub(prev_ms) as f64) / 1000.0; + if secs > 0.0 { + let bps = bytes_delta as f64 / secs; + // EWMA smoothing: α = 1/4 + if self.goodput_bps == 0.0 { + self.goodput_bps = bps; + } else { + self.goodput_bps += (bps - self.goodput_bps) * 0.25; + } + } + } + } + + // --- Jitter trend --- + self.jitter_trend.update(rr.jitter as f64); + + // --- Save for next delta --- + self.prev_rr_cum_packets = rr.cumulative_packets_recv; + self.prev_rr_cum_bytes = rr.cumulative_bytes_recv; + self.prev_rr_highest_counter = rr.highest_counter; + self.prev_rr_ecn_ce = rr.ecn_ce_count; + self.prev_rr_reorder = rr.cumulative_reorder_count; + self.prev_rr_ms = Some(now_ms); + self.has_prev_rr = true; + + (!had_srtt && self.srtt.initialized(), log) + } + + /// Update the reverse delivery ratio from our own receiver state. + /// + /// Computes a per-interval delta (same as forward ratio) rather than + /// a lifetime cumulative ratio, so ETX responds to recent conditions. + pub fn update_reverse_delivery(&mut self, our_recv_packets: u64, peer_highest: u64) { + if self.has_prev_reverse { + let counter_span = peer_highest.saturating_sub(self.prev_reverse_highest); + let packets_delta = our_recv_packets.saturating_sub(self.prev_reverse_packets); + + if counter_span > 0 { + let delivery = (packets_delta as f64) / (counter_span as f64); + self.delivery_ratio_reverse = delivery.clamp(0.0, 1.0); + self.etx = compute_etx(self.delivery_ratio_forward, self.delivery_ratio_reverse); + self.etx_trend.update(self.etx); + } + } + + self.prev_reverse_packets = our_recv_packets; + self.prev_reverse_highest = peer_highest; + self.has_prev_reverse = true; + } + + /// Current smoothed RTT in milliseconds, or `None` if not yet measured. + pub fn srtt_ms(&self) -> Option { + if self.srtt.initialized() { + Some(self.srtt.srtt_us() as f64 / 1000.0) + } else { + None + } + } + + /// Current loss rate (0.0 = no loss, 1.0 = total loss). + pub fn loss_rate(&self) -> f64 { + 1.0 - self.delivery_ratio_forward + } + + /// Smoothed loss rate (long-term EWMA), or `None` if not yet initialized. + pub fn smoothed_loss(&self) -> Option { + if self.loss_trend.initialized() { + Some(self.loss_trend.long()) + } else { + None + } + } + + /// Smoothed ETX (long-term EWMA), or `None` if not yet initialized. + pub fn smoothed_etx(&self) -> Option { + if self.etx_trend.initialized() { + Some(self.etx_trend.long()) + } else { + None + } + } + + /// Current smoothed goodput in bytes/sec, or 0 if not yet measured. + pub fn goodput_bps(&self) -> f64 { + self.goodput_bps + } + + /// Cumulative ECN CE count from the most recent ReceiverReport. + pub fn last_ecn_ce_count(&self) -> u32 { + self.prev_rr_ecn_ce + } +} + +impl Default for MmpMetrics { + fn default() -> Self { + Self::new() + } +} diff --git a/src/proto/mmp/mod.rs b/src/proto/mmp/mod.rs index 37db794..400dbe8 100644 --- a/src/proto/mmp/mod.rs +++ b/src/proto/mmp/mod.rs @@ -7,8 +7,12 @@ //! - `core.rs` — the snapshot read-seams, the [`MmpAction`] effect vocabulary, //! and the pure `plan_*` decisions (line-invariant across the master/next //! report shapes). -//! - `state.rs` — [`Mmp`] (the reporting anchor) plus the owned sender/receiver/ -//! metrics/path-MTU state machines (`u64`-ms time seam, `no_std`+`alloc`). +//! - `state.rs` — [`Mmp`] (the reporting anchor) plus the [`MmpPeerState`]/ +//! [`MmpSessionState`] per-entity aggregates. +//! - `sender.rs`/`receiver.rs`/`metrics.rs`/`path_mtu.rs` — the owned sender/ +//! receiver/derived-metrics/path-MTU role state machines (`u64`-ms time seam, +//! `no_std`+`alloc`). +//! - `limits.rs` — the module tuning constants. //! - `algorithms.rs` — the pure estimators (jitter/SRTT/dual-EWMA/OWD-trend/ETX) //! and the spin-bit state (`no_std`+`alloc`). //! - `wire.rs` — the link-layer and session-layer report codecs. @@ -23,6 +27,11 @@ use serde::{Deserialize, Serialize}; mod algorithms; mod core; +mod limits; +mod metrics; +mod path_mtu; +mod receiver; +mod sender; mod state; mod wire; @@ -34,7 +43,8 @@ pub(crate) use core::{ BackoffUpdate, LinkReportKind, LinkReportSnapshot, MmpAction, PeerLivenessSnapshot, SendResult, SessionReportKind, SessionReportSnapshot, }; -pub(crate) use state::{Mmp, MmpMetrics, MmpPeerState, MmpSessionState, RrLog}; +pub(crate) use metrics::{MmpMetrics, RrLog}; +pub(crate) use state::{Mmp, MmpPeerState, MmpSessionState}; pub use wire::{ PathMtuNotification, ReceiverReport, SenderReport, SessionReceiverReport, SessionSenderReport, }; @@ -70,52 +80,11 @@ impl fmt::Display for MmpMode { // Constants // ============================================================================ -// --- EWMA parameters --- - -/// Dual EWMA short-term: α = 1/4. -pub const EWMA_SHORT_ALPHA: f64 = 0.25; - -/// Dual EWMA long-term: α = 1/32. -pub const EWMA_LONG_ALPHA: f64 = 1.0 / 32.0; - -// --- Timing defaults (milliseconds) --- - -/// Default report interval before SRTT is available (cold start). -pub const DEFAULT_COLD_START_INTERVAL_MS: u64 = 200; - -/// Minimum report interval (SRTT clamp floor). -/// -/// Raised from 100ms to 1000ms: parent re-evaluation runs every 60s, -/// so 60 samples/cycle is more than sufficient for EWMA convergence (~10). -/// The cold-start phase uses `DEFAULT_COLD_START_INTERVAL_MS` (200ms) for -/// fast initial SRTT convergence before transitioning to this floor. -pub const MIN_REPORT_INTERVAL_MS: u64 = 1_000; - -/// Maximum report interval (SRTT clamp ceiling). -pub const MAX_REPORT_INTERVAL_MS: u64 = 5_000; - -/// Number of SRTT samples before transitioning from cold-start to normal floor. -/// -/// During cold-start, report intervals use `DEFAULT_COLD_START_INTERVAL_MS` as -/// the floor to gather SRTT samples quickly. After this many updates, the floor -/// switches to `MIN_REPORT_INTERVAL_MS`. -pub const COLD_START_SAMPLES: u32 = 5; - -/// Default OWD ring buffer capacity. -pub const DEFAULT_OWD_WINDOW_SIZE: usize = 32; - -/// Default operator log interval in seconds. -pub const DEFAULT_LOG_INTERVAL_SECS: u64 = 30; - -// --- Session-layer timing defaults --- -// Session reports are routed end-to-end (bandwidth cost on every transit link), -// so intervals are higher than link-layer. - -/// Session-layer minimum report interval. -pub const MIN_SESSION_REPORT_INTERVAL_MS: u64 = 500; - -/// Session-layer maximum report interval. -pub const MAX_SESSION_REPORT_INTERVAL_MS: u64 = 10_000; - -/// Session-layer cold-start report interval (before SRTT is available). -pub const SESSION_COLD_START_INTERVAL_MS: u64 = 1_000; +// The module tuning constants live in `limits.rs`; re-exported here so external +// callers keep the `crate::proto::mmp::` path. +pub use limits::{ + COLD_START_SAMPLES, DEFAULT_COLD_START_INTERVAL_MS, DEFAULT_LOG_INTERVAL_SECS, + DEFAULT_OWD_WINDOW_SIZE, EWMA_LONG_ALPHA, EWMA_SHORT_ALPHA, MAX_REPORT_INTERVAL_MS, + MAX_SESSION_REPORT_INTERVAL_MS, MIN_REPORT_INTERVAL_MS, MIN_SESSION_REPORT_INTERVAL_MS, + SESSION_COLD_START_INTERVAL_MS, +}; diff --git a/src/proto/mmp/path_mtu.rs b/src/proto/mmp/path_mtu.rs new file mode 100644 index 0000000..f9b676d --- /dev/null +++ b/src/proto/mmp/path_mtu.rs @@ -0,0 +1,175 @@ +//! Path MTU tracking for a single session (sans-IO, session-layer only). +//! +//! Destination side observes `path_mtu` from incoming envelopes and emits +//! notifications; source side applies received notifications to bound outbound +//! datagram size. All time inputs are injected `u64` milliseconds. + +/// Path MTU tracking for a single session. +/// +/// Destination side: observes `path_mtu` from incoming SessionDatagram envelopes +/// and generates PathMtuNotification messages back to the source. +/// +/// Source side: applies received PathMtuNotification to limit outbound datagram +/// size. Decrease is immediate; increase requires 3 consecutive notifications. +pub struct PathMtuState { + /// Current effective path MTU (what we use for sending). + current_mtu: u16, + /// Last observed path MTU from incoming datagrams (destination-side). + last_observed_mtu: u16, + /// Whether the observed MTU has changed since the last notification. + observed_changed: bool, + /// Last time (injected `u64` ms) a PathMtuNotification was sent. + last_notification_ms: Option, + /// Notification interval in ms: max(10s, 5 * SRTT). Default 10s. + notification_interval_ms: u64, + /// For source-side increase tracking: consecutive higher-value notifications. + consecutive_increase_count: u8, + /// Time (injected `u64` ms) of the first notification in the current + /// increase sequence. + first_increase_ms: Option, + /// The MTU value being proposed for increase. + pending_increase_mtu: u16, +} + +impl PathMtuState { + /// Create path MTU state with no initial measurement. + pub fn new() -> Self { + Self { + current_mtu: u16::MAX, + last_observed_mtu: u16::MAX, + observed_changed: false, + last_notification_ms: None, + notification_interval_ms: 10_000, + consecutive_increase_count: 0, + first_increase_ms: None, + pending_increase_mtu: 0, + } + } + + /// Current effective path MTU (source-side, for sending). + pub fn current_mtu(&self) -> u16 { + self.current_mtu + } + + /// Last observed incoming path MTU (destination-side). + pub fn last_observed_mtu(&self) -> u16 { + self.last_observed_mtu + } + + /// Update notification interval from SRTT: max(10s, 5 * SRTT). + pub fn update_interval_from_srtt(&mut self, srtt_ms: f64) { + self.notification_interval_ms = ((srtt_ms * 5.0) as u64).max(10_000); + } + + /// Seed source-side current_mtu from outbound transport MTU. + /// + /// Called on each send. Only decreases (never increases) the current_mtu + /// so the destination's PathMtuNotification can still raise it later. + /// Ensures current_mtu doesn't stay at u16::MAX before any notification + /// arrives from the destination. + pub fn seed_source_mtu(&mut self, outbound_mtu: u16) { + if outbound_mtu < self.current_mtu { + self.current_mtu = outbound_mtu; + } + } + + // --- Destination side --- + + /// Observe the path_mtu from an incoming SessionDatagram envelope. + /// + /// Called on the destination (receiver) side for every session message. + pub fn observe_incoming_mtu(&mut self, path_mtu: u16) { + if path_mtu != self.last_observed_mtu { + self.observed_changed = true; + self.last_observed_mtu = path_mtu; + } + } + + /// Check if a PathMtuNotification should be sent. + /// + /// Send on first measurement, on decrease (immediate), or periodic + /// confirmation at the notification interval. `now_ms` is the injected + /// monotonic time in milliseconds. + pub fn should_send_notification(&self, now_ms: u64) -> bool { + if self.last_observed_mtu == u16::MAX { + return false; // No measurement yet + } + match self.last_notification_ms { + None => true, // First measurement + Some(last) => { + // Immediate on decrease + if self.observed_changed && self.last_observed_mtu < self.current_mtu { + return true; + } + // Periodic confirmation + now_ms.saturating_sub(last) >= self.notification_interval_ms + } + } + } + + /// Build a PathMtuNotification from current state. + /// + /// Returns the path_mtu value to send. Caller handles encoding. + pub fn build_notification(&mut self, now_ms: u64) -> Option { + if self.last_observed_mtu == u16::MAX { + return None; + } + self.last_notification_ms = Some(now_ms); + self.observed_changed = false; + Some(self.last_observed_mtu) + } + + // --- Source side --- + + /// Apply a received PathMtuNotification. + /// + /// - Decrease: immediate (take the lower value). + /// - Increase: require 3 consecutive notifications with the same higher + /// value, spanning at least 2 * notification_interval. + /// + /// `now_ms` is the injected monotonic time in milliseconds. Returns `true` + /// if the effective MTU changed. + pub fn apply_notification(&mut self, reported_mtu: u16, now_ms: u64) -> bool { + if reported_mtu < self.current_mtu { + // Decrease: immediate + self.current_mtu = reported_mtu; + self.consecutive_increase_count = 0; + self.first_increase_ms = None; + return true; + } + + if reported_mtu > self.current_mtu { + // Increase: track consecutive notifications + if reported_mtu == self.pending_increase_mtu { + self.consecutive_increase_count += 1; + } else { + // Different value: reset sequence + self.pending_increase_mtu = reported_mtu; + self.consecutive_increase_count = 1; + self.first_increase_ms = Some(now_ms); + } + + // Accept increase after 3 consecutive spanning 2 * interval + if self.consecutive_increase_count >= 3 + && let Some(first_ms) = self.first_increase_ms + { + let required_ms = self.notification_interval_ms * 2; + if now_ms.saturating_sub(first_ms) >= required_ms { + self.current_mtu = reported_mtu; + self.consecutive_increase_count = 0; + self.first_increase_ms = None; + return true; + } + } + } + + // No change (equal or increase not yet confirmed) + false + } +} + +impl Default for PathMtuState { + fn default() -> Self { + Self::new() + } +} diff --git a/src/proto/mmp/receiver.rs b/src/proto/mmp/receiver.rs new file mode 100644 index 0000000..15b9f44 --- /dev/null +++ b/src/proto/mmp/receiver.rs @@ -0,0 +1,447 @@ +//! Per-peer receiver-side MMP state (sans-IO). +//! +//! Accumulates per-frame observations (loss bursts, jitter, OWD trend, ECN) +//! and produces `ReceiverReport` snapshots. All time inputs are injected `u64` +//! milliseconds. + +use super::algorithms::{JitterEstimator, OwdTrendDetector}; +use super::wire::ReceiverReport; +use super::{ + COLD_START_SAMPLES, DEFAULT_COLD_START_INTERVAL_MS, DEFAULT_OWD_WINDOW_SIZE, + MAX_REPORT_INTERVAL_MS, MIN_REPORT_INTERVAL_MS, +}; + +/// Grace period after rekey before resuming jitter calculation. +/// +/// During rekey cutover, frames from the old session may still arrive via the +/// drain window (DRAIN_WINDOW_SECS = 10s). These carry large sender timestamps +/// from the old session, producing enormous transit deltas that spike the EWMA +/// jitter estimator. We suppress jitter updates for drain window + 5s margin. +const REKEY_JITTER_GRACE_SECS: u64 = 15; + +// ============================================================================ +// Gap Tracker (burst loss detection) +// ============================================================================ + +/// Tracks counter gaps to detect loss bursts. +/// +/// Each gap in the counter sequence is a burst of lost frames. +/// Maintains per-interval statistics that are reset when a report is built. +struct GapTracker { + /// Next expected counter value. + expected_next: Option, + /// Whether we are currently in a burst (gap). + in_burst: bool, + /// Length of the current burst. + current_burst_len: u16, + + // --- Per-interval stats (reset on report) --- + /// Number of distinct burst events this interval. + burst_count: u32, + /// Longest burst in this interval. + max_burst_len: u16, + /// Sum of all burst lengths (for mean computation). + total_burst_len: u64, +} + +impl GapTracker { + fn new() -> Self { + Self { + expected_next: None, + in_burst: false, + current_burst_len: 0, + burst_count: 0, + max_burst_len: 0, + total_burst_len: 0, + } + } + + /// Process a received counter value. Returns the number of lost frames + /// detected (0 if in order or first frame). + fn observe(&mut self, counter: u64) -> u64 { + let Some(expected) = self.expected_next else { + // First frame: initialize + self.expected_next = Some(counter + 1); + return 0; + }; + + let lost = if counter > expected { + // Gap detected + let gap = counter - expected; + if self.in_burst { + // Extend current burst + self.current_burst_len = self.current_burst_len.saturating_add(gap as u16); + } else { + // New burst + self.in_burst = true; + self.current_burst_len = gap as u16; + self.burst_count += 1; + } + gap + } else { + // In-order or duplicate (counter <= expected) + if self.in_burst { + // End current burst + self.finish_burst(); + } + 0 + }; + + // Update expected (always advance to counter+1 or keep expected if + // this was a late/reordered frame) + if counter >= expected { + self.expected_next = Some(counter + 1); + } + + lost + } + + /// Finish the current burst and record its stats. + fn finish_burst(&mut self) { + if self.in_burst { + self.max_burst_len = self.max_burst_len.max(self.current_burst_len); + self.total_burst_len += self.current_burst_len as u64; + self.in_burst = false; + self.current_burst_len = 0; + } + } + + /// Get interval stats and reset for next interval. + fn take_interval_stats(&mut self) -> (u32, u16, u16) { + // Finish any in-progress burst + self.finish_burst(); + + let count = self.burst_count; + let max_len = self.max_burst_len; + let mean_len = if count > 0 { + // u8.8 fixed-point: (total / count) * 256 + let mean_f = (self.total_burst_len as f64) / (count as f64); + (mean_f * 256.0) as u16 + } else { + 0 + }; + + // Reset interval + self.burst_count = 0; + self.max_burst_len = 0; + self.total_burst_len = 0; + + (count, max_len, mean_len) + } +} + +// ============================================================================ +// ReceiverState +// ============================================================================ + +/// Per-peer receiver-side MMP state. +/// +/// Accumulates per-frame observations and produces `ReceiverReport` snapshots. +pub struct ReceiverState { + // --- Cumulative (lifetime) --- + cumulative_packets_recv: u64, + cumulative_bytes_recv: u64, + cumulative_reorder_count: u64, + + /// Highest counter value ever received. + highest_counter: u64, + + // --- Current interval --- + interval_packets_recv: u32, + interval_bytes_recv: u32, + + // --- Jitter --- + jitter: JitterEstimator, + + // --- OWD trend --- + owd_trend: OwdTrendDetector, + /// Monotonic sequence counter for OWD samples. + owd_seq: u32, + + // --- Loss tracking --- + gap_tracker: GapTracker, + + // --- ECN --- + ecn_ce_count: u32, + + // --- Timestamp echo --- + /// Sender timestamp from the most recent frame (for echo). + last_sender_timestamp: u32, + /// Local time (injected `u64` ms) when the most recent frame was received + /// (for dwell / jitter computation). + last_recv_ms: Option, + + // --- Rekey grace --- + /// When set, jitter updates are suppressed until this injected-ms instant + /// passes. Prevents drain-window frames from spiking the jitter estimator. + rekey_jitter_grace_until_ms: Option, + + // --- Report timing (injected `u64` ms) --- + last_report_ms: Option, + report_interval_ms: u64, + /// Whether any frames have been received since the last report. + interval_has_data: bool, + + // --- Cold-start tracking --- + /// Number of SRTT-based interval updates received. + srtt_sample_count: u32, +} + +impl ReceiverState { + pub fn new(owd_window_size: usize) -> Self { + Self::new_with_cold_start(owd_window_size, DEFAULT_COLD_START_INTERVAL_MS) + } + + /// Create with a custom cold-start interval (ms). + /// + /// Used by session-layer MMP which needs a longer initial interval + /// since reports consume bandwidth on every transit link. + pub fn new_with_cold_start(owd_window_size: usize, cold_start_ms: u64) -> Self { + Self { + cumulative_packets_recv: 0, + cumulative_bytes_recv: 0, + cumulative_reorder_count: 0, + highest_counter: 0, + interval_packets_recv: 0, + interval_bytes_recv: 0, + jitter: JitterEstimator::new(), + owd_trend: OwdTrendDetector::new(owd_window_size), + owd_seq: 0, + gap_tracker: GapTracker::new(), + ecn_ce_count: 0, + last_sender_timestamp: 0, + last_recv_ms: None, + rekey_jitter_grace_until_ms: None, + last_report_ms: None, + report_interval_ms: cold_start_ms, + interval_has_data: false, + srtt_sample_count: 0, + } + } + + /// Reset counter-dependent state for rekey cutover. + /// + /// After cutover, the new session starts with counter 0 and reset + /// timestamps. Without resetting, the old `highest_counter` and + /// `GapTracker.expected_next` cause false reorder/loss detection. + /// `now_ms` is the injected monotonic time in milliseconds. + pub fn reset_for_rekey(&mut self, now_ms: u64) { + self.highest_counter = 0; + self.cumulative_reorder_count = 0; + self.gap_tracker = GapTracker::new(); + self.interval_packets_recv = 0; + self.interval_bytes_recv = 0; + self.jitter = JitterEstimator::new(); + self.owd_trend.clear(); + self.owd_seq = 0; + self.last_sender_timestamp = 0; + self.last_recv_ms = None; + self.rekey_jitter_grace_until_ms = Some(now_ms + REKEY_JITTER_GRACE_SECS * 1000); + self.ecn_ce_count = 0; + self.interval_has_data = false; + // Keep cumulative_packets_recv, cumulative_bytes_recv (lifetime stats) + // Keep last_report_ms, report_interval_ms (report scheduling) + } + + /// Record a received frame from this peer. + /// + /// Called on the RX path after AEAD decryption, before message dispatch. + /// + /// - `counter`: AEAD counter from outer header + /// - `sender_timestamp_ms`: session-relative timestamp from inner header (ms) + /// - `bytes`: wire payload size + /// - `ce_flag`: CE bit from flags byte + /// - `now_ms`: injected monotonic local time in milliseconds + pub fn record_recv( + &mut self, + counter: u64, + sender_timestamp_ms: u32, + bytes: usize, + ce_flag: bool, + now_ms: u64, + ) { + self.interval_has_data = true; + self.cumulative_packets_recv += 1; + self.cumulative_bytes_recv += bytes as u64; + self.interval_packets_recv = self.interval_packets_recv.saturating_add(1); + self.interval_bytes_recv = self.interval_bytes_recv.saturating_add(bytes as u32); + + // Reordering detection: counter < highest means out-of-order + if counter < self.highest_counter { + self.cumulative_reorder_count += 1; + } else { + self.highest_counter = counter; + } + + // Loss/burst detection + let _lost = self.gap_tracker.observe(counter); + + // ECN + if ce_flag { + self.ecn_ce_count = self.ecn_ce_count.saturating_add(1); + } + + // Jitter: compute transit time delta + // Transit = recv_local - sender_timestamp (in µs for precision) + // We use the injected monotonic ms clock for the local reference. + let sender_us = (sender_timestamp_ms as i64) * 1000; + // We compute the delta between consecutive transits using relative + // millisecond differences (scaled to µs to match the estimator input). + // Skip during post-rekey grace period to avoid drain-window spikes. + let in_grace = self + .rekey_jitter_grace_until_ms + .is_some_and(|deadline| now_ms < deadline); + if !in_grace { + self.rekey_jitter_grace_until_ms = None; // clear expired grace + if let Some(prev_recv) = self.last_recv_ms { + let recv_delta_us = (now_ms.saturating_sub(prev_recv) as i64) * 1000; + let send_delta_us = sender_us - (self.last_sender_timestamp as i64 * 1000); + let transit_delta = (recv_delta_us - send_delta_us) as i32; + self.jitter.update(transit_delta); + } + } + + // OWD trend: use sender timestamp as a proxy for send time + // and the injected ms delta from a fixed reference as receive time. + // Since we only need the *trend* (slope), absolute offsets cancel out. + if let Some(first_recv) = self.last_recv_ms.or(Some(now_ms)) { + let recv_offset_us = (now_ms.saturating_sub(first_recv) as i64) * 1000; + let owd_us = recv_offset_us - sender_us; + self.owd_seq = self.owd_seq.wrapping_add(1); + self.owd_trend.push(self.owd_seq, owd_us); + } + + // Timestamp echo state + self.last_sender_timestamp = sender_timestamp_ms; + self.last_recv_ms = Some(now_ms); + } + + /// Build a ReceiverReport from current state and reset the interval. + /// + /// Returns `None` if no frames have been received since the last report. + /// `now_ms` is the injected monotonic time in milliseconds. + pub fn build_report(&mut self, now_ms: u64) -> Option { + if !self.interval_has_data { + return None; + } + + // Dwell time: ms between last frame reception and report generation. + // If it no longer fits on the wire, the timestamp echo cannot produce + // a valid RTT sample. Preserve the counters but suppress the echo. + let (timestamp_echo, dwell_time) = self + .last_recv_ms + .map(|t| { + let dwell_ms = now_ms.saturating_sub(t); + if dwell_ms > u64::from(u16::MAX) { + (0, u16::MAX) + } else { + (self.last_sender_timestamp, dwell_ms as u16) + } + }) + .unwrap_or((0, 0)); + + let (burst_count, max_burst, mean_burst) = self.gap_tracker.take_interval_stats(); + + let report = ReceiverReport { + highest_counter: self.highest_counter, + cumulative_packets_recv: self.cumulative_packets_recv, + cumulative_bytes_recv: self.cumulative_bytes_recv, + timestamp_echo, + dwell_time, + max_burst_loss: max_burst, + mean_burst_loss: mean_burst, + jitter: self.jitter.jitter_us(), + ecn_ce_count: self.ecn_ce_count, + owd_trend: self.owd_trend.trend_us_per_sec(), + burst_loss_count: burst_count, + cumulative_reorder_count: self.cumulative_reorder_count as u32, + interval_packets_recv: self.interval_packets_recv, + interval_bytes_recv: self.interval_bytes_recv, + }; + + // Reset interval + self.interval_packets_recv = 0; + self.interval_bytes_recv = 0; + self.interval_has_data = false; + self.last_report_ms = Some(now_ms); + + Some(report) + } + + /// Check if it's time to send a report. + pub fn should_send_report(&self, now_ms: u64) -> bool { + if !self.interval_has_data { + return false; + } + match self.last_report_ms { + None => true, + Some(last) => now_ms.saturating_sub(last) >= self.report_interval_ms, + } + } + + /// Update the report interval based on SRTT (link-layer defaults). + /// + /// Receiver reports at 1× SRTT clamped to [floor, MAX]. During cold-start + /// (first `COLD_START_SAMPLES` updates), the floor is the cold-start + /// interval (200ms) for fast SRTT convergence. After that, it rises to + /// `MIN_REPORT_INTERVAL_MS` (1000ms) for steady-state efficiency. + pub fn update_report_interval_from_srtt(&mut self, srtt_us: i64) { + self.srtt_sample_count = self.srtt_sample_count.saturating_add(1); + let floor = if self.srtt_sample_count <= COLD_START_SAMPLES { + DEFAULT_COLD_START_INTERVAL_MS + } else { + MIN_REPORT_INTERVAL_MS + }; + self.update_report_interval_with_bounds(srtt_us, floor, MAX_REPORT_INTERVAL_MS); + } + + /// Update the report interval based on SRTT with custom bounds. + /// + /// Used by session-layer MMP which needs higher clamp values since + /// each report consumes bandwidth on every transit link. + pub fn update_report_interval_with_bounds(&mut self, srtt_us: i64, min_ms: u64, max_ms: u64) { + if srtt_us <= 0 { + return; + } + let interval_ms = ((srtt_us as u64) / 1000).clamp(min_ms, max_ms); + self.report_interval_ms = interval_ms; + } + + // --- Accessors --- + + pub fn cumulative_packets_recv(&self) -> u64 { + self.cumulative_packets_recv + } + + pub fn cumulative_bytes_recv(&self) -> u64 { + self.cumulative_bytes_recv + } + + pub fn highest_counter(&self) -> u64 { + self.highest_counter + } + + pub fn jitter_us(&self) -> u32 { + self.jitter.jitter_us() + } + + pub fn report_interval_ms(&self) -> u64 { + self.report_interval_ms + } + + /// Local monotonic time (injected `u64` ms) of the most recent received + /// frame, or `None` if none has been received. + pub fn last_recv_ms(&self) -> Option { + self.last_recv_ms + } + + pub fn ecn_ce_count(&self) -> u32 { + self.ecn_ce_count + } +} + +impl Default for ReceiverState { + fn default() -> Self { + Self::new(DEFAULT_OWD_WINDOW_SIZE) + } +} diff --git a/src/proto/mmp/sender.rs b/src/proto/mmp/sender.rs new file mode 100644 index 0000000..6c98158 --- /dev/null +++ b/src/proto/mmp/sender.rs @@ -0,0 +1,213 @@ +//! Per-peer sender-side MMP state (sans-IO). +//! +//! Records cumulative and interval TX counters and produces `SenderReport` +//! snapshots on demand. All time inputs are injected `u64` milliseconds. + +use super::wire::SenderReport; +use super::{ + COLD_START_SAMPLES, DEFAULT_COLD_START_INTERVAL_MS, MAX_REPORT_INTERVAL_MS, + MIN_REPORT_INTERVAL_MS, +}; + +/// Per-peer sender-side MMP state. +/// +/// Records cumulative and interval counters for every frame transmitted +/// to this peer. Produces `SenderReport` snapshots on demand. +pub struct SenderState { + // --- Cumulative (lifetime) --- + cumulative_packets_sent: u64, + cumulative_bytes_sent: u64, + + // --- Current interval --- + interval_start_counter: u64, + interval_start_timestamp: u32, + interval_bytes_sent: u32, + /// Counter of the most recently sent frame. + last_counter: u64, + /// Timestamp of the most recently sent frame. + last_timestamp: u32, + /// Whether any frames have been sent in the current interval. + interval_has_data: bool, + + // --- Report timing (injected `u64` ms) --- + last_report_ms: Option, + report_interval_ms: u64, + + // --- Send failure backoff --- + /// Consecutive send failure count for backoff calculation. + consecutive_send_failures: u32, + + // --- Cold-start tracking --- + /// Number of SRTT-based interval updates received. + srtt_sample_count: u32, +} + +impl SenderState { + pub fn new() -> Self { + Self::new_with_cold_start(DEFAULT_COLD_START_INTERVAL_MS) + } + + /// Create with a custom cold-start interval (ms). + /// + /// Used by session-layer MMP which needs a longer initial interval + /// since reports consume bandwidth on every transit link. + pub fn new_with_cold_start(cold_start_ms: u64) -> Self { + Self { + cumulative_packets_sent: 0, + cumulative_bytes_sent: 0, + interval_start_counter: 0, + interval_start_timestamp: 0, + interval_bytes_sent: 0, + last_counter: 0, + last_timestamp: 0, + interval_has_data: false, + last_report_ms: None, + report_interval_ms: cold_start_ms, + consecutive_send_failures: 0, + srtt_sample_count: 0, + } + } + + /// Record a frame sent to this peer. + /// + /// Called on the TX path for every encrypted link message. + /// `counter` is the AEAD nonce/counter, `timestamp` is the inner header + /// session-relative timestamp (ms), `bytes` is the wire payload size. + pub fn record_sent(&mut self, counter: u64, timestamp: u32, bytes: usize) { + if !self.interval_has_data { + self.interval_start_counter = counter; + self.interval_start_timestamp = timestamp; + self.interval_has_data = true; + } + self.last_counter = counter; + self.last_timestamp = timestamp; + self.interval_bytes_sent = self.interval_bytes_sent.saturating_add(bytes as u32); + self.cumulative_packets_sent += 1; + self.cumulative_bytes_sent += bytes as u64; + } + + /// Build a SenderReport from current state and reset the interval. + /// + /// Returns `None` if no frames have been sent since the last report. + /// `now_ms` is the injected monotonic time in milliseconds. + pub fn build_report(&mut self, now_ms: u64) -> Option { + if !self.interval_has_data { + return None; + } + + let report = SenderReport { + interval_start_counter: self.interval_start_counter, + interval_end_counter: self.last_counter, + interval_start_timestamp: self.interval_start_timestamp, + interval_end_timestamp: self.last_timestamp, + interval_bytes_sent: self.interval_bytes_sent, + cumulative_packets_sent: self.cumulative_packets_sent, + cumulative_bytes_sent: self.cumulative_bytes_sent, + }; + + // Reset interval + self.interval_has_data = false; + self.interval_bytes_sent = 0; + self.last_report_ms = Some(now_ms); + + Some(report) + } + + /// Check if it's time to send a report. + /// + /// When consecutive send failures have occurred, the effective interval + /// is multiplied by an exponential backoff factor (2^failures, capped at 32×). + pub fn should_send_report(&self, now_ms: u64) -> bool { + if !self.interval_has_data { + return false; + } + match self.last_report_ms { + None => true, // Never sent a report — send immediately + Some(last) => { + let effective_ms = (self.report_interval_ms as f64 + * self.send_failure_backoff_multiplier()) + as u64; + now_ms.saturating_sub(last) >= effective_ms + } + } + } + + /// Record a send failure. Returns the new consecutive failure count. + pub fn record_send_failure(&mut self) -> u32 { + self.consecutive_send_failures += 1; + self.consecutive_send_failures + } + + /// Record a successful send. Returns the previous failure count (for summary logging). + pub fn record_send_success(&mut self) -> u32 { + let prev = self.consecutive_send_failures; + self.consecutive_send_failures = 0; + prev + } + + /// Get the backoff multiplier based on consecutive failures. + /// + /// Returns 1.0 for no failures, 2.0 for 1 failure, 4.0 for 2, ... + /// capped at 32.0 (5 failures). Computed as an exact power-of-two bit shift + /// (`2^k == 1 << k`), keeping the module `core`-only (no `libm` for `powi`). + pub fn send_failure_backoff_multiplier(&self) -> f64 { + if self.consecutive_send_failures == 0 { + 1.0 + } else { + (1u64 << self.consecutive_send_failures.min(5)) as f64 + } + } + + /// Update the report interval based on SRTT (link-layer defaults). + /// + /// Sender reports at 2× SRTT clamped to [floor, MAX]. During cold-start + /// (first `COLD_START_SAMPLES` updates), the floor is the cold-start + /// interval (200ms) for fast SRTT convergence. After that, it rises to + /// `MIN_REPORT_INTERVAL_MS` (1000ms) for steady-state efficiency. + pub fn update_report_interval_from_srtt(&mut self, srtt_us: i64) { + self.srtt_sample_count = self.srtt_sample_count.saturating_add(1); + let floor = if self.srtt_sample_count <= COLD_START_SAMPLES { + DEFAULT_COLD_START_INTERVAL_MS + } else { + MIN_REPORT_INTERVAL_MS + }; + self.update_report_interval_with_bounds(srtt_us, floor, MAX_REPORT_INTERVAL_MS); + } + + /// Update the report interval based on SRTT with custom bounds. + /// + /// Used by session-layer MMP which needs higher clamp values since + /// each report consumes bandwidth on every transit link. + pub fn update_report_interval_with_bounds(&mut self, srtt_us: i64, min_ms: u64, max_ms: u64) { + if srtt_us <= 0 { + return; + } + let interval_us = (srtt_us * 2) as u64; + let interval_ms = (interval_us / 1000).clamp(min_ms, max_ms); + self.report_interval_ms = interval_ms; + } + + // --- Accessors --- + + pub fn cumulative_packets_sent(&self) -> u64 { + self.cumulative_packets_sent + } + + pub fn cumulative_bytes_sent(&self) -> u64 { + self.cumulative_bytes_sent + } + + pub fn report_interval_ms(&self) -> u64 { + self.report_interval_ms + } + + pub fn consecutive_send_failures(&self) -> u32 { + self.consecutive_send_failures + } +} + +impl Default for SenderState { + fn default() -> Self { + Self::new() + } +} diff --git a/src/proto/mmp/state.rs b/src/proto/mmp/state.rs index c4184a3..5730a17 100644 --- a/src/proto/mmp/state.rs +++ b/src/proto/mmp/state.rs @@ -1,12 +1,11 @@ -//! Owned MMP protocol state (sans-IO). +//! Owned MMP protocol state anchor and per-peer/per-session aggregates (sans-IO). //! -//! The per-peer/per-session sender, receiver, derived-metrics, spin-bit and -//! path-MTU state machines migrated out of the async node shell. All time -//! inputs are injected `u64` milliseconds (the shell reads its monotonic clock -//! at the edge and passes the value in); nothing here reads a clock, logs, or -//! bumps a counter. `no_std`+`alloc`-clean: `core` arithmetic only, the ring -//! buffer comes from `super::algorithms` (which uses `alloc`), and observability -//! is returned to the shell as an [`RrLog`] outcome rather than emitted here. +//! [`Mmp`] is the stateless reporting anchor owned by `Node`; the per-entity +//! aggregates [`MmpPeerState`]/[`MmpSessionState`] wrap the owned role state +//! machines that live in sibling modules (`sender`, `receiver`, `metrics`, +//! `path_mtu`). All time inputs are injected `u64` milliseconds (the shell reads +//! its monotonic clock at the edge and passes the value in); nothing here reads a +//! clock, logs, or bumps a counter. `no_std`+`alloc`-clean. //! //! [`Mmp`] itself remains the stateless reporting anchor owned by `Node` (the //! live per-entity state lives on the peers'/sessions' shell structs); it exists @@ -15,22 +14,12 @@ use core::fmt::{self, Debug}; -use super::algorithms::{ - DualEwma, JitterEstimator, OwdTrendDetector, SpinBitState, SrttEstimator, compute_etx, -}; -use super::wire::{ReceiverReport, SenderReport}; -use super::{ - COLD_START_SAMPLES, DEFAULT_COLD_START_INTERVAL_MS, DEFAULT_OWD_WINDOW_SIZE, - MAX_REPORT_INTERVAL_MS, MIN_REPORT_INTERVAL_MS, MmpMode, SESSION_COLD_START_INTERVAL_MS, -}; - -/// Grace period after rekey before resuming jitter calculation. -/// -/// During rekey cutover, frames from the old session may still arrive via the -/// drain window (DRAIN_WINDOW_SECS = 10s). These carry large sender timestamps -/// from the old session, producing enormous transit deltas that spike the EWMA -/// jitter estimator. We suppress jitter updates for drain window + 5s margin. -const REKEY_JITTER_GRACE_SECS: u64 = 15; +use super::algorithms::SpinBitState; +use super::metrics::MmpMetrics; +use super::path_mtu::PathMtuState; +use super::receiver::ReceiverState; +use super::sender::SenderState; +use super::{MmpMode, SESSION_COLD_START_INTERVAL_MS}; /// MMP reporting subsystem anchor owned by [`Node`](crate::node::Node). /// @@ -51,964 +40,6 @@ impl Mmp { } } -// ============================================================================ -// ReceiverReport processing outcome (observability hoisted to the shell) -// ============================================================================ - -/// The observability outcome of [`MmpMetrics::process_receiver_report`], returned -/// so the async shell can emit the operator `trace!` points that used to live -/// mid-decision (per the sans-IO rule that migrated code carries no `tracing`). -/// -/// Exactly one variant applies per call; the shell has the incoming report and -/// our timestamp, so this only carries the values the shell cannot otherwise -/// reconstruct (the previous cumulative snapshot / the derived RTT). -pub enum RrLog { - /// No RTT log point fires (the report carried no timestamp echo). - None, - /// The report was ignored as stale/duplicate. Carries the previous - /// cumulative snapshot for the operator trace. - Stale { - prev_highest: u64, - prev_packets: u64, - prev_bytes: u64, - }, - /// A valid RTT sample was taken; carries the derived RTT and the SRTT as it - /// stood **before** this sample was folded in (matching the original log, - /// which read `srtt` before `update`). - RttSample { rtt_ms: u32, srtt_ms: f64 }, - /// The report carried an echo but the derived RTT was invalid (wrapped / - /// non-positive). - InvalidRtt, -} - -// ============================================================================ -// SenderState -// ============================================================================ - -/// Per-peer sender-side MMP state. -/// -/// Records cumulative and interval counters for every frame transmitted -/// to this peer. Produces `SenderReport` snapshots on demand. -pub struct SenderState { - // --- Cumulative (lifetime) --- - cumulative_packets_sent: u64, - cumulative_bytes_sent: u64, - - // --- Current interval --- - interval_start_counter: u64, - interval_start_timestamp: u32, - interval_bytes_sent: u32, - /// Counter of the most recently sent frame. - last_counter: u64, - /// Timestamp of the most recently sent frame. - last_timestamp: u32, - /// Whether any frames have been sent in the current interval. - interval_has_data: bool, - - // --- Report timing (injected `u64` ms) --- - last_report_ms: Option, - report_interval_ms: u64, - - // --- Send failure backoff --- - /// Consecutive send failure count for backoff calculation. - consecutive_send_failures: u32, - - // --- Cold-start tracking --- - /// Number of SRTT-based interval updates received. - srtt_sample_count: u32, -} - -impl SenderState { - pub fn new() -> Self { - Self::new_with_cold_start(DEFAULT_COLD_START_INTERVAL_MS) - } - - /// Create with a custom cold-start interval (ms). - /// - /// Used by session-layer MMP which needs a longer initial interval - /// since reports consume bandwidth on every transit link. - pub fn new_with_cold_start(cold_start_ms: u64) -> Self { - Self { - cumulative_packets_sent: 0, - cumulative_bytes_sent: 0, - interval_start_counter: 0, - interval_start_timestamp: 0, - interval_bytes_sent: 0, - last_counter: 0, - last_timestamp: 0, - interval_has_data: false, - last_report_ms: None, - report_interval_ms: cold_start_ms, - consecutive_send_failures: 0, - srtt_sample_count: 0, - } - } - - /// Record a frame sent to this peer. - /// - /// Called on the TX path for every encrypted link message. - /// `counter` is the AEAD nonce/counter, `timestamp` is the inner header - /// session-relative timestamp (ms), `bytes` is the wire payload size. - pub fn record_sent(&mut self, counter: u64, timestamp: u32, bytes: usize) { - if !self.interval_has_data { - self.interval_start_counter = counter; - self.interval_start_timestamp = timestamp; - self.interval_has_data = true; - } - self.last_counter = counter; - self.last_timestamp = timestamp; - self.interval_bytes_sent = self.interval_bytes_sent.saturating_add(bytes as u32); - self.cumulative_packets_sent += 1; - self.cumulative_bytes_sent += bytes as u64; - } - - /// Build a SenderReport from current state and reset the interval. - /// - /// Returns `None` if no frames have been sent since the last report. - /// `now_ms` is the injected monotonic time in milliseconds. - pub fn build_report(&mut self, now_ms: u64) -> Option { - if !self.interval_has_data { - return None; - } - - let report = SenderReport { - interval_start_counter: self.interval_start_counter, - interval_end_counter: self.last_counter, - interval_start_timestamp: self.interval_start_timestamp, - interval_end_timestamp: self.last_timestamp, - interval_bytes_sent: self.interval_bytes_sent, - cumulative_packets_sent: self.cumulative_packets_sent, - cumulative_bytes_sent: self.cumulative_bytes_sent, - }; - - // Reset interval - self.interval_has_data = false; - self.interval_bytes_sent = 0; - self.last_report_ms = Some(now_ms); - - Some(report) - } - - /// Check if it's time to send a report. - /// - /// When consecutive send failures have occurred, the effective interval - /// is multiplied by an exponential backoff factor (2^failures, capped at 32×). - pub fn should_send_report(&self, now_ms: u64) -> bool { - if !self.interval_has_data { - return false; - } - match self.last_report_ms { - None => true, // Never sent a report — send immediately - Some(last) => { - let effective_ms = (self.report_interval_ms as f64 - * self.send_failure_backoff_multiplier()) - as u64; - now_ms.saturating_sub(last) >= effective_ms - } - } - } - - /// Record a send failure. Returns the new consecutive failure count. - pub fn record_send_failure(&mut self) -> u32 { - self.consecutive_send_failures += 1; - self.consecutive_send_failures - } - - /// Record a successful send. Returns the previous failure count (for summary logging). - pub fn record_send_success(&mut self) -> u32 { - let prev = self.consecutive_send_failures; - self.consecutive_send_failures = 0; - prev - } - - /// Get the backoff multiplier based on consecutive failures. - /// - /// Returns 1.0 for no failures, 2.0 for 1 failure, 4.0 for 2, ... - /// capped at 32.0 (5 failures). Computed as an exact power-of-two bit shift - /// (`2^k == 1 << k`), keeping the module `core`-only (no `libm` for `powi`). - pub fn send_failure_backoff_multiplier(&self) -> f64 { - if self.consecutive_send_failures == 0 { - 1.0 - } else { - (1u64 << self.consecutive_send_failures.min(5)) as f64 - } - } - - /// Update the report interval based on SRTT (link-layer defaults). - /// - /// Sender reports at 2× SRTT clamped to [floor, MAX]. During cold-start - /// (first `COLD_START_SAMPLES` updates), the floor is the cold-start - /// interval (200ms) for fast SRTT convergence. After that, it rises to - /// `MIN_REPORT_INTERVAL_MS` (1000ms) for steady-state efficiency. - pub fn update_report_interval_from_srtt(&mut self, srtt_us: i64) { - self.srtt_sample_count = self.srtt_sample_count.saturating_add(1); - let floor = if self.srtt_sample_count <= COLD_START_SAMPLES { - DEFAULT_COLD_START_INTERVAL_MS - } else { - MIN_REPORT_INTERVAL_MS - }; - self.update_report_interval_with_bounds(srtt_us, floor, MAX_REPORT_INTERVAL_MS); - } - - /// Update the report interval based on SRTT with custom bounds. - /// - /// Used by session-layer MMP which needs higher clamp values since - /// each report consumes bandwidth on every transit link. - pub fn update_report_interval_with_bounds(&mut self, srtt_us: i64, min_ms: u64, max_ms: u64) { - if srtt_us <= 0 { - return; - } - let interval_us = (srtt_us * 2) as u64; - let interval_ms = (interval_us / 1000).clamp(min_ms, max_ms); - self.report_interval_ms = interval_ms; - } - - // --- Accessors --- - - pub fn cumulative_packets_sent(&self) -> u64 { - self.cumulative_packets_sent - } - - pub fn cumulative_bytes_sent(&self) -> u64 { - self.cumulative_bytes_sent - } - - pub fn report_interval_ms(&self) -> u64 { - self.report_interval_ms - } - - pub fn consecutive_send_failures(&self) -> u32 { - self.consecutive_send_failures - } -} - -impl Default for SenderState { - fn default() -> Self { - Self::new() - } -} - -// ============================================================================ -// Gap Tracker (burst loss detection) -// ============================================================================ - -/// Tracks counter gaps to detect loss bursts. -/// -/// Each gap in the counter sequence is a burst of lost frames. -/// Maintains per-interval statistics that are reset when a report is built. -struct GapTracker { - /// Next expected counter value. - expected_next: Option, - /// Whether we are currently in a burst (gap). - in_burst: bool, - /// Length of the current burst. - current_burst_len: u16, - - // --- Per-interval stats (reset on report) --- - /// Number of distinct burst events this interval. - burst_count: u32, - /// Longest burst in this interval. - max_burst_len: u16, - /// Sum of all burst lengths (for mean computation). - total_burst_len: u64, -} - -impl GapTracker { - fn new() -> Self { - Self { - expected_next: None, - in_burst: false, - current_burst_len: 0, - burst_count: 0, - max_burst_len: 0, - total_burst_len: 0, - } - } - - /// Process a received counter value. Returns the number of lost frames - /// detected (0 if in order or first frame). - fn observe(&mut self, counter: u64) -> u64 { - let Some(expected) = self.expected_next else { - // First frame: initialize - self.expected_next = Some(counter + 1); - return 0; - }; - - let lost = if counter > expected { - // Gap detected - let gap = counter - expected; - if self.in_burst { - // Extend current burst - self.current_burst_len = self.current_burst_len.saturating_add(gap as u16); - } else { - // New burst - self.in_burst = true; - self.current_burst_len = gap as u16; - self.burst_count += 1; - } - gap - } else { - // In-order or duplicate (counter <= expected) - if self.in_burst { - // End current burst - self.finish_burst(); - } - 0 - }; - - // Update expected (always advance to counter+1 or keep expected if - // this was a late/reordered frame) - if counter >= expected { - self.expected_next = Some(counter + 1); - } - - lost - } - - /// Finish the current burst and record its stats. - fn finish_burst(&mut self) { - if self.in_burst { - self.max_burst_len = self.max_burst_len.max(self.current_burst_len); - self.total_burst_len += self.current_burst_len as u64; - self.in_burst = false; - self.current_burst_len = 0; - } - } - - /// Get interval stats and reset for next interval. - fn take_interval_stats(&mut self) -> (u32, u16, u16) { - // Finish any in-progress burst - self.finish_burst(); - - let count = self.burst_count; - let max_len = self.max_burst_len; - let mean_len = if count > 0 { - // u8.8 fixed-point: (total / count) * 256 - let mean_f = (self.total_burst_len as f64) / (count as f64); - (mean_f * 256.0) as u16 - } else { - 0 - }; - - // Reset interval - self.burst_count = 0; - self.max_burst_len = 0; - self.total_burst_len = 0; - - (count, max_len, mean_len) - } -} - -// ============================================================================ -// ReceiverState -// ============================================================================ - -/// Per-peer receiver-side MMP state. -/// -/// Accumulates per-frame observations and produces `ReceiverReport` snapshots. -pub struct ReceiverState { - // --- Cumulative (lifetime) --- - cumulative_packets_recv: u64, - cumulative_bytes_recv: u64, - cumulative_reorder_count: u64, - - /// Highest counter value ever received. - highest_counter: u64, - - // --- Current interval --- - interval_packets_recv: u32, - interval_bytes_recv: u32, - - // --- Jitter --- - jitter: JitterEstimator, - - // --- OWD trend --- - owd_trend: OwdTrendDetector, - /// Monotonic sequence counter for OWD samples. - owd_seq: u32, - - // --- Loss tracking --- - gap_tracker: GapTracker, - - // --- ECN --- - ecn_ce_count: u32, - - // --- Timestamp echo --- - /// Sender timestamp from the most recent frame (for echo). - last_sender_timestamp: u32, - /// Local time (injected `u64` ms) when the most recent frame was received - /// (for dwell / jitter computation). - last_recv_ms: Option, - - // --- Rekey grace --- - /// When set, jitter updates are suppressed until this injected-ms instant - /// passes. Prevents drain-window frames from spiking the jitter estimator. - rekey_jitter_grace_until_ms: Option, - - // --- Report timing (injected `u64` ms) --- - last_report_ms: Option, - report_interval_ms: u64, - /// Whether any frames have been received since the last report. - interval_has_data: bool, - - // --- Cold-start tracking --- - /// Number of SRTT-based interval updates received. - srtt_sample_count: u32, -} - -impl ReceiverState { - pub fn new(owd_window_size: usize) -> Self { - Self::new_with_cold_start(owd_window_size, DEFAULT_COLD_START_INTERVAL_MS) - } - - /// Create with a custom cold-start interval (ms). - /// - /// Used by session-layer MMP which needs a longer initial interval - /// since reports consume bandwidth on every transit link. - pub fn new_with_cold_start(owd_window_size: usize, cold_start_ms: u64) -> Self { - Self { - cumulative_packets_recv: 0, - cumulative_bytes_recv: 0, - cumulative_reorder_count: 0, - highest_counter: 0, - interval_packets_recv: 0, - interval_bytes_recv: 0, - jitter: JitterEstimator::new(), - owd_trend: OwdTrendDetector::new(owd_window_size), - owd_seq: 0, - gap_tracker: GapTracker::new(), - ecn_ce_count: 0, - last_sender_timestamp: 0, - last_recv_ms: None, - rekey_jitter_grace_until_ms: None, - last_report_ms: None, - report_interval_ms: cold_start_ms, - interval_has_data: false, - srtt_sample_count: 0, - } - } - - /// Reset counter-dependent state for rekey cutover. - /// - /// After cutover, the new session starts with counter 0 and reset - /// timestamps. Without resetting, the old `highest_counter` and - /// `GapTracker.expected_next` cause false reorder/loss detection. - /// `now_ms` is the injected monotonic time in milliseconds. - pub fn reset_for_rekey(&mut self, now_ms: u64) { - self.highest_counter = 0; - self.cumulative_reorder_count = 0; - self.gap_tracker = GapTracker::new(); - self.interval_packets_recv = 0; - self.interval_bytes_recv = 0; - self.jitter = JitterEstimator::new(); - self.owd_trend.clear(); - self.owd_seq = 0; - self.last_sender_timestamp = 0; - self.last_recv_ms = None; - self.rekey_jitter_grace_until_ms = Some(now_ms + REKEY_JITTER_GRACE_SECS * 1000); - self.ecn_ce_count = 0; - self.interval_has_data = false; - // Keep cumulative_packets_recv, cumulative_bytes_recv (lifetime stats) - // Keep last_report_ms, report_interval_ms (report scheduling) - } - - /// Record a received frame from this peer. - /// - /// Called on the RX path after AEAD decryption, before message dispatch. - /// - /// - `counter`: AEAD counter from outer header - /// - `sender_timestamp_ms`: session-relative timestamp from inner header (ms) - /// - `bytes`: wire payload size - /// - `ce_flag`: CE bit from flags byte - /// - `now_ms`: injected monotonic local time in milliseconds - pub fn record_recv( - &mut self, - counter: u64, - sender_timestamp_ms: u32, - bytes: usize, - ce_flag: bool, - now_ms: u64, - ) { - self.interval_has_data = true; - self.cumulative_packets_recv += 1; - self.cumulative_bytes_recv += bytes as u64; - self.interval_packets_recv = self.interval_packets_recv.saturating_add(1); - self.interval_bytes_recv = self.interval_bytes_recv.saturating_add(bytes as u32); - - // Reordering detection: counter < highest means out-of-order - if counter < self.highest_counter { - self.cumulative_reorder_count += 1; - } else { - self.highest_counter = counter; - } - - // Loss/burst detection - let _lost = self.gap_tracker.observe(counter); - - // ECN - if ce_flag { - self.ecn_ce_count = self.ecn_ce_count.saturating_add(1); - } - - // Jitter: compute transit time delta - // Transit = recv_local - sender_timestamp (in µs for precision) - // We use the injected monotonic ms clock for the local reference. - let sender_us = (sender_timestamp_ms as i64) * 1000; - // We compute the delta between consecutive transits using relative - // millisecond differences (scaled to µs to match the estimator input). - // Skip during post-rekey grace period to avoid drain-window spikes. - let in_grace = self - .rekey_jitter_grace_until_ms - .is_some_and(|deadline| now_ms < deadline); - if !in_grace { - self.rekey_jitter_grace_until_ms = None; // clear expired grace - if let Some(prev_recv) = self.last_recv_ms { - let recv_delta_us = (now_ms.saturating_sub(prev_recv) as i64) * 1000; - let send_delta_us = sender_us - (self.last_sender_timestamp as i64 * 1000); - let transit_delta = (recv_delta_us - send_delta_us) as i32; - self.jitter.update(transit_delta); - } - } - - // OWD trend: use sender timestamp as a proxy for send time - // and the injected ms delta from a fixed reference as receive time. - // Since we only need the *trend* (slope), absolute offsets cancel out. - if let Some(first_recv) = self.last_recv_ms.or(Some(now_ms)) { - let recv_offset_us = (now_ms.saturating_sub(first_recv) as i64) * 1000; - let owd_us = recv_offset_us - sender_us; - self.owd_seq = self.owd_seq.wrapping_add(1); - self.owd_trend.push(self.owd_seq, owd_us); - } - - // Timestamp echo state - self.last_sender_timestamp = sender_timestamp_ms; - self.last_recv_ms = Some(now_ms); - } - - /// Build a ReceiverReport from current state and reset the interval. - /// - /// Returns `None` if no frames have been received since the last report. - /// `now_ms` is the injected monotonic time in milliseconds. - pub fn build_report(&mut self, now_ms: u64) -> Option { - if !self.interval_has_data { - return None; - } - - // Dwell time: ms between last frame reception and report generation. - // If it no longer fits on the wire, the timestamp echo cannot produce - // a valid RTT sample. Preserve the counters but suppress the echo. - let (timestamp_echo, dwell_time) = self - .last_recv_ms - .map(|t| { - let dwell_ms = now_ms.saturating_sub(t); - if dwell_ms > u64::from(u16::MAX) { - (0, u16::MAX) - } else { - (self.last_sender_timestamp, dwell_ms as u16) - } - }) - .unwrap_or((0, 0)); - - let (burst_count, max_burst, mean_burst) = self.gap_tracker.take_interval_stats(); - - let report = ReceiverReport { - highest_counter: self.highest_counter, - cumulative_packets_recv: self.cumulative_packets_recv, - cumulative_bytes_recv: self.cumulative_bytes_recv, - timestamp_echo, - dwell_time, - max_burst_loss: max_burst, - mean_burst_loss: mean_burst, - jitter: self.jitter.jitter_us(), - ecn_ce_count: self.ecn_ce_count, - owd_trend: self.owd_trend.trend_us_per_sec(), - burst_loss_count: burst_count, - cumulative_reorder_count: self.cumulative_reorder_count as u32, - interval_packets_recv: self.interval_packets_recv, - interval_bytes_recv: self.interval_bytes_recv, - }; - - // Reset interval - self.interval_packets_recv = 0; - self.interval_bytes_recv = 0; - self.interval_has_data = false; - self.last_report_ms = Some(now_ms); - - Some(report) - } - - /// Check if it's time to send a report. - pub fn should_send_report(&self, now_ms: u64) -> bool { - if !self.interval_has_data { - return false; - } - match self.last_report_ms { - None => true, - Some(last) => now_ms.saturating_sub(last) >= self.report_interval_ms, - } - } - - /// Update the report interval based on SRTT (link-layer defaults). - /// - /// Receiver reports at 1× SRTT clamped to [floor, MAX]. During cold-start - /// (first `COLD_START_SAMPLES` updates), the floor is the cold-start - /// interval (200ms) for fast SRTT convergence. After that, it rises to - /// `MIN_REPORT_INTERVAL_MS` (1000ms) for steady-state efficiency. - pub fn update_report_interval_from_srtt(&mut self, srtt_us: i64) { - self.srtt_sample_count = self.srtt_sample_count.saturating_add(1); - let floor = if self.srtt_sample_count <= COLD_START_SAMPLES { - DEFAULT_COLD_START_INTERVAL_MS - } else { - MIN_REPORT_INTERVAL_MS - }; - self.update_report_interval_with_bounds(srtt_us, floor, MAX_REPORT_INTERVAL_MS); - } - - /// Update the report interval based on SRTT with custom bounds. - /// - /// Used by session-layer MMP which needs higher clamp values since - /// each report consumes bandwidth on every transit link. - pub fn update_report_interval_with_bounds(&mut self, srtt_us: i64, min_ms: u64, max_ms: u64) { - if srtt_us <= 0 { - return; - } - let interval_ms = ((srtt_us as u64) / 1000).clamp(min_ms, max_ms); - self.report_interval_ms = interval_ms; - } - - // --- Accessors --- - - pub fn cumulative_packets_recv(&self) -> u64 { - self.cumulative_packets_recv - } - - pub fn cumulative_bytes_recv(&self) -> u64 { - self.cumulative_bytes_recv - } - - pub fn highest_counter(&self) -> u64 { - self.highest_counter - } - - pub fn jitter_us(&self) -> u32 { - self.jitter.jitter_us() - } - - pub fn report_interval_ms(&self) -> u64 { - self.report_interval_ms - } - - /// Local monotonic time (injected `u64` ms) of the most recent received - /// frame, or `None` if none has been received. - pub fn last_recv_ms(&self) -> Option { - self.last_recv_ms - } - - pub fn ecn_ce_count(&self) -> u32 { - self.ecn_ce_count - } -} - -impl Default for ReceiverState { - fn default() -> Self { - Self::new(DEFAULT_OWD_WINDOW_SIZE) - } -} - -// ============================================================================ -// MmpMetrics (derived metrics) -// ============================================================================ - -/// Derived MMP metrics, updated from incoming ReceiverReports. -/// -/// This lives on the sender side: when we receive a ReceiverReport from -/// our peer describing what they observed about our traffic, we process -/// it here to compute RTT, loss, goodput, and trend indicators. -pub struct MmpMetrics { - /// Smoothed RTT from timestamp echo. - pub srtt: SrttEstimator, - - /// Dual EWMA trend detectors. - pub rtt_trend: DualEwma, - pub loss_trend: DualEwma, - pub goodput_trend: DualEwma, - pub jitter_trend: DualEwma, - pub etx_trend: DualEwma, - - /// Forward delivery ratio (what fraction of our frames the peer received). - pub delivery_ratio_forward: f64, - /// Reverse delivery ratio (set when we compute from our own receiver state). - pub delivery_ratio_reverse: f64, - /// ETX computed from bidirectional delivery ratios. - pub etx: f64, - - /// Smoothed goodput in bytes/sec (forward direction: what the peer received from us). - pub goodput_bps: f64, - - // --- State for delta computation --- - /// Previous ReceiverReport's cumulative counters (for computing interval deltas). - prev_rr_cum_packets: u64, - prev_rr_cum_bytes: u64, - prev_rr_highest_counter: u64, - prev_rr_ecn_ce: u32, - prev_rr_reorder: u32, - /// Time (injected `u64` ms) of previous ReceiverReport (for goodput rate). - prev_rr_ms: Option, - /// Whether we have a previous ReceiverReport for delta computation. - has_prev_rr: bool, - - // --- State for reverse delivery ratio delta computation --- - /// Previous reverse-side cumulative packets received (our receiver state). - prev_reverse_packets: u64, - /// Previous reverse-side highest counter (our receiver state). - prev_reverse_highest: u64, - /// Whether we have a previous reverse-side snapshot for delta computation. - has_prev_reverse: bool, -} - -impl MmpMetrics { - /// Reset state derived from ReceiverReport counters for rekey cutover. - /// - /// The new session starts with counter 0, so the prev_rr deltas must - /// be reset to avoid computing bogus loss/goodput from the counter - /// discontinuity. RTT (SRTT) is preserved since it remains valid. - pub fn reset_for_rekey(&mut self) { - self.prev_rr_cum_packets = 0; - self.prev_rr_cum_bytes = 0; - self.prev_rr_highest_counter = 0; - self.prev_rr_ecn_ce = 0; - self.prev_rr_reorder = 0; - self.prev_rr_ms = None; - self.has_prev_rr = false; - self.delivery_ratio_forward = 1.0; - self.prev_reverse_packets = 0; - self.prev_reverse_highest = 0; - self.has_prev_reverse = false; - // Keep srtt, etx, trends, goodput_bps — they'll refresh from data - } - - pub fn new() -> Self { - Self { - srtt: SrttEstimator::new(), - rtt_trend: DualEwma::new(), - loss_trend: DualEwma::new(), - goodput_trend: DualEwma::new(), - jitter_trend: DualEwma::new(), - etx_trend: DualEwma::new(), - delivery_ratio_forward: 1.0, - delivery_ratio_reverse: 1.0, - etx: 1.0, - goodput_bps: 0.0, - prev_rr_cum_packets: 0, - prev_rr_cum_bytes: 0, - prev_rr_highest_counter: 0, - prev_rr_ecn_ce: 0, - prev_rr_reorder: 0, - prev_rr_ms: None, - has_prev_rr: false, - prev_reverse_packets: 0, - prev_reverse_highest: 0, - has_prev_reverse: false, - } - } - - /// Process an incoming ReceiverReport (from the peer about our traffic). - /// - /// `our_timestamp_ms` is the current session-relative time in ms (for RTT). - /// `now_ms` is the injected monotonic time in ms (for goodput rate). - /// - /// Returns `(first_srtt, log)`: `first_srtt` is `true` if this report - /// produced the first SRTT measurement (transition from uninitialized to - /// initialized); `log` is the [`RrLog`] observability outcome the shell - /// emits (the `tracing` that used to fire here now lives shell-side). - pub fn process_receiver_report( - &mut self, - rr: &ReceiverReport, - our_timestamp_ms: u32, - now_ms: u64, - ) -> (bool, RrLog) { - let had_srtt = self.srtt.initialized(); - - if self.has_prev_rr { - let counters_regressed = rr.highest_counter < self.prev_rr_highest_counter - || rr.cumulative_packets_recv < self.prev_rr_cum_packets - || rr.cumulative_bytes_recv < self.prev_rr_cum_bytes - || rr.ecn_ce_count < self.prev_rr_ecn_ce - || rr.cumulative_reorder_count < self.prev_rr_reorder; - let duplicate_counters = rr.highest_counter == self.prev_rr_highest_counter - && rr.cumulative_packets_recv == self.prev_rr_cum_packets - && rr.cumulative_bytes_recv == self.prev_rr_cum_bytes - && rr.ecn_ce_count == self.prev_rr_ecn_ce - && rr.cumulative_reorder_count == self.prev_rr_reorder; - // Safe to drop: reports are only built after interval data, so - // a fresh report always advances at least one cumulative counter. - if counters_regressed || duplicate_counters { - return ( - false, - RrLog::Stale { - prev_highest: self.prev_rr_highest_counter, - prev_packets: self.prev_rr_cum_packets, - prev_bytes: self.prev_rr_cum_bytes, - }, - ); - } - } - - let mut log = RrLog::None; - - // --- RTT from timestamp echo --- - // RTT = now - echoed_timestamp - dwell_time - if rr.timestamp_echo > 0 { - let echo_ms = rr.timestamp_echo; - let dwell_ms = u32::from(rr.dwell_time); - let rtt_sample_ms = echo_ms - .checked_add(dwell_ms) - .and_then(|send_done_ms| our_timestamp_ms.checked_sub(send_done_ms)); - - match rtt_sample_ms { - Some(rtt_ms) if rtt_ms > 0 => { - let rtt_us = (rtt_ms as i64) * 1000; - // Capture SRTT before folding in this sample (the original - // trace read `srtt` before `update`). - let srtt_ms = self.srtt.srtt_us() as f64 / 1000.0; - log = RrLog::RttSample { rtt_ms, srtt_ms }; - self.srtt.update(rtt_us); - self.rtt_trend.update(rtt_us as f64); - } - _ => { - log = RrLog::InvalidRtt; - } - } - } - - // --- Loss rate from cumulative counters --- - // Delta: frames the peer should have received vs. actually received - if self.has_prev_rr { - let counter_span = rr - .highest_counter - .saturating_sub(self.prev_rr_highest_counter); - let packets_delta = rr - .cumulative_packets_recv - .saturating_sub(self.prev_rr_cum_packets); - - if counter_span > 0 { - let delivery = (packets_delta as f64) / (counter_span as f64); - self.delivery_ratio_forward = delivery.clamp(0.0, 1.0); - let loss_rate = 1.0 - self.delivery_ratio_forward; - self.loss_trend.update(loss_rate); - self.etx = compute_etx(self.delivery_ratio_forward, self.delivery_ratio_reverse); - self.etx_trend.update(self.etx); - } - } - - // --- Goodput from cumulative bytes + time delta --- - if self.has_prev_rr { - let bytes_delta = rr - .cumulative_bytes_recv - .saturating_sub(self.prev_rr_cum_bytes); - self.goodput_trend.update(bytes_delta as f64); - - // Compute bytes/sec if we have a time reference - if let Some(prev_ms) = self.prev_rr_ms { - let secs = (now_ms.saturating_sub(prev_ms) as f64) / 1000.0; - if secs > 0.0 { - let bps = bytes_delta as f64 / secs; - // EWMA smoothing: α = 1/4 - if self.goodput_bps == 0.0 { - self.goodput_bps = bps; - } else { - self.goodput_bps += (bps - self.goodput_bps) * 0.25; - } - } - } - } - - // --- Jitter trend --- - self.jitter_trend.update(rr.jitter as f64); - - // --- Save for next delta --- - self.prev_rr_cum_packets = rr.cumulative_packets_recv; - self.prev_rr_cum_bytes = rr.cumulative_bytes_recv; - self.prev_rr_highest_counter = rr.highest_counter; - self.prev_rr_ecn_ce = rr.ecn_ce_count; - self.prev_rr_reorder = rr.cumulative_reorder_count; - self.prev_rr_ms = Some(now_ms); - self.has_prev_rr = true; - - (!had_srtt && self.srtt.initialized(), log) - } - - /// Update the reverse delivery ratio from our own receiver state. - /// - /// Computes a per-interval delta (same as forward ratio) rather than - /// a lifetime cumulative ratio, so ETX responds to recent conditions. - pub fn update_reverse_delivery(&mut self, our_recv_packets: u64, peer_highest: u64) { - if self.has_prev_reverse { - let counter_span = peer_highest.saturating_sub(self.prev_reverse_highest); - let packets_delta = our_recv_packets.saturating_sub(self.prev_reverse_packets); - - if counter_span > 0 { - let delivery = (packets_delta as f64) / (counter_span as f64); - self.delivery_ratio_reverse = delivery.clamp(0.0, 1.0); - self.etx = compute_etx(self.delivery_ratio_forward, self.delivery_ratio_reverse); - self.etx_trend.update(self.etx); - } - } - - self.prev_reverse_packets = our_recv_packets; - self.prev_reverse_highest = peer_highest; - self.has_prev_reverse = true; - } - - /// Current smoothed RTT in milliseconds, or `None` if not yet measured. - pub fn srtt_ms(&self) -> Option { - if self.srtt.initialized() { - Some(self.srtt.srtt_us() as f64 / 1000.0) - } else { - None - } - } - - /// Current loss rate (0.0 = no loss, 1.0 = total loss). - pub fn loss_rate(&self) -> f64 { - 1.0 - self.delivery_ratio_forward - } - - /// Smoothed loss rate (long-term EWMA), or `None` if not yet initialized. - pub fn smoothed_loss(&self) -> Option { - if self.loss_trend.initialized() { - Some(self.loss_trend.long()) - } else { - None - } - } - - /// Smoothed ETX (long-term EWMA), or `None` if not yet initialized. - pub fn smoothed_etx(&self) -> Option { - if self.etx_trend.initialized() { - Some(self.etx_trend.long()) - } else { - None - } - } - - /// Current smoothed goodput in bytes/sec, or 0 if not yet measured. - pub fn goodput_bps(&self) -> f64 { - self.goodput_bps - } - - /// Cumulative ECN CE count from the most recent ReceiverReport. - pub fn last_ecn_ce_count(&self) -> u32 { - self.prev_rr_ecn_ce - } -} - -impl Default for MmpMetrics { - fn default() -> Self { - Self::new() - } -} - // ============================================================================ // Per-Peer MMP State // ============================================================================ @@ -1163,177 +194,3 @@ impl Debug for MmpSessionState { .finish_non_exhaustive() } } - -// ============================================================================ -// Path MTU State (session-layer only) -// ============================================================================ - -/// Path MTU tracking for a single session. -/// -/// Destination side: observes `path_mtu` from incoming SessionDatagram envelopes -/// and generates PathMtuNotification messages back to the source. -/// -/// Source side: applies received PathMtuNotification to limit outbound datagram -/// size. Decrease is immediate; increase requires 3 consecutive notifications. -pub struct PathMtuState { - /// Current effective path MTU (what we use for sending). - current_mtu: u16, - /// Last observed path MTU from incoming datagrams (destination-side). - last_observed_mtu: u16, - /// Whether the observed MTU has changed since the last notification. - observed_changed: bool, - /// Last time (injected `u64` ms) a PathMtuNotification was sent. - last_notification_ms: Option, - /// Notification interval in ms: max(10s, 5 * SRTT). Default 10s. - notification_interval_ms: u64, - /// For source-side increase tracking: consecutive higher-value notifications. - consecutive_increase_count: u8, - /// Time (injected `u64` ms) of the first notification in the current - /// increase sequence. - first_increase_ms: Option, - /// The MTU value being proposed for increase. - pending_increase_mtu: u16, -} - -impl PathMtuState { - /// Create path MTU state with no initial measurement. - pub fn new() -> Self { - Self { - current_mtu: u16::MAX, - last_observed_mtu: u16::MAX, - observed_changed: false, - last_notification_ms: None, - notification_interval_ms: 10_000, - consecutive_increase_count: 0, - first_increase_ms: None, - pending_increase_mtu: 0, - } - } - - /// Current effective path MTU (source-side, for sending). - pub fn current_mtu(&self) -> u16 { - self.current_mtu - } - - /// Last observed incoming path MTU (destination-side). - pub fn last_observed_mtu(&self) -> u16 { - self.last_observed_mtu - } - - /// Update notification interval from SRTT: max(10s, 5 * SRTT). - pub fn update_interval_from_srtt(&mut self, srtt_ms: f64) { - self.notification_interval_ms = ((srtt_ms * 5.0) as u64).max(10_000); - } - - /// Seed source-side current_mtu from outbound transport MTU. - /// - /// Called on each send. Only decreases (never increases) the current_mtu - /// so the destination's PathMtuNotification can still raise it later. - /// Ensures current_mtu doesn't stay at u16::MAX before any notification - /// arrives from the destination. - pub fn seed_source_mtu(&mut self, outbound_mtu: u16) { - if outbound_mtu < self.current_mtu { - self.current_mtu = outbound_mtu; - } - } - - // --- Destination side --- - - /// Observe the path_mtu from an incoming SessionDatagram envelope. - /// - /// Called on the destination (receiver) side for every session message. - pub fn observe_incoming_mtu(&mut self, path_mtu: u16) { - if path_mtu != self.last_observed_mtu { - self.observed_changed = true; - self.last_observed_mtu = path_mtu; - } - } - - /// Check if a PathMtuNotification should be sent. - /// - /// Send on first measurement, on decrease (immediate), or periodic - /// confirmation at the notification interval. `now_ms` is the injected - /// monotonic time in milliseconds. - pub fn should_send_notification(&self, now_ms: u64) -> bool { - if self.last_observed_mtu == u16::MAX { - return false; // No measurement yet - } - match self.last_notification_ms { - None => true, // First measurement - Some(last) => { - // Immediate on decrease - if self.observed_changed && self.last_observed_mtu < self.current_mtu { - return true; - } - // Periodic confirmation - now_ms.saturating_sub(last) >= self.notification_interval_ms - } - } - } - - /// Build a PathMtuNotification from current state. - /// - /// Returns the path_mtu value to send. Caller handles encoding. - pub fn build_notification(&mut self, now_ms: u64) -> Option { - if self.last_observed_mtu == u16::MAX { - return None; - } - self.last_notification_ms = Some(now_ms); - self.observed_changed = false; - Some(self.last_observed_mtu) - } - - // --- Source side --- - - /// Apply a received PathMtuNotification. - /// - /// - Decrease: immediate (take the lower value). - /// - Increase: require 3 consecutive notifications with the same higher - /// value, spanning at least 2 * notification_interval. - /// - /// `now_ms` is the injected monotonic time in milliseconds. Returns `true` - /// if the effective MTU changed. - pub fn apply_notification(&mut self, reported_mtu: u16, now_ms: u64) -> bool { - if reported_mtu < self.current_mtu { - // Decrease: immediate - self.current_mtu = reported_mtu; - self.consecutive_increase_count = 0; - self.first_increase_ms = None; - return true; - } - - if reported_mtu > self.current_mtu { - // Increase: track consecutive notifications - if reported_mtu == self.pending_increase_mtu { - self.consecutive_increase_count += 1; - } else { - // Different value: reset sequence - self.pending_increase_mtu = reported_mtu; - self.consecutive_increase_count = 1; - self.first_increase_ms = Some(now_ms); - } - - // Accept increase after 3 consecutive spanning 2 * interval - if self.consecutive_increase_count >= 3 - && let Some(first_ms) = self.first_increase_ms - { - let required_ms = self.notification_interval_ms * 2; - if now_ms.saturating_sub(first_ms) >= required_ms { - self.current_mtu = reported_mtu; - self.consecutive_increase_count = 0; - self.first_increase_ms = None; - return true; - } - } - } - - // No change (equal or increase not yet confirmed) - false - } -} - -impl Default for PathMtuState { - fn default() -> Self { - Self::new() - } -} diff --git a/src/proto/mmp/tests/state.rs b/src/proto/mmp/tests/state.rs index 790cf77..378234a 100644 --- a/src/proto/mmp/tests/state.rs +++ b/src/proto/mmp/tests/state.rs @@ -1,6 +1,7 @@ //! Tests for the owned MMP protocol state machines. -use crate::proto::mmp::state::{ReceiverState, SenderState}; +use crate::proto::mmp::receiver::ReceiverState; +use crate::proto::mmp::sender::SenderState; use crate::proto::mmp::{ COLD_START_SAMPLES, DEFAULT_LOG_INTERVAL_SECS, DEFAULT_OWD_WINDOW_SIZE, MAX_REPORT_INTERVAL_MS, MIN_REPORT_INTERVAL_MS, MmpMetrics, MmpMode, ReceiverReport, diff --git a/src/proto/stp/limits.rs b/src/proto/stp/limits.rs index 9fc36ca..033a0bd 100644 --- a/src/proto/stp/limits.rs +++ b/src/proto/stp/limits.rs @@ -11,7 +11,7 @@ //! `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 +//! (monotonic milliseconds, read by the shell from `crate::time::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. diff --git a/src/time.rs b/src/time.rs new file mode 100644 index 0000000..d684f47 --- /dev/null +++ b/src/time.rs @@ -0,0 +1,16 @@ +//! Shell-side time seam: the process-monotonic millisecond clock the sans-IO +//! cores take as an injected `u64`. + +use std::sync::LazyLock; +use std::time::Instant; + +/// Monotonic millisecond clock for the MMP time seam. +/// +/// The owned `proto::mmp` state takes injected `u64` milliseconds rather than +/// reading a clock. The shell reads this process-monotonic clock at each edge +/// and passes the value in; only deltas between two `mono_ms()` reads are ever +/// compared, so the (process-relative) epoch is immaterial. +pub(crate) fn mono_ms() -> u64 { + static START: LazyLock = LazyLock::new(Instant::now); + START.elapsed().as_millis() as u64 +} From dc9334e7256a4a3651e262a986302833460438bd Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 15:13:41 +0000 Subject: [PATCH 4/9] 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); } From a2400d823f2e8a45e197c760b03f79d12dd72a21 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 19:00:25 +0000 Subject: [PATCH 5/9] proto/stp: extract ParentDeclaration and relocate the coordinate type to a shared proto/coord module Two behavior-neutral relocations, wire bytes unchanged: - Move the ParentDeclaration type and its impls out of state.rs into a dedicated declaration.rs, re-exported at the same path, and relocate the inline wire.rs test module into tests/wire.rs alongside the other stp unit tests (reusing the shared test helpers). wire.rs drops to non-test code only. - Move TreeCoordinate, CoordEntry, and the coordinate wire codec out of proto/stp into a shared proto/coord module (a peer of proto/link), re-exported from proto::stp so every existing import path keeps resolving unchanged. Coordinate is a shared addressing primitive with many non-stp consumers, so it no longer belongs under the spanning-tree subsystem. The codec carries a documented temporary dependency on stp::TreeError until a dedicated CoordError is introduced. --- src/proto/{stp/coordinate.rs => coord.rs} | 103 ++++- src/proto/mod.rs | 1 + src/proto/stp/declaration.rs | 144 +++++++ src/proto/stp/mod.rs | 16 +- src/proto/stp/state.rs | 141 +------ src/proto/stp/tests/mod.rs | 1 + src/proto/stp/tests/wire.rs | 336 +++++++++++++++++ src/proto/stp/wire.rs | 437 ---------------------- 8 files changed, 593 insertions(+), 586 deletions(-) rename src/proto/{stp/coordinate.rs => coord.rs} (65%) create mode 100644 src/proto/stp/declaration.rs create mode 100644 src/proto/stp/tests/wire.rs diff --git a/src/proto/stp/coordinate.rs b/src/proto/coord.rs similarity index 65% rename from src/proto/stp/coordinate.rs rename to src/proto/coord.rs index c293dd4..64e04cf 100644 --- a/src/proto/stp/coordinate.rs +++ b/src/proto/coord.rs @@ -1,9 +1,15 @@ -//! Tree coordinates and distance calculations. +//! Tree coordinate addressing primitive: the `TreeCoordinate` path type, its +//! `CoordEntry` elements, and their wire codec. A shared `proto` primitive +//! (peer of `link.rs`), re-exported from `proto::stp` for source continuity. use core::fmt; -use super::TreeError; use crate::NodeAddr; +use crate::proto::Error; +// TEMPORARY: coord depends upward on stp's TreeError here. This inversion is +// intentional and resolved in the next pass when a no_std-clean CoordError is +// created and TreeCoordinate is cut over to it. +use crate::proto::stp::TreeError; /// Metadata for a single node in a tree coordinate path. /// @@ -223,3 +229,96 @@ impl fmt::Debug for TreeCoordinate { write!(f, "])") } } + +// ============================================================================ +// Coordinate Wire Format Helpers +// ============================================================================ + +/// Wire size of a TreeCoordinate in address-only format: 2 + entries × 16. +pub(crate) fn coords_wire_size(coords: &TreeCoordinate) -> usize { + 2 + coords.entries().len() * 16 +} + +/// Encode a TreeCoordinate as address-only wire format: count(u16 LE) + addrs(16 × n). +/// +/// Session-layer messages serialize coordinates as NodeAddr arrays (16 bytes each), +/// without the sequence/timestamp metadata used by the tree gossip protocol. +pub(crate) fn encode_coords(coords: &TreeCoordinate, buf: &mut Vec) { + let addrs: Vec<&NodeAddr> = coords.node_addrs().collect(); + let count = addrs.len() as u16; + buf.extend_from_slice(&count.to_le_bytes()); + for addr in addrs { + buf.extend_from_slice(addr.as_bytes()); + } +} + +/// Decode a TreeCoordinate from address-only wire format. +/// +/// Returns the decoded coordinate and the number of bytes consumed. +pub(crate) fn decode_coords(data: &[u8]) -> Result<(TreeCoordinate, usize), Error> { + if data.len() < 2 { + return Err(Error::MessageTooShort { + expected: 2, + got: data.len(), + }); + } + let count = u16::from_le_bytes([data[0], data[1]]) as usize; + let needed = 2 + count * 16; + if data.len() < needed { + return Err(Error::MessageTooShort { + expected: needed, + got: data.len(), + }); + } + if count == 0 { + return Err(Error::Malformed("coordinate with zero entries".into())); + } + let mut addrs = Vec::with_capacity(count); + for i in 0..count { + let offset = 2 + i * 16; + let mut bytes = [0u8; 16]; + bytes.copy_from_slice(&data[offset..offset + 16]); + addrs.push(NodeAddr::from_bytes(bytes)); + } + let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; + Ok((coord, needed)) +} + +/// Decode an optional coordinate field (count may be 0). +/// +/// Returns None if count is 0, Some(coord) otherwise, plus bytes consumed. +pub(crate) fn decode_optional_coords( + data: &[u8], +) -> Result<(Option, usize), Error> { + if data.len() < 2 { + return Err(Error::MessageTooShort { + expected: 2, + got: data.len(), + }); + } + let count = u16::from_le_bytes([data[0], data[1]]) as usize; + let needed = 2 + count * 16; + if data.len() < needed { + return Err(Error::MessageTooShort { + expected: needed, + got: data.len(), + }); + } + if count == 0 { + return Ok((None, 2)); + } + let mut addrs = Vec::with_capacity(count); + for i in 0..count { + let offset = 2 + i * 16; + let mut bytes = [0u8; 16]; + bytes.copy_from_slice(&data[offset..offset + 16]); + addrs.push(NodeAddr::from_bytes(bytes)); + } + let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; + Ok((Some(coord), needed)) +} + +/// Encode a count of zero (for empty/absent coordinate fields). +pub(crate) fn encode_empty_coords(buf: &mut Vec) { + buf.extend_from_slice(&0u16.to_le_bytes()); +} diff --git a/src/proto/mod.rs b/src/proto/mod.rs index 7b0953b..6cb6281 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -7,6 +7,7 @@ mod error; pub use error::Error; pub(crate) mod bloom; +pub(crate) mod coord; pub(crate) mod discovery; pub(crate) mod fmp; pub(crate) mod fsp; diff --git a/src/proto/stp/declaration.rs b/src/proto/stp/declaration.rs new file mode 100644 index 0000000..9cb49d9 --- /dev/null +++ b/src/proto/stp/declaration.rs @@ -0,0 +1,144 @@ +//! Parent declaration: a node's signed announcement of its spanning-tree parent. + +use core::fmt; + +use crate::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, + /// 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/mod.rs b/src/proto/stp/mod.rs index d0fb51d..70d4af5 100644 --- a/src/proto/stp/mod.rs +++ b/src/proto/stp/mod.rs @@ -13,13 +13,14 @@ //! - `state.rs` — `TreeState` + `ParentDeclaration` data + the `&self` //! ranking/election methods (`evaluate_parent`, `should_be_root`, //! `find_next_hop`). -//! - `coordinate.rs` — `TreeCoordinate` / `CoordEntry`. +//! - `TreeCoordinate` / `CoordEntry` now live in the shared +//! [`crate::proto::coord`] primitive and are re-exported here for continuity. //! - `limits.rs` — the flap-dampening / hold-down state machine. //! - `wire.rs` — `TreeAnnounce` + `validate_semantics` (the one std-tethered //! file). -mod coordinate; mod core; +mod declaration; mod limits; mod state; mod wire; @@ -31,13 +32,14 @@ use thiserror::Error; use crate::{IdentityError, NodeAddr}; -pub use coordinate::{CoordEntry, TreeCoordinate}; -pub(crate) use core::{ParentEval, Stp, TreeDecision}; -pub use state::{ParentDeclaration, TreeState}; -pub use wire::TreeAnnounce; -pub(crate) use wire::{ +pub use crate::proto::coord::{CoordEntry, TreeCoordinate}; +pub(crate) use crate::proto::coord::{ coords_wire_size, decode_coords, decode_optional_coords, encode_coords, encode_empty_coords, }; +pub(crate) use core::{ParentEval, Stp, TreeDecision}; +pub use declaration::ParentDeclaration; +pub use state::TreeState; +pub use wire::TreeAnnounce; /// Errors related to spanning tree operations. #[derive(Debug, Error)] diff --git a/src/proto/stp/state.rs b/src/proto/stp/state.rs index 9d6aeba..fad3481 100644 --- a/src/proto/stp/state.rs +++ b/src/proto/stp/state.rs @@ -5,7 +5,7 @@ use core::fmt; use super::core::ParentEval; use super::limits::FlapDampener; -use super::{CoordEntry, TreeCoordinate}; +use super::{CoordEntry, ParentDeclaration, TreeCoordinate}; use crate::NodeAddr; /// Local spanning tree state for a node. @@ -547,142 +547,3 @@ 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/mod.rs b/src/proto/stp/tests/mod.rs index 15de4ca..7119999 100644 --- a/src/proto/stp/tests/mod.rs +++ b/src/proto/stp/tests/mod.rs @@ -4,3 +4,4 @@ mod coordinate; mod limits; mod state; mod util; +mod wire; diff --git a/src/proto/stp/tests/wire.rs b/src/proto/stp/tests/wire.rs new file mode 100644 index 0000000..559e9f5 --- /dev/null +++ b/src/proto/stp/tests/wire.rs @@ -0,0 +1,336 @@ +//! TreeAnnounce wire codec unit tests. + +use super::util::{make_coords, make_node_addr}; +use crate::identity::Identity; +use crate::proto::Error; +use crate::proto::stp::wire::TreeAnnounce; +use crate::proto::stp::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError}; + +/// 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()); +} + +#[test] +fn test_tree_announce() { + let node = make_node_addr(1); + let parent = make_node_addr(2); + let decl = ParentDeclaration::new(node, parent, 1, 1000); + let ancestry = make_coords(&[1, 2, 0]); + + let announce = TreeAnnounce::new(decl, ancestry); + + assert_eq!(announce.declaration.node_addr(), &node); + assert_eq!(announce.ancestry.depth(), 2); +} + +#[test] +fn test_tree_announce_encode_decode_root() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + + // Root declaration: parent == self + let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 5000); + sign_decl(&mut decl, &identity); + + // Root ancestry: just the root itself + let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 5000)]).unwrap(); + + let announce = TreeAnnounce::new(decl, ancestry); + let encoded = announce.encode().unwrap(); + + // msg_type (1) + version (1) + seq (8) + ts (8) + parent (16) + count (2) + 1 entry (32) + sig (64) = 132 + assert_eq!(encoded.len(), 132); + assert_eq!(encoded[0], 0x10); // LinkMessageType::TreeAnnounce + + // Decode strips msg_type byte (as dispatcher does) + let decoded = TreeAnnounce::decode(&encoded[1..]).unwrap(); + + assert_eq!(decoded.declaration.node_addr(), &node_addr); + assert_eq!(decoded.declaration.parent_id(), &node_addr); + assert_eq!(decoded.declaration.sequence(), 1); + assert_eq!(decoded.declaration.timestamp(), 5000); + assert!(decoded.declaration.is_root()); + assert!(decoded.declaration.is_signed()); + assert_eq!(decoded.ancestry.depth(), 0); // root has depth 0 + assert_eq!(decoded.ancestry.entries().len(), 1); + assert_eq!(decoded.ancestry.entries()[0].node_addr, node_addr); + assert_eq!(decoded.ancestry.entries()[0].sequence, 1); + assert_eq!(decoded.ancestry.entries()[0].timestamp, 5000); +} + +#[test] +fn test_tree_announce_encode_decode_depth3() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + let parent = make_node_addr(2); + let grandparent = make_node_addr(3); + let root = make_node_addr(4); + + let mut decl = ParentDeclaration::new(node_addr, parent, 5, 10000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![ + CoordEntry::new(node_addr, 5, 10000), + CoordEntry::new(parent, 4, 9000), + CoordEntry::new(grandparent, 3, 8000), + CoordEntry::new(root, 2, 7000), + ]) + .unwrap(); + + let announce = TreeAnnounce::new(decl, ancestry); + let encoded = announce.encode().unwrap(); + + // 1 + 99 + 4*32 = 228 + assert_eq!(encoded.len(), 228); + + let decoded = TreeAnnounce::decode(&encoded[1..]).unwrap(); + + assert_eq!(decoded.declaration.node_addr(), &node_addr); + assert_eq!(decoded.declaration.parent_id(), &parent); + assert_eq!(decoded.declaration.sequence(), 5); + assert_eq!(decoded.declaration.timestamp(), 10000); + assert!(!decoded.declaration.is_root()); + assert_eq!(decoded.ancestry.depth(), 3); + assert_eq!(decoded.ancestry.entries().len(), 4); + + // Verify all entries preserved + let entries = decoded.ancestry.entries(); + assert_eq!(entries[0].node_addr, node_addr); + assert_eq!(entries[0].sequence, 5); + assert_eq!(entries[1].node_addr, parent); + assert_eq!(entries[1].sequence, 4); + assert_eq!(entries[2].node_addr, grandparent); + assert_eq!(entries[2].timestamp, 8000); + assert_eq!(entries[3].node_addr, root); + assert_eq!(entries[3].timestamp, 7000); + + // Root ID is last entry + assert_eq!(decoded.ancestry.root_id(), &root); +} + +#[test] +fn test_tree_announce_decode_unsupported_version() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + + let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 1000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 1000)]).unwrap(); + let announce = TreeAnnounce::new(decl, ancestry); + let mut encoded = announce.encode().unwrap(); + + // Corrupt version byte (byte index 1, after msg_type) + encoded[1] = 0xFF; + + let result = TreeAnnounce::decode(&encoded[1..]); + assert!(matches!(result, Err(Error::UnsupportedVersion(0xFF)))); +} + +#[test] +fn test_tree_announce_decode_truncated() { + // Way too short + let result = TreeAnnounce::decode(&[0x01]); + assert!(matches!( + result, + Err(Error::MessageTooShort { expected: 99, .. }) + )); + + // Just under minimum (98 bytes) + let short = vec![0u8; 98]; + let result = TreeAnnounce::decode(&short); + assert!(matches!( + result, + Err(Error::MessageTooShort { expected: 99, .. }) + )); +} + +#[test] +fn test_tree_announce_decode_ancestry_count_mismatch() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + + let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 1000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 1000)]).unwrap(); + let announce = TreeAnnounce::new(decl, ancestry); + let mut encoded = announce.encode().unwrap(); + + // The ancestry_count is at offset: 1 (msg_type) + 1 (version) + 8 (seq) + 8 (ts) + 16 (parent) = 34 + // Set ancestry_count to 5 but we only have 1 entry's worth of data + encoded[34] = 5; + encoded[35] = 0; + + let result = TreeAnnounce::decode(&encoded[1..]); + assert!(matches!(result, Err(Error::MessageTooShort { .. }))); +} + +#[test] +fn test_tree_announce_encode_unsigned_fails() { + let node = make_node_addr(1); + let decl = ParentDeclaration::new(node, node, 1, 1000); + let ancestry = make_coords(&[1, 0]); + + let announce = TreeAnnounce::new(decl, ancestry); + let result = announce.encode(); + assert!(matches!(result, Err(Error::InvalidSignature))); +} + +/// Tests that a well-formed non-root ancestry is accepted. +#[test] +fn test_tree_announce_validate_semantics_accepts_valid_non_root() { + use crate::identity::Identity; + + // Regenerate until the random identity's node_addr is numerically + // larger than both fixed parent (02:..) and root (01:..), so the + // root-minimum invariant holds deterministically. + let identity = loop { + let id = Identity::generate(); + if id.node_addr().as_bytes()[0] > 1 { + break id; + } + }; + let node_addr = *identity.node_addr(); + let parent = make_node_addr(2); + let root = make_node_addr(1); + + let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![ + CoordEntry::new(node_addr, 5, 1000), + CoordEntry::new(parent, 4, 900), + CoordEntry::new(root, 3, 800), + ]) + .unwrap(); + + let announce = TreeAnnounce::new(decl, ancestry); + assert!(announce.validate_semantics().is_ok()); +} + +/// Tests that an ancestry is rejected if the final node_addr is not the smallest entry in the path. +#[test] +fn test_tree_announce_validate_semantics_rejects_non_minimal_root() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + let smaller = make_node_addr(0); + let advertised_root = make_node_addr(1); + + let mut decl = ParentDeclaration::new(node_addr, smaller, 5, 1000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![ + CoordEntry::new(node_addr, 5, 1000), + CoordEntry::new(smaller, 4, 900), + CoordEntry::new(advertised_root, 3, 800), + ]) + .unwrap(); + + let announce = TreeAnnounce::new(decl, ancestry); + assert!(matches!( + announce.validate_semantics(), + Err(TreeError::AncestryRootNotMinimum { + advertised, + minimum, + }) if advertised == advertised_root && minimum == smaller + )); +} + +/// Tests that an ancestry is rejected if the first ancestry hop does not match the signed parent_id. +#[test] +fn test_tree_announce_validate_semantics_rejects_parent_mismatch() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + let declared_parent = make_node_addr(2); + let ancestry_parent = make_node_addr(3); + + let mut decl = ParentDeclaration::new(node_addr, declared_parent, 5, 1000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![ + CoordEntry::new(node_addr, 5, 1000), + CoordEntry::new(ancestry_parent, 4, 900), + CoordEntry::new(make_node_addr(1), 3, 800), + ]) + .unwrap(); + + let announce = TreeAnnounce::new(decl, ancestry); + assert!(matches!( + announce.validate_semantics(), + Err(TreeError::AncestryParentMismatch { + declared, + ancestry, + }) if declared == declared_parent && ancestry == ancestry_parent + )); +} + +/// Tests that an ancestry is rejected if the first path entry does not match the signed sender node_addr. +#[test] +fn test_tree_announce_validate_semantics_rejects_sender_mismatch() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + let ancestry_sender = make_node_addr(9); + let parent = make_node_addr(2); + + let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![ + CoordEntry::new(ancestry_sender, 5, 1000), + CoordEntry::new(parent, 4, 900), + CoordEntry::new(make_node_addr(1), 3, 800), + ]) + .unwrap(); + + let announce = TreeAnnounce::new(decl, ancestry); + assert!(matches!( + announce.validate_semantics(), + Err(TreeError::AncestryNodeMismatch { + declared, + ancestry, + }) if declared == node_addr && ancestry == ancestry_sender + )); +} + +/// Tests that a self-root declaration is rejected if its ancestry contains extra ancestors. +#[test] +fn test_tree_announce_validate_semantics_rejects_root_with_ancestors() { + use crate::identity::Identity; + + let identity = Identity::generate(); + let node_addr = *identity.node_addr(); + + let mut decl = ParentDeclaration::self_root(node_addr, 5, 1000); + sign_decl(&mut decl, &identity); + + let ancestry = TreeCoordinate::new(vec![ + CoordEntry::new(node_addr, 5, 1000), + CoordEntry::new(make_node_addr(0), 4, 900), + ]) + .unwrap(); + + let announce = TreeAnnounce::new(decl, ancestry); + assert!(matches!( + announce.validate_semantics(), + Err(TreeError::RootDeclarationMismatch) + )); +} diff --git a/src/proto/stp/wire.rs b/src/proto/stp/wire.rs index 984b7df..fce4797 100644 --- a/src/proto/stp/wire.rs +++ b/src/proto/stp/wire.rs @@ -244,440 +244,3 @@ impl TreeAnnounce { }) } } -// ============================================================================ -// Coordinate Wire Format Helpers -// ============================================================================ - -/// Wire size of a TreeCoordinate in address-only format: 2 + entries × 16. -pub(crate) fn coords_wire_size(coords: &TreeCoordinate) -> usize { - 2 + coords.entries().len() * 16 -} - -/// Encode a TreeCoordinate as address-only wire format: count(u16 LE) + addrs(16 × n). -/// -/// Session-layer messages serialize coordinates as NodeAddr arrays (16 bytes each), -/// without the sequence/timestamp metadata used by the tree gossip protocol. -pub(crate) fn encode_coords(coords: &TreeCoordinate, buf: &mut Vec) { - let addrs: Vec<&NodeAddr> = coords.node_addrs().collect(); - let count = addrs.len() as u16; - buf.extend_from_slice(&count.to_le_bytes()); - for addr in addrs { - buf.extend_from_slice(addr.as_bytes()); - } -} - -/// Decode a TreeCoordinate from address-only wire format. -/// -/// Returns the decoded coordinate and the number of bytes consumed. -pub(crate) fn decode_coords(data: &[u8]) -> Result<(TreeCoordinate, usize), Error> { - if data.len() < 2 { - return Err(Error::MessageTooShort { - expected: 2, - got: data.len(), - }); - } - let count = u16::from_le_bytes([data[0], data[1]]) as usize; - let needed = 2 + count * 16; - if data.len() < needed { - return Err(Error::MessageTooShort { - expected: needed, - got: data.len(), - }); - } - if count == 0 { - return Err(Error::Malformed("coordinate with zero entries".into())); - } - let mut addrs = Vec::with_capacity(count); - for i in 0..count { - let offset = 2 + i * 16; - let mut bytes = [0u8; 16]; - bytes.copy_from_slice(&data[offset..offset + 16]); - addrs.push(NodeAddr::from_bytes(bytes)); - } - let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; - Ok((coord, needed)) -} - -/// Decode an optional coordinate field (count may be 0). -/// -/// Returns None if count is 0, Some(coord) otherwise, plus bytes consumed. -pub(crate) fn decode_optional_coords( - data: &[u8], -) -> Result<(Option, usize), Error> { - if data.len() < 2 { - return Err(Error::MessageTooShort { - expected: 2, - got: data.len(), - }); - } - let count = u16::from_le_bytes([data[0], data[1]]) as usize; - let needed = 2 + count * 16; - if data.len() < needed { - return Err(Error::MessageTooShort { - expected: needed, - got: data.len(), - }); - } - if count == 0 { - return Ok((None, 2)); - } - let mut addrs = Vec::with_capacity(count); - for i in 0..count { - let offset = 2 + i * 16; - let mut bytes = [0u8; 16]; - bytes.copy_from_slice(&data[offset..offset + 16]); - addrs.push(NodeAddr::from_bytes(bytes)); - } - let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; - Ok((Some(coord), needed)) -} - -/// Encode a count of zero (for empty/absent coordinate fields). -pub(crate) fn encode_empty_coords(buf: &mut Vec) { - buf.extend_from_slice(&0u16.to_le_bytes()); -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::identity::Identity; - - fn make_node_addr(val: u8) -> NodeAddr { - let mut bytes = [0u8; 16]; - bytes[0] = val; - 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() - } - - #[test] - fn test_tree_announce() { - let node = make_node_addr(1); - let parent = make_node_addr(2); - let decl = ParentDeclaration::new(node, parent, 1, 1000); - let ancestry = make_coords(&[1, 2, 0]); - - let announce = TreeAnnounce::new(decl, ancestry); - - assert_eq!(announce.declaration.node_addr(), &node); - assert_eq!(announce.ancestry.depth(), 2); - } - - #[test] - fn test_tree_announce_encode_decode_root() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - - // Root declaration: parent == self - let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 5000); - sign_decl(&mut decl, &identity); - - // Root ancestry: just the root itself - let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 5000)]).unwrap(); - - let announce = TreeAnnounce::new(decl, ancestry); - let encoded = announce.encode().unwrap(); - - // msg_type (1) + version (1) + seq (8) + ts (8) + parent (16) + count (2) + 1 entry (32) + sig (64) = 132 - assert_eq!(encoded.len(), 132); - assert_eq!(encoded[0], 0x10); // LinkMessageType::TreeAnnounce - - // Decode strips msg_type byte (as dispatcher does) - let decoded = TreeAnnounce::decode(&encoded[1..]).unwrap(); - - assert_eq!(decoded.declaration.node_addr(), &node_addr); - assert_eq!(decoded.declaration.parent_id(), &node_addr); - assert_eq!(decoded.declaration.sequence(), 1); - assert_eq!(decoded.declaration.timestamp(), 5000); - assert!(decoded.declaration.is_root()); - assert!(decoded.declaration.is_signed()); - assert_eq!(decoded.ancestry.depth(), 0); // root has depth 0 - assert_eq!(decoded.ancestry.entries().len(), 1); - assert_eq!(decoded.ancestry.entries()[0].node_addr, node_addr); - assert_eq!(decoded.ancestry.entries()[0].sequence, 1); - assert_eq!(decoded.ancestry.entries()[0].timestamp, 5000); - } - - #[test] - fn test_tree_announce_encode_decode_depth3() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - let parent = make_node_addr(2); - let grandparent = make_node_addr(3); - let root = make_node_addr(4); - - let mut decl = ParentDeclaration::new(node_addr, parent, 5, 10000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![ - CoordEntry::new(node_addr, 5, 10000), - CoordEntry::new(parent, 4, 9000), - CoordEntry::new(grandparent, 3, 8000), - CoordEntry::new(root, 2, 7000), - ]) - .unwrap(); - - let announce = TreeAnnounce::new(decl, ancestry); - let encoded = announce.encode().unwrap(); - - // 1 + 99 + 4*32 = 228 - assert_eq!(encoded.len(), 228); - - let decoded = TreeAnnounce::decode(&encoded[1..]).unwrap(); - - assert_eq!(decoded.declaration.node_addr(), &node_addr); - assert_eq!(decoded.declaration.parent_id(), &parent); - assert_eq!(decoded.declaration.sequence(), 5); - assert_eq!(decoded.declaration.timestamp(), 10000); - assert!(!decoded.declaration.is_root()); - assert_eq!(decoded.ancestry.depth(), 3); - assert_eq!(decoded.ancestry.entries().len(), 4); - - // Verify all entries preserved - let entries = decoded.ancestry.entries(); - assert_eq!(entries[0].node_addr, node_addr); - assert_eq!(entries[0].sequence, 5); - assert_eq!(entries[1].node_addr, parent); - assert_eq!(entries[1].sequence, 4); - assert_eq!(entries[2].node_addr, grandparent); - assert_eq!(entries[2].timestamp, 8000); - assert_eq!(entries[3].node_addr, root); - assert_eq!(entries[3].timestamp, 7000); - - // Root ID is last entry - assert_eq!(decoded.ancestry.root_id(), &root); - } - - #[test] - fn test_tree_announce_decode_unsupported_version() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - - let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 1000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 1000)]).unwrap(); - let announce = TreeAnnounce::new(decl, ancestry); - let mut encoded = announce.encode().unwrap(); - - // Corrupt version byte (byte index 1, after msg_type) - encoded[1] = 0xFF; - - let result = TreeAnnounce::decode(&encoded[1..]); - assert!(matches!(result, Err(Error::UnsupportedVersion(0xFF)))); - } - - #[test] - fn test_tree_announce_decode_truncated() { - // Way too short - let result = TreeAnnounce::decode(&[0x01]); - assert!(matches!( - result, - Err(Error::MessageTooShort { expected: 99, .. }) - )); - - // Just under minimum (98 bytes) - let short = vec![0u8; 98]; - let result = TreeAnnounce::decode(&short); - assert!(matches!( - result, - Err(Error::MessageTooShort { expected: 99, .. }) - )); - } - - #[test] - fn test_tree_announce_decode_ancestry_count_mismatch() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - - let mut decl = ParentDeclaration::new(node_addr, node_addr, 1, 1000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![CoordEntry::new(node_addr, 1, 1000)]).unwrap(); - let announce = TreeAnnounce::new(decl, ancestry); - let mut encoded = announce.encode().unwrap(); - - // The ancestry_count is at offset: 1 (msg_type) + 1 (version) + 8 (seq) + 8 (ts) + 16 (parent) = 34 - // Set ancestry_count to 5 but we only have 1 entry's worth of data - encoded[34] = 5; - encoded[35] = 0; - - let result = TreeAnnounce::decode(&encoded[1..]); - assert!(matches!(result, Err(Error::MessageTooShort { .. }))); - } - - #[test] - fn test_tree_announce_encode_unsigned_fails() { - let node = make_node_addr(1); - let decl = ParentDeclaration::new(node, node, 1, 1000); - let ancestry = make_coords(&[1, 0]); - - let announce = TreeAnnounce::new(decl, ancestry); - let result = announce.encode(); - assert!(matches!(result, Err(Error::InvalidSignature))); - } - - /// Tests that a well-formed non-root ancestry is accepted. - #[test] - fn test_tree_announce_validate_semantics_accepts_valid_non_root() { - use crate::identity::Identity; - - // Regenerate until the random identity's node_addr is numerically - // larger than both fixed parent (02:..) and root (01:..), so the - // root-minimum invariant holds deterministically. - let identity = loop { - let id = Identity::generate(); - if id.node_addr().as_bytes()[0] > 1 { - break id; - } - }; - let node_addr = *identity.node_addr(); - let parent = make_node_addr(2); - let root = make_node_addr(1); - - let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![ - CoordEntry::new(node_addr, 5, 1000), - CoordEntry::new(parent, 4, 900), - CoordEntry::new(root, 3, 800), - ]) - .unwrap(); - - let announce = TreeAnnounce::new(decl, ancestry); - assert!(announce.validate_semantics().is_ok()); - } - - /// Tests that an ancestry is rejected if the final node_addr is not the smallest entry in the path. - #[test] - fn test_tree_announce_validate_semantics_rejects_non_minimal_root() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - let smaller = make_node_addr(0); - let advertised_root = make_node_addr(1); - - let mut decl = ParentDeclaration::new(node_addr, smaller, 5, 1000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![ - CoordEntry::new(node_addr, 5, 1000), - CoordEntry::new(smaller, 4, 900), - CoordEntry::new(advertised_root, 3, 800), - ]) - .unwrap(); - - let announce = TreeAnnounce::new(decl, ancestry); - assert!(matches!( - announce.validate_semantics(), - Err(TreeError::AncestryRootNotMinimum { - advertised, - minimum, - }) if advertised == advertised_root && minimum == smaller - )); - } - - /// Tests that an ancestry is rejected if the first ancestry hop does not match the signed parent_id. - #[test] - fn test_tree_announce_validate_semantics_rejects_parent_mismatch() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - let declared_parent = make_node_addr(2); - let ancestry_parent = make_node_addr(3); - - let mut decl = ParentDeclaration::new(node_addr, declared_parent, 5, 1000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![ - CoordEntry::new(node_addr, 5, 1000), - CoordEntry::new(ancestry_parent, 4, 900), - CoordEntry::new(make_node_addr(1), 3, 800), - ]) - .unwrap(); - - let announce = TreeAnnounce::new(decl, ancestry); - assert!(matches!( - announce.validate_semantics(), - Err(TreeError::AncestryParentMismatch { - declared, - ancestry, - }) if declared == declared_parent && ancestry == ancestry_parent - )); - } - - /// Tests that an ancestry is rejected if the first path entry does not match the signed sender node_addr. - #[test] - fn test_tree_announce_validate_semantics_rejects_sender_mismatch() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - let ancestry_sender = make_node_addr(9); - let parent = make_node_addr(2); - - let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![ - CoordEntry::new(ancestry_sender, 5, 1000), - CoordEntry::new(parent, 4, 900), - CoordEntry::new(make_node_addr(1), 3, 800), - ]) - .unwrap(); - - let announce = TreeAnnounce::new(decl, ancestry); - assert!(matches!( - announce.validate_semantics(), - Err(TreeError::AncestryNodeMismatch { - declared, - ancestry, - }) if declared == node_addr && ancestry == ancestry_sender - )); - } - - /// Tests that a self-root declaration is rejected if its ancestry contains extra ancestors. - #[test] - fn test_tree_announce_validate_semantics_rejects_root_with_ancestors() { - use crate::identity::Identity; - - let identity = Identity::generate(); - let node_addr = *identity.node_addr(); - - let mut decl = ParentDeclaration::self_root(node_addr, 5, 1000); - sign_decl(&mut decl, &identity); - - let ancestry = TreeCoordinate::new(vec![ - CoordEntry::new(node_addr, 5, 1000), - CoordEntry::new(make_node_addr(0), 4, 900), - ]) - .unwrap(); - - let announce = TreeAnnounce::new(decl, ancestry); - assert!(matches!( - announce.validate_semantics(), - Err(TreeError::RootDeclarationMismatch) - )); - } -} From 9697026c81917c92816acc8056ba95d5cb25937c Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 15:51:24 +0000 Subject: [PATCH 6/9] proto: share a per-address rate limiter and backoff helper across subsystems Hoist the two line-for-line-identical per-destination minimum-interval limiters (discovery forward, routing error) into a shared PerAddrRateLimiter, and fold the duplicated exponential-backoff math (discovery originator, FMP retry) into a shared backoff_ms helper, both in a new proto/rate_limit module. The subsystem limiters become thin delegating newtypes so should_forward/should_send and the backoff call conventions are preserved. Behavior is byte-identical. --- src/proto/discovery/limits.rs | 52 ++++++++++-------------------- src/proto/fmp/limits.rs | 5 +-- src/proto/mod.rs | 1 + src/proto/rate_limit.rs | 60 +++++++++++++++++++++++++++++++++++ src/proto/routing/limits.rs | 40 +++++------------------ 5 files changed, 87 insertions(+), 71 deletions(-) create mode 100644 src/proto/rate_limit.rs diff --git a/src/proto/discovery/limits.rs b/src/proto/discovery/limits.rs index c3e9dc6..b92a4b1 100644 --- a/src/proto/discovery/limits.rs +++ b/src/proto/discovery/limits.rs @@ -14,6 +14,7 @@ //! nodes generating fresh request_ids at high rate. use crate::NodeAddr; +use crate::proto::rate_limit::PerAddrRateLimiter; use alloc::collections::BTreeMap; // ============================================================================ @@ -34,9 +35,6 @@ const DEFAULT_BACKOFF_BASE_SECS: u64 = 0; /// Default maximum backoff cap. `0` = disabled. const DEFAULT_BACKOFF_MAX_SECS: u64 = 0; -/// Backoff multiplier per consecutive failure. -const BACKOFF_MULTIPLIER: u64 = 2; - /// Exponential backoff for failed discovery lookups. /// /// Tracks targets whose lookups have timed out and suppresses @@ -91,10 +89,11 @@ impl DiscoveryBackoff { pub fn record_failure(&mut self, target: &NodeAddr, now_ms: u64) { let failures = self.entries.get(target).map_or(0, |e| e.failures) + 1; - let backoff_ms = self - .base_ms - .saturating_mul(BACKOFF_MULTIPLIER.saturating_pow(failures.saturating_sub(1))) - .min(self.max_ms); + let backoff_ms = crate::proto::rate_limit::backoff_ms( + failures.saturating_sub(1), + self.base_ms, + self.max_ms, + ); self.entries.insert( *target, @@ -160,29 +159,20 @@ const FORWARD_MAX_AGE_MS: u64 = 60_000; /// Tracks the last time a LookupRequest was forwarded for each target /// and enforces a minimum interval to prevent floods from misbehaving /// nodes generating fresh request_ids. -pub struct DiscoveryForwardRateLimiter { - last_forwarded: BTreeMap, - min_interval_ms: u64, - max_age_ms: u64, -} +pub struct DiscoveryForwardRateLimiter(PerAddrRateLimiter); impl DiscoveryForwardRateLimiter { /// Create with default parameters (2s interval). pub fn new() -> Self { - Self { - last_forwarded: BTreeMap::new(), - min_interval_ms: DEFAULT_FORWARD_MIN_INTERVAL_MS, - max_age_ms: FORWARD_MAX_AGE_MS, - } + Self(PerAddrRateLimiter::new( + DEFAULT_FORWARD_MIN_INTERVAL_MS, + FORWARD_MAX_AGE_MS, + )) } /// Create with a custom minimum interval in milliseconds. pub fn with_interval_ms(min_interval_ms: u64) -> Self { - Self { - last_forwarded: BTreeMap::new(), - min_interval_ms, - max_age_ms: FORWARD_MAX_AGE_MS, - } + Self(PerAddrRateLimiter::new(min_interval_ms, FORWARD_MAX_AGE_MS)) } /// Check if we should forward a lookup for this target. @@ -190,32 +180,24 @@ impl DiscoveryForwardRateLimiter { /// Returns true if enough time has passed since the last forward /// for this target. Updates internal state when returning true. pub fn should_forward(&mut self, target: &NodeAddr, now_ms: u64) -> bool { - if let Some(&last) = self.last_forwarded.get(target) - && now_ms.saturating_sub(last) < self.min_interval_ms - { - return false; - } - - self.last_forwarded.insert(*target, now_ms); - self.cleanup(now_ms); - true + self.0.check_and_record(target, now_ms) } /// Replace the minimum interval in milliseconds (e.g., set to zero to disable). #[cfg(test)] pub fn set_interval_ms(&mut self, interval_ms: u64) { - self.min_interval_ms = interval_ms; + self.0.set_interval_ms(interval_ms); } /// Remove entries older than max_age. + #[cfg(test)] pub(crate) fn cleanup(&mut self, now_ms: u64) { - self.last_forwarded - .retain(|_, &mut last| now_ms.saturating_sub(last) < self.max_age_ms); + self.0.cleanup(now_ms); } #[cfg(test)] pub fn len(&self) -> usize { - self.last_forwarded.len() + self.0.len() } } diff --git a/src/proto/fmp/limits.rs b/src/proto/fmp/limits.rs index 24407ad..f304d2b 100644 --- a/src/proto/fmp/limits.rs +++ b/src/proto/fmp/limits.rs @@ -9,8 +9,5 @@ /// Uses exponential backoff: `base_interval_ms * 2^retry_count`, capped at /// `max_backoff_ms`. pub(crate) fn backoff_ms(retry_count: u32, base_interval_ms: u64, max_backoff_ms: u64) -> u64 { - let multiplier = 1u64.checked_shl(retry_count).unwrap_or(u64::MAX); - base_interval_ms - .saturating_mul(multiplier) - .min(max_backoff_ms) + crate::proto::rate_limit::backoff_ms(retry_count, base_interval_ms, max_backoff_ms) } diff --git a/src/proto/mod.rs b/src/proto/mod.rs index 6cb6281..e5fb961 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -13,5 +13,6 @@ pub(crate) mod fmp; pub(crate) mod fsp; pub(crate) mod link; pub(crate) mod mmp; +pub(crate) mod rate_limit; pub(crate) mod routing; pub(crate) mod stp; diff --git a/src/proto/rate_limit.rs b/src/proto/rate_limit.rs new file mode 100644 index 0000000..2eeb83a --- /dev/null +++ b/src/proto/rate_limit.rs @@ -0,0 +1,60 @@ +//! Shared `proto` rate-limiting / backoff primitives: a per-address +//! minimum-interval limiter and an exponential backoff helper, hoisted out of +//! the subsystem `limits.rs` files. + +use crate::NodeAddr; +use alloc::collections::BTreeMap; + +/// Per-address minimum-interval rate limiter. Tracks the last event time per +/// address and enforces a minimum interval, evicting entries older than a max age. +pub(crate) struct PerAddrRateLimiter { + last: BTreeMap, + min_interval_ms: u64, + max_age_ms: u64, +} + +impl PerAddrRateLimiter { + pub(crate) fn new(min_interval_ms: u64, max_age_ms: u64) -> Self { + Self { + last: BTreeMap::new(), + min_interval_ms, + max_age_ms, + } + } + + /// Returns true (and records `now_ms`) if enough time has elapsed since the + /// last event for `addr`, or this is the first; false if within the interval. + pub(crate) fn check_and_record(&mut self, addr: &NodeAddr, now_ms: u64) -> bool { + if let Some(&last) = self.last.get(addr) + && now_ms.saturating_sub(last) < self.min_interval_ms + { + return false; + } + self.last.insert(*addr, now_ms); + self.cleanup(now_ms); + true + } + + pub(crate) fn cleanup(&mut self, now_ms: u64) { + self.last + .retain(|_, &mut last| now_ms.saturating_sub(last) < self.max_age_ms); + } + + #[cfg(test)] + pub(crate) fn set_interval_ms(&mut self, interval_ms: u64) { + self.min_interval_ms = interval_ms; + } + + #[cfg(test)] + pub(crate) fn len(&self) -> usize { + self.last.len() + } +} + +/// Exponential (base-2) backoff: `base_ms * 2^exponent`, saturating, capped at +/// `cap_ms`. Shared by the discovery originator backoff (exponent = failures-1) +/// and the FMP retry scheduler (exponent = retry_count). +pub(crate) fn backoff_ms(exponent: u32, base_ms: u64, cap_ms: u64) -> u64 { + let multiplier = 1u64.checked_shl(exponent).unwrap_or(u64::MAX); + base_ms.saturating_mul(multiplier).min(cap_ms) +} diff --git a/src/proto/routing/limits.rs b/src/proto/routing/limits.rs index d16cdfb..01762d1 100644 --- a/src/proto/routing/limits.rs +++ b/src/proto/routing/limits.rs @@ -9,7 +9,7 @@ //! portability and deterministic ordering. use crate::NodeAddr; -use alloc::collections::BTreeMap; +use crate::proto::rate_limit::PerAddrRateLimiter; /// Default minimum interval between error signals: 100 ms (max 10 errors/sec /// per destination). @@ -22,35 +22,19 @@ const MAX_AGE_MS: u64 = 10_000; /// /// Tracks the last time a routing error was sent for each destination /// address and enforces a minimum interval to prevent floods. -pub struct RoutingErrorRateLimiter { - /// Maps destination NodeAddr to the last time (Unix ms) we sent an error - /// about it. - last_sent: BTreeMap, - /// Minimum interval between error signals for the same destination (ms). - min_interval_ms: u64, - /// Maximum age of entries before cleanup (ms). - max_age_ms: u64, -} +pub struct RoutingErrorRateLimiter(PerAddrRateLimiter); impl RoutingErrorRateLimiter { /// Create a new rate limiter. /// /// Default: max 10 errors/sec per destination (100ms interval). pub fn new() -> Self { - Self { - last_sent: BTreeMap::new(), - min_interval_ms: DEFAULT_MIN_INTERVAL_MS, - max_age_ms: MAX_AGE_MS, - } + Self(PerAddrRateLimiter::new(DEFAULT_MIN_INTERVAL_MS, MAX_AGE_MS)) } /// Create a rate limiter with a custom minimum interval in milliseconds. pub fn with_interval_ms(min_interval_ms: u64) -> Self { - Self { - last_sent: BTreeMap::new(), - min_interval_ms, - max_age_ms: MAX_AGE_MS, - } + Self(PerAddrRateLimiter::new(min_interval_ms, MAX_AGE_MS)) } /// Check if we should send a routing error for this destination at @@ -60,26 +44,18 @@ impl RoutingErrorRateLimiter { /// this destination, or if this is the first error. Updates internal /// state when returning true. pub fn should_send(&mut self, dest_addr: &NodeAddr, now_ms: u64) -> bool { - if let Some(&last) = self.last_sent.get(dest_addr) - && now_ms.saturating_sub(last) < self.min_interval_ms - { - return false; - } - - self.last_sent.insert(*dest_addr, now_ms); - self.cleanup(now_ms); - true + self.0.check_and_record(dest_addr, now_ms) } /// Remove entries older than max_age. + #[cfg(test)] pub(crate) fn cleanup(&mut self, now_ms: u64) { - self.last_sent - .retain(|_, &mut last| now_ms.saturating_sub(last) < self.max_age_ms); + self.0.cleanup(now_ms); } #[cfg(test)] pub fn len(&self) -> usize { - self.last_sent.len() + self.0.len() } } From 653873117667ef7ab4be23b6c9db87a2f02ce38b Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 16:13:53 +0000 Subject: [PATCH 7/9] proto: replace the string Malformed error with typed variants and drop thiserror Replace Malformed(String) with a no_std-clean set: Malformed(&static str) for the static decode diagnostics, BadSizeClass { got, max } for the two bloom size-class checks, and typed sources BadCoord(CoordError) / BadBloom(BloomError) for the coordinate and bloom construction failures. Add a dedicated CoordError so proto/coord no longer depends upward on stp TreeError, resolving the temporary inversion from the coordinate relocation. Drop the thiserror derive from the four proto error types (Error, TreeError, BloomError, CoordError) in favor of hand-rolled core::fmt::Display and core::error::Error impls. thiserror remains in use elsewhere in the crate. Wire decode decisions are unchanged; only the error type and its diagnostic text change. --- src/lib.rs | 4 +- src/proto/bloom/mod.rs | 36 ++++++++-- src/proto/bloom/tests/wire.rs | 23 +++---- src/proto/bloom/wire.rs | 26 ++++---- src/proto/coord.rs | 35 +++++++--- src/proto/discovery/wire.rs | 10 +-- src/proto/error.rs | 97 +++++++++++++++++++++++---- src/proto/stp/mod.rs | 107 ++++++++++++++++++++++++------ src/proto/stp/tests/coordinate.rs | 4 +- src/proto/stp/wire.rs | 23 +++---- 10 files changed, 263 insertions(+), 102 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index af83669..5d38a36 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,9 @@ pub use upper::config::{DnsConfig, TunConfig}; pub use discovery::{BootstrapHandoffResult, EstablishedTraversal}; // Re-export tree types (relocated from tree:: to proto::stp) -pub use proto::stp::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError, TreeState}; +pub use proto::stp::{ + CoordEntry, CoordError, ParentDeclaration, TreeCoordinate, TreeError, TreeState, +}; // Re-export bloom filter types (relocated from bloom:: to proto::bloom) pub use proto::bloom::{BloomError, BloomFilter, BloomState}; diff --git a/src/proto/bloom/mod.rs b/src/proto/bloom/mod.rs index 2c195ff..cf4332e 100644 --- a/src/proto/bloom/mod.rs +++ b/src/proto/bloom/mod.rs @@ -26,22 +26,44 @@ mod wire; #[cfg(test)] mod tests; -use thiserror::Error; - pub use core::BloomFilter; pub use limits::{DEFAULT_FILTER_SIZE_BITS, DEFAULT_HASH_COUNT, V1_SIZE_CLASS}; pub use state::BloomState; pub use wire::FilterAnnounce; /// Errors related to Bloom filter operations. -#[derive(Debug, Error)] +#[derive(Debug)] pub enum BloomError { - #[error("invalid filter size: expected {expected} bits, got {got}")] - InvalidSize { expected: usize, got: usize }, + /// Filter bit length did not match the expected size. + InvalidSize { + /// Expected number of bits. + expected: usize, + /// Number of bits received. + got: usize, + }, - #[error("filter size must be a multiple of 8, got {0}")] + /// Filter size was not a multiple of 8 bits. SizeNotByteAligned(usize), - #[error("hash count must be positive")] + /// Hash count was zero. ZeroHashCount, } + +impl ::core::fmt::Display for BloomError { + fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + match self { + BloomError::InvalidSize { expected, got } => { + write!( + f, + "invalid filter size: expected {expected} bits, got {got}" + ) + } + BloomError::SizeNotByteAligned(n) => { + write!(f, "filter size must be a multiple of 8, got {n}") + } + BloomError::ZeroHashCount => write!(f, "hash count must be positive"), + } + } +} + +impl ::core::error::Error for BloomError {} diff --git a/src/proto/bloom/tests/wire.rs b/src/proto/bloom/tests/wire.rs index 8bf4f3d..91cc597 100644 --- a/src/proto/bloom/tests/wire.rs +++ b/src/proto/bloom/tests/wire.rs @@ -1,5 +1,6 @@ //! Tests for the bloom wire codec (`FilterAnnounce`). +use crate::proto::Error; use crate::proto::bloom::BloomFilter; use crate::proto::bloom::FilterAnnounce; use crate::proto::link::LinkMessageType; @@ -66,13 +67,10 @@ fn test_filter_announce_decode_rejects_bad_size_class() { encoded[10] = 5; // invalid size_class > MAX_SIZE_CLASS let result = FilterAnnounce::decode(&encoded[1..]); - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("invalid size_class") - ); + assert!(matches!( + result, + Err(Error::BadSizeClass { got: 5, max: 3 }) + )); } #[test] @@ -83,13 +81,10 @@ fn test_filter_announce_decode_rejects_non_v1_size_class() { let encoded = announce.encode().unwrap(); let result = FilterAnnounce::decode(&encoded[1..]); - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("unsupported size_class") - ); + assert!(matches!( + result, + Err(Error::BadSizeClass { got: 0, max: 1 }) + )); } #[test] diff --git a/src/proto/bloom/wire.rs b/src/proto/bloom/wire.rs index e36136b..e180982 100644 --- a/src/proto/bloom/wire.rs +++ b/src/proto/bloom/wire.rs @@ -81,9 +81,7 @@ impl FilterAnnounce { /// ``` pub fn encode(&self) -> Result, Error> { if !self.is_valid() { - return Err(Error::Malformed( - "filter size does not match size_class".into(), - )); + return Err(Error::Malformed("filter size does not match size_class")); } let filter_bytes = self.filter.as_bytes(); @@ -121,7 +119,7 @@ impl FilterAnnounce { let sequence = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad sequence".into()))?, + .map_err(|_| Error::Malformed("bad sequence"))?, ); pos += 8; @@ -135,18 +133,18 @@ impl FilterAnnounce { // Validate size_class range if size_class > Self::MAX_SIZE_CLASS { - return Err(Error::Malformed(format!( - "invalid size_class: {size_class} (max {})", - Self::MAX_SIZE_CLASS - ))); + return Err(Error::BadSizeClass { + got: size_class, + max: Self::MAX_SIZE_CLASS, + }); } // v1 compliance check if size_class != super::V1_SIZE_CLASS { - return Err(Error::Malformed(format!( - "unsupported size_class: {size_class} (v1 requires {})", - super::V1_SIZE_CLASS - ))); + return Err(Error::BadSizeClass { + got: size_class, + max: super::V1_SIZE_CLASS, + }); } // Expected filter size from size_class @@ -160,8 +158,8 @@ impl FilterAnnounce { } // Construct BloomFilter from bytes - let filter = BloomFilter::from_slice(&payload[pos..], hash_count) - .map_err(|e| Error::Malformed(format!("invalid bloom filter: {e}")))?; + let filter = + BloomFilter::from_slice(&payload[pos..], hash_count).map_err(Error::BadBloom)?; let announce = Self { filter, diff --git a/src/proto/coord.rs b/src/proto/coord.rs index 64e04cf..341bdc4 100644 --- a/src/proto/coord.rs +++ b/src/proto/coord.rs @@ -6,10 +6,23 @@ use core::fmt; use crate::NodeAddr; use crate::proto::Error; -// TEMPORARY: coord depends upward on stp's TreeError here. This inversion is -// intentional and resolved in the next pass when a no_std-clean CoordError is -// created and TreeCoordinate is cut over to it. -use crate::proto::stp::TreeError; + +/// Errors from constructing a `TreeCoordinate`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CoordError { + /// Coordinate path had zero entries. + EmptyCoordinate, +} + +impl core::fmt::Display for CoordError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + CoordError::EmptyCoordinate => write!(f, "invalid tree coordinate: empty path"), + } + } +} + +impl core::error::Error for CoordError {} /// Metadata for a single node in a tree coordinate path. /// @@ -71,9 +84,9 @@ impl TreeCoordinate { /// Create a coordinate from a path of entries (self to root). /// /// The path must be non-empty and ordered from the node to the root. - pub fn new(path: Vec) -> Result { + pub fn new(path: Vec) -> Result { if path.is_empty() { - return Err(TreeError::EmptyCoordinate); + return Err(CoordError::EmptyCoordinate); } Ok(Self(path)) } @@ -82,9 +95,9 @@ impl TreeCoordinate { /// /// Convenience constructor for cases where only routing is needed. /// Each entry gets sequence=0, timestamp=0. - pub fn from_addrs(addrs: Vec) -> Result { + pub fn from_addrs(addrs: Vec) -> Result { if addrs.is_empty() { - return Err(TreeError::EmptyCoordinate); + return Err(CoordError::EmptyCoordinate); } Ok(Self(addrs.into_iter().map(CoordEntry::addr_only).collect())) } @@ -271,7 +284,7 @@ pub(crate) fn decode_coords(data: &[u8]) -> Result<(TreeCoordinate, usize), Erro }); } if count == 0 { - return Err(Error::Malformed("coordinate with zero entries".into())); + return Err(Error::Malformed("coordinate with zero entries")); } let mut addrs = Vec::with_capacity(count); for i in 0..count { @@ -280,7 +293,7 @@ pub(crate) fn decode_coords(data: &[u8]) -> Result<(TreeCoordinate, usize), Erro bytes.copy_from_slice(&data[offset..offset + 16]); addrs.push(NodeAddr::from_bytes(bytes)); } - let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; + let coord = TreeCoordinate::from_addrs(addrs).map_err(Error::BadCoord)?; Ok((coord, needed)) } @@ -314,7 +327,7 @@ pub(crate) fn decode_optional_coords( bytes.copy_from_slice(&data[offset..offset + 16]); addrs.push(NodeAddr::from_bytes(bytes)); } - let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; + let coord = TreeCoordinate::from_addrs(addrs).map_err(Error::BadCoord)?; Ok((Some(coord), needed)) } diff --git a/src/proto/discovery/wire.rs b/src/proto/discovery/wire.rs index e61806c..c717af4 100644 --- a/src/proto/discovery/wire.rs +++ b/src/proto/discovery/wire.rs @@ -98,7 +98,7 @@ impl LookupRequest { let request_id = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad request_id".into()))?, + .map_err(|_| Error::Malformed("bad request_id"))?, ); pos += 8; @@ -118,7 +118,7 @@ impl LookupRequest { let min_mtu = u16::from_le_bytes( payload[pos..pos + 2] .try_into() - .map_err(|_| Error::Malformed("bad min_mtu".into()))?, + .map_err(|_| Error::Malformed("bad min_mtu"))?, ); pos += 2; @@ -223,7 +223,7 @@ impl LookupResponse { let request_id = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad request_id".into()))?, + .map_err(|_| Error::Malformed("bad request_id"))?, ); pos += 8; @@ -235,7 +235,7 @@ impl LookupResponse { let path_mtu = u16::from_le_bytes( payload[pos..pos + 2] .try_into() - .map_err(|_| Error::Malformed("bad path_mtu".into()))?, + .map_err(|_| Error::Malformed("bad path_mtu"))?, ); pos += 2; @@ -249,7 +249,7 @@ impl LookupResponse { }); } let proof = Signature::from_slice(&payload[pos..pos + 64]) - .map_err(|_| Error::Malformed("bad proof signature".into()))?; + .map_err(|_| Error::Malformed("bad proof signature"))?; Ok(Self { request_id, diff --git a/src/proto/error.rs b/src/proto/error.rs index 04658cb..43f2c8c 100644 --- a/src/proto/error.rs +++ b/src/proto/error.rs @@ -1,31 +1,100 @@ //! Protocol error types. -use thiserror::Error; - /// Errors related to protocol message handling. -#[derive(Debug, Error)] +#[derive(Debug)] pub enum Error { - #[error("invalid message type: 0x{0:02x}")] + /// Message type byte was not recognized. InvalidMessageType(u8), - #[error("message too short: expected at least {expected}, got {got}")] - MessageTooShort { expected: usize, got: usize }, + /// Message was shorter than the minimum expected length. + MessageTooShort { + /// Minimum number of bytes expected. + expected: usize, + /// Number of bytes actually present. + got: usize, + }, - #[error("message too long: max {max}, got {got}")] - MessageTooLong { max: usize, got: usize }, + /// Message exceeded the maximum allowed length. + MessageTooLong { + /// Maximum number of bytes allowed. + max: usize, + /// Number of bytes actually present. + got: usize, + }, - #[error("invalid signature")] + /// Signature failed to parse or verify. InvalidSignature, - #[error("unsupported protocol version: {0}")] + /// Protocol version byte was not supported. UnsupportedVersion(u8), - #[error("malformed message: {0}")] - Malformed(String), + /// Message was structurally malformed; the string names the field. + Malformed(&'static str), - #[error("hop limit exceeded")] + /// Size class byte was out of range for this message. + BadSizeClass { + /// The size class value received. + got: u8, + /// The maximum (or required) size class. + max: u8, + }, + + /// A tree coordinate failed to construct during decode. + BadCoord(crate::proto::coord::CoordError), + + /// A bloom filter failed to construct during decode. + BadBloom(crate::proto::bloom::BloomError), + + /// Hop limit was exceeded while forwarding. HopLimitExceeded, - #[error("ttl expired")] + /// Time-to-live expired while forwarding. TtlExpired, } + +impl core::fmt::Display for Error { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Error::InvalidMessageType(t) => write!(f, "invalid message type: 0x{t:02x}"), + Error::MessageTooShort { expected, got } => { + write!( + f, + "message too short: expected at least {expected}, got {got}" + ) + } + Error::MessageTooLong { max, got } => { + write!(f, "message too long: max {max}, got {got}") + } + Error::InvalidSignature => write!(f, "invalid signature"), + Error::UnsupportedVersion(v) => write!(f, "unsupported protocol version: {v}"), + Error::Malformed(m) => write!(f, "malformed message: {m}"), + Error::BadSizeClass { got, max } => write!(f, "bad size class: {got} (max {max})"), + Error::BadCoord(e) => write!(f, "bad coordinate: {e}"), + Error::BadBloom(e) => write!(f, "bad bloom filter: {e}"), + Error::HopLimitExceeded => write!(f, "hop limit exceeded"), + Error::TtlExpired => write!(f, "ttl expired"), + } + } +} + +impl core::error::Error for Error { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Error::BadCoord(e) => Some(e), + Error::BadBloom(e) => Some(e), + _ => None, + } + } +} + +impl From for Error { + fn from(e: crate::proto::coord::CoordError) -> Self { + Error::BadCoord(e) + } +} + +impl From for Error { + fn from(e: crate::proto::bloom::BloomError) -> Self { + Error::BadBloom(e) + } +} diff --git a/src/proto/stp/mod.rs b/src/proto/stp/mod.rs index 70d4af5..2c679e2 100644 --- a/src/proto/stp/mod.rs +++ b/src/proto/stp/mod.rs @@ -28,11 +28,9 @@ mod wire; #[cfg(test)] mod tests; -use thiserror::Error; - use crate::{IdentityError, NodeAddr}; -pub use crate::proto::coord::{CoordEntry, TreeCoordinate}; +pub use crate::proto::coord::{CoordEntry, CoordError, TreeCoordinate}; pub(crate) use crate::proto::coord::{ coords_wire_size, decode_coords, decode_optional_coords, encode_coords, encode_empty_coords, }; @@ -42,51 +40,118 @@ pub use state::TreeState; pub use wire::TreeAnnounce; /// Errors related to spanning tree operations. -#[derive(Debug, Error)] +#[derive(Debug)] pub enum TreeError { - #[error("invalid tree coordinate: empty path")] + /// Coordinate path had zero entries. EmptyCoordinate, - #[error("invalid ancestry: does not reach claimed root")] + /// Ancestry path does not reach the claimed root. AncestryNotToRoot, - #[error("invalid ancestry: root declaration must contain only the sender")] + /// Root declaration contained hops other than the sender. RootDeclarationMismatch, - #[error("invalid ancestry: non-root declaration must include a parent hop")] + /// Non-root declaration was missing its parent hop. AncestryTooShort, - #[error("invalid ancestry: sender {declared} does not match first path entry {ancestry}")] + /// Declared sender does not match the first ancestry entry. AncestryNodeMismatch { + /// The declared sender address. declared: NodeAddr, + /// The first ancestry path entry. ancestry: NodeAddr, }, - #[error( - "invalid ancestry: signed parent {declared} does not match first ancestry hop {ancestry}" - )] + /// Signed parent does not match the first ancestry hop. AncestryParentMismatch { + /// The signed parent address. declared: NodeAddr, + /// The first ancestry hop. ancestry: NodeAddr, }, - #[error( - "invalid ancestry: advertised root {advertised} is not the minimum path entry {minimum}" - )] + /// Advertised root is not the minimum path entry. AncestryRootNotMinimum { + /// The advertised root address. advertised: NodeAddr, + /// The minimum path entry. minimum: NodeAddr, }, - #[error("signature verification failed for node {0:?}")] + /// Signature verification failed for the given node. InvalidSignature(NodeAddr), - #[error("sequence number regression: got {got}, expected > {expected}")] - SequenceRegression { got: u64, expected: u64 }, + /// Sequence number regressed below the expected value. + SequenceRegression { + /// The received sequence number. + got: u64, + /// The value the sequence had to exceed. + expected: u64, + }, - #[error("parent not in peers: {0:?}")] + /// Declared parent was not among the known peers. ParentNotPeer(NodeAddr), - #[error("identity error: {0}")] - Identity(#[from] IdentityError), + /// An identity operation failed. + Identity(IdentityError), +} + +impl ::core::fmt::Display for TreeError { + fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + match self { + TreeError::EmptyCoordinate => write!(f, "invalid tree coordinate: empty path"), + TreeError::AncestryNotToRoot => { + write!(f, "invalid ancestry: does not reach claimed root") + } + TreeError::RootDeclarationMismatch => write!( + f, + "invalid ancestry: root declaration must contain only the sender" + ), + TreeError::AncestryTooShort => write!( + f, + "invalid ancestry: non-root declaration must include a parent hop" + ), + TreeError::AncestryNodeMismatch { declared, ancestry } => write!( + f, + "invalid ancestry: sender {declared} does not match first path entry {ancestry}" + ), + TreeError::AncestryParentMismatch { declared, ancestry } => write!( + f, + "invalid ancestry: signed parent {declared} does not match first ancestry hop {ancestry}" + ), + TreeError::AncestryRootNotMinimum { + advertised, + minimum, + } => write!( + f, + "invalid ancestry: advertised root {advertised} is not the minimum path entry {minimum}" + ), + TreeError::InvalidSignature(node) => { + write!(f, "signature verification failed for node {node:?}") + } + TreeError::SequenceRegression { got, expected } => { + write!( + f, + "sequence number regression: got {got}, expected > {expected}" + ) + } + TreeError::ParentNotPeer(node) => write!(f, "parent not in peers: {node:?}"), + TreeError::Identity(e) => write!(f, "identity error: {e}"), + } + } +} + +impl ::core::error::Error for TreeError { + fn source(&self) -> Option<&(dyn ::core::error::Error + 'static)> { + match self { + TreeError::Identity(e) => Some(e), + _ => None, + } + } +} + +impl From for TreeError { + fn from(e: IdentityError) -> Self { + TreeError::Identity(e) + } } diff --git a/src/proto/stp/tests/coordinate.rs b/src/proto/stp/tests/coordinate.rs index 772fda2..0ae4e7a 100644 --- a/src/proto/stp/tests/coordinate.rs +++ b/src/proto/stp/tests/coordinate.rs @@ -1,7 +1,7 @@ //! TreeCoordinate unit tests. use super::util::{make_coords, make_node_addr}; -use crate::proto::stp::{CoordEntry, TreeCoordinate, TreeError}; +use crate::proto::stp::{CoordEntry, CoordError, TreeCoordinate}; #[test] fn test_tree_coordinate_root() { @@ -33,7 +33,7 @@ fn test_tree_coordinate_path() { #[test] fn test_tree_coordinate_empty_fails() { let result = TreeCoordinate::from_addrs(vec![]); - assert!(matches!(result, Err(TreeError::EmptyCoordinate))); + assert!(matches!(result, Err(CoordError::EmptyCoordinate))); } #[test] diff --git a/src/proto/stp/wire.rs b/src/proto/stp/wire.rs index fce4797..45a9efb 100644 --- a/src/proto/stp/wire.rs +++ b/src/proto/stp/wire.rs @@ -153,7 +153,7 @@ impl TreeAnnounce { let sequence = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad sequence".into()))?, + .map_err(|_| Error::Malformed("bad sequence"))?, ); pos += 8; @@ -161,7 +161,7 @@ impl TreeAnnounce { let timestamp = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad timestamp".into()))?, + .map_err(|_| Error::Malformed("bad timestamp"))?, ); pos += 8; @@ -169,7 +169,7 @@ impl TreeAnnounce { let parent = NodeAddr::from_bytes( payload[pos..pos + 16] .try_into() - .map_err(|_| Error::Malformed("bad parent".into()))?, + .map_err(|_| Error::Malformed("bad parent"))?, ); pos += 16; @@ -177,7 +177,7 @@ impl TreeAnnounce { let ancestry_count = u16::from_le_bytes( payload[pos..pos + 2] .try_into() - .map_err(|_| Error::Malformed("bad ancestry count".into()))?, + .map_err(|_| Error::Malformed("bad ancestry count"))?, ) as usize; pos += 2; @@ -196,19 +196,19 @@ impl TreeAnnounce { let node_addr = NodeAddr::from_bytes( payload[pos..pos + 16] .try_into() - .map_err(|_| Error::Malformed("bad entry node_addr".into()))?, + .map_err(|_| Error::Malformed("bad entry node_addr"))?, ); pos += 16; let entry_seq = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad entry sequence".into()))?, + .map_err(|_| Error::Malformed("bad entry sequence"))?, ); pos += 8; let entry_ts = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad entry timestamp".into()))?, + .map_err(|_| Error::Malformed("bad entry timestamp"))?, ); pos += 8; entries.push(CoordEntry::new(node_addr, entry_seq, entry_ts)); @@ -217,7 +217,7 @@ impl TreeAnnounce { // signature (64) let sig_bytes: [u8; 64] = payload[pos..pos + 64] .try_into() - .map_err(|_| Error::Malformed("bad signature".into()))?; + .map_err(|_| Error::Malformed("bad signature"))?; // 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 @@ -226,17 +226,14 @@ impl TreeAnnounce { // The first entry's node_addr is the declaring node if entries.is_empty() { - return Err(Error::Malformed( - "ancestry must have at least one entry".into(), - )); + return Err(Error::Malformed("ancestry must have at least one entry")); } let node_addr = entries[0].node_addr; let declaration = ParentDeclaration::with_signature(node_addr, parent, sequence, timestamp, sig_bytes); - let ancestry = TreeCoordinate::new(entries) - .map_err(|e| Error::Malformed(format!("bad ancestry: {}", e)))?; + let ancestry = TreeCoordinate::new(entries).map_err(Error::BadCoord)?; Ok(Self { declaration, From 5d13090d8f2e5f7d10bc0493a7fca5b1202a258a Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 16:37:17 +0000 Subject: [PATCH 8/9] proto: add a bounds-checked byte reader/writer and adopt it across the wire codecs Introduce a shared proto/codec module with a cursor Reader (short reads fail with MessageTooShort { expected: position + needed, got: total }) and an append Writer, and adopt them across the seven subsystem wire codecs, replacing the repetitive manual slicing, try_into, and from_le_bytes extraction. Each existing length check maps to the reader (an up-front minimum becomes require at position zero, so the per-field expected values, including the tree-announce expected 99, are reproduced exactly); the bloom exact-length check stays explicit since it also rejects over-long payloads. Encoded bytes and decode decisions are unchanged. --- src/proto/bloom/wire.rs | 47 +++---- src/proto/codec.rs | 109 +++++++++++++++ src/proto/discovery/wire.rs | 81 +++--------- src/proto/fmp/wire.rs | 11 +- src/proto/fsp/wire.rs | 119 +++++------------ src/proto/mmp/wire.rs | 257 +++++++++++++++++------------------- src/proto/mod.rs | 1 + src/proto/routing/wire.rs | 88 ++++++------ src/proto/stp/wire.rs | 98 ++++---------- 9 files changed, 365 insertions(+), 446 deletions(-) create mode 100644 src/proto/codec.rs diff --git a/src/proto/bloom/wire.rs b/src/proto/bloom/wire.rs index e180982..0b3c8a3 100644 --- a/src/proto/bloom/wire.rs +++ b/src/proto/bloom/wire.rs @@ -2,6 +2,7 @@ use super::BloomFilter; use crate::proto::Error; +use crate::proto::codec::{Reader, Writer}; use crate::proto::link::LinkMessageType; /// Bloom filter announcement for reachability propagation. @@ -86,50 +87,37 @@ impl FilterAnnounce { let filter_bytes = self.filter.as_bytes(); let size = 1 + Self::MIN_PAYLOAD_SIZE + filter_bytes.len(); - let mut buf = Vec::with_capacity(size); + let mut w = Writer::with_capacity(size); // msg_type - buf.push(LinkMessageType::FilterAnnounce.to_byte()); + w.write_u8(LinkMessageType::FilterAnnounce.to_byte()); // sequence (8 LE) - buf.extend_from_slice(&self.sequence.to_le_bytes()); + w.write_u64_le(self.sequence); // hash_count - buf.push(self.hash_count); + w.write_u8(self.hash_count); // size_class - buf.push(self.size_class); + w.write_u8(self.size_class); // filter_bits - buf.extend_from_slice(filter_bytes); + w.write_bytes(filter_bytes); - Ok(buf) + Ok(w.into_vec()) } /// Decode from link-layer payload (after msg_type byte stripped by dispatcher). /// /// The payload starts with the sequence field. pub fn decode(payload: &[u8]) -> Result { - if payload.len() < Self::MIN_PAYLOAD_SIZE { - return Err(Error::MessageTooShort { - expected: Self::MIN_PAYLOAD_SIZE, - got: payload.len(), - }); - } - - let mut pos = 0; + let mut reader = Reader::new(payload); + reader.require(Self::MIN_PAYLOAD_SIZE)?; // sequence (8 LE) - let sequence = u64::from_le_bytes( - payload[pos..pos + 8] - .try_into() - .map_err(|_| Error::Malformed("bad sequence"))?, - ); - pos += 8; + let sequence = reader.read_u64_le()?; // hash_count - let hash_count = payload[pos]; - pos += 1; + let hash_count = reader.read_u8()?; // size_class - let size_class = payload[pos]; - pos += 1; + let size_class = reader.read_u8()?; // Validate size_class range if size_class > Self::MAX_SIZE_CLASS { @@ -147,9 +135,11 @@ impl FilterAnnounce { }); } - // Expected filter size from size_class + // Expected filter size from size_class. The remaining length must match + // exactly (an over-long payload is rejected too), so this stays an + // explicit `!=` check rather than a `Reader::require` lower-bound gate. let expected_filter_bytes = 512usize << size_class; - let remaining = payload.len() - pos; + let remaining = reader.remaining(); if remaining != expected_filter_bytes { return Err(Error::MessageTooShort { expected: Self::MIN_PAYLOAD_SIZE + expected_filter_bytes, @@ -158,8 +148,7 @@ impl FilterAnnounce { } // Construct BloomFilter from bytes - let filter = - BloomFilter::from_slice(&payload[pos..], hash_count).map_err(Error::BadBloom)?; + let filter = BloomFilter::from_slice(reader.rest(), hash_count).map_err(Error::BadBloom)?; let announce = Self { filter, diff --git a/src/proto/codec.rs b/src/proto/codec.rs new file mode 100644 index 0000000..0ca0a97 --- /dev/null +++ b/src/proto/codec.rs @@ -0,0 +1,109 @@ +//! Bounds-checked byte reader/writer shared across the proto wire codecs. +//! +//! `Reader` fails a short read with `Error::MessageTooShort { expected, got }` +//! where `expected` is the cumulative byte offset it needed (`position + n`) and +//! `got` is the total buffer length — reproducing the codecs' existing per-field +//! and up-front length-check values exactly. +use crate::proto::Error; + +pub(crate) struct Reader<'a> { + buf: &'a [u8], + pos: usize, +} + +impl<'a> Reader<'a> { + pub(crate) fn new(buf: &'a [u8]) -> Self { + Self { buf, pos: 0 } + } + #[allow(dead_code)] + pub(crate) fn position(&self) -> usize { + self.pos + } + pub(crate) fn remaining(&self) -> usize { + self.buf.len() - self.pos + } + pub(crate) fn rest(&self) -> &'a [u8] { + &self.buf[self.pos..] + } + /// Ensure at least `n` more bytes are available; else MessageTooShort. + pub(crate) fn require(&self, n: usize) -> Result<(), Error> { + if self.pos + n > self.buf.len() { + return Err(Error::MessageTooShort { + expected: self.pos + n, + got: self.buf.len(), + }); + } + Ok(()) + } + /// Advance the cursor by `n` (caller has already validated bounds, e.g. via a + /// sub-decoder that returned a consumed count). Debug-panics if out of range. + pub(crate) fn advance(&mut self, n: usize) { + self.pos += n; + debug_assert!(self.pos <= self.buf.len()); + } + pub(crate) fn read_u8(&mut self) -> Result { + self.require(1)?; + let v = self.buf[self.pos]; + self.pos += 1; + Ok(v) + } + pub(crate) fn read_array(&mut self) -> Result<[u8; N], Error> { + self.require(N)?; + let mut a = [0u8; N]; + a.copy_from_slice(&self.buf[self.pos..self.pos + N]); + self.pos += N; + Ok(a) + } + pub(crate) fn read_u16_le(&mut self) -> Result { + Ok(u16::from_le_bytes(self.read_array::<2>()?)) + } + pub(crate) fn read_u32_le(&mut self) -> Result { + Ok(u32::from_le_bytes(self.read_array::<4>()?)) + } + pub(crate) fn read_u64_le(&mut self) -> Result { + Ok(u64::from_le_bytes(self.read_array::<8>()?)) + } + pub(crate) fn read_bytes(&mut self, n: usize) -> Result<&'a [u8], Error> { + self.require(n)?; + let s = &self.buf[self.pos..self.pos + n]; + self.pos += n; + Ok(s) + } +} + +pub(crate) struct Writer { + buf: alloc::vec::Vec, +} +impl Writer { + pub(crate) fn new() -> Self { + Self { + buf: alloc::vec::Vec::new(), + } + } + pub(crate) fn with_capacity(n: usize) -> Self { + Self { + buf: alloc::vec::Vec::with_capacity(n), + } + } + pub(crate) fn write_u8(&mut self, v: u8) { + self.buf.push(v); + } + pub(crate) fn write_u16_le(&mut self, v: u16) { + self.buf.extend_from_slice(&v.to_le_bytes()); + } + pub(crate) fn write_u32_le(&mut self, v: u32) { + self.buf.extend_from_slice(&v.to_le_bytes()); + } + pub(crate) fn write_u64_le(&mut self, v: u64) { + self.buf.extend_from_slice(&v.to_le_bytes()); + } + pub(crate) fn write_bytes(&mut self, b: &[u8]) { + self.buf.extend_from_slice(b); + } + pub(crate) fn len(&self) -> usize { + self.buf.len() + } + pub(crate) fn into_vec(self) -> alloc::vec::Vec { + self.buf + } +} diff --git a/src/proto/discovery/wire.rs b/src/proto/discovery/wire.rs index c717af4..c31c539 100644 --- a/src/proto/discovery/wire.rs +++ b/src/proto/discovery/wire.rs @@ -2,6 +2,7 @@ use crate::NodeAddr; use crate::proto::Error; +use crate::proto::codec::Reader; use crate::proto::stp::TreeCoordinate; use crate::proto::stp::{decode_coords, encode_coords}; use secp256k1::schnorr::Signature; @@ -86,43 +87,20 @@ impl LookupRequest { pub fn decode(payload: &[u8]) -> Result { // Minimum: request_id(8) + target(16) + origin(16) + ttl(1) + min_mtu(2) // + coords_count(2) = 45 bytes - if payload.len() < 45 { - return Err(Error::MessageTooShort { - expected: 45, - got: payload.len(), - }); - } + let mut reader = Reader::new(payload); + reader.require(45)?; - let mut pos = 0; + let request_id = reader.read_u64_le()?; - let request_id = u64::from_le_bytes( - payload[pos..pos + 8] - .try_into() - .map_err(|_| Error::Malformed("bad request_id"))?, - ); - pos += 8; + let target = NodeAddr::from_bytes(reader.read_array::<16>()?); - let mut target_bytes = [0u8; 16]; - target_bytes.copy_from_slice(&payload[pos..pos + 16]); - let target = NodeAddr::from_bytes(target_bytes); - pos += 16; + let origin = NodeAddr::from_bytes(reader.read_array::<16>()?); - let mut origin_bytes = [0u8; 16]; - origin_bytes.copy_from_slice(&payload[pos..pos + 16]); - let origin = NodeAddr::from_bytes(origin_bytes); - pos += 16; + let ttl = reader.read_u8()?; - let ttl = payload[pos]; - pos += 1; + let min_mtu = reader.read_u16_le()?; - let min_mtu = u16::from_le_bytes( - payload[pos..pos + 2] - .try_into() - .map_err(|_| Error::Malformed("bad min_mtu"))?, - ); - pos += 2; - - let (origin_coords, _consumed) = decode_coords(&payload[pos..])?; + let (origin_coords, _consumed) = decode_coords(reader.rest())?; Ok(Self { request_id, @@ -211,44 +189,19 @@ impl LookupResponse { /// Decode from wire format (after msg_type byte has been consumed). pub fn decode(payload: &[u8]) -> Result { // Minimum: request_id(8) + target(16) + path_mtu(2) + coords_count(2) + proof(64) = 92 - if payload.len() < 92 { - return Err(Error::MessageTooShort { - expected: 92, - got: payload.len(), - }); - } + let mut reader = Reader::new(payload); + reader.require(92)?; - let mut pos = 0; + let request_id = reader.read_u64_le()?; - let request_id = u64::from_le_bytes( - payload[pos..pos + 8] - .try_into() - .map_err(|_| Error::Malformed("bad request_id"))?, - ); - pos += 8; + let target = NodeAddr::from_bytes(reader.read_array::<16>()?); - let mut target_bytes = [0u8; 16]; - target_bytes.copy_from_slice(&payload[pos..pos + 16]); - let target = NodeAddr::from_bytes(target_bytes); - pos += 16; + let path_mtu = reader.read_u16_le()?; - let path_mtu = u16::from_le_bytes( - payload[pos..pos + 2] - .try_into() - .map_err(|_| Error::Malformed("bad path_mtu"))?, - ); - pos += 2; + let (target_coords, consumed) = decode_coords(reader.rest())?; + reader.advance(consumed); - let (target_coords, consumed) = decode_coords(&payload[pos..])?; - pos += consumed; - - if payload.len() < pos + 64 { - return Err(Error::MessageTooShort { - expected: pos + 64, - got: payload.len(), - }); - } - let proof = Signature::from_slice(&payload[pos..pos + 64]) + let proof = Signature::from_slice(reader.read_bytes(64)?) .map_err(|_| Error::Malformed("bad proof signature"))?; Ok(Self { diff --git a/src/proto/fmp/wire.rs b/src/proto/fmp/wire.rs index 0da6fd3..b9d8b6c 100644 --- a/src/proto/fmp/wire.rs +++ b/src/proto/fmp/wire.rs @@ -7,6 +7,7 @@ //! in `crate::proto::link`. use crate::proto::Error; +use crate::proto::codec::Reader; use crate::proto::link::LinkMessageType; use std::fmt; @@ -153,13 +154,9 @@ impl Disconnect { /// Decode from link-layer payload (after msg_type byte has been consumed). pub fn decode(payload: &[u8]) -> Result { - if payload.is_empty() { - return Err(Error::MessageTooShort { - expected: 1, - got: 0, - }); - } - let reason = DisconnectReason::from_byte(payload[0]).unwrap_or(DisconnectReason::Other); + let mut reader = Reader::new(payload); + let reason = + DisconnectReason::from_byte(reader.read_u8()?).unwrap_or(DisconnectReason::Other); Ok(Self { reason }) } } diff --git a/src/proto/fsp/wire.rs b/src/proto/fsp/wire.rs index b2ec872..ce0e933 100644 --- a/src/proto/fsp/wire.rs +++ b/src/proto/fsp/wire.rs @@ -33,6 +33,7 @@ //! | 0x3 | - | Handshake msg3 | SessionMsg3 (Noise XK msg3) | use crate::proto::Error; +use crate::proto::codec::{Reader, Writer}; use crate::proto::stp::{TreeCoordinate, decode_coords, decode_optional_coords, encode_coords}; use std::fmt; @@ -647,37 +648,18 @@ impl SessionSetup { /// Decode from wire format (after 4-byte FSP prefix has been consumed). pub fn decode(payload: &[u8]) -> Result { - if payload.is_empty() { - return Err(Error::MessageTooShort { - expected: 1, - got: 0, - }); - } - let flags = SessionFlags::from_byte(payload[0]); - let mut offset = 1; + let mut reader = Reader::new(payload); + let flags = SessionFlags::from_byte(reader.read_u8()?); - let (src_coords, consumed) = decode_coords(&payload[offset..])?; - offset += consumed; + let (src_coords, consumed) = decode_coords(reader.rest())?; + reader.advance(consumed); - let (dest_coords, consumed) = decode_coords(&payload[offset..])?; - offset += consumed; + let (dest_coords, consumed) = decode_coords(reader.rest())?; + reader.advance(consumed); - if payload.len() < offset + 2 { - return Err(Error::MessageTooShort { - expected: offset + 2, - got: payload.len(), - }); - } - let hs_len = u16::from_le_bytes([payload[offset], payload[offset + 1]]) as usize; - offset += 2; + let hs_len = reader.read_u16_le()? as usize; - if payload.len() < offset + hs_len { - return Err(Error::MessageTooShort { - expected: offset + hs_len, - got: payload.len(), - }); - } - let handshake_payload = payload[offset..offset + hs_len].to_vec(); + let handshake_payload = reader.read_bytes(hs_len)?.to_vec(); Ok(Self { src_coords, @@ -774,37 +756,18 @@ impl SessionAck { /// Decode from wire format (after 4-byte FSP prefix has been consumed). pub fn decode(payload: &[u8]) -> Result { - if payload.is_empty() { - return Err(Error::MessageTooShort { - expected: 1, - got: 0, - }); - } - let flags = payload[0]; - let mut offset = 1; + let mut reader = Reader::new(payload); + let flags = reader.read_u8()?; - let (src_coords, consumed) = decode_coords(&payload[offset..])?; - offset += consumed; + let (src_coords, consumed) = decode_coords(reader.rest())?; + reader.advance(consumed); - let (dest_coords, consumed) = decode_coords(&payload[offset..])?; - offset += consumed; + let (dest_coords, consumed) = decode_coords(reader.rest())?; + reader.advance(consumed); - if payload.len() < offset + 2 { - return Err(Error::MessageTooShort { - expected: offset + 2, - got: payload.len(), - }); - } - let hs_len = u16::from_le_bytes([payload[offset], payload[offset + 1]]) as usize; - offset += 2; + let hs_len = reader.read_u16_le()? as usize; - if payload.len() < offset + hs_len { - return Err(Error::MessageTooShort { - expected: offset + hs_len, - got: payload.len(), - }); - } - let handshake_payload = payload[offset..offset + hs_len].to_vec(); + let handshake_payload = reader.read_bytes(hs_len)?.to_vec(); Ok(Self { src_coords, @@ -855,49 +818,31 @@ impl SessionMsg3 { /// where ver_phase = 0x03 (version 0, phase MSG3). pub fn encode(&self) -> Vec { // Build body first to compute payload_len - let mut body = Vec::new(); - body.push(self.flags); + let mut body = Writer::new(); + body.write_u8(self.flags); let hs_len = self.handshake_payload.len() as u16; - body.extend_from_slice(&hs_len.to_le_bytes()); - body.extend_from_slice(&self.handshake_payload); + body.write_u16_le(hs_len); + body.write_bytes(&self.handshake_payload); // Prepend 4-byte FSP common prefix let payload_len = body.len() as u16; - let mut buf = Vec::with_capacity(4 + body.len()); - buf.push(0x03); // version 0, phase 0x3 (MSG3) - buf.push(0x00); // flags (must be zero for handshake) - buf.extend_from_slice(&payload_len.to_le_bytes()); - buf.extend_from_slice(&body); - buf + let body = body.into_vec(); + let mut w = Writer::with_capacity(4 + body.len()); + w.write_u8(0x03); // version 0, phase 0x3 (MSG3) + w.write_u8(0x00); // flags (must be zero for handshake) + w.write_u16_le(payload_len); + w.write_bytes(&body); + w.into_vec() } /// Decode from wire format (after 4-byte FSP prefix has been consumed). pub fn decode(payload: &[u8]) -> Result { - if payload.is_empty() { - return Err(Error::MessageTooShort { - expected: 1, - got: 0, - }); - } - let flags = payload[0]; - let mut offset = 1; + let mut reader = Reader::new(payload); + let flags = reader.read_u8()?; - if payload.len() < offset + 2 { - return Err(Error::MessageTooShort { - expected: offset + 2, - got: payload.len(), - }); - } - let hs_len = u16::from_le_bytes([payload[offset], payload[offset + 1]]) as usize; - offset += 2; + let hs_len = reader.read_u16_le()? as usize; - if payload.len() < offset + hs_len { - return Err(Error::MessageTooShort { - expected: offset + hs_len, - got: payload.len(), - }); - } - let handshake_payload = payload[offset..offset + hs_len].to_vec(); + let handshake_payload = reader.read_bytes(hs_len)?.to_vec(); Ok(Self { flags, diff --git a/src/proto/mmp/wire.rs b/src/proto/mmp/wire.rs index 342d407..282f8fa 100644 --- a/src/proto/mmp/wire.rs +++ b/src/proto/mmp/wire.rs @@ -7,6 +7,7 @@ //! between the two layers. Wire format follows the MMP design doc. use crate::proto::Error; +use crate::proto::codec::{Reader, Writer}; // ============================================================================ // SenderReport (msg_type 0x01, 48-byte body including type byte) @@ -82,39 +83,35 @@ pub struct ReceiverReport { impl SenderReport { /// Encode to wire format (48 bytes: msg_type + 3 reserved + 44 payload). pub fn encode(&self) -> Vec { - let mut buf = Vec::with_capacity(48); - buf.push(0x01); // msg_type - buf.extend_from_slice(&[0u8; 3]); // reserved - buf.extend_from_slice(&self.interval_start_counter.to_le_bytes()); - buf.extend_from_slice(&self.interval_end_counter.to_le_bytes()); - buf.extend_from_slice(&self.interval_start_timestamp.to_le_bytes()); - buf.extend_from_slice(&self.interval_end_timestamp.to_le_bytes()); - buf.extend_from_slice(&self.interval_bytes_sent.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_packets_sent.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_bytes_sent.to_le_bytes()); - buf + let mut w = Writer::with_capacity(48); + w.write_u8(0x01); // msg_type + w.write_bytes(&[0u8; 3]); // reserved + w.write_u64_le(self.interval_start_counter); + w.write_u64_le(self.interval_end_counter); + w.write_u32_le(self.interval_start_timestamp); + w.write_u32_le(self.interval_end_timestamp); + w.write_u32_le(self.interval_bytes_sent); + w.write_u64_le(self.cumulative_packets_sent); + w.write_u64_le(self.cumulative_bytes_sent); + w.into_vec() } /// Decode from payload after msg_type byte has been consumed. /// /// `payload` starts at the reserved bytes (offset 1 in the wire format). pub fn decode(payload: &[u8]) -> Result { - if payload.len() < 47 { - return Err(Error::MessageTooShort { - expected: 47, - got: payload.len(), - }); - } + let mut reader = Reader::new(payload); + reader.require(47)?; // Skip 3 reserved bytes - let p = &payload[3..]; + reader.advance(3); Ok(Self { - interval_start_counter: u64::from_le_bytes(p[0..8].try_into().unwrap()), - interval_end_counter: u64::from_le_bytes(p[8..16].try_into().unwrap()), - interval_start_timestamp: u32::from_le_bytes(p[16..20].try_into().unwrap()), - interval_end_timestamp: u32::from_le_bytes(p[20..24].try_into().unwrap()), - interval_bytes_sent: u32::from_le_bytes(p[24..28].try_into().unwrap()), - cumulative_packets_sent: u64::from_le_bytes(p[28..36].try_into().unwrap()), - cumulative_bytes_sent: u64::from_le_bytes(p[36..44].try_into().unwrap()), + interval_start_counter: reader.read_u64_le()?, + interval_end_counter: reader.read_u64_le()?, + interval_start_timestamp: reader.read_u32_le()?, + interval_end_timestamp: reader.read_u32_le()?, + interval_bytes_sent: reader.read_u32_le()?, + cumulative_packets_sent: reader.read_u64_le()?, + cumulative_bytes_sent: reader.read_u64_le()?, }) } } @@ -122,55 +119,54 @@ impl SenderReport { impl ReceiverReport { /// Encode to wire format (68 bytes: msg_type + 3 reserved + 64 payload). pub fn encode(&self) -> Vec { - let mut buf = Vec::with_capacity(68); - buf.push(0x02); // msg_type - buf.extend_from_slice(&[0u8; 3]); // reserved - buf.extend_from_slice(&self.highest_counter.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_packets_recv.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_bytes_recv.to_le_bytes()); - buf.extend_from_slice(&self.timestamp_echo.to_le_bytes()); - buf.extend_from_slice(&self.dwell_time.to_le_bytes()); - buf.extend_from_slice(&self.max_burst_loss.to_le_bytes()); - buf.extend_from_slice(&self.mean_burst_loss.to_le_bytes()); - buf.extend_from_slice(&[0u8; 2]); // reserved - buf.extend_from_slice(&self.jitter.to_le_bytes()); - buf.extend_from_slice(&self.ecn_ce_count.to_le_bytes()); - buf.extend_from_slice(&self.owd_trend.to_le_bytes()); - buf.extend_from_slice(&self.burst_loss_count.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_reorder_count.to_le_bytes()); - buf.extend_from_slice(&self.interval_packets_recv.to_le_bytes()); - buf.extend_from_slice(&self.interval_bytes_recv.to_le_bytes()); - buf + let mut w = Writer::with_capacity(68); + w.write_u8(0x02); // msg_type + w.write_bytes(&[0u8; 3]); // reserved + w.write_u64_le(self.highest_counter); + w.write_u64_le(self.cumulative_packets_recv); + w.write_u64_le(self.cumulative_bytes_recv); + w.write_u32_le(self.timestamp_echo); + w.write_u16_le(self.dwell_time); + w.write_u16_le(self.max_burst_loss); + w.write_u16_le(self.mean_burst_loss); + w.write_bytes(&[0u8; 2]); // reserved + w.write_u32_le(self.jitter); + w.write_u32_le(self.ecn_ce_count); + w.write_bytes(&self.owd_trend.to_le_bytes()); + w.write_u32_le(self.burst_loss_count); + w.write_u32_le(self.cumulative_reorder_count); + w.write_u32_le(self.interval_packets_recv); + w.write_u32_le(self.interval_bytes_recv); + w.into_vec() } /// Decode from payload after msg_type byte has been consumed. /// /// `payload` starts at the reserved bytes (offset 1 in the wire format). pub fn decode(payload: &[u8]) -> Result { - if payload.len() < 67 { - return Err(Error::MessageTooShort { - expected: 67, - got: payload.len(), - }); - } + let mut reader = Reader::new(payload); + reader.require(67)?; // Skip 3 reserved bytes - let p = &payload[3..]; + reader.advance(3); Ok(Self { - highest_counter: u64::from_le_bytes(p[0..8].try_into().unwrap()), - cumulative_packets_recv: u64::from_le_bytes(p[8..16].try_into().unwrap()), - cumulative_bytes_recv: u64::from_le_bytes(p[16..24].try_into().unwrap()), - timestamp_echo: u32::from_le_bytes(p[24..28].try_into().unwrap()), - dwell_time: u16::from_le_bytes(p[28..30].try_into().unwrap()), - max_burst_loss: u16::from_le_bytes(p[30..32].try_into().unwrap()), - mean_burst_loss: u16::from_le_bytes(p[32..34].try_into().unwrap()), + highest_counter: reader.read_u64_le()?, + cumulative_packets_recv: reader.read_u64_le()?, + cumulative_bytes_recv: reader.read_u64_le()?, + timestamp_echo: reader.read_u32_le()?, + dwell_time: reader.read_u16_le()?, + max_burst_loss: reader.read_u16_le()?, + mean_burst_loss: reader.read_u16_le()?, // skip 2 reserved bytes at p[34..36] - jitter: u32::from_le_bytes(p[36..40].try_into().unwrap()), - ecn_ce_count: u32::from_le_bytes(p[40..44].try_into().unwrap()), - owd_trend: i32::from_le_bytes(p[44..48].try_into().unwrap()), - burst_loss_count: u32::from_le_bytes(p[48..52].try_into().unwrap()), - cumulative_reorder_count: u32::from_le_bytes(p[52..56].try_into().unwrap()), - interval_packets_recv: u32::from_le_bytes(p[56..60].try_into().unwrap()), - interval_bytes_recv: u32::from_le_bytes(p[60..64].try_into().unwrap()), + jitter: { + reader.advance(2); + reader.read_u32_le()? + }, + ecn_ce_count: reader.read_u32_le()?, + owd_trend: i32::from_le_bytes(reader.read_array::<4>()?), + burst_loss_count: reader.read_u32_le()?, + cumulative_reorder_count: reader.read_u32_le()?, + interval_packets_recv: reader.read_u32_le()?, + interval_bytes_recv: reader.read_u32_le()?, }) } } @@ -214,36 +210,32 @@ pub const SESSION_SENDER_REPORT_SIZE: usize = 46; impl SessionSenderReport { /// Encode to wire format (46 bytes body). pub fn encode(&self) -> Vec { - let mut buf = Vec::with_capacity(SESSION_SENDER_REPORT_SIZE); - buf.extend_from_slice(&[0u8; 2]); // reserved - buf.extend_from_slice(&self.interval_start_counter.to_le_bytes()); - buf.extend_from_slice(&self.interval_end_counter.to_le_bytes()); - buf.extend_from_slice(&self.interval_start_timestamp.to_le_bytes()); - buf.extend_from_slice(&self.interval_end_timestamp.to_le_bytes()); - buf.extend_from_slice(&self.interval_bytes_sent.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_packets_sent.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_bytes_sent.to_le_bytes()); - buf + let mut w = Writer::with_capacity(SESSION_SENDER_REPORT_SIZE); + w.write_bytes(&[0u8; 2]); // reserved + w.write_u64_le(self.interval_start_counter); + w.write_u64_le(self.interval_end_counter); + w.write_u32_le(self.interval_start_timestamp); + w.write_u32_le(self.interval_end_timestamp); + w.write_u32_le(self.interval_bytes_sent); + w.write_u64_le(self.cumulative_packets_sent); + w.write_u64_le(self.cumulative_bytes_sent); + w.into_vec() } /// Decode from body (after FSP inner header has been stripped). pub fn decode(body: &[u8]) -> Result { - if body.len() < SESSION_SENDER_REPORT_SIZE { - return Err(Error::MessageTooShort { - expected: SESSION_SENDER_REPORT_SIZE, - got: body.len(), - }); - } + let mut reader = Reader::new(body); + reader.require(SESSION_SENDER_REPORT_SIZE)?; // Skip 2 reserved bytes - let p = &body[2..]; + reader.advance(2); Ok(Self { - interval_start_counter: u64::from_le_bytes(p[0..8].try_into().unwrap()), - interval_end_counter: u64::from_le_bytes(p[8..16].try_into().unwrap()), - interval_start_timestamp: u32::from_le_bytes(p[16..20].try_into().unwrap()), - interval_end_timestamp: u32::from_le_bytes(p[20..24].try_into().unwrap()), - interval_bytes_sent: u32::from_le_bytes(p[24..28].try_into().unwrap()), - cumulative_packets_sent: u64::from_le_bytes(p[28..36].try_into().unwrap()), - cumulative_bytes_sent: u64::from_le_bytes(p[36..44].try_into().unwrap()), + interval_start_counter: reader.read_u64_le()?, + interval_end_counter: reader.read_u64_le()?, + interval_start_timestamp: reader.read_u32_le()?, + interval_end_timestamp: reader.read_u32_le()?, + interval_bytes_sent: reader.read_u32_le()?, + cumulative_packets_sent: reader.read_u64_le()?, + cumulative_bytes_sent: reader.read_u64_le()?, }) } } @@ -297,52 +289,51 @@ pub const SESSION_RECEIVER_REPORT_SIZE: usize = 66; impl SessionReceiverReport { /// Encode to wire format (66 bytes body). pub fn encode(&self) -> Vec { - let mut buf = Vec::with_capacity(SESSION_RECEIVER_REPORT_SIZE); - buf.extend_from_slice(&[0u8; 2]); // reserved - buf.extend_from_slice(&self.highest_counter.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_packets_recv.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_bytes_recv.to_le_bytes()); - buf.extend_from_slice(&self.timestamp_echo.to_le_bytes()); - buf.extend_from_slice(&self.dwell_time.to_le_bytes()); - buf.extend_from_slice(&self.max_burst_loss.to_le_bytes()); - buf.extend_from_slice(&self.mean_burst_loss.to_le_bytes()); - buf.extend_from_slice(&[0u8; 2]); // reserved - buf.extend_from_slice(&self.jitter.to_le_bytes()); - buf.extend_from_slice(&self.ecn_ce_count.to_le_bytes()); - buf.extend_from_slice(&self.owd_trend.to_le_bytes()); - buf.extend_from_slice(&self.burst_loss_count.to_le_bytes()); - buf.extend_from_slice(&self.cumulative_reorder_count.to_le_bytes()); - buf.extend_from_slice(&self.interval_packets_recv.to_le_bytes()); - buf.extend_from_slice(&self.interval_bytes_recv.to_le_bytes()); - buf + let mut w = Writer::with_capacity(SESSION_RECEIVER_REPORT_SIZE); + w.write_bytes(&[0u8; 2]); // reserved + w.write_u64_le(self.highest_counter); + w.write_u64_le(self.cumulative_packets_recv); + w.write_u64_le(self.cumulative_bytes_recv); + w.write_u32_le(self.timestamp_echo); + w.write_u16_le(self.dwell_time); + w.write_u16_le(self.max_burst_loss); + w.write_u16_le(self.mean_burst_loss); + w.write_bytes(&[0u8; 2]); // reserved + w.write_u32_le(self.jitter); + w.write_u32_le(self.ecn_ce_count); + w.write_bytes(&self.owd_trend.to_le_bytes()); + w.write_u32_le(self.burst_loss_count); + w.write_u32_le(self.cumulative_reorder_count); + w.write_u32_le(self.interval_packets_recv); + w.write_u32_le(self.interval_bytes_recv); + w.into_vec() } /// Decode from body (after FSP inner header has been stripped). pub fn decode(body: &[u8]) -> Result { - if body.len() < SESSION_RECEIVER_REPORT_SIZE { - return Err(Error::MessageTooShort { - expected: SESSION_RECEIVER_REPORT_SIZE, - got: body.len(), - }); - } + let mut reader = Reader::new(body); + reader.require(SESSION_RECEIVER_REPORT_SIZE)?; // Skip 2 reserved bytes - let p = &body[2..]; + reader.advance(2); Ok(Self { - highest_counter: u64::from_le_bytes(p[0..8].try_into().unwrap()), - cumulative_packets_recv: u64::from_le_bytes(p[8..16].try_into().unwrap()), - cumulative_bytes_recv: u64::from_le_bytes(p[16..24].try_into().unwrap()), - timestamp_echo: u32::from_le_bytes(p[24..28].try_into().unwrap()), - dwell_time: u16::from_le_bytes(p[28..30].try_into().unwrap()), - max_burst_loss: u16::from_le_bytes(p[30..32].try_into().unwrap()), - mean_burst_loss: u16::from_le_bytes(p[32..34].try_into().unwrap()), + highest_counter: reader.read_u64_le()?, + cumulative_packets_recv: reader.read_u64_le()?, + cumulative_bytes_recv: reader.read_u64_le()?, + timestamp_echo: reader.read_u32_le()?, + dwell_time: reader.read_u16_le()?, + max_burst_loss: reader.read_u16_le()?, + mean_burst_loss: reader.read_u16_le()?, // skip 2 reserved bytes at p[34..36] - jitter: u32::from_le_bytes(p[36..40].try_into().unwrap()), - ecn_ce_count: u32::from_le_bytes(p[40..44].try_into().unwrap()), - owd_trend: i32::from_le_bytes(p[44..48].try_into().unwrap()), - burst_loss_count: u32::from_le_bytes(p[48..52].try_into().unwrap()), - cumulative_reorder_count: u32::from_le_bytes(p[52..56].try_into().unwrap()), - interval_packets_recv: u32::from_le_bytes(p[56..60].try_into().unwrap()), - interval_bytes_recv: u32::from_le_bytes(p[60..64].try_into().unwrap()), + jitter: { + reader.advance(2); + reader.read_u32_le()? + }, + ecn_ce_count: reader.read_u32_le()?, + owd_trend: i32::from_le_bytes(reader.read_array::<4>()?), + burst_loss_count: reader.read_u32_le()?, + cumulative_reorder_count: reader.read_u32_le()?, + interval_packets_recv: reader.read_u32_le()?, + interval_bytes_recv: reader.read_u32_le()?, }) } } @@ -380,14 +371,10 @@ impl PathMtuNotification { /// Decode from body (after FSP inner header has been stripped). pub fn decode(body: &[u8]) -> Result { - if body.len() < PATH_MTU_NOTIFICATION_SIZE { - return Err(Error::MessageTooShort { - expected: PATH_MTU_NOTIFICATION_SIZE, - got: body.len(), - }); - } + let mut reader = Reader::new(body); + reader.require(PATH_MTU_NOTIFICATION_SIZE)?; Ok(Self { - path_mtu: u16::from_le_bytes([body[0], body[1]]), + path_mtu: reader.read_u16_le()?, }) } } diff --git a/src/proto/mod.rs b/src/proto/mod.rs index e5fb961..b48e7b7 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -7,6 +7,7 @@ mod error; pub use error::Error; pub(crate) mod bloom; +pub(crate) mod codec; pub(crate) mod coord; pub(crate) mod discovery; pub(crate) mod fmp; diff --git a/src/proto/routing/wire.rs b/src/proto/routing/wire.rs index 803c90c..c867629 100644 --- a/src/proto/routing/wire.rs +++ b/src/proto/routing/wire.rs @@ -12,6 +12,7 @@ use crate::NodeAddr; use crate::proto::Error; +use crate::proto::codec::{Reader, Writer}; use crate::proto::stp::{ TreeCoordinate, decode_optional_coords, encode_coords, encode_empty_coords, }; @@ -109,34 +110,29 @@ impl CoordsRequired { pub fn encode(&self) -> Vec { // Body: msg_type + flags(reserved) + dest_addr + reporter let body_len = 1 + 1 + 16 + 16; // 34 bytes - let mut buf = Vec::with_capacity(4 + body_len); + let mut w = Writer::with_capacity(4 + body_len); // FSP prefix: version 0, phase 0x0, U flag set - buf.push(0x00); // version 0, phase 0x0 - buf.push(0x04); // U flag + w.write_u8(0x00); // version 0, phase 0x0 + w.write_u8(0x04); // U flag let payload_len = body_len as u16; - buf.extend_from_slice(&payload_len.to_le_bytes()); + w.write_u16_le(payload_len); // msg_type byte (after prefix, before body) - buf.push(RoutingSignalType::CoordsRequired.to_byte()); - buf.push(0x00); // reserved flags - buf.extend_from_slice(self.dest_addr.as_bytes()); - buf.extend_from_slice(self.reporter.as_bytes()); - buf + w.write_u8(RoutingSignalType::CoordsRequired.to_byte()); + w.write_u8(0x00); // reserved flags + w.write_bytes(self.dest_addr.as_bytes()); + w.write_bytes(self.reporter.as_bytes()); + w.into_vec() } /// Decode from wire format (after FSP prefix and msg_type byte consumed). pub fn decode(payload: &[u8]) -> Result { // flags(1) + dest_addr(16) + reporter(16) = 33 - if payload.len() < 33 { - return Err(Error::MessageTooShort { - expected: 33, - got: payload.len(), - }); - } + let mut reader = Reader::new(payload); + reader.require(33)?; // payload[0] is flags (reserved, ignored) - let mut dest_bytes = [0u8; 16]; - dest_bytes.copy_from_slice(&payload[1..17]); - let mut reporter_bytes = [0u8; 16]; - reporter_bytes.copy_from_slice(&payload[17..33]); + reader.advance(1); + let dest_bytes = reader.read_array::<16>()?; + let reporter_bytes = reader.read_array::<16>()?; Ok(Self { dest_addr: NodeAddr::from_bytes(dest_bytes), @@ -217,19 +213,14 @@ impl PathBroken { /// Decode from wire format (after FSP prefix and msg_type byte consumed). pub fn decode(payload: &[u8]) -> Result { // flags(1) + dest_addr(16) + reporter(16) + coords_count(2) = 35 minimum - if payload.len() < 35 { - return Err(Error::MessageTooShort { - expected: 35, - got: payload.len(), - }); - } + let mut reader = Reader::new(payload); + reader.require(35)?; // payload[0] is flags (reserved, ignored) - let mut dest_bytes = [0u8; 16]; - dest_bytes.copy_from_slice(&payload[1..17]); - let mut reporter_bytes = [0u8; 16]; - reporter_bytes.copy_from_slice(&payload[17..33]); + reader.advance(1); + let dest_bytes = reader.read_array::<16>()?; + let reporter_bytes = reader.read_array::<16>()?; - let (last_known_coords, _consumed) = decode_optional_coords(&payload[33..])?; + let (last_known_coords, _consumed) = decode_optional_coords(reader.rest())?; Ok(Self { dest_addr: NodeAddr::from_bytes(dest_bytes), @@ -284,36 +275,31 @@ impl MtuExceeded { /// Error signals use phase=0x0 with U flag set. pub fn encode(&self) -> Vec { let body_len = MTU_EXCEEDED_SIZE; // 36 bytes - let mut buf = Vec::with_capacity(4 + body_len); + let mut w = Writer::with_capacity(4 + body_len); // FSP prefix: version 0, phase 0x0, U flag set - buf.push(0x00); // version 0, phase 0x0 - buf.push(0x04); // U flag + w.write_u8(0x00); // version 0, phase 0x0 + w.write_u8(0x04); // U flag let payload_len = body_len as u16; - buf.extend_from_slice(&payload_len.to_le_bytes()); + w.write_u16_le(payload_len); // msg_type byte - buf.push(RoutingSignalType::MtuExceeded.to_byte()); - buf.push(0x00); // reserved flags - buf.extend_from_slice(self.dest_addr.as_bytes()); - buf.extend_from_slice(self.reporter.as_bytes()); - buf.extend_from_slice(&self.mtu.to_le_bytes()); - buf + w.write_u8(RoutingSignalType::MtuExceeded.to_byte()); + w.write_u8(0x00); // reserved flags + w.write_bytes(self.dest_addr.as_bytes()); + w.write_bytes(self.reporter.as_bytes()); + w.write_u16_le(self.mtu); + w.into_vec() } /// Decode from wire format (after FSP prefix and msg_type byte consumed). pub fn decode(payload: &[u8]) -> Result { // flags(1) + dest_addr(16) + reporter(16) + mtu(2) = 35 - if payload.len() < 35 { - return Err(Error::MessageTooShort { - expected: 35, - got: payload.len(), - }); - } + let mut reader = Reader::new(payload); + reader.require(35)?; // payload[0] is flags (reserved, ignored) - let mut dest_bytes = [0u8; 16]; - dest_bytes.copy_from_slice(&payload[1..17]); - let mut reporter_bytes = [0u8; 16]; - reporter_bytes.copy_from_slice(&payload[17..33]); - let mtu = u16::from_le_bytes([payload[33], payload[34]]); + reader.advance(1); + let dest_bytes = reader.read_array::<16>()?; + let reporter_bytes = reader.read_array::<16>()?; + let mtu = reader.read_u16_le()?; Ok(Self { dest_addr: NodeAddr::from_bytes(dest_bytes), diff --git a/src/proto/stp/wire.rs b/src/proto/stp/wire.rs index 45a9efb..805bc00 100644 --- a/src/proto/stp/wire.rs +++ b/src/proto/stp/wire.rs @@ -3,6 +3,7 @@ use super::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError}; use crate::NodeAddr; use crate::proto::Error; +use crate::proto::codec::{Reader, Writer}; use crate::proto::link::LinkMessageType; use secp256k1::schnorr::Signature; @@ -103,121 +104,72 @@ impl TreeAnnounce { let entries = self.ancestry.entries(); let ancestry_count = entries.len() as u16; let size = 1 + Self::MIN_PAYLOAD_SIZE + entries.len() * CoordEntry::WIRE_SIZE; - let mut buf = Vec::with_capacity(size); + let mut w = Writer::with_capacity(size); // msg_type - buf.push(LinkMessageType::TreeAnnounce.to_byte()); + w.write_u8(LinkMessageType::TreeAnnounce.to_byte()); // version - buf.push(Self::VERSION_1); + w.write_u8(Self::VERSION_1); // sequence (8 LE) - buf.extend_from_slice(&self.declaration.sequence().to_le_bytes()); + w.write_u64_le(self.declaration.sequence()); // timestamp (8 LE) - buf.extend_from_slice(&self.declaration.timestamp().to_le_bytes()); + w.write_u64_le(self.declaration.timestamp()); // parent (16) - buf.extend_from_slice(self.declaration.parent_id().as_bytes()); + w.write_bytes(self.declaration.parent_id().as_bytes()); // ancestry_count (2 LE) - buf.extend_from_slice(&ancestry_count.to_le_bytes()); + w.write_u16_le(ancestry_count); // ancestry entries (32 bytes each) for entry in entries { - buf.extend_from_slice(entry.node_addr.as_bytes()); // 16 - buf.extend_from_slice(&entry.sequence.to_le_bytes()); // 8 - buf.extend_from_slice(&entry.timestamp.to_le_bytes()); // 8 + w.write_bytes(entry.node_addr.as_bytes()); // 16 + w.write_u64_le(entry.sequence); // 8 + w.write_u64_le(entry.timestamp); // 8 } // outer signature (64) - buf.extend_from_slice(signature.as_ref()); + w.write_bytes(signature.as_ref()); - Ok(buf) + Ok(w.into_vec()) } /// Decode from link-layer payload (after msg_type byte stripped by dispatcher). /// /// The payload starts with the version byte. pub fn decode(payload: &[u8]) -> Result { - if payload.len() < Self::MIN_PAYLOAD_SIZE { - return Err(Error::MessageTooShort { - expected: Self::MIN_PAYLOAD_SIZE, - got: payload.len(), - }); - } - - let mut pos = 0; + let mut reader = Reader::new(payload); + reader.require(Self::MIN_PAYLOAD_SIZE)?; // version - let version = payload[pos]; - pos += 1; + let version = reader.read_u8()?; if version != Self::VERSION_1 { return Err(Error::UnsupportedVersion(version)); } // sequence (8 LE) - let sequence = u64::from_le_bytes( - payload[pos..pos + 8] - .try_into() - .map_err(|_| Error::Malformed("bad sequence"))?, - ); - pos += 8; + let sequence = reader.read_u64_le()?; // timestamp (8 LE) - let timestamp = u64::from_le_bytes( - payload[pos..pos + 8] - .try_into() - .map_err(|_| Error::Malformed("bad timestamp"))?, - ); - pos += 8; + let timestamp = reader.read_u64_le()?; // parent (16) - let parent = NodeAddr::from_bytes( - payload[pos..pos + 16] - .try_into() - .map_err(|_| Error::Malformed("bad parent"))?, - ); - pos += 16; + let parent = NodeAddr::from_bytes(reader.read_array::<16>()?); // ancestry_count (2 LE) - let ancestry_count = u16::from_le_bytes( - payload[pos..pos + 2] - .try_into() - .map_err(|_| Error::Malformed("bad ancestry count"))?, - ) as usize; - pos += 2; + let ancestry_count = reader.read_u16_le()? as usize; // Validate remaining length: entries + signature let expected_remaining = ancestry_count * CoordEntry::WIRE_SIZE + 64; - if payload.len() - pos < expected_remaining { - return Err(Error::MessageTooShort { - expected: pos + expected_remaining, - got: payload.len(), - }); - } + reader.require(expected_remaining)?; // ancestry entries (32 bytes each) let mut entries = Vec::with_capacity(ancestry_count); for _ in 0..ancestry_count { - let node_addr = NodeAddr::from_bytes( - payload[pos..pos + 16] - .try_into() - .map_err(|_| Error::Malformed("bad entry node_addr"))?, - ); - pos += 16; - let entry_seq = u64::from_le_bytes( - payload[pos..pos + 8] - .try_into() - .map_err(|_| Error::Malformed("bad entry sequence"))?, - ); - pos += 8; - let entry_ts = u64::from_le_bytes( - payload[pos..pos + 8] - .try_into() - .map_err(|_| Error::Malformed("bad entry timestamp"))?, - ); - pos += 8; + let node_addr = NodeAddr::from_bytes(reader.read_array::<16>()?); + let entry_seq = reader.read_u64_le()?; + let entry_ts = reader.read_u64_le()?; entries.push(CoordEntry::new(node_addr, entry_seq, entry_ts)); } // signature (64) - let sig_bytes: [u8; 64] = payload[pos..pos + 64] - .try_into() - .map_err(|_| Error::Malformed("bad signature"))?; + let sig_bytes: [u8; 64] = reader.read_array::<64>()?; // 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 From 2b009196b5d223a1e77aa53dba9e599d42ab2c2e Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 8 Jul 2026 19:00:25 +0000 Subject: [PATCH 9/9] proto: no_std hygiene for the fmt/alloc imports and the filter math Bring the proto tree closer to a std+alloc-only posture, behavior unchanged on every decision path: - Sweep the remaining std::fmt and std::collections imports over to core::fmt and alloc::collections across the wire codecs and their tests. Subsystem files that shadow the core name with a child core module use the leading-colon ::core::fmt form. Imports only. - Replace the two f64::powi calls (bloom false-positive-rate, FMP backoff timer) with a shared core-only square-and-multiply helper in proto/math, bit-identical to std::powi (a guard test pins this against every exponent the codecs reach), and route the diagnostic estimated-count natural log through libm::log. The FPR reject decision and the backoff timer are bit-for-bit unchanged; only the debug-only count estimate may differ by at most one ULP. --- Cargo.lock | 1 + Cargo.toml | 1 + src/proto/bloom/core.rs | 4 +-- src/proto/bloom/tests/state.rs | 2 +- src/proto/fmp/core.rs | 2 +- src/proto/fmp/wire.rs | 2 +- src/proto/fsp/tests/core.rs | 2 +- src/proto/fsp/wire.rs | 2 +- src/proto/link.rs | 2 +- src/proto/math.rs | 55 ++++++++++++++++++++++++++++++++++ src/proto/mod.rs | 1 + src/proto/stp/tests/limits.rs | 2 +- src/proto/stp/tests/state.rs | 2 +- src/proto/stp/tests/util.rs | 2 +- 14 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 src/proto/math.rs diff --git a/Cargo.lock b/Cargo.lock index fa8c8c8..bed9868 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1087,6 +1087,7 @@ dependencies = [ "hex", "hkdf", "libc", + "libm", "mdns-sd", "nostr", "nostr-sdk", diff --git a/Cargo.toml b/Cargo.toml index 6c38f75..29a3ed0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ secp256k1 = { version = "0.30", features = ["rand", "global-context"] } sha2 = "0.10" hkdf = "0.12" ring = "0.17" +libm = "0.2" rand = "0.10.1" crossbeam-channel = "0.5" thiserror = "2.0" diff --git a/src/proto/bloom/core.rs b/src/proto/bloom/core.rs index 6ef9600..e7be1d8 100644 --- a/src/proto/bloom/core.rs +++ b/src/proto/bloom/core.rs @@ -141,7 +141,7 @@ impl BloomFilter { /// Current false-positive rate: fill ratio raised to the hash count. pub fn fpr(&self) -> f64 { - self.fill_ratio().powi(self.hash_count as i32) + crate::proto::math::powi(self.fill_ratio(), self.hash_count as u32) } /// Estimate the number of elements in the filter. @@ -170,7 +170,7 @@ impl BloomFilter { return None; } - Some(-(m / k) * (1.0 - fill).ln()) + Some(-(m / k) * libm::log(1.0 - fill)) } /// Check if the filter is empty. diff --git a/src/proto/bloom/tests/state.rs b/src/proto/bloom/tests/state.rs index 6c74746..159b276 100644 --- a/src/proto/bloom/tests/state.rs +++ b/src/proto/bloom/tests/state.rs @@ -1,6 +1,6 @@ //! Tests for `BloomState` (announcement state management). -use std::collections::BTreeMap; +use alloc::collections::BTreeMap; use crate::proto::bloom::{BloomFilter, BloomState}; use crate::testutil::make_node_addr; diff --git a/src/proto/fmp/core.rs b/src/proto/fmp/core.rs index 0ae419c..dd7af64 100644 --- a/src/proto/fmp/core.rs +++ b/src/proto/fmp/core.rs @@ -640,5 +640,5 @@ impl Fmp { /// arithmetic (the exponent is the resend count *after* this attempt). fn next_resend_at_ms(now_ms: u64, interval_ms: u64, backoff: f64, prior_count: u32) -> u64 { let count = prior_count + 1; - now_ms + (interval_ms as f64 * backoff.powi(count as i32)) as u64 + now_ms + (interval_ms as f64 * crate::proto::math::powi(backoff, count)) as u64 } diff --git a/src/proto/fmp/wire.rs b/src/proto/fmp/wire.rs index b9d8b6c..715b1f4 100644 --- a/src/proto/fmp/wire.rs +++ b/src/proto/fmp/wire.rs @@ -9,7 +9,7 @@ use crate::proto::Error; use crate::proto::codec::Reader; use crate::proto::link::LinkMessageType; -use std::fmt; +use ::core::fmt; /// Handshake message type identifiers. /// diff --git a/src/proto/fsp/tests/core.rs b/src/proto/fsp/tests/core.rs index 5c41b62..efe1c68 100644 --- a/src/proto/fsp/tests/core.rs +++ b/src/proto/fsp/tests/core.rs @@ -9,7 +9,7 @@ use crate::proto::fsp::core::{ use crate::proto::fsp::limits::FSP_CUTOVER_DELAY_MS; use crate::proto::stp::TreeCoordinate; use crate::testutil::make_node_addr; -use std::collections::VecDeque; +use alloc::collections::VecDeque; fn coords(byte: u8) -> TreeCoordinate { TreeCoordinate::from_addrs(vec![make_node_addr(byte)]).unwrap() diff --git a/src/proto/fsp/wire.rs b/src/proto/fsp/wire.rs index ce0e933..6c1db97 100644 --- a/src/proto/fsp/wire.rs +++ b/src/proto/fsp/wire.rs @@ -35,7 +35,7 @@ use crate::proto::Error; use crate::proto::codec::{Reader, Writer}; use crate::proto::stp::{TreeCoordinate, decode_coords, decode_optional_coords, encode_coords}; -use std::fmt; +use ::core::fmt; // ============================================================================ // Constants diff --git a/src/proto/link.rs b/src/proto/link.rs index b04d02a..01dca20 100644 --- a/src/proto/link.rs +++ b/src/proto/link.rs @@ -2,7 +2,7 @@ use crate::NodeAddr; use crate::proto::Error; -use std::fmt; +use core::fmt; // ============================================================================ // Link-Layer Message Types diff --git a/src/proto/math.rs b/src/proto/math.rs new file mode 100644 index 0000000..3b7c1dd --- /dev/null +++ b/src/proto/math.rs @@ -0,0 +1,55 @@ +//! no_std-shaped numeric helpers for the protocol cores. +//! +//! `f64::powi` and the transcendental methods live in `std`, not `core`. The +//! helpers here keep the small amount of protocol math the codecs need in a +//! `core`-only shape so the cores stay portable, without changing any result. + +/// Raise `base` to a non-negative integer power via square-and-multiply. +/// +/// Bit-identical to `f64::powi` for the exponents the codecs use: it mirrors the +/// compiler-rt `__powidf2` multiply order (multiply-then-square, skipping the +/// final square), so — floating-point multiplication being non-associative — it +/// reproduces `powi` exactly rather than a naive left-to-right product. The +/// `powi_bit_identical_to_std` test pins this. Keeping it bit-identical matters: +/// the bloom false-positive-rate feeds a reject comparison and the FMP backoff +/// feeds a `u64` timer, so any drift could change a decision. +pub(crate) fn powi(base: f64, exp: u32) -> f64 { + let mut result = 1.0_f64; + let mut b = base; + let mut e = exp; + loop { + if e & 1 == 1 { + result *= b; + } + e >>= 1; + if e == 0 { + break; + } + b *= b; + } + result +} + +#[cfg(test)] +mod tests { + use super::powi; + + #[test] + fn powi_bit_identical_to_std() { + // The core square-and-multiply must match f64::powi bit-for-bit across + // representative bases and every exponent the protocol math reaches, so + // the FPR reject decision and the backoff timer are unchanged. + let bases = [ + 0.0, 1.0, 0.5, 0.5469, 0.5493, 0.5508, 0.9999, 1.5, 2.0, 3.7, 0.1, 1e-3, + ]; + for &base in &bases { + for exp in 0u32..=64 { + assert_eq!( + powi(base, exp).to_bits(), + base.powi(exp as i32).to_bits(), + "powi mismatch at base={base}, exp={exp}" + ); + } + } + } +} diff --git a/src/proto/mod.rs b/src/proto/mod.rs index b48e7b7..ac08f03 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -13,6 +13,7 @@ pub(crate) mod discovery; pub(crate) mod fmp; pub(crate) mod fsp; pub(crate) mod link; +pub(crate) mod math; pub(crate) mod mmp; pub(crate) mod rate_limit; pub(crate) mod routing; diff --git a/src/proto/stp/tests/limits.rs b/src/proto/stp/tests/limits.rs index 078fd18..387e5dc 100644 --- a/src/proto/stp/tests/limits.rs +++ b/src/proto/stp/tests/limits.rs @@ -1,6 +1,6 @@ //! Flap dampening / hold-down unit tests. -use std::collections::{BTreeMap, BTreeSet}; +use alloc::collections::{BTreeMap, BTreeSet}; use super::util::{make_coords, make_costs, make_node_addr}; use crate::proto::stp::{ParentDeclaration, ParentEval, TreeState}; diff --git a/src/proto/stp/tests/state.rs b/src/proto/stp/tests/state.rs index 70bcaa9..60e0777 100644 --- a/src/proto/stp/tests/state.rs +++ b/src/proto/stp/tests/state.rs @@ -1,6 +1,6 @@ //! ParentDeclaration, TreeState, parent-selection, and find_next_hop unit tests. -use std::collections::{BTreeMap, BTreeSet}; +use alloc::collections::{BTreeMap, BTreeSet}; use super::util::{add_peer, make_coords, make_costs, make_node_addr, make_tree_state}; use crate::proto::stp::{ParentDeclaration, ParentEval, TreeCoordinate, TreeState}; diff --git a/src/proto/stp/tests/util.rs b/src/proto/stp/tests/util.rs index 58ee826..b6b5571 100644 --- a/src/proto/stp/tests/util.rs +++ b/src/proto/stp/tests/util.rs @@ -1,6 +1,6 @@ //! Shared test helpers for the STP primitive unit tests. -use std::collections::BTreeMap; +use alloc::collections::BTreeMap; use crate::NodeAddr; use crate::proto::stp::{ParentDeclaration, TreeCoordinate, TreeState};