Address PR #62 review feedback: NWC relay security, N+1 zap queries, manual-payment confirmation

Addresses Copilot review comments on PR #62:

- nwc.ts: reject plaintext ws:// Wallet Connect relays, requiring wss://.
  Every request is signed by the connection secret (a private key); even
  with an encrypted payload, an unencrypted transport still leaks metadata
  about the connection and admits tampering.
- ZapButton/useZaps: fixed the N+1 query pattern — a feed page mounted one
  unconditional zap-receipts query (limit 500) per rendered note. Added a
  `revealed` gate so the query only fires once a note is actually hovered
  or focused (the same interaction that already reveals the action row via
  CSS), confirmed live: 0 queries fired across 49 mounted notes before any
  interaction, exactly 1 after hovering one.
- ZapDialog: manual-payment confirmation compared the note's total receipt
  *count* against a baseline, so anyone else zapping the same note while
  the dialog waited would falsely confirm the viewer's own unpaid invoice.
  Added `hasValidReceiptForInvoice` to match against the specific invoice
  instead, with unit tests covering the exact race the review described.
- useZaps.ts: corrected a docstring overclaiming that receipt validation
  prevents "forged" receipts from inflating totals — it only rules out
  structurally invalid data; NIP-57 receipts are vouched for by the
  recipient's own LNURL server, so trusting one is inherent to the
  protocol, not something client-side validation can prove.

Investigated but did not change: the review's claim that
`nip04.decrypt(...)` needs an `await` because it returns a Promise. Not
correct for this project's actual `nostr-tools` dependency — confirmed by
running the real encrypt/decrypt round trip, `decrypt` is synchronous and
returns the plaintext string directly, so the existing `JSON.parse` call
already worked.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BYiUtZMQeA5RHggQw73wto

src/components/nostr/NoteCard.tsx

@@ -1,3 +1,4 @@
+import { useState } from 'react';
 import { MessageSquare, Repeat2 } from 'lucide-react';
 import type { NostrEvent } from '@nostrify/nostrify';
 import { nip19 } from 'nostr-tools';
@@ -26,6 +27,10 @@ export function NoteCard({ event, compact, className }: NoteCardProps) {
   const { openApp } = useWindowManager();
   const { toast } = useToast();
   const hints = useRelayHints();
+  // Tracks the same interaction that reveals the action row via CSS
+  // (`group-hover`/`focus-within`), so `ZapButton` can defer its relay query
+  // until this note is actually looked at instead of firing on every mount.
+  const [revealed, setRevealed] = useState(false);
 
   const copyLink = async () => {
     try {
@@ -45,6 +50,8 @@ export function NoteCard({ event, compact, className }: NoteCardProps) {
         'group border-b border-border px-4 py-3 transition-colors last:border-b-0 hover:bg-muted/40',
         className,
       )}
+      onMouseEnter={() => setRevealed(true)}
+      onFocus={() => setRevealed(true)}
     >
       <AuthorLine pubkey={event.pubkey} createdAt={event.created_at} />
 
@@ -71,7 +78,7 @@ export function NoteCard({ event, compact, className }: NoteCardProps) {
               <Repeat2 className="size-3.5" aria-hidden />
               Copy link
             </Button>
-            <ZapButton target={event} />
+            <ZapButton target={event} revealed={revealed} />
             <BookmarkButton target={{ type: 'e', value: event.id }} />
           </div>
         )}

src/components/nostr/ZapButton.tsx

@@ -9,12 +9,24 @@ import { useAuthor } from '@/hooks/useAuthor';
 import { useZapReceipts, summarizeZapReceipts, formatSats } from '@/hooks/useZaps';
 import { cn } from '@/lib/utils';
 
-/** Shows a note's zap total and opens the zap flow (NIP-57), from the feed or a thread. */
-export function ZapButton({ target, className }: { target: NostrEvent; className?: string }) {
+/**
+ * Shows a note's zap total and opens the zap flow (NIP-57), from the feed or
+ * a thread.
+ *
+ * `revealed` gates the zap-receipts query itself, not just the total's
+ * visibility: a feed page mounts one of these per note, and firing every
+ * one's relay query unconditionally on mount turns a page of notes into a
+ * page of concurrent queries before anyone's looked at any of them. Callers
+ * in a list (`NoteCard`) pass `revealed` once the row is actually hovered or
+ * focused — the same interaction that already reveals the row via CSS — so
+ * off-screen or never-looked-at notes never fetch. A caller showing the
+ * button on its own (the thread root) can just leave it `true`.
+ */
+export function ZapButton({ target, className, revealed = true }: { target: NostrEvent; className?: string; revealed?: boolean }) {
   const { user } = useCurrentUser();
   const [authOpen, setAuthOpen] = useState(false);
   const [zapOpen, setZapOpen] = useState(false);
-  const receipts = useZapReceipts(target.id);
+  const receipts = useZapReceipts(target.id, { enabled: revealed || zapOpen });
   const recipient = useAuthor(target.pubkey);
 
   const { totalSats } = summarizeZapReceipts(receipts.data);

src/components/nostr/ZapDialog.tsx

@@ -7,7 +7,7 @@ import { Textarea } from '@/components/ui/textarea';
 import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription } from '@/components/ui/dialog';
 import { QRCodeCanvas } from '@/components/ui/qrcode';
 import { useToast } from '@/hooks/useToast';
-import { useCreateZapInvoice, useZapReceipts, summarizeZapReceipts, formatSats } from '@/hooks/useZaps';
+import { useCreateZapInvoice, useZapReceipts, hasValidReceiptForInvoice, formatSats } from '@/hooks/useZaps';
 import { useNwcConnection, usePayWithNwc } from '@/hooks/useNwc';
 import { cn } from '@/lib/utils';
 
@@ -38,14 +38,15 @@ export function ZapDialog({ open, onClose, target, recipientMetadata, recipientM
   const [invoice, setInvoice] = useState<string | null>(null);
   const [amountSats, setAmountSats] = useState(0);
   const [errorMessage, setErrorMessage] = useState('');
-  const [receiptBaseline, setReceiptBaseline] = useState(0);
 
   // Re-fetched on an interval only while this dialog is showing an unpaid
   // manual invoice. Whether that now means "paid" is derived at render time
   // below instead of copied into state, so there is nothing to keep in sync
-  // by hand.
+  // by hand. Matched against the specific invoice, not just a receipt-count
+  // increase — someone else zapping the same note while this dialog waits
+  // must not be mistaken for this payment completing.
   const receipts = useZapReceipts(target.id, { refetchInterval: stage === 'manual' ? 4_000 : false });
-  const manualPaymentConfirmed = stage === 'manual' && summarizeZapReceipts(receipts.data).count > receiptBaseline;
+  const manualPaymentConfirmed = stage === 'manual' && invoice !== null && hasValidReceiptForInvoice(receipts.data, invoice);
   const effectiveStage: Stage = manualPaymentConfirmed ? 'paid' : stage;
 
   const resolvedAmount = customAmount.trim() ? Number(customAmount) : amount;
@@ -57,9 +58,6 @@ export function ZapDialog({ open, onClose, target, recipientMetadata, recipientM
     setErrorMessage('');
 
     try {
-      const { count } = summarizeZapReceipts(receipts.data);
-      setReceiptBaseline(count);
-
       const result = await createInvoice.mutateAsync({
         target,
         recipientMetadata,

src/hooks/useZaps.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it } from 'vitest';
 import { finalizeEvent, generateSecretKey, getPublicKey, nip57 } from 'nostr-tools';
 import type { NostrEvent } from '@nostrify/nostrify';
-import { summarizeZapReceipts, formatSats } from './useZaps';
+import { summarizeZapReceipts, hasValidReceiptForInvoice, formatSats } from './useZaps';
 
 const targetId = 'a'.repeat(64);
 // A syntactically valid-enough bolt11 for getSatoshisAmountFromBolt11: "lnbc"
@@ -87,3 +87,26 @@ describe('summarizeZapReceipts', () => {
     expect(summary.count).toBe(1);
   });
 });
+
+describe('hasValidReceiptForInvoice', () => {
+  const MY_INVOICE = `lnbc210n1${'q'.repeat(45)}`;
+
+  it('returns false for no receipts', () => {
+    expect(hasValidReceiptForInvoice(undefined, MY_INVOICE)).toBe(false);
+  });
+
+  it('matches a valid receipt paying the exact invoice', () => {
+    const mine = receipt({ id: '1', tags: [['e', targetId], ['bolt11', MY_INVOICE], ['description', signedZapRequest(21_000).json]] });
+    expect(hasValidReceiptForInvoice([mine], MY_INVOICE)).toBe(true);
+  });
+
+  it('ignores a receipt for a different invoice — someone else zapping the same note must not confirm this payment', () => {
+    const someoneElses = receipt({ id: '1' }); // uses the default FAKE_BOLT11, not MY_INVOICE
+    expect(hasValidReceiptForInvoice([someoneElses], MY_INVOICE)).toBe(false);
+  });
+
+  it('ignores a receipt matching the invoice but with an invalid description', () => {
+    const forged = receipt({ id: '1', tags: [['e', targetId], ['bolt11', MY_INVOICE], ['description', 'not json']] });
+    expect(hasValidReceiptForInvoice([forged], MY_INVOICE)).toBe(false);
+  });
+});

src/hooks/useZaps.ts

@@ -12,13 +12,19 @@ function zapReceiptsQueryKey(eventId: string) {
   return ['nostr', 'zap-receipts', eventId] as const;
 }
 
-/** NIP-57 zap receipts (kind 9735) referencing `eventId`. */
-export function useZapReceipts(eventId: string | undefined, opts?: { refetchInterval?: number | false }) {
+/**
+ * NIP-57 zap receipts (kind 9735) referencing `eventId`. Pass `enabled: false`
+ * until the caller actually needs the total — a note list mounts one of these
+ * per row, and firing all of them unconditionally turns a page of notes into
+ * a page of relay queries (an N+1 pattern) before anyone's looked at any of
+ * them.
+ */
+export function useZapReceipts(eventId: string | undefined, opts?: { refetchInterval?: number | false; enabled?: boolean }) {
   const { nostr } = useNostr();
 
   return useQuery<NostrEvent[]>({
     queryKey: zapReceiptsQueryKey(eventId ?? ''),
-    enabled: Boolean(eventId),
+    enabled: Boolean(eventId) && (opts?.enabled ?? true),
     queryFn: async ({ signal }) => {
       const events = await nostr.query(
         [{ kinds: [ZAP_RECEIPT_KIND], '#e': [eventId!], limit: 500 }],
@@ -39,9 +45,12 @@ export interface ZapSummary {
 /**
  * Sums zap receipts defensively: a receipt only counts if it carries a
  * `bolt11` invoice and a `description` whose embedded zap request is a
- * well-formed, signature-valid Nostr event. Anything else — a malformed or
- * forged receipt a relay happens to serve back — is dropped rather than
- * inflating the total.
+ * well-formed, signature-valid Nostr event. That rules out garbage a relay
+ * happens to serve back — malformed data, or a request signature that
+ * doesn't check out — but it is not proof any payment actually happened.
+ * NIP-57 receipts are published by the recipient's own LNURL server, so
+ * trusting that a receipt means "paid" is inherent to the protocol; this
+ * validation only keeps structurally-invalid noise out of the total.
  */
 export function summarizeZapReceipts(events: NostrEvent[] | undefined): ZapSummary {
   const seen = new Set<string>();
@@ -67,6 +76,21 @@ export function summarizeZapReceipts(events: NostrEvent[] | undefined): ZapSumma
   return { totalSats, count };
 }
 
+/**
+ * True if any signature-valid receipt among `events` pays exactly `invoice`.
+ * Used to confirm a specific manual payment — checking whether the note's
+ * receipt *count* went up is not enough, since anyone else zapping the same
+ * note while the payer is waiting would also bump the count and falsely
+ * confirm a still-unpaid invoice.
+ */
+export function hasValidReceiptForInvoice(events: NostrEvent[] | undefined, invoice: string): boolean {
+  return (events ?? []).some((receipt) => {
+    const bolt11 = tagValue(receipt, 'bolt11');
+    const description = tagValue(receipt, 'description');
+    return bolt11 === invoice && Boolean(description) && nip57.validateZapRequest(description!) === null;
+  });
+}
+
 /** "1.2K" for 1234 — compact, locale-aware, and never wraps a note's action row. */
 export function formatSats(sats: number): string {
   return new Intl.NumberFormat(undefined, { notation: 'compact', maximumFractionDigits: 1 }).format(sats);

src/lib/nwc.test.ts

@@ -39,6 +39,11 @@ describe('parseNwcUri', () => {
     expect(() => parseNwcUri(uri)).toThrow();
   });
 
+  it('rejects a plaintext ws:// relay', () => {
+    const uri = `nostr+walletconnect://${PUBKEY}?relay=ws%3A%2F%2Frelay.example.com&secret=${SECRET}`;
+    expect(() => parseNwcUri(uri)).toThrow();
+  });
+
   it('lowercases hex fields', () => {
     const uri = `nostr+walletconnect://${PUBKEY.toUpperCase()}?relay=wss%3A%2F%2Frelay.example.com&secret=${SECRET.toUpperCase()}`;
     const connection = parseNwcUri(uri);

src/lib/nwc.ts

@@ -37,8 +37,12 @@ export function parseNwcUri(uri: string): NwcConnection {
   if (!HEX_64.test(parsed.pubkey) || !HEX_64.test(parsed.secret)) {
     throw new Error('That Wallet Connect link is missing a valid key.');
   }
-  if (!parsed.relay.startsWith('wss://') && !parsed.relay.startsWith('ws://')) {
-    throw new Error('That Wallet Connect link has an invalid relay.');
+  // wss:// only: every request to the wallet is signed by `secret` (a private
+  // key) and, even though the payload is encrypted, an unencrypted `ws://`
+  // transport still leaks metadata about the connection (who you're paying,
+  // when, how often) to anyone on the network path, and admits tampering.
+  if (!parsed.relay.startsWith('wss://')) {
+    throw new Error('That Wallet Connect link must use a wss:// relay.');
   }
 
   return { pubkey: parsed.pubkey.toLowerCase(), relay: parsed.relay, secret: parsed.secret.toLowerCase() };