node: extract immutable state into a shared context and atomic metric registry

Store node counters in an atomic metric registry read through &self, and
introduce a shared NodeContext bundle holding the effectively-immutable
fields (config, identity, startup epoch, node profile, capability limits).
Source the immutable config and identity reads across the receive hot
path, the XX handshake/session/rekey state machines, and the discovery,
tree, bloom, retry, and lifecycle modules through the context accessors.
Includes the bloom delta/full/NACK/resize and byte-total counters in the
registry. The Node fields and the context are rebuilt in lockstep at every
mutation site.
This commit is contained in:
Johnathan Corgan
2026-06-02 13:05:03 +00:00
parent 88e48fdc68
commit e181df0ac4
30 changed files with 1301 additions and 1003 deletions
+102
View File
@@ -0,0 +1,102 @@
//! Registry-counter coverage tests for the bloom-v2 metric counters that
//! the mesh-lab suites do not reliably exercise.
//!
//! The send-path bloom counters (`deltas_sent`, `full_sends`,
//! `total_compressed_bytes`, `total_raw_bytes`) fire on every filter
//! announce and are covered by the steady-state suites. The three
//! condition-dependent counters (`nacks_sent`, `nacks_received`,
//! `size_changes`) only fire on out-of-sequence deltas, inbound NACKs,
//! and adaptive resizes — none of which occur in the stable, lossless
//! mesh-lab scenarios. These tests drive each of those paths directly and
//! assert the registry counter increments.
use super::*;
use crate::bloom::{BloomFilter, V1_SIZE_CLASS};
use crate::peer::ActivePeer;
use crate::protocol::{FilterAnnounce, FilterNack};
/// Inject a synthetic active peer with a known NodeAddr; returns it.
fn inject_peer(node: &mut Node) -> NodeAddr {
let peer_identity = make_peer_identity();
let peer_addr = *peer_identity.node_addr();
let peer = ActivePeer::new(peer_identity, LinkId::new(1), 0);
node.peers.insert(peer_addr, peer);
peer_addr
}
/// Encode a FilterAnnounce to the payload format handle_filter_announce
/// expects (msg_type byte stripped).
fn encode_announce(announce: &FilterAnnounce) -> Vec<u8> {
let (mut full, _stats) = announce.encode().unwrap();
full.remove(0); // strip msg_type byte
full
}
/// An out-of-sequence delta to a peer with no stored filter makes the node
/// send a NACK, bumping `nacks_sent`.
#[tokio::test]
async fn test_bloom_nacks_sent_counter() {
let mut node = make_node();
let peer_addr = inject_peer(&mut node);
// Fresh peer: filter_sequence == 0. A delta whose base_seq does not
// match the expected base (0) is out-of-sequence → NACK.
let announce = FilterAnnounce::delta(BloomFilter::new(), 2, 5, V1_SIZE_CLASS);
let payload = encode_announce(&announce);
node.handle_filter_announce(&peer_addr, &payload).await;
assert_eq!(
node.metrics().bloom.nacks_sent.get(),
1,
"registry nacks_sent must increment on out-of-sequence delta"
);
}
/// An inbound FilterNack bumps `nacks_received`.
#[tokio::test]
async fn test_bloom_nacks_received_counter() {
let mut node = make_node();
let peer_addr = inject_peer(&mut node);
let mut payload = FilterNack { expected_seq: 7 }.encode();
payload.remove(0); // strip msg_type byte (decode expects the seq only)
node.handle_filter_nack(&peer_addr, &payload).await;
assert_eq!(
node.metrics().bloom.nacks_received.get(),
1,
"registry nacks_received must increment on inbound NACK"
);
}
/// A fresh Full node starts at V1_SIZE_CLASS with a nearly empty outgoing
/// filter (just its own addr), so the first adaptive-sizing pass steps the
/// size class down, bumping `size_changes`.
#[tokio::test]
async fn test_bloom_size_changes_counter() {
let mut node = make_node();
// check_adaptive_sizing needs at least one peer for the representative
// outgoing-filter computation.
let _peer = inject_peer(&mut node);
assert_eq!(
node.bloom_state.size_class(),
V1_SIZE_CLASS,
"fresh node starts at the v1 size class"
);
node.check_bloom_state().await;
assert_eq!(
node.metrics().bloom.size_changes.get(),
1,
"registry size_changes must increment on adaptive resize"
);
assert_eq!(
node.bloom_state.size_class(),
V1_SIZE_CLASS - 1,
"near-empty outgoing filter steps the size class down"
);
}
+18 -21
View File
@@ -43,24 +43,22 @@ async fn test_m1_rejects_all_ones_filter_announce() {
let announce = FilterAnnounce::full(all_ones, 1, 1);
let payload = encode_payload(&announce);
let before_fill_exceeded = node.stats().bloom.fill_exceeded;
let before_accepted = node.stats().bloom.accepted;
let before_fill_exceeded = node.metrics().bloom.fill_exceeded.get();
let before_accepted = node.metrics().bloom.accepted.get();
node.handle_filter_announce(&peer_addr, &payload).await;
let after = &node.stats().bloom;
// While the typed-rejection rollout is in progress the call site bumps
// counter directly AND dispatches through record_reject, which
// hits the same counter. A later change will collapse this to a
// single increment by removing the legacy direct bump; for now
// the rejection-path event yields a +2 delta.
// The rejection path bumps the counter once, through the typed
// record_reject dispatch. The legacy direct bump has been removed,
// so the rejection-path event yields a +1 delta.
assert_eq!(
after.fill_exceeded,
before_fill_exceeded + 2,
node.metrics().bloom.fill_exceeded.get(),
before_fill_exceeded + 1,
"fill_exceeded counter must increment on all-ones rejection"
);
assert_eq!(
after.accepted, before_accepted,
node.metrics().bloom.accepted.get(),
before_accepted,
"accepted counter must NOT increment on rejection"
);
@@ -93,18 +91,18 @@ async fn test_m1_accepts_sub_cap_filter() {
let announce = FilterAnnounce::full(filter, 1, 1);
let payload = encode_payload(&announce);
let before_fill_exceeded = node.stats().bloom.fill_exceeded;
let before_accepted = node.stats().bloom.accepted;
let before_fill_exceeded = node.metrics().bloom.fill_exceeded.get();
let before_accepted = node.metrics().bloom.accepted.get();
node.handle_filter_announce(&peer_addr, &payload).await;
let after = &node.stats().bloom;
assert_eq!(
after.fill_exceeded, before_fill_exceeded,
node.metrics().bloom.fill_exceeded.get(),
before_fill_exceeded,
"fill_exceeded must NOT increment on legitimate sub-cap filter"
);
assert_eq!(
after.accepted,
node.metrics().bloom.accepted.get(),
before_accepted + 1,
"accepted must increment on legitimate filter"
);
@@ -164,9 +162,8 @@ async fn test_m1_sequence_not_advanced_allows_recovery() {
"compliant announce at same seq must be accepted after rejection"
);
assert_eq!(peer.filter_sequence(), 1);
// Direct bump + record_reject dispatch both increment the same
// counter while the typed-rejection rollout is in progress. A later
// change collapses these back to a single increment.
assert_eq!(node.stats().bloom.fill_exceeded, 2);
assert_eq!(node.stats().bloom.accepted, 1);
// The typed record_reject dispatch increments the counter once; the
// legacy direct bump has been removed.
assert_eq!(node.metrics().bloom.fill_exceeded.get(), 1);
assert_eq!(node.metrics().bloom.accepted.get(), 1);
}
+6 -4
View File
@@ -333,11 +333,12 @@ async fn test_third_peer_can_handshake_via_adopted_transport_socket() {
#[tokio::test]
async fn test_adopted_udp_inherits_mtu_from_single_primary_config() {
let mut node = make_node();
node.config.transports.udp = TransportInstances::Single(UdpConfig {
let mut config = Config::new();
config.transports.udp = TransportInstances::Single(UdpConfig {
mtu: Some(1500),
..Default::default()
});
let mut node = make_node_with(config);
let (packet_tx, packet_rx) = packet_channel(64);
node.packet_tx = Some(packet_tx);
@@ -370,7 +371,6 @@ async fn test_adopted_udp_inherits_mtu_from_single_primary_config() {
#[tokio::test]
async fn test_adopted_udp_inherits_mtu_from_named_primary_config() {
let mut node = make_node();
let mut named = HashMap::new();
named.insert(
"primary".to_string(),
@@ -386,7 +386,9 @@ async fn test_adopted_udp_inherits_mtu_from_named_primary_config() {
..Default::default()
},
);
node.config.transports.udp = TransportInstances::Named(named);
let mut config = Config::new();
config.transports.udp = TransportInstances::Named(named);
let mut node = make_node_with(config);
let (packet_tx, packet_rx) = packet_channel(64);
node.packet_tx = Some(packet_tx);
+9 -9
View File
@@ -1232,8 +1232,8 @@ async fn test_check_pending_lookups_default_sequence_unreachable() {
node.pending_lookups
.insert(target_addr, PendingLookup::new(0));
let baseline_initiated = node.stats().discovery.req_initiated;
let baseline_timed_out = node.stats().discovery.resp_timed_out;
let baseline_initiated = node.metrics().discovery.req_initiated.get();
let baseline_timed_out = node.metrics().discovery.resp_timed_out.get();
// --- t = 1100ms: first retry deadline (1*1000) ---
node.check_pending_lookups(1100).await;
@@ -1246,7 +1246,7 @@ async fn test_check_pending_lookups_default_sequence_unreachable() {
assert_eq!(entry.last_sent_ms, 1100);
}
assert_eq!(
node.stats().discovery.req_initiated,
node.metrics().discovery.req_initiated.get(),
baseline_initiated + 1,
"retry #1 must invoke initiate_lookup exactly once"
);
@@ -1262,7 +1262,7 @@ async fn test_check_pending_lookups_default_sequence_unreachable() {
assert_eq!(entry.last_sent_ms, 3100);
}
assert_eq!(
node.stats().discovery.req_initiated,
node.metrics().discovery.req_initiated.get(),
baseline_initiated + 2,
"retry #2 must invoke initiate_lookup exactly once more"
);
@@ -1278,7 +1278,7 @@ async fn test_check_pending_lookups_default_sequence_unreachable() {
assert_eq!(entry.last_sent_ms, 7100);
}
assert_eq!(
node.stats().discovery.req_initiated,
node.metrics().discovery.req_initiated.get(),
baseline_initiated + 3,
"retry #3 must invoke initiate_lookup exactly once more"
);
@@ -1290,12 +1290,12 @@ async fn test_check_pending_lookups_default_sequence_unreachable() {
"8s window not yet expired: pending_lookup must persist"
);
assert_eq!(
node.stats().discovery.req_initiated,
node.metrics().discovery.req_initiated.get(),
baseline_initiated + 3,
"no new attempt before final deadline"
);
assert_eq!(
node.stats().discovery.resp_timed_out,
node.metrics().discovery.resp_timed_out.get(),
baseline_timed_out,
"no timeout before final deadline"
);
@@ -1314,13 +1314,13 @@ async fn test_check_pending_lookups_default_sequence_unreachable() {
);
// resp_timed_out counter ticked.
assert_eq!(
node.stats().discovery.resp_timed_out,
node.metrics().discovery.resp_timed_out.get(),
baseline_timed_out + 1,
"final timeout must increment discovery.resp_timed_out"
);
// No additional initiate_lookup on the timeout step.
assert_eq!(
node.stats().discovery.req_initiated,
node.metrics().discovery.req_initiated.get(),
baseline_initiated + 3,
"the final-timeout step must NOT call initiate_lookup"
);
+3 -2
View File
@@ -757,8 +757,9 @@ fn test_detect_congestion_with_transport_drops() {
#[test]
fn test_detect_congestion_disabled_ecn() {
let mut node = make_node();
node.config.node.ecn.enabled = false;
let mut config = Config::new();
config.node.ecn.enabled = false;
let mut node = Node::new(config).unwrap();
// Even with transport drops, disabled ECN should return false
let tid = TransportId::new(1);
+9 -1
View File
@@ -8,6 +8,7 @@ mod acl;
#[cfg(target_os = "linux")]
mod ble;
mod bloom;
mod bloom_metrics;
mod bloom_poison;
mod bootstrap;
mod decrypt_failure;
@@ -24,7 +25,14 @@ mod tcp;
mod unit;
pub(super) fn make_node() -> Node {
let config = Config::new();
make_node_with(Config::new())
}
/// Build a test node from an explicit `Config`. Prefer this over poking
/// `node.config.*` after construction: immutable fields are mirrored into the
/// shared `NodeContext` at build time, so a post-construction field poke is
/// invisible to any reader that has migrated onto the `config()` accessor.
pub(super) fn make_node_with(config: Config) -> Node {
Node::new(config).unwrap()
}
+3 -2
View File
@@ -1468,8 +1468,9 @@ fn test_purge_idle_sessions_cleans_pending_packets() {
#[test]
fn test_purge_idle_sessions_disabled_when_zero() {
let mut node = make_node();
node.config.node.session.idle_timeout_secs = 0;
let mut config = Config::new();
config.node.session.idle_timeout_secs = 0;
let mut node = make_node_with(config);
let remote = Identity::generate();
let remote_addr = *remote.node_addr();
+2 -2
View File
@@ -832,7 +832,7 @@ async fn test_rejects_tree_announce_with_inconsistent_root() {
.coords()
.unwrap()
.clone();
let accepted_before = nodes[1].node.stats().tree.accepted;
let accepted_before = nodes[1].node.metrics().tree.accepted.get();
// Use two fixed synthetic ancestors so the forged path is explicit:
// - fake_parent = 00000000000000000000000000000000
@@ -877,7 +877,7 @@ async fn test_rejects_tree_announce_with_inconsistent_root() {
nodes[1].node.tree_state().my_coords().depth(),
current_depth
);
assert_eq!(nodes[1].node.stats().tree.accepted, accepted_before);
assert_eq!(nodes[1].node.metrics().tree.accepted.get(), accepted_before);
assert_eq!(
nodes[1].node.get_peer(&a_addr).unwrap().coords().unwrap(),
&peer_coords_before
+23 -14
View File
@@ -6,7 +6,7 @@
//! All tests use 127.0.0.1:0 (ephemeral ports) and need no privileges.
use super::*;
use crate::config::TcpConfig;
use crate::config::{Config, TcpConfig};
use crate::transport::tcp::TcpTransport;
use crate::transport::{TransportAddr, TransportHandle, TransportId, packet_channel};
use spanning_tree::{
@@ -20,7 +20,14 @@ use std::time::Duration;
/// TcpTransport instead of UDP. Binds to 127.0.0.1:0 for an
/// ephemeral port.
async fn make_test_node_tcp() -> TestNode {
let mut node = make_node();
make_test_node_tcp_with(Config::new()).await
}
/// Like `make_test_node_tcp` but builds the node from an explicit `Config`,
/// so immutable fields (e.g. heartbeat/link-dead timeouts) are set before the
/// `NodeContext` is built rather than poked afterward.
async fn make_test_node_tcp_with(config: Config) -> TestNode {
let mut node = make_node_with(config);
let transport_id = TransportId::new(1);
let config = TcpConfig {
@@ -166,13 +173,14 @@ async fn test_tcp_mixed_transport_coexistence() {
/// link-dead timeout fires.
#[tokio::test]
async fn test_tcp_connection_loss_detection() {
let mut nodes = vec![make_test_node_tcp().await, make_test_node_tcp().await];
// Short heartbeat/link-dead timeouts for faster test execution
for tn in nodes.iter_mut() {
tn.node.config.node.heartbeat_interval_secs = 1;
tn.node.config.node.link_dead_timeout_secs = 3;
}
let mut config = Config::new();
config.node.heartbeat_interval_secs = 1;
config.node.link_dead_timeout_secs = 3;
let mut nodes = vec![
make_test_node_tcp_with(config.clone()).await,
make_test_node_tcp_with(config).await,
];
// Establish peering
initiate_handshake(&mut nodes, 0, 1).await;
@@ -215,13 +223,14 @@ async fn test_tcp_connection_loss_detection() {
/// Verifies that bidirectional peering is restored.
#[tokio::test]
async fn test_tcp_reconnection_after_link_death() {
let mut nodes = vec![make_test_node_tcp().await, make_test_node_tcp().await];
// Short timeouts
for tn in nodes.iter_mut() {
tn.node.config.node.heartbeat_interval_secs = 1;
tn.node.config.node.link_dead_timeout_secs = 3;
}
let mut config = Config::new();
config.node.heartbeat_interval_secs = 1;
config.node.link_dead_timeout_secs = 3;
let mut nodes = vec![
make_test_node_tcp_with(config.clone()).await,
make_test_node_tcp_with(config).await,
];
// Establish initial peering
initiate_handshake(&mut nodes, 0, 1).await;
+32
View File
@@ -1006,6 +1006,38 @@ fn active_peer_same_path_discovery_refreshes_stale_peer() {
));
}
#[tokio::test]
async fn node_context_mirrors_config_and_immutable_facades() {
let mut node = make_node();
// The immutable facades read the shared NodeContext.
let expected_addr = *node.identity().node_addr();
assert_eq!(node.node_addr(), &expected_addr);
assert!(!node.is_leaf_only());
let _ = node.uptime();
assert_eq!(node.config().peers().len(), 0);
// update_peers must rebuild the context so config() — which now reads the
// context — reflects the new peer list. Guards the copy-on-write sync.
let peer = Identity::generate();
let new_peer = crate::config::PeerConfig {
npub: peer.npub(),
alias: None,
addresses: vec![],
connect_policy: crate::config::ConnectPolicy::OnDemand,
auto_reconnect: false,
via_nostr: false,
};
node.update_peers(vec![new_peer]).await.unwrap();
assert_eq!(
node.config().peers().len(),
1,
"config() must reflect update_peers through the rebuilt context"
);
assert_eq!(node.config().peers()[0].npub, peer.npub());
}
#[tokio::test]
async fn update_peers_races_new_alternative_without_dropping_active_peer() {
let mut node = make_node();