bitvid/docs/zap-audit-plan.md

Zap System Audit — Plan & Findings

Status: in progress (audit started 2026-06-23). Branch: unstable. Pre-launch priority #3. The zap flow feels clunky and errors on send.

Architecture (map for the next session)

• Two controllers: js/ui/components/video-modal/zapController.js (the in-modal UI — button, popover, comment/amount inputs; dispatches video:zap / zap:open) and js/ui/zapController.js (the logic — open(), sendZap(), retry, receipts). ModalManager wires video:zap → zapController.sendZap.
• Payment flow: sendZap → splitAndZap (js/payments/zapSplit.js) → per share (creator, platform) processShare → lnurl.fetchPayServiceData → buildZapRequest (kind 9734, comment in content) → lnurl.requestInvoice (nostr= zapRequest + optional comment= param) → wallet.sendPayment (NWC, nwcClient.js) → zapReceiptValidator (kind 9735).
• Fee split: PLATFORM_FEE_PERCENT (instance-config, currently 30) → platformShare = floor(amount*fee/100), creatorShare = amount - platformShare; platform address via platformAddress.js / PLATFORM_LUD16_OVERRIDE.

Findings so far

2026-06-23 session 3 (live-test: successful zap reported as error)

• A successful zap was shown as a red error (FIXED, item #4). Live test: zapping [email protected] (CORS-friendly) actually PAID — the platform fee landed — but bitvid showed "Awaiting validated zap receipt… No zap receipt was published on the advertised relays." Root cause: a share whose PAYMENT succeeded (NWC returned a preimage) but whose kind-9735 receipt couldn't be validated was routed through notifyError as a warning. The preimage is proof the invoice was paid; the 9735 is supplementary and a static client commonly can't see it (relay coverage/timing/late publish). Fixed (js/ui/zapController.js): paid-but-unvalidated is now a SUCCESS with a soft note ("Payment went through; couldn't confirm the zap receipt on relays"). Genuine payment failures (status !== success) still go through the failure/retry path. Spec-corrected test in tests/video-modal-zap.test.mjs (see TEST_INTEGRITY.md). RECEIPT DISCOVERY FIXED (item #4, 2026-06-23): the validator queried receipts with { kinds:[9735], "#bolt11":[invoice] } — but most relays DON'T index arbitrary tags like bolt11, so the query returned nothing even when the receipt was published. New buildReceiptFilters (js/payments/zapReceiptValidator.js) queries by the reliably-indexed #e (event) / #a (coordinate) + #p (recipient) tags from the zap request, falling back to #bolt11 only if no anchor exists; the precise match (author pubkey + bolt11 + description hash) still confirms the right receipt. Tests in tests/zap-receipt-pool.test.mjs + spec-corrected tests/zap-split.test.mjs. TIMING GAP CLOSED (2026-06-23): the validator now POLLS for the receipt — up to 3 lookups (default), ~1.2s apart — so a receipt the recipient's service publishes a few seconds after EOSE is still caught. Stops as soon as it's found (no delay in the common case); attempts/delay/sleep are injectable (receiptLookupAttempts, receiptLookupDelayMs, sleep) and tests stub sleep so they don't wait. Reason now distinguishes "no events at all" vs "events but none matched". Tests: testValidateZapReceiptRetriesForLateReceipt (found on 3rd poll) + testValidateZapReceiptGivesUpAfterRetries in tests/zap-split.test.mjs. NOTE: in the no-receipt case this adds ~2.4s before the (success-with-note) status shows; acceptable for a payment confirmation. A fully background validate-and-upgrade flow remains a possible future refinement.
• LOG NOISE seen during the live test (pre-existing, separate items): cold-start RELAY STORM (~28 REQ/s, item #9); bad req: uneven size input to from_hex (odd-length hex reaching a relay — the sanitizer #9 should catch this; re-confirm); watch-history bucketing items with watchedAt:0 into a 1970-01 month (minor watch-history bug).

2026-06-23 session 2 (live-test fixes)

• Couldn't type in the zap comment box (FIXED, real bug). The shared popover engine treats every panel as an ARIA menu and preventDefault'd single-character
  • arrow keydowns → every keystroke in the comment/amount fields was swallowed. handleMenuKeydown now ignores events from editable targets (input/textarea/select/contenteditable/role=textbox). Regression test in tests/ui/popoverEngine.test.mjs.
• NWC wallet didn't restore "like storage" (FIXED, real bug). The "already offered" flag in settingsRestorePrompt.js was per-PUBKEY, so the first item offered (storage) flagged the whole account and the wallet note was never offered afterward. Now tracked PER-ITEM. Decision (user, 2026-06-23): keep the one-time CONFIRM for the wallet (it's a spending secret) — not silent auto-restore. Regression test in tests/settings-restore-prompt.test.mjs.
• "Click the Zap button many times before it opens" (FIX — needs live confirm). A click during an in-flight open canceled it (pendingToggle="close"), so rapid double-clicks toggled the popover back closed. Now an in-flight-open click is ignored unless the dialog is already visibly open. (video-modal/zapController.js)
• CORS confirmed for goblinbox (unitypay.cash/.well-known/lnurlp/goblinbox → net::ERR_FAILED). With the NWC fix in place, wallet.hasUri:true — the send fails purely at the LNURL fetch (CORS), not NWC. This is the data point for the proxy decision (item 1).

2026-06-23 session (fixes landed)

• NWC wallet connection was BROKEN (launch-blocker, FIXED). parseNwcUri (js/payments/nwcClient.js) passed the hex secret to nostr-tools getPublicKey, which in v2.x requires a Uint8Array → it threw expected Uint8Array and the whole NWC connect failed (NWC is the only zap payment path). Now converts hex→bytes (try hex, fall back to hexToBytesCompat). Real-tools regression test: tests/nwc-parse-uri.test.mjs. (The mock-based tests/nwc-client.test.mjs is still quarantined — its nostr-tools mock is shadowed by the frozen canonical toolkit; needs a harness rework, but the bug itself is fixed + guarded.)
• In-flight "Sending… sats" status was mis-toned as warning (FIXED). A pending state was colored text-warning-strong (alarming) and counted as a warning. Now neutral. (js/ui/zapController.js)
• Success path didn't fully reset the form (FIXED). The terminal success path cleared only the comment (setZapComment) while the validation-warning path used the holistic resetZapForm. Unified to resetZapForm({amount:"",comment:""}) so amount + comment reset consistently after a send. (js/ui/zapController.js)
• Fee-fallback is no-bypass (verified). resolvePlatformFeePercent falls back to the configured PLATFORM_FEE_PERCENT (30) on an unparseable override — a junk override can't silently disable the fee. (Stale test that expected 0 corrected.)
• 3a popover — FIXED separately (see TODO 3a); the in-modal zap dialog now portals to #uiOverlay so it positions bottom-end under the button.

Earlier findings

• Send error = recipient LNURL unreachable (CORS/offline). [PARTLY FIXED] Sending to a creator whose LNURL host is down or lacks CORS headers (observed: unitypay.cash/.well-known/lnurlp/goblinbox) makes the browser fetch reject with "Failed to fetch". Shipped (dd1ce113): fetchPayServiceData now throws a clear, coded (lnurl-unreachable) message. Root issue remains: a static client CANNOT fetch CORS-less LNURL hosts — see "CORS proxy" below.
• Comment box works. Confirmed via screenshot — the textarea renders and binds; the comment is carried in the zap-request content + the LNURL comment param. The earlier "message doesn't work" was the popover being mis-positioned/hard to use (see 3a), not a comment bug.
• Popover mis-positioning (zap + embed) — see TODO 3a; shared popoverEngine.

Status: CLOSED 2026-06-24

All audit items below are resolved:

1. CORS / LNURL — DECIDED: accept graceful degradation, keep the visual zappability flag, no proxy (breaks static rule + invoice-swap risk; WebLN's only bypass pays directly with no receipt/split). See TODO #3.
2. Popover positioning (3a) — FIXED (portaled to #uiOverlay).
3. Platform-fee split correctness — AUDITED + FIXED. Rounding never loses sats (creator gets the remainder); sub-1-sat fee → 0 and the whole zap goes to the creator; fee clampPercent-bounded and junk override falls back to default; creator==platform collapses to one full-amount payment (was double-paying). "Fee landed in my own wallet" is expected (sender is the platform operator, or self-zap), not a bug. Tests in tests/zap-split.test.mjs.
4. Receipt validation (9735) — FIXED (author + bolt11 + description-hash match, polling for late receipts). tests/zap-receipt-pool.test.mjs.
5. NWC budget/retry UX — actionable budget-exhausted message + remaining-share summary + Retry button.
6. General clunkiness — maintainer confirms the flow works well; accepted.

To do (historical — completed)

1. CORS / LNURL proxy — DECISION: re-test first (2026-06-23). Because the NWC connection was fully broken (hex secret → getPublicKey; fixed this session), an unknown share of the observed "send errors" were the NWC bug, NOT CORS. Plan: deploy the NWC fix, run real zaps, and measure the actual CORS failure rate before committing to a proxy. THEN choose: (a) small Vercel edge proxy that fetches LNURL pay-data + invoice and returns with CORS [recommended if the rate is high — what web wallets do]; (b) document the limit / CORS-only hosts; (c) route LN-address resolution via the connected NWC wallet. Also confirm whether unitypay.cash is simply offline vs. CORS-less.
2. Popover positioning (3a). Fix the shared engine safely (portal in-modal panels out to #uiOverlay, or remove the modal-subtree containing block). Needs the live computed position/left/top + DOM parent of #modalZapDialog. VERIFY non-modal popovers still position right after.
3. Platform-fee split correctness. Verify recipients/percentages/rounding; ensure the fee can't swallow the whole zap, be bypassed, or (earlier report) land the platform fee in the sender's own wallet when zapping certain targets. Check the self-zap / creator==platform edge case. Add cheat-resistant tests.
4. Receipt validation (kind 9735). Confirm accuracy before crediting; the unvalidatedReceipts/failureReceipts paths and the "Awaiting validated zap receipt" state. (Earlier fix: zapReceiptValidator SimplePool .list shim.)
5. NWC error UX. "Budget exceeded" and partial-share retry flow — make the messaging + retry obviously usable (it's currently terse).
6. General clunkiness. Step back on the whole flow: amount presets, the split summary, success/partial/failure states, focus, and mobile.

Cheat-resistant testing notes

• fetchPayServiceData / requestInvoice accept an injectable fetcher — test failure modes by injecting rejecting/erroring fetchers (see tests/lnurl-fetch-error.test.mjs).
• splitAndZap takes injectable dependencies (see tests/zap-split.test.mjs) — use it to assert fee math, rounding, and recipient routing without real LN.