mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-11-06 12:52:26 +00:00
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 <[email protected]>
This commit is contained in:
1 parent
dd1ce113c1
commit
07b42788d5
2 files changed
+82
-5
No files matched your search
@@ -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.
|
||||
@@ -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 "<field>"` —
|
||||
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`).
|
||||
|
||||
Reference in new issue
Block a user