diff --git a/src/node/dataplane/peer_actions.rs b/src/node/dataplane/peer_actions.rs index b5f1d42..719a379 100644 --- a/src/node/dataplane/peer_actions.rs +++ b/src/node/dataplane/peer_actions.rs @@ -199,7 +199,6 @@ impl Node { // (`handle_msg1` L665): the send error text is surfaced // at the executor point where the failure is now handled. warn!(link_id = %link, error = %e, "Failed to send msg2"); - self.connections.remove(&link); self.links.remove(&link); self.addr_to_link .remove(&(ambient.transport_id, ambient.remote_addr.clone())); @@ -247,8 +246,7 @@ impl Node { let wire_msg2 = if ambient.is_outbound { None } else { - self.connections - .get(&promote_link) + self.leg(&promote_link) .and_then(|c| c.handshake_msg2().map(|m| m.to_vec())) }; @@ -393,10 +391,11 @@ impl Node { // records the reject but performs no link/index teardown // and leaves the `pending_outbound` entry for the stale- // connection reaper. The leg's machine must go with the - // leg, though: `promote_connection` consumed the - // `connections` entry before erring, so the reaper (which - // sweeps `connections`) can never reach this link's - // machine — dropping it here is the only disposal point. + // leg, though: `promote_connection` took the pending + // connection off the machine before erring, so the reaper + // (which sweeps the machines' embedded legs) can never + // reach this link's machine — dropping it here is the + // only disposal point. warn!( target: "fips::node::handlers::handshake", link_id = %promote_link, @@ -581,27 +580,22 @@ impl Node { if our_inbound_wins { // Larger node side: swap to the inbound session so it pairs // with the peer's kept outbound session. - let inbound_session = match self - .connections - .get_mut(&link) - .and_then(|c| c.take_session()) - { - Some(s) => s, - None => { - self.connections.remove(&link); - self.remove_link(&link); - self.remove_peer_machine(link); - self.stats_mut().record_reject(RejectReason::Handshake( - HandshakeReject::BadState, - )); - return; - } - }; + let inbound_session = + match self.leg_mut(&link).and_then(|c| c.take_session()) { + Some(s) => s, + None => { + self.remove_link(&link); + self.remove_peer_machine(link); + self.stats_mut().record_reject(RejectReason::Handshake( + HandshakeReject::BadState, + )); + return; + } + }; if let Some(peer_ref) = self.peers.get_mut(&peer) { let old_our_index = peer_ref.replace_session(inbound_session, our_index, their_index); let Some(transport_id) = peer_ref.transport_id() else { - self.connections.remove(&link); self.remove_link(&link); self.remove_peer_machine(link); self.stats_mut().record_reject(RejectReason::Handshake( @@ -636,8 +630,8 @@ impl Node { // Both branches tear down the temporary inbound link fully // (including its `addr_to_link` mapping) via `remove_link`, - // disposing the leg's machine with it. - self.connections.remove(&link); + // disposing the leg's machine (and its embedded connection) + // with it. self.remove_link(&link); self.remove_peer_machine(link); return; @@ -677,7 +671,7 @@ impl Node { // Rekey: process as responder, store new session as pending. let noise_session = { - let Some(conn) = self.connections.get_mut(&link) else { + let Some(conn) = self.leg_mut(&link) else { warn!(link_id = %link, "Connection removed during rekey msg3 processing"); self.links.remove(&link); self.remove_peer_machine(link); @@ -694,7 +688,6 @@ impl Node { Some(s) => s, None => { warn!("Rekey msg3: no session from handshake"); - self.connections.remove(&link); self.links.remove(&link); self.remove_peer_machine(link); self.stats_mut() @@ -713,13 +706,13 @@ impl Node { self.peers_by_index .insert((ambient.transport_id, our_new_index.as_u32()), peer); - // Clean up: remove the temporary connection/link and the leg's - // machine (the established peer keeps its own, keyed by its own - // link). Do NOT remove addr_to_link — the entry must remain - // pointing to the original link so the established peer stays - // routable, so this uses the bare `links.remove` rather than - // the full `remove_link`. - self.connections.remove(&link); + // Clean up: remove the temporary link and the leg's machine + // (dropping its embedded connection; the established peer + // keeps its own machine, keyed by its own link). Do NOT + // remove addr_to_link — the entry must remain pointing to + // the original link so the established peer stays routable, + // so this uses the bare `links.remove` rather than the full + // `remove_link`. self.links.remove(&link); self.remove_peer_machine(link); diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index 0fa5dec..213133a 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -299,22 +299,21 @@ impl Node { self.links.insert(link_id, link); self.addr_to_link.insert(addr_key, link_id); - self.connections.insert(link_id, conn); - // The leg's persistent control machine is born alongside its pending + + // Build the msg2 response, storing it on the connection for potential + // resend before the connection is embedded on the machine below. + let wire_msg2 = build_msg2(our_index, header.sender_idx, &msg2_response); + conn.set_handshake_msg2(wire_msg2.clone()); + + // The leg's persistent control machine is born carrying its pending // connection, parked at `SentMsg2` awaiting msg3 (identity is unknown // until then). Inserted before the msg2 send below so no suspension - // point observes a leg without a machine. `handle_msg3` steps this - // same machine; every teardown path disposes it with the leg. - self.peer_machines.insert( - link_id, - PeerMachine::inbound_msg2_sent(link_id, our_index, packet.timestamp_ms), - ); - - // Build and send msg2 response, storing for potential resend - let wire_msg2 = build_msg2(our_index, header.sender_idx, &msg2_response); - if let Some(conn) = self.connections.get_mut(&link_id) { - conn.set_handshake_msg2(wire_msg2.clone()); - } + // point observes a leg in flight without a machine. `handle_msg3` + // steps this same machine; every teardown path disposes it with the + // embedded leg. + let mut machine = PeerMachine::inbound_msg2_sent(link_id, our_index, packet.timestamp_ms); + machine.set_leg(conn); + self.peer_machines.insert(link_id, machine); if let Some(transport) = self.transports.get(&packet.transport_id) { match transport.send(&packet.remote_addr, &wire_msg2).await { @@ -333,8 +332,8 @@ impl Node { error = %e, "Failed to send msg2" ); - // Clean up on failure - self.connections.remove(&link_id); + // Clean up on failure (the machine disposal drops the + // embedded connection with it) self.links.remove(&link_id); self.addr_to_link .remove(&(packet.transport_id, packet.remote_addr)); @@ -362,7 +361,7 @@ impl Node { /// (if already promoted). fn find_stored_msg2(&self, link_id: LinkId) -> Option> { // Check pending connection first - if let Some(conn) = self.connections.get(&link_id) + if let Some(conn) = self.leg(&link_id) && let Some(msg2) = conn.handshake_msg2() { return Some(msg2.to_vec()); @@ -412,9 +411,17 @@ impl Node { }; // Check if this is a rekey msg2: the handshake state is on the - // ActivePeer (not a PeerConnection), so self.connections won't have it. - // Look for a peer with matching rekey_our_index. - if !self.connections.contains_key(&link_id) { + // ActivePeer (not a PeerConnection), so the link's machine — if one + // survives at all — carries no pending connection. 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 + // rekey_our_index. + if self + .peer_machines + .get(&link_id) + .is_none_or(|machine| machine.leg().is_none()) + { let noise_msg2 = &packet.data[header.noise_msg2_offset..]; // Find peer with rekey in progress for this index @@ -592,7 +599,7 @@ impl Node { let our_profile = self.node_profile(); let (peer_identity, msg3_bytes, our_index) = { - let Some(conn) = self.connections.get_mut(&link_id) else { + let Some(conn) = self.leg_mut(&link_id) else { warn!(link_id = %link_id, "Connection removed during msg2 processing"); self.pending_outbound.remove(&key); self.stats_mut() @@ -672,8 +679,8 @@ impl Node { .is_err() { self.pending_outbound.remove(&key); - self.connections.remove(&link_id); - // Drop the machine persisted at dial — this leg never promotes. + // Drop the machine persisted at dial — this leg never promotes, + // and its pending connection is dropped with it. self.remove_peer_machine(link_id); self.remove_link(&link_id); self.stats_mut() @@ -687,10 +694,10 @@ impl Node { // identified dial misdirected at ourselves lands here too (the // learned identity overwrites the dial-time expectation and is // never compared against it). This leg never promotes; its machine - // goes with it. The index, link, and `pending_outbound` entry are - // deliberately NOT freed here (pre-existing shape). + // goes with it (dropping the embedded pending connection). The + // index, link, and `pending_outbound` entry are deliberately NOT + // freed here (pre-existing shape). debug!(link_id = %link_id, "Discovered self via shared-media beacon, dropping"); - self.connections.remove(&link_id); self.remove_peer_machine(link_id); self.stats_mut() .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); @@ -718,7 +725,7 @@ impl Node { error = %e, "Failed to send msg3" ); - if let Some(conn) = self.connections.get_mut(&link_id) { + if let Some(conn) = self.leg_mut(&link_id) { conn.mark_failed(); } self.stats_mut() @@ -746,11 +753,19 @@ impl Node { // This ensures both nodes use the same Noise handshake (the winner's // outbound = the loser's inbound). if self.peers.contains_key(&peer_node_addr) { - // The dial-persisted outbound machine is not consumed by the inline - // Swap/Keep resolution below (which mutates the existing promoted peer - // directly, with no machine). Drop it on entry so none of this block's - // exits leave a dangling machine — matching the pre-persistence path, + // Extract the outbound connection from its machine FIRST — the + // machine owns it, so disposing the machine before the take would + // destroy the connection. The dial-persisted outbound machine is + // not consumed by the inline Swap/Keep resolution below (which + // mutates the existing promoted peer directly, with no machine), + // so drop it right after the take — unconditionally, whether or + // not a connection was carried — so none of this block's exits + // leave a dangling machine. Matches the pre-persistence path, // which created no machine for a cross-connection. + let taken_conn = self + .peer_machines + .get_mut(&link_id) + .and_then(|machine| machine.take_leg()); self.remove_peer_machine(link_id); let our_outbound_wins = cross_connection_winner( self.identity().node_addr(), @@ -758,8 +773,7 @@ impl Node { true, // this IS our outbound ); - // Extract the outbound connection - let mut conn = match self.connections.remove(&link_id) { + let mut conn = match taken_conn { Some(c) => c, None => { self.pending_outbound.remove(&key); @@ -923,7 +937,7 @@ impl Node { } None => { // A miss is a state-machine inconsistency (e.g. a test seeding - // `connections`/`pending_outbound` directly): rebuild the + // a connection/`pending_outbound` entry directly): rebuild the // machine defensively and persist it, so the promotion feedback // below still finds it and the promoted peer keeps a machine. debug_assert!( @@ -946,7 +960,7 @@ impl Node { // The outbound Msg2 Promote step cancels the two dial-armed handshake // timers (the machine survives promotion, so they would otherwise linger // in `peer_timers` until `drive_peer_timers` lazily discards them — the - // promoted leg's `connections` entry is gone and the machine has left + // promoted leg's pending connection is consumed and the machine has left // `SentMsg1`, so they can no longer fire) and then promotes. // `PromoteToActive` is still what performs the promotion. debug_assert_eq!( @@ -1023,7 +1037,7 @@ impl Node { let our_profile = self.node_profile(); let (peer_identity, our_index, remote_epoch) = { // Get the pending connection - let conn = match self.connections.get_mut(&link_id) { + let conn = match self.leg_mut(&link_id) { Some(c) => c, None => { debug!( @@ -1047,12 +1061,11 @@ impl Node { error = %e, "Msg3 processing failed" ); - // Clean up. Capture the index before removing the - // connection; reading it after the remove would always - // return None and leak the allocated index. - let our_idx_to_free = - self.connections.get(&link_id).and_then(|c| c.our_index()); - self.connections.remove(&link_id); + // Clean up. Capture the index before disposing the + // machine (and the connection embedded on it); reading + // it after the disposal would always return None and + // leak the allocated index. + let our_idx_to_free = self.leg(&link_id).and_then(|c| c.our_index()); self.remove_link(&link_id); self.remove_peer_machine(link_id); if let Some(idx) = our_idx_to_free { @@ -1070,7 +1083,6 @@ impl Node { Ok(()) => {} Err(e) => { warn!(link_id = %link_id, our_profile = %our_profile, error = %e, "FMP negotiation failed"); - self.connections.remove(&link_id); self.remove_link(&link_id); self.remove_peer_machine(link_id); self.stats_mut() @@ -1085,7 +1097,6 @@ impl Node { Some(id) => *id, None => { warn!("Identity not learned from msg3"); - self.connections.remove(&link_id); self.remove_link(&link_id); self.remove_peer_machine(link_id); self.stats_mut() @@ -1119,7 +1130,7 @@ impl Node { // and the initiator has a matching session from processing // msg2. Reason `Other` is used instead of `SecurityViolation` // to avoid naming the ACL mechanism on the wire. - let reject_info = match self.connections.get_mut(&link_id) { + let reject_info = match self.leg_mut(&link_id) { Some(conn) => match (conn.their_index(), conn.take_session()) { (Some(idx), Some(session)) => Some((idx, session)), _ => None, @@ -1139,7 +1150,6 @@ impl Node { ) .await; } - self.connections.remove(&link_id); self.remove_link(&link_id); self.remove_peer_machine(link_id); self.stats_mut() @@ -1149,7 +1159,6 @@ impl Node { if peer_node_addr == *self.identity().node_addr() { debug!(link_id = %link_id, "Received msg3 from self, dropping"); - self.connections.remove(&link_id); self.remove_link(&link_id); self.remove_peer_machine(link_id); self.stats_mut() @@ -1248,7 +1257,6 @@ impl Node { // We keep our in-progress rekey and drop their msg3; return the // msg1-allocated inbound index rather than orphaning it. let _ = self.index_allocator.free(our_index); - self.connections.remove(&link_id); self.links.remove(&link_id); self.remove_peer_machine(link_id); self.stats_mut() @@ -1276,7 +1284,6 @@ impl Node { // Duplicate handshake: the active peer is untouched, so return the // msg1-allocated inbound index rather than orphaning it. let _ = self.index_allocator.free(our_index); - self.connections.remove(&link_id); self.links.remove(&link_id); self.remove_peer_machine(link_id); return; @@ -1380,8 +1387,12 @@ impl Node { link_id = %link_id, "Leaf node rejecting additional peer (single-peer enforcement)" ); - // Clean up the connection and its control machine - if let Some(conn) = self.connections.remove(&link_id) + // Clean up the connection (taken off its machine first) and its + // control machine + if let Some(conn) = self + .peer_machines + .get_mut(&link_id) + .and_then(|machine| machine.take_leg()) && let Some(idx) = conn.our_index() { let _ = self.index_allocator.free(idx); @@ -1391,10 +1402,13 @@ impl Node { return Err(NodeError::MaxPeersExceeded { max: 1 }); } - // Remove the connection from pending + // Take the pending connection off its control machine. The machine + // survives the promotion (it becomes the active peer's control + // machine), left with no pending connection. let mut connection = self - .connections - .remove(&link_id) + .peer_machines + .get_mut(&link_id) + .and_then(|machine| machine.take_leg()) .ok_or(NodeError::ConnectionNotFound(link_id))?; // Verify handshake is complete and extract session @@ -1595,14 +1609,13 @@ impl Node { // peer. The outbound will be cleaned up in handle_msg2 or by // the 30s handshake timeout. let pending_to_same_peer: Vec = self - .connections - .iter() - .filter(|(_, conn)| { + .connections() + .filter(|conn| { conn.expected_identity() .map(|id| *id.node_addr() == peer_node_addr) .unwrap_or(false) }) - .map(|(lid, _)| *lid) + .map(|conn| conn.link_id()) .collect(); for pending_link_id in &pending_to_same_peer { diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index f1a6b79..28f1ae4 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -14,11 +14,12 @@ impl LifecycleView for Node { // `is_failed()` legs are always reaped here (~1s), as before. The // idle-timeout is reaped here ONLY for legs whose timeout is not already // driven by a machine `HandshakeTimeout` timer — i.e. inbound legs (IK - // inbound arms none) and machine-less test connections. Outbound legs with - // an armed timer are reaped by `drive_handshake_timeouts`, so excluding - // them here avoids a double reap. - self.connections + // inbound arms none). Outbound legs with an armed timer are reaped by + // `drive_handshake_timeouts`, so excluding them here avoids a double + // reap. + self.peer_machines .iter() + .filter_map(|(link_id, machine)| machine.leg().map(|conn| (link_id, conn))) .filter(|(link_id, conn)| { conn.is_failed() || (conn.is_timed_out(now_ms, timeout_ms) @@ -57,7 +58,7 @@ impl Node { /// the retry-then-teardown choreography is the pure /// [`Fmp::poll_timeouts`](crate::proto::fmp::Fmp::poll_timeouts) decision. pub(in crate::node) fn check_timeouts(&mut self) { - if self.connections.is_empty() { + if self.connection_count() == 0 { return; } @@ -70,7 +71,7 @@ impl Node { ConnAction::ScheduleRetry { peer } => self.note_handshake_timeout(peer, now_ms), ConnAction::Teardown { link } => { // Log before cleanup (needs live connection state). - if let Some(conn) = self.connections.get(&link) { + if let Some(conn) = self.leg(&link) { let direction = conn.direction(); if conn.is_failed() { debug!( @@ -101,14 +102,21 @@ impl Node { /// the link and address mapping. Does not log — callers provide context-appropriate /// log messages. pub(in crate::node) fn cleanup_stale_connection(&mut self, link_id: LinkId, _now_ms: u64) { - let conn = match self.connections.remove(&link_id) { + // Take the connection off its machine BEFORE disposing the machine + // (the machine owns it), keeping it readable for the index/link + // cleanup below. The machine shares the connection's `link_id` and + // lifetime; dropping it here means a reaped handshake leg leaves no + // dangling machine. A no-op for promoted peers — `promote_connection` + // already consumed their connection, so this reaper never runs for + // them. + let conn = match self + .peer_machines + .get_mut(&link_id) + .and_then(|machine| machine.take_leg()) + { Some(c) => c, None => return, }; - // A dial-persisted outbound control machine shares the connection's - // `link_id` and lifetime; drop it here so a reaped handshake leg leaves - // no dangling machine. A no-op for promoted peers — `promote_connection` - // already removed their connection, so this reaper never runs for them. self.remove_peer_machine(link_id); let transport_id = conn.transport_id(); @@ -159,7 +167,7 @@ impl Node { /// reflex, then `cleanup_stale_connection` (which drops the machine + timers). /// /// `check_timeouts` keeps reaping everything else — `is_failed()` legs and the - /// idle-timeout of legs without a machine timer (inbound / machine-less). + /// idle-timeout of legs without a machine timer (inbound legs). fn drive_handshake_timeouts(&mut self, now_ms: u64) { let timeout_ms = self.config().node.rate_limit.handshake_timeout_secs * 1000; let timer_links: Vec = self @@ -169,7 +177,7 @@ impl Node { .map(|(link, _)| *link) .collect(); for link in timer_links { - let (reap, retry_peer) = match self.connections.get(&link) { + let (reap, retry_peer) = match self.leg(&link) { Some(conn) if conn.is_timed_out(now_ms, timeout_ms) => { let retry_peer = if conn.is_outbound() { conn.expected_identity().map(|id| *id.node_addr()) @@ -239,7 +247,7 @@ impl Node { // Skip a resend whose target peer already promoted via the inbound // cross-connection path — resending msg1 would start a new handshake // (session mismatch). - if let Some(conn) = self.connections.get(&link) + if let Some(conn) = self.leg(&link) && conn .expected_identity() .map(|id| self.peers.contains_key(id.node_addr())) @@ -264,7 +272,7 @@ impl Node { continue; } }; - match self.connections.get(&link).and_then(|c| c.handshake_msg1()) { + match self.leg(&link).and_then(|c| c.handshake_msg1()) { // Armed but the stored wire isn't there yet — leave the timer and // retry next tick (matches the old candidate filter skipping it). None => continue, @@ -296,7 +304,7 @@ impl Node { continue; }; - let (transport_id, remote_addr) = match self.connections.get(&link) { + let (transport_id, remote_addr) = match self.leg(&link) { Some(conn) => match (conn.transport_id(), conn.source_addr()) { (Some(tid), Some(addr)) => (tid, addr.clone()), _ => continue, diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index bdf478f..ba4c9a7 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -372,7 +372,7 @@ impl Node { } fn is_connecting_to_peer(&self, peer_node_addr: &NodeAddr) -> bool { - self.connections.values().any(|conn| { + self.connections().any(|conn| { conn.expected_identity() .map(|id| id.node_addr() == peer_node_addr) .unwrap_or(false) @@ -385,7 +385,7 @@ impl Node { transport_id: TransportId, remote_addr: &TransportAddr, ) -> bool { - self.connections.values().any(|conn| { + self.connections().any(|conn| { conn.expected_identity() .map(|id| id.node_addr() == peer_node_addr) .unwrap_or(false) @@ -583,15 +583,16 @@ impl Node { /// Prepare an outbound Noise msg1 at dial for an IDENTIFIED leg: allocate the /// session index, run the Noise leaf, frame the wire, arm the shell-side - /// resend, track `pending_outbound`, and persist the connection. Returns - /// `Err` on index-allocation or Noise failure (cleaning the partial leg, - /// including the dial-time control machine), leaving the armed wire on the - /// connection for `send_stored_msg1` to transmit. Does NOT send — so the - /// fallible setup can propagate its error synchronously before any machine - /// drive. Anonymous-discovery legs use the monolithic `start_handshake` - /// instead, whose machine is born after its fallible setup — so this is - /// the only caller path whose Err arms clean up `peer_machines` (the - /// dial-time machine predates the failure). + /// resend, track `pending_outbound`, and embed the prepared connection on + /// the dial-time control machine. Returns `Err` on index-allocation or + /// Noise failure (cleaning the partial leg, including the dial-time control + /// machine), leaving the armed wire on the connection for + /// `send_stored_msg1` to transmit. Does NOT send — so the fallible setup + /// can propagate its error synchronously before any machine drive. + /// Anonymous-discovery legs use the monolithic `start_handshake` instead, + /// whose machine is born after its fallible setup — so this is the only + /// caller path whose Err arms clean up `peer_machines` (the dial-time + /// machine predates the failure). pub(in crate::node) fn prepare_outbound_msg1( &mut self, link_id: LinkId, @@ -658,7 +659,21 @@ impl Node { // Track in pending_outbound for msg2 dispatch self.pending_outbound .insert((transport_id, our_index.as_u32()), link_id); - self.connections.insert(link_id, connection); + + // The dial-born machine (persisted in `initiate_connection`) carries + // the prepared connection from here. Every live caller dialed first, + // so the machine exists; 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" + ); + self.peer_machines + .entry(link_id) + .or_insert_with(|| { + PeerMachine::new_outbound(link_id, Some(peer_identity), current_time_ms) + }) + .set_leg(connection); Ok(()) } @@ -745,23 +760,21 @@ impl Node { // Track in pending_outbound for msg2 dispatch self.pending_outbound .insert((transport_id, our_index.as_u32()), link_id); - self.connections.insert(link_id, connection); - // The leg's persistent control machine is born alongside its pending + // The leg's persistent control machine is born carrying its pending // connection, after all fallible setup (the index-alloc and Noise Err // returns above predate it and need no disposal). Both production // callers dial anonymously (identified dials persist their machine at // dial and go through `prepare_outbound_msg1`), so the machine starts // identity-less; `handle_msg2` crystallizes the identity the XX // handshake learns. Inserted before the send below so no suspension - // point observes a leg without a machine. The send-failure arm retains - // the failed connection, and the machine stays with it — the stale- - // connection reaper disposes both together. No timers are armed and no - // event is dispatched: this path sends msg1 inline, so the machine - // parks at `Discovered` until msg2. - self.peer_machines.insert( - link_id, - PeerMachine::new_outbound(link_id, peer_identity, current_time_ms), - ); + // point observes a leg in flight without a machine. The send-failure + // arm retains the failed connection, and the machine keeps carrying + // it — the stale-connection reaper disposes both together. No timers + // are armed and no event is dispatched: this path sends msg1 inline, + // so the machine parks at `Discovered` until msg2. + let mut machine = PeerMachine::new_outbound(link_id, peer_identity, current_time_ms); + machine.set_leg(connection); + self.peer_machines.insert(link_id, machine); // Send the wire format handshake message if let Some(transport) = self.transports.get(&transport_id) { @@ -782,7 +795,7 @@ impl Node { ); // Mark connection as failed but don't remove it yet // The event loop can handle retry logic - if let Some(conn) = self.connections.get_mut(&link_id) { + if let Some(conn) = self.leg_mut(&link_id) { conn.mark_failed(); } } @@ -804,15 +817,11 @@ impl Node { transport_id: TransportId, remote_addr: &TransportAddr, ) { - let wire_msg1 = match self - .connections - .get(&link_id) - .and_then(|c| c.handshake_msg1()) - { + let wire_msg1 = match self.leg(&link_id).and_then(|c| c.handshake_msg1()) { Some(w) => w.to_vec(), None => return, }; - let our_index = self.connections.get(&link_id).and_then(|c| c.our_index()); + let our_index = self.leg(&link_id).and_then(|c| c.our_index()); // Send the wire format handshake message if let Some(transport) = self.transports.get(&transport_id) { @@ -835,7 +844,7 @@ impl Node { ); // Mark connection as failed but don't remove it yet // The event loop can handle retry logic - if let Some(conn) = self.connections.get_mut(&link_id) { + if let Some(conn) = self.leg_mut(&link_id) { conn.mark_failed(); } } @@ -1066,14 +1075,13 @@ impl Node { ); let now_ms = Self::now_ms(); let stale: Vec = self - .connections - .iter() - .filter(|(_, conn)| { + .connections() + .filter(|conn| { conn.expected_identity() .map(|id| id.node_addr() == &peer_addr) .unwrap_or(false) }) - .map(|(link_id, _)| *link_id) + .map(|conn| conn.link_id()) .collect(); for link_id in stale { self.cleanup_stale_connection(link_id, now_ms); @@ -2043,7 +2051,7 @@ impl Node { info!("Node started:"); info!(" state: {}", self.supervisor.state); info!(" transports: {}", self.transports.len()); - info!(" connections: {}", self.connections.len()); + info!(" connections: {}", self.connection_count()); Ok(()) } @@ -2887,18 +2895,17 @@ impl Node { /// The `connected` / `connecting` sets gate the floor, retry-dial, overlay, /// and LAN layers; `in_flight_by_peer` feeds the opportunistic layer's /// per-peer parallel cap, computed exactly as the deleted - /// `path_candidate_attempt_budget` did: `connections(expected == addr) + + /// `path_candidate_attempt_budget` did: `pending legs(expected == addr) + /// pending_connects(addr)`. The scalar counts stay unpopulated at the /// ceiling-only posture (no layer reads them; see [`Observed`]). pub(in crate::node) fn observe_peering(&self) -> Observed { let connected: HashSet = self.peers.keys().copied().collect(); let connecting: HashSet = self - .connections - .values() + .connections() .filter_map(|conn| conn.expected_identity().map(|id| *id.node_addr())) .collect(); let mut in_flight_by_peer: HashMap = HashMap::new(); - for conn in self.connections.values() { + for conn in self.connections() { if let Some(id) = conn.expected_identity() { *in_flight_by_peer.entry(*id.node_addr()).or_default() += 1; } @@ -2940,9 +2947,11 @@ impl Node { } fn outbound_handshake_slots(&self) -> usize { + // Count pending connections (legs), not machines: a dial-born machine + // in the connect window has no connection yet but DOES hold a + // `pending_connects` slot, so counting machines would tally it twice. let used = self - .connections - .len() + .connection_count() .saturating_add(self.peering.pending_connects.len()); if self.max_connections() == 0 { usize::MAX @@ -2968,8 +2977,7 @@ impl Node { } let in_flight_for_peer = self - .connections - .values() + .connections() .filter(|conn| { conn.expected_identity() .map(|identity| identity.node_addr() == peer_node_addr) diff --git a/src/node/mod.rs b/src/node/mod.rs index 193d79b..a661a6c 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -297,7 +297,8 @@ struct PendingConnect { /// ## Peer Lifecycle /// /// Peers go through two phases: -/// 1. **Connection phase** (`connections`): Handshake in progress, indexed by LinkId +/// 1. **Connection phase**: Handshake in progress; the pending connection is +/// carried by its per-peer control machine (`peer_machines`), indexed by LinkId /// 2. **Active phase** (`peers`): Authenticated, indexed by NodeAddr /// /// The `addr_to_link` map enables dispatching incoming packets to the right @@ -364,23 +365,17 @@ pub struct Node { /// rx_loop select arm that feeds `Event::ChildExited` to the supervisor FSM. child_exit_rx: Option>, - // === Connections (Handshake Phase) === - /// Pending connections (handshake in progress). - /// Indexed by LinkId since we don't know the peer's identity yet. - connections: HashMap, - // === Per-Peer Control Machines === /// Per-peer lifecycle control FSMs, keyed by the stable `LinkId` that spans - /// the handshake→active lifetime. A parallel structure introduced by the - /// node-runtime decomposition: `connections`/`peers` stay byte-unchanged (hot - /// path pristine) and are cut over to this machine home path-by-path. - /// Machines are inserted at the identified dial and born `established()` - /// inside `promote_connection` (msg3 decisions run through a throwaway - /// machine), and stepped in production by the handshake handlers, the - /// rekey-cadence and liveness-reap routers, and the lifecycle paths, with - /// the executor (`dataplane/peer_actions.rs`) performing the returned - /// actions. Timer FIRING decisions remain shell-side: `PeerEvent::Timeout` - /// is never dispatched in production. + /// 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` + /// 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, + /// and the lifecycle paths, with the executor (`dataplane/peer_actions.rs`) + /// performing the returned actions. Timer FIRING decisions remain + /// shell-side: `PeerEvent::Timeout` is never dispatched in production. peer_machines: HashMap, /// Per-peer timer store, keyed by `LinkId` then `TimerKind`, holding each @@ -667,7 +662,6 @@ impl Node { packet_rx: None, child_exit_tx: None, child_exit_rx: None, - connections: HashMap::new(), peer_machines: HashMap::new(), peer_timers: HashMap::new(), peers: HashMap::new(), @@ -818,7 +812,6 @@ impl Node { packet_rx: None, child_exit_tx: None, child_exit_rx: None, - connections: HashMap::new(), peer_machines: HashMap::new(), peer_timers: HashMap::new(), peers: HashMap::new(), @@ -1562,7 +1555,7 @@ impl Node { tun_state: self.tun_state, tun_name: self.tun_name.clone(), effective_ipv6_mtu: self.effective_ipv6_mtu(), - connection_count: self.connections.len(), + connection_count: self.connection_count(), peer_count: self.peers.len(), link_count: self.links.len(), transport_count: self.transports.len(), @@ -2193,7 +2186,10 @@ impl Node { /// Number of pending connections (handshake in progress). pub fn connection_count(&self) -> usize { - self.connections.len() + self.peer_machines + .values() + .filter(|machine| machine.leg().is_some()) + .count() } /// Number of authenticated peers. @@ -2314,31 +2310,19 @@ impl Node { self.peer_timers.remove(&link); } - /// Gates the leg→machine direction of `debug_assert_peer_maps_coherent`. - /// Asserting it requires the convention that every `connections` insert is - /// paired with a `peer_machines` insert before the next await point — - /// which now holds everywhere: identified dials persist their machine - /// before the leg exists (`initiate_connection`), anonymous-discovery legs - /// are born with one inside `start_handshake` alongside the connection - /// insert, inbound legs get theirs in `handle_msg1` at the same point, and - /// the `add_connection` test seam seeds one for directly-inserted legs. - #[cfg(debug_assertions)] - const CHECK_LEGS_HAVE_MACHINES: bool = true; - /// Debug-build coherence sweep over the peer-lifecycle maps, run once per /// rx-loop tick and invoked directly by unit tests. /// /// Machine→carrier: every `peer_machines` entry must have a live carrier — - /// a pending connection on the same link, an active peer on it, or a + /// its own embedded pending connection, an active peer on its link, or a /// pending connect still resolving toward it. A machine with none of these - /// is unreachable by every teardown path and has leaked. - /// - /// Leg→machine: every pending connection carries a control machine (gated - /// by `CHECK_LEGS_HAVE_MACHINES` above). + /// is unreachable by every teardown path and has leaked. (The pending + /// connection lives inside the machine, so no separate connection→machine + /// direction exists to check.) #[cfg(debug_assertions)] pub(in crate::node) fn debug_assert_peer_maps_coherent(&self) { - for link in self.peer_machines.keys() { - let has_carrier = self.connections.contains_key(link) + for (link, machine) in &self.peer_machines { + let has_carrier = machine.leg().is_some() || self.peers.values().any(|peer| peer.link_id() == *link) || self .peering @@ -2351,22 +2335,13 @@ impl Node { (no pending connection, active peer, or pending connect)" ); } - - if Self::CHECK_LEGS_HAVE_MACHINES { - for link in self.connections.keys() { - assert!( - self.peer_machines.contains_key(link), - "pending connection on link {link} has no control machine" - ); - } - } } /// Operator-visible msg1 resend count for a pending handshake `link`, read /// from the per-peer machine (the counter's home once the resend drive moved - /// off the shell connection). Machine-less connections (inbound legs that - /// never resend, and test-created connections) report 0, matching what the - /// shell connection reported before the counter moved. + /// off the shell connection). A link with no machine reports 0, and inbound + /// machines never resend, matching what the shell connection reported + /// before the counter moved. pub(crate) fn connection_resend_count(&self, link: LinkId) -> u32 { self.peer_machines .get(&link) @@ -2387,8 +2362,7 @@ impl Node { .values() .any(|link| link.transport_id() == transport_id) || self - .connections - .values() + .connections() .any(|conn| conn.transport_id() == Some(transport_id)) || self .peers @@ -2423,58 +2397,86 @@ 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> { + 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. /// - /// Also seeds a control machine for the leg when none exists yet, keeping - /// the leg→machine invariant (`debug_assert_peer_maps_coherent`) intact - /// for callers that insert a connection directly rather than through the - /// dial or inbound-msg1 paths, which pair the two inserts themselves. + /// 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.link_id(); - if self.connections.contains_key(&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.connections.len() >= self.max_connections() { + if self.max_connections() > 0 && self.connection_count() >= self.max_connections() { return Err(NodeError::MaxConnectionsExceeded { max: self.max_connections(), }); } - self.peer_machines.entry(link_id).or_insert_with(|| { - let now = connection.started_at(); - if connection.is_outbound() { - PeerMachine::new_outbound(link_id, connection.expected_identity().copied(), now) - } else { - PeerMachine::new_inbound(link_id, now) - } - }); - - self.connections.insert(link_id, connection); + self.peer_machines + .entry(link_id) + .or_insert_with(|| { + let now = connection.started_at(); + if connection.is_outbound() { + PeerMachine::new_outbound(link_id, connection.expected_identity().copied(), now) + } else { + PeerMachine::new_inbound(link_id, now) + } + }) + .set_leg(connection); Ok(()) } /// Get a connection by LinkId. pub fn get_connection(&self, link_id: &LinkId) -> Option<&PeerConnection> { - self.connections.get(link_id) + self.leg(link_id) } /// Get a mutable connection by LinkId. pub fn get_connection_mut(&mut self, link_id: &LinkId) -> Option<&mut PeerConnection> { - self.connections.get_mut(link_id) + self.leg_mut(link_id) } /// Remove a connection, disposing its control machine alongside - /// (the disposal complement of `add_connection`'s machine seeding). + /// (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); - self.connections.remove(link_id) + connection } /// Iterate over all connections. pub fn connections(&self) -> impl Iterator { - self.connections.values() + self.peer_machines + .values() + .filter_map(|machine| machine.leg()) } // === Peer Management (Active Phase) === @@ -3044,7 +3046,8 @@ impl Node { /// Unlike `send_encrypted_link_message`, this does not look the peer up /// in `self.peers`. It takes the session and wire parameters directly, /// so it can be used during handshake teardown (before promotion) where - /// the peer state lives in `self.connections` rather than `self.peers`. + /// the peer state lives on the pending connection embedded in its control + /// machine rather than in `self.peers`. /// /// The inner-header timestamp is set to 0 — the session has just been /// established and no session-elapsed reference is available yet; this diff --git a/src/node/peering/reconcile.rs b/src/node/peering/reconcile.rs index 4c6078d..92ca817 100644 --- a/src/node/peering/reconcile.rs +++ b/src/node/peering/reconcile.rs @@ -147,7 +147,7 @@ pub(crate) struct Observed { #[allow(dead_code)] // ceiling-only posture: no layer reads the scalar counts (future set-point) pub peers: usize, - /// `self.connections.len()`. + /// `self.connection_count()`. #[allow(dead_code)] // ceiling-only posture: no layer reads the scalar counts (future set-point) pub connections: usize, diff --git a/src/node/tests/acl.rs b/src/node/tests/acl.rs index 711fc9d..58b869f 100644 --- a/src/node/tests/acl.rs +++ b/src/node/tests/acl.rs @@ -76,16 +76,10 @@ async fn test_outbound_msg2_denied_after_acl_reload() { node_a .addr_to_link .insert((transport_id, remote_addr.clone()), link_id_a); - node_a.connections.insert(link_id_a, conn_a); + node_a.add_connection(conn_a).unwrap(); node_a .pending_outbound .insert((transport_id, our_index_a.as_u32()), link_id_a); - // Mirror the production dial path: an outbound leg persists its control - // machine at dial. - node_a.peer_machines.insert( - link_id_a, - crate::peer::machine::PeerMachine::new_outbound(link_id_a, Some(peer_b_identity), 1000), - ); let mut conn_b = PeerConnection::inbound(LinkId::new(2), 1000); let responder_epoch = [0x11; 8]; @@ -202,16 +196,12 @@ async fn test_inbound_msg3_denied_triggers_disconnect() { node_a .addr_to_link .insert((transport_id_a, addr_b.clone()), link_id_a); - node_a.connections.insert(link_id_a, conn_a); + // Mirror the production dial path: the seam seeds the outbound leg's + // control machine and embeds the connection on it. + node_a.add_connection(conn_a).unwrap(); node_a .pending_outbound .insert((transport_id_a, our_index_a.as_u32()), link_id_a); - // Mirror the production dial path: an outbound leg persists its control - // machine at dial. - node_a.peer_machines.insert( - link_id_a, - crate::peer::machine::PeerMachine::new_outbound(link_id_a, Some(peer_b_identity), 1000), - ); let transport = node_a.transports.get(&transport_id_a).unwrap(); transport diff --git a/src/node/tests/handshake.rs b/src/node/tests/handshake.rs index 3aafe28..eaf872c 100644 --- a/src/node/tests/handshake.rs +++ b/src/node/tests/handshake.rs @@ -81,16 +81,10 @@ async fn test_two_node_handshake_udp() { Duration::from_millis(100), ); node_a.links.insert(link_id_a, link_a); - node_a.connections.insert(link_id_a, conn_a); + node_a.add_connection(conn_a).unwrap(); node_a .pending_outbound .insert((transport_id_a, our_index_a.as_u32()), link_id_a); - // Mirror the production dial path: an outbound leg persists its control - // machine at dial. - node_a.peer_machines.insert( - link_id_a, - crate::peer::machine::PeerMachine::new_outbound(link_id_a, Some(peer_b_identity), 1000), - ); // Send msg1 from A to B over UDP let transport = node_a.transports.get(&transport_id_a).unwrap(); @@ -118,7 +112,7 @@ async fn test_two_node_handshake_udp() { "Node B should have 0 peers after msg1 (XX awaits msg3)" ); assert_eq!( - node_b.connections.len(), + node_b.connection_count(), 1, "Node B should have 1 pending connection awaiting msg3" ); @@ -346,16 +340,10 @@ async fn test_run_rx_loop_handshake() { Duration::from_millis(100), ); node_a.links.insert(link_id_a, link_a); - node_a.connections.insert(link_id_a, conn_a); + node_a.add_connection(conn_a).unwrap(); node_a .pending_outbound .insert((transport_id_a, our_index_a.as_u32()), link_id_a); - // Mirror the production dial path: an outbound leg persists its control - // machine at dial. - node_a.peer_machines.insert( - link_id_a, - crate::peer::machine::PeerMachine::new_outbound(link_id_a, Some(peer_b_identity), 1000), - ); // Send msg1 from A to B over real UDP let transport = node_a.transports.get(&transport_id_a).unwrap(); @@ -394,7 +382,7 @@ async fn test_run_rx_loop_handshake() { "Node B should have 0 peers after rx loop processed msg1 (XX awaits msg3)" ); assert_eq!( - node_b.connections.len(), + node_b.connection_count(), 1, "Node B should have 1 pending connection" ); @@ -445,7 +433,7 @@ async fn test_run_rx_loop_handshake() { // This test verifies rx_loop correctly dispatches PHASE_MSG1 (Phase 2) // and PHASE_MSG2 (Phase 3). B still has a pending connection awaiting msg3. assert_eq!( - node_b.connections.len(), + node_b.connection_count(), 1, "Node B should still have pending connection awaiting msg3" ); @@ -540,16 +528,10 @@ async fn test_cross_connection_both_initiate() { node_a .addr_to_link .insert((transport_id_a, remote_addr_b.clone()), link_id_a_out); - node_a.connections.insert(link_id_a_out, conn_a); + node_a.add_connection(conn_a).unwrap(); node_a .pending_outbound .insert((transport_id_a, our_index_a.as_u32()), link_id_a_out); - // Mirror the production dial path: an outbound leg persists its control - // machine at dial. - node_a.peer_machines.insert( - link_id_a_out, - crate::peer::machine::PeerMachine::new_outbound(link_id_a_out, Some(peer_b_identity), 1000), - ); // Node B initiates to Node A let link_id_b_out = node_b.allocate_link_id(); @@ -576,16 +558,10 @@ async fn test_cross_connection_both_initiate() { node_b .addr_to_link .insert((transport_id_b, remote_addr_a.clone()), link_id_b_out); - node_b.connections.insert(link_id_b_out, conn_b); + node_b.add_connection(conn_b).unwrap(); node_b .pending_outbound .insert((transport_id_b, our_index_b.as_u32()), link_id_b_out); - // Mirror the production dial path: an outbound leg persists its control - // machine at dial. - node_b.peer_machines.insert( - link_id_b_out, - crate::peer::machine::PeerMachine::new_outbound(link_id_b_out, Some(peer_a_identity), 1000), - ); // Both send msg1 over UDP let transport = node_a.transports.get(&transport_id_a).unwrap(); @@ -750,7 +726,7 @@ async fn test_stale_connection_cleanup() { node.links.insert(link_id, link); node.addr_to_link .insert((transport_id, remote_addr.clone()), link_id); - node.connections.insert(link_id, conn); + node.add_connection(conn).unwrap(); node.pending_outbound .insert((transport_id, our_index.as_u32()), link_id); @@ -828,7 +804,7 @@ async fn test_failed_connection_cleanup() { node.links.insert(link_id, link); node.addr_to_link .insert((transport_id, remote_addr.clone()), link_id); - node.connections.insert(link_id, conn); + node.add_connection(conn).unwrap(); node.pending_outbound .insert((transport_id, our_index.as_u32()), link_id); @@ -924,11 +900,11 @@ async fn test_resend_scheduling() { .insert((transport_id, remote_addr.clone()), link_id); node.pending_outbound .insert((transport_id, our_index.as_u32()), link_id); - node.connections.insert(link_id, conn); - // The msg1-resend counter and its due timer live on the per-peer machine. - // Dial it to `SentMsg1` (connectionless: no connect step) and arm its - // retransmit timer at now + 1000ms, mirroring what a real dial arms. + // The msg1-resend counter and its due timer live on the per-peer machine, + // which also carries the pending connection. Dial it to `SentMsg1` + // (connectionless: no connect step) and arm its retransmit timer at + // now + 1000ms, mirroring what a real dial arms. let mut machine = crate::peer::machine::PeerMachine::new_outbound(link_id, Some(peer_identity), now_ms); let _ = machine.step( @@ -941,6 +917,7 @@ async fn test_resend_scheduling() { now_ms, &mut node.index_allocator, ); + machine.set_leg(conn); node.peer_machines.insert(link_id, machine); node.peer_timers.entry(link_id).or_default().insert( crate::peer::machine::TimerKind::HandshakeRetransmit, @@ -999,11 +976,11 @@ async fn test_handshake_timeout_drive() { node.links.insert(link_id, link); node.addr_to_link .insert((transport_id, remote_addr.clone()), link_id); - node.connections.insert(link_id, conn); node.pending_outbound .insert((transport_id, our_index.as_u32()), link_id); - // Machine in SentMsg1 with a HandshakeTimeout timer armed at dial + 30s. + // Machine in SentMsg1, carrying the pending connection, with a + // HandshakeTimeout timer armed at dial + 30s. let mut machine = crate::peer::machine::PeerMachine::new_outbound(link_id, Some(peer_identity), dial_ms); let _ = machine.step( @@ -1016,6 +993,7 @@ async fn test_handshake_timeout_drive() { dial_ms, &mut node.index_allocator, ); + machine.set_leg(conn); node.peer_machines.insert(link_id, machine); node.peer_timers.entry(link_id).or_default().insert( crate::peer::machine::TimerKind::HandshakeTimeout, @@ -1505,14 +1483,10 @@ async fn drive_to_msg3( .node .addr_to_link .insert((initiator.transport_id, responder.addr.clone()), link_id); - initiator.node.connections.insert(link_id, conn); - // Mirror the production dial path: an identified outbound leg persists - // its control machine at dial, and the promote feedback later - // crystallizes that same machine in place. - initiator.node.peer_machines.insert( - link_id, - crate::peer::machine::PeerMachine::new_outbound(link_id, Some(peer_identity), now_ms), - ); + // Mirror the production dial path: the seam seeds the identified outbound + // leg's control machine (carrying the connection) at dial, and the promote + // feedback later crystallizes that same machine in place. + initiator.node.add_connection(conn).unwrap(); initiator .node .pending_outbound @@ -1698,8 +1672,8 @@ async fn test_inbound_machine_born_at_msg1_and_crystallized_at_promote() { // After msg1 the responder's window leg carries a machine parked at // `SentMsg2`, seeded with the leg's msg1-allocated index. - assert_eq!(responder.node.connections.len(), 1); - let leg_link = *responder.node.connections.keys().next().unwrap(); + assert_eq!(responder.node.connection_count(), 1); + let leg_link = responder.node.connections().next().unwrap().link_id(); let leg_index = responder .node .get_connection(&leg_link) @@ -1777,7 +1751,10 @@ async fn test_msg3_crypto_fail_disposes_leg_machine() { responder.node.handle_msg3(msg3).await; assert_eq!(responder.node.peer_count(), 0, "no promotion"); - assert!(responder.node.connections.is_empty(), "leg torn down"); + assert!( + responder.node.connections().next().is_none(), + "leg torn down" + ); assert!( responder.node.peer_machines.is_empty(), "crypto-fail teardown disposes the leg's machine" @@ -1864,8 +1841,8 @@ async fn test_anonymous_dial_births_identityless_machine_at_leg_birth() { .await .expect("anonymous dial"); - assert_eq!(initiator.node.connections.len(), 1); - let leg_link = *initiator.node.connections.keys().next().unwrap(); + assert_eq!(initiator.node.connection_count(), 1); + let leg_link = initiator.node.connections().next().unwrap().link_id(); let machine = initiator .node .peer_machines @@ -1900,7 +1877,7 @@ async fn test_anonymous_msg2_crystallizes_identity_and_promotes() { .initiate_connection(initiator.transport_id, responder.addr.clone(), None) .await .expect("anonymous dial"); - let leg_link = *initiator.node.connections.keys().next().unwrap(); + let leg_link = initiator.node.connections().next().unwrap().link_id(); initiator.node.debug_assert_peer_maps_coherent(); // Responder answers msg1 with msg2; the initiator's msg2 processing learns @@ -1960,7 +1937,7 @@ async fn test_anonymous_self_connect_drop_disposes_machine() { .initiate_connection(node.transport_id, self_addr, None) .await .expect("anonymous self dial"); - let leg_link = *node.node.connections.keys().next().unwrap(); + let leg_link = node.node.connections().next().unwrap().link_id(); assert_eq!(node.node.peer_machines.len(), 1); // We answer our own msg1, then our msg2 processing discovers the learned @@ -1972,7 +1949,7 @@ async fn test_anonymous_self_connect_drop_disposes_machine() { assert_eq!(node.node.peer_count(), 0, "no promotion"); assert!( - !node.node.connections.contains_key(&leg_link), + node.node.get_connection(&leg_link).is_none(), "self-connect drop removes the outbound leg" ); assert!( diff --git a/src/node/tests/spanning_tree.rs b/src/node/tests/spanning_tree.rs index 069805b..7a4286b 100644 --- a/src/node/tests/spanning_tree.rs +++ b/src/node/tests/spanning_tree.rs @@ -162,14 +162,7 @@ pub(super) async fn initiate_handshake(nodes: &mut [TestNode], i: usize, j: usiz .node .addr_to_link .insert((transport_id, responder_addr.clone()), link_id); - initiator.node.connections.insert(link_id, conn); - // Mirror the production dial path: an identified outbound leg persists - // its control machine at dial, and the promote feedback later - // crystallizes that same machine in place. - initiator.node.peer_machines.insert( - link_id, - crate::peer::machine::PeerMachine::new_outbound(link_id, Some(peer_identity), 1000), - ); + initiator.node.add_connection(conn).unwrap(); initiator .node .pending_outbound diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 712220d..883a35b 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -137,8 +137,7 @@ async fn test_try_peer_addresses_races_all_concrete_udp_candidates() { .unwrap(); let mut addrs = node - .connections - .values() + .connections() .filter_map(|conn| conn.source_addr().and_then(|addr| addr.as_str())) .collect::>(); addrs.sort(); @@ -733,7 +732,7 @@ fn test_promote_cleans_up_pending_outbound_to_same_peer() { node.links.insert(pending_link_id, pending_link); node.addr_to_link .insert((transport_id, pending_addr.clone()), pending_link_id); - node.connections.insert(pending_link_id, pending_conn); + node.add_connection(pending_conn).unwrap(); node.pending_outbound .insert((transport_id, pending_index.as_u32()), pending_link_id); @@ -1255,8 +1254,7 @@ async fn update_peers_races_new_alternative_without_dropping_active_peer() { assert_eq!(node.peer_count(), 1, "existing link must stay live"); assert_eq!(node.connection_count(), 1); assert_eq!( - node.connections - .values() + node.connections() .next() .and_then(|conn| conn.source_addr()), Some(&new_addr) @@ -2027,16 +2025,12 @@ async fn drive_xx_handshake( Duration::from_millis(100), ); node_a.links.insert(link_id_a, link_a); - node_a.connections.insert(link_id_a, conn_a); + // Mirror the production dial path: the seam seeds the outbound leg's + // control machine and embeds the connection on it. + node_a.add_connection(conn_a).unwrap(); node_a .pending_outbound .insert((transport_id, our_index_a.as_u32()), link_id_a); - // Mirror the production dial path: an outbound leg persists its control - // machine at dial. - node_a.peer_machines.insert( - link_id_a, - crate::peer::machine::PeerMachine::new_outbound(link_id_a, Some(peer_b_identity), 1000), - ); let transport = node_a.transports.get(&transport_id).unwrap(); transport diff --git a/src/peer/machine.rs b/src/peer/machine.rs index 1233e62..34e7a3a 100644 --- a/src/peer/machine.rs +++ b/src/peer/machine.rs @@ -78,6 +78,7 @@ #![allow(dead_code)] +use crate::peer::PeerConnection; use crate::proto::fmp::{ ConnAction, ConnSnapshot, ConnectionState, EstablishSnapshot, Fmp, InboundDecision, InboundReject, PeerSnapshot, PromotionResult, RekeyCfg, RekeyResendSnapshot, WireOutcome, @@ -428,6 +429,12 @@ pub(crate) struct PeerMachine { /// The crystallized peer node address. Known up front for outbound (from the /// dial identity); learned at msg3 for inbound (from [`WireOutcome`]). node_addr: Option, + /// The pending handshake connection this machine owns while the leg 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). Pure storage — the machine never reads + /// or drives it; the shell reaches it through the accessors below. + leg: Option, /// Pure handshake-phase bookkeeping (link/direction/indices/transport/ /// stored handshake bytes/epoch). Reused verbatim from the FMP state core. conn: ConnectionState, @@ -475,6 +482,7 @@ impl PeerMachine { link, identity, node_addr: identity.map(|id| *id.node_addr()), + leg: None, conn: match identity { Some(id) => ConnectionState::outbound(link, id, now), None => ConnectionState::outbound_anonymous(link, now), @@ -505,6 +513,7 @@ impl PeerMachine { link, identity: None, node_addr: None, + leg: None, conn: ConnectionState::inbound(link, now), remote_epoch: None, rekey_in_progress: false, @@ -572,6 +581,7 @@ impl PeerMachine { link, identity: Some(identity), node_addr: Some(addr), + leg: None, conn, remote_epoch, rekey_in_progress: false, @@ -594,6 +604,28 @@ impl PeerMachine { self.state } + /// The pending handshake connection, if this leg is still in the + /// handshake window. + pub(crate) fn leg(&self) -> Option<&PeerConnection> { + 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 + /// teardown consume it by value). + 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) { + self.leg = Some(leg); + } + /// The index we allocated for this peer's inbound session at msg1. `None` /// before allocation. The inbound cross-connection / rekey-respond triggers /// and the executor read this to perform the shell registry surgery with the