diff --git a/src/lib.rs b/src/lib.rs index f2c94ec..fd1097e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -92,7 +92,7 @@ pub use cache::{CacheEntry, CacheError, CacheStats, CoordCache}; pub use proto::fmp::{PromotionResult, cross_connection_winner}; // Re-export peer types -pub use peer::{ActivePeer, ConnectivityState, PeerConnection, PeerError}; +pub use peer::{ActivePeer, ConnectivityState, PeerError}; // Re-export node types pub use node::{Node, NodeError, NodeState, UpdatePeersOutcome}; diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index 21f41e3..ec2280e 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -6,11 +6,11 @@ use crate::node::acl::PeerAclContext; use crate::node::dataplane::PeerActionCtx; use crate::node::reject::{HandshakeReject, RejectReason}; use crate::node::{Node, NodeError}; +use crate::peer::ActivePeer; use crate::peer::machine::{ - CrossConnOutcome, FailReason, HandshakePhase, PeerAction, PeerEvent, PeerMachine, PeerState, - TimerKind, + CrossConnOutcome, FailReason, HandshakeCrypto, HandshakePhase, PeerAction, PeerEvent, + PeerMachine, PeerState, TimerKind, }; -use crate::peer::{ActivePeer, PeerConnection}; use crate::proto::fmp::wire::{Msg1Header, Msg2Header, build_msg2}; use crate::proto::fmp::{ EstablishSnapshot, EstablishView, InboundDecision, InboundReject, OutboundSnapshot, @@ -264,27 +264,17 @@ impl Node { // === CRYPTO COST PAID HERE === let link_id = self.allocate_link_id(); - let conn = PeerConnection::inbound_with_transport( - link_id, - packet.transport_id, - packet.remote_addr.clone(), - packet.timestamp_ms, - ); // The control machine drives the handshake, so it is built here, above - // the crypto, carrying the pending connection. It stays a local: it - // enters `peer_machines` only at the promote tails, so a rejected msg1 - // still leaves no registry trace and allocates no index. + // the crypto. It stays a local: it enters `peer_machines` only at the + // promote tails, so a rejected msg1 still leaves no registry trace and + // allocates no index. let mut machine = PeerMachine::new_inbound(link_id, packet.timestamp_ms); - // The inbound connection carries the transport ID from msg1, but the - // machine's carrier is only written on the outbound dial. Seed it here - // so the promotion hand-off reads it from the surviving carrier, - // matching the connection's own inbound seed. + // Seed the carrier with the transport and address msg1 arrived on, so + // the promotion hand-off reads them from it. machine.set_conn_transport_id(packet.transport_id); - // The inbound connection is constructed carrying the peer's address; - // seed the surviving carrier with it at the same point. machine.set_conn_source_addr(packet.remote_addr.clone()); - machine.set_leg(conn); + machine.set_leg(HandshakeCrypto::new()); let our_keypair = self.identity().keypair(); let noise_msg1 = &packet.data[header.noise_msg1_offset..]; @@ -328,10 +318,7 @@ impl Node { // state; from here the decision reads only `wire` and the snapshot. let wire = WireOutcome { peer_identity, - remote_epoch: machine - .leg() - .expect("pending connection attached above") - .remote_epoch(), + remote_epoch: machine.conn_remote_epoch(), their_index: header.sender_idx, msg2_payload: msg2_response, }; @@ -900,8 +887,8 @@ impl Node { }; // Check if this is a rekey msg2: the handshake state is on the - // ActivePeer (not a PeerConnection), so the link's machine — if one - // survives at all — carries no pending connection. A bare machine + // ActivePeer, not in a handshake carrier, so the link's machine — if + // one survives at all — carries no pending handshake. A bare machine // lookup would NOT discriminate here: an established peer's machine // stays keyed by this link, so the pending connection's presence is // what marks a fresh establish. Look for a peer with matching @@ -1392,6 +1379,7 @@ impl Node { let carrier_transport_id = machine.conn_transport_id(); let carrier_source_addr = machine.conn_source_addr().cloned(); let carrier_is_outbound = machine.conn_is_outbound(); + let carrier_remote_epoch = machine.conn_remote_epoch(); let link_stats = machine.conn_link_stats().clone(); // Verify handshake is complete and extract session @@ -1420,7 +1408,7 @@ impl Node { link_id, reason: "missing source_addr".into(), })?; - let remote_epoch = connection.remote_epoch(); + let remote_epoch = carrier_remote_epoch; let peer_node_addr = *verified_identity.node_addr(); let is_outbound = carrier_is_outbound; diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index ad7267c..e950d11 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -193,14 +193,14 @@ impl Node { .collect(); for link in timer_links { // The idle-timeout threshold reads the survivor carrier's - // last-activity (the leg no longer projects it); the leg still - // supplies direction/identity for the retry decision below. + // last-activity; presence of a pending handshake is what decides + // between reaping and dropping an orphan timer. let timed_out = self .peer_machines .get(&link) .is_some_and(|machine| machine.conn_is_timed_out(now_ms, timeout_ms)); - let (reap, retry_peer) = match self.leg(&link) { - Some(_) if timed_out => { + let (reap, retry_peer) = match self.has_pending_leg(&link) { + true if timed_out => { let retry_peer = if self .peer_machines .get(&link) @@ -216,8 +216,8 @@ impl Node { (true, retry_peer) } // Not yet idle-timed-out: leave the timer for a later tick. - Some(_) => (false, None), - None => { + true => (false, None), + false => { // Orphan timer (connection already reaped elsewhere) — drop it. if let Some(timers) = self.peer_timers.get_mut(&link) { timers.remove(&TimerKind::HandshakeTimeout); diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index 6df800e..f7ae0aa 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -15,8 +15,7 @@ use crate::node::acl::PeerAclContext; use crate::node::dataplane::PeerActionCtx; use crate::nostr::{BootstrapEvent, NostrRendezvous}; use crate::nostr::{BootstrapHandoffResult, EstablishedTraversal}; -use crate::peer::PeerConnection; -use crate::peer::machine::{PeerEvent, PeerMachine}; +use crate::peer::machine::{HandshakeCrypto, PeerEvent, PeerMachine}; use crate::proto::fmp::wire::build_msg1; use crate::proto::fmp::{Disconnect, DisconnectReason}; use crate::transport::{Link, LinkDirection, LinkId, TransportAddr, TransportId, packet_channel}; @@ -380,7 +379,7 @@ impl Node { }) } - fn is_connecting_to_peer_on_path( + pub(in crate::node) fn is_connecting_to_peer_on_path( &self, peer_node_addr: &NodeAddr, transport_id: TransportId, @@ -582,14 +581,12 @@ impl Node { ) -> Result<(), NodeError> { let peer_node_addr = *peer_identity.node_addr(); - // Create connection in handshake phase (outbound knows expected identity) let current_time_ms = Self::now_ms(); - let connection = PeerConnection::outbound(link_id, peer_identity, current_time_ms); - // The control machine drives the handshake, so it takes the connection - // before the crypto runs. The machine was born at dial and persisted in - // `initiate_connection`, so every live caller already has one; recover - // with a fresh one if a direct caller ever skips the dial. + // The control machine drives the handshake, so it takes the crypto + // carrier before the crypto runs. The machine was born at dial and + // persisted in `initiate_connection`, so every live caller already has + // one; recover with a fresh one if a direct caller ever skips the dial. debug_assert!( self.peer_machines.contains_key(&link_id), "outbound msg1 prepared for link {link_id} with no dial-time machine" @@ -597,7 +594,7 @@ impl Node { self.peer_machines .entry(link_id) .or_insert_with(|| PeerMachine::new_outbound(link_id, peer_identity, current_time_ms)) - .set_leg(connection); + .set_leg(HandshakeCrypto::new()); // Allocate a session index for this handshake let our_index = match self.index_allocator.allocate() { diff --git a/src/node/mod.rs b/src/node/mod.rs index 2d5fa71..c308633 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -37,8 +37,8 @@ use self::reloadable::Reloadable; pub(crate) const REKEY_JITTER_SECS: i64 = 15; use crate::cache::CoordCache; use crate::node::session::SessionEntry; +use crate::peer::ActivePeer; use crate::peer::machine::{PeerMachine, TimerKind}; -use crate::peer::{ActivePeer, PeerConnection}; use crate::proto::bloom::{BloomFilter, BloomState}; use crate::proto::fmp::Fmp; use crate::proto::fmp::wire::{ @@ -351,9 +351,9 @@ pub struct Node { // === Per-Peer Control Machines === /// Per-peer lifecycle control FSMs, keyed by the stable `LinkId` that spans - /// the handshake→active lifetime. Each machine owns its pending handshake - /// connection (the `PeerConnection` leg) while the handshake is in - /// progress — the single LinkId-keyed per-peer map on `Node`; `peers` + /// the handshake→active lifetime. Each machine owns its handshake crypto + /// carrier while the handshake is in progress — the single LinkId-keyed + /// per-peer map on `Node`; `peers` /// stays byte-unchanged (hot path pristine). /// Machines are inserted at dial and inbound msg1, and stepped in production /// by the handshake handlers, the rekey-cadence and liveness-reap routers, @@ -2405,114 +2405,29 @@ impl Node { // === Connection Management (Handshake Phase) === - /// The pending connection for `link_id`, read through the control machine - /// that carries it. - fn leg(&self, link_id: &LinkId) -> Option<&PeerConnection> { + /// Whether `link_id` has a pending handshake, read through the control + /// machine that carries it. + fn has_pending_leg(&self, link_id: &LinkId) -> bool { self.peer_machines .get(link_id) - .and_then(|machine| machine.leg()) - } - - /// Mutable access to the pending connection for `link_id`. - fn leg_mut(&mut self, link_id: &LinkId) -> Option<&mut PeerConnection> { - self.peer_machines - .get_mut(link_id) - .and_then(|machine| machine.leg_mut()) - } - - /// Add a pending connection. - /// - /// Seeds a control machine for the leg when none exists yet and embeds the - /// connection on it, for callers that insert a connection directly rather - /// than through the dial or inbound-msg1 paths, which build the machine - /// themselves. - pub fn add_connection(&mut self, connection: PeerConnection) -> Result<(), NodeError> { - let link_id = connection.state().link_id(); - - if self - .peer_machines - .get(&link_id) .is_some_and(|machine| machine.leg().is_some()) - { - return Err(NodeError::ConnectionAlreadyExists(link_id)); - } - - if self.max_connections() > 0 && self.connection_count() >= self.max_connections() { - return Err(NodeError::MaxConnectionsExceeded { - max: self.max_connections(), - }); - } - - let machine = self.peer_machines.entry(link_id).or_insert_with(|| { - let now = connection.state().started_at(); - match connection.state().expected_identity() { - Some(identity) if connection.state().is_outbound() => { - PeerMachine::new_outbound(link_id, *identity, now) - } - _ => PeerMachine::new_inbound(link_id, now), - } - }); - // Seed the surviving carrier's peer index and transport from the - // pre-built leg so the promotion hand-off reads them from the machine, - // matching the establish paths that write them on the machine directly. - if let Some(ours) = connection.state().our_index() { - machine.set_conn_our_index(ours); - } - if let Some(their) = connection.state().their_index() { - machine.set_conn_their_index(their); - } - if let Some(tid) = connection.state().transport_id() { - machine.set_conn_transport_id(tid); - } - if let Some(addr) = connection.state().source_addr() { - machine.set_conn_source_addr(addr.clone()); - } - machine.set_leg(connection); - Ok(()) } - /// Test-support: seed a control machine for `seed.link_id` the way - /// [`Node::add_connection`] does, without the caller having to build a - /// free-standing leg first. + /// Test-support: seed a control machine for `seed.link_id` directly, + /// without the caller having to stage a pending handshake by hand. /// - /// The carrier seeding below is a verbatim copy of `add_connection`'s: the - /// conditional writes (`our_index`, `their_index`, `transport_id`, - /// `source_addr`) and `set_leg`, built through the same - /// `entry(..).or_insert_with(..)` so an existing leg-less machine keeps its - /// constructor-side fields. The seeded carrier matches what the establish - /// paths write: every field a promotion reads is present. Post-construction - /// `started_at` and the stored handshake bytes are not seeded here, exactly - /// as they are not for a test that goes through `add_connection` today. - /// - /// The duplication is deliberate: keeping the carrier writes visible here - /// is what lets each later step of the leg dissolution revise them at a - /// single reviewable site. The two bodies must stay in sync until - /// `add_connection` itself is removed; any drift surfaces as a test - /// failure while both paths still exist. + /// The machine is chosen the way the establish paths choose it — outbound + /// when the seed names a peer, inbound otherwise — and its carrier is + /// seeded with every field a promotion reads. Built through + /// `entry(..).or_insert_with(..)`, so an existing handshake-less machine + /// keeps its constructor-side fields rather than being rebuilt. + /// Post-construction `started_at` and the stored handshake bytes are not + /// seeded; the establish paths write those at their own points. #[cfg(test)] pub(crate) fn seed_handshake_machine(&mut self, seed: HandshakeSeed) -> Result<(), NodeError> { let link_id = seed.link_id; - let mut connection = match seed.expected_identity { - Some(identity) => PeerConnection::outbound(link_id, identity, seed.started_at_ms), - None => PeerConnection::inbound(link_id, seed.started_at_ms), - }; - if let Some(id) = seed.transport_id { - connection.state_mut().set_transport_id(id); - } - let seeded_source_addr = seed.source_addr.clone(); - if let Some(index) = seed.our_index { - connection.state_mut().set_our_index(index); - } - if let Some(index) = seed.their_index { - connection.state_mut().set_their_index(index); - } - - if self - .peer_machines - .get(&link_id) - .is_some_and(|machine| machine.leg().is_some()) - { + if self.has_pending_leg(&link_id) { return Err(NodeError::ConnectionAlreadyExists(link_id)); } @@ -2522,54 +2437,31 @@ impl Node { }); } - let machine = self.peer_machines.entry(link_id).or_insert_with(|| { - let now = connection.state().started_at(); - match connection.state().expected_identity() { - Some(identity) if connection.state().is_outbound() => { - PeerMachine::new_outbound(link_id, *identity, now) - } - _ => PeerMachine::new_inbound(link_id, now), - } - }); - if let Some(ours) = connection.state().our_index() { - machine.set_conn_our_index(ours); + let started_at_ms = seed.started_at_ms; + let expected_identity = seed.expected_identity; + let machine = + self.peer_machines + .entry(link_id) + .or_insert_with(|| match expected_identity { + Some(identity) => PeerMachine::new_outbound(link_id, identity, started_at_ms), + None => PeerMachine::new_inbound(link_id, started_at_ms), + }); + if let Some(index) = seed.our_index { + machine.set_conn_our_index(index); } - if let Some(their) = connection.state().their_index() { - machine.set_conn_their_index(their); + if let Some(index) = seed.their_index { + machine.set_conn_their_index(index); } - if let Some(tid) = connection.state().transport_id() { - machine.set_conn_transport_id(tid); + if let Some(id) = seed.transport_id { + machine.set_conn_transport_id(id); } - if let Some(addr) = seeded_source_addr { + if let Some(addr) = seed.source_addr { machine.set_conn_source_addr(addr); } - machine.set_leg(connection); + machine.set_leg(crate::peer::machine::HandshakeCrypto::new()); Ok(()) } - /// Get a connection by LinkId. - pub fn get_connection(&self, link_id: &LinkId) -> Option<&PeerConnection> { - self.leg(link_id) - } - - /// Get a mutable connection by LinkId. - pub fn get_connection_mut(&mut self, link_id: &LinkId) -> Option<&mut PeerConnection> { - self.leg_mut(link_id) - } - - /// Remove a connection, disposing its control machine alongside - /// (the disposal complement of `add_connection`'s machine seeding). The - /// connection is taken off the machine BEFORE the machine is dropped, so - /// the caller still receives it. - pub fn remove_connection(&mut self, link_id: &LinkId) -> Option { - let connection = self - .peer_machines - .get_mut(link_id) - .and_then(|machine| machine.take_leg()); - self.remove_peer_machine(*link_id); - connection - } - /// Iterate over the control machines that carry a pending connection. /// /// Carrying a pending connection is what makes a machine handshake-phase, @@ -3279,10 +3171,9 @@ impl fmt::Debug for Node { /// Test-support seed spec for [`Node::seed_handshake_machine`]. /// -/// Mirrors the leg constructors plus the setters tests apply to a connection -/// *before* handing it to `add_connection`. Only fields that exist on the leg -/// at add time belong here; crypto is run afterwards through -/// `get_connection_mut`, which `add_connection` never reads. +/// Carries the carrier fields a seeded machine needs before any crypto runs. +/// Only fields the establish paths write at seed time belong here; the Noise +/// handshake is driven afterwards through the machine's own crypto methods. #[cfg(test)] #[derive(Debug, Clone)] pub(crate) struct HandshakeSeed { diff --git a/src/node/tests/establish_chartests.rs b/src/node/tests/establish_chartests.rs index 42fd01f..1004eba 100644 --- a/src/node/tests/establish_chartests.rs +++ b/src/node/tests/establish_chartests.rs @@ -243,7 +243,7 @@ async fn chartest_msg1_duplicate_pending_resends_stored_msg2() { ); assert_eq!(node.peer_count(), 0, "duplicate msg1 promotes nothing"); assert!( - node.get_connection(&link_id).is_some(), + node.has_pending_leg(&link_id), "pending connection is left intact" ); assert_eq!( @@ -376,7 +376,7 @@ async fn chartest_msg1_inbound_promote_defers_pending_outbound_to_same_identity( assert!(peer.has_session()); assert_eq!(node.peer_count(), 1); assert!( - node.get_connection(&out_link).is_some(), + node.has_pending_leg(&out_link), "pending outbound to the same identity must be preserved (deferred cleanup)" ); assert!( diff --git a/src/node/tests/handshake.rs b/src/node/tests/handshake.rs index 4c535bd..67f9395 100644 --- a/src/node/tests/handshake.rs +++ b/src/node/tests/handshake.rs @@ -814,7 +814,7 @@ async fn test_failed_connection_cleanup() { let machine = node .peer_machines .get_mut(&link_id) - .expect("machine seeded by add_connection"); + .expect("machine seeded by the handshake seeder"); let alloc = &mut node.index_allocator; let actions = machine.step( crate::peer::machine::PeerEvent::HandshakeSendFailed, diff --git a/src/node/tests/mod.rs b/src/node/tests/mod.rs index a302e26..9d80748 100644 --- a/src/node/tests/mod.rs +++ b/src/node/tests/mod.rs @@ -1,5 +1,6 @@ use super::*; use crate::PeerIdentity; +use crate::peer::machine::HandshakeCrypto; use crate::transport::{LinkDirection, ReceivedPacket, TransportAddr, packet_channel}; use crate::utils::index::SessionIndex; use std::time::Duration; @@ -93,18 +94,14 @@ pub(super) fn outbound_leg( current_time_ms: u64, ) -> PeerMachine { let mut machine = PeerMachine::new_outbound(link_id, expected_identity, current_time_ms); - machine.set_leg(PeerConnection::outbound( - link_id, - expected_identity, - current_time_ms, - )); + machine.set_leg(HandshakeCrypto::new()); machine } /// The responder twin of [`outbound_leg`]. pub(super) fn inbound_leg(link_id: LinkId, current_time_ms: u64) -> PeerMachine { let mut machine = PeerMachine::new_inbound(link_id, current_time_ms); - machine.set_leg(PeerConnection::inbound(link_id, current_time_ms)); + machine.set_leg(HandshakeCrypto::new()); machine } @@ -112,8 +109,7 @@ pub(super) fn inbound_leg(link_id: LinkId, current_time_ms: u64) -> PeerMachine /// /// Returns the peer identity. The leg is outbound, in Complete state, with /// session, indices, and transport info set, and is installed on the node -/// through [`Node::seed_handshake_machine`] — the test-surface twin of -/// `Node::add_connection`. +/// through [`Node::seed_handshake_machine`]. pub(super) fn seed_completed_connection( node: &mut Node, link_id: LinkId, diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 280bbf4..63a1ea6 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -342,9 +342,9 @@ fn test_node_connection_management() { assert_eq!(node.connection_count(), 1); - assert!(node.get_connection(&link_id).is_some()); + assert!(node.has_pending_leg(&link_id)); - node.remove_connection(&link_id); + node.remove_peer_machine(link_id); assert_eq!(node.connection_count(), 0); } @@ -364,7 +364,7 @@ fn test_node_connection_duplicate() { #[cfg(debug_assertions)] #[test] -fn test_peer_maps_coherent_after_add_connection() { +fn test_peer_maps_coherent_after_seeding_a_handshake() { let mut node = make_node(); let identity = make_peer_identity(); @@ -374,7 +374,7 @@ fn test_peer_maps_coherent_after_add_connection() { assert!( node.peer_machines.contains_key(&link_id), - "add_connection seeds a control machine for its leg" + "seeding a handshake creates its control machine" ); node.debug_assert_peer_maps_coherent(); } @@ -2067,7 +2067,7 @@ fn nostr_rendezvous_outbound_admission_atomic_roundtrip() { /// to `addr_b`. Returns the sender's NodeAddr so the test can assert on /// identity-keyed maps. /// -/// Uses the same outbound-PeerConnection->Noise IK pattern as the +/// Uses the same outbound-machine->Noise IK pattern as the /// integration handshake tests, but inlined and unit-scoped. async fn craft_and_send_msg1( node_b: &Node, @@ -2405,6 +2405,126 @@ fn test_failed_connection_is_retained_and_reaped() { assert_eq!(node.connection_count(), 0); } +/// Handshake-phase membership is decided by whether a crypto carrier is +/// ATTACHED, never by whether either Noise handle inside it is populated. +/// +/// The distinction is the whole reason the carrier is a struct rather than a +/// pair of bare handle fields. A carrier legitimately sits attached and empty: +/// `mark_failed` drops the initiation handle and deliberately keeps the +/// carrier so the sweep can reclaim it, `take_session` empties the other, and +/// every handshake begins with both handles unset. If presence were derived +/// from the handles, every failed handshake would vanish from the sweep, +/// the count, and the peering budget at once — a permanent leak that no +/// existing test would notice. +/// +/// This drives the empty-carrier shape past every presence predicate on +/// `Node` and asserts each one reports "present", then detaches and asserts +/// each reports "absent". +#[test] +fn handshake_presence_tracks_the_carrier_not_the_noise_handles() { + use crate::proto::fmp::LifecycleView; + + let mut node = make_node(); + let link_id = LinkId::new(31); + let transport_id = TransportId::new(9); + let peer_identity = make_peer_identity(); + let peer_addr = TransportAddr::from_string("10.0.0.9:9999"); + + node.seed_handshake_machine( + HandshakeSeed::outbound(link_id, peer_identity, 1000) + .with_transport_id(transport_id) + .with_source_addr(peer_addr.clone()), + ) + .unwrap(); + + // A freshly seeded carrier holds neither handle — the construction window. + let leg = node.peer_machines.get(&link_id).unwrap().leg().unwrap(); + assert!( + leg.noise_handshake.is_none() && leg.noise_session.is_none(), + "the seeded carrier must start with both handles empty" + ); + + // Every presence predicate must see it, handles or not. + let assert_present = |node: &Node, when: &str| { + assert_eq!(node.connection_count(), 1, "connection_count: {when}"); + assert_eq!(node.connections().count(), 1, "connections(): {when}"); + assert!(node.has_pending_leg(&link_id), "has_pending_leg: {when}"); + assert_eq!( + node.stale_connections(1_000_000, 30_000).len(), + 1, + "stale_connections: {when}" + ); + assert!( + node.is_connecting_to_peer_on_path(peer_identity.node_addr(), transport_id, &peer_addr), + "is_connecting_to_peer_on_path: {when}" + ); + // The last two predicates sit inline in functions with no callable + // seam, so these MIRROR them rather than exercising them: the shape is + // pinned here, but a mutation at the production site would not fail + // this test. Both sites read `machine.leg().is_some()` verbatim. + assert!( + node.peer_machines.values().any(|machine| { + machine.leg().is_some() && machine.conn_transport_id() == Some(transport_id) + }), + "transport-in-use: {when}" + ); + // The complement of the rekey-msg2 discriminator: a machine carrying + // a pending handshake marks a fresh establish, so `handle_msg2` must + // NOT take its rekey-completion branch. + assert!( + node.peer_machines + .get(&link_id) + .is_some_and(|machine| machine.leg().is_some()), + "rekey-msg2 discriminator: {when}" + ); + // Fires the live-carrier coherence assertion; a machine that had gone + // invisible would panic here rather than fail an assert_eq above. + node.debug_assert_peer_maps_coherent(); + }; + + assert_present(&node, "freshly seeded, both handles empty"); + + // Drive to the failed shape: the initiation handle is dropped and the + // carrier is deliberately retained for the sweep. + let our_keypair = node.identity().keypair(); + let startup_epoch = node.startup_epoch(); + let machine = node.peer_machines.get_mut(&link_id).unwrap(); + machine + .start_handshake(our_keypair, startup_epoch, 1000) + .unwrap(); + assert!( + machine.leg().unwrap().noise_handshake.is_some(), + "start_handshake arms the initiation handle" + ); + machine.mark_failed(); + machine.mark_send_failed(); + + let leg = node.peer_machines.get(&link_id).unwrap().leg().unwrap(); + assert!( + leg.noise_handshake.is_none() && leg.noise_session.is_none(), + "a failed handshake holds neither handle" + ); + assert_present(&node, "failed, both handles empty"); + + // Detaching the carrier — and only that — ends handshake-phase membership. + node.peer_machines.get_mut(&link_id).unwrap().take_leg(); + assert_eq!(node.connection_count(), 0, "connection_count after detach"); + assert_eq!(node.connections().count(), 0, "connections() after detach"); + assert!( + !node.has_pending_leg(&link_id), + "has_pending_leg after detach" + ); + assert_eq!( + node.stale_connections(1_000_000, 30_000).len(), + 0, + "stale_connections after detach" + ); + assert!( + !node.is_connecting_to_peer_on_path(peer_identity.node_addr(), transport_id, &peer_addr), + "is_connecting_to_peer_on_path after detach" + ); +} + /// The identity a responder discovers in msg1 must land on the surviving /// carrier, not only on the pending leg. Everything that names an inbound /// peer mid-handshake reads the carrier: the stale-connection sweep's @@ -2424,11 +2544,7 @@ fn inbound_msg1_records_the_learned_identity_on_the_carrier() { let initiator_link = LinkId::new(78); let mut initiator = crate::peer::machine::PeerMachine::new_outbound(initiator_link, node_identity, 1000); - initiator.set_leg(crate::peer::PeerConnection::outbound( - initiator_link, - node_identity, - 1000, - )); + initiator.set_leg(crate::peer::machine::HandshakeCrypto::new()); let noise_msg1 = initiator .start_handshake(sender.keypair(), [9u8; 8], 1000) .unwrap(); diff --git a/src/peer/active.rs b/src/peer/active.rs index 9690430..3b5ace3 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -315,7 +315,7 @@ impl ActivePeer { /// Create from verified identity with existing link stats. /// - /// Used when promoting from PeerConnection, preserving handshake stats. + /// Used when promoting a completed handshake, preserving its link stats. /// For peers with Noise sessions, use `with_session` instead. pub fn with_stats( identity: PeerIdentity, diff --git a/src/peer/connection.rs b/src/peer/connection.rs deleted file mode 100644 index 4a26caf..0000000 --- a/src/peer/connection.rs +++ /dev/null @@ -1,149 +0,0 @@ -//! Peer Connection (Handshake Phase) -//! -//! Represents an in-progress connection before authentication completes. -//! PeerConnection tracks the Noise IK handshake and transitions to -//! ActivePeer upon successful authentication. Neither the handshake *phase* -//! (initial / sent_msg1 / complete / failed) nor the handshake operations are -//! tracked here — both live on the per-peer control machine, which drives the -//! Noise handles held below. - -use crate::PeerIdentity; -use crate::noise::{self, NoiseSession}; -use crate::proto::fmp::ConnectionState; -use crate::transport::{LinkId, TransportAddr, TransportId}; -use std::fmt; - -/// A connection in the handshake phase, before authentication completes. -/// -/// For outbound connections, we know the expected peer identity from config. -/// For inbound connections, we learn the identity during the Noise handshake. -/// -/// This is the shell holder for the FMP crypto/state split: the pure -/// connection bookkeeping lives in [`ConnectionState`] (`proto::fmp::state`), -/// and the two Noise crypto handles stay here beside it. Pure public methods -/// delegate to `self.state`; the control machine drives the handles and -/// records each result here and on its own carrier. -pub struct PeerConnection { - /// Pure, runtime-agnostic connection bookkeeping. - state: ConnectionState, - - /// Noise handshake state (consumes on completion). - /// - /// Driven by the control machine, which owns the handshake operations. - pub(crate) noise_handshake: Option, - - /// Completed Noise session (available after handshake complete). - /// - /// Driven by the control machine, which owns the handshake operations. - pub(crate) noise_session: Option, -} - -impl PeerConnection { - /// Create a new outbound connection (we are initiating). - /// - /// For outbound, we know who we're trying to reach from configuration. - /// The Noise handshake will be initialized when `start_handshake` is called. - pub fn outbound( - link_id: LinkId, - expected_identity: PeerIdentity, - current_time_ms: u64, - ) -> Self { - Self { - state: ConnectionState::outbound(link_id, expected_identity, current_time_ms), - noise_handshake: None, - noise_session: None, - } - } - - /// Create a new inbound connection (they are initiating). - /// - /// For inbound, we don't know who they are until we decrypt their - /// identity from Noise message 1. - pub fn inbound(link_id: LinkId, current_time_ms: u64) -> Self { - Self { - state: ConnectionState::inbound(link_id, current_time_ms), - noise_handshake: None, - noise_session: None, - } - } - - /// Create a new inbound connection with transport information. - /// - /// Used when processing msg1 where we know the transport and source address. - pub fn inbound_with_transport( - link_id: LinkId, - transport_id: TransportId, - source_addr: TransportAddr, - current_time_ms: u64, - ) -> Self { - Self { - state: ConnectionState::inbound_with_transport( - link_id, - transport_id, - source_addr, - current_time_ms, - ), - noise_handshake: None, - noise_session: None, - } - } - - // === Epoch Accessors === - - /// Get the remote peer's startup epoch (available after handshake). - pub fn remote_epoch(&self) -> Option<[u8; 8]> { - self.state.remote_epoch() - } - - // === Crypto handle plumbing (the control machine drives the handshake) === - - /// Mutable access to the pure bookkeeping, so the control machine's - /// handshake operations can record their results here as well as on the - /// surviving carrier. - pub(crate) fn state(&self) -> &ConnectionState { - &self.state - } - - pub(crate) fn state_mut(&mut self) -> &mut ConnectionState { - &mut self.state - } -} - -impl fmt::Debug for PeerConnection { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("PeerConnection") - .field("link_id", &self.state.link_id()) - .field("direction", &self.state.direction()) - .field("expected_identity", &self.state.expected_identity()) - .field("has_noise_handshake", &self.noise_handshake.is_some()) - .field("has_noise_session", &self.noise_session.is_some()) - .field("our_index", &self.state.our_index()) - .field("their_index", &self.state.their_index()) - .field("transport_id", &self.state.transport_id()) - .field("started_at", &self.state.started_at()) - .field("last_activity", &self.state.last_activity()) - .finish() - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::Identity; - - fn make_peer_identity() -> PeerIdentity { - let identity = Identity::generate(); - PeerIdentity::from_pubkey(identity.pubkey()) - } - - #[test] - fn test_connection_timing() { - let identity = make_peer_identity(); - let conn = PeerConnection::outbound(LinkId::new(1), identity, 1000); - - assert_eq!(conn.state().duration(1500), 500); - assert_eq!(conn.state().idle_time(1500), 500); - assert!(!conn.state().is_timed_out(1500, 1000)); - assert!(conn.state().is_timed_out(2500, 1000)); - } -} diff --git a/src/peer/machine.rs b/src/peer/machine.rs index f3adcc4..115aeef 100644 --- a/src/peer/machine.rs +++ b/src/peer/machine.rs @@ -57,7 +57,6 @@ #![allow(dead_code)] use crate::noise::{self, NoiseError, NoiseSession}; -use crate::peer::PeerConnection; use crate::proto::fmp::{ ConnAction, ConnSnapshot, ConnectionState, EstablishSnapshot, Fmp, InboundDecision, OutboundDecision, OutboundSnapshot, PeerSnapshot, PromotionResult, RekeyCfg, @@ -417,6 +416,46 @@ fn no_pending_connection() -> NoiseError { } } +/// The handshake-phase Noise crypto, owned by the control machine. +/// +/// PRESENCE OF THIS STRUCT (`PeerMachine::leg().is_some()`) IS THE +/// HANDSHAKE-PHASE CARRIER SIGNAL — it is what `Node::connections()`, +/// `connection_count()`, the stale-connection sweep, the transport-in-use +/// check, and the peering budget all key on. It is attached and detached at +/// exactly the points the pending connection was, and its presence is NOT a +/// function of whether either handle is populated. +/// +/// A present-but-empty value is legal and load-bearing: `mark_failed` drops +/// the initiation handle while deliberately retaining the carrier so the +/// sweep can reclaim it, and `take_session` empties the other. Deriving +/// presence from handle presence would make every failed handshake invisible +/// to the sweep — a permanent leak. See the presence tests in this module. +pub(crate) struct HandshakeCrypto { + /// Noise handshake state (consumed on completion). + pub(crate) noise_handshake: Option, + /// Completed Noise session (available once the handshake completes). + pub(crate) noise_session: Option, +} + +impl HandshakeCrypto { + /// A fresh carrier holding neither handle, as every handshake begins. + pub(crate) fn new() -> Self { + Self { + noise_handshake: None, + noise_session: None, + } + } +} + +impl std::fmt::Debug for HandshakeCrypto { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("HandshakeCrypto") + .field("has_noise_handshake", &self.noise_handshake.is_some()) + .field("has_noise_session", &self.noise_session.is_some()) + .finish() + } +} + /// Per-peer control FSM. Holds control-tier lifecycle state only; the /// send-critical state is published as `PeerSendState` and mutated via the /// emitted [`PeerAction`]s. @@ -424,13 +463,13 @@ pub(crate) struct PeerMachine { state: PeerState, link: LinkId, identity: Option, - /// The pending handshake connection this machine owns while it is in the - /// handshake window. `None` before the connection is built (the dial - /// window) and after promotion consumes it (the machine survives as the - /// active peer's control machine). Its bookkeeping is storage the shell - /// reaches through the accessors below; its Noise handles are driven by - /// this machine's handshake operations. - leg: Option, + /// The handshake-phase Noise crypto this machine owns while it is in the + /// handshake window. `None` before the handshake begins (the dial window) + /// and after promotion consumes it (the machine survives as the active + /// peer's control machine). Its presence — not the state of the handles + /// inside it — is what marks this machine as carrying a pending + /// handshake; see [`HandshakeCrypto`]. + leg: Option, /// Pure handshake-phase bookkeeping (link/direction/indices/transport/ /// stored handshake bytes/epoch). Reused verbatim from the FMP state core. conn: ConnectionState, @@ -532,25 +571,21 @@ impl PeerMachine { self.state } - /// The pending handshake connection, if this leg is still in the - /// handshake window. - pub(crate) fn leg(&self) -> Option<&PeerConnection> { + /// The handshake crypto carrier, if this machine is still in the + /// handshake window. Presence answers "is there a pending handshake + /// here", independently of whether either handle is populated. + pub(crate) fn leg(&self) -> Option<&HandshakeCrypto> { self.leg.as_ref() } - /// Mutable access to the pending handshake connection. - pub(crate) fn leg_mut(&mut self) -> Option<&mut PeerConnection> { - self.leg.as_mut() - } - - /// Take the pending handshake connection off the machine (promotion and + /// Take the handshake crypto carrier off the machine (promotion and /// teardown consume it by value). - pub(crate) fn take_leg(&mut self) -> Option { + pub(crate) fn take_leg(&mut self) -> Option { self.leg.take() } - /// Embed a pending handshake connection on the machine. - pub(crate) fn set_leg(&mut self, leg: PeerConnection) { + /// Attach a handshake crypto carrier to the machine. + pub(crate) fn set_leg(&mut self, leg: HandshakeCrypto) { self.leg = Some(leg); } @@ -593,7 +628,6 @@ impl PeerMachine { let msg1 = hs.write_message_1()?; leg.noise_handshake = Some(hs); - leg.state_mut().touch(current_time_ms); msg1 }; @@ -636,11 +670,9 @@ impl PeerMachine { .remote_static() .expect("remote static available after msg1"); let learned_identity = PeerIdentity::from_pubkey_full(remote_static); - leg.state_mut().set_expected_identity(learned_identity); // Capture remote epoch from msg1 let remote_epoch = hs.remote_epoch(); - leg.state_mut().set_remote_epoch(remote_epoch); // Generate message 2 let msg2 = hs.write_message_2()?; @@ -648,7 +680,6 @@ impl PeerMachine { // Handshake is complete for responder let session = hs.into_session()?; leg.noise_session = Some(session); - leg.state_mut().touch(current_time_ms); (msg2, learned_identity, remote_epoch) }; @@ -690,11 +721,9 @@ impl PeerMachine { // Capture remote epoch from msg2 let remote_epoch = hs.remote_epoch(); - leg.state_mut().set_remote_epoch(remote_epoch); let session = hs.into_session()?; leg.noise_session = Some(session); - leg.state_mut().touch(current_time_ms); remote_epoch }; @@ -785,6 +814,14 @@ impl PeerMachine { self.conn.expected_identity() } + /// Remote startup epoch of the surviving carrier, recorded by the + /// handshake operations at the message that reveals it (msg1 inbound, + /// msg2 outbound). Promotion reads it to seed the active peer and to + /// detect a peer restart across a reconnect. + pub(crate) fn conn_remote_epoch(&self) -> Option<[u8; 8]> { + self.conn.remote_epoch() + } + /// Stored wire-format msg1 of the surviving carrier — the resend source for /// the outbound handshake retransmit, now that the leg no longer carries it. pub(crate) fn conn_handshake_msg1(&self) -> Option<&[u8]> { @@ -860,17 +897,14 @@ impl PeerMachine { self.conn.link_stats() } - /// Record the peer session index on the surviving carrier. Seeds the - /// carrier from a pre-built leg (`Node::add_connection`) so the promotion - /// hand-off matches the establish paths that write it on the machine. + /// Record the peer session index on the surviving carrier, so the + /// promotion hand-off reads it from the machine. pub(crate) fn set_conn_their_index(&mut self, index: SessionIndex) { self.conn.set_their_index(index); } - /// Record the transport ID on the surviving carrier. Populated on the - /// inbound establish path (the leg seeds it at msg1, but the machine's - /// carrier is only written on the outbound dial) and when seeding the - /// carrier from a pre-built leg (`Node::add_connection`). + /// Record the transport ID on the surviving carrier. Written on the + /// inbound establish path at msg1 and on the outbound dial. pub(crate) fn set_conn_transport_id(&mut self, id: TransportId) { self.conn.set_transport_id(id); } @@ -2871,7 +2905,7 @@ mod tests { 100, &mut alloc, ); - m.set_leg(PeerConnection::outbound(LinkId::new(1), peer, 100)); + m.set_leg(HandshakeCrypto::new()); assert!(m.is_handshaking_sent_msg1()); assert!(!m.is_failed()); assert_eq!(m.displayed_handshake_state(), "sent_msg1"); @@ -3220,17 +3254,13 @@ mod tests { current_time_ms: u64, ) -> PeerMachine { let mut machine = PeerMachine::new_outbound(link_id, expected_identity, current_time_ms); - machine.set_leg(PeerConnection::outbound( - link_id, - expected_identity, - current_time_ms, - )); + machine.set_leg(HandshakeCrypto::new()); machine } fn inbound_leg(link_id: LinkId, current_time_ms: u64) -> PeerMachine { let mut machine = PeerMachine::new_inbound(link_id, current_time_ms); - machine.set_leg(PeerConnection::inbound(link_id, current_time_ms)); + machine.set_leg(HandshakeCrypto::new()); machine } @@ -3294,20 +3324,14 @@ mod tests { assert_eq!(discovered.pubkey(), initiator_identity.pubkey()); // Responder learned initiator's epoch - assert_eq!( - responder_conn.leg().unwrap().remote_epoch(), - Some(initiator_epoch) - ); + assert_eq!(responder_conn.conn_remote_epoch(), Some(initiator_epoch)); // Initiator completes handshake initiator_conn.complete_handshake(&msg2, 1300).unwrap(); assert!(initiator_conn.has_session()); // Initiator learned responder's epoch - assert_eq!( - initiator_conn.leg().unwrap().remote_epoch(), - Some(responder_epoch) - ); + assert_eq!(initiator_conn.conn_remote_epoch(), Some(responder_epoch)); // Both have sessions assert!(initiator_conn.has_session()); diff --git a/src/peer/mod.rs b/src/peer/mod.rs index 690dfd0..1443fe3 100644 --- a/src/peer/mod.rs +++ b/src/peer/mod.rs @@ -1,17 +1,16 @@ //! Peer Management //! //! Two-phase peer lifecycle: -//! 1. **PeerConnection** - Handshake phase, before identity is verified +//! 1. **PeerMachine** with a handshake carrier attached - handshake phase, +//! before identity is verified //! 2. **ActivePeer** - Authenticated phase, after successful Noise handshake mod active; #[cfg(any(target_os = "linux", target_os = "macos"))] pub(crate) mod connected_udp; -mod connection; pub(crate) mod machine; pub use active::{ActivePeer, ConnectivityState}; -pub use connection::PeerConnection; use crate::NodeAddr; use crate::transport::LinkId; diff --git a/src/proto/fmp/core.rs b/src/proto/fmp/core.rs index 4d46ce2..d134974 100644 --- a/src/proto/fmp/core.rs +++ b/src/proto/fmp/core.rs @@ -188,9 +188,9 @@ pub(crate) struct RekeyCfg { /// The result of the shell-side Noise wire step (Phase B) for one inbound /// handshake msg1, handed to the establish decision core. /// -/// The Noise step (`receive_handshake_init`) runs shell-side on the -/// `PeerConnection`: it reads **no** `Node` registry state — the load-bearing -/// invariant of this decomposition — and yields the learned peer identity, the +/// The Noise step (`receive_handshake_init`) runs on the control machine: it +/// reads **no** `Node` registry state — the essential invariant of this +/// decomposition — and yields the learned peer identity, the /// remote startup epoch, the sender's session index, and the opaque msg2 noise /// payload to frame and send. The core never parses or builds Noise bytes; the /// payload is an opaque blob. @@ -404,7 +404,7 @@ pub(crate) enum InboundReject { /// cross-connection and the tie-break decides whether we swap our session to the /// (winning) outbound one or keep our existing inbound session. The rekey-msg2 /// completion path is handled by a separate shell driver (it mutates -/// `ActivePeer`, not a `PeerConnection`) and never reaches this decision. +/// `ActivePeer`, not a pending handshake) and never reaches this decision. #[derive(Debug, PartialEq, Eq)] pub(crate) enum OutboundDecision { /// No existing peer for this identity: promote the completed outbound diff --git a/src/proto/fmp/mod.rs b/src/proto/fmp/mod.rs index 96d2e8a..8a97041 100644 --- a/src/proto/fmp/mod.rs +++ b/src/proto/fmp/mod.rs @@ -17,8 +17,9 @@ //! the [`cross_connection_winner`] tie-break helper. //! - `limits.rs` — the pure connection-retry backoff math. //! - `state.rs` — [`ConnectionState`], the pure handshake-phase connection -//! bookkeeping (owned by the shell `PeerConnection` beside its Noise crypto -//! handles), plus [`Fmp`], the (stateless) lifecycle anchor owned by `Node`. +//! bookkeeping (owned by the per-peer control machine beside its Noise +//! crypto carrier), plus [`Fmp`], the (stateless) lifecycle anchor owned by +//! `Node`. //! The handshake phase itself lives on the per-peer control machine. //! - `wire.rs` — the FMP link-framing codec: handshake message types, //! disconnect reasons, and the orderly disconnect message. Also carries the diff --git a/src/proto/fmp/state.rs b/src/proto/fmp/state.rs index a961516..5903d04 100644 --- a/src/proto/fmp/state.rs +++ b/src/proto/fmp/state.rs @@ -9,11 +9,11 @@ //! //! [`ConnectionState`] owns every **pure** field of the handshake-phase //! connection. The Noise crypto handles (`noise::HandshakeState`, -//! `NoiseSession`) stay shell-owned in -//! [`PeerConnection`](crate::peer::PeerConnection), which holds a -//! `ConnectionState` alongside them and drives the two halves side by side. The -//! shell's XX transition methods drive the Noise objects, then write learned -//! results back through the pure setters here (`set_expected_identity`, +//! `NoiseSession`) stay shell-owned in the per-peer control machine's +//! handshake carrier, which the machine drives alongside its own +//! `ConnectionState`. The machine's transition methods drive the Noise +//! objects, then write learned results back through the pure setters here +//! (`set_expected_identity`, //! `set_remote_epoch`, `touch`). //! //! This state is `no_std`+`alloc`-clean with respect to transport: the @@ -35,10 +35,9 @@ use crate::utils::index::SessionIndex; /// Pure, runtime-agnostic bookkeeping for a connection in the handshake phase. /// /// Owns every non-crypto field of the handshake-phase connection. The Noise -/// crypto handles live beside it in the shell -/// [`PeerConnection`](crate::peer::PeerConnection); this struct is written only -/// as plain data — the shell extracts learned identity/epoch out of the crypto -/// objects and sets them here through the setters. +/// crypto handles live beside it on the per-peer control machine; this struct +/// is written only as plain data — the machine extracts learned identity and +/// epoch out of the crypto objects and sets them here through the setters. #[derive(Debug)] pub struct ConnectionState { // === Link Reference === @@ -148,6 +147,7 @@ impl ConnectionState { /// Create the pure state for a new inbound connection with transport info. /// /// Used when processing msg1 where we know the transport and source address. + #[cfg(test)] pub fn inbound_with_transport( link_id: LinkId, transport_id: TransportId, diff --git a/src/transport/tcp/mod.rs b/src/transport/tcp/mod.rs index 23e95bd..5d000fb 100644 --- a/src/transport/tcp/mod.rs +++ b/src/transport/tcp/mod.rs @@ -124,7 +124,7 @@ impl TcpTransport { /// node-wide `node.limits.max_connections` > built-in default. This is a /// per-transport *raw-accept* ceiling; the true node-wide peer budget is /// still enforced downstream by the handshake-phase `max_connections` - /// admission check (`Node::add_connection`), so deriving this ceiling + /// admission check, so deriving this ceiling /// from `max_connections` does not let multiple transports exceed the /// node-wide total — it only stops the transport from rejecting inbound /// below the configured node budget.