Fix/dedupe inbound requests

.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.

.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.

.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.

src/payments/client-payments.test.ts

@@ -854,6 +854,7 @@ describe('withClientPayments()', () => {
       handlers: [{ pmi: 'fake', async handle(): Promise<void> {} }],
       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);

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;
         }

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({

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 };
   }
 

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 };
 

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 };
   }
 }

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;