fix(withdraw): complete the user-facing charge on collateral-routed crypto withdraws - #2484
Closed
abalinda wants to merge 31 commits into
Closed
fix(withdraw): complete the user-facing charge on collateral-routed crypto withdraws#2484abalinda wants to merge 31 commits into
abalinda wants to merge 31 commits into
Conversation
Name mismatch (Bridge BE01) is the top cause of returned SEPA/ACH deposits, but the beneficiary/account-holder name was shown as a plain row while amount and reference got prominent warnings. Add a warning so users send from an account in their own name and enter the recipient name exactly as shown.
Fold the name-match guidance into the existing 'Double check before sending' checklist card instead of a separate warning card, so there is one source of pre-send do-nots (no drift, no signal dilution from a fourth alarm card). Omit it for MX/SPEI, where paying from a third-party account is a supported flow and the own-name rule does not apply.
BE01 validates the creditor (recipient) name too, not just the sender account. Add the recipient-name-exact line alongside the sender rule in the same non-MX branch.
fix: warn about name mismatch on bank deposit details
…lent again
The Jul-16/17 card outage ran 19h and was found by a customer in Dhaka, not by
us. The reason is structural, not bad luck: a WebSDK that never launches throws
nothing, requests nothing and logs nothing. Sentry cannot see code that does not
run, so the only trace was throughput quietly going to zero. This closes that
class of failure rather than the one instance of it.
Watchdog: if the modal is open and the SDK has not launched within 20s, show the
error UI instead of the infinite spinner. Deliberately keyed on `visible` ALONE —
every silent path (null container, token that never arrives, script that never
loads) ends with the init effect simply not running, so anything keyed on those
deps would also never run. Watching the one fact the user experiences ("I opened
it and nothing happened") catches the whole class, including causes we have not
thought of yet.
Telemetry: kyc_sdk_launched / kyc_sdk_launch_timeout / kyc_sdk_init_failed. The
timeout event carries hadAccessToken / sdkScriptLoaded / hadContainer, which are
the three facts that distinguish the silent stalls from each other. Makes
"opened but never launched" alertable instead of archaeology.
Also corrects the mechanism comments shipped in #2441: the one-commit delay comes
from headlessui's <Transition> promoting tree state inside an effect, NOT the
Portal (isolation-tested: dialog-only mounts the container synchronously;
transition-only does not). The fix was right, the explanation was not, and a
wrong comment misleads the next reader.
Tests: the watchdog fires and surfaces the error UI when the SDK never launches,
and does not fire once it has.
feat(kyc): watchdog + telemetry so a stuck Sumsub SDK can never be silent again
The in-app card is composited layer-by-layer in code, so it drifted from the finalised Rain art (what Apple Wallet shows): it rendered a mascot with no PEANUT wordmark and an inverted-white Visa with no tier. Swap the top row to the PEANUT lockup (mascot + black wordmark) and a black VISA with a Platinum tier line. Closes TASK-20469.
The composited hand used -rotate-15 + h-full + a negative top inset, which shoved it up so the fingertips crowded the Visa and the gesture tilted off from the finalised Rain art. Anchor it upright at the bottom (h-90%) so the fingertips clear the top logos and the arm sweeps to the bottom-right corner.
Two corrections after seeing the real card: - The PEANUT wordmark was rendering white (PEANUT_LOGO is the dark-bg variant). Use PEANUTMAN + PEANUT_LOGO_BLACK for the black lockup the finalised art shows. - The bottom-anchored hand covered the eye toggle and the revealed PAN/CVV. Slide it diagonally off the bottom-right corner while details are shown or being fetched (loading skeletons included), and move the masked/error eye inline with the number so it stays in a hand-free zone. The hand slides back when the card is re-masked.
Lowering the per-transaction limit left the card holding more than its new target: the auto-balancer only ever tops up (rebalance-decision no-ops when collateral >= limit), so the excess sat in the collateral contract indefinitely. After the limit PATCH succeeds, sign a FORCED collateral-only withdrawal of the excess back to the user's smart wallet - the PATCH-first ordering keeps the auto-balancer from racing the withdrawal by topping the collateral back up. UX: the user only sees the passkey prompt. The unified displayed balance is identical before/after (both buckets are folded into one), and the intent kind AUTO_REBALANCE maps to INTERNAL_TRANSFER, which history hides. Failures are non-fatal by design: the limit change already stuck, and re-saving the limit retries the return. forceStrategy on useSignSpendBundle exists because live-balance routing would pick smart-only (a self-transfer no-op) whenever the smart wallet covers the amount - the exact trap the lock-card flow comments around.
CodeRabbit review: the forced-path insufficient branch skipped the CARD_WITHDRAW_FAILED capture resolveSpendStrategy emits, leaving a gap in the withdraw funnel for the excess-return flow; and the forced branch had no direct tests.
…mit-decrease feat(card): return excess collateral to wallet on limit decrease
Backend now marks every Bridge rail blocked for UK residents with reason code 'uk_resident_blocked'. Surface it honestly instead of the generic 'we couldn't unlock this / contact support' (ID-failure) framing: - InitiateKycModal gains a 'region-unavailable' variant — 'Not available in the UK', regulatory copy, single 'Got it' dismiss (no verify, no contact-support) - add-money + withdraw bank pages pick that variant when the gate's reason is uk_resident_blocked Card block rides on the existing eligibility machinery (BE denylist).
- title 'Not available for UK residents' (conveys residence, not the destination country — a UK resident is blocked even withdrawing to SEPA) - drop the card mention from the bank-flow copy - CTA 'Withdraw funds' -> /withdraw so users can exit via crypto - wire the region-unavailable variant into AddWithdrawCountriesList (the country-selection modal), not just the add-money/withdraw bank pages
fix(card): match in-app card art to finalised Visa Platinum lockup
…ailable check (Hugo review) The 'reason.code === uk_resident_blocked' -> region-unavailable ternary was copy-pasted across the add-money/withdraw bank pages and the country-list. Centralize in one helper alongside getKycModalVariant.
The bare-badge /invite screen labeled its primary CTA "Claim your badge" while actually routing to signup, and framed login as an alternative to claiming rather than the other path to it. Only logged-out visitors ever see this variant (logged-in users are auto-claimed), so the buttons now say what they do: Sign up / Log in, with the description explaining that either one claims the badge.
Keeps all per-variant strings (title, description, ctaLabel, loginLabel) in one place instead of burying one ternary in the JSX.
feat(kyc): honest UK-resident block on Bridge bank flows (TASK-20729)
…copy fix: honest signup/login CTAs on vanity badge claim screen
SimpleFi (early Argentina QR provider) is deprecated and its creation path was removed in peanut-api-ts#1218. Drop the FE-side remnants: the payment-app logo asset + export, the DEPRECATED_SIMPLEFI avatar branch in getAvatarUrl, and the SIMPLEFI keys in test mocks. Historical rows are safe: 247 transaction intents with provider DEPRECATED_SIMPLEFI still arrive on the wire, so the Provider union keeps the value (mirrors DEPRECATED_SQUID), their kind=QR_PAY still dispatches to the qrPay strategy (locked by the vendored qr_pay-deprecated_simplefi render-snapshot case), and getAvatarUrl now falls back to undefined -> generic default avatar (new test).
Grafana has been decommissioned at Peanut; support agents now use the prod DB + PostHog per the updated ops playbook (mono 2d873bc). Drop the dead links the FE still generated: the Crisp session-data grafana_dashboard entry (useCrispUserData / crisp.ts / useCrispProxyUrl / crisp-proxy page), the GRAFANA_DASHBOARD_BASE_URL constant, and the dev invites-graph click-to-Grafana handler (click now just selects the node). The Crisp session-data push itself is unchanged beyond the deleted entry.
…afana chore: remove decommissioned SimpleFi + Grafana remnants
…ed intent
Manteca entry points (MantecaAddMoney, qr-pay, withdraw/manteca,
MantecaFlowManager) instantiate useMultiPhaseKycFlow({}) and pass 'LATAM'
only as an override to handleInitiateKyc, so completion/abandon captures
read the undefined hook-level regionIntent: LATAM successes fired as
kyc_approved with region_intent None and abandons as kyc_abandoned. That
made manteca_kyc_completed undercount and paged a false 'Funnel dead:
manteca-kyc' alert (2026-07-22). Track the last initiated intent in a ref
and let it win over the prop for all lifecycle captures.
…s-attribution fix(analytics): attribute manteca KYC lifecycle events to the initiated intent
…ithdraws Same-chain crypto withdraws funded from Rain card collateral succeeded on-chain but never completed their charge intent — the page skipped recordPayment and never passed chargeId, so the charge rotted PENDING until the reaper failed it. History hides both rows (phantom + abandoned-draft filters), so users watched their balance drop with no Activity entry (98 charges / 55 users in 14 days). Mirror the direct-send contract: pass chargeDetails.uuid into sendMoney (the backend uses the charge as the prep and settles it in /submit's trusted completion) and always recordPayment — mixed spends rely on it, collateral-only re-enters trusted completion idempotently as the recovery net. recordPayment failures after a collateral-routed same-chain spend degrade to the success view instead of a false 'failed': the funds have already moved. Also refresh the three stale hook/service comments that instructed callers to SKIP recordPayment on collateral-only — the guidance that seeded this bug.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdates crypto withdrawal confirmation so collateral-routed charges receive the charge UUID, payment recording is attempted with route-specific safeguards, and failures are handled differently across collateral, mixed, cross-chain, and smart-only flows. Adds regression tests covering these scenarios. ChangesCrypto withdrawal reconciliation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WithdrawCryptoPage
participant sendMoney
participant recordPayment
participant Sentry
WithdrawCryptoPage->>sendMoney: Submit withdrawal with charge UUID
sendMoney-->>WithdrawCryptoPage: Return transaction and strategy
WithdrawCryptoPage->>recordPayment: Record mined transaction payment
recordPayment-->>WithdrawCryptoPage: Resolve or reject
WithdrawCryptoPage->>Sentry: Capture skipped or downgraded recording diagnostic
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
Contributor
Code-analysis diffPainscore total: 6233.75 → 6235.1 (+1.35) 🆕 New findings (26)
…and 6 more. ✅ Resolved (26)
…and 6 more. 📈 Painscore deltas (top movers)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
Adversarial review find: on a receipt-wait timeout the kernel path returns only the userOp hash. Feeding that to the validator for an untagged mixed charge can never match a tx — retry exhaustion would flip the successful withdrawal to FAILED, worse than the pre-fix stuck PENDING. Skip the record with a Sentry breadcrumb in that case (collateral-only stays safe: backend-broadcast tx hash + trusted-path settlement). Also pin cross-chain rethrow + this skip in tests.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/`(mobile-ui)/withdraw/crypto/__tests__/crypto-withdraw-confirm.test.tsx:
- Around line 351-367: Extend the cross-chain mixed withdrawal test coverage
around confirm to mock a successful sendTransactions result with a userOpHash
but receipt: null, then verify the unmined transaction path handles
recordPayment correctly without using a mined transaction hash. Preserve the
existing assertions for failure reporting and ensure the test specifically
exercises the mixed cross-chain scenario.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 28985bd7-aa79-44f3-8fea-d3babc889156
📒 Files selected for processing (5)
src/app/(mobile-ui)/withdraw/crypto/__tests__/crypto-withdraw-confirm.test.tsxsrc/app/(mobile-ui)/withdraw/crypto/page.tsxsrc/hooks/wallet/useSendMoney.tssrc/hooks/wallet/useSpendBundle.tssrc/services/rain.ts
…ce + isDiffToken routing Structured review pass: the bug-seeding 'skip recordPayment' guidance survived in two more comments (useSpendBundle prepare call-site, useWallet sendMoney return doc) — the exact rot this PR exists to purge. Also close the two coverage gaps a scoping regression could slip through: mixed same-chain tolerance and token-boundary-only routing.
Contributor
Author
|
Superseded by #2491 — same 5 commits cherry-picked onto |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Crypto withdraws funded from Rain card collateral (wallet at $0 after auto-sweep — the norm for card users) succeed on-chain but stay stuck PENDING and never appear in Activity, while the balance visibly drops. Users read this as lost funds — 98 stuck charges / 55 users in the last 14 days, all
CRYPTO_WITHDRAW(Notion P2 task).Root cause: the withdraw page was the only charge-backed spend flow that neither passed
chargeIdintosendMoneynor calledrecordPaymentfor collateral-routed same-chain spends. The user-facing charge intent therefore never completed. History's read side was already built expecting the charge to be the canonical row: it hides never-paid charges as "abandoned drafts" and hides the rain-prepare intent as a "phantom" — so a successful withdrawal was completely invisible.The fix mirrors the proven direct-send / contribute-pot / semantic-request contract:
chargeDetails.uuidaschargeId— oncollateral-onlythe backend uses the charge as the preparation and settles it server-side in/submit(trusted completion; live on prod since08004d52).recordPayment—mixedspends rely on it (the kernel's plainusdc.transfer(recipient)is validator-matchable); forcollateral-onlythe backend routes it through the same trusted-completion path idempotently — the designed recovery net when/submit's post-mining bookkeeping fails (the backend comment explicitly names FErecordPaymentas that recovery).useSpendBundle,useSendMoney,services/rain) that instructed callers to SKIPrecordPaymenton collateral-only — the guidance that seeded this bug.Design notes / accepted trade-offs
recordPaymentfailures on collateral-routed same-chain paths are tolerated (Sentry warning + success view without payment details): funds have already moved, and forcollateral-onlythe backend has already settled the charge. A mixed-path double failure leaves the charge PENDING exactly as pre-fix — never worse than today.collateral-onlyis unaffected: its hash is a backend-broadcast EVM tx and its charge settles via the trusted path, receipt or not.origin/dev:recordPaymentalways fired there and still hard-fails on error.chargeIdcloses a pre-fix double-broadcast hazard — retrying after "submit succeeded but FE errored" now gets a 409 (Charge already completed) from/prepareinstead of silently preparing + broadcasting a second collateral withdrawal.Risks / breaking changes
recordPaymentre-entry contract is live on prodmain.smart-onlyand cross-chain paths verified unchanged (independent review sweep of everysendMoney/spendcaller found no other flow with this bug shape).QA
npm test: 148 suites green. New suitecrypto-withdraw-confirm.test.tsx(6 tests): chargeId reachessendMoney; mixed same-chain records the mined hash; collateral soft-fail still lands on the success view; mixed-without-receipt skips the record; cross-chain rethrow preserved; smart-only hard-fail preserved. The bug-pinning tests verified to fail against pre-fixorigin/dev.typecheck+prettierclean.Summary by CodeRabbit