TASK-23054 fix withdraw Continue on EUR amounts and the crypto address book step - #3443
Conversation
…s book step Continue on a EUR amount did nothing on staging. AmountInput re-reported the amount on every parent render because it depended on the secondaryDenomination object literal. Since the bank-currency field reports into URL state, every render wrote the URL, and in Next.js a URL write restores the router and discards the navigation in flight. Continue re-renders the page, so its push to the review never landed. The field now reports only real changes, and the flow skips writing an unchanged bank amount. Withdraw -> Crypto on the full method list opened an empty destination form while saved addresses existed; it now returns to the saved-destinations screen. QA-30's per-rail address count also made Back from the bank list leave Withdraw for users with only crypto addresses saved. Audit follow-ups on the same flows: the minimum converts with the quote rate (one rate source), the legacy rate hook fails instead of inventing 1:1, a failed quote refresh keeps the review and any open KYC or terms step mounted with an inline retry and never confirms the stale quote, mid-typing amounts are normalized to what the quote API accepts, and the Manteca lock expiry comes from the remaining time the API measured instead of the phone clock.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: peanutprotocol/peanut-ui/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code-analysis diffPainscore total: 8802.34 → 8807.32 (+4.98) 🆕 New findings (85)
…and 65 more. ✅ Resolved (82)
…and 62 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![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 minor retry-button issue; no blocking defect found.
Findings
-
MINOR · src/features/withdraw/views/WithdrawBankReviewView.tsx:247 · Disable the submit Retry while the quote is unavailable
If a submit first fails and sets error.showError, then the next 30-second quote refresh fails, the new inline quote Retry appears but the footer's submit Retry remains enabled. Clicking the footer Retry calls onSubmit, which now returns immediately when isQuoteCurrent is false, so the button appears to do nothing. Disable that footer Retry when !isSubmitReady (or route it to quote refetch) while the quote is failed. -
MAJOR · src/features/withdraw/price-lock.ts:17 · [claude-opus] Manteca lock expiry reads expiresInMs, which the API never sends
receivePriceLockonly setsdeadlinewhen the response includesexpiresInMs.isPriceLockExpiredreturns false wheneverdeadlineis undefined. ThelockExpiresAtvalue passed to the Rain-cooldown recovery (two places in withdraw/manteca/page.tsx) is nowpriceLock.deadline; before this change it was worked out fromexpiresAt. In peanut-api-ts, POST /manteca/withdraw/init (src/routes/manteca/withdraw.ts:238-247) returnspriceLockCode, price, expiresAt, usdAmount, fiatAmount, currency, depositAddress, legalEntityand noexpiresInMs. None of the open TASK-23054 PRs in peanut-api-ts (#1702, #1705, #1706) add it. Until the API sends it, two protections that worked before are turned off on every Manteca bank withdrawal. (1) The check before signing never sees an expired quote, although the code comment says an expired quote leads to a re-lock and reconfirm, never a signature and never a submission. The app now signs the transfer and relies on Manteca to reject the expired lock. (2) The recovery step no longer knows when the lock expires, so it can no longer wait out a Rain cooldown inside the lock and re-quotes instead. The author knows the field can be missing ('as it did before TASK-23054'), but it is missing on every response today, not now and then. The other half may be an open peanut-api-ts PR this review cannot see, so this is major rather than blocking. Fix: addexpiresInMsto the /withdraw/init response and its response schema in peanut-api-ts before or alongside this change. Or, whenexpiresInMsis missing, fall back toDate.parse(expiresAt)so this change does not weaken the current behaviour.
Checked clean
- Exact head, PR metadata, merge base, and changed withdraw paths inspected.
- Quote amount pattern and response fields checked against the sibling API checkout; they match.
- AmountInput reporting and URL-change regression tests inspected.
- Price-lock fallback matches the PR's stated API follow-up: expiresInMs is not yet sent by the sibling API.
- At review time, typecheck and native-export passed; unit and press-contract checks were still in progress.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: cf3fa10f081b · Context: repo, sibling-api · Took 7m
…destinations api#1707 sends expiresInMs with the withdraw and QR-pay locks. Both flows now stamp each lock with a device-clock deadline when it arrives (receiveLock), so a phone clock that runs fast no longer expires a live lock and re-quotes on every tap. Without the field there is no device-side check. The helper moved to utils so withdraw and QR pay share one. Back from the crypto and Manteca pages with no in-app history landed on the full method list (?showAll=true), which skips the saved addresses; it now lands on /withdraw, which shows the saved destinations when there are any. The submit Retry on the bank review now also waits for a current quote: after a failed submit and a failed quote refresh it looked live and did nothing.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
P1 is fixed; the Manteca lock-expiry dependency remains until the API supplies the TTL.
Findings
- MAJOR · src/utils/price-lock.utils.ts:20 · Deploy the server-measured lock TTL before relying on it
P2 still applies at this head and now also affects QR Pay. The current API checkout returns expiresAt/expireAt but no expiresInMs from either Manteca init route. For that response, receiveLock sets no deadline, so a lock left open past its expiry is signed and sent instead of being re-quoted before signing; fixed-amount QR warm-up also has no expiry and is skipped. The provider may reject the expired lock, but the intended pre-sign recovery is lost. Ensure the paired API TTL change is deployed before this UI behavior, or retain a safe transition path for old API responses. This is a rollout dependency, not evidence that the paired API work is absent.
Checked clean
- P1 fixed: submit Retry now shares isSubmitReady with normal submit, with a failed-refresh test.
- New QR and Manteca lock paths, crypto/Manteca Back fallbacks, and relevant tests inspected at this head.
- Current sibling API policy checkout returns no expiresInMs on Manteca withdraw or QR init; an open paired API change is not in that checkout.
- Earlier withdraw amount, quote, and deposit-copy changes remain covered by the prior review; no new finding there.
- CI checks for this exact head were not yet available in the check-runs response at review time.
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: a51635dfcbdf · Context: repo, sibling-api · Took 6m
|
@chip defer — re the server-measured lock TTL finding: api#1707 (expiresInMs on both Manteca init routes) merges before this PR. Without it the UI skips the phone-clock check entirely, i.e. the pre-api#1703 behaviour, never worse. |
🖼 Visual diff — 5 screens moved7 of 164 shots changed · 157 identical · baseline
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. |
|
English · Español · Español (Argentina) · Português (Brasil) After merge: f67137f → 908950d. Capture complete in all locales. |
This branch was successfully deployed
1 participant
Two withdraw regressions from staging QA on 2026-09-24 (TASK-23054).
1. Continue on a EUR amount did nothing (blocker)
Cause. In
AmountInput, the effect that reports the typed amount to the parent listed thesecondaryDenominationprop as a dependency. Every caller passes that prop as an object literal, so the effect ran again on every parent render. Since #3438 the bank-currency field reports intosetDestinationAmount, which is URL state (nuqs). nuqs writes the URL on every call, even when the value is the same. Under Next.js each URL write dispatches a router restore, and a restore discards the navigation still in flight. Continue re-renders the page (setError), so the push to/withdraw/<country>/bankwas discarded every time. The UI never reached the review, so the API saw no quote or offramp request.Fix.
AmountInputdepends on the field-keyeddenominationsmemo, not the prop object. It reports only when an amount or a denomination really changes.handleDestinationAmountChangeskips an unchanged amount. A 30-second quote refresh re-reports the same bank amount and would otherwise write the URL again.The USD amount step and Manteca are not affected: the USD step has no secondary denomination, and Manteca stores its amounts in
useState.2. Withdraw → Crypto skipped the address book
?showAll=true, for example the fallback when Back leaves the crypto or Manteca page with no in-app history) went from Crypto straight to an empty destination form. With saved addresses, Crypto now returns to the saved-destinations screen, where the addresses are listed.hasSavedDestinationsfrom the addresses offered on the current rail. For a user whose only saved destinations are crypto addresses, Back from the bank list (rail=bank) left Withdraw instead of returning to the address book. The count now leaves the addresses out only for Send → Bank.3. Withdraw quote and rate follow-ups (code audit W4, W1 rows)
/bridge/exchange-ratehook.useGetExchangeRateanswered'1'when the call failed. That rate showed "1 USD = 1.0000 EUR" and turned the COP minimum into $4,000. A failed call is now an error with no rate, andExchangeRateshows "-".useBridgeOfframpQuotekeeps the last quote when a 30-second refresh fails, andisErrorshows that the quote is not current. The review page shows the Retry screen only before the first quote. After a failed refresh, the review shows the error inline with a retry, and an open KYC, terms or confirm step stays open. Submit is disabled, and the handler also refuses a quote that is not current, because the terms step calls the handler directly. On the amount step the field stays open with the last rate."90."or".5"goes into the URL, it is converted to the form the API accepts ("90","0.5"). The review page reads the URL the same way, and an amount the API refuses counts as no amount, so the review sends the user back to/withdrawinstead of showing "rate unavailable" in a loop.receiveLockinsrc/utils/price-lock.utils.ts): the time left that api#1707 measures (expiresInMs), counted from arrival. A phone clock that runs fast no longer expires a live lock. Without the field there is no device-side check, and the provider rejects an expired lock on submit. The QR warm-up (useSmartSpendPreparation) takes the same deadline. The openapi snapshot declares no response body for these routes, so no type regeneration is needed./withdraw?showAll=true, the full method list, which skipped the saved addresses. They now fall back to/withdraw. It shows the saved destinations when the user has any, and the method list otherwise.Tests
withdraw-states.test.tsx: the realAmountInputon a EUR account, with URL writes that re-render the page. Type EUR 5, re-render, and assert that nothing writes the URL and that Continue pushes/withdraw/germany/bank?destinationAmount=5. This test fails on dev.AmountInput/__tests__/reporting.test.tsx: a parent re-render with equal denominations reports nothing. A rate change still reports again.WithdrawMethodView.test.tsx: Crypto with saved addresses shows them. Crypto with nothing saved opens the form. Back from the bank list returns to the address book. Send → Bank still leaves the flow on Back and never shows the address book.useBridgeOfframpFlow.test.tsx: the GB minimum converts with the quote rate. A failed refresh keeps the quote but submit is blocked, also through a direct handler call.90.and.5are quoted as90and0.5.12.345sends the user back to the flow entry.withdraw-states.test.tsx: a failed refresh keeps the field open. £2 at 0.79 is under the £3 minimum, with no call to the old rate hook. A typed90.is handed on as90.WithdrawBankReviewView.test.tsx: the inline quote error with retry.useBridgeOfframpQuote.test.tsxanduseGetExchangeRate.test.tscover the new failure behavior.bank-amount.test.tscoversnormalizeBankAmount.price-lock.utils.test.ts,manteca-withdraw-gates.test.tsxandqr-pay-states.test.tsx: a lock with no time left re-quotes. A phone clock that runs fast does not expire a live lock, and neither does a response withoutexpiresInMs.useSmartSpendPreparation.test.tsxuses the device deadline.crypto-withdraw-confirm.test.tsxandmanteca-withdraw-gates.test.tsx: Back without history falls back to/withdraw.WithdrawBankReviewView.test.tsx: the submit Retry waits for a current quote.Local:
tsc --noEmit, eslint on the changed files, full jest (--runInBand) andnpm run buildpass.