From 07b42788d5ca0d61fad02fc96685de8aa291fc68 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Mon, 22 Jun 2026 22:26:07 -0400 Subject: [PATCH] docs: zap-audit plan + capture cross-session learnings in TODO - docs/zap-audit-plan.md: architecture map, findings (LNURL CORS send-error, comment box confirmed working, popover mis-position), and prioritized next steps (CORS/LNURL proxy decision first, then popover, fee-split correctness, receipt validation, NWC UX). Notes the injectable-fetcher/deps test seams. - TODO #3 points to the doc; #9 adds two unverified learnings: confirm the community-blacklist batching actually dropped the kind-30000 storm (still saw =32 in a later load), and trace the `from_hex` malformed-hex source via the new sanitizer dev-log. Planning/docs only. Co-Authored-By: Claude Opus 4.8 --- docs/zap-audit-plan.md | 62 ++++++++++++++++++++++++++++++ todo/TODO_2026-06-20_pre-launch.md | 25 +++++++++--- 2 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 docs/zap-audit-plan.md diff --git a/docs/zap-audit-plan.md b/docs/zap-audit-plan.md new file mode 100644 index 00000000..b8c51cfa --- /dev/null +++ b/docs/zap-audit-plan.md @@ -0,0 +1,62 @@ +# 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 +- **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`. + +## To do (next session, in priority order) +1. **CORS / LNURL proxy decision (the real reliability fix).** A static client + can't fetch LNURL endpoints that omit CORS headers, so zaps to many creators + fail. Options: (a) a small server-side/edge proxy (Vercel function) that fetches + the LNURL pay-service data + invoice and returns it with CORS — what most web + wallets do; (b) document the limitation and only support CORS-enabled hosts; + (c) route via the connected NWC wallet if it can resolve LN addresses. Decide, + then implement. Confirm whether `unitypay.cash` is also just offline. +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. diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index 276edc0c..20f82d3c 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -115,11 +115,17 @@ side action bar). Both share `js/ui/overlay/popoverEngine.js`. `left`, `top`, and its DOM parent (is it under `#uiOverlay` at body level or still nested in the modal?). -### 3. Zaps system + platform-fee zap split -- [ ] Audit the zap flow (NWC / `nwcClient.js`, `zapController.js`, `zapReceiptValidator.js`). -- [ ] Verify the platform-fee split: correct recipients, correct percentages, rounding, - and that the fee can't silently swallow the whole zap or be bypassed. -- [ ] Confirm zap-receipt validation (kind 9735) is accurate before crediting. +### 3. Zaps system + platform-fee zap split — full plan in docs/zap-audit-plan.md +Audit started 2026-06-23. See the doc for architecture map + findings. Summary: +- [x] **Send-error softened** (`dd1ce113`): recipient LNURL unreachable/CORS now + shows a clear message, not raw "Failed to fetch". +- [ ] **CORS / LNURL proxy decision** (the real reliability fix — static client + can't fetch CORS-less LNURL hosts; many zaps fail). Top of the next session. +- [ ] **Popover positioning (3a)**, **platform-fee split correctness** (incl. the + earlier "fee landed in my own wallet" report + self-zap edge), **receipt + validation (9735)**, **NWC budget/retry UX**, **general clunkiness**. +- [x] Comment box confirmed WORKING (the "message doesn't work" was the popover + mis-position making it hard to use — see 3a). ### 4. View counter accuracy & reliability - [ ] Audit the view-counter system (`viewEvents.js`, `reactionCounter.js`) for @@ -170,6 +176,15 @@ un-hides when WebTorrent flips green). The real gaps: kind-30000 REQ *per curator* (~28-32 parallel) at cold start. Collapsed into a single batched multi-author REQ (`js/adminListBatch.js`), routed through the subscription manager. This was the dominant `kind 30000` storm. +- [ ] **VERIFY the batching actually reduced it.** A later logged-OUT load + (2026-06-23 00:45) STILL showed `kind 30000=32` — either that build wasn't + loaded, or there's ANOTHER kind-30000 source besides + `loadCommunityBlacklistEntries` (admin editor/whitelist/blacklist lists? a + per-author follow-set fetch?). Re-check on a fresh hard-refresh; if still ~32, + trace the actual emitter. +- [ ] **`from_hex` source not root-caused.** The sanitizer (`733cb593`) defends + + dev-logs `[toolkit] dropped N invalid hex value(s) from filter ""` — + watch that log to find which subscription ships the odd-length hex, fix at source. - [x] **Relay `from_hex` REQ rejects fixed** (`733cb593`): one odd-length/non-hex value in `ids/authors/#e/#p/#q` made strict relays reject a whole REQ; now sanitized at the pool choke point (`normalizeFilterList`).