fix(protocol): ask the relays for a stranger's key packages when a session is wanted

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

bindings/web/src/index.d.ts

@@ -122,7 +122,7 @@ export class OfflineNode extends EventTarget {
   sendMessage(to: string, content: string, options?: SendOptions): Promise<string>;
   /**
    * 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<SessionStart>;

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) {

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<Address> {
         // 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::<Address>() else {
-            return false;
-        };
+        let address = user_id.parse::<Address>().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();

crates/offline-protocol-web/src/node.rs

@@ -470,7 +470,6 @@ impl Node {
         content: &str,
         ephemeral: bool,
     ) -> NodeResult<String> {
-        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<SessionStart> {
-        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<Vec<u8>> {
         let manager = self
             .protocol

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

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();

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.
     ///

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)?))
     }
 

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

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<String>,
@@ -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,