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