From 4b927599131dd93d03ee6b56eec64ef6d156b330 Mon Sep 17 00:00:00 2001 From: SkateSpots Date: Fri, 9 Oct 2026 21:35:06 +0700 Subject: [PATCH] fix(marketplace): close P0 trade/offer bugs + deep flow audit Deep audit (MARKETPLACE_FLOW_AUDIT.md): full trade/escrow/offers lifecycle, severity-ranked findings, and a state-coverage matrix. Fixes: - Grid Mark as sold now opens MarkSoldDialog, so the kind-31871 attestation (buyer, method) is recorded from the grid too, not just the detail page. - Offers accept attaches a payment pointer only when the accepting user is the listing owner (seller); previously the buyer could publish their own lud16 as the payee and misdirect payment. - advertiseEscrowKey refuses to publish kind 0 before the profile loads, so it cannot overwrite name/picture/lud16 with a cashu_escrow-only profile. - useListingOffers passes requireAny, so the offers panel error state actually fires on an outage (previously an outage looked like no offers). - Offer messages invalidate the offer queries, so accept/counter/decline update immediately instead of after the stale window. --- AGENTS.md | 5 +- MARKETPLACE_FLOW_AUDIT.md | 196 ++++++++++++++++++++++++++ src/components/GearSwapItemCard.tsx | 26 ++-- src/components/ListingOffersPanel.tsx | 8 +- src/hooks/useMarketplaceOffers.ts | 10 +- src/hooks/wallet/useCashuEscrow.ts | 10 +- 6 files changed, 231 insertions(+), 24 deletions(-) create mode 100644 MARKETPLACE_FLOW_AUDIT.md diff --git a/AGENTS.md b/AGENTS.md index 5e5da30..bba6760 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2102,8 +2102,9 @@ Rules: - `/src/contexts/`: React context providers (AppContext, LanguageContext, NWCContext) - `NIP.md`: **source of truth** for every custom/adopted Nostr kind and its schema - `AGENTS.md` (this file): the only cross-session memory — keep it current -- `UX_AUDIT.md` (pre-refactor, partly stale) and `UX_AUDIT_2026-10-09.md` - (current app-wide + marketplace UI/UX audit, with a recommended fix order), +- `UX_AUDIT.md` (pre-refactor, partly stale), `UX_AUDIT_2026-10-09.md` + (app-wide + marketplace UI/UX audit), and `MARKETPLACE_FLOW_AUDIT.md` (the deep + trade/escrow/offers/error audit, with severity-ranked findings and a fix order), `GROUP_MESSAGING.md`, `ESCROW.md`: feature decision/backlog docs - `/src/test/`: Testing utilities including TestApp component - `/public/`: Static assets diff --git a/MARKETPLACE_FLOW_AUDIT.md b/MARKETPLACE_FLOW_AUDIT.md new file mode 100644 index 0000000..6cbd2c5 --- /dev/null +++ b/MARKETPLACE_FLOW_AUDIT.md @@ -0,0 +1,196 @@ +# Marketplace flow, escrow, and error audit — 2026-10-09 + +Companion to `UX_AUDIT_2026-10-09.md`. This is the deep, sequenced audit of the +marketplace: the full trade lifecycle, escrow, the offers/payment path, and every +error/empty/confusing state. It is meant to be the reference before touching the +marketplace. + +Method: three focused exploration passes (trade/escrow; offers/payment; error and +confusion matrix) against the source, plus direct verification of the highest +severity findings. Confidence legend: + +- **[verified]** — I read the code and confirmed it. +- **[reported]** — found by the exploration pass; confirm in source before acting. + +Severity: **P0** = money loss, data loss, or a wrong outcome; **P1** = flow +dead-end / stale or misleading state; **P2** = polish (outage copy, native-only +visibility, terminology, translation). + +--- + +## 1. The lifecycle, end to end + +1. **Browse** — `/marketplace` hub, Browse tab -> `GearSwapPage` grid + (`GearSwapItemCard`). +2. **Listing detail** — `/listing/:pubkey/:identifier` -> seller block, price, + `ListingTradeActions` (buy / trade / escrow), optional "Make an offer", + `ListingOffersPanel`, `TradeReputationCard`. +3. **Offer negotiation** (kind `30403`) — offer -> counter -> accept / decline / + withdraw; on accept a `payment` pointer may be attached; then in-app pay. +4. **Close the deal** — seller "Mark as sold" -> `MarkSoldDialog` (buyer, method, + amount) -> `markSold` (kind `30402` status=sold) + `publishTrade` (kind `31871`). +5. **Mutual reputation** — buyer "Confirm this trade" -> second `31871` with + optional payment proof -> `useTradeReputation`. +6. **Shipment** (kind `34967`) — seller "shipped", buyer "received"/"issue". +7. **Escrow** (native only) — buyer locks Cashu P2PK to the seller with a locktime + and buyer refund key; seller redeems; buyer refunds after locktime. + +--- + +## 2. P0 — money / data / correctness + +- **[verified] Offers "accept" attaches the wrong payee.** `respond` in + `ListingOffersPanel.tsx` (~line 95) attaches the *accepting user's* `lud16` as + the `payment` pointer with no role check. When the buyer accepts the seller's + counter, the buyer's address is published as the payee; `PayWithSats` is passed + `pointer = latest.payment` and `sellerPubkey = latest.seller` + (`ListingOffersPanel.tsx` ~332), so the pointer and the payee can be different + parties — the payment can be misdirected. **Fixed this pass** (only the listing + owner attaches a pointer). +- **[verified] `advertiseEscrowKey` could wipe the profile.** It published kind 0 + from `author.data?.metadata ?? {}` (`src/hooks/wallet/useCashuEscrow.ts:134`); + if the profile query had not resolved, it wrote a profile containing only + `cashu_escrow`, erasing name/picture/about/lud16/nip05. **Fixed this pass** + (refuses to publish until `author.data` has loaded). +- **[reported] Escrow funding can strand funds.** `EscrowRequestDialog.tsx:64-87` + mints and P2PK-locks the funds, then publishes the `31872`; if the publish + throws, the minted token exists only in a local variable, and a retry mints a + second escrow with no way to recover the first. Related: + `src/lib/cashuEscrow.ts:282-284` never persists the proofs. Native-only and + unverified (see `ESCROW.md`), but this is the worst-case path. +- **[reported] Buyer refund is unreachable on web.** `ListingTradeActions.tsx:196-208` + gates refund on `escrow.available` (native), so a web user holding an escrowed + token can see the `EscrowPanel` but cannot reclaim after locktime. +- **[reported] A sold listing can be left with no attestation.** `MarkSoldDialog.tsx:70-77` + runs `markSold` then `publishTrade` non-atomically; if the second fails (or is + queued offline, which resolves — `useNostrPublish.ts:77-89`), the listing is + sold but no `31871` exists, so no reputation and no confirmation path. + +--- + +## 3. P1 — flow dead-ends and misleading state + +- **[verified] Grid "Mark as sold" skipped the trade record.** `GearSwapItemCard` + called only `markSold`, so no `31871`/buyer/reputation came from the grid, + unlike the detail and dashboard. **Fixed this pass** (opens `MarkSoldDialog`). +- **[reported] Ask-seller is only when already sold** (`ListingTradeActions.tsx:254`): + a buyer who paid off-app while the listing is still active cannot ask the seller + to record it — a dead end until the seller acts. +- **[reported] `escrow = required` does not block Buy** (`ListingTradeActions.tsx:90-99`): + the badge shows but direct Buy-with-sats still proceeds. +- **[reported] Revocation is not reliably honored.** Attestations are deduped by + event id only (`useTradeAttestations.ts:224-226, 406-408`), so a stale `valid` + copy from one relay can outlive a newer `revoked`; `iConfirmed` can stay true. +- **[reported] `useTradeReputation` skips `onlyVerified`** (`:216-222`), unlike + `useTradeIntegrity`/`useShipmentStatus`, so a hostile relay can inject forged + reputation events. +- **[reported] Request back-reference is wrong.** `publishTrade` writes the + listing `a` coord as the `request` tag (`useTradeAttestations.ts:141`), but + NIP.md documents `31872::`; both call sites pass the coord + (`TradesPage.tsx:210`, `ConversationPage.tsx:193`). +- **[reported] Receipts are one global blob, keyed by listing coord, not + account-scoped** (`receipts.ts:64-71`), and `ListingOffersPanel` never reads + them back (`:74,335`), so the Pay button can reappear and a prior trade's + receipt can be attached to a later deal. +- **[reported] Pay path correctness:** unsupported pointer kinds (`creq`/`clink`) + render an enabled Pay button (`PayWithSats.tsx:117-138`); `pointerKind` is + trusted verbatim (`:55-63`); no BOLT11 expiry check (`bolt11.ts:37-67`); a + non-SATS `amount` set by another client is displayed as one currency and paid as + sats (`ListingOffersPanel.tsx:52` vs `usePayPaymentPointer.ts:70`). +- **[reported] Wallet reliability:** a normal NWC failure aborts with no WebLN/Cashu + fallback (`usePayPaymentPointer.ts:85-111`), unlike `useZaps`; an NWC + settled-after-timeout returns an empty preimage (`useNWC.ts:237`), silently + downgrading a would-be verifiable receipt. +- **[verified] Offers never refreshed after a message.** `publishMessage` did not + invalidate the offer queries, so accept/counter/decline lagged behind the 60s + stale window. **Fixed this pass.** +- **[reported] Offer UI confusions:** every offer/counter is `SATS` with no + currency selector (`OfferDialog.tsx:54`, `useMarketplaceOffers.ts:264`); the + counter rejects note-only while the offer allows it (`ListingOffersPanel.tsx:141` + vs `OfferDialog.tsx:48`); decline/withdraw default their content to the other + party's previous note (`useMarketplaceOffers.ts:355`); the accept button never + restates the amount being accepted. + +--- + +## 4. P2 — outage-as-empty, native-only, terminology, i18n + +Outage renders as truth (a relay failure looks like "none"): + +- `useTradeReputation` -> `TradeReputationCard` returns null, `TradeReputationPanel` + shows the empty state, `TradesPage` shows "No trades". +- `usePendingTradeConfirmations` / `useIncomingTradeRequests` -> "Nothing waiting". +- `useListingTrade` / `useShipmentStatus` -> `ShippingStatusCard` shows nothing + shipped. +- `useSellerSales` catches to 0 -> every seller is "new seller"; with an active + trust filter, `useBatchTrustScores` zeros hide every listing. +- `useSellerInfo` -> null (drops account-age/verified signals); `useListingReports` + -> 0 (hides report warnings); `SellerDashboard` -> "no listings yet". +- `useListingOffers` now passes `requireAny` and `ListingOffersPanel` shows an + error — **fixed this pass** (previously an outage looked like "no offers"). + +Native-only, visible-but-dead on web: + +- escrow "required" badge on web (`MarketplaceItemCard.tsx:366`); the escrow action + and `EscrowRequestDialog` native-only warning are unreachable because the dialog + mounts only when `canEscrow` (`ListingTradeActions.tsx:388`); + `EscrowRequestDialog.tsx:134` is dead code; refund gated on `escrow.available`; + Buy-with-sats falls back to a native-only Cashu melt. + +Terminology drift: "Marketplace" / "Gear Swap" / "Trades" / "My Trades" used for +the same surfaces; "Want to Buy" vs "WTB"; "Sold" vs "SOLD"; pickup/ships/both +labelled from different keys; two `loadFailed` namespaces. + +Hardcoded and untranslated: raw `status.toUpperCase()` +(`ListingDetailPage`, `MarketplaceItemCard`), raw trade `state` (`TradesPage`), +raw offer `status` (`ListingOffersPanel`), raw currency codes +(`ListingOffersPanel`, `SellerDashboard`, `types.ts formatPrice`), raw price +`frequency`, raw shipping value (`GearSwapItemCard`), hardcoded "PoW", +`TRADE_METHODS` values, and the "Plebeian Market" promo page. + +--- + +## 5. State coverage matrix (summary) + +| Surface | loading | empty | error/outage | not-logged-in | own | sold/expired | native-only | +| --- | --- | --- | --- | --- | --- | --- | --- | +| GearSwap grid | yes | yes | yes | n/a | card menu | filtered silently | n/a | +| Listing detail | yes | yes | yes | partial | yes | sold/inactive; **no expired** | badge only | +| Hub tabs | no | no | no | selling/reputation | n/a | n/a | n/a | +| Seller dashboard | yes | yes | **missing** | hub gates | n/a | sold; expired as active | n/a | +| Trades page | yes | yes | **missing** | yes | n/a | n/a | n/a | +| Reputation card | **none** | **none** | **none** | n/a | n/a | n/a | n/a | +| Offers panel | no | no | fixed (now fires) | returns null | returns null | no state guard | n/a | +| Escrow panel | no | returns null | no | n/a | n/a | no trade tie | read-only on web | +| Shipping card | no | no | **missing** | returns null | seller/buyer | no sold/expired tie | n/a | + +--- + +## 6. Fixed in this pass + +1. Grid "Mark as sold" now opens `MarkSoldDialog` (records the `31871`). +2. Offers accept: only the listing owner attaches a payment pointer (payee fix). +3. `advertiseEscrowKey` refuses to publish kind 0 before the profile has loaded. +4. `useListingOffers` passes `requireAny`; the offers panel error state now fires. +5. Offer messages invalidate the offer queries (no more stale Pay button). + (Plus, earlier today: `useMarketplaceItem` moderation filter, offer CTA guards, + URL-addressable hub tabs, offers panel error branch.) + +## 7. Recommended order + +1. **P0 escrow safety:** make `EscrowRequestDialog` persist/attach the minted token + before publishing (or roll back and surface it) so a failed publish cannot + strand funds; persist the token so a reload cannot lose it. Native-only, but it + is real money. +2. **P0 payee/pointer hardening:** verify `PayWithSats` only offers payment when a + seller-authored pointer exists and the payee matches the payee's key; add the + BOLT11 expiry check and disable unsupported kinds before the click. +3. **P1 correctness:** attestation dedupe by author+kind+d (revocation), add + `onlyVerified` to `useTradeReputation`, honor `escrow=required`, allow + ask-seller on an active listing, make Mark-sold atomic or verify the attestation. +4. **P1 state:** account-scope and read back receipts; fix the `request` tag; + NWC fallback + empty-preimage handling. +5. **P2:** outage states everywhere, native-only explanation on web, terminology + consolidation, and the untranslated values. + +Items tagged **[reported]** should be re-confirmed in source before a fix. diff --git a/src/components/GearSwapItemCard.tsx b/src/components/GearSwapItemCard.tsx index 3933e6e..08e3157 100644 --- a/src/components/GearSwapItemCard.tsx +++ b/src/components/GearSwapItemCard.tsx @@ -22,6 +22,7 @@ import { MessageCircle, MapPin, Search, CheckCircle, ShieldCheck, AlertTriangle, import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, DropdownMenuSeparator } from '@/components/ui/dropdown-menu'; import { AlertDialog, AlertDialogAction, AlertDialogCancel, AlertDialogContent, AlertDialogDescription, AlertDialogFooter, AlertDialogHeader, AlertDialogTitle } from '@/components/ui/alert-dialog'; import { ReportDialog } from '@/components/ReportDialog'; +import { MarkSoldDialog } from '@/components/MarkSoldDialog'; import { formatDistanceToNow } from '@/lib/dateFnsI18n'; import { useTranslation } from '@/contexts/language'; @@ -41,13 +42,14 @@ export function GearSwapItemCard({ item, trustScore, salesCount }: GearSwapItemC const distanceKm = viewerLoc ? itemDistanceKm(item, viewerLoc) : null; const author = useAuthor(item.pubkey); const { mutateAsync: deleteEntity, isPending: isRemoving } = useDeleteEntity(); - const { markSold, relist, isPending: isUpdatingStatus } = useMarkListingSold(); + const { relist, isPending: isUpdatingStatus } = useMarkListingSold(); const { toast } = useToast(); const metadata = author.data?.metadata; const displayName = metadata?.name ?? genUserName(item.pubkey); const avatar = metadata?.picture; const [showReportDialog, setShowReportDialog] = useState(false); const [showRemoveDialog, setShowRemoveDialog] = useState(false); + const [showMarkSold, setShowMarkSold] = useState(false); const npub = nip19.npubEncode(item.pubkey); const isOwnListingCheck = user?.pubkey === item.pubkey; @@ -110,19 +112,6 @@ export function GearSwapItemCard({ item, trustScore, salesCount }: GearSwapItemC const isSold = item.status === 'sold'; - const handleMarkSold = async () => { - try { - await markSold(item.event); - toast({ title: t('marketUi.card.markedSold'), description: t('marketUi.card.markedSoldBody') }); - } catch (error) { - toast({ - title: t('marketUi.card.couldNotUpdate'), - description: error instanceof Error ? error.message : t('marketUi.card.somethingWrong'), - variant: 'destructive', - }); - } - }; - const handleRelist = async () => { try { await relist(item.event); @@ -246,7 +235,7 @@ export function GearSwapItemCard({ item, trustScore, salesCount }: GearSwapItemC { e.stopPropagation(); - handleMarkSold(); + setShowMarkSold(true); }} disabled={isUpdatingStatus} > @@ -430,6 +419,13 @@ export function GearSwapItemCard({ item, trustScore, salesCount }: GearSwapItemC + + {/* Report Dialog */} { setBusy(thread.latest.id); try { - // On accept, attach my Lightning address (if I have one) so the buyer can - // pay in-app. If I have none, their side falls back to my profile `lud16`. - const myLud16 = metadata?.lud16?.trim(); + // Only the SELLER (listing owner) attaches a payment pointer on accept. + // If the buyer accepts the seller's counter, the buyer must NOT publish + // their own address as the payee — that would misdirect the payment. + const isSeller = user?.pubkey === item.pubkey; + const myLud16 = isSeller ? metadata?.lud16?.trim() : undefined; await respondToOffer( thread.latest, status, diff --git a/src/hooks/useMarketplaceOffers.ts b/src/hooks/useMarketplaceOffers.ts index d041188..f896e52 100644 --- a/src/hooks/useMarketplaceOffers.ts +++ b/src/hooks/useMarketplaceOffers.ts @@ -1,4 +1,4 @@ -import { useQuery } from '@tanstack/react-query'; +import { useQuery, useQueryClient } from '@tanstack/react-query'; import type { NostrEvent } from '@nostrify/nostrify'; import { useNostrPublish } from '@/hooks/useNostrPublish'; import { useCurrentUser } from '@/hooks/useCurrentUser'; @@ -193,7 +193,7 @@ export function useListingOffers(listing?: { pubkey: string; identifier: string if (!coord) return { threads: [] }; const events = await querySkateContent( [{ kinds: [OFFER_KIND], '#a': [coord], limit: 200 }], - { signal: AbortSignal.timeout(8000) }, + { signal: AbortSignal.timeout(8000), requireAny: true }, ); const messages = dedupeNewest(events) .map(parseOffer) @@ -252,6 +252,7 @@ export function useIncomingOffers(pubkey: string | undefined) { export function useOfferActions() { const { user } = useCurrentUser(); const { mutateAsync: publish } = useNostrPublish(); + const queryClient = useQueryClient(); /** * Publish one negotiation message. `d` is unique per message so the thread is @@ -308,6 +309,11 @@ export function useOfferActions() { } await publish({ kind: OFFER_KIND, content: note?.trim() || '', tags }); + // Refresh every offer view so the new message/state (and the Pay button) + // appears immediately instead of after the 60s stale window. + queryClient.invalidateQueries({ queryKey: ['listing-offers'] }); + queryClient.invalidateQueries({ queryKey: ['offer-threads-to-me'] }); + queryClient.invalidateQueries({ queryKey: ['incoming-offers'] }); }; /** Buyer: open the negotiation. */ diff --git a/src/hooks/wallet/useCashuEscrow.ts b/src/hooks/wallet/useCashuEscrow.ts index 57b8720..3f64ecb 100644 --- a/src/hooks/wallet/useCashuEscrow.ts +++ b/src/hooks/wallet/useCashuEscrow.ts @@ -131,13 +131,19 @@ export function useCashuEscrow() { const pubkey = await getEscrowPubkey(); if (!pubkey) throw new Error('Could not derive your escrow key.'); - const current = (author.data?.metadata ?? {}) as Record; + // NEVER publish kind 0 unless the existing profile has actually loaded — + // otherwise this would overwrite the user's name/picture/about/lud16 with a + // profile containing only `cashu_escrow`. + if (author.data === undefined) { + throw new Error('Your profile has not loaded yet. Try again in a moment.'); + } + const current = (author.data.metadata ?? {}) as Record; await publish({ kind: 0, content: JSON.stringify({ ...current, cashu_escrow: pubkey }), }); return pubkey; - }, [user, getEscrowPubkey, author.data?.metadata, publish]); + }, [user, getEscrowPubkey, author.data, publish]); return { available,