mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-22 07:48:26 +00:00
peer: persist the outbound control machine at dial
Create and persist the per-peer control machine when an outbound handshake is dialed, keyed by its link, instead of building a transient at msg2. The msg2 completion path now looks up that persisted machine to drive the promote, falling back to a transient only if none is present (e.g. a direct-seeded test). The machine parks in the Discovered state until promotion and is inert to the liveness reap and rekey cadence while unpromoted, since it is absent from the peers map. It is removed on every path that ends the outbound leg without promoting -- the stale-connection reaper, the msg2 authorization-failure arm, and the cross-connection resolution block -- mirroring the connection's own lifetime so no dangling machine survives. Its session index is deliberately left unset on the machine (the shell owns the index on its connection), so a later inbound restart does not emit a spurious decrypt-session unregister. A regression test covers that invariant.
This commit is contained in:
@@ -292,10 +292,9 @@ impl Node {
|
||||
// pre-refactor outbound warn! ("Failed to promote
|
||||
// connection").
|
||||
//
|
||||
// The transient outbound machine was inserted BEFORE
|
||||
// execute; it is additive state that
|
||||
// did not exist pre-refactor, so removing the just-
|
||||
// inserted machine on failure is neutral vs old and
|
||||
// The outbound machine was persisted at dial; it is
|
||||
// additive state that did not exist pre-refactor, so
|
||||
// removing it on promote failure is neutral vs old and
|
||||
// prevents a leak.
|
||||
warn!(
|
||||
target: "fips::node::handlers::handshake",
|
||||
|
||||
@@ -973,6 +973,8 @@ impl Node {
|
||||
}
|
||||
}
|
||||
self.connections.remove(&link_id);
|
||||
// Drop the machine persisted at dial — this leg never promotes.
|
||||
self.peer_machines.remove(&link_id);
|
||||
self.remove_link(&link_id);
|
||||
if let Some(idx) = our_index {
|
||||
let _ = self.index_allocator.free(idx);
|
||||
@@ -1007,6 +1009,12 @@ impl Node {
|
||||
let out_snap = self.outbound_snapshot(&peer_node_addr);
|
||||
let out_decision = self.fmp.establish_outbound(&out_snap);
|
||||
if out_decision != OutboundDecision::Promote {
|
||||
// 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,
|
||||
// which created no machine for a cross-connection.
|
||||
self.peer_machines.remove(&link_id);
|
||||
// Extract the outbound connection
|
||||
let mut conn = match self.connections.remove(&link_id) {
|
||||
Some(c) => c,
|
||||
@@ -1132,43 +1140,55 @@ impl Node {
|
||||
// loser-link surgery is wired later). Direct analog of the inbound
|
||||
// net-new arm — no ordering constraint, lowest risk.
|
||||
//
|
||||
// Build a TRANSIENT outbound machine, step `Msg2 →
|
||||
// [PromoteToActive]`, execute it (→ `promote_connection` →
|
||||
// `PromotionResolved{Promoted}` → inert `RegisterDecryptSession`), and
|
||||
// insert into `peer_machines` only on the Promoted (Established) tail. The
|
||||
// outbound `our_index` was allocated at DIAL (unchanged), the outbound
|
||||
// promote sends nothing on the wire, and `promote_connection` frees
|
||||
// nothing new — so the index sequence, `peers`/`peers_by_index`/
|
||||
// `addr_to_link` state, and metrics are byte-identical to the pre-refactor
|
||||
// Promoted arm. `pending_outbound` lifecycle stays shell-side (removed on
|
||||
// the Established tail, exactly where the pre-refactor Ok arm removed it);
|
||||
// the machine never touches it.
|
||||
let mut machine = PeerMachine::new_outbound(link_id, peer_identity, packet.timestamp_ms);
|
||||
|
||||
// Step `Msg2 → [PromoteToActive]`. The machine re-runs the pure
|
||||
// `establish_outbound` on the snapshot (a harmless second pure call);
|
||||
// `has_existing_peer == false` reproduces the `Promote` decision.
|
||||
let promote_actions = machine.step(
|
||||
PeerEvent::Msg2 {
|
||||
their_index: header.sender_idx,
|
||||
out: out_snap,
|
||||
},
|
||||
packet.timestamp_ms,
|
||||
&mut self.index_allocator,
|
||||
);
|
||||
// Look up the outbound machine persisted at DIAL (`start_handshake`),
|
||||
// parked in `Discovered`, and step `Msg2 → [PromoteToActive]` in place.
|
||||
// `on_msg2` is state-independent — it decides from the outbound snapshot,
|
||||
// not the machine's state — and reads the machine's unset `conn.our_index`
|
||||
// as `None`, so stepping the persisted (vs the former transient) machine
|
||||
// is byte-identical: same `Promote` decision (`has_existing_peer == false`),
|
||||
// same `our_index == None`. The machine is already in `peer_machines`
|
||||
// (inserted at dial), so the executor's `PromoteToActive` arm can feed
|
||||
// `PromotionResolved` back via the same lookup. A defensive transient
|
||||
// reproduces the pre-persistence path if the machine is somehow absent — a
|
||||
// state-machine inconsistency, since every dialed leg persists one. The
|
||||
// outbound `our_index` was allocated at DIAL (unchanged), the promote
|
||||
// sends nothing on the wire, and `promote_connection` frees nothing new,
|
||||
// so index sequence, `peers`/`peers_by_index`/`addr_to_link` state, and
|
||||
// metrics are byte-identical. `pending_outbound` lifecycle stays shell-side
|
||||
// (removed on the Established tail); the machine never touches it.
|
||||
let promote_actions = match self.peer_machines.get_mut(&link_id) {
|
||||
Some(machine) => machine.step(
|
||||
PeerEvent::Msg2 {
|
||||
their_index: header.sender_idx,
|
||||
out: out_snap,
|
||||
},
|
||||
packet.timestamp_ms,
|
||||
&mut self.index_allocator,
|
||||
),
|
||||
None => {
|
||||
// No machine persisted at dial (e.g. a test that seeds
|
||||
// `connections`/`pending_outbound` directly, or any path that
|
||||
// reaches msg2 without `start_handshake`): reproduce the
|
||||
// pre-persistence transient exactly.
|
||||
let mut machine =
|
||||
PeerMachine::new_outbound(link_id, peer_identity, packet.timestamp_ms);
|
||||
let actions = machine.step(
|
||||
PeerEvent::Msg2 {
|
||||
their_index: header.sender_idx,
|
||||
out: out_snap,
|
||||
},
|
||||
packet.timestamp_ms,
|
||||
&mut self.index_allocator,
|
||||
);
|
||||
self.peer_machines.insert(link_id, machine);
|
||||
actions
|
||||
}
|
||||
};
|
||||
debug_assert_eq!(
|
||||
promote_actions,
|
||||
vec![PeerAction::PromoteToActive { link: link_id }]
|
||||
);
|
||||
|
||||
// Register the machine (Promote tail only — the Swap/Keep/rekey arms above
|
||||
// all returned without inserting). Inserted BEFORE execute so the
|
||||
// executor's `PromoteToActive` arm can feed `PromotionResolved` back into
|
||||
// it via the `peer_machines` lookup. The outbound `link_id` was allocated
|
||||
// at dial and the dial path never inserts a machine, so this
|
||||
// cannot collide with an existing entry.
|
||||
self.peer_machines.insert(link_id, machine);
|
||||
|
||||
// Execute `[PromoteToActive]`. The executor calls `promote_connection`,
|
||||
// feeds `PromotionResolved{Promoted}` back, registers the decrypt-worker
|
||||
// session (the register was relocated into the executor's
|
||||
|
||||
@@ -115,6 +115,11 @@ impl Node {
|
||||
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.peer_machines.remove(&link_id);
|
||||
let transport_id = conn.transport_id();
|
||||
|
||||
// Free session index and pending_outbound if allocated
|
||||
|
||||
@@ -15,6 +15,7 @@ use crate::node::acl::PeerAclContext;
|
||||
use crate::nostr::{BootstrapEvent, NostrRendezvous};
|
||||
use crate::nostr::{BootstrapHandoffResult, EstablishedTraversal};
|
||||
use crate::peer::PeerConnection;
|
||||
use crate::peer::machine::PeerMachine;
|
||||
use crate::proto::fmp::wire::build_msg1;
|
||||
use crate::proto::fmp::{Disconnect, DisconnectReason};
|
||||
use crate::transport::{Link, LinkDirection, LinkId, TransportAddr, TransportId, packet_channel};
|
||||
@@ -582,6 +583,18 @@ impl Node {
|
||||
.insert((transport_id, our_index.as_u32()), link_id);
|
||||
self.connections.insert(link_id, connection);
|
||||
|
||||
// Persist the outbound control machine at dial, keyed by the same
|
||||
// `link_id` as the connection. It parks in `Discovered` (inert to reap
|
||||
// and rekey — it is absent from `peers`, never established) until
|
||||
// `handle_msg2` looks it up to drive the promote. Its `our_index` is
|
||||
// deliberately left unset so a later inbound restart does not emit a
|
||||
// spurious `UnregisterDecryptSession`. It is removed wherever the
|
||||
// connection is torn down without promoting (the stale reaper and the
|
||||
// msg2 ACL-fail / cross-connection arms), mirroring the connection's
|
||||
// own lifetime.
|
||||
let machine = PeerMachine::new_outbound(link_id, peer_identity, current_time_ms);
|
||||
self.peer_machines.insert(link_id, machine);
|
||||
|
||||
// Send the wire format handshake message
|
||||
if let Some(transport) = self.transports.get(&transport_id) {
|
||||
match transport.send(&remote_addr, &wire_msg1).await {
|
||||
|
||||
@@ -2002,6 +2002,97 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ---- Test 7b: dial-persisted outbound promote leaves our_index unset ---
|
||||
// An outbound machine persisted at DIAL (`new_outbound`, `Discovered`, with
|
||||
// `conn.our_index` UNSET — the shell owns the index on its own
|
||||
// `PeerConnection`, never on the machine) must, on promote via msg2, end with
|
||||
// `our_index == None`, exactly as the pre-persistence transient did. The
|
||||
// guard: a subsequent inbound restart then emits NO
|
||||
// `UnregisterDecryptSession` (contrast `restart_override`, whose machine has
|
||||
// `our_index == Some`). A leaked `Some(dial_index)` here would wrongly
|
||||
// unregister — on index reuse, ANOTHER peer's — worker session; keeping the
|
||||
// field `None` is the Model-B dial-persistence neutrality property.
|
||||
#[test]
|
||||
fn dial_persisted_outbound_promote_no_restart_unregister() {
|
||||
let mut alloc = IndexAllocator::new();
|
||||
let peer = peer_identity();
|
||||
let peer_addr = *peer.node_addr();
|
||||
let our = *peer_identity().node_addr();
|
||||
|
||||
// Persisted at dial: Discovered, conn.our_index deliberately NOT set.
|
||||
let mut m = PeerMachine::new_outbound(LinkId::new(1), peer, 0);
|
||||
assert_eq!(m.our_index(), None);
|
||||
|
||||
// Promote via msg2 from Discovered (the production path — the former
|
||||
// transient was likewise stepped from `new_outbound` without a state set).
|
||||
let out = OutboundSnapshot {
|
||||
has_existing_peer: false,
|
||||
our_outbound_wins: false,
|
||||
};
|
||||
let promote = m.step(
|
||||
PeerEvent::Msg2 {
|
||||
their_index: SessionIndex::new(0x77),
|
||||
out,
|
||||
},
|
||||
300,
|
||||
&mut alloc,
|
||||
);
|
||||
assert_eq!(
|
||||
promote,
|
||||
vec![PeerAction::PromoteToActive {
|
||||
link: LinkId::new(1)
|
||||
}]
|
||||
);
|
||||
assert_eq!(
|
||||
m.our_index(),
|
||||
None,
|
||||
"outbound promote must leave our_index unset"
|
||||
);
|
||||
|
||||
// Drive promotion to Established (from Discovered, as in production).
|
||||
let _ = m.step(
|
||||
PeerEvent::PromotionResolved {
|
||||
result: PromotionResult::Promoted(peer_addr),
|
||||
},
|
||||
300,
|
||||
&mut alloc,
|
||||
);
|
||||
assert_eq!(m.state(), PeerState::Established { addr: peer_addr });
|
||||
assert_eq!(m.our_index(), None);
|
||||
|
||||
// A subsequent inbound restart (peer restart, new epoch) must NOT emit
|
||||
// UnregisterDecryptSession, because our_index is None.
|
||||
let mut est = est_new_peer(our);
|
||||
est.has_existing_peer = true;
|
||||
est.existing_peer_epoch = Some([1u8; 8]);
|
||||
let wire = wire_outcome(peer, Some([2u8; 8]), 0x88);
|
||||
let restart = m.step(
|
||||
PeerEvent::InboundMsg1 {
|
||||
link: LinkId::new(1),
|
||||
wire,
|
||||
est,
|
||||
},
|
||||
1_000,
|
||||
&mut alloc,
|
||||
);
|
||||
assert!(
|
||||
!restart
|
||||
.iter()
|
||||
.any(|a| matches!(a, PeerAction::UnregisterDecryptSession { .. })),
|
||||
"no UnregisterDecryptSession when the promoted outbound machine's our_index is None"
|
||||
);
|
||||
assert!(
|
||||
restart.iter().any(|a| matches!(
|
||||
a,
|
||||
PeerAction::ReportLost {
|
||||
kind: LostKind::LinkDead,
|
||||
..
|
||||
}
|
||||
)),
|
||||
"restart still reports the loss via the link-dead reconnect reflex"
|
||||
);
|
||||
}
|
||||
|
||||
// ---- Test 8: liveness -> LinkDeadSuspected -> ReportLost --------------
|
||||
#[test]
|
||||
fn liveness_to_link_dead() {
|
||||
|
||||
Reference in New Issue
Block a user