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.
This commit is contained in:
parent
5f8975d631
commit
4b92759913
6 changed files with 231 additions and 24 deletions
|
|
@ -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
|
||||
|
|
|
|||
196
MARKETPLACE_FLOW_AUDIT.md
Normal file
196
MARKETPLACE_FLOW_AUDIT.md
Normal file
|
|
@ -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:<requestor>:<d>`; 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.
|
||||
|
|
@ -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
|
|||
<DropdownMenuItem
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
handleMarkSold();
|
||||
setShowMarkSold(true);
|
||||
}}
|
||||
disabled={isUpdatingStatus}
|
||||
>
|
||||
|
|
@ -430,6 +419,13 @@ export function GearSwapItemCard({ item, trustScore, salesCount }: GearSwapItemC
|
|||
</div>
|
||||
</CardContent>
|
||||
|
||||
<MarkSoldDialog
|
||||
open={showMarkSold}
|
||||
onOpenChange={setShowMarkSold}
|
||||
event={item.event}
|
||||
listing={{ pubkey: item.pubkey, identifier: item.identifier }}
|
||||
/>
|
||||
|
||||
{/* Report Dialog */}
|
||||
<ReportDialog
|
||||
open={showReportDialog}
|
||||
|
|
|
|||
|
|
@ -95,9 +95,11 @@ export function ListingOffersPanel({ item, isOwner }: { item: MarketplaceItem; i
|
|||
const respond = async (thread: OfferThread, status: 'accepted' | 'declined') => {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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. */
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>;
|
||||
// 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<string, unknown>;
|
||||
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,
|
||||
|
|
|
|||
Loading…
Reference in a new issue