From f396d71826a706e149b4cd859b8e2a0bb483e958 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 24 May 2026 16:55:46 +0000 Subject: [PATCH] node: deterministic tie-breaker for cross-init NAT traversal adoption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When both peers' Nostr-mediated UDP punches complete within the same scheduling window, each side's `BootstrapEvent::Established` event arrives with `is_connecting_to_peer` already true: each side received an inbound msg1 from the peer's pre-punch outbound attempt, which created a connecting-state record. The deduplication skip then fires on both sides, neither installs the fresh traversal socket as canonical, and the peer-adoption budget (45 s) expires. Cross-node wall-clock alignment of the skip log line in observed failures was within ~1 ms — simultaneous dual- fire under contention, the dual-initiation pattern. Apply the deterministic NodeAddr tie-breaker already used at `handlers/handshake.rs:269` for rekey dual-initiation and in `peer::cross_connection_winner` for cross-connection resolution. Smaller NodeAddr wins as adopter: enumerate the in-flight connections whose `expected_identity` points at this peer, tear them down via the canonical `cleanup_stale_connection` helper, and fall through to `adopt_established_traversal`. Larger NodeAddr loses and keeps the existing `continue` semantics; the loser's in-flight outbound is reconciled by `handle_msg1`'s cross- connection logic when the winner's fresh msg1 arrives over the adopted socket. `cleanup_stale_connection` visibility bumped from module-private to `pub(in crate::node)` so it is callable from `lifecycle.rs`. The defensive re-check inside `adopt_established_traversal` itself is left as-is — after the outer cleanup the winner reaches it with `is_connecting_to_peer == false`, so the inner skip won't trip. The `BootstrapEvent::Failed` arm is unchanged: there is no winning outcome on dual failure, and the existing skip + retry-schedule semantics are correct. --- CHANGELOG.md | 19 ++++++++++++++++++ src/node/handlers/timeout.rs | 2 +- src/node/lifecycle.rs | 37 ++++++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a4ef43..8c52ac1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- NAT-traversal cross-init adoption is now deterministic under + simultaneous dual-initiation. Previously, when two peers' + Nostr-mediated UDP punches completed within the same scheduling + window, each side's bootstrap-completion event arrived with an + in-flight handshake already recorded against the other peer (each + side had received an inbound msg1 from the other's pre-punch + outbound attempt). The deduplication skip then fired on both + sides, neither installed the fresh traversal socket as canonical, + and the 45-second peer-adoption budget expired with both nodes + stuck waiting for an adoption that never happened. The handler now + applies the same deterministic NodeAddr tie-breaker the codebase + already uses for rekey dual-initiation and cross-connection + resolution: the smaller NodeAddr wins as adopter, tears down its + in-flight handshake state, and proceeds with adoption; the larger + NodeAddr keeps the skip semantics, and its in-flight outbound is + reconciled by the cross-connection logic when the winner's fresh + msg1 arrives over the adopted socket. The dual cross-init stall is + eliminated; cross-init NAT-traversal completes in well under a + second even under host CPU contention. - FSP session rekey is now hitless under packet loss and reordering. Previously, a rekey could leave the two endpoints holding different key sets for a brief window — if a handshake message was lost in diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index a026d1b..e87a3db 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -62,7 +62,7 @@ impl Node { /// Frees the session index, removes pending_outbound entry, and cleans up /// the link and address mapping. Does not log — callers provide context-appropriate /// log messages. - fn cleanup_stale_connection(&mut self, link_id: LinkId, _now_ms: u64) { + pub(in crate::node) fn cleanup_stale_connection(&mut self, link_id: LinkId, _now_ms: u64) { let conn = match self.connections.remove(&link_id) { Some(c) => c, None => return, diff --git a/src/node/lifecycle.rs b/src/node/lifecycle.rs index a8ee430..97a9419 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -423,11 +423,44 @@ impl Node { continue; } if self.is_connecting_to_peer(&peer_addr) { + // Dual cross-init: both nodes' Nostr-mediated punches + // completed simultaneously, and each side already + // holds in-flight handshake state for the other. + // Apply the deterministic NodeAddr tie-breaker — + // smaller NodeAddr wins as adopter (same convention + // as cross_connection_winner and the rekey dual- + // init resolution at handshake.rs:269). The winner + // tears down its in-flight state and adopts the + // fresh traversal socket; the loser keeps continue + // semantics, and its existing cross-connection + // logic in handle_msg1 reconciles when the winner's + // fresh msg1 arrives over the adopted socket. + let our_addr = self.identity.node_addr(); + if our_addr >= &peer_addr { + debug!( + peer_npub = %peer_npub, + "Dual cross-init NAT traversal: we lose (larger addr), keeping in-flight handshake" + ); + continue; + } debug!( peer_npub = %peer_npub, - "Ignoring established NAT traversal while peer handshake is already in progress" + "Dual cross-init NAT traversal: we win (smaller addr), tearing down in-flight handshake to adopt fresh socket" ); - continue; + let now_ms = Self::now_ms(); + let stale: Vec = self + .connections + .iter() + .filter(|(_, conn)| { + conn.expected_identity() + .map(|id| id.node_addr() == &peer_addr) + .unwrap_or(false) + }) + .map(|(link_id, _)| *link_id) + .collect(); + for link_id in stale { + self.cleanup_stale_connection(link_id, now_ms); + } } } match self.adopt_established_traversal(traversal).await {