From 2e98e69ddf03849f2fe5bc333bb411af310ea6e6 Mon Sep 17 00:00:00 2001
From: Avi <avi@aiolabs.com>
Date: Wed, 23 Sep 2026 09:00:17 -0500
Subject: [PATCH] fix(pairing): subscribe to signer replies BEFORE the
 handshake publishes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The relay pushes events only to subscriptions that exist at delivery time.
Both connect flows spawned the handshake task before run_demux registered the
kind-24133 author subscription, so a fast signer reply (the live Sep 23 Amber
scan: clean connect + secret echo, then get_public_key) landed in the gap and
was silently dropped — the identity RPC timed out 30s later and the session
died with 'The signer would not reveal its public key' after the connection
was already stored, leaving the UI signed in with no profile.

Both run_sign_task (bunker flow) and run_paired (QR pairing) now subscribe
first via subscribe_to_signer and hand the stream + subscription to run_demux.
futures-util promoted from dev-dependency to runtime. cargo test 209+3e2e
green, clippy 0, fmt clean, release rebuilt.
---
 Cargo.toml                 |  1 +
 src/signer/nip46_client.rs | 55 +++++++++++++++++++++++++++++++-------
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 4e15e5b..431527f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,6 +20,7 @@ rpassword = "7"
 sha2 = "0.10"
 async-trait = "0.1"
 keyring = "4.2"
+futures-util = "0.3"
 
 [dev-dependencies]
 base64 = "0.22"
diff --git a/src/signer/nip46_client.rs b/src/signer/nip46_client.rs
index c413aa0..628374b 100644
--- a/src/signer/nip46_client.rs
+++ b/src/signer/nip46_client.rs
@@ -870,6 +870,14 @@ impl Nip46ClientSigner {
         conversation: ConversationKey,
         client: Client,
     ) -> Result<(), String> {
+        // Subscribe BEFORE the handshake publishes anything: relays only push
+        // events to subscriptions that exist at delivery time, and the
+        // identity RPC (`get_public_key`) fires within milliseconds of this
+        // point. When the spawn raced ahead of the subscription, Amber's fast
+        // reply landed in the gap and was lost — the live Sep 23 scan died
+        // exactly there ("signer would not reveal its public key" after a
+        // clean `connect` + secret echo).
+        let (notifications, subscription) = self.subscribe_to_signer(&client, uri.peer).await?;
         {
             let handshake = self.clone();
             let peer = uri.peer;
@@ -879,7 +887,31 @@ impl Nip46ClientSigner {
                 }
             });
         }
-        self.run_demux(uri, keys, conversation, client).await
+        self.run_demux(uri, keys, conversation, client, notifications, subscription)
+            .await
+    }
+
+    /// Open the notification stream and register the kind-24133 author
+    /// subscription for the signer. Kept separate so every connect path can
+    /// establish it BEFORE publishing its first RPC.
+    async fn subscribe_to_signer(
+        &self,
+        client: &Client,
+        peer: PublicKey,
+    ) -> Result<
+        (
+            std::pin::Pin<Box<dyn futures_util::Stream<Item = ClientNotification> + Send>>,
+            Output<SubscriptionId>,
+        ),
+        String,
+    > {
+        let filter = Filter::new().kind(Kind::NostrConnect).author(peer);
+        let notifications = client.notifications();
+        let subscription = client
+            .subscribe(filter)
+            .await
+            .map_err(|e| format!("Could not subscribe: {e}"))?;
+        Ok((notifications, subscription))
     }
 
     /// Get current connection status.
@@ -1177,6 +1209,12 @@ impl Nip46ClientSigner {
         // Update connected relays
         self.inner.lock().await.client = Some(client.clone());
 
+        // Subscribe BEFORE the handshake publishes `connect`: relays only
+        // push the signer's replies to subscriptions that exist at delivery
+        // time, so registering after the first publish can silently drop the
+        // ack (see run_paired for the live incident this fixes).
+        let (notifications, subscription) = self.subscribe_to_signer(&client, uri.peer).await?;
+
         // The connect handshake runs as its own task: it publishes `connect`
         // and then must AWAIT the signer's ack — which can wait on a human
         // approving us in Amber — followed by `get_public_key` to learn the
@@ -1195,7 +1233,8 @@ impl Nip46ClientSigner {
             });
         }
 
-        self.run_demux(uri, keys, conversation, client).await
+        self.run_demux(uri, keys, conversation, client, notifications, subscription)
+            .await
     }
 
     /// The long-lived request/response demux loop, shared by both connect
@@ -1209,15 +1248,11 @@ impl Nip46ClientSigner {
         keys: Keys,
         conversation: ConversationKey,
         client: Client,
+        mut notifications: std::pin::Pin<
+            Box<dyn futures_util::Stream<Item = ClientNotification> + Send>,
+        >,
+        subscription: Output<SubscriptionId>,
     ) -> Result<(), String> {
-        // Subscribe to kind 24133 from signer
-        let filter = Filter::new().kind(Kind::NostrConnect).author(uri.peer);
-        let mut notifications = client.notifications();
-        let subscription = client
-            .subscribe(filter)
-            .await
-            .map_err(|e| format!("Could not subscribe: {e}"))?;
-
         // Handle incoming requests
         loop {
             let incoming =