diff --git a/src/daemon/admin/index.ts b/src/daemon/admin/index.ts index e19aaa7..a085607 100644 --- a/src/daemon/admin/index.ts +++ b/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 - // rejection (process-terminating under Node defaults). #48 / CS-3. - this.notifyAdminsOfNewConnection(connectionString).catch((e) => - console.log('notifyAdminsOfNewConnection failed:', e?.message ?? e), - ); + this.notifyAdminsOfNewConnection(connectionString); } - }).catch((e) => console.log('config() failed during admin init:', e?.message ?? e)); + }); } public async config(): Promise { @@ -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 - // than a fixed sleep — these external public relays can be slow to come - // 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)); - } + // Give the connections a moment to come up before publishing. + await new Promise((r) => setTimeout(r, 2500)); - // try/finally so a throw mid-loop can't leak the pool's reconnect loops + - // sockets for the process lifetime (#48 / review CS-3). dmUser itself is - // 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(); + 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); } + pool.stop(); } /** diff --git a/src/daemon/lib/relay-pool.ts b/src/daemon/lib/relay-pool.ts index cb5f120..6ca0c89 100644 --- a/src/daemon/lib/relay-pool.ts +++ b/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, 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 - // 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); - }, + onevent: (e: Event) => s.onevent(e), oneose: () => s.oneose?.(), }); this.active.set(id, sub); @@ -189,39 +179,16 @@ class ManagedRelay { private connectOnce(): Promise { return new Promise((resolve) => { void (async () => { - // enableReconnect:false — WE own reconnect, not nostr-tools. let relay: Relay; try { - relay = new Relay(this.url, { enableReconnect: false }); - } catch (e: any) { - 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 }); + // enableReconnect:false — WE own reconnect, not nostr-tools. + relay = await Relay.connect(this.url, { enableReconnect: false }); } 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 | 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 = 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( - url, - this.registry, - (...a: any[]) => this.log(`[relay:${url}]`, ...a), - (id) => this.markSeen(id), + new ManagedRelay(url, this.registry, (...a: any[]) => + this.log(`[relay:${url}]`, ...a), ), ); } - /** 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(); diff --git a/src/utils/dm-user.ts b/src/utils/dm-user.ts index 34c6add..db3dc06 100644 --- a/src/utils/dm-user.ts +++ b/src/utils/dm-user.ts @@ -12,26 +12,22 @@ export async function dmUser( content: string, pool: RelayPool, ): Promise { - // Guard the whole thing: nip19.decode throws on a malformed npub (passes the - // startsWith check but fails the bech32 checksum), and that previously threw - // *before* the publish try, escaping the caller. Best-effort — never throw. - // (review CS-3) + 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, + ); 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); } } diff --git a/tests/relay-pool.test.ts b/tests/relay-pool.test.ts index 5dbc76b..148f144 100644 --- a/tests/relay-pool.test.ts +++ b/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(); -});