diff --git a/.changeset/client-unencrypted-dedup-ordering.md b/.changeset/client-unencrypted-dedup-ordering.md new file mode 100644 index 0000000..1e4b2d9 --- /dev/null +++ b/.changeset/client-unencrypted-dedup-ordering.md @@ -0,0 +1,4 @@ +'@contextvm/sdk': patch +--- + +`NostrClientTransport` records an unencrypted inbound event id only after its signature verifies, matching the server pipeline and the client's own encrypted path. Previously a copy with a corrupted signature that arrived first would mark the id, and the genuine copy would then be dropped as a duplicate. diff --git a/.changeset/dedupe-unencrypted-requests.md b/.changeset/dedupe-unencrypted-requests.md new file mode 100644 index 0000000..6029d01 --- /dev/null +++ b/.changeset/dedupe-unencrypted-requests.md @@ -0,0 +1,5 @@ +--- +'@contextvm/sdk': patch +--- + +De-duplicate unencrypted inbound events by id on `NostrServerTransport`, as gift-wrapped events already were. The relay pool deliberately forwards every copy, so a plaintext request published to N relays was processed N times, and a non-idempotent tool ran N times for one call. The id is recorded only after the signature check, so a copy with a bad signature cannot suppress the genuine request. Retries that sign a new event in a later second get a new id and are unaffected; a byte-identical repeat of a request within the same second is dropped, the same edge case encrypted requests already had. The same dedup also protects against re-delivery on relay reconnects: subscriptions keep their original `since`, so resubscribes replay events the transport has already processed. diff --git a/.changeset/explicit-payment-retry-floor.md b/.changeset/explicit-payment-retry-floor.md new file mode 100644 index 0000000..0103666 --- /dev/null +++ b/.changeset/explicit-payment-retry-floor.md @@ -0,0 +1,4 @@ +'@contextvm/sdk': patch +--- + +Floor the explicit-gating `-32042` payment retry at `minRetryDelayMs` (default 1s), mirroring the `-32043` retry. An instantly-satisfied payment callback could retry within the same second as the original request, producing a byte-identical Nostr event that servers de-duplicate by id, leaving the paid request unexecuted. diff --git a/src/payments/client-payments.test.ts b/src/payments/client-payments.test.ts index fb039c8..5123dca 100644 --- a/src/payments/client-payments.test.ts +++ b/src/payments/client-payments.test.ts @@ -854,6 +854,7 @@ describe('withClientPayments()', () => { handlers: [{ pmi: 'fake', async handle(): Promise {} }], paymentInteraction: 'explicit_gating', onPaymentRequired: async () => ({ paid: true }), + minRetryDelayMs: 1, }); paid.onmessage = (msg) => observed.push(msg); await paid.start(); @@ -883,8 +884,8 @@ describe('withClientPayments()', () => { { eventId: 'evt4', correlatedEventId: 'req-event-id-3' }, ); - // Wait for async processing - await new Promise((r) => setTimeout(r, 0)); + // Wait for async processing (retry is floored at minRetryDelayMs) + await new Promise((r) => setTimeout(r, 25)); // Error should not be delivered to caller expect(observed).toHaveLength(0); diff --git a/src/payments/client-payments.ts b/src/payments/client-payments.ts index 916a305..6370f55 100644 --- a/src/payments/client-payments.ts +++ b/src/payments/client-payments.ts @@ -22,6 +22,7 @@ import type { } from './types.js'; import { LruCache } from '../core/utils/lru-cache.js'; import { createLogger } from '../core/utils/logger.js'; +import { sleep } from '../core/utils/utils.js'; import type { OriginalRequestContext, PendingRequest, @@ -68,8 +69,9 @@ export interface ClientPaymentsOptions { */ defaultPaymentTtlMs?: number; /** - * Minimum delay before a `-32043` Payment Pending retry is re-sent - * (milliseconds), applied after the server-provided `retry_after` backoff. + * Minimum delay before an explicit-gating retry (`-32042` after a satisfied + * payment, or a `-32043` Payment Pending retry) is re-sent (milliseconds). + * For `-32043` it is applied after the server-provided `retry_after` backoff. * * Guards against `retry_after: 0`: an immediate retry within the same second * can produce a byte-identical Nostr event (same content, same tags, same @@ -427,6 +429,10 @@ export function withClientPayments( requestEventId, method: rawRequest.method, }); + // Same floor as the -32043 retry: a sub-second retry can produce a + // byte-identical Nostr event (created_at has second resolution), + // which servers de-duplicate by event id. + await sleep(minRetryDelayMs); await transport.send(rawRequest); return; } diff --git a/src/relay/applesauce-relay-pool.ts b/src/relay/applesauce-relay-pool.ts index 547fe55..dad8ea1 100644 --- a/src/relay/applesauce-relay-pool.ts +++ b/src/relay/applesauce-relay-pool.ts @@ -480,13 +480,14 @@ export class ApplesauceRelayPool implements RelayHandler { // We deliberately subscribe to the raw `RelayGroup.req()` message stream // and forward EVERY event without deduplication. Dedup is intentionally NOT // performed at the relay layer: - // - The transport layer already deduplicates gift-wrap envelopes and - // decrypted inner events via its own `seenEventIds` cache, with - // protocol-aware semantics. - // - The explicit-gating payment flow republishes the SAME request event - // id after payment and relies on the server re-observing it. Relay- - // layer dedup by event id (whether applesauce 6.0.3's `distinct()` or a - // local `Set`) silently swallows that retry and deadlocks the flow. + // - The transport layer already deduplicates gift-wrap envelopes, + // decrypted inner events and unencrypted events via its own + // `seenEventIds` cache, with protocol-aware semantics. + // - Payment retries re-sign the request (floored at `minRetryDelayMs` in + // `client-payments` so a sub-second retry never reuses an event id), + // and rely on the server re-observing it. Relay-layer dedup by event id + // (whether applesauce 6.0.3's `distinct()` or a local `Set`) silently + // swallows that retry and deadlocks the flow. const sub = this.relayGroup .req(filters, { reconnect: Infinity, resubscribe: Infinity }) .subscribe({ diff --git a/src/transport/nostr-client/event-pipeline.ts b/src/transport/nostr-client/event-pipeline.ts index 96fea74..32f370e 100644 --- a/src/transport/nostr-client/event-pipeline.ts +++ b/src/transport/nostr-client/event-pipeline.ts @@ -147,15 +147,6 @@ export class ClientEventPipeline { private handleUnencryptedEvent( event: NostrEvent, ): UnwrappedClientEvent | null { - // Deduplicate plain inbound deliveries before dispatch. - if (this.deps.seenEventIds.has(event.id)) { - this.deps.logger.debug('Skipping duplicate inbound event', { - eventId: event.id, - }); - return null; - } - this.deps.seenEventIds.set(event.id, true); - if (!this.isFromExpectedServer(event)) return null; if (!verifyEvent(event)) { @@ -168,6 +159,18 @@ export class ClientEventPipeline { ); return null; } + + // Deduplicate plain inbound deliveries before dispatch. The id is marked + // only after the signature check so a bad-signature copy delivered first + // cannot suppress the genuine event (same ordering as the server). + if (this.deps.seenEventIds.has(event.id)) { + this.deps.logger.debug('Skipping duplicate inbound event', { + eventId: event.id, + }); + return null; + } + this.deps.seenEventIds.set(event.id, true); + return { event }; } diff --git a/src/transport/nostr-server-transport.dedup-response.test.ts b/src/transport/nostr-server-transport.dedup-response.test.ts index c8ee186..c1f8f07 100644 --- a/src/transport/nostr-server-transport.dedup-response.test.ts +++ b/src/transport/nostr-server-transport.dedup-response.test.ts @@ -236,6 +236,96 @@ describe.serial('NostrServerTransport duplicate response prevention', () => { ).toBe(1); }); + function makeUnencryptedRequest( + id: number, + clientSk: Uint8Array = generateSecretKey(), + createdAt = 1, + ): NostrEvent { + const serverPubkey = getPublicKey( + Uint8Array.from(Buffer.from('1'.repeat(64), 'hex')), + ); + return finalizeEvent( + { + kind: 25910, + created_at: createdAt, + tags: [['p', serverPubkey]], + content: JSON.stringify({ + jsonrpc: '2.0', + id, + method: 'tools/list', + params: {}, + }), + }, + clientSk, + ); + } + + // A relay delivers a fresh JSON object; a spread copy would also carry + // nostr-tools' cached verification flag. + const asDeliveredByRelay = (event: NostrEvent): NostrEvent => + JSON.parse(JSON.stringify(event)) as NostrEvent; + + it('processes an unencrypted request only once when several relays deliver it', async () => { + const transport = new NostrServerTransport({ + signer: new PrivateKeySigner('1'.repeat(64)), + relayHandler: makeCountingRelayHandler({ publishCalls: 0 }), + encryptionMode: EncryptionMode.OPTIONAL, + }); + const onmessage = mock(() => {}); + transport.onmessage = onmessage; + const request = makeUnencryptedRequest(1); + + for (let relay = 0; relay < 3; relay += 1) { + await transport['processIncomingEvent'](asDeliveredByRelay(request)); + } + + expect(onmessage).toHaveBeenCalledTimes(1); + expect( + transport.getInternalStateForTesting().correlationStore.eventRouteCount, + ).toBe(1); + }); + + it('does not let a bad-signature copy suppress the genuine unencrypted request', async () => { + const transport = new NostrServerTransport({ + signer: new PrivateKeySigner('1'.repeat(64)), + relayHandler: makeCountingRelayHandler({ publishCalls: 0 }), + encryptionMode: EncryptionMode.DISABLED, + }); + const onmessage = mock(() => {}); + transport.onmessage = onmessage; + const request = makeUnencryptedRequest(1); + + await transport['processIncomingEvent']({ + ...asDeliveredByRelay(request), + sig: '0'.repeat(128), + }); + expect(onmessage).not.toHaveBeenCalled(); + + await transport['processIncomingEvent'](asDeliveredByRelay(request)); + expect(onmessage).toHaveBeenCalledTimes(1); + }); + + it('still processes distinct unencrypted requests with identical payloads', async () => { + const transport = new NostrServerTransport({ + signer: new PrivateKeySigner('1'.repeat(64)), + relayHandler: makeCountingRelayHandler({ publishCalls: 0 }), + encryptionMode: EncryptionMode.DISABLED, + }); + const onmessage = mock(() => {}); + transport.onmessage = onmessage; + + // Same client, same JSON-RPC payload, new signed event (e.g. a retry). + const clientSk = generateSecretKey(); + await transport['processIncomingEvent']( + asDeliveredByRelay(makeUnencryptedRequest(1, clientSk)), + ); + await transport['processIncomingEvent']( + asDeliveredByRelay(makeUnencryptedRequest(1, clientSk, 2)), + ); + + expect(onmessage).toHaveBeenCalledTimes(2); + }); + it('accepts ephemeral gift wrap envelopes (21059) when encryption is required', async () => { const counter = { publishCalls: 0 }; diff --git a/src/transport/nostr-server/event-pipeline.ts b/src/transport/nostr-server/event-pipeline.ts index a977c0d..0f185d7 100644 --- a/src/transport/nostr-server/event-pipeline.ts +++ b/src/transport/nostr-server/event-pipeline.ts @@ -169,6 +169,17 @@ export class ServerEventPipeline { ); return null; } + + // Each relay delivers its own copy of a request. Mark only after the + // signature check so a bad-signature copy cannot suppress the real one. + if (this.deps.seenEventIds.has(event.id)) { + this.deps.logger.debug('Skipping duplicate unencrypted event', { + eventId: event.id, + }); + return null; + } + this.deps.seenEventIds.set(event.id, true); + return { event, isEncrypted: false }; } } diff --git a/src/transport/nostr-transport-deduplication.test.ts b/src/transport/nostr-transport-deduplication.test.ts index 2d538b1..24a6272 100644 --- a/src/transport/nostr-transport-deduplication.test.ts +++ b/src/transport/nostr-transport-deduplication.test.ts @@ -207,6 +207,51 @@ describe('gift-wrap pre-decrypt deduplication', () => { expect(received).toHaveLength(1); }); + test('client: does not let a bad-signature copy suppress the genuine plain event', async () => { + const serverSk = generateSecretKey(); + const serverPubkey = getPublicKey(serverSk); + const clientPriv = '1'.repeat(64); + + const transport = new NostrClientTransport({ + signer: new PrivateKeySigner(clientPriv), + relayHandler: makeNoopRelayHandler(), + serverPubkey, + encryptionMode: EncryptionMode.DISABLED, + }); + + const received: unknown[] = []; + transport.onmessage = (msg) => received.push(msg); + + const plainEvent = finalizeEvent( + { + kind: 25910, + created_at: 1, + tags: [ + ['p', getPublicKey(Uint8Array.from(Buffer.from(clientPriv, 'hex')))], + ], + content: JSON.stringify({ + jsonrpc: '2.0', + method: 'notifications/test', + }), + }, + serverSk, + ); + + // A relay delivers a fresh JSON object; a spread copy would also carry + // nostr-tools' cached verification flag. + const asDeliveredByRelay = (event: NostrEvent): NostrEvent => + JSON.parse(JSON.stringify(event)) as NostrEvent; + + await transport['processIncomingEvent']({ + ...asDeliveredByRelay(plainEvent), + sig: '0'.repeat(128), + }); + expect(received).toHaveLength(0); + + await transport['processIncomingEvent'](asDeliveredByRelay(plainEvent)); + expect(received).toHaveLength(1); + }); + test('client: processes a decrypted inner event only once even if delivered in multiple gift-wrap envelopes', async () => { decryptCallCount = 0;