From 225140910546a04daf3ccae4b6651174ddccd44e Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Mon, 26 Oct 2026 00:44:22 +0530 Subject: [PATCH 1/2] fix(protocol): ask the relays for a stranger's key packages when a session is wanted A device that knew another only by its address, and shared only a Nostr relay with it, never formed a session on any native binding. The peer's key packages stood on the relay and nothing fetched them: a send with no session only queued, so no frame was built for the transport to attach a lookup to; discovery's first push never names a peer that has sent nothing; and the tick's mesh push stands down whenever Nostr claims to reach the peer. The engine now asks the relays where it finds no key package for a peer it must reach: a send, an explicit establish_secure_session, and the reconciliation tick while a message waits, which repeats a lookup no relay could answer. A blocked peer, a peer on a live mesh link, a peer with a session, and every peer with cold contact off are never looked up. The transport's per-peer interval is read before the session check, so a waiting peer costs one storage read per interval. Closes #633 --- CHANGELOG.md | 12 + .../offline-protocol-transport/src/nostr.rs | 85 ++++- crates/offline-protocol/src/config.rs | 6 +- crates/offline-protocol/src/protocol/mod.rs | 9 +- .../src/protocol/nostr_publication.rs | 66 +++- crates/offline-protocol/src/protocol/send.rs | 8 +- .../offline-protocol/src/protocol/session.rs | 14 +- .../src/protocol/tests/mod.rs | 161 +++++++++ .../src/protocol/tests/nostr_cold_contact.rs | 315 ++++++++++++++++++ .../offline-protocol/src/transport_manager.rs | 14 + docs/bridges/web.md | 23 +- docs/nostr.md | 6 +- docs/state-machines/session-lifecycle.md | 32 ++ 13 files changed, 712 insertions(+), 39 deletions(-) create mode 100644 crates/offline-protocol/src/protocol/tests/nostr_cold_contact.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6760e8e6..95d47a55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -226,6 +226,18 @@ archived by series under [docs/changelog/](docs/changelog/); see the ### Fixed +- **A device known only by its address is reached over Nostr on every + binding.** Cold first contact never started outside the browser: a send to + a peer with no session only queued, and nothing asked the relays for the + key packages that peer published, so two phones sharing only a relay never + formed a session. The engine now asks when a send or + `establish_secure_session` finds no key package for the peer, and the + tick asks again every 5 minutes while a message waits. A peer on a live + Bluetooth or Wi-Fi link, a blocked peer, and every peer with cold contact + off are never looked up. The browser's own lookup is gone; the engine's + replaces it + ([#633](https://github.com/Offline-Protocol/offline-protocol-sdk/issues/633)). + - **An iPhone names a central that cannot be dialled back.** The iOS peripheral now serves the Hello characteristic, as Android and the Python service do: a central writes its identity assertion once, and the iPhone diff --git a/crates/offline-protocol-transport/src/nostr.rs b/crates/offline-protocol-transport/src/nostr.rs index c3d8e864..8eda2726 100644 --- a/crates/offline-protocol-transport/src/nostr.rs +++ b/crates/offline-protocol-transport/src/nostr.rs @@ -932,24 +932,31 @@ impl NostrTransport { queued } - /// Queues a resolution without waking the platform. + /// Whether a request for `user_id`'s published key packages would be + /// queued now, without queueing one. /// - /// Used from the send path, which is already running inside the platform's - /// poll loop: waking it from there would re-enter the transport partway - /// through building an event, to tell it something it is about to look for - /// anyway. - fn enqueue_resolution(&self, user_id: &str) -> bool { + /// The engine asks this before it reads storage to decide whether it + /// wants the packages at all, so that a peer with a message waiting costs + /// one storage read per `RESOLUTION_RETRY_INTERVAL` rather than one per + /// tick. It is advisory: the answer can change before + /// [`Self::request_peer_key_packages`] runs, which re-checks everything. + pub fn resolution_due(&self, user_id: &str) -> bool { + self.admissible_resolution(user_id).is_some() + } + + /// The checks a resolution request must pass, read without changing + /// anything: cold contact on, the id an address, the rate limit lapsed, + /// and room in the queue for an id not already in it. + fn admissible_resolution(&self, user_id: &str) -> Option
{ // A queued id becomes the `#p` tag of a relay query, so it is held to // the same address requirement as a send recipient — and held here, // at the queue's only gate, so `next_query` can never pop an id whose // tag derivation fails after the entry was already consumed. Subsumes // the emptiness check this replaces: an empty id is not an address. if !self.cold_contact_enabled() { - return false; + return None; } - let Ok(address) = user_id.parse::
() else { - return false; - }; + let address = user_id.parse::
().ok()?; // Read the rate limit without stamping it. Stamping here would burn the // whole interval on a request the queue then refuses, so a peer dropped @@ -960,12 +967,40 @@ impl NostrTransport { let attempts = self.resolve_attempts.lock_or_recover(); if let Some(last) = attempts.get(user_id) { if last.elapsed() < RESOLUTION_RETRY_INTERVAL { - return false; + return None; } } } + let queue = self.resolve_queue.lock_or_recover(); + if queue.iter().any(|q| *q == address) || queue.len() >= MAX_PENDING_RESOLUTIONS { + return None; + } + Some(address) + } + + /// Forgets every resolution attempt, as if `RESOLUTION_RETRY_INTERVAL` + /// had lapsed for every peer. For tests that drive a retry without + /// waiting out the interval. + #[cfg(any(test, feature = "test-utils"))] + pub fn forget_resolution_attempts_for_test(&self) { + self.resolve_attempts.lock_or_recover().clear(); + } + + /// Queues a resolution without waking the platform. + /// + /// Used from the send path, which is already running inside the platform's + /// poll loop: waking it from there would re-enter the transport partway + /// through building an event, to tell it something it is about to look for + /// anyway. + fn enqueue_resolution(&self, user_id: &str) -> bool { + let Some(address) = self.admissible_resolution(user_id) else { + return false; + }; + { + // Checked again under the lock that pushes, so two callers racing + // past the read above cannot queue one peer twice or overrun the cap. let mut queue = self.resolve_queue.lock_or_recover(); if queue.iter().any(|q| *q == address) { return false; @@ -3796,6 +3831,34 @@ mod tests { assert!(transport.request_peer_key_packages(&addr("carol"))); } + /// The probe says what a request would do and does none of it: the engine + /// asks it before a storage read, so a probe that queued or stamped would + /// spend the interval on a peer the engine then decides not to ask for. + #[test] + fn test_resolution_due_agrees_with_the_request_and_changes_nothing() { + let transport = NostrTransport::new(addr("alice")).unwrap(); + assert!(transport.resolution_due(&addr("bob"))); + assert!( + transport.resolution_due(&addr("bob")), + "asking twice changes nothing" + ); + assert!( + transport.next_query().unwrap().is_none(), + "the probe queued nothing" + ); + + assert!(transport.request_peer_key_packages(&addr("bob"))); + assert!( + !transport.resolution_due(&addr("bob")), + "queued, then rate-limited" + ); + assert!(transport.resolution_due(&addr("carol"))); + assert!(!transport.resolution_due("not-an-address")); + + transport.set_cold_contact_enabled(false); + assert!(!transport.resolution_due(&addr("carol"))); + } + #[test] fn test_query_event_opens_only_for_the_peer_it_was_issued_for() { let alice = NostrTransport::new(addr("alice")).unwrap(); diff --git a/crates/offline-protocol/src/config.rs b/crates/offline-protocol/src/config.rs index 0a4139ad..d4760c66 100644 --- a/crates/offline-protocol/src/config.rs +++ b/crates/offline-protocol/src/config.rs @@ -431,9 +431,11 @@ pub struct TransportConfig { /// which is itself off by default, so nothing is published unless an app /// deliberately turns Nostr on. /// - /// Buys **cold first contact**: a peer known only by username becomes + /// Buys **cold first contact**: a peer known only by address becomes /// reachable over Nostr with no prior key-package exchange over some other - /// transport, which is otherwise impossible. It also bounds the sealing + /// transport, which is otherwise impossible. A send to, or a session + /// asked for with, a peer this device holds no key package for asks the + /// relays for one that peer published. It also bounds the sealing /// downgrade that follows a peer-key eviction, since a forgotten key is /// re-resolved on the next send instead of waiting for a fresh exchange. /// diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index a02141ea..846692ef 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -3144,7 +3144,8 @@ impl OfflineProtocol { /// 1. Checks if a session already exists (returns Ok(None) if so) /// 2. Checks for a pending key package from the peer /// 3. If found, imports the key package, creates the session, and sends the Welcome message - /// 4. If no key package is available, returns an error + /// 4. If no key package is available, asks the Nostr relays for one the + /// peer published (with cold contact on) and returns `SessionNotReady` /// /// This method is designed for use by application code that needs explicit control /// over session establishment, as opposed to the automatic encryption flow. @@ -3247,7 +3248,11 @@ impl OfflineProtocol { } } - // No key package available (memory nor storage) — return non-terminal state so caller can retry + // No key package available (memory nor storage). Ask the relays for + // one the peer published, then return the non-terminal state so the + // caller can retry; the package that answers establishes the session + // on arrival. + self.request_nostr_key_package_for(peer_id); Err(Error::SessionNotReady(self.establishment_state(peer_id)?)) } diff --git a/crates/offline-protocol/src/protocol/nostr_publication.rs b/crates/offline-protocol/src/protocol/nostr_publication.rs index 8a26b6d0..31c5f463 100644 --- a/crates/offline-protocol/src/protocol/nostr_publication.rs +++ b/crates/offline-protocol/src/protocol/nostr_publication.rs @@ -1,12 +1,13 @@ //! Nostr key-package publication slots and peer key-package resolution. //! -//! Cold first contact over Nostr — reaching a peer known only by username, with -//! no prior key-package exchange over some other transport — needs the peer's -//! key package to be *fetchable* rather than pushed. This module owns the -//! publishing half of that: it keeps a small set of single-use MLS key packages -//! standing in addressable relay records, replaces each as it is consumed, and -//! feeds the answers to outbound resolution queries back into the ordinary -//! key-package handler. +//! Cold first contact over Nostr (reaching a peer known only by address, with +//! no prior key-package exchange over some other transport) needs the peer's +//! key package to be *fetchable* rather than pushed. This module owns both +//! halves of that: it keeps a small set of single-use MLS key packages +//! standing in addressable relay records, replaces each as it is consumed, +//! asks the relays for a peer's records when a session with that peer is +//! wanted and nothing else can produce one, and feeds the answers back into +//! the ordinary key-package handler. //! //! # Why slots, rather than one record //! @@ -439,6 +440,57 @@ impl OfflineProtocol { Ok(message) } + /// Asks the relays for `peer_id`'s published key packages when a session + /// with them is wanted and only the relays can produce one. Returns + /// whether a query was queued. + /// + /// Invariant: a session wanted with a peer this device holds no key + /// package for is enough to ask every carrier that can produce one. For a + /// peer known only by address and reachable only through a relay, the + /// relays are that carrier, and nothing else ever asked them: a send with + /// no session only queues, so no frame is built for the transport to + /// attach a lookup to; discovery's first push never names a peer that has + /// sent nothing; and the tick's mesh push stands down whenever Nostr + /// claims to reach the peer. The records were published and never + /// fetched, so cold first contact never started on any binding (issue + /// #633). + /// + /// The gates run cheapest first, and the storage read comes last: + /// + /// - **Blocked peers are never asked about.** A query names the peer's + /// routing tag to every relay, which is interest in them. + /// - **Cold contact must be on.** Off, nothing is published to fetch. + /// - **A live mesh link wins.** The link carries the key-package exchange + /// within a tick, and a query would only tell the relays which address + /// this device wants. + /// - **The transport's rate limit decides how often.** One query per peer + /// per interval, read before the session check so that a peer with a + /// message waiting costs one storage read per interval, not per tick. + /// - **A session already formed needs nothing.** + /// + /// Callers reach this only where they have found no key package for the + /// peer, so it does not look for one itself. + pub(crate) fn request_nostr_key_package_for(&mut self, peer_id: &str) -> bool { + if self.mls_manager.is_none() + || !self.config.encryption.enabled + || peer_id == self.local_id + || self.is_user_blocked(peer_id) + || !self.transport_manager.nostr_cold_contact_active() + || self.transport_manager.mesh_can_address(peer_id) + || !self.transport_manager.nostr_resolution_due(peer_id) + { + return false; + } + if !matches!(self.has_mls_session(peer_id), Ok(false)) { + return false; + } + let queued = self.transport_manager.request_nostr_key_packages(peer_id); + if queued { + debug!(peer_id = %peer_id, "No key package for a wanted session, asking the Nostr relays"); + } + queued + } + /// Handles a key-package record returned by a resolution query. /// /// Deliberately narrow: this channel answers exactly one question, so diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index 362eec88..bc8c6675 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -76,7 +76,8 @@ impl OfflineProtocol { /// encrypted before sending. If no session exists with the recipient but we have /// their key package, a session is created automatically. If no key package is /// available and `store_pending` is enabled, the message is queued until a key - /// package is received. + /// package is received; with Nostr cold contact on, the relays are asked for + /// one the recipient published. pub fn send_message( &mut self, recipient: impl Into, @@ -1114,7 +1115,10 @@ impl OfflineProtocol { } } } else { - // No key package available (memory nor storage) + // No key package available (memory nor storage). A peer + // reachable only through a relay produces none unprompted, so + // ask the relays for one it published. + self.request_nostr_key_package_for(recipient); self.emit_mls_session_missing( Some(recipient), None, diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index a8ed4ecc..93234c33 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -1792,7 +1792,9 @@ impl OfflineProtocol { /// /// Only a peer already pushed to qualifies; the first push belongs to /// discovery, except for a peer only the mesh reaches, which discovery - /// never names (see [`Self::push_key_package_across_mesh`]). The stamp is taken before the send, so a send that fails + /// never names (see [`Self::push_key_package_across_mesh`]), and a peer + /// only a relay reaches, whose published package is fetched instead (see + /// [`Self::request_nostr_key_package_for`]). The stamp is taken before the send, so a send that fails /// still waits out the interval instead of being retried on every tick. /// A send that fails does not count as a repeat, though: the backoff is /// for a peer that was asked and did not answer, and a peer whose frame @@ -1814,6 +1816,13 @@ impl OfflineProtocol { } let Some((sent_at, repeats)) = self.key_package_sent_to.get_mut(peer_id) else { self.push_key_package_across_mesh(peer_id); + // A peer only a relay reaches is asked about instead, on the + // transport's own interval: a query no relay answered (none held + // a record yet, or none was live) is not asked again by anything + // else while the message waits. + if !self.pending_key_packages.contains_key(peer_id) { + self.request_nostr_key_package_for(peer_id); + } return; }; if sent_at.elapsed() < StdDuration::from_secs(key_package_resend_wait_secs(*repeats)) { @@ -1864,7 +1873,8 @@ impl OfflineProtocol { /// addresses the application asked to reach, never to whoever is heard. /// - **Only when nothing else reaches the peer.** A direct link means /// discovery has already pushed, and an infrastructure carrier has its - /// own exchange; both are left alone. + /// own exchange (for Nostr, fetching the peer's published package; see + /// [`Self::request_nostr_key_package_for`]); both are left alone. /// - **Only once by this route.** A push that a neighbour took is recorded /// like any other, so every repeat after it is the re-push ladder's, on /// its doubling wait. A push nobody took is tried again on the next diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index c9c81f0c..59c3fecc 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -10,6 +10,7 @@ mod ephemeral; mod file_stores; mod leaf_pairing; mod mesh_session; +mod nostr_cold_contact; mod per_send_app_id; mod service_seal; @@ -37988,6 +37989,166 @@ fn nostr_resolution_drops_anything_that_is_not_a_key_package() { ); } +// ============================================================================ +// COLD CONTACT: THE ENGINE ASKS THE RELAYS (issue #633) +// ============================================================================ + +/// A started engine with Nostr, and nothing else, to reach peers by. +#[cfg(test)] +fn started_with_nostr(user_id: &str) -> OfflineProtocol { + let mut protocol = protocol_with_nostr(user_id); + protocol.start().unwrap(); + protocol +} + +/// Pops the next resolution query, returning the routing tag it asks about. +#[cfg(test)] +fn next_resolution_tag(protocol: &OfflineProtocol) -> Option { + let handle = nostr_transport_of(protocol); + let query = as_nostr(&handle).next_query().unwrap()?; + let req: serde_json::Value = serde_json::from_str(&query.req_json).unwrap(); + req[2]["#p"][0].as_str().map(str::to_string) +} + +#[cfg(test)] +fn routing_tag_of(label: &str) -> String { + let address = id(label).parse::().unwrap(); + offline_protocol_transport::nostr_crypto::routing_tag_for_address(&address).unwrap() +} + +/// A send to a peer this device holds no key package for asks the relays for +/// the one it published. Before this nothing did: the send only queued, no +/// frame was built for the transport to attach a lookup to, and cold contact +/// never started on any binding. +#[test] +fn a_send_with_no_key_package_asks_the_relays_for_one() { + let mut alice = started_with_nostr("alice"); + alice + .send_message(id("bob"), "hello", None, None::) + .unwrap(); + assert_eq!( + next_resolution_tag(&alice), + Some(routing_tag_of("bob")), + "the send queued no lookup of bob's published key packages" + ); + assert_eq!( + next_resolution_tag(&alice), + None, + "one lookup, not one per path" + ); +} + +#[test] +fn establishing_with_no_key_package_asks_the_relays_for_one() { + let mut alice = started_with_nostr("alice"); + assert!(matches!( + alice.establish_secure_session(&id("bob")), + Err(Error::SessionNotReady(EstablishmentState::NoKeyPackage)) + )); + assert_eq!(next_resolution_tag(&alice), Some(routing_tag_of("bob"))); +} + +/// One query per peer per interval, however many sends wait: a peer who has +/// published nothing must not cost a round trip per message. +#[test] +fn repeated_sends_ask_once_per_interval() { + let mut alice = started_with_nostr("alice"); + for _ in 0..3 { + alice + .send_message(id("bob"), "hello", None, None::) + .unwrap(); + } + alice.last_reconciliation_at = None; + alice.run_throttled_reconciliation("test"); + assert!(next_resolution_tag(&alice).is_some()); + assert_eq!(next_resolution_tag(&alice), None); +} + +/// A query names the peer's routing tag to every relay. Nothing is worth +/// that for a blocked peer, and there is nothing to fetch with cold contact +/// off. +#[test] +fn no_lookup_for_a_blocked_peer_or_with_cold_contact_off() { + let mut alice = started_with_nostr("alice"); + alice.block_user(&id("bob")).unwrap(); + let _ = alice.establish_secure_session(&id("bob")); + assert_eq!( + next_resolution_tag(&alice), + None, + "a blocked peer was looked up" + ); + + let mut config = create_test_config(); + config.profile = "carol".to_string(); + config.encryption.enabled = true; + config.transport.nostr_enabled = true; + config.transport.nostr_cold_contact_enabled = false; + let mut carol = OfflineProtocol::new(config).unwrap(); + carol + .initialize_mls_for_test(Arc::new(InMemoryStorage::new())) + .unwrap(); + let nostr = NostrTransport::new(local_id(&carol)).unwrap(); + carol + .transport_manager_mut() + .add_transport(TransportType::Nostr, Box::new(nostr)); + carol.start().unwrap(); + carol + .send_message(id("bob"), "hello", None, None::) + .unwrap(); + assert_eq!( + next_resolution_tag(&carol), + None, + "cold contact off still looked up" + ); +} + +/// A peer on a live mesh link forms its session over the link within a tick; +/// a query would only tell the relays which address this device wants. +#[test] +fn a_peer_on_a_live_link_is_not_looked_up_on_the_relays() { + let mut alice = started_with_nostr("alice"); + let link = MockTransport::new(TransportType::WiFiDirect); + link.set_status(TransportStatus::Available); + link.add_connected_peer(id("bob"), -40); + alice + .transport_manager_mut() + .add_transport(TransportType::WiFiDirect, Box::new(link)); + let _ = alice.establish_secure_session(&id("bob")); + alice + .send_message(id("bob"), "hello", None, None::) + .unwrap(); + assert_eq!( + next_resolution_tag(&alice), + None, + "a linked peer was looked up" + ); + + // The control: a peer with no link is. + let _ = alice.establish_secure_session(&id("carol")); + assert_eq!(next_resolution_tag(&alice), Some(routing_tag_of("carol"))); +} + +/// A message waiting for a session keeps asking, on the transport's +/// interval: a lookup no relay could answer (none held a record yet, or none +/// was live) is otherwise never repeated while the message waits. +#[test] +fn the_tick_asks_again_while_a_message_waits() { + let mut alice = started_with_nostr("alice"); + alice + .send_message(id("bob"), "hello", None, None::) + .unwrap(); + assert!(next_resolution_tag(&alice).is_some()); + as_nostr(&nostr_transport_of(&alice)).forget_resolution_attempts_for_test(); + + alice.last_reconciliation_at = None; + alice.run_throttled_reconciliation("test"); + assert_eq!( + next_resolution_tag(&alice), + Some(routing_tag_of("bob")), + "the waiting message did not ask again once the interval lapsed" + ); +} + /// `start()` is the one place every caller — FFI, RN, and a pure-Rust embedder /// that installed its own transport — passes through with the config in hand, /// so it is the only thing that makes these switches real. Both had exactly one diff --git a/crates/offline-protocol/src/protocol/tests/nostr_cold_contact.rs b/crates/offline-protocol/src/protocol/tests/nostr_cold_contact.rs new file mode 100644 index 00000000..5059d8a6 --- /dev/null +++ b/crates/offline-protocol/src/protocol/tests/nostr_cold_contact.rs @@ -0,0 +1,315 @@ +//! Cold first contact over Nostr, between two devices that share nothing but +//! a relay (issue #633). +//! +//! alice holds bob's address and nothing else: no link, no prior key-package +//! exchange, no session. Encryption is required, so nothing she sends can +//! leave until a session exists, and the only way one can form is for her to +//! fetch the key package bob published. What is pinned is that the engine +//! asks for it by itself: the host half here does only what every binding's +//! Nostr driver does (move events, answer queries, report the relay live), +//! and asks for nothing. +//! +//! The relay is the smallest one that behaves like a real relay for what the +//! transport relies on: it stores what is not ephemeral, replaces an +//! addressable event by `(kind, pubkey, d)`, filters on `kinds`, `since`, +//! `limit` and any `#` tag, answers a query with what it holds, and forwards +//! a new event to every live subscription it matches. + +use std::sync::{Arc, Mutex}; + +use base64::Engine as _; +use serde_json::Value; + +use offline_protocol_transport::nostr::{NostrTransport, ResolvedRecord}; +use offline_protocol_transport::{Transport, TransportStatus, TransportType}; + +use crate::mls::InMemoryStorage; +use crate::protocol::tests::create_test_config_for_user; +use crate::protocol::{OfflineProtocol, TestProtocolStateStorage}; +use crate::test_identity::id; +use crate::{Event, ProtocolConfig}; + +#[derive(Default)] +struct Relay { + stored: Vec, +} + +impl Relay { + fn publish(&mut self, event: &Value) { + let kind = event["kind"].as_u64().unwrap_or(0); + if (20_000..30_000).contains(&kind) { + return; + } + if (30_000..40_000).contains(&kind) { + let d = tag(event, "d"); + self.stored.retain(|held| { + !(held["kind"] == event["kind"] + && held["pubkey"] == event["pubkey"] + && tag(held, "d") == d) + }); + } + self.stored.push(event.clone()); + } + + fn matching(&self, filter: &Value) -> Vec { + let mut found: Vec = self + .stored + .iter() + .filter(|event| matches(filter, event)) + .cloned() + .collect(); + found.sort_by_key(|event| std::cmp::Reverse(event["created_at"].as_i64())); + if let Some(limit) = filter["limit"].as_u64() { + found.truncate(limit as usize); + } + found + } +} + +fn tag(event: &Value, name: &str) -> Option { + let found = event["tags"].as_array()?.iter().find(|t| t[0] == name)?; + found[1].as_str().map(str::to_string) +} + +fn matches(filter: &Value, event: &Value) -> bool { + if let Some(kinds) = filter["kinds"].as_array() { + if !kinds.contains(&event["kind"]) { + return false; + } + } + if let (Some(since), Some(at)) = (filter["since"].as_i64(), event["created_at"].as_i64()) { + if at < since { + return false; + } + } + for (key, wanted) in filter.as_object().into_iter().flatten() { + let Some(name) = key.strip_prefix('#') else { + continue; + }; + let held: Vec<&Value> = event["tags"] + .as_array() + .into_iter() + .flatten() + .filter(|t| t[0] == name) + .map(|t| &t[1]) + .collect(); + let wanted = wanted.as_array().cloned().unwrap_or_default(); + if !wanted.iter().any(|w| held.contains(&w)) { + return false; + } + } + true +} + +struct Node { + protocol: OfflineProtocol, + address: String, + events: Arc>>, + inbox: Vec, + /// The standing inbox subscription's filter. + subscription: Value, + /// Resolution queries this node issued. + queries: usize, +} + +fn strict_config(label: &str) -> ProtocolConfig { + let mut config = create_test_config_for_user(label); + config.encryption.enabled = true; + config.encryption.require_encryption = true; + config.encryption.auto_key_exchange = true; + config.encryption.store_pending = true; + config.transport.nostr_enabled = true; + config.transport.nostr_cold_contact_enabled = true; + config +} + +impl Node { + fn new(label: &str) -> Self { + let mut protocol = OfflineProtocol::new(strict_config(label)).expect("protocol"); + protocol + .initialize_mls( + crate::test_identity::seeded_storage(label), + Arc::new(TestProtocolStateStorage { + storage: Arc::new(InMemoryStorage::new()), + }), + ) + .expect("initialize_mls"); + // Built on the derived address, as every binding builds it. + let nostr = NostrTransport::new(id(label)).expect("nostr transport"); + protocol + .transport_manager_mut() + .add_transport(TransportType::Nostr, Box::new(nostr)); + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let sink = events.clone(); + protocol.on_event(move |event| sink.lock().unwrap().push(event)); + protocol.start().expect("start"); + let mut node = Self { + protocol, + address: id(label), + events, + inbox: Vec::new(), + subscription: Value::Null, + queries: 0, + }; + // What a driver does when its first relay comes up. + node.transport() + .on_status_changed(TransportStatus::Available); + node.protocol.flush_outbox_all(); + let req: Value = serde_json::from_str( + &node + .nostr(|nostr| nostr.create_subscription("inbox")) + .expect("subscription"), + ) + .expect("req json"); + node.subscription = req[2].clone(); + node + } + + fn transport(&self) -> Arc { + self.protocol + .transport_manager() + .get_transport(TransportType::Nostr) + .expect("nostr slot") + } + + fn nostr(&self, f: impl FnOnce(&NostrTransport) -> T) -> T { + let transport = self.transport(); + f(transport + .as_any() + .downcast_ref::() + .expect("nostr transport")) + } + + /// The receive path every binding runs on an inbox event: unseal, hand + /// the bytes to the transport, drain, then the reachability notice under + /// the protocol sender. + fn receive(&mut self, event: &Value) { + let sender = event["pubkey"].as_str().expect("pubkey"); + let content = event["content"].as_str().expect("content"); + let raw = base64::engine::general_purpose::STANDARD + .decode(content) + .unwrap_or_else(|_| content.as_bytes().to_vec()); + let data = self.nostr(|nostr| nostr.unseal_event_payload(sender, &raw).into_owned()); + let transport = self.transport(); + let real_sender = transport + .deserialize_message(&data) + .ok() + .map(|message| message.sender.as_str().to_string()); + transport.on_data_received(data).expect("on_data_received"); + self.drain(); + if let Some(peer) = real_sender { + self.protocol.on_neighbor_discovered(&peer); + } + } + + fn drain(&mut self) { + while let Some(message) = self.protocol.receive_message() { + self.inbox.push(message.content); + } + } + + fn delivered(&self, message_id: &str) -> bool { + self.events.lock().unwrap().iter().any( + |event| matches!(event, Event::MessageDelivered { message_id: id, .. } if id == message_id), + ) + } +} + +/// One round: each device publishes what it holds and issues its queries, +/// the relay answers and forwards, then every device runs its tick. +fn step(relay: &mut Relay, nodes: &mut [Node]) { + for i in 0..nodes.len() { + while let Some(signed) = nodes[i] + .nostr(|nostr| nostr.get_next_signed_event()) + .expect("sign") + { + let sent: Value = serde_json::from_str(&signed.event_json).expect("event json"); + let event = sent[1].clone(); + relay.publish(&event); + nodes[i] + .protocol + .on_transport_send_confirmed(&signed.message_id) + .expect("confirm"); + nodes[i].transport().confirm_sent(&signed.message_id); + for (j, other) in nodes.iter_mut().enumerate() { + if j != i && matches(&other.subscription, &event) { + other.receive(&event); + } + } + } + while let Some(query) = nodes[i].nostr(|nostr| nostr.next_query()).expect("query") { + nodes[i].queries += 1; + let req: Value = serde_json::from_str(&query.req_json).expect("req json"); + for event in relay.matching(&req[2]) { + let opened = nodes[i] + .nostr(|nostr| nostr.open_query_event(&query.query_id, &event.to_string())) + .expect("open"); + if let Some(ResolvedRecord::KeyPackage { plaintext, .. }) = opened { + nodes[i] + .protocol + .handle_resolved_key_package(&plaintext) + .expect("resolved key package"); + nodes[i].drain(); + } + } + nodes[i].nostr(|nostr| nostr.complete_query(&query.query_id)); + } + } + for node in nodes.iter_mut() { + node.drain(); + // The reconciliation scan is throttled to once every two seconds; + // the test steps far faster than that. + node.protocol.last_reconciliation_at = None; + node.protocol.process().expect("process"); + } +} + +#[test] +fn a_device_known_only_by_its_address_is_reached_over_nostr() { + let mut relay = Relay::default(); + let mut nodes = vec![Node::new("alice"), Node::new("bob")]; + // bob's key packages reach the relay before alice knows he exists. + for _ in 0..3 { + step(&mut relay, &mut nodes); + } + assert!( + relay.stored.iter().any(|event| event["kind"] == 30443), + "precondition: bob's key packages stand on the relay" + ); + assert_eq!( + nodes[0].queries, 0, + "precondition: alice has asked about no one" + ); + + let bob = nodes[1].address.clone(); + let message_id = nodes[0] + .protocol + .send_message(&bob, "found you through a relay", None, None::) + .expect("queued until a session exists") + .as_str() + .to_string(); + + for _ in 0..40 { + step(&mut relay, &mut nodes); + if nodes[0].delivered(&message_id) { + break; + } + } + assert_eq!( + nodes[1] + .inbox + .iter() + .filter(|content| content.as_str() == "found you through a relay") + .count(), + 1, + "bob receives the message from a device whose only path to him is a relay, once" + ); + assert!( + nodes[0].delivered(&message_id), + "and alice holds bob's acknowledgement" + ); + assert!( + nodes[0].queries >= 1, + "the session formed from the key package alice fetched" + ); +} diff --git a/crates/offline-protocol/src/transport_manager.rs b/crates/offline-protocol/src/transport_manager.rs index cb84bb90..8840befc 100644 --- a/crates/offline-protocol/src/transport_manager.rs +++ b/crates/offline-protocol/src/transport_manager.rs @@ -418,6 +418,20 @@ impl TransportManager { .unwrap_or(false) } + /// Whether asking the relays for `peer_id`'s published key packages would + /// queue a query now. `false` when Nostr is not installed. Reads only. + pub fn nostr_resolution_due(&self, peer_id: &str) -> bool { + self.nostr_transport() + .is_some_and(|nostr| nostr.resolution_due(peer_id)) + } + + /// Asks the relays for `peer_id`'s published key packages. Returns whether + /// a query was queued; `false` when Nostr is not installed. + pub fn request_nostr_key_packages(&self, peer_id: &str) -> bool { + self.nostr_transport() + .is_some_and(|nostr| nostr.request_peer_key_packages(peer_id)) + } + /// Enables or disables username discovery on the Nostr transport. pub fn set_nostr_username_discovery_enabled(&mut self, enabled: bool) { if let Some(nostr) = self.nostr_transport() { diff --git a/docs/bridges/web.md b/docs/bridges/web.md index cf77fd01..ded2c973 100644 --- a/docs/bridges/web.md +++ b/docs/bridges/web.md @@ -418,18 +418,17 @@ The 10 s is the sixth hand-mirrored constant pinned here as in the two mobile trackers. **Reaching a device by its address alone asks the relays for its key -packages.** `sendMessage` and `establishSecureSession` to a device with no -session and no live link ask for the packages it published (cold contact, -on by default); a linked device forms its session over the link, and asking -the relays anyway would only tell them which address this node wants; -the first one that opens forms the session, and a message sent meanwhile -waits for it. The engine does not ask on its own: a session-less send only -queues, and its own key-package push reaches only the mesh (a gap on every -binding, filed as -[#633](https://github.com/Offline-Protocol/offline-protocol-sdk/issues/633); -once the engine asks, this request goes). Without the request, two browsers that share only a relay never meet, which is what the -negative control in `a_device_known_only_by_its_address_is_reached_over_nostr` -shows. `establishSecureSession` answers `"created"`, `"existing"` or +packages.** The engine asks, as it does on every binding: `sendMessage` and +`establishSecureSession` to a device with no session and no live link queue +a lookup of the packages it published (cold contact, on by default), and the +tick asks again while a message waits. A linked device forms its session +over the link, and asking the relays anyway would only tell them which +address this node wants. The first package that opens forms the session, +and a message sent meanwhile waits for it. The worker only moves the query +and its answers; it decides nothing +([#633](https://github.com/Offline-Protocol/offline-protocol-sdk/issues/633) +is where the engine stopped leaving this to the binding). +`establishSecureSession` answers `"created"`, `"existing"` or `"pending"`; for a device met only by its address, pending is the ordinary first answer. diff --git a/docs/nostr.md b/docs/nostr.md index 7a705a3b..07cf6c21 100644 --- a/docs/nostr.md +++ b/docs/nostr.md @@ -362,7 +362,11 @@ Sealing a frame requires the recipient's Nostr public key, and until it is known To make the fallback rare rather than routine, each install **publishes MLS key packages as fetchable relay records**, and resolves a peer's before sealing to them. - **Publishing.** `NOSTR_KEY_PACKAGE_SLOTS` (5) single-use key packages are published as NIP-33 addressable events (kind `30443`), one per slot, tagged `[["d", ], ["p", ]]` and signed by the install's real Nostr key. Slots refresh on the process tick: a package consumed by a Welcome, or expired, is replaced and republished under the same slot id. -- **Resolving.** A send to a peer whose per-install key we lack queues a query — `{"kinds":[30443],"#p":[]}` — issued to every connected relay. *That send still goes out on the bootstrap leg*: blocking on a relay round-trip would turn a metadata upgrade into latency, and the round-trip may have nothing to return. The answer upgrades the next frame. +- **Resolving.** A query, `{"kinds":[30443],"#p":[]}`, issued to every connected relay, asks for a peer's records. Two things queue one, at most once per peer every 5 minutes: + - **A session is wanted and no key package is held.** A send to, or an explicit `establish_secure_session` with, a peer this device holds no session and no key package for asks, and the reconciliation tick asks again while a message waits. This is the query cold contact depends on: with encryption required, that send builds no frame until a session exists, so nothing else would ask. A peer on a live Bluetooth or Wi-Fi link is not looked up, since the link carries the exchange; nor is a blocked peer. + - **A frame is sealed without the peer's per-install key.** *That frame still goes out on the bootstrap leg*: blocking on a relay round-trip would turn a metadata upgrade into latency, and the round-trip may have nothing to return. The answer upgrades the next frame. + + Each query tells the relays it reaches that this device is interested in the queried tag. That is the price of fetching rather than pushing, and it is paid only for peers the application asked to reach. This is what makes **cold first contact** possible at all: a peer known only by address — from an invite or a QR code — becomes reachable over Nostr with no prior key-package exchange over some other transport. Note what changed with addressing: cold contact by *username* is gone, because a username no longer resolves to anything. Reaching a stranger means holding their address first. diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index 80cfe5f9..5e08eaac 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -26,6 +26,7 @@ stateDiagram-v2 [*] --> None None --> KeyPackageSent: push our key package None --> Establishing: peer's key package received + None --> Establishing: peer's published key package fetched from a relay KeyPackageSent --> Joined: peer built a Welcome against it KeyPackageSent --> KeyPackageSent: no session after the interval, push again @@ -140,6 +141,37 @@ The recipient's side has three rules of its own: every frame of both crossing each device in between. The fresh package a new session sends for group invites still goes, after the Welcome. +### A peer only a relay reaches is asked about + +**A session wanted with a peer this device holds no key package for is +enough to ask every carrier that can produce one.** For a peer known only by +address and reachable only through a Nostr relay, that carrier is the relay: +the peer published single-use key packages there ([the Nostr +guide](../nostr.md#published-key-packages-and-cold-first-contact)), and +fetching one is the whole of cold first contact. + +Nothing asked before this rule (issue #633). A send with no session only +queues, so no frame is built for the transport to attach a lookup to; +discovery's first push never names a peer that has sent nothing; and the +mesh push above stands down whenever Nostr claims to reach the peer. The +records were published and never fetched. + +So three places ask the relays, each only where it has found no key package: +a send, an explicit `establish_secure_session`, and the reconciliation tick +for each peer holding a queued message that has never been pushed to. The +tick is what repeats it: a lookup no relay could answer (none held a record +yet, or none was live) is otherwise never asked again while the message +waits. The transport allows one lookup per peer every 5 minutes, and that +check runs before the session check so that a waiting peer costs one +storage read per interval rather than one per tick. + +No lookup is made for a blocked peer, with cold contact off, for a peer that +already has a session, or for a peer on a live mesh link: the link carries +the exchange within a tick, and a lookup would only tell the relays which +address this device wants. A fetched package is handled as a pushed one is, +so the session is built on arrival and our own package goes back, which is +what heals a fetched package that was already consumed. + ### An unconfirmed session holds one probe An unconfirmed session is probed on a fixed cadence, and **a peer holds at most From 7356bfd72b64fc123a8ff273768d50af6696e0f2 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Mon, 26 Oct 2026 00:44:22 +0530 Subject: [PATCH 2/2] refactor(bindings): drop the browser's own key-package request The engine asks the relays for a session-less peer's published key packages itself now, with the same live-link gate the browser applied, so the browser node's reach_over_nostr and its helpers go. a_linked_peer_is_not_looked_up_on_the_relays now pins the engine's gate. --- bindings/web/src/index.d.ts | 2 +- bindings/web/src/index.js | 2 +- crates/offline-protocol-web/src/node.rs | 30 ++----------------- crates/offline-protocol-web/src/nostr_host.rs | 12 -------- .../offline-protocol-web/src/nostr_tests.rs | 4 +-- 5 files changed, 7 insertions(+), 43 deletions(-) diff --git a/bindings/web/src/index.d.ts b/bindings/web/src/index.d.ts index 3e2e715e..ca1fc651 100644 --- a/bindings/web/src/index.d.ts +++ b/bindings/web/src/index.d.ts @@ -122,7 +122,7 @@ export class OfflineNode extends EventTarget { sendMessage(to: string, content: string, options?: SendOptions): Promise; /** * Starts a session with a device. "pending" means its key package has not - * arrived; with relays, the node has asked them for it, and + * arrived; with relays, the engine has asked them for it, and * `secure_session_established` follows. */ establishSecureSession(address: string): Promise; diff --git a/bindings/web/src/index.js b/bindings/web/src/index.js index 2fc3cb58..1417888f 100644 --- a/bindings/web/src/index.js +++ b/bindings/web/src/index.js @@ -144,7 +144,7 @@ export class OfflineNode extends EventTarget { /// Starts a session with `address`: "created" (the Welcome went out now), /// "existing", or "pending" while its key package has not arrived. On a - /// node with relays, "pending" has asked them for the packages the device + /// node with relays, the engine has asked them for the packages the device /// published, and `secure_session_established` follows. A message sent /// meanwhile waits for the session, so calling this first is optional. establishSecureSession(address) { diff --git a/crates/offline-protocol-web/src/node.rs b/crates/offline-protocol-web/src/node.rs index 83c080d7..864f4541 100644 --- a/crates/offline-protocol-web/src/node.rs +++ b/crates/offline-protocol-web/src/node.rs @@ -470,7 +470,6 @@ impl Node { content: &str, ephemeral: bool, ) -> NodeResult { - self.reach_over_nostr(recipient); let id = self.protocol.send_message_with( recipient, content, @@ -540,13 +539,13 @@ impl Node { /// - [`SessionStart::Created`]: this call built it and sent the Welcome; /// - [`SessionStart::Existing`]: there already is one; /// - [`SessionStart::Pending`]: no key package from `peer` yet. On a node - /// with relays this call has asked them for the peer's published - /// packages, and `secure_session_established` follows once one arrives. + /// with relays and no live link to `peer`, the engine has asked them + /// for the peer's published packages, and `secure_session_established` + /// follows once one arrives. /// /// Pending is not an error here, as it is in the engine: for a device met /// only by its address it is the ordinary first answer. pub fn establish_secure_session(&mut self, peer: &str) -> NodeResult { - self.reach_over_nostr(peer); match self.protocol.establish_secure_session(peer) { Ok(Some(_)) => Ok(SessionStart::Created), Ok(None) => Ok(SessionStart::Existing), @@ -595,34 +594,11 @@ impl Node { .map_err(mls) } - /// Asks the relays for `peer`'s published key packages when there is no - /// session with it yet and no live link to it. Nothing else does: a - /// session-less send only queues, and the engine's own key-package push - /// reaches only the mesh. That is a core gap on every binding, filed as - /// #633; once the engine asks for itself, this goes. - /// - /// A linked peer is skipped: the link carries the key-package exchange - /// within a tick, and a relay query would only publish which address - /// this node wants (its `#p` filter) for nothing. - fn reach_over_nostr(&self, peer: &str) { - if !self.nostr_enabled() || self.peers.contains_key(peer) || self.has_session(peer) { - return; - } - self.request_nostr_key_packages(peer); - } - /// Whether `peer` is announced on a peer-stream link now. pub(crate) fn stream_linked(&self, peer: &str) -> bool { self.peers.contains_key(peer) } - fn has_session(&self, peer: &str) -> bool { - self.protocol - .mls_manager() - .and_then(|manager| manager.read().ok()?.has_session(peer).ok()) - .unwrap_or(false) - } - fn preamble(&self) -> NodeResult> { let manager = self .protocol diff --git a/crates/offline-protocol-web/src/nostr_host.rs b/crates/offline-protocol-web/src/nostr_host.rs index 9bf941ec..253843be 100644 --- a/crates/offline-protocol-web/src/nostr_host.rs +++ b/crates/offline-protocol-web/src/nostr_host.rs @@ -279,18 +279,6 @@ impl Node { } } - /// Asks the relays for `peer`'s published key packages. The transport - /// rate-limits the request per peer, so a caller may repeat it. - pub(crate) fn request_nostr_key_packages(&self, peer: &str) -> bool { - self.with_nostr(|nostr| nostr.request_peer_key_packages(peer)) - .unwrap_or(false) - } - - /// Whether Nostr is wired on this node. - pub(crate) fn nostr_enabled(&self) -> bool { - self.nostr_host.is_some() - } - fn open_query_event(&mut self, query_id: &str, event: &Value) -> NodeResult<()> { let event_json = event.to_string(); let opened = self diff --git a/crates/offline-protocol-web/src/nostr_tests.rs b/crates/offline-protocol-web/src/nostr_tests.rs index 2dd3fc7c..c17469e8 100644 --- a/crates/offline-protocol-web/src/nostr_tests.rs +++ b/crates/offline-protocol-web/src/nostr_tests.rs @@ -356,8 +356,8 @@ fn linked_pair() -> (Peer, Peer) { /// A peer this node holds a live link to forms its session over the link; /// asking the relays for its key packages would only publish which address -/// this node wants. Without the link check in `reach_over_nostr` a query -/// comes out here. +/// this node wants. Without the engine's mesh-link gate on its key-package +/// request a query comes out here. #[test] fn a_linked_peer_is_not_looked_up_on_the_relays() { let (mut a, b) = linked_pair();