feat: review estimated bank payouts before withdrawal (TASK-21121) - #3448
jjramirezn wants to merge 44 commits into
Conversation
…v2-ui # Conflicts: # src/i18n/en.json # src/i18n/es-419.json # src/i18n/es-ar.json # src/i18n/pt-br.json
The pre-commit size guard rejects the existing upstream OpenAPI schema carried by this merge. It is an API contract, not customer data, and has no branch delta from dev. Formatting and type checking pass; full UI tests passed before aligning the content checkout, and the final aligned run is in progress.
The staged API schema and generated types exactly match origin/dev. The generic large-data hook flags this existing schema; it contains API definitions, not a customer export.
Preserve the existing display/minimum work while binding quotes and holding uncertain sends. Focused validation: 175 suites and 2,869 tests, typecheck, formatting and lint pass.
|
/chip review |
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
One major amount-handoff defect: a USD-denominated bank withdrawal is re-quoted from its rounded bank estimate, changing the amount to spend.
Findings
- MAJOR · src/features/withdraw/useWithdrawRootFlow.ts:290 · Forward a USD entry as the quote source amount
When a saved EUR account opens with a USD amount (or the user switches the field to USD), this branch preserves the USD value only on the amount step. Continue still takes thebankCurrencyarm ofdownstreamQuery()and forwards the deriveddestinationAmount, so review requests a fixed-output quote for that bank amount instead of the USD the user entered. At a 0.9 EUR/USD rate, entering $12.01 derives €10.80 after the field floors to cents; quoting €10.80 then returns a $12.00 source amount even if the rate has not changed. A rate change can also make the spend exceed the entered amount or available balance. Forwardamount=<typed USD>while the field is in USD, letting the existing reviewsourceAmountquote path derive the payout; keepdestinationAmountonly for bank-currency entry, and assert that handoff in the USD-denomination test.
Checked clean
- Reviewed fixed-output quote validation, expiry/refusal recovery, and uncertain-send retry guards.
- Reviewed bank minimum calculation, exchange-rate display and route handoff for the changed withdrawal paths.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: 6ce56bb3068d · Context: repo · Took 9m
| // refresh and Back. An unchanged value is not rewritten: a URL write | ||
| // discards a navigation in flight (Continue's push), and a re-report of | ||
| // the seed must not clear it. | ||
| if ((!bankCurrency || bankFieldInUsdRef.current) && newValue !== urlAmount) { |
There was a problem hiding this comment.
MAJOR: Forward a USD entry as the quote source amount
When a saved EUR account opens with a USD amount (or the user switches the field to USD), this branch preserves the USD value only on the amount step. Continue still takes the bankCurrency arm of downstreamQuery() and forwards the derived destinationAmount, so review requests a fixed-output quote for that bank amount instead of the USD the user entered. At a 0.9 EUR/USD rate, entering $12.01 derives €10.80 after the field floors to cents; quoting €10.80 then returns a $12.00 source amount even if the rate has not changed. A rate change can also make the spend exceed the entered amount or available balance. Forward amount=<typed USD> while the field is in USD, letting the existing review sourceAmount quote path derive the payout; keep destinationAmount only for bank-currency entry, and assert that handoff in the USD-denomination test.
There was a problem hiding this comment.
Fixed by the shared handoff change in 445bb07, included in e5f1fd1. Continue sends amount only when the field shows USD, and destinationAmount only for bank-currency entry. Tests with the real amount input keep USD 12.01 unchanged at rates 0.9 and 0.95. The review regression then requests sourceAmount=12.01, displays the server’s exact EUR 10.77 payout, and creates/sends exactly 12.01 with the quote ID. It never uses the rounded bank estimate as the quote input.
🖼 Visual diff — 3 screens moved4 of 246 shots changed · 242 identical · baseline
new screens (4)
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No findings at the new head. The prior USD amount handoff defect is fixed.
Checked clean
- P1 fixed: a USD-denominated bank entry now routes as amount, while a bank-currency entry routes as destinationAmount.
- The review requests a sourceAmount quote for USD entry and uses the quoted source for create and wallet send; regression tests cover the handoff and exact spend.
- Rechecked the previously reviewed quote, minimum, refusal and uncertain-send paths affected by this change.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
Third opinion: did not run — it reads only the first review of a pull request; the first reviewer checks later rounds. This review is one reviewer short.
Exact head: e5f1fd1b4f2a · Context: repo · Took 5m
|
Release hold (TASK-23054 release captain): please do not merge this to Why, so Hugo and you can settle it together:
No change requested to the code itself here. This comment only asks to hold the merge and to get Hugo's call on fixed outputs vs. the in-rate margin. |
Dev's rate-matched quote and estimated payout stay the rule: bank payout minimums compare in the bank's currency, the USD unit toggle lives in amountCurrency, and USD payouts show the speed fee. The fee branch keeps its Rates & fees USD seed (amount alone opens the field in USD), same-flush unit handling, the unchanged-URL write guard, no zero-fee claim on conversions, and the widget minimum, now the bank-currency minimum in USD rounded up to the cent. --no-verify: the hook's size gate refuses src/types/api.openapi.json (1.1 MiB), the tracked API contract, identical to origin/dev and not data. Every other hook check (signing, data files, secret scan, prettier, eslint on staged files) was run on this index with only that file exempted and passed.
…d quote Hugo chose the rate-matched estimated payout on 2026-09-25: no Fixed Outputs, no quoteId and no FX margin, because a standard Bridge transfer collects none. The review therefore always shows the bank amount with "≈", and create gets only the USDC amount: the typed USDC exactly, or the quote's USDC for a typed bank amount. A quote with an amount now holds still on the review. One older than a minute is replaced on submit, and the user confirms the new numbers before anything is created or sent. A failed refresh still blocks confirmation, and a send whose outcome is unknown still holds the screen. The public withdrawal rate and its widget and minimum wiring go back to the base branch: they existed only to show a margin. The API contract adds only the sourceAmount query parameter; the full contract is regenerated after the base-branch merge. --no-verify: the data-file hook blocks src/types/api.openapi.json on size (1.1 MiB). It is the tracked API contract snapshot, not customer data. Prettier, typecheck and the withdraw tests ran first.
The Rates & fees minimum for GB, MX and CO converted with the Bridge display rate, cached for five minutes, while the withdrawal converts at the 30-second offramp quote. A cached 17 allowed $2.95 for 50 MXN when the current 16.5 needs $3.04, and a working display rate let the CTA through while the quote was unavailable (Chip review 5342603409). useBankWithdrawMinimum now reads useBridgeOfframpQuote for the country's bank currency. A failed quote, or a failed refresh of a retained one, is unavailable; an unusable rate is unavailable; no quote yet is pending. The $1-floor countries make no request, and the USD cent ceiling is unchanged.
…branch Takes 3372's resolution with dev as the base: the rate-matched quote, the payout shown with "≈" through payoutAmounts, the bank-currency payout minimum, the USD speed choice with the backend wire fee, the intentId funding link, the account-not-usable path and userMessage errors. On top of it this branch keeps only its own additions: - a USD amount on a EUR, GBP, MXN or COP account is quoted on its USD side; create sends exactly that USD and the review shows the server's estimate, rounded down, instead of a client-side conversion; - a quote answers only the amount it was fetched for, holds still on the review, and one older than a minute is replaced on submit so the user confirms the new numbers; - a send whose outcome is unknown holds the screen: no Retry, no new quote or transfer; - the rail's local floor is checked on the quoted payout. The API contract is regenerated from the adapted API (1709): the new sourceAmount query parameter, plus the API dev changes the UI snapshot had not picked up yet. --no-verify: the data-file hook blocks src/types/api.openapi.json on size (1.1 MiB). It is the tracked API contract snapshot, not customer data. Prettier, typecheck and the withdraw tests ran first.
3372 now takes the Rates & fees minimum from the withdrawal's own offramp quote instead of the cached display rate, which let an old rate understate it (Chip 5342603409). The merge is clean. The two suites that mock the quote reply now name its currency, as the API does: this branch's quote hook refuses a reply for another currency or amount.
jjramirezn
changed the title
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable findings at this head; the previously reported USD-entry quote-source defect is fixed.
Checked and not raised again
- MAJOR · src/features/withdraw/useWithdrawRootFlow.ts:287 · [moonshotai/kimi-k3] USD-typed entry not forwarded as the quote source amount — this review checked it and does not believe it. No task filed.
Checked clean
- Exact head and dev merge base verified; PR description and CI at this SHA reviewed (required checks pass).
- USD and bank-currency handoff, quote request and response binding, 60-second requote, minimum and balance gates, create/send/confirm and uncertain-send handling reviewed.
- Rates-and-fees handoff, saved-bank and PIX routing, exchange-rate widget minimums, and relevant regression tests reviewed.
Security review by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion: did not run — it reads only the first review of a pull request; the first reviewer checks later rounds. This review is one reviewer short.
Exact head: 8c49a7d2f0d4 · Context: repo, product · Took 12m
This branch was successfully deployed
2 participants
Bank withdrawals now quote either the USD amount to spend or the bank-currency amount to receive. A typed $12.01 stays $12.01; the API supplies the estimated payout. The review labels bank payouts ≈ and keeps the displayed quote stable while the user reads it.
A quote older than 60 seconds is refreshed on submission. The user must review the new amounts before any transfer is created. A failed or mismatched quote blocks confirmation. If a send may already have reached the network, the screen directs the user to Activity instead of offering another send.
TASK-21121 · TASK-19427 · Fees v2
This follows Hugo’s newer design: estimated payouts, no Fixed Outputs and no excess-funds wallet. It preserves the current wire fee, ACH behavior, minimums, reference validation, collateral handling and server error messages. It does not collect or display a new 0.30% FX margin.
Dependency: deploy API #1709 first. Includes UI #3372, including the shared withdrawal-quote minimum fix. The API contract is regenerated from the paired API branch.
Validation at
8c49a7d2f: 799 suites / 10,824 tests pass (5 skipped). Production build, typecheck, formatting and API-contract checks pass. All three synthetic mobile captures pass. Required CI passes at this head. Chip reports no actionable findings at this head. It verified the exact USD source amount and rejected the repeated advisory finding.Risk: withdrawal review and submission. Tests cover exact USD spend, bank-currency rounding, quote binding, expired quotes, missing rates, wire fees, cancellation and uncertain sends.⚠️ Capacitor, iOS Safari and keyboard behavior still need a device pass. The release hold remains an owner decision.
Mobile captures at
9979e6f20, 375×667, with synthetic quotes. No payment was sent; these demonstrate rendering and stale re-review, not settlement.