diff --git a/src/nostr/runtime.rs b/src/nostr/runtime.rs index 355b6f8..11b1800 100644 --- a/src/nostr/runtime.rs +++ b/src/nostr/runtime.rs @@ -1223,6 +1223,16 @@ impl NostrRendezvous { } } + // Resolve the answer's relays before binding a socket and running STUN. + // Nothing in the relay choice depends on what STUN observes, and an offer + // from a peer we share no relay with cannot be answered at all — doing it + // in this order spends a STUN round trip, and holds an offer slot for its + // duration, only to discard the result. + let relays = self.preferred_signal_relays(sender, None).await?; + if relays.is_empty() { + return Err(BootstrapError::MissingRelays(offer.sender_npub.clone())); + } + let base_socket = std::net::UdpSocket::bind(("0.0.0.0", 0))?; base_socket.set_nonblocking(true)?; let (reflexive_address, local_addresses, stun_server) = observe_traversal_addresses( @@ -1257,7 +1267,6 @@ impl NostrRendezvous { (!accepted).then_some("no-usable-addresses".to_string()), Some(offer_received_at), ); - let relays = self.preferred_signal_relays(sender, None).await?; let answer_event = self.send_signal(&relays, sender, &answer).await?; debug!( peer = %peer_short, @@ -1384,22 +1393,21 @@ impl NostrRendezvous { target_pubkey: PublicKey, advert: Option<&OverlayAdvert>, ) -> Result, BootstrapError> { - let mut merged = self.find_recipient_inbox_relays(target_pubkey).await?; - if let Some(advert) = advert - && let Some(relays) = advert.signal_relays.as_ref() - { - for relay in relays { - if !merged.contains(relay) { - merged.push(relay.clone()); - } - } - } - for relay in &self.config.dm_relays { - if !merged.contains(relay) { - merged.push(relay.clone()); - } - } - Ok(merged) + let inbox = self.find_recipient_inbox_relays(target_pubkey).await?; + let pool: HashSet = self.client.pool().all_relays().await.into_keys().collect(); + let usable = signal_relays( + &inbox, + advert.and_then(|advert| advert.signal_relays.as_deref()), + &self.config.dm_relays, + &pool, + ); + debug!( + peer = %target_pubkey.to_bech32().map(|npub| short_npub(&npub)).unwrap_or_default(), + inbox = inbox.len(), + usable = usable.len(), + "traversal: signal relays resolved against the client pool" + ); + Ok(usable) } async fn find_recipient_inbox_relays( @@ -1599,6 +1607,56 @@ impl NostrRendezvous { } } +/// Retain only the candidates the client pool actually holds. +/// +/// `send_event_to` rejects the whole send with `RelayNotFound` if any single URL +/// is outside the pool, so a signal addressed to a peer's advertised relays fails +/// entirely on one relay we are not configured with. Filtering first turns that +/// into a send to the relays we share. +/// +/// Comparison is on the normalized `RelayUrl` rather than the raw string, because +/// the pool is keyed that way: a candidate spelled `wss://relay.example/` matches +/// a configured `wss://relay.example`. Order is preserved, candidates that fail +/// to parse are dropped, and duplicates that normalize alike are collapsed. +fn retain_pooled_relays(candidates: &[String], pool: &HashSet) -> Vec { + let mut seen: HashSet = HashSet::new(); + let mut usable = Vec::with_capacity(candidates.len()); + for candidate in candidates { + let Ok(url) = RelayUrl::parse(candidate) else { + continue; + }; + if pool.contains(&url) && seen.insert(url.clone()) { + usable.push(url.to_string()); + } + } + usable +} + +/// Choose the relays a traversal signal for one peer should be sent to. +/// +/// The candidates are the peer's NIP-17 inbox relays, then the relays its advert +/// nominates for signaling, then our own DM relays — remote-supplied first, ours +/// last, so a peer's preference is honored where we can act on it. The result is +/// whatever survives [`retain_pooled_relays`]. +/// +/// This is the whole decision, kept synchronous so it can be exercised without a +/// relay client: the caller's only job is to supply the fetched inbox list and +/// the pool. +pub(super) fn signal_relays( + inbox: &[String], + advert_signal: Option<&[String]>, + dm_relays: &[String], + pool: &HashSet, +) -> Vec { + let mut merged: Vec = inbox.to_vec(); + for relay in advert_signal.unwrap_or_default().iter().chain(dm_relays) { + if !merged.contains(relay) { + merged.push(relay.clone()); + } + } + retain_pooled_relays(&merged, pool) +} + #[cfg(test)] impl NostrRendezvous { /// Build a minimal `NostrRendezvous` for unit tests. No relay client is diff --git a/src/nostr/tests.rs b/src/nostr/tests.rs index 82609c5..29b4cab 100644 --- a/src/nostr/tests.rs +++ b/src/nostr/tests.rs @@ -1,6 +1,8 @@ -use nostr::prelude::{EventBuilder, Kind, Tag, Timestamp}; +use std::collections::HashSet; -use super::runtime::NostrRendezvous; +use nostr::prelude::{EventBuilder, Kind, RelayUrl, Tag, Timestamp}; + +use super::runtime::{NostrRendezvous, signal_relays}; use super::signal::{ FreshnessOutcome, build_signal_event, create_traversal_answer, create_traversal_offer, estimate_clock_skew, validate_offer_freshness, validate_traversal_answer_for_offer, @@ -708,3 +710,150 @@ fn now_ms_tracks_the_wall_clock() { "now_ms() is ahead of the wall clock: {sampled} > {after}" ); } + +fn pool(urls: &[&str]) -> HashSet { + urls.iter() + .map(|url| RelayUrl::parse(url).expect("test pool url parses")) + .collect() +} + +fn candidates(urls: &[&str]) -> Vec { + urls.iter().map(|url| url.to_string()).collect() +} + +#[test] +fn out_of_pool_relay_does_not_suppress_the_shared_ones() { + let usable = signal_relays( + &candidates(&[ + "wss://relay.damus.io", + "wss://temp.iris.to", + "wss://nos.lol", + ]), + None, + &[], + &pool(&[ + "wss://relay.damus.io", + "wss://nos.lol", + "wss://offchain.pub", + ]), + ); + assert_eq!( + usable, + vec![ + "wss://relay.damus.io".to_string(), + "wss://nos.lol".to_string() + ], + "the unknown relay must be dropped without taking the shared ones with it" + ); +} + +#[test] +fn trailing_slash_and_host_case_variants_are_retained() { + let usable = signal_relays( + &candidates(&["wss://Relay.Damus.io/", "wss://nos.lol"]), + None, + &[], + &pool(&["wss://relay.damus.io", "wss://nos.lol"]), + ); + assert_eq!( + usable.len(), + 2, + "normalized spellings of a configured relay are the same relay: {usable:?}" + ); +} + +#[test] +fn duplicates_that_normalize_alike_are_collapsed() { + let usable = signal_relays( + &candidates(&["wss://nos.lol", "wss://nos.lol/", "wss://NOS.LOL"]), + None, + &[], + &pool(&["wss://nos.lol"]), + ); + assert_eq!(usable, vec!["wss://nos.lol".to_string()]); +} + +#[test] +fn unparseable_candidates_are_dropped_rather_than_failing_the_set() { + let usable = signal_relays( + &candidates(&["not a url", "wss://nos.lol"]), + None, + &[], + &pool(&["wss://nos.lol"]), + ); + assert_eq!(usable, vec!["wss://nos.lol".to_string()]); +} + +#[test] +fn no_shared_relay_yields_an_empty_set_for_the_caller_to_reject() { + let usable = signal_relays( + &candidates(&["wss://temp.iris.to"]), + None, + &[], + &pool(&["wss://nos.lol"]), + ); + assert!( + usable.is_empty(), + "with no overlap the caller must see nothing to send to, not a doomed send" + ); +} + +#[test] +fn signal_relays_merges_all_three_sources_then_filters() { + let usable = signal_relays( + &candidates(&["wss://temp.iris.to", "wss://nos.lol"]), + Some(&candidates(&[ + "wss://relay.damus.io", + "wss://unknown.example", + ])), + &candidates(&["wss://offchain.pub"]), + &pool(&[ + "wss://nos.lol", + "wss://relay.damus.io", + "wss://offchain.pub", + ]), + ); + assert_eq!( + usable, + vec![ + "wss://nos.lol".to_string(), + "wss://relay.damus.io".to_string(), + "wss://offchain.pub".to_string(), + ], + "every source must contribute, and only the out-of-pool entries drop out" + ); +} + +#[test] +fn signal_relays_keeps_our_dm_relays_when_the_peer_shares_nothing() { + let usable = signal_relays( + &candidates(&["wss://temp.iris.to"]), + Some(&candidates(&["wss://also.unknown"])), + &candidates(&["wss://nos.lol"]), + &pool(&["wss://nos.lol"]), + ); + assert_eq!( + usable, + vec!["wss://nos.lol".to_string()], + "our own DM relays are always in the pool, so the result is never empty \ + while any are configured" + ); +} + +#[test] +fn signal_relays_without_an_advert_still_resolves() { + let usable = signal_relays( + &candidates(&["wss://nos.lol", "wss://temp.iris.to"]), + None, + &candidates(&["wss://offchain.pub"]), + &pool(&["wss://nos.lol", "wss://offchain.pub"]), + ); + assert_eq!( + usable, + vec![ + "wss://nos.lol".to_string(), + "wss://offchain.pub".to_string() + ], + "the responder path passes no advert and must still produce a target set" + ); +}