Fix/dedupe inbound requests - #103
Merged
Merged
Conversation
ServerEventPipeline.unwrap checks seenEventIds for gift-wrap envelopes and their decrypted inner events, but handleUnencryptedEvent only verified the signature. The relay pool forwards every copy by design (dedup is left to the transport), so a plaintext request published to N relays reached the server N times and was dispatched N times. Seen in production with cordn, whose client sends coordinator requests unencrypted over three default relays: every postGroupMessage was stored at three consecutive cursors with identical ciphertext, so each group member saw two undecryptable copies of every message. Any non-idempotent tool behind a multi-relay server is affected the same way. Apply the same event-id dedup to unencrypted events, recording the id only after verifyEvent succeeds: an id commits to everything but the signature, so marking first would let a copy with a bad signature, delivered first, suppress the genuine request. This relies on the previous commit: with the client nonce, two distinct requests never share an id, so only real duplicates are dropped. Retries (including the payment flow's transport.send(rawRequest)) sign a new event and are unaffected. Clients without the nonce keep the edge case encrypted requests already had: an identical repeat within the same second is dropped. Tests: one request delivered by three relays is dispatched once; a bad-signature copy delivered first does not block the genuine one; two signed requests with the same JSON-RPC payload are both processed. (cherry picked from commit 1f6d2a78f33f961ec4a1060cea30eb1ad1272df9)
The nonce commit is not being imported; the same-second repeat edge case is documented as accepted behavior, matching the encrypted path.
A paid callback that resolves within the same second as the original request re-signs a byte-identical event (created_at has second resolution), which the server's event-id dedupe drops, leaving the paid request unexecuted. Apply the same floor the -32043 retry already uses.
…e check Mirrors the server pipeline ordering: a bad-signature copy delivered first can no longer mark the id and suppress the genuine event.
Subscriptions keep their start-time since, so resubscribes replay already-processed events; the transport-level dedup is what prevents re-dispatch. The pool comment claimed payment retries republish the same event id - with the minRetryDelayMs floors they re-sign with a new id, and association is content-keyed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
De-duplicate unencrypted inbound Nostr events by event id on
NostrServerTransport, matching what the gift-wrap path already did. Two companion fixes close the interactions this creates, and the client-side dedup ordering is brought in line with the server.External contribution by cloudfodder cloudfodder@relay.tools (self-hosted GitLab:
code.relay.tools/forks/contextvm-sdk, MR !1, tip1f6d2a7), cherry-picked as892fcfawith authorship preserved (cherry-pick -x). Their branch also contained a client-request nonce commit, which we deliberately did not import (see Rejected alternatives).Motivation
The relay pool forwards every copy of an event by design (dedup is the transport's job — see
applesauce-relay-pool.ts). Gift-wrap envelopes and decrypted inner events were already deduped viaseenEventIds, but unencrypted events were only signature-checked, so:sinceatstart()and resubscribe with the same filter, so every relay reconnect replays all events since transport start — pre-fix, the unencrypted path re-dispatched all of them, even with a single relay.Changes
892fcfaverifyEventsucceeds (a bad-signature copy delivered first cannot suppress the genuine event)f8a992d6a59bb3-32042retry atminRetryDelayMs, mirroring the-32043retry — a sub-second retry re-signs a byte-identical event (id hash excludes the signature), which dedup would drop, deadlocking the paid requestfb54b36verifyEvent, matching the server ordering (was mark-then-verify) + regression test8f6184527b5018minRetryDelayMsJSDoc now covers-32042retries; widen retry test waitAccepted edge case
A byte-identical repeat within the same second (same JSON-RPC id, params,
created_at) is dropped as a duplicate — identical to the behavior the encrypted path has always had. A restarted client repeating a request within the same second must wait for the next second or retry after timeout. Payment retries are unaffected: they are floored atminRetryDelayMsand re-sign in a later second, and payment authorization is content-keyed (sha256(clientPubkey + method + params)), never event-id-keyed.Testing
bun test --concurrent --timeout=60000: 653 pass / 0 fail;tsc --noEmitandeslint src/clean.Release notes
Three patch changesets included: the dedup fix, the
-32042retry floor, and the client dedup ordering fix.).