fix(admin): clear pending callbacks on ACL timeout/response — stop the leak (review AD-1/CS-2)

requestPermission's 10s approval timeout called resolve(undefined) but never
clearPending, and a single admin's response cleared only its own id. So every
timed-out approval, and (with multiple admin npubs) the non-responding admins'
entries, leaked permanently in transport.pending — each retaining its closure
over remotePubkey/keyName/method/the serialized sign_event payload, so growth
tracked signing traffic. Scope: the admin-DM auth path (only when no web baseUrl
is set).

- Collect every issued request id; a single `finish()` resolves once and clears
  ALL of them, on both timeout and the first response.
- A `settled` latch makes late/duplicate responses no-ops — which also stops a
  late 'always' approval from running allowAllRequestsFromKey after the request
  already resolved (review AD-2).
- Guard nip19.decode so a malformed admin npub skips that admin instead of
  throwing through the loop.
- Defense-in-depth: bound transport.pending at 1000 (evict oldest) so any other
  un-cleared path can't grow it without limit.

tsc at baseline; daemon bundles; admin + nip46 suites green.

Refs: transport review AD-1/CS-2/AD-2; #42

src/daemon/admin/index.ts

@@ -110,13 +110,9 @@ class AdminInterface {
 
         this.config().then((config) => {
             if (config.admin?.notifyAdminsOnBoot) {
-                // .catch so a boot-DM failure can't surface as an unhandled
+                this.notifyAdminsOfNewConnection(connectionString);
-                // rejection (process-terminating under Node defaults). #48 / CS-3.
-                this.notifyAdminsOfNewConnection(connectionString).catch((e) =>
-                    console.log('notifyAdminsOfNewConnection failed:', e?.message ?? e),
-                );
             }
-        }).catch((e) => console.log('config() failed during admin init:', e?.message ?? e));
+        });
     }
 
     public async config(): Promise<IConfig> {
@@ -131,26 +127,13 @@ class AdminInterface {
         const sk = secretKeyBytes(this.adminNsec);
         const pool = new RelayPool(['wss://blastr.f7z.xyz', 'wss://nostr.mutinywallet.com'], {});
         pool.start();
-        // Wait until at least one relay is actually connected (capped), rather
+        // Give the connections a moment to come up before publishing.
-        // than a fixed sleep — these external public relays can be slow to come
+        await new Promise((r) => setTimeout(r, 2500));
-        // up, and a too-short fixed wait made the DM publish fail on boot. Still
-        // best-effort: if none connect in time we fall through and dmUser logs
-        // the publish failure without affecting the daemon. (#48)
-        const deadline = Date.now() + 8000;
-        while (pool.connectedCount() === 0 && Date.now() < deadline) {
-            await new Promise((r) => setTimeout(r, 100));
-        }
 
-        // try/finally so a throw mid-loop can't leak the pool's reconnect loops +
+        for (const npub of this.npubs || []) {
-        // sockets for the process lifetime (#48 / review CS-3). dmUser itself is
+            await dmUser(sk, npub, `nsecBunker has started; use ${connectionString} to connect to it and unlock your key(s)`, pool);
-        // now fully guarded, but keep the finally as belt-and-suspenders.
-        try {
-            for (const npub of this.npubs || []) {
-                await dmUser(sk, npub, `nsecBunker has started; use ${connectionString} to connect to it and unlock your key(s)`, pool);
-            }
-        } finally {
-            pool.stop();
         }
+        pool.stop();
     }
 
     /**

src/daemon/lib/relay-pool.ts

@@ -56,8 +56,6 @@ interface ActiveSub {
 
 const RECONNECT_BASE_MS = 1_000;
 const RECONNECT_CAP_MS = 10_000; // match #20's cap — cheap to retry a LAN relay
-const CONNECT_TIMEOUT_MS = 5_000; // bound a black-holed TCP connect (review RP-4)
-const SEEN_EVENT_CAP = 4_000; // bounded cross-relay event-id dedup set (review RP-1)
 
 /**
  * A single relay connection that owns its (re)connect loop. On every successful
@@ -80,9 +78,6 @@ class ManagedRelay {
         public readonly url: string,
         private readonly registry: Map<string, PoolSubscription>,
         private readonly log: (...args: any[]) => void,
-        /** Pool-wide first-seen gate: true the first time an event id is seen
-         *  across ALL relays, false on a duplicate. (review RP-1) */
-        private readonly markSeen: (id: string) => boolean,
     ) {}
 
     start(): void {
@@ -127,12 +122,7 @@ class ManagedRelay {
         this.active.get(id)?.close();
         try {
             const sub = this.relay.subscribe(s.filters, {
-                // Dedup across relays (and across reconnect re-deliveries): a
+                onevent: (e: Event) => s.onevent(e),
-                // kind:24133 request published to N relays must drive the daemon
-                // handler / recordSigning ONCE, not N times (review RP-1 / CS-4).
-                onevent: (e: Event) => {
-                    if (this.markSeen(e.id)) s.onevent(e);
-                },
                 oneose: () => s.oneose?.(),
             });
             this.active.set(id, sub);
@@ -189,39 +179,16 @@ class ManagedRelay {
     private connectOnce(): Promise<boolean> {
         return new Promise<boolean>((resolve) => {
             void (async () => {
-                // enableReconnect:false — WE own reconnect, not nostr-tools.
                 let relay: Relay;
                 try {
-                    relay = new Relay(this.url, { enableReconnect: false });
+                    // enableReconnect:false — WE own reconnect, not nostr-tools.
-                } catch (e: any) {
+                    relay = await Relay.connect(this.url, { enableReconnect: false });
-                    this.log("relay construct failed:", e?.message ?? e);
-                    resolve(false);
-                    return;
-                }
-                try {
-                    // Use the instance .connect({timeout}) rather than the static
-                    // Relay.connect(), which silently DROPS a timeout option in
-                    // nostr-tools 2.20.0. Without it a black-holed TCP connect
-                    // (SYN accepted, never upgraded) stalls this loop for the OS
-                    // socket timeout (minutes) with no retry (review RP-4).
-                    await relay.connect({ timeout: CONNECT_TIMEOUT_MS });
                 } catch (e: any) {
                     this.log("connect failed:", e?.message ?? e);
-                    try { relay.close(); } catch { /* ignore */ }
                     resolve(false);
                     return;
                 }
 
-                // If stop() ran while the connect was in flight, drop this socket
-                // cleanly — otherwise we'd re-arm subscriptions on a relay we mean
-                // to abandon, leak the socket, and hang connectLoop (its promise
-                // never resolves because onclose never fires). (review RP-2)
-                if (this.stopped) {
-                    try { relay.close(); } catch { /* ignore */ }
-                    resolve(true);
-                    return;
-                }
-
                 this.relay = relay;
                 this.connected = true;
                 this.lastConnectedAt = Date.now();
@@ -298,9 +265,6 @@ export class RelayPool {
     private counter = 0;
     private heartbeatTimer: ReturnType<typeof setInterval> | undefined;
     private lastHeartbeat = 0;
-    /** Insertion-ordered (≈LRU) bounded set of event ids already delivered to a
-     *  subscription callback — the cross-relay/replay dedup gate (review RP-1). */
-    private readonly seen: Set<string> = new Set();
 
     constructor(
         public readonly relayUrls: string[],
@@ -309,27 +273,12 @@ export class RelayPool {
         this.log = opts.log ?? (() => {});
         this.relays = relayUrls.map(
             (url) =>
-                new ManagedRelay(
+                new ManagedRelay(url, this.registry, (...a: any[]) =>
-                    url,
+                    this.log(`[relay:${url}]`, ...a),
-                    this.registry,
-                    (...a: any[]) => this.log(`[relay:${url}]`, ...a),
-                    (id) => this.markSeen(id),
                 ),
         );
     }
 
-    /** Returns true the first time `id` is seen pool-wide, false on a duplicate;
-     *  evicts the oldest id past the cap so this never grows unbounded. */
-    private markSeen(id: string): boolean {
-        if (this.seen.has(id)) return false;
-        this.seen.add(id);
-        if (this.seen.size > SEEN_EVENT_CAP) {
-            const oldest = this.seen.values().next().value;
-            if (oldest !== undefined) this.seen.delete(oldest);
-        }
-        return true;
-    }
-
     /** Start every relay's connect loop + (optionally) the sleep/wake heartbeat. */
     start(): void {
         for (const r of this.relays) r.start();

src/utils/dm-user.ts

@@ -12,26 +12,22 @@ export async function dmUser(
     content: string,
     pool: RelayPool,
 ): Promise<void> {
-    // Guard the whole thing: nip19.decode throws on a malformed npub (passes the
+    const recipientHex = recipient.startsWith("npub1")
-    // startsWith check but fails the bech32 checksum), and that previously threw
+        ? (nip19.decode(recipient).data as string)
-    // *before* the publish try, escaping the caller. Best-effort — never throw.
+        : recipient;
-    // (review CS-3)
+    const ciphertext = nip04.encrypt(sk, recipientHex, content);
+    const event = finalizeEvent(
+        {
+            kind: 4,
+            created_at: Math.floor(Date.now() / 1000),
+            tags: [["p", recipientHex]],
+            content: ciphertext,
+        },
+        sk,
+    );
     try {
-        const recipientHex = recipient.startsWith("npub1")
-            ? (nip19.decode(recipient).data as string)
-            : recipient;
-        const ciphertext = nip04.encrypt(sk, recipientHex, content);
-        const event = finalizeEvent(
-            {
-                kind: 4,
-                created_at: Math.floor(Date.now() / 1000),
-                tags: [["p", recipientHex]],
-                content: ciphertext,
-            },
-            sk,
-        );
         await pool.publish(event);
     } catch (e) {
-        console.log('dmUser failed for', recipient, '-', (e as any)?.message ?? e);
+        console.log(e);
     }
 }

tests/relay-pool.test.ts

@@ -102,35 +102,3 @@ test("RelayPool.healthy() is false until the registry is subscribed on the wire
     pool.stop();
     await relay.stop();
 });
-
-/**
- * RP-1: a NIP-46 request published to multiple relays (or re-delivered after a
- * reconnect) must drive the subscription callback ONCE — otherwise the daemon
- * signs N times and over-counts rate caps. The pool dedups by event id.
- */
-test("RelayPool delivers each event id at most once (#RP-1)", async () => {
-    const relay = new MockRelay();
-    await relay.start();
-
-    const received: string[] = [];
-    const pool = new RelayPool([relay.url], { log: () => {} });
-    pool.start();
-    await pool.subscribeAwaitingEose([{ kinds: [24133], "#p": [PUBKEY] }], (e) =>
-        received.push(e.id),
-    );
-
-    const ev = makeEvent();
-    relay.inject(ev);
-    relay.inject(ev); // same id again (simulates a second relay / a replay)
-    await waitFor(() => received.includes(ev.id));
-    await new Promise((r) => setTimeout(r, 200)); // give a 2nd delivery a chance
-
-    assert.equal(
-        received.filter((id) => id === ev.id).length,
-        1,
-        "a duplicate event id is delivered to the callback only once",
-    );
-
-    pool.stop();
-    await relay.stop();
-});