mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-09 08:14:42 +00:00
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, capability limits). Source the immutable config and identity reads across the receive hot path, the handshake/session/mmp/encrypted state machines, and the discovery, tree, bloom, retry, and lifecycle modules through the context accessors rather than direct field reads. The Node fields and the context are rebuilt in lockstep at every mutation site.
This commit is contained in:
@@ -43,24 +43,22 @@ async fn test_m1_rejects_all_ones_filter_announce() {
|
||||
let announce = FilterAnnounce::new(all_ones, 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 the 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::new(filter, 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);
|
||||
}
|
||||
|
||||
@@ -292,11 +292,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);
|
||||
@@ -329,7 +330,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(),
|
||||
@@ -345,7 +345,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);
|
||||
|
||||
@@ -1110,8 +1110,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;
|
||||
@@ -1124,7 +1124,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"
|
||||
);
|
||||
@@ -1140,7 +1140,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"
|
||||
);
|
||||
@@ -1156,7 +1156,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"
|
||||
);
|
||||
@@ -1168,12 +1168,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"
|
||||
);
|
||||
@@ -1192,13 +1192,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"
|
||||
);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -24,7 +24,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()
|
||||
}
|
||||
|
||||
|
||||
@@ -1470,8 +1470,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();
|
||||
|
||||
@@ -830,7 +830,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
|
||||
@@ -875,7 +875,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
@@ -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;
|
||||
|
||||
@@ -1003,6 +1003,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();
|
||||
|
||||
Reference in New Issue
Block a user