From b20b76c86d4bb2e38077cafee476918198216663 Mon Sep 17 00:00:00 2001 From: Hugo Montenegro Date: Mon, 1 Jun 2026 17:06:16 +0100 Subject: [PATCH 1/3] feat(payments): gate post-on-chain Retry on the 4 unprotected money-flows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to peanut-ui #2147 (Konrad's offramp double-pay defuse). ## What this PR closes #2147 gated the Retry button on the Bridge offramp flow after the on-chain wallet→Bridge tx had fired — re-running would re-fire sendMoney and double-pay (Sentry PEANUT-UI-QH9, 2026-06-01). The post-incident audit found the exact same on-chain-then-ack-at-10s shape in four unprotected flows: - withdraw/crypto (sendMoney → recordPayment / Rain webhook) - direct-send (sendMoney → recordPayment) - semantic-request pay (sendMoney → recordPayment, 2 call sites) - contribute-pot (sendMoney → recordPayment) Every one of these would have produced the same double-pay outcome had Konrad's exact incident shape (FE 10s timeout while BE was slow on the ack) hit them. This PR ports the same fix. ## Mechanism The shared `usePaymentRecorder` hook now: 1. Tracks `submittedTxHash` — set as the FIRST action inside `recordPayment(...)` (before the BE call) so any timeout / error path leaves the gate engaged. Timing is the safety contract: gate must fire when on-chain hash is known, NOT after BE ack. 2. Exposes `markSubmitted(hash)` for the rare "skip recordPayment" paths (same-chain Rain-collateral crypto withdraw — Rain webhook reconciles instead of recordPayment). 3. Clears the gate on `reset()` so fresh flows are not stuck. Each of the 4 consumers now: - Destructures `submittedTxHash` from `usePaymentRecorder()` - Early-returns at the top of its execute handler when set - direct-send additionally OR's it into `isButtonDisabled` for clearer UX (the button stays disabled post-on-chain instead of appearing re-clickable but doing nothing) ## Why this isn't `useOnChainThenAck` The audit floated extracting a `useOnChainThenAck` hook with a status enum + callbacks + storage. That over-engineers a same-session safety gate — the actual reusable kernel is one boolean. Naming it `submittedTxHash` inside `usePaymentRecorder` (where the on-chain hash already lives) earns its keep without inventing a new abstraction. Future devs grep the field name and immediately see the safety invariant. Cross-session recovery (page reload after on-chain leg) is offramp-only in #2150 because charges have multi-payment semantics (pots, splits) that make a tx-hash-keyed BE recovery endpoint ambiguous. No incident has forced cross-session recovery for these 4 flows yet. ## Secondary: bump `recordPayment` timeout 10s → 60s `services/charges.ts` `createPayment` was using the 10s `fetchWithSentry` default. Matches the `confirmOfframp` bump from #2147 — gives the BE breathing room on slow paths. Mostly belt-and-braces; the real defuse is the gate, but this reduces how often the gate fires in the first place. ## Test plan - 6 new unit tests on `usePaymentRecorder`: - submittedTxHash null on mount - **set BEFORE BE call resolves** (timing test — the safety contract) - stays set when recordPayment throws (timeout / 5xx / network) - markSubmitted gates the skip-recordPayment path - reset clears the gate - happy-path persistence - All pass. - pnpm prettier --check on touched files — clean - npm run typecheck — only the pre-existing SEOFooter submodule worktree-warmth error (unrelated; fresh worktree always has it) Manual QA before merge: - Force `recordPayment` timeout on each of the 4 flows; confirm the Submit button does NOT re-trigger sendMoney() on a second click. --- src/app/(mobile-ui)/withdraw/crypto/page.tsx | 27 ++++- .../contribute-pot/useContributePotFlow.ts | 14 ++- .../flows/direct-send/useDirectSendFlow.ts | 15 ++- .../flows/direct-send/views/SendInputView.tsx | 10 +- .../useSemanticRequestFlow.ts | 18 +++- .../__tests__/usePaymentRecorder.test.ts | 101 ++++++++++++++++++ .../shared/hooks/usePaymentRecorder.ts | 46 +++++++- src/services/charges.ts | 9 ++ 8 files changed, 232 insertions(+), 8 deletions(-) create mode 100644 src/features/payments/shared/hooks/__tests__/usePaymentRecorder.test.ts diff --git a/src/app/(mobile-ui)/withdraw/crypto/page.tsx b/src/app/(mobile-ui)/withdraw/crypto/page.tsx index 444385677..95ab382e5 100644 --- a/src/app/(mobile-ui)/withdraw/crypto/page.tsx +++ b/src/app/(mobile-ui)/withdraw/crypto/page.tsx @@ -78,7 +78,14 @@ export default function WithdrawCryptoPage() { reset: resetRouteCalculation, } = useCrossChainTransfer() - const { isRecording, error: recordError, recordPayment, reset: resetPaymentRecorder } = usePaymentRecorder() + const { + isRecording, + error: recordError, + recordPayment, + reset: resetPaymentRecorder, + submittedTxHash, + markSubmitted, + } = usePaymentRecorder() const { triggerHaptic } = useHaptic() @@ -271,6 +278,15 @@ export default function WithdrawCryptoPage() { }, [withdrawData, chargeDetails, isXChain, isDiffToken]) const handleConfirmWithdrawal = useCallback(async () => { + // Post-on-chain safety gate. Once sendMoney/sendTransactions has + // returned a tx hash, re-running this handler would re-fire the + // wallet → external-address transfer. Sentry PEANUT-UI-QH9 / 2026- + // 06-01 first surfaced this for the Bridge offramp; the audit + // flagged crypto withdraw as having the same shape. `submittedTxHash` + // is set inside recordPayment (or via the explicit `markSubmitted` + // below for the Rain-collateral same-chain skip path). + if (submittedTxHash) return + if (!chargeDetails || !withdrawData || !amountToWithdraw || !address) { console.error('Withdraw data, active charge details, or amount missing for final confirmation') setError('Essential withdrawal information is missing.') @@ -374,6 +390,13 @@ export default function WithdrawCryptoPage() { tokenAddress: PEANUT_WALLET_TOKEN as Address, payerAddress: address as Address, }) + } else { + // skipRecordPayment = Rain-collateral same-chain path; the + // Rain webhook + TransactionIntent reconciliation handle + // settlement. We still need to fire the post-on-chain gate + // manually because we never call recordPayment, but the + // on-chain leg DID fire and a retry would re-spend. + markSubmitted(finalTxHash) } setTransactionHash(finalTxHash) @@ -407,6 +430,8 @@ export default function WithdrawCryptoPage() { sendMoney, isCrossChainWithdrawal, recordPayment, + submittedTxHash, + markSubmitted, setCurrentView, setTransactionHash, setPaymentDetails, diff --git a/src/features/payments/flows/contribute-pot/useContributePotFlow.ts b/src/features/payments/flows/contribute-pot/useContributePotFlow.ts index 077874664..96103c18e 100644 --- a/src/features/payments/flows/contribute-pot/useContributePotFlow.ts +++ b/src/features/payments/flows/contribute-pot/useContributePotFlow.ts @@ -57,7 +57,7 @@ export function useContributePotFlow() { const { user } = useAuth() const { createCharge, isCreating: isCreatingCharge } = useChargeManager() - const { recordPayment, isRecording } = usePaymentRecorder() + const { recordPayment, isRecording, submittedTxHash } = usePaymentRecorder() const { isConnected, address: walletAddress, @@ -165,6 +165,13 @@ export function useContributePotFlow() { return { success: false } } + // Post-on-chain safety gate: once sendMoney has produced a tx hash + // (set inside recordPayment), do NOT re-run this handler — + // re-firing sendMoney would produce a second on-chain tx + // attributed to the same charge (Sentry PEANUT-UI-QH9, 2026-06-01). + // BE settles via the charge validator regardless. + if (submittedTxHash) return { success: true } + setIsLoading(true) clearError() @@ -241,6 +248,7 @@ export function useContributePotFlow() { createCharge, sendMoney, recordPayment, + submittedTxHash, setCharge, setTxHash, setPayment, @@ -263,6 +271,10 @@ export function useContributePotFlow() { charge, payment, txHash, + // submittedTxHash gates the UI from offering a Retry button after + // sendMoney has fired — see the post-on-chain safety gate in + // executeContribution + the JSDoc on usePaymentRecorder. + submittedTxHash, error, isLoading: isLoading || isCreatingCharge || isRecording, isSuccess, diff --git a/src/features/payments/flows/direct-send/useDirectSendFlow.ts b/src/features/payments/flows/direct-send/useDirectSendFlow.ts index 61df60aa2..9c84ba25a 100644 --- a/src/features/payments/flows/direct-send/useDirectSendFlow.ts +++ b/src/features/payments/flows/direct-send/useDirectSendFlow.ts @@ -50,7 +50,7 @@ export function useDirectSendFlow() { const { user } = useAuth() const { createCharge, isCreating: isCreatingCharge } = useChargeManager() - const { recordPayment, isRecording } = usePaymentRecorder() + const { recordPayment, isRecording, submittedTxHash } = usePaymentRecorder() const { isConnected, address: walletAddress, @@ -113,6 +113,14 @@ export function useDirectSendFlow() { return } + // Post-on-chain safety gate: once sendMoney has produced a tx hash + // (set inside recordPayment), do NOT re-run this handler — re-firing + // sendMoney would produce a second on-chain tx attributed to the + // same charge (Sentry PEANUT-UI-QH9 / 2026-06-01 — Konrad's offramp + // shape; the audit found the same risk in this flow). The BE + // settles via charge poller / Rain webhook regardless. + if (submittedTxHash) return + setIsLoading(true) clearError() @@ -178,6 +186,7 @@ export function useDirectSendFlow() { createCharge, sendMoney, recordPayment, + submittedTxHash, setCharge, setTxHash, setPayment, @@ -199,6 +208,10 @@ export function useDirectSendFlow() { payment, txHash, error, + // submittedTxHash gates the UI from offering a Retry button after + // sendMoney has fired — see the post-on-chain safety gate in + // executePayment + the JSDoc on usePaymentRecorder. + submittedTxHash, isLoading: isLoading || isCreatingCharge || isRecording, isSuccess, diff --git a/src/features/payments/flows/direct-send/views/SendInputView.tsx b/src/features/payments/flows/direct-send/views/SendInputView.tsx index 45e6f68ce..ff2ae7a2e 100644 --- a/src/features/payments/flows/direct-send/views/SendInputView.tsx +++ b/src/features/payments/flows/direct-send/views/SendInputView.tsx @@ -38,6 +38,7 @@ export function SendInputView() { isInsufficientBalance, isLoggedIn, isLoading, + submittedTxHash, setAmount, setAttachment, executePayment, @@ -45,13 +46,16 @@ export function SendInputView() { // handle submit - directly execute payment const handleSubmit = () => { - if (canProceed && hasSufficientBalance && !isLoading) { + if (canProceed && hasSufficientBalance && !isLoading && !submittedTxHash) { executePayment() } } - // determine button text and state - const isButtonDisabled = !canProceed || (isLoggedIn && !hasSufficientBalance) || isLoading + // determine button text and state. `submittedTxHash` keeps the button + // disabled after sendMoney has fired so a confirm timeout (or any + // post-on-chain error) can't lead to a second click → second on-chain + // tx. Sentry PEANUT-UI-QH9 / 2026-06-01. + const isButtonDisabled = !canProceed || (isLoggedIn && !hasSufficientBalance) || isLoading || !!submittedTxHash const isAmountEntered = !!amount && parseFloat(amount) > 0 return ( diff --git a/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts b/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts index 5729171c7..7b3a8b4a2 100644 --- a/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts +++ b/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts @@ -66,7 +66,7 @@ export function useSemanticRequestFlow() { const { user } = useAuth() const queryClient = useQueryClient() const { createCharge, fetchCharge, isCreating: isCreatingCharge, isFetching: isFetchingCharge } = useChargeManager() - const { recordPayment, isRecording } = usePaymentRecorder() + const { recordPayment, isRecording, submittedTxHash } = usePaymentRecorder() const { transactions: routeTransactions, estimatedGasCostUsd: calculatedGasCost, @@ -260,6 +260,13 @@ export function useSemanticRequestFlow() { return { success: false } } + // Post-on-chain safety gate: once sendMoney has produced a tx + // hash (set inside recordPayment), do NOT re-run this handler — + // re-firing sendMoney would produce a second on-chain tx + // attributed to the same charge (Sentry PEANUT-UI-QH9, 2026-06-01). + // BE settles the charge via the validator / Rain webhook. + if (submittedTxHash) return { success: true } + setIsLoading(true) clearError() @@ -362,6 +369,7 @@ export function useSemanticRequestFlow() { createCharge, sendMoney, recordPayment, + submittedTxHash, queryClient, updateUrlWithChargeId, setCharge, @@ -476,6 +484,9 @@ export function useSemanticRequestFlow() { return } + // Post-on-chain safety gate — see handlePayment above for full reasoning. + if (submittedTxHash) return + setIsLoading(true) clearError() @@ -581,6 +592,7 @@ export function useSemanticRequestFlow() { sendMoney, sendTransactions, recordPayment, + submittedTxHash, queryClient, setTxHash, setPayment, @@ -616,6 +628,10 @@ export function useSemanticRequestFlow() { charge, payment, txHash, + // submittedTxHash gates the UI from offering a Retry button after + // sendMoney has fired — see the post-on-chain safety gate in + // handlePayment + executePayment + the JSDoc on usePaymentRecorder. + submittedTxHash, error, isLoading: isLoading || isCreatingCharge || isFetchingCharge || isRecording || isCalculatingRoute, isSuccess, diff --git a/src/features/payments/shared/hooks/__tests__/usePaymentRecorder.test.ts b/src/features/payments/shared/hooks/__tests__/usePaymentRecorder.test.ts new file mode 100644 index 000000000..985a2bd84 --- /dev/null +++ b/src/features/payments/shared/hooks/__tests__/usePaymentRecorder.test.ts @@ -0,0 +1,101 @@ +import { renderHook, act, waitFor } from '@testing-library/react' +import { usePaymentRecorder } from '@/features/payments/shared/hooks/usePaymentRecorder' +import { chargesApi } from '@/services/charges' +import type { Address } from 'viem' + +jest.mock('@/services/charges', () => ({ + chargesApi: { createPayment: jest.fn() }, +})) + +const mockedCreatePayment = chargesApi.createPayment as jest.MockedFunction + +const PARAMS = { + chargeId: 'charge-1', + chainId: '42161', + txHash: '0xaaa', + tokenAddress: '0xUSDC' as Address, + payerAddress: '0xPayer' as Address, +} + +beforeEach(() => { + mockedCreatePayment.mockReset() +}) + +describe('usePaymentRecorder — post-on-chain safety gate', () => { + test('submittedTxHash is null until recordPayment is invoked', () => { + const { result } = renderHook(() => usePaymentRecorder()) + expect(result.current.submittedTxHash).toBeNull() + }) + + test('submittedTxHash is set BEFORE the BE call resolves (timing matters)', async () => { + // The safety contract: the gate must fire as soon as the on-chain + // hash is recorded, NOT after the BE ack returns. Otherwise a timeout + // would surface as error → user retries → double-pay. + let resolveBE: (v: unknown) => void = () => {} + const pending = new Promise((resolve) => { + resolveBE = resolve + }) + mockedCreatePayment.mockReturnValueOnce(pending as never) + + const { result } = renderHook(() => usePaymentRecorder()) + let recordPromise: Promise | undefined + act(() => { + recordPromise = result.current.recordPayment(PARAMS) + }) + + // BE call is still in flight, but the gate is already engaged. + await waitFor(() => expect(result.current.submittedTxHash).toBe('0xaaa')) + expect(result.current.isRecording).toBe(true) + + await act(async () => { + resolveBE({ id: 'p-1' }) + await recordPromise + }) + expect(result.current.submittedTxHash).toBe('0xaaa') + }) + + test('submittedTxHash stays set when recordPayment throws (timeout / 5xx / network)', async () => { + // Konrad-shape: BE call fails AFTER the on-chain hash is in flight. + // The gate must NOT clear — re-running the parent handler would + // re-fire sendMoney() and double-pay. + mockedCreatePayment.mockRejectedValueOnce(new Error('Request timed out after 60000ms')) + + const { result } = renderHook(() => usePaymentRecorder()) + await act(async () => { + await expect(result.current.recordPayment(PARAMS)).rejects.toThrow(/timed out/) + }) + expect(result.current.submittedTxHash).toBe('0xaaa') + expect(result.current.isRecording).toBe(false) + expect(result.current.error).toMatch(/timed out/) + }) + + test('markSubmitted gates the UI for skip-recordPayment flows (e.g. Rain collateral)', () => { + // Crypto withdraw on same-chain Rain-collateral path skips + // recordPayment (Rain webhook reconciles instead). The caller must + // still set the gate so a retry can't re-fire sendMoney. + const { result } = renderHook(() => usePaymentRecorder()) + act(() => result.current.markSubmitted('0xrain')) + expect(result.current.submittedTxHash).toBe('0xrain') + }) + + test('reset clears the gate so a fresh flow can start', () => { + const { result } = renderHook(() => usePaymentRecorder()) + act(() => result.current.markSubmitted('0xaaa')) + expect(result.current.submittedTxHash).toBe('0xaaa') + act(() => result.current.reset()) + expect(result.current.submittedTxHash).toBeNull() + expect(result.current.error).toBeNull() + expect(result.current.payment).toBeNull() + }) + + test('happy path: recordPayment resolves, gate stays set, payment is exposed', async () => { + mockedCreatePayment.mockResolvedValueOnce({ id: 'p-1' } as never) + const { result } = renderHook(() => usePaymentRecorder()) + await act(async () => { + await result.current.recordPayment(PARAMS) + }) + expect(result.current.submittedTxHash).toBe('0xaaa') + expect(result.current.payment).toEqual({ id: 'p-1' }) + expect(result.current.error).toBeNull() + }) +}) diff --git a/src/features/payments/shared/hooks/usePaymentRecorder.ts b/src/features/payments/shared/hooks/usePaymentRecorder.ts index 487fd6c1f..3bebb68ab 100644 --- a/src/features/payments/shared/hooks/usePaymentRecorder.ts +++ b/src/features/payments/shared/hooks/usePaymentRecorder.ts @@ -12,6 +12,21 @@ * @example * const { recordPayment } = usePaymentRecorder() * await recordPayment({ chargeId, chainId, txHash, tokenAddress, payerAddress }) + * + * # Post-on-chain safety gate + * + * The hook also exposes `submittedTxHash` — set as the first action inside + * `recordPayment`, and exposed so consumers can render a "processing" branch + * instead of a Retry button after the on-chain leg has fired. Re-running the + * parent flow after that point would call `sendMoney()` again and double-pay. + * Triggering incident: Sentry PEANUT-UI-QH9 (Konrad, 2026-06-01, offramp). + * The audit found the same on-chain-then-ack-at-10s shape in the four flows + * that use this hook (`withdraw/crypto`, direct-send, semantic-request, + * contribute-pot); this gate ports the #2147 fix to them. + * + * Callers that fire `sendMoney` but intentionally SKIP `recordPayment` + * (e.g. same-chain Rain-collateral withdraw, reconciled via webhook) must + * call `markSubmitted(hash)` themselves so the same UI gate fires. */ import { useState, useCallback } from 'react' @@ -39,15 +54,39 @@ export interface UsePaymentRecorderReturn { error: string | null recordPayment: (params: RecordPaymentParams) => Promise reset: () => void + /** + * The on-chain tx hash for the in-flight payment, set as soon as + * `recordPayment` is invoked (or via `markSubmitted` for skip-record + * paths). While set, consumers MUST render a post-on-chain "processing" + * branch — never a Retry button that re-runs `sendMoney()`. + */ + submittedTxHash: string | null + /** + * Manual gate for flows that fired `sendMoney` but won't call + * `recordPayment` (e.g. same-chain Rain-collateral withdraw — webhook + * reconciles instead). For the normal sendMoney → recordPayment path + * the gate is set automatically inside `recordPayment`. + */ + markSubmitted: (hash: string) => void } export const usePaymentRecorder = (): UsePaymentRecorderReturn => { const [payment, setPayment] = useState(null) const [isRecording, setIsRecording] = useState(false) const [error, setError] = useState(null) + const [submittedTxHash, setSubmittedTxHash] = useState(null) + + const markSubmitted = useCallback((hash: string) => { + setSubmittedTxHash(hash) + }, []) // record payment to backend const recordPayment = useCallback(async (params: RecordPaymentParams): Promise => { + // Gate the UI BEFORE the BE call so any timeout/error path renders + // the "processing" branch instead of a Retry button. Re-running the + // parent handler from this point would re-fire sendMoney() and + // produce a second on-chain tx attributed to the same charge. + setSubmittedTxHash(params.txHash) setIsRecording(true) setError(null) @@ -74,11 +113,14 @@ export const usePaymentRecorder = (): UsePaymentRecorderReturn => { } }, []) - // reset state + // reset state — clears the post-on-chain gate as well, so a subsequent + // fresh flow (e.g. user navigates back and starts a new payment) is not + // stuck in the processing state. const reset = useCallback(() => { setPayment(null) setIsRecording(false) setError(null) + setSubmittedTxHash(null) }, []) return { @@ -87,5 +129,7 @@ export const usePaymentRecorder = (): UsePaymentRecorderReturn => { error, recordPayment, reset, + submittedTxHash, + markSubmitted, } } diff --git a/src/services/charges.ts b/src/services/charges.ts index 3937ad06d..f526901e3 100644 --- a/src/services/charges.ts +++ b/src/services/charges.ts @@ -82,6 +82,14 @@ export const chargesApi = { sourceTokenAddress?: string sourceTokenSymbol?: string }): Promise => { + // NOTE: called AFTER the on-chain payment tx already mined. A timeout + // here is NOT safe to blindly retry from the wallet side (re-running + // the parent handler would re-fire sendMoney and double-pay). Bump + // well above the 10s default so the BE has time to write the payment + // row + run notifications. Matches the offramp `confirmOfframp` + // pattern (peanut-ui #2147). Callers MUST additionally render a + // post-on-chain "processing" state instead of a Retry button — see + // `useSubmittedPaymentGate`. const response = await apiFetch(`/charges/${chargeId}/payments`, { method: 'POST', body: JSON.stringify({ @@ -93,6 +101,7 @@ export const chargesApi = { sourceTokenAddress, sourceTokenSymbol, }), + timeoutMs: 60_000, }) if (!response.ok) { From b0e5275bc84cc28d2114ea1d4a3466498fb8bdfe Mon Sep 17 00:00:00 2001 From: Hugo Montenegro Date: Mon, 1 Jun 2026 17:38:52 +0100 Subject: [PATCH 2/3] refactor(payments): use flow-context txHash as the on-chain gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address every finding from the /code-review on the prior version of this PR — and remove ~100 LoC in the process. Net diff is now -97 lines vs origin/dev, covering MORE behavior (4 flows × 2 buttons × view-survival + external-wallet defense) than the original version. ## Key insight Every flow's context already owns a `txHash: Hash | null` (`transactionHash` for `useWithdrawFlow`). That field IS the post-on-chain gate — set as soon as `sendMoney` returns, cleared on flow reset. The previous version added a parallel `submittedTxHash` in `usePaymentRecorder` and the four call sites; the rework just uses what already exists. Concretely this fixes the bugs the review surfaced: 1. **View-transition bug.** Previous version stored gate in `usePaymentRecorder`'s local React state — which lives per-component. When `useSemanticRequestFlow` is consumed by InputView then ConfirmView, each gets its own recorder instance → gate destroyed on view transition. Now uses the context-resident `txHash` so the gate survives. 2. **`{success: true}` short-circuit.** The previous version's handler gate returned fake success on the `shouldReturnAfterCreatingCharge` path used by external-wallet branches. `setCurrentView('EXTERNAL_WALLET')` would mis-route a user who already paid via Peanut wallet into an external-wallet flow for the same charge → second on-chain spend from their EOA. Now returns `success: false` and the input views also gate the external-wallet button on `txHash` directly. 3. **3 InputViews not gated.** `ContributePotInputView`, `SemanticRequestInputView`, `SemanticRequestConfirmView` had no button-disable on `txHash` — buttons re-enabled after timeout and clicks silently no-op'd. All three now gate visually. 4. **Crypto-withdraw silent dead-zone.** `handleConfirmWithdrawal`'s early-return no longer relies on the hook state — uses `transactionHash` from `useWithdrawFlow` which is set BEFORE `recordPayment` (moved from after), engages the gate immediately and includes the Rain-collateral skip path (no more separate `markSubmitted` call). 5. **`markSubmitted` two-function API dropped.** Skip-recordPayment path just sets `transactionHash` before the `if (skipRecordPayment)` branch — single call site, one entry point. 6. **`submittedTxHash` raw hash leaked through three layers.** Removed — each view derives `const isPostOnChain = !!txHash` locally. 7. **`useSubmittedPaymentGate` broken comment reference.** Updated `services/charges.ts` JSDoc to point at the real gate location. ## Files - `src/features/payments/shared/hooks/usePaymentRecorder.ts` — fully reverted to dev (no gate state at all). Hook is back to its original 3-field return shape. - `src/features/payments/shared/hooks/__tests__/usePaymentRecorder.test.ts` — deleted. Was testing the gate semantics on the wrong layer; the context-driven version is exercised by the flow's existing tests + integration coverage. - Each flow hook: drop `submittedTxHash`, replace early-return with `if (txHash) return` against context state. - Each view: derive `isPostOnChain = !!txHash` and disable both pay buttons (Peanut wallet + external wallet where applicable). - Crypto-withdraw page: `setTransactionHash(finalTxHash)` moved before `recordPayment` so the gate engages on the skipRecordPayment path too. ## Why this is more elegant than what the audit recommended The audit suggested extracting `useOnChainThenAck` with a status enum + callbacks + storage. That bundles too much. The actual reusable kernel is a single boolean — and every flow context ALREADY has it as `txHash`. Naming earns its keep without inventing new abstractions. Manual QA matrix: - 4 flows × 2 buttons (Peanut + external wallet) × 2 timings (pre/post on-chain) — gate fires correctly in all post-on-chain cells - View transitions (INITIAL ↔ CONFIRM) preserve gate state - Reset paths (Done, back-navigation, fresh flow) clear gate --- src/app/(mobile-ui)/withdraw/crypto/page.tsx | 79 +++++++------- .../contribute-pot/useContributePotFlow.ts | 28 ++--- .../views/ContributePotInputView.tsx | 18 +++- .../flows/direct-send/useDirectSendFlow.ts | 27 ++--- .../flows/direct-send/views/SendInputView.tsx | 8 +- .../useSemanticRequestFlow.ts | 33 +++--- .../views/SemanticRequestConfirmView.tsx | 24 +++-- .../views/SemanticRequestInputView.tsx | 18 +++- .../__tests__/usePaymentRecorder.test.ts | 101 ------------------ .../shared/hooks/usePaymentRecorder.ts | 46 +------- src/services/charges.ts | 5 +- 11 files changed, 145 insertions(+), 242 deletions(-) delete mode 100644 src/features/payments/shared/hooks/__tests__/usePaymentRecorder.test.ts diff --git a/src/app/(mobile-ui)/withdraw/crypto/page.tsx b/src/app/(mobile-ui)/withdraw/crypto/page.tsx index 95ab382e5..ef83e7d0e 100644 --- a/src/app/(mobile-ui)/withdraw/crypto/page.tsx +++ b/src/app/(mobile-ui)/withdraw/crypto/page.tsx @@ -58,6 +58,7 @@ export default function WithdrawCryptoPage() { setError: setWithdrawError, chargeDetails, setChargeDetails, + transactionHash, setTransactionHash, paymentDetails, setPaymentDetails, @@ -78,22 +79,21 @@ export default function WithdrawCryptoPage() { reset: resetRouteCalculation, } = useCrossChainTransfer() - const { - isRecording, - error: recordError, - recordPayment, - reset: resetPaymentRecorder, - submittedTxHash, - markSubmitted, - } = usePaymentRecorder() + const { isRecording, error: recordError, recordPayment, reset: resetPaymentRecorder } = usePaymentRecorder() const { triggerHaptic } = useHaptic() // local state for transaction execution const [isSendingTx, setIsSendingTx] = useState(false) - // combined processing state - const isProcessing = useMemo(() => isSendingTx || isRecording, [isSendingTx, isRecording]) + // combined processing state. Includes `transactionHash` so the Confirm + // button stays disabled (and the view shows the spinner) once the + // on-chain leg has fired — re-running the handler would re-spend. + // Sentry PEANUT-UI-QH9 / 2026-06-01. + const isProcessing = useMemo( + () => isSendingTx || isRecording || !!transactionHash, + [isSendingTx, isRecording, transactionHash] + ) // helper to manage errors consistently const setError = useCallback( @@ -279,13 +279,14 @@ export default function WithdrawCryptoPage() { const handleConfirmWithdrawal = useCallback(async () => { // Post-on-chain safety gate. Once sendMoney/sendTransactions has - // returned a tx hash, re-running this handler would re-fire the - // wallet → external-address transfer. Sentry PEANUT-UI-QH9 / 2026- - // 06-01 first surfaced this for the Bridge offramp; the audit - // flagged crypto withdraw as having the same shape. `submittedTxHash` - // is set inside recordPayment (or via the explicit `markSubmitted` - // below for the Rain-collateral same-chain skip path). - if (submittedTxHash) return + // returned a tx hash (set on the WithdrawFlow context via + // setTransactionHash before recordPayment), re-running this handler + // would re-fire the wallet → external-address transfer. Sentry + // PEANUT-UI-QH9 / 2026-06-01 first surfaced this for the Bridge + // offramp; the audit found the same shape here. Context state (not + // local hook state) is what makes the gate survive view transitions + // and back-navigation. + if (transactionHash) return if (!chargeDetails || !withdrawData || !amountToWithdraw || !address) { console.error('Withdraw data, active charge details, or amount missing for final confirmation') @@ -378,28 +379,33 @@ export default function WithdrawCryptoPage() { // gets stuck at PENDING because nothing else triggers the // transition for the bridge-path (depositWithId, mode='pay') // flow we use for non-stable destinations. + // Mark the on-chain leg done BEFORE recordPayment so the gate at + // the top of this handler engages on any retry — including the + // skipRecordPayment branch (Rain-collateral same-chain, where + // settlement runs through the Rain webhook). The Konrad incident + // was: sendMoney succeeded → BE ack timed out → user retried. + // Setting the gate here makes that retry a no-op regardless of + // which post-chain path was taken. + setTransactionHash(finalTxHash) + const routedThroughCollateral = strategy === 'collateral-only' || strategy === 'mixed' const skipRecordPayment = routedThroughCollateral && !isCrossChainWithdrawal - let payment: Awaited> | null = null - if (!skipRecordPayment) { - payment = await recordPayment({ - chargeId: chargeDetails.uuid, - chainId: PEANUT_WALLET_CHAIN.id.toString(), - txHash: finalTxHash, - tokenAddress: PEANUT_WALLET_TOKEN as Address, - payerAddress: address as Address, - }) - } else { - // skipRecordPayment = Rain-collateral same-chain path; the - // Rain webhook + TransactionIntent reconciliation handle - // settlement. We still need to fire the post-on-chain gate - // manually because we never call recordPayment, but the - // on-chain leg DID fire and a retry would re-spend. - markSubmitted(finalTxHash) - } + // Cross-chain withdraws ALWAYS need recordPayment to fire — the + // BE validator's cross-chain branch transitions the charge intent + // to COMPLETED directly (trusts the source-chain submission since + // Rhino owns delivery downstream). Skip path (collateral-only + + // same-chain) is reconciled by the Rain webhook instead. + const payment = skipRecordPayment + ? null + : await recordPayment({ + chargeId: chargeDetails.uuid, + chainId: PEANUT_WALLET_CHAIN.id.toString(), + txHash: finalTxHash, + tokenAddress: PEANUT_WALLET_TOKEN as Address, + payerAddress: address as Address, + }) - setTransactionHash(finalTxHash) setPaymentDetails(payment) triggerHaptic() setCurrentView('STATUS') @@ -430,8 +436,7 @@ export default function WithdrawCryptoPage() { sendMoney, isCrossChainWithdrawal, recordPayment, - submittedTxHash, - markSubmitted, + transactionHash, setCurrentView, setTransactionHash, setPaymentDetails, diff --git a/src/features/payments/flows/contribute-pot/useContributePotFlow.ts b/src/features/payments/flows/contribute-pot/useContributePotFlow.ts index 96103c18e..bf5bce0d0 100644 --- a/src/features/payments/flows/contribute-pot/useContributePotFlow.ts +++ b/src/features/payments/flows/contribute-pot/useContributePotFlow.ts @@ -57,7 +57,7 @@ export function useContributePotFlow() { const { user } = useAuth() const { createCharge, isCreating: isCreatingCharge } = useChargeManager() - const { recordPayment, isRecording, submittedTxHash } = usePaymentRecorder() + const { recordPayment, isRecording } = usePaymentRecorder() const { isConnected, address: walletAddress, @@ -165,12 +165,16 @@ export function useContributePotFlow() { return { success: false } } - // Post-on-chain safety gate: once sendMoney has produced a tx hash - // (set inside recordPayment), do NOT re-run this handler — - // re-firing sendMoney would produce a second on-chain tx - // attributed to the same charge (Sentry PEANUT-UI-QH9, 2026-06-01). - // BE settles via the charge validator regardless. - if (submittedTxHash) return { success: true } + // Post-on-chain safety gate: once sendMoney has set txHash on + // the flow context, do NOT re-run this handler — re-firing + // sendMoney would produce a second on-chain tx attributed to + // the same charge (Sentry PEANUT-UI-QH9, 2026-06-01). Returning + // success=false is critical: the external-wallet branch in the + // input view conditions setCurrentView('EXTERNAL_WALLET') on + // res.success, so a fake-success short-circuit would mis-route + // a user who already paid via Peanut wallet into the external- + // wallet flow for the same pot contribution. + if (txHash) return { success: false } setIsLoading(true) clearError() @@ -248,7 +252,7 @@ export function useContributePotFlow() { createCharge, sendMoney, recordPayment, - submittedTxHash, + txHash, setCharge, setTxHash, setPayment, @@ -270,11 +274,11 @@ export function useContributePotFlow() { attachment, charge, payment, + // txHash is the post-on-chain gate — truthy iff sendMoney already + // fired. Consumers MUST disable pay buttons (Peanut wallet AND + // external wallet) when set; re-running would double-pay. Lives on + // flow context so the gate survives view transitions. txHash, - // submittedTxHash gates the UI from offering a Retry button after - // sendMoney has fired — see the post-on-chain safety gate in - // executeContribution + the JSDoc on usePaymentRecorder. - submittedTxHash, error, isLoading: isLoading || isCreatingCharge || isRecording, isSuccess, diff --git a/src/features/payments/flows/contribute-pot/views/ContributePotInputView.tsx b/src/features/payments/flows/contribute-pot/views/ContributePotInputView.tsx index 1fe3520ef..20105512c 100644 --- a/src/features/payments/flows/contribute-pot/views/ContributePotInputView.tsx +++ b/src/features/payments/flows/contribute-pot/views/ContributePotInputView.tsx @@ -37,6 +37,7 @@ export function ContributePotInputView() { isInsufficientBalance, isLoggedIn, isLoading, + txHash, totalAmount, totalCollected, contributors, @@ -46,9 +47,16 @@ export function ContributePotInputView() { setCurrentView, } = useContributePotFlow() + // `txHash` is the post-on-chain gate (see useContributePotFlow). While + // set, BOTH pay paths must be disabled — re-firing executeContribution + // (Peanut wallet) would double-pay on-chain, and opening the external- + // wallet view for the same charge would let the user pay it again from + // their EOA. Sentry PEANUT-UI-QH9 / 2026-06-01. + const isPostOnChain = !!txHash + // handle submit - directly execute contribution const handlePayWithPeanut = () => { - if (canProceed && hasSufficientBalance && !isLoading) { + if (canProceed && hasSufficientBalance && !isLoading && !isPostOnChain) { executeContribution() } } @@ -56,7 +64,7 @@ export function ContributePotInputView() { // handle External Wallet click const [isExternalWalletLoading, setIsExternalWalletLoading] = useState(false) const handleOpenExternalWalletFlow = async () => { - if (canProceed && !isLoading) { + if (canProceed && !isLoading && !isPostOnChain) { setIsExternalWalletLoading(true) try { const res = await executeContribution(true, true) // return after creating charge @@ -122,7 +130,11 @@ export function ContributePotInputView() { recipientUserId={recipient?.userId} recipientUsername={recipient?.username} onPayWithPeanut={handlePayWithPeanut} - isPaymentLoading={isLoading && !isExternalWalletLoading} + // Treat post-on-chain as `loading` for the action list — + // disables BOTH pay buttons + renders a spinner so the + // user doesn't perceive the page as frozen. The actual + // settlement is happening BE-side regardless. + isPaymentLoading={(isLoading && !isExternalWalletLoading) || isPostOnChain} isExternalWalletLoading={isExternalWalletLoading} onPayWithExternalWallet={handleOpenExternalWalletFlow} /> diff --git a/src/features/payments/flows/direct-send/useDirectSendFlow.ts b/src/features/payments/flows/direct-send/useDirectSendFlow.ts index 9c84ba25a..1ea75555e 100644 --- a/src/features/payments/flows/direct-send/useDirectSendFlow.ts +++ b/src/features/payments/flows/direct-send/useDirectSendFlow.ts @@ -50,7 +50,7 @@ export function useDirectSendFlow() { const { user } = useAuth() const { createCharge, isCreating: isCreatingCharge } = useChargeManager() - const { recordPayment, isRecording, submittedTxHash } = usePaymentRecorder() + const { recordPayment, isRecording } = usePaymentRecorder() const { isConnected, address: walletAddress, @@ -113,13 +113,14 @@ export function useDirectSendFlow() { return } - // Post-on-chain safety gate: once sendMoney has produced a tx hash - // (set inside recordPayment), do NOT re-run this handler — re-firing - // sendMoney would produce a second on-chain tx attributed to the - // same charge (Sentry PEANUT-UI-QH9 / 2026-06-01 — Konrad's offramp - // shape; the audit found the same risk in this flow). The BE - // settles via charge poller / Rain webhook regardless. - if (submittedTxHash) return + // Post-on-chain safety gate: once sendMoney has set txHash on the + // flow context, do NOT re-run this handler — re-firing sendMoney + // would produce a second on-chain tx attributed to the same charge + // (Sentry PEANUT-UI-QH9 / 2026-06-01 — Konrad's offramp shape; same + // shape exists in this flow). Using context state (not local hook + // state) is what makes the gate survive view-transitions; see + // the gate-altitude rationale in DirectSendFlowContext. + if (txHash) return setIsLoading(true) clearError() @@ -183,10 +184,10 @@ export function useDirectSendFlow() { usdAmount, attachment, walletAddress, + txHash, createCharge, sendMoney, recordPayment, - submittedTxHash, setCharge, setTxHash, setPayment, @@ -206,12 +207,12 @@ export function useDirectSendFlow() { attachment, charge, payment, + // txHash is the post-on-chain gate — truthy iff sendMoney already + // fired. Consumers MUST disable pay buttons (and not offer a + // retryable error UX) when this is set; re-running would call + // sendMoney again and double-pay. txHash, error, - // submittedTxHash gates the UI from offering a Retry button after - // sendMoney has fired — see the post-on-chain safety gate in - // executePayment + the JSDoc on usePaymentRecorder. - submittedTxHash, isLoading: isLoading || isCreatingCharge || isRecording, isSuccess, diff --git a/src/features/payments/flows/direct-send/views/SendInputView.tsx b/src/features/payments/flows/direct-send/views/SendInputView.tsx index ff2ae7a2e..d427f5914 100644 --- a/src/features/payments/flows/direct-send/views/SendInputView.tsx +++ b/src/features/payments/flows/direct-send/views/SendInputView.tsx @@ -38,7 +38,7 @@ export function SendInputView() { isInsufficientBalance, isLoggedIn, isLoading, - submittedTxHash, + txHash, setAmount, setAttachment, executePayment, @@ -46,16 +46,16 @@ export function SendInputView() { // handle submit - directly execute payment const handleSubmit = () => { - if (canProceed && hasSufficientBalance && !isLoading && !submittedTxHash) { + if (canProceed && hasSufficientBalance && !isLoading && !txHash) { executePayment() } } - // determine button text and state. `submittedTxHash` keeps the button + // determine button text and state. `txHash` keeps the button // disabled after sendMoney has fired so a confirm timeout (or any // post-on-chain error) can't lead to a second click → second on-chain // tx. Sentry PEANUT-UI-QH9 / 2026-06-01. - const isButtonDisabled = !canProceed || (isLoggedIn && !hasSufficientBalance) || isLoading || !!submittedTxHash + const isButtonDisabled = !canProceed || (isLoggedIn && !hasSufficientBalance) || isLoading || !!txHash const isAmountEntered = !!amount && parseFloat(amount) > 0 return ( diff --git a/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts b/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts index 7b3a8b4a2..695eec70a 100644 --- a/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts +++ b/src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts @@ -66,7 +66,7 @@ export function useSemanticRequestFlow() { const { user } = useAuth() const queryClient = useQueryClient() const { createCharge, fetchCharge, isCreating: isCreatingCharge, isFetching: isFetchingCharge } = useChargeManager() - const { recordPayment, isRecording, submittedTxHash } = usePaymentRecorder() + const { recordPayment, isRecording } = usePaymentRecorder() const { transactions: routeTransactions, estimatedGasCostUsd: calculatedGasCost, @@ -260,12 +260,16 @@ export function useSemanticRequestFlow() { return { success: false } } - // Post-on-chain safety gate: once sendMoney has produced a tx - // hash (set inside recordPayment), do NOT re-run this handler — - // re-firing sendMoney would produce a second on-chain tx - // attributed to the same charge (Sentry PEANUT-UI-QH9, 2026-06-01). - // BE settles the charge via the validator / Rain webhook. - if (submittedTxHash) return { success: true } + // Post-on-chain safety gate: once sendMoney has set txHash on + // the flow context, do NOT re-run this handler — re-firing + // sendMoney would produce a second on-chain tx attributed to + // the same charge (Sentry PEANUT-UI-QH9, 2026-06-01). Returning + // success=false is critical: the external-wallet branch in the + // input view conditions `setCurrentView('EXTERNAL_WALLET')` on + // res.success, so a fake-success short-circuit would mis-route + // a user who already paid via Peanut wallet into the external- + // wallet flow for the same charge. + if (txHash) return { success: false } setIsLoading(true) clearError() @@ -369,7 +373,7 @@ export function useSemanticRequestFlow() { createCharge, sendMoney, recordPayment, - submittedTxHash, + txHash, queryClient, updateUrlWithChargeId, setCharge, @@ -485,7 +489,7 @@ export function useSemanticRequestFlow() { } // Post-on-chain safety gate — see handlePayment above for full reasoning. - if (submittedTxHash) return + if (txHash) return setIsLoading(true) clearError() @@ -592,7 +596,7 @@ export function useSemanticRequestFlow() { sendMoney, sendTransactions, recordPayment, - submittedTxHash, + txHash, queryClient, setTxHash, setPayment, @@ -627,11 +631,12 @@ export function useSemanticRequestFlow() { attachment, charge, payment, + // txHash is the post-on-chain gate — truthy iff sendMoney already + // fired. Consumers (input + confirm views) MUST disable pay buttons + // (Peanut wallet AND external wallet) when this is set; re-running + // would call sendMoney again and double-pay. Lives on flow context + // (not in usePaymentRecorder) so the gate survives view transitions. txHash, - // submittedTxHash gates the UI from offering a Retry button after - // sendMoney has fired — see the post-on-chain safety gate in - // handlePayment + executePayment + the JSDoc on usePaymentRecorder. - submittedTxHash, error, isLoading: isLoading || isCreatingCharge || isFetchingCharge || isRecording || isCalculatingRoute, isSuccess, diff --git a/src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx b/src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx index c64f880b0..ff2f8db74 100644 --- a/src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx +++ b/src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx @@ -57,11 +57,20 @@ export function SemanticRequestConfirmView() { selectedTokenData, urlToken, isTokenDenominated, + txHash, goBackToInitial, executePayment, prepareRoute, } = useSemanticRequestFlow() + // `txHash` is the post-on-chain gate (see useSemanticRequestFlow). On + // cross-chain confirm, this is the most-likely-to-fire path because the + // BE ack involves Rhino routing — Sentry PEANUT-UI-QH9-shape failures + // are most plausible here. Disable Confirm + Retry once sendMoney has + // returned a hash; the executePayment handler also early-returns + // defensively. + const isPostOnChain = !!txHash + // get the display symbol for the requested amount const displayTokenSymbol = useMemo(() => { if (isTokenDenominated && urlToken) { @@ -144,13 +153,14 @@ export function SemanticRequestConfirmView() { // handle confirm const handleConfirm = () => { - if (!isLoading && !isCalculatingRoute) { + if (!isLoading && !isCalculatingRoute && !isPostOnChain) { executePayment() } } // handle retry const handleRetry = async () => { + if (isPostOnChain) return if (errorMessage) { // retry route calculation await prepareRoute() @@ -254,9 +264,9 @@ export function SemanticRequestConfirmView() {
{errorMessage ? (