node: deterministic tie-breaker for cross-init NAT traversal adoption

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.
This commit is contained in:
Johnathan Corgan
2026-05-24 18:07:57 +00:00
parent 6e5cb8965f
commit f396d71826
3 changed files with 55 additions and 3 deletions
+19
View File
@@ -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
+1 -1
View File
@@ -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,
+35 -2
View File
@@ -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<LinkId> = 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 {