mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
discovery/nostr: elect a single traversal session for dual-auto_connect peers
When two peers each auto_connect to the other, each runs both an initiator and a responder NAT-traversal session and binds a separate UDP socket per session. Each side adopts only the first Established event and drops the loser session's socket; when the two sides adopt mismatched sessions, each sends its Noise msg1 to a peer port the peer has already stopped draining, and both handshakes stall. Deterministically keep the session initiated by the smaller NodeAddr, decided on the responder path: decline an incoming offer only when we also have an in-flight outbound initiator for the same peer and our NodeAddr is smaller. The peer's redundant initiator then times out, leaving a single matching socket pair on both ends. Asymmetric (one-sided) auto_connect has no co-active initiator and is never suppressed, so connectivity is preserved; an undecidable NodeAddr falls through to answering. Reuses the NodeAddr tie-breaker convention already used by the cross-connection and rekey dual-init paths. Adds a unit test for the election helper.
This commit is contained in:
@@ -32,6 +32,7 @@ use super::types::{
|
||||
};
|
||||
use crate::config::{NostrDiscoveryConfig, PeerConfig};
|
||||
use crate::discovery::EstablishedTraversal;
|
||||
use crate::{NodeAddr, PeerIdentity};
|
||||
|
||||
const ADVERT_CACHE_STALE_GRACE_MULTIPLIER: u64 = 2;
|
||||
|
||||
@@ -50,6 +51,42 @@ fn short_id(id: &str) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// Decide whether an incoming-offer responder session should be suppressed
|
||||
/// in favour of our own already-running outbound initiator session.
|
||||
///
|
||||
/// Two peers that each have the other as `auto_connect` simultaneously run an
|
||||
/// initiator traversal *and* a responder traversal for the same peer, binding a
|
||||
/// separate UDP socket per session. Each node then emits two
|
||||
/// `BootstrapEvent::Established` events and `adopt_established_traversal` keeps
|
||||
/// only the first on a non-deterministic race; when the two nodes' independent
|
||||
/// races resolve to mismatched sessions, each side's Noise msg1 lands on a peer
|
||||
/// port the peer already stopped draining and both handshakes stall (root cause
|
||||
/// of ISSUE-2026-0031).
|
||||
///
|
||||
/// To collapse the four-socket dance to a single, guaranteed-matching socket
|
||||
/// pair, both nodes deterministically keep the session **initiated by the
|
||||
/// smaller `NodeAddr`** — reusing the project's existing NodeAddr tie-breaker
|
||||
/// convention (`cross_connection_winner`, the rekey dual-init resolution, and
|
||||
/// the dual-cross-init adopt path in `lifecycle.rs`).
|
||||
///
|
||||
/// This is evaluated on the responder path, where the session being handled is
|
||||
/// *peer-initiated*. It returns `true` (suppress this responder session) only
|
||||
/// when genuine duplication exists — i.e. we also have an in-flight outbound
|
||||
/// initiator for this same peer (`have_active_initiator`) — and our own
|
||||
/// initiator session is the preferred one (`our_addr < peer_addr`). When there
|
||||
/// is no co-active initiator (the asymmetric / one-sided `auto_connect` case,
|
||||
/// where only one session exists at all) it never suppresses, so connectivity
|
||||
/// is preserved. The `our_addr == peer_addr` case (self / loopback) and any
|
||||
/// caller that cannot derive a peer `NodeAddr` likewise fall through to "do not
|
||||
/// suppress".
|
||||
pub(super) fn suppress_responder_for_own_initiator(
|
||||
our_addr: &NodeAddr,
|
||||
peer_addr: &NodeAddr,
|
||||
have_active_initiator: bool,
|
||||
) -> bool {
|
||||
have_active_initiator && our_addr < peer_addr
|
||||
}
|
||||
|
||||
fn endpoint_summary(endpoints: &[OverlayEndpointAdvert]) -> String {
|
||||
endpoints
|
||||
.iter()
|
||||
@@ -1131,6 +1168,45 @@ impl NostrDiscovery {
|
||||
"traversal: offer accepted within clock-skew tolerance"
|
||||
);
|
||||
}
|
||||
// Collapse the dual-`auto_connect` four-socket dance to a single
|
||||
// session. When we also have an in-flight outbound initiator for this
|
||||
// same peer (genuine symmetric duplication), both nodes deterministically
|
||||
// keep the session initiated by the smaller NodeAddr. If our own
|
||||
// initiator session is the preferred one, decline to answer this offer:
|
||||
// not answering lets the peer's redundant initiator time out, so only the
|
||||
// single matching socket pair survives on both sides. Asymmetric /
|
||||
// one-sided `auto_connect` (no co-active initiator) is never suppressed,
|
||||
// preserving connectivity. See `suppress_responder_for_own_initiator`.
|
||||
if self.active_initiators.lock().await.contains(&sender_npub) {
|
||||
match (
|
||||
PeerIdentity::from_npub(&self.npub),
|
||||
PeerIdentity::from_npub(&sender_npub),
|
||||
) {
|
||||
(Ok(ours), Ok(theirs)) => {
|
||||
if suppress_responder_for_own_initiator(
|
||||
ours.node_addr(),
|
||||
theirs.node_addr(),
|
||||
true,
|
||||
) {
|
||||
debug!(
|
||||
peer = %peer_short,
|
||||
session = %short_id(&offer.session_id),
|
||||
"traversal: responder session suppressed, our outbound initiator wins (smaller addr)"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
// Could not derive a NodeAddr for one side; fall through and
|
||||
// answer rather than risk suppressing the only session.
|
||||
trace!(
|
||||
peer = %peer_short,
|
||||
"traversal: could not derive NodeAddr for dedup, answering offer"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self.mark_session_seen(&offer.session_id).await?;
|
||||
|
||||
let base_socket = std::net::UdpSocket::bind(("0.0.0.0", 0))?;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use nostr::prelude::{EventBuilder, Kind, Tag, Timestamp};
|
||||
|
||||
use super::runtime::NostrDiscovery;
|
||||
use super::runtime::{NostrDiscovery, suppress_responder_for_own_initiator};
|
||||
use super::signal::{
|
||||
FreshnessOutcome, build_signal_event, create_traversal_answer, create_traversal_offer,
|
||||
estimate_clock_skew, validate_offer_freshness, validate_traversal_answer_for_offer,
|
||||
@@ -14,6 +14,7 @@ use super::{
|
||||
ADVERT_IDENTIFIER, ADVERT_KIND, ADVERT_VERSION, OverlayAdvert, OverlayEndpointAdvert,
|
||||
OverlayTransportKind, PunchHint, PunchPacketKind, TraversalAddress,
|
||||
};
|
||||
use crate::NodeAddr;
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
enum NatType {
|
||||
@@ -628,3 +629,44 @@ async fn signal_events_use_current_timestamps() {
|
||||
assert!(created_at >= before);
|
||||
assert!(created_at <= after);
|
||||
}
|
||||
|
||||
fn node_addr(first_byte: u8) -> NodeAddr {
|
||||
let mut bytes = [0u8; 16];
|
||||
bytes[0] = first_byte;
|
||||
NodeAddr::from_bytes(bytes)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn responder_suppression_election() {
|
||||
let smaller = node_addr(0x01);
|
||||
let larger = node_addr(0x02);
|
||||
|
||||
// Symmetric dual-auto_connect (co-active initiator on both sides):
|
||||
// both nodes must keep the session initiated by the smaller NodeAddr.
|
||||
|
||||
// Smaller-addr node handling the larger node's offer: our own outbound
|
||||
// initiator (smaller) is preferred, so suppress this responder session.
|
||||
assert!(suppress_responder_for_own_initiator(
|
||||
&smaller, &larger, true
|
||||
));
|
||||
|
||||
// Larger-addr node handling the smaller node's offer: the smaller node's
|
||||
// session is preferred, so do NOT suppress — answer it.
|
||||
assert!(!suppress_responder_for_own_initiator(
|
||||
&larger, &smaller, true
|
||||
));
|
||||
|
||||
// Asymmetric / one-sided auto_connect: no co-active initiator means only
|
||||
// one session exists; never suppress, regardless of address ordering.
|
||||
assert!(!suppress_responder_for_own_initiator(
|
||||
&smaller, &larger, false
|
||||
));
|
||||
assert!(!suppress_responder_for_own_initiator(
|
||||
&larger, &smaller, false
|
||||
));
|
||||
|
||||
// Self / loopback (equal addresses): never suppress.
|
||||
assert!(!suppress_responder_for_own_initiator(
|
||||
&smaller, &smaller, true
|
||||
));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user