diff --git a/src/control/queries.rs b/src/control/queries.rs index 7908ab5..842c7a7 100644 --- a/src/control/queries.rs +++ b/src/control/queries.rs @@ -1320,7 +1320,7 @@ pub fn show_connections(node: &Node) -> Value { let mut conn_json = json!({ "link_id": conn.link_id().as_u64(), "direction": format!("{}", conn.direction()), - "handshake_state": format!("{}", conn.handshake_state()), + "handshake_state": node.connection_handshake_state(conn.link_id()), "started_at_ms": conn.started_at(), "idle_ms": now.saturating_sub(conn.last_activity()), "resend_count": node.connection_resend_count(conn.link_id()), diff --git a/src/lib.rs b/src/lib.rs index 720efb8..8d31898 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,7 +95,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, HandshakeState, PeerConnection, PeerError}; +pub use peer::{ActivePeer, ConnectivityState, PeerConnection, 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 475bdbb..7605113 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -640,7 +640,18 @@ impl Node { error = %e, "Handshake completion failed" ); + // Drop the leg's Noise handle (byte-identical point) and + // record the failure on the control machine as `send_failed` + // — the failure state's new home. The machine PHASE stays + // exactly where the old leg-carried failure left it + // (`Handshaking{SentMsg1}`): the stale-connection sweep + // reclaims the leg via the machine `is_failed()` at the next + // tick, before any projection or resend, byte-identical to + // the pre-collapse leg mark. conn.mark_failed(); + if let Some(machine) = self.peer_machines.get_mut(&link_id) { + machine.mark_send_failed(); + } self.stats_mut() .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); return; @@ -653,7 +664,13 @@ impl Node { Ok(()) => {} Err(e) => { warn!(link_id = %link_id, our_profile = %our_profile, error = %e, "FMP negotiation failed"); + // Failure moves to the machine (`send_failed`); the phase + // stays `Handshaking{SentMsg1}` so the sweep reclaims the + // leg exactly as the pre-collapse leg mark did. conn.mark_failed(); + if let Some(machine) = self.peer_machines.get_mut(&link_id) { + machine.mark_send_failed(); + } self.stats_mut() .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); return; @@ -741,9 +758,16 @@ impl Node { error = %e, "Failed to send msg3" ); + // Failure moves to the machine (`send_failed`); the phase + // stays `Handshaking{SentMsg1}` (promote has not run yet) so + // the sweep reclaims the leg exactly as the pre-collapse leg + // mark did. if let Some(conn) = self.leg_mut(&link_id) { conn.mark_failed(); } + if let Some(machine) = self.peer_machines.get_mut(&link_id) { + machine.mark_send_failed(); + } self.stats_mut() .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); return; diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index 28f1ae4..d39a423 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -19,15 +19,15 @@ impl LifecycleView for Node { // reap. self.peer_machines .iter() - .filter_map(|(link_id, machine)| machine.leg().map(|conn| (link_id, conn))) - .filter(|(link_id, conn)| { - conn.is_failed() + .filter_map(|(link_id, machine)| machine.leg().map(|conn| (link_id, machine, conn))) + .filter(|(link_id, machine, conn)| { + machine.is_failed() || (conn.is_timed_out(now_ms, timeout_ms) && !self.peer_timers.get(*link_id).is_some_and(|timers| { timers.contains_key(&TimerKind::HandshakeTimeout) })) }) - .map(|(link_id, conn)| ConnSnapshot { + .map(|(link_id, _machine, conn)| ConnSnapshot { link: *link_id, is_outbound: conn.is_outbound(), retry_addr: conn.expected_identity().map(|id| *id.node_addr()), @@ -70,10 +70,16 @@ impl Node { match action { ConnAction::ScheduleRetry { peer } => self.note_handshake_timeout(peer, now_ms), ConnAction::Teardown { link } => { - // Log before cleanup (needs live connection state). + // Log before cleanup (needs live connection state). The + // failure signal is now read from the control machine; the + // leg still carries direction/idle for the log fields. + let is_failed = self + .peer_machines + .get(&link) + .is_some_and(|machine| machine.is_failed()); if let Some(conn) = self.leg(&link) { let direction = conn.direction(); - if conn.is_failed() { + if is_failed { debug!( link_id = %link, direction = %direction, diff --git a/src/node/mod.rs b/src/node/mod.rs index a661a6c..7aa3d33 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -2021,7 +2021,7 @@ impl Node { .map(|conn| snap::ConnectionRow { link_id: conn.link_id().as_u64(), direction: format!("{}", conn.direction()), - handshake_state: format!("{}", conn.handshake_state()), + handshake_state: self.connection_handshake_state(conn.link_id()).to_string(), started_at_ms: conn.started_at(), last_activity_ms: conn.last_activity(), resend_count: self.connection_resend_count(conn.link_id()), @@ -2348,6 +2348,17 @@ impl Node { .map_or(0, |machine| machine.resend_count()) } + /// Operator-visible handshake-state string for a pending handshake `link`, + /// derived from the per-peer control machine (the phase's home now that the + /// leg no longer carries it). Every leg surfaced by `connections()` is + /// embedded in a machine, so the lookup resolves; the `"initial"` default is + /// unreachable in that view and only guards a missing machine. + pub(crate) fn connection_handshake_state(&self, link: LinkId) -> &'static str { + self.peer_machines + .get(&link) + .map_or("initial", |machine| machine.displayed_handshake_state()) + } + pub(crate) fn cleanup_bootstrap_transport_if_unused(&mut self, transport_id: TransportId) { if !self .supervisor diff --git a/src/node/tests/handshake.rs b/src/node/tests/handshake.rs index eaf872c..a814cfe 100644 --- a/src/node/tests/handshake.rs +++ b/src/node/tests/handshake.rs @@ -792,7 +792,6 @@ async fn test_failed_connection_cleanup() { conn.set_our_index(our_index); conn.set_transport_id(transport_id); conn.set_source_addr(remote_addr.clone()); - conn.mark_failed(); // Simulate send failure let link = Link::connectionless( link_id, @@ -808,6 +807,24 @@ async fn test_failed_connection_cleanup() { node.pending_outbound .insert((transport_id, our_index.as_u32()), link_id); + // Simulate a stored-handshake send failure through the control machine — + // the failure carrier the stale-connection sweep now reads (the leg no + // longer carries a failed phase of its own). + { + let machine = node + .peer_machines + .get_mut(&link_id) + .expect("machine seeded by add_connection"); + let alloc = &mut node.index_allocator; + let actions = machine.step( + crate::peer::machine::PeerEvent::HandshakeSendFailed, + now_ms, + alloc, + ); + assert!(actions.is_empty()); + assert!(machine.is_failed()); + } + assert_eq!(node.connection_count(), 1); // Failed connections should be cleaned up immediately regardless of age diff --git a/src/peer/connection.rs b/src/peer/connection.rs index 31dcf69..526a60d 100644 --- a/src/peer/connection.rs +++ b/src/peer/connection.rs @@ -1,8 +1,11 @@ //! Peer Connection (Handshake Phase) //! //! Represents an in-progress connection before authentication completes. -//! PeerConnection tracks the Noise IK handshake state and transitions to -//! ActivePeer upon successful authentication. +//! PeerConnection tracks the Noise IK handshake and transitions to +//! ActivePeer upon successful authentication. The handshake *phase* (initial / +//! sent_msg1 / complete / failed) is no longer tracked here — it lives on the +//! per-peer control machine; the leg's crypto methods gate on the presence of +//! their Noise handles (`noise_handshake` / `noise_session`) directly. use crate::PeerIdentity; use crate::noise::{self, NoiseError, NoiseSession}; @@ -12,12 +15,6 @@ use crate::utils::index::SessionIndex; use secp256k1::Keypair; use std::fmt; -// The pure handshake-phase bookkeeping (`ConnectionState`) and its -// `HandshakeState` phase enum now live in `proto::fmp::state`. Re-export -// `HandshakeState` here so its original public path -// (`crate::peer::HandshakeState`) is preserved for existing call sites. -pub use crate::proto::fmp::HandshakeState; - /// A connection in the handshake phase, before authentication completes. /// /// For outbound connections, we know the expected peer identity from config. @@ -114,11 +111,6 @@ impl PeerConnection { self.state.direction() } - /// Get the handshake state. - pub fn handshake_state(&self) -> HandshakeState { - self.state.handshake_state() - } - /// Get the expected/learned peer identity, if known. pub fn expected_identity(&self) -> Option<&PeerIdentity> { self.state.expected_identity() @@ -134,21 +126,6 @@ impl PeerConnection { self.state.is_inbound() } - /// Check if handshake is in progress. - pub fn is_in_progress(&self) -> bool { - self.state.is_in_progress() - } - - /// Check if handshake completed. - pub fn is_complete(&self) -> bool { - self.state.is_complete() - } - - /// Check if handshake failed. - pub fn is_failed(&self) -> bool { - self.state.is_failed() - } - /// When the connection started. pub fn started_at(&self) -> u64 { self.state.started_at() @@ -281,20 +258,15 @@ impl PeerConnection { }); } - if self.state.handshake_state() != HandshakeState::Initial { - return Err(NoiseError::WrongState { - expected: "initial state".to_string(), - got: self.state.handshake_state().to_string(), - }); - } - - // XX initiator: no remote static needed upfront + // XX initiator: no remote static needed upfront. The old `!= Initial` + // phase guard is dropped — this method creates the Noise handshake + // handle, so there is no `take().expect()` to protect, and its Err path + // was unreachable in production (one call per fresh outbound leg). let mut hs = noise::HandshakeState::new_initiator(our_keypair); hs.set_local_epoch(epoch); let msg1 = hs.write_message_1()?; self.noise_handshake = Some(hs); - self.state.set_handshake_state(HandshakeState::SentMsg1); self.state.touch(current_time_ms); Ok(msg1) @@ -324,13 +296,6 @@ impl PeerConnection { }); } - if self.state.handshake_state() != HandshakeState::Initial { - return Err(NoiseError::WrongState { - expected: "initial state".to_string(), - got: self.state.handshake_state().to_string(), - }); - } - let mut hs = noise::HandshakeState::new_responder(our_keypair); hs.set_local_epoch(epoch); @@ -346,10 +311,11 @@ impl PeerConnection { msg2.extend_from_slice(&encrypted); } - // XX: handshake NOT complete yet — need msg3. - // Keep the handshake state for complete_handshake_msg3(). + // XX: handshake NOT complete yet — need msg3. Keep the Noise handshake + // handle for complete_handshake_msg3(); the phase (formerly + // `ReceivedMsg1`) now lives on the control machine, so no phase is set + // here. The handle's presence is the leg-local `ReceivedMsg1` signal. self.noise_handshake = Some(hs); - self.state.set_handshake_state(HandshakeState::ReceivedMsg1); self.state.touch(current_time_ms); Ok(msg2) @@ -370,10 +336,14 @@ impl PeerConnection { negotiation_payload: Option<&[u8]>, current_time_ms: u64, ) -> Result<(Vec, Option>), NoiseError> { - if self.state.handshake_state() != HandshakeState::SentMsg1 { + // The leg is at `SentMsg1` iff its Noise handshake handle is present + // (set by `start_handshake`, taken here on completion). Gate on the + // handle directly now that the phase enum is gone — byte-equivalent to + // the old `!= SentMsg1` guard for every reachable transition. + if self.noise_handshake.is_none() { return Err(NoiseError::WrongState { expected: "sent_msg1 state".to_string(), - got: self.state.handshake_state().to_string(), + got: "no active handshake".to_string(), }); } @@ -422,7 +392,6 @@ impl PeerConnection { // Handshake complete for initiator let session = hs.into_session()?; self.noise_session = Some(session); - self.state.set_handshake_state(HandshakeState::Complete); self.state.touch(current_time_ms); Ok((msg3, received_negotiation)) @@ -440,10 +409,15 @@ impl PeerConnection { message: &[u8], current_time_ms: u64, ) -> Result>, NoiseError> { - if self.state.handshake_state() != HandshakeState::ReceivedMsg1 { + // The responder leg is at `ReceivedMsg1` iff its Noise handshake handle + // is present (set by `receive_handshake_init`, taken here on msg3). Gate + // on the handle directly now that the phase enum is gone — byte-equivalent + // to the old `!= ReceivedMsg1` guard for every reachable transition, and + // it protects the `take().expect()` below. + if self.noise_handshake.is_none() { return Err(NoiseError::WrongState { expected: "received_msg1 state".to_string(), - got: self.state.handshake_state().to_string(), + got: "no active handshake".to_string(), }); } @@ -480,10 +454,10 @@ impl PeerConnection { // Capture remote epoch from msg3 self.state.set_remote_epoch(hs.remote_epoch()); - // Handshake complete for responder + // Handshake complete for responder. The completion signal is now the + // Noise session's presence (the phase enum is gone); no phase is set. let session = hs.into_session()?; self.noise_session = Some(session); - self.state.set_handshake_state(HandshakeState::Complete); self.state.touch(current_time_ms); Ok(received_negotiation) @@ -494,24 +468,23 @@ impl PeerConnection { /// Returns the NoiseSession for use in ActivePeer. Can only be called /// once after handshake completes. pub fn take_session(&mut self) -> Option { - if self.state.handshake_state() == HandshakeState::Complete { - self.noise_session.take() - } else { - None - } + // The session exists iff the handshake reached `Complete`, so taking it + // unconditionally is byte-equivalent to the old `== Complete` gate. + self.noise_session.take() } /// Check if we have a completed session ready to take. pub fn has_session(&self) -> bool { - self.state.handshake_state() == HandshakeState::Complete && self.noise_session.is_some() + self.noise_session.is_some() } // === State Transitions (for manual control if needed) === - /// Mark handshake as failed. Sets the pure lifecycle state and drops the - /// shell-owned crypto handshake handle. + /// Drop the shell-owned crypto handshake handle. The failure *state* now + /// lives on the control machine (`PeerMachine`); this only releases the + /// leg's Noise handle at the identical point it was released before, so a + /// subsequent `complete_handshake` on this leg still reports `WrongState`. pub fn mark_failed(&mut self) { - self.state.mark_failed(); self.noise_handshake = None; } @@ -533,7 +506,6 @@ impl fmt::Debug for PeerConnection { f.debug_struct("PeerConnection") .field("link_id", &self.state.link_id()) .field("direction", &self.state.direction()) - .field("handshake_state", &self.state.handshake_state()) .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()) @@ -568,18 +540,6 @@ mod tests { epoch } - #[test] - fn test_handshake_state_properties() { - assert!(HandshakeState::Initial.is_in_progress()); - assert!(HandshakeState::SentMsg1.is_in_progress()); - assert!(HandshakeState::ReceivedMsg1.is_in_progress()); - assert!(!HandshakeState::Complete.is_in_progress()); - assert!(!HandshakeState::Failed.is_in_progress()); - - assert!(HandshakeState::Complete.is_complete()); - assert!(HandshakeState::Failed.is_failed()); - } - #[test] fn test_outbound_connection() { let identity = make_peer_identity(); @@ -587,7 +547,7 @@ mod tests { assert!(conn.is_outbound()); assert!(!conn.is_inbound()); - assert_eq!(conn.handshake_state(), HandshakeState::Initial); + assert!(!conn.has_session()); assert!(conn.expected_identity().is_some()); assert_eq!(conn.started_at(), 1000); } @@ -598,7 +558,7 @@ mod tests { assert!(conn.is_inbound()); assert!(!conn.is_outbound()); - assert_eq!(conn.handshake_state(), HandshakeState::Initial); + assert!(!conn.has_session()); assert!(conn.expected_identity().is_none()); assert_eq!(conn.started_at(), 2000); } @@ -625,16 +585,16 @@ mod tests { let msg1 = initiator_conn .start_handshake(initiator_keypair, initiator_epoch, 1100) .unwrap(); - assert_eq!(initiator_conn.handshake_state(), HandshakeState::SentMsg1); + // Post-msg1 the initiator holds an in-flight handshake, not yet a session. + assert!(!initiator_conn.has_session()); // Responder processes msg1 and sends msg2 (XX: does NOT complete yet) let msg2 = responder_conn .receive_handshake_init(responder_keypair, responder_epoch, &msg1, None, 1200) .unwrap(); - assert_eq!( - responder_conn.handshake_state(), - HandshakeState::ReceivedMsg1 - ); + // XX: the responder parks awaiting msg3 — it holds an in-flight + // handshake, not yet a session (the deleted phase was `ReceivedMsg1`). + assert!(!responder_conn.has_session()); // Responder does NOT know initiator's identity yet (XX property) assert!(responder_conn.expected_identity().is_none()); @@ -642,16 +602,17 @@ mod tests { let (msg3, _neg) = initiator_conn .complete_handshake(&msg2, None, 1300) .unwrap(); - assert_eq!(initiator_conn.handshake_state(), HandshakeState::Complete); + // The initiator completes at msg3 generation: it now holds a session. + assert!(initiator_conn.has_session()); // Initiator learned responder's identity from msg2 let discovered = initiator_conn.expected_identity().unwrap(); assert_eq!(discovered.pubkey(), responder_identity.pubkey()); assert_eq!(initiator_conn.remote_epoch(), Some(responder_epoch)); - // Responder processes msg3 (completes handshake) + // Responder processes msg3 (completes handshake): it now holds a session. let _neg = responder_conn.complete_handshake_msg3(&msg3, 1400).unwrap(); - assert_eq!(responder_conn.handshake_state(), HandshakeState::Complete); + assert!(responder_conn.has_session()); // Responder learned initiator's identity from msg3 let discovered = responder_conn.expected_identity().unwrap(); @@ -686,13 +647,19 @@ mod tests { #[test] fn test_connection_failure() { + // `mark_failed` releases the leg's Noise handshake handle. The failure + // *state* now lives on the control machine, but the leg-local effect is + // still observable: a completion attempt afterward reports `WrongState` + // (the handle-presence gate) and no session is produced. let identity = make_peer_identity(); + let keypair = make_keypair(); let mut conn = PeerConnection::outbound(LinkId::new(1), identity, 1000); + conn.start_handshake(keypair, make_epoch(), 1100).unwrap(); conn.mark_failed(); - assert!(conn.is_failed()); - assert!(!conn.is_in_progress()); - assert!(!conn.is_complete()); + + assert!(!conn.has_session()); + assert!(conn.complete_handshake(&[0u8; 96], None, 1200).is_err()); } #[test] diff --git a/src/peer/machine.rs b/src/peer/machine.rs index 8decd41..d4ae10f 100644 --- a/src/peer/machine.rs +++ b/src/peer/machine.rs @@ -162,6 +162,57 @@ pub(crate) enum HandshakePhase { SentMsg2, } +/// Map a lifecycle state to the operator-visible pending-connection handshake +/// string. Total over `PeerState`; byte-identical to the strings the deleted +/// leg `HandshakeState` `Display` produced for every (leg, machine) pairing that +/// rests in the pending-connection view. On XX three arms are +/// production-reachable in that view (verified against the pre-collapse leg +/// display): +/// - `Handshaking{SentMsg1}` → `"sent_msg1"` (outbound identified leg). +/// - `Handshaking{SentMsg2}` → `"received_msg1"` (the inbound responder leg, +/// which parks at `SentMsg2` after replying to msg1 while the deleted leg +/// phase displayed `received_msg1` — a direction-aware synthesis with no leg +/// twin). +/// - `Discovered` → `"sent_msg1"` (GAP A: the anonymous-outbound leg rests with +/// its machine parked at `Discovered` while its leg is already at `SentMsg1`; +/// the anon path sends msg1 inline and dispatches no handshake-start event, so +/// advancing the machine to `Handshaking{SentMsg1}` here would arm a retransmit +/// timer the anon leg does not have — the overload is the neutral choice). +/// +/// The `send_failed` → `"failed"` override is applied by the caller +/// ([`displayed_handshake_state`](PeerMachine::displayed_handshake_state)). The +/// remaining arms are kept total for a complete mapping and are not reachable in +/// the view. +fn handshake_state_str(state: PeerState) -> &'static str { + match state { + PeerState::Handshaking { + phase: HandshakePhase::Initial, + .. + } => "initial", + PeerState::Handshaking { + phase: HandshakePhase::SentMsg1, + .. + } => "sent_msg1", + PeerState::Handshaking { + phase: HandshakePhase::SentMsg2, + .. + } => "received_msg1", + // GAP A: the anonymous-outbound leg rests in the view with its machine + // parked at `Discovered` while the leg itself is at `SentMsg1`, so it + // displayed `"sent_msg1"` before the collapse. The only in-view + // `Discovered`-with-leg case is this anon leg (identified dials leave + // `Discovered` atomically within their handler before any tick), so the + // overload preserves display parity byte-for-byte. + PeerState::Discovered => "sent_msg1", + PeerState::Established { .. } + | PeerState::Active { .. } + | PeerState::Maintaining { .. } + | PeerState::Closing { .. } => "complete", + PeerState::Failed { .. } => "failed", + PeerState::Connecting { .. } | PeerState::Closed { .. } => "initial", + } +} + /// Which maintenance sub-machine `Maintaining` is running. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum MaintainKind { @@ -461,6 +512,13 @@ pub(crate) struct PeerMachine { conn: ConnectionState, /// Remote startup epoch (establish-path-only; NOT in send-state). remote_epoch: Option<[u8; 8]>, + /// A stored-handshake send failure was observed on this leg. The failure is + /// carried as a flag (not a `PeerState::Failed` transition) so retransmit + /// eligibility (`is_handshaking_sent_msg1`) survives until the + /// stale-connection sweep reclaims the leg. It drives both `is_failed` + /// (reaping) and the displayed handshake state (`"failed"`), reproducing the + /// pre-collapse leg `is_failed`/display signal byte-for-byte. + send_failed: bool, // --- rekey negotiation sub-state (control tier; NOT the pending send slot) --- rekey_in_progress: bool, @@ -509,6 +567,7 @@ impl PeerMachine { None => ConnectionState::outbound_anonymous(link, now), }, remote_epoch: None, + send_failed: false, rekey_in_progress: false, rekey_our_index: None, rekey_msg1: None, @@ -537,6 +596,7 @@ impl PeerMachine { leg: None, conn: ConnectionState::inbound(link, now), remote_epoch: None, + send_failed: false, rekey_in_progress: false, rekey_our_index: None, rekey_msg1: None, @@ -605,6 +665,7 @@ impl PeerMachine { leg: None, conn, remote_epoch, + send_failed: false, rekey_in_progress: false, rekey_our_index: None, rekey_msg1: None, @@ -708,6 +769,41 @@ impl PeerMachine { ) } + /// Whether this peer's handshake has failed. The sole failure carrier now + /// that the leg's phase enum is gone: a terminal `PeerState::Failed` + /// (hard crypto/transport/ACL failures) OR the `send_failed` flag (a stored + /// handshake-initiation send failure that deliberately keeps the machine at + /// `Handshaking{SentMsg1}`). The stale-connection sweep reads this to reclaim + /// the leg, exactly as it read the leg's `is_failed` before. + pub(crate) fn is_failed(&self) -> bool { + matches!(self.state, PeerState::Failed { .. }) || self.send_failed + } + + /// The operator-visible handshake-state string for the pending-connection + /// view, derived from the machine phase. Byte-identical to the strings the + /// leg's `HandshakeState` `Display` produced before the phase collapsed onto + /// the machine. A `send_failed` leg renders `"failed"` while its phase stays + /// `SentMsg1`, matching the pre-collapse leg display. + pub(crate) fn displayed_handshake_state(&self) -> &'static str { + if self.send_failed { + return "failed"; + } + handshake_state_str(self.state) + } + + /// Record a handshake failure that the shell observed on the leg (e.g. a + /// `complete_handshake` that rejected msg2), WITHOUT leaving the current + /// handshake phase. Mirrors the `HandshakeSendFailed` carve-out: the failure + /// is carried as `send_failed`, so the machine PHASE is unchanged (matching + /// the pre-collapse behavior, where the shell marked only the leg failed and + /// left the machine in place) while `is_failed`/display report the failure. + /// The stale-connection sweep reclaims the leg via + /// [`is_failed`](Self::is_failed) at the next tick, before any projection or + /// resend. + pub(crate) fn mark_send_failed(&mut self) { + self.send_failed = true; + } + /// The crystallized node address, if identity is known. fn addr(&self) -> Option { self.node_addr @@ -849,8 +945,11 @@ impl PeerMachine { /// actions are emitted. fn on_handshake_send_failed(&mut self) -> Vec { if let Some(leg) = self.leg.as_mut() { + // Drop the leg's Noise handshake handle at the identical point as + // before; the failure *state* is recorded on the machine. leg.mark_failed(); } + self.send_failed = true; Vec::new() } @@ -2872,7 +2971,8 @@ mod tests { ); m.set_leg(PeerConnection::outbound(LinkId::new(1), peer, 100)); assert!(m.is_handshaking_sent_msg1()); - assert!(!m.leg().expect("leg embedded").is_failed()); + assert!(!m.is_failed()); + assert_eq!(m.displayed_handshake_state(), "sent_msg1"); let actions = m.step(PeerEvent::HandshakeSendFailed, 200, &mut alloc); assert_eq!(actions, Vec::new(), "HandshakeSendFailed emits no actions"); @@ -2881,11 +2981,17 @@ mod tests { "retransmit eligibility survives a send failure" ); assert!( - m.leg().expect("leg retained").is_failed(), - "the leg carries the failed mark the sweep reads" + m.is_failed(), + "the machine carries the failed mark the sweep reads" + ); + assert_eq!( + m.displayed_handshake_state(), + "failed", + "the send-failed leg still displays as failed" ); - // With no leg (e.g. after take_leg) the event is a defensive no-op. + // With no leg (e.g. after take_leg) the event stays a defensive no-op + // for the leg handle and keeps the state retransmit-eligible. let _ = m.take_leg(); let actions = m.step(PeerEvent::HandshakeSendFailed, 300, &mut alloc); assert_eq!(actions, Vec::new()); @@ -2893,6 +2999,78 @@ mod tests { assert!(m.leg().is_none()); } + // ---- Test 7f: handshake_state_str is a total, byte-identical mapping --- + // Pins the displayed-string derivation for every `PeerState` arm against the + // strings the deleted leg `HandshakeState` `Display` produced. + #[test] + fn handshake_state_str_total_mapping() { + let link = LinkId::new(1); + let addr = *peer_identity().node_addr(); + + assert_eq!( + handshake_state_str(PeerState::Handshaking { + link, + phase: HandshakePhase::Initial, + }), + "initial" + ); + assert_eq!( + handshake_state_str(PeerState::Handshaking { + link, + phase: HandshakePhase::SentMsg1, + }), + "sent_msg1" + ); + assert_eq!( + handshake_state_str(PeerState::Handshaking { + link, + phase: HandshakePhase::SentMsg2, + }), + "received_msg1" + ); + + assert_eq!( + handshake_state_str(PeerState::Established { addr }), + "complete" + ); + assert_eq!(handshake_state_str(PeerState::Active { addr }), "complete"); + assert_eq!( + handshake_state_str(PeerState::Maintaining { + addr, + kind: MaintainKind::Mtu, + }), + "complete" + ); + assert_eq!( + handshake_state_str(PeerState::Closing { + addr, + reason: CloseReason::Requested, + }), + "complete" + ); + + assert_eq!( + handshake_state_str(PeerState::Failed { + reason: FailReason::HandshakeFailed, + }), + "failed" + ); + + // GAP A: the anonymous-outbound leg rests at `Discovered` while its leg + // is at `SentMsg1`, so the pending-connection view displayed "sent_msg1". + assert_eq!(handshake_state_str(PeerState::Discovered), "sent_msg1"); + assert_eq!( + handshake_state_str(PeerState::Connecting { link }), + "initial" + ); + assert_eq!( + handshake_state_str(PeerState::Closed { + backoff_deadline_ms: 0, + }), + "initial" + ); + } + // ---- Test 8: liveness -> LinkDeadSuspected -> ReportLost -------------- #[test] fn liveness_to_link_dead() { diff --git a/src/peer/mod.rs b/src/peer/mod.rs index 65e2c33..690dfd0 100644 --- a/src/peer/mod.rs +++ b/src/peer/mod.rs @@ -11,7 +11,7 @@ mod connection; pub(crate) mod machine; pub use active::{ActivePeer, ConnectivityState}; -pub use connection::{HandshakeState, PeerConnection}; +pub use connection::PeerConnection; use crate::NodeAddr; use crate::transport::LinkId; diff --git a/src/proto/fmp/mod.rs b/src/proto/fmp/mod.rs index 231473b..10bac15 100644 --- a/src/proto/fmp/mod.rs +++ b/src/proto/fmp/mod.rs @@ -20,8 +20,8 @@ //! - `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) and its [`HandshakeState`] phase enum, plus [`Fmp`], the -//! (stateless) lifecycle anchor owned by `Node`. +//! handles), plus [`Fmp`], the (stateless) lifecycle anchor owned by `Node`. +//! The handshake phase itself lives on the per-peer control machine. //! - `wire.rs` — the FMP wire codec: XX handshake message types, disconnect //! reasons, the orderly disconnect message, and the negotiation payload. //! Also carries the relocated FMP link wire framing (moved from @@ -43,7 +43,6 @@ pub(crate) use core::{ }; pub use core::{PromotionResult, cross_connection_winner}; pub(crate) use limits::backoff_ms; -pub use state::HandshakeState; pub(crate) use state::{ConnectionState, Fmp}; pub(crate) use wire::{Disconnect, DisconnectReason}; pub use wire::{HandshakeMessageType, NegotiationPayload, NodeProfile, TlvEntry}; diff --git a/src/proto/fmp/state.rs b/src/proto/fmp/state.rs index d72c22e..4e429d6 100644 --- a/src/proto/fmp/state.rs +++ b/src/proto/fmp/state.rs @@ -1,19 +1,20 @@ //! Sans-IO FMP connection-lifecycle state. //! //! The pure, runtime-agnostic bookkeeping for an in-progress FMP peer -//! connection — link/direction identity, the handshake-phase enum, learned -//! peer identity and epoch, index/transport/address tracking, handshake-resend -//! scheduling, and link statistics — extracted out of the async node shell. +//! connection — link/direction identity, learned peer identity and epoch, +//! index/transport/address tracking, handshake-resend scheduling, and link +//! statistics — extracted out of the async node shell. The handshake phase +//! itself lives solely on the per-peer control machine +//! ([`PeerMachine`](crate::peer::machine::PeerMachine)), not here. //! //! [`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 validate against the pure phase, drive the -//! Noise objects, then write learned results back through the pure setters here -//! (`set_handshake_state`, `set_expected_identity`, `set_remote_epoch`, -//! `touch`). +//! shell's XX 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 //! identifier/address/statistics value types are the plain-data `transport` @@ -31,59 +32,6 @@ use super::wire::NodeProfile; use crate::PeerIdentity; use crate::transport::{LinkDirection, LinkId, LinkStats, TransportAddr, TransportId}; use crate::utils::index::SessionIndex; -use core::fmt; - -/// Handshake protocol state machine. -/// -/// For Noise XX pattern: -/// - Initiator: Initial → SentMsg1 → Complete (after processing msg2 + sending msg3) -/// - Responder: Initial → ReceivedMsg1 → Complete (after processing msg3) -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum HandshakeState { - /// Initial state, ready to start handshake. - Initial, - /// Initiator: Sent message 1, awaiting message 2. - SentMsg1, - /// Responder: Received message 1, ready to send message 2. - ReceivedMsg1, - /// Handshake completed successfully. - Complete, - /// Handshake failed. - Failed, -} - -impl HandshakeState { - /// Check if handshake is still in progress. - pub fn is_in_progress(&self) -> bool { - matches!( - self, - HandshakeState::Initial | HandshakeState::SentMsg1 | HandshakeState::ReceivedMsg1 - ) - } - - /// Check if handshake completed successfully. - pub fn is_complete(&self) -> bool { - matches!(self, HandshakeState::Complete) - } - - /// Check if handshake failed. - pub fn is_failed(&self) -> bool { - matches!(self, HandshakeState::Failed) - } -} - -impl fmt::Display for HandshakeState { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s = match self { - HandshakeState::Initial => "initial", - HandshakeState::SentMsg1 => "sent_msg1", - HandshakeState::ReceivedMsg1 => "received_msg1", - HandshakeState::Complete => "complete", - HandshakeState::Failed => "failed", - }; - write!(f, "{}", s) - } -} /// Pure, runtime-agnostic bookkeeping for a connection in the handshake phase. /// @@ -101,10 +49,6 @@ pub struct ConnectionState { /// Connection direction (we initiated or they initiated). direction: LinkDirection, - // === Handshake State === - /// Current handshake state. - handshake_state: HandshakeState, - /// Expected peer identity (known for outbound, learned for inbound). /// Updated after receiving their static key in the handshake. expected_identity: Option, @@ -171,7 +115,6 @@ impl ConnectionState { Self { link_id, direction: LinkDirection::Outbound, - handshake_state: HandshakeState::Initial, expected_identity: Some(expected_identity), started_at: current_time_ms, last_activity: current_time_ms, @@ -199,7 +142,6 @@ impl ConnectionState { Self { link_id, direction: LinkDirection::Outbound, - handshake_state: HandshakeState::Initial, expected_identity: None, started_at: current_time_ms, last_activity: current_time_ms, @@ -225,7 +167,6 @@ impl ConnectionState { Self { link_id, direction: LinkDirection::Inbound, - handshake_state: HandshakeState::Initial, expected_identity: None, started_at: current_time_ms, last_activity: current_time_ms, @@ -255,7 +196,6 @@ impl ConnectionState { Self { link_id, direction: LinkDirection::Inbound, - handshake_state: HandshakeState::Initial, expected_identity: None, started_at: current_time_ms, last_activity: current_time_ms, @@ -285,11 +225,6 @@ impl ConnectionState { self.direction } - /// Get the handshake state. - pub fn handshake_state(&self) -> HandshakeState { - self.handshake_state - } - /// Get the expected/learned peer identity, if known. pub fn expected_identity(&self) -> Option<&PeerIdentity> { self.expected_identity.as_ref() @@ -305,21 +240,6 @@ impl ConnectionState { self.direction == LinkDirection::Inbound } - /// Check if handshake is in progress. - pub fn is_in_progress(&self) -> bool { - self.handshake_state.is_in_progress() - } - - /// Check if handshake completed. - pub fn is_complete(&self) -> bool { - self.handshake_state.is_complete() - } - - /// Check if handshake failed. - pub fn is_failed(&self) -> bool { - self.handshake_state.is_failed() - } - /// When the connection started. pub fn started_at(&self) -> u64 { self.started_at @@ -425,20 +345,6 @@ impl ConnectionState { self.peer_profile = Some(peer_profile); } - // === Handshake Phase Advance === - - /// Advance the pure handshake phase. Driven by the shell after it has - /// stepped the Noise objects. - pub fn set_handshake_state(&mut self, state: HandshakeState) { - self.handshake_state = state; - } - - /// Mark the pure handshake phase failed. The shell drops the crypto handle - /// separately. - pub fn mark_failed(&mut self) { - self.handshake_state = HandshakeState::Failed; - } - // === Handshake Resend === /// Store the wire-format msg1 bytes for resend and schedule the first resend. diff --git a/src/proto/fmp/tests/state.rs b/src/proto/fmp/tests/state.rs index f3be620..0d17665 100644 --- a/src/proto/fmp/tests/state.rs +++ b/src/proto/fmp/tests/state.rs @@ -1,9 +1,9 @@ -//! Unit tests for the pure FMP connection state ([`ConnectionState`]) and its -//! [`HandshakeState`] phase enum. These exercise the extracted bookkeeping -//! directly, with no crypto involved; the crypto-driving transition behavior is -//! covered by the shell `peer::connection` suite. +//! Unit tests for the pure FMP connection state ([`ConnectionState`]). These +//! exercise the extracted bookkeeping directly, with no crypto involved; the +//! crypto-driving transition behavior is covered by the shell `peer::connection` +//! suite, and the handshake phase itself lives on the control machine. -use crate::proto::fmp::{ConnectionState, HandshakeState, NodeProfile}; +use crate::proto::fmp::{ConnectionState, NodeProfile}; use crate::transport::{LinkId, TransportAddr, TransportId}; use crate::utils::index::SessionIndex; use crate::{Identity, PeerIdentity}; @@ -12,21 +12,6 @@ fn make_peer_identity() -> PeerIdentity { PeerIdentity::from_pubkey(Identity::generate().pubkey()) } -#[test] -fn handshake_state_predicates() { - assert!(HandshakeState::Initial.is_in_progress()); - assert!(HandshakeState::SentMsg1.is_in_progress()); - assert!(HandshakeState::ReceivedMsg1.is_in_progress()); - assert!(!HandshakeState::Complete.is_in_progress()); - assert!(!HandshakeState::Failed.is_in_progress()); - - assert!(HandshakeState::Complete.is_complete()); - assert!(!HandshakeState::Initial.is_complete()); - - assert!(HandshakeState::Failed.is_failed()); - assert!(!HandshakeState::Complete.is_failed()); -} - #[test] fn outbound_initializes_pure_fields() { let identity = make_peer_identity(); @@ -34,8 +19,6 @@ fn outbound_initializes_pure_fields() { assert!(state.is_outbound()); assert!(!state.is_inbound()); - assert_eq!(state.handshake_state(), HandshakeState::Initial); - assert!(state.is_in_progress()); assert!(state.expected_identity().is_some()); assert_eq!(state.link_id(), LinkId::new(1)); assert_eq!(state.started_at(), 1000); @@ -53,7 +36,6 @@ fn inbound_initializes_pure_fields() { assert!(state.is_inbound()); assert!(!state.is_outbound()); - assert_eq!(state.handshake_state(), HandshakeState::Initial); assert!(state.expected_identity().is_none()); assert_eq!(state.started_at(), 2000); } @@ -111,27 +93,6 @@ fn identity_and_epoch_setters() { assert_eq!(state.remote_epoch(), Some([9u8; 8])); } -#[test] -fn handshake_state_advance_and_fail() { - let mut state = ConnectionState::outbound(LinkId::new(1), make_peer_identity(), 0); - assert!(state.is_in_progress()); - - state.set_handshake_state(HandshakeState::SentMsg1); - assert_eq!(state.handshake_state(), HandshakeState::SentMsg1); - assert!(state.is_in_progress()); - assert!(!state.is_complete()); - - state.set_handshake_state(HandshakeState::Complete); - assert!(state.is_complete()); - assert!(!state.is_in_progress()); - - state.mark_failed(); - assert!(state.is_failed()); - assert!(!state.is_in_progress()); - assert!(!state.is_complete()); - assert_eq!(state.handshake_state(), HandshakeState::Failed); -} - #[test] fn resend_bookkeeping() { let mut state = ConnectionState::outbound(LinkId::new(1), make_peer_identity(), 0); @@ -182,7 +143,6 @@ fn outbound_anonymous_initializes_pure_fields() { let state = ConnectionState::outbound_anonymous(LinkId::new(5), 4000); assert!(state.is_outbound()); assert!(!state.is_inbound()); - assert_eq!(state.handshake_state(), HandshakeState::Initial); assert!(state.expected_identity().is_none()); assert_eq!(state.link_id(), LinkId::new(5)); assert_eq!(state.started_at(), 4000);