fix(transport): harden RelayPool — connect timeout, stop-race, cross-relay dedup (review RP-1/2/4) #50
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/relay-pool-hardening"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Folds the three transport-review medium findings that have real production teeth into the
RelayPool. (No critical/high were found; this is the highest-value cleanup.)RP-4 —
Relay.connecthad no timeout → a black-holed connect stalls a relay for minutesconnectOnceused the staticRelay.connect(), which silently drops a{timeout}option in nostr-tools 2.20.0 (it forwards options only to the constructor). So a host that accepts the SYN but never upgrades the WS, or a black-holed packet, stalled that relay's loop for the OS socket timeout with no retry. Now constructs theRelayand calls the instance.connect({ timeout: 5000 }), which arms the timeout → prompt reject + backoff. Production-relevant: the demo bunker is single-relay, so a stalled connect = a stalled bunker.RP-2 —
stop()during an in-flight connect leaked the socket + hungconnectLoopconnectOncedidn't re-checkstoppedafter theawait. Ifstop()ran mid-connect, the resolved socket re-armed subscriptions on a relay we meant to drop, leaked it, andconnectLoopnever terminated (its promise never resolved —onclosenever fires on a live socket). Now drops the socket cleanly and resolves if we were stopped mid-connect.RP-1 — no cross-relay event dedup → rate caps over-count N×
Each
ManagedRelayforwarded matching events straight to the shared callback. A kind:24133 request published to N relays (the normal NIP-46 pattern), or re-delivered after a reconnect, droveBackend.handleRequest→permitCallback→recordSigningN times, so a configured cap (e.g. 20/hr) bound ~N× tighter (fails closed, not open — no unauthorized signing). Added a bounded (4000-id, ≈LRU) pool-wide seen-event-id set;oneventfires at most once per id. Also closes the CS-4 replay vector (a relay that retains an ephemeral kind:24133 can't drive a re-sign after a flap).Tests
tests/relay-pool.test.tsadds a dedup assertion (a duplicate event id reaches the callback once).relay 3 / nip46 1 / admin 2green; daemon bundles;tscat baseline.Refs: transport review RP-1/RP-2/RP-4 + CS-4; #42
🤖 Generated with Claude Code
Folds three medium findings from the transport review into the RelayPool: - RP-4: connectOnce used the static Relay.connect(), which silently DROPS a {timeout} option in nostr-tools 2.20.0, so a black-holed TCP connect (SYN accepted, never upgraded) stalled that relay's loop for the OS socket timeout (minutes) with no retry. Now constructs the Relay and calls the instance .connect({ timeout: 5000 }), which honours the timeout → prompt reject + backoff. - RP-2: connectOnce didn't re-check `stopped` after the await. If stop() ran while a connect was in flight, the resolved socket re-armed subscriptions on a relay we meant to abandon, leaked the socket, and hung connectLoop (its promise never resolved because onclose never fired). Now drops the socket cleanly and resolves if stopped mid-connect. - RP-1: no cross-relay event dedup — a kind:24133 request published to N relays (the normal NIP-46 pattern), or re-delivered after a reconnect, drove the daemon handler + recordSigning N times, making rate caps bind ~N× tighter (fails closed, not open). Added a bounded (4000-id, ≈LRU) pool-wide seen-set; onevent fires at most once per event id. Closes the CS-4 replay vector too. Test: tests/relay-pool.test.ts asserts a duplicate event id is delivered once. relay 3 / nip46 1 / admin 2 green; daemon bundles; tsc at baseline. Refs: transport review RP-1/RP-2/RP-4, CS-4; #42