fix(withdraw): show the cross-chain withdrawal cap message and retry the route (TASK-22154) - #2931
fix(withdraw): show the cross-chain withdrawal cap message and retry the route (TASK-22154)#2931abalinda wants to merge 8 commits into
Conversation
…te the route (TASK-22154) peanut-api-ts #1497 answers POST /rhino/sda-transfer with 429 { error, code: XCHAIN_WITHDRAW_LIMIT_REACHED, retryAfterSec } once a user is over the per-user cross-chain withdrawal cap. Today postRhino throws "Failed to provision SDA transfer: 429 {json}" and the confirm view shows that string; its Retry then calls onConfirm with no prepared transactions and dead-ends on "transaction not prepared". - postRhino throws an ApiError (message = backend text, plus code and retryAfterSec), and apiErrorFromResponse now carries retryAfterSec. - friendlyError maps XCHAIN_WITHDRAW_LIMIT_REACHED to localized copy that states the wait in the coarsest unit (minutes / hours / days) and points at Arbitrum (no limit) and support; en, es-419, es-AR, pt-BR. - useCrossChainTransfer surfaces errors through useFriendlyError instead of echoing err.message. - Retry after a route error recomputes the route instead of failing on the transactions the failed route never built. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
GitHub Actions
Inactive
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds structured cross-chain limit errors, propagates bridge context and retry metadata, localizes the messages, and recalculates unavailable withdrawal routes when users retry. ChangesCross-chain withdrawal limit handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This change improves cross-chain cap messaging, preserves cap attribution for bridge quotes and commits, and safely recalculates routes on retry. No unresolved merge-readiness risk is identified. Sequence Diagram(s)sequenceDiagram
participant CryptoWithdrawPage
participant useCrossChainTransfer
participant RhinoBridge
participant ErrorClassifier
CryptoWithdrawPage->>useCrossChainTransfer: calculateCurrentRoute()
useCrossChainTransfer->>RhinoBridge: request quote with withdrawal context
RhinoBridge-->>useCrossChainTransfer: return structured limit error
useCrossChainTransfer->>ErrorClassifier: classify error with retryAfterSec
ErrorClassifier-->>useCrossChainTransfer: return localized friendly error
useCrossChainTransfer-->>CryptoWithdrawPage: display route error
CryptoWithdrawPage->>useCrossChainTransfer: retry calculation
useCrossChainTransfer->>RhinoBridge: request quote again
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 10 files. (8 skipped: 8 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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: 7753.44 → 7758.44 (+5) 🆕 New findings (25)
…and 5 more. ✅ Resolved (24)
…and 4 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 — changes requested
Request changes: the intended 429 Retry path still dead-ends because handleConfirmWithdrawal closes over the pre-error routeError.
Findings
-
MAJOR · src/app/(mobile-ui)/withdraw/crypto/page.tsx:346 · Retry reads a stale route error
handleConfirmWithdrawal now branches on routeError, but its useCallback dependency array still omits both routeError and calculateCurrentRoute. On a cross-chain confirm, the callback is memoized while routeError is null; when provisioning later returns the cap 429, only routeError changes, so clicking Retry can still fall through to txNotPrepared instead of recalculating. Add both dependencies and a regression test that rerenders from transactions=null/error=null to transactions=null/error=<429 message>, clicks Retry, and asserts calculate is called again. -
MINOR · src/utils/friendly-error.utils.tsx:219 · [moonshotai/kimi-k3] Cap wait time is rounded down, so users retry while still blocked
For XCHAIN_WITHDRAW_LIMIT_REACHED the displayed wait is computed with Math.floor at the chosen unit: days = Math.floor(minutes / 1440), hours = Math.floor(minutes / 60). Any retryAfterSec that is not an exact multiple of the displayed unit under-reports the wait — e.g. retryAfterSec = 47h gives minutes=2820, days=1, so the ICU message renders 'Try again in about 1 day' when the cap actually lifts in ~2 days; 119 minutes renders 'about 1 hour' when ~2 hours remain. A user who trusts the message and retries at the stated time hits the 429 again (the new Retry path re-provisions the route and re-fails), looping the same error. The PR's own test encodes this (at(2*86400+60) -> days: 2). Fix: round the displayed unit up, e.g. const days = Math.ceil(minutes / 1440) and const hours = Math.ceil(minutes / 60) (or keep floor but only when the remainder is 0), so the copy never promises a retry earlier than retryAfterSec. -
MAJOR · src/i18n/app/messages/en.json:3185 · [claude-opus] Cross-chain withdrawal cap contradicts product/ and the live help pages
This PR ships user-facing copy for a per-user cap on withdrawals to non-Arbitrum networks (src/i18n/app/messages/en.json:3185and the three other locales), backed by peanut-api-ts#1497's rungs of 10/hour, 20/day, 30/30-days. Product truth says the opposite in five places: -
product/networks.md:339— "Crypto send | None | None | No limits on crypto withdrawals" -
product/kyc.md:37— "Crypto deposits and withdrawals have no KYC requirement. No limits either." -
product/kyc.md:64and:206— "No limits on crypto transactions" / "No limits — send and receive freely between wallets" -
product/quick-ref.md:76— "Crypto | No hard limit"
The generated customer-facing pages inherit it, and they are exactly the pages for the destinations this cap applies to: content/withdraw/solana/en.md:27, content/withdraw/base/en.md:27, content/withdraw/ethereum/en.md:27, content/withdraw/polygon/en.md:27, content/withdraw/tron/en.md:27 all read "No maximum. There are no limits on wallet withdrawals", and content/help/transaction-limits/en.md:20 says "Crypto operations have no limits at all" with an FAQ at :67 repeating it.
The code is the correct side — the cap is deliberate (TASK-22154, 748 x $0.50 withdrawals in 30 days). product/ is the stale side. A user who hits this will read "You reached the limit for withdrawals to other networks" in the app and "there are no limits on wallet withdrawals" on peanut.me for the same network, and support has no documented number to quote or exemption process to point at.
Fix: before the backend's xchain_withdraw_cap.enabled flag is flipped, update product/networks.md, product/kyc.md (both the no_kyc_required note and limits.crypto) and product/quick-ref.md to state the cross-chain rungs, the Arbitrum exemption and the raise-via-support path, then regenerate the affected content/withdraw/* and content/help/transaction-limits pages through the update-content path. The flag default keeps this off at merge, so it is a pre-flip blocker rather than a merge blocker.
-
MAJOR · src/features/payments/shared/hooks/useCrossChainTransfer.ts:332 · [claude-opus] Every entry into the confirm view mints a new charge and burns a cap slot
peanut-api-ts#1497 reserves a cap slot at provision time and reasons that "opening the confirm screen and backing out costs nothing lasting" because the reservation expires afterXCHAIN_RESERVATION_MS(10 min). That holds only if one withdrawal attempt provisions once. In peanut-ui it does not: -
useCrossChainTransfer.calculatecallsprovisionSdaTransfer(src/features/payments/shared/hooks/useCrossChainTransfer.ts:332) during route calculation, which runs on entering CONFIRM (src/app/(mobile-ui)/withdraw/crypto/page.tsx:196-198) — before the user has seen the fee or signed anything. -
Backing out calls
handleBackFromConfirm, which clearschargeDetails(page.tsx:560-564), and re-reviewing runshandleSetupReview, which creates a fresh request + charge every time (page.tsx:258-286).
So each review pass is a distinct contextId, gets its own rhinoProvisionedAt stamp, and holds its own slot for 10 minutes — the destination-binding idempotency in #1497 keys on the charge and cannot collapse them. The hourly rung is 10. A user comparing destinations (Solana, back, Base, back, Optimism...) accumulates one reservation per pass, and completed withdrawals in the same hour count too: someone who legitimately made 6 cross-chain withdrawals this hour and then browses 4 destinations is refused with "You reached the limit for withdrawals to other networks" having initiated nothing. This PR's own Retry path is fine — it reuses the same charge, so it is idempotent.
The other half is an open PR I cannot see in full, so the fix may belong on either side: have the backend count only funded/committed provisions (or shorten the reservation), or have peanut-ui reuse the existing charge across confirm re-entries instead of creating a new request+charge, or defer provisionSdaTransfer from route calculation to the user's confirm tap. Worth settling before the flag is flipped, since neither repo's tests exercise the multi-charge case.
- MAJOR · src/app/(mobile-ui)/withdraw/crypto/page.tsx:346 · [claude-opus] New Retry branch in the withdraw confirm handler has no test
handleConfirmWithdrawalis the handler that broadcasts the withdrawal, and this PR adds a new control-flow branch to it: when there are no prepared transactions and a route error is set, clear the errors, re-runcalculateCurrentRoute()and return, instead of settingerrors.txNotPrepared(src/app/(mobile-ui)/withdraw/crypto/page.tsx:346-352). No test is added for it.
CONTRIBUTING.md:495 makes this a hard rule ("if code moves money or mutates shared state, it needs a test before merge"), and :508 extends it to anything that gates a flow. The harness for it already exists and needs no new scaffolding: src/app/(mobile-ui)/withdraw/crypto/__tests__/crypto-withdraw-confirm.test.tsx mocks useCrossChainTransfer as a mutable object (line 240) whose error field defaults to null (line 236), and fires the handler through the confirm-withdraw test-id button (line 122). It already has a crypto withdraw retry describe block (line 439).
The exact untested case: with mockCrossChainTransfer.error set to the cap message and transactions empty, tapping Retry must call mockCrossChainTransfer.calculate again and must NOT set errors.txNotPrepared — and the mirror case, error: null with transactions empty, must still set errors.txNotPrepared so the existing dead-end guard is not lost. Note this branch re-enters the provisioning path, so it is one tap away from the money-moving leg, not inert.
Inline anchors unavailable for 1 finding(s); the findings remain in this summary.
Checked clean
- Exact detached HEAD, supplied base SHA, and merge-base match; CI check-runs are green.
- Paired API response contract emits 429 with XCHAIN_WITHDRAW_LIMIT_REACHED and positive retryAfterSec.
- ApiError parsing, wire-code classification, and localized minute/hour/day copy were traced across all four app locales.
- Withdraw confirm route calculation and Retry state transitions were checked against the existing confirm-view behavior and tests.
- The generated privacy version and hash match the legal document in the unchanged pinned content submodule.
- Security pass found no new authorization bypass, secret exposure, or unsafe error rendering beyond the existing filtered ApiError path.
- The linked product task requires an actionable retry path; the Lexicon defines no conflicting cross-chain cap term.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 3 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: bc900921391d · Context: repo, ci, api, product · Took 20m (queued 7m)
…-stablecoin withdrawals peanut-api-ts #1497 gates POST /rhino/bridge/quote with the same per-user cross-chain cap as the SDA route when the quote names its charge. Send context + contextId from the bridge path (ETH/WETH/… destinations), which never touched /rhino/sda-transfer and so was uncapped. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
GitHub Actions
Inactive
…nder-promises Chip review (bc90092): - handleConfirmWithdrawal branched on routeError but its dependency list omitted routeError and calculateCurrentRoute, so a 429 that arrived after the callback was memoized still fell through to "transaction not prepared". Both are dependencies now; regression test added to crypto-withdraw-confirm.test.tsx. - The cap wait rounded the shown unit down (119 min → "1 hour", 47 h → "1 day"). It now rounds up, and a unit is used only once the wait reaches it. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
GitHub Actions
Inactive
Chip on peanut-api-ts#1497 (60991481): rhino-bridge.ts postJson/getJson still threw a plain Error, so a cap 429 on POST /rhino/bridge/quote would have rendered the generic 'contact support' copy. Same apiErrorFromResponse as rhino-sda.ts now. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
GitHub Actions
Inactive
There was a problem hiding this comment.
Chip review — changes requested
One previously raised product blocker remains: the new cross-chain cap conflicts with the still-published no-limits promise. Retry closure, bridge error mapping, wait rounding, and regression coverage are fixed, and exact-head CI is green.
Findings
-
MAJOR · src/i18n/app/messages/en.json:3185 · Cross-chain cap still contradicts published limits
Oncexchain_withdraw_capis enabled, a capped Base, Polygon, Solana, Tron, or Ethereum withdrawal will show this new limit while the live transaction-limits and network withdrawal pages still promise that crypto withdrawals are unlimited or have no maximum. Regenerate and deploy those pages with the cap rungs, the Arbitrum exemption, and the support increase path before the flag can be enabled. -
MINOR · src/constants/legal-versions.generated.ts:27 · [moonshotai/kimi-k3] Unrelated privacy-document version bump bundled into the withdrawal-cap hotfix
The generated consent/integrity registry bumps theprivacydocument from 2026-07-15 to 2026-08-27 with a new hash, inside a hotfix about cross-chain withdrawal caps. Shipping this invalidates every user's stored privacy consent (forced re-prompt/consent gate on next launch) and, if the corresponding legal-document revision is not served in lockstep, the embedded hash will not match the document the app fetches, breaking the version/hash verification. Nothing else in this diff relates to legal documents. Fix: revert this hunk here and ship it together with the actual privacy-policy change, coordinating the content deploy. -
MINOR · src/app/(mobile-ui)/withdraw/crypto/page.tsx:337 · [moonshotai/kimi-k3] Route-error Retry re-provisions with no in-flight guard, letting concurrent calculations race and burn cap slots
In handleConfirmWithdrawal's new branch (if (routeError) { clearErrors(); calculateCurrentRoute(); return }) nothing checksisCalculatingor disables Retry while a recalculation is running, so a quick double-tap on Retry fires two concurrentcalculate()calls. Per the PR's own design notes each provision holds a per-user cross-chain cap slot for 10 minutes on the API, so overlapping provisions consume extra cap slots (making the 429 worse for an already-capped user) and the two resolutions race last-write-wins ontransactions/sdaAddressstate — the user may then sign transactions built from a superseded provision. Fix: early-return in the retry branch whenisCalculatingis true (and/or disable the confirm Retry while in flight), matching the guard the normal confirm path gets viaisSendingTx. -
MAJOR · src/constants/legal-versions.generated.ts:32 · [claude-opus] Privacy version bump is ahead of peanut-api-ts, so every signup's consent row is clamped and loses its hash
This PR bumpsprivacyfrom2026-07-15to2026-08-27(hashda1d914e…, which matches mono/content/legal/privacy/en.md exactly, so the FE value is the correct one). peanut-api-ts still publishesprivacy: '2026-07-15'in src/consent/consts.ts (CURRENT_LEGAL_VERSIONS).sanitizeEchoedDocumentsin peanut-api-ts/src/consent/service.ts treats any echoed version greater than its own as a spoofed future date: it pushes{ version: currentVersion, hash: null }and logs 'clamping future-dated echoed version'.
Concretely: every signup sends x-accepted-legal with the privacy doc (src/hooks/useZeroDev.ts:102 → signupConsentDocuments()), and ReConsentModal sends the same shape to /users/consent/accept. After this merges, each of those rows is ledgered as version 2026-07-15 with documentHash = null — the opposite of what the ledger exists for ('stores what the user was shown, not what either side assumes', service.ts header). The FE's own generated-file comment says the same. So the ledger silently records the wrong revision and loses the content attestation, for every new user, until the backend catches up.
This normally ships as a pair and the other half is probably an open peanut-api-ts PR I cannot see — the pinned checkout holds merged code only, and neither of the two TASK-22154 PRs touches consent, so I cannot confirm it is handled. Fix: land a peanut-api-ts change setting CURRENT_LEGAL_VERSIONS.privacy = '2026-08-27' (which also correctly fires the ToS §17 re-consent click-through the content update on 2026-08-27 should have triggered), or drop the unrelated legal-versions regeneration from this withdraw PR and ship it with its backend half.
- MAJOR · src/features/payments/shared/hooks/useCrossChainTransfer.ts:465 · [claude-opus] Bridge-quote charge binding — the only thing that applies the cap to non-stablecoin withdrawals — is untested
AGENTS.md §'Testing & verifiability': 'if code moves money or mutates shared state, it needs a test before merge', plus 'every custom hook that fetches data, gates a flow, or holds persistent state needs a test'. Sendingcontext/contextIdon the quote is not a rename or a type change: it is what makes the API reserve a cap slot and bind the charge to its destination (peanut-api-ts#1497 gates/rhino/bridge/quoteonlyif (contextId)), so if it is ever dropped the cross-chain cap silently stops counting every ETH/WETH withdrawal while everything still looks green.
The untested case: a cross-chain withdraw whose destination token is outside SDA_SUPPORTED_TOKENS takes runBridgePath, and the POST to /rhino/bridge/quote must carry context: 'withdraw' and contextId = the charge uuid — and must omit both for claim-xchain, which has no charge and would be refused 403 by the gate. Nothing covers it: there is no useCrossChainTransfer test file, and getBridgeQuote appears in no test in the repo (only rhino-bridge.ts, the hook, and the generated openapi). The withdraw page test mocks the whole hook, so it cannot see the quote body.
A small unit test on runBridgePath (or on useCrossChainTransfer.calculate with the rhino-bridge service mocked) asserting the quote body for both branches would close it.
Inline anchors unavailable for 2 finding(s); the findings remain in this summary.
Checked clean
- Pinned HEAD, merge base, author, base ref, and PR metadata matched the supplied values.
- Previously raised retry closure findings P1/P7 and retry coverage findings P4/P10 are fixed: the callback uses current dependencies and the route-error retry regression test is present.
- Previously raised bridge mapping findings P5/P8 are fixed: both Rhino service wrappers now preserve the API code and retryAfterSec through ApiError before useCrossChainTransfer localizes the error.
- Previously raised rounding findings P2/P11 are fixed: displayed minutes, hours, and days round upward, with boundary cases covered.
- Rhino SDA/bridge error parsing, bridge context propagation, localized ICU selection, and confirm retry failure paths were reviewed; no additional actionable defect was found.
- Canonical Lexicon was read and does not define cross-chain withdrawal limits; current product and published content sources still state that crypto withdrawals have no limits or no maximum.
- All GitHub checks at the exact head completed successfully, including unit, typecheck, eslint, e2e, format, CodeQL, and ci-success.
- A focused local Jest rerun was unavailable because the detached worktree has no installed Jest binary; exact-head unit CI completed successfully.
Second opinion by moonshotai/kimi-k3: 2 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 2 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: 42de3c5b9fe8 · Context: repo, product, other-repo · Took 18m
…e binding, drop stray legal-versions bump Chip review (42de3c5): - Retry recomputes only when no calculation is in flight, so a double tap cannot provision twice (each provision holds a cap slot) or race the route state. - useCrossChainTransfer test: the bridge quote carries context + contextId for a withdraw and omits both for claim-xchain. - src/constants/legal-versions.generated.ts was regenerated by the dev server's predev hook against a newer content submodule and committed by mistake; restored to main. The privacy bump ships with its backend half. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
Chip review — changes requested
One major issue remains: P3, P5, and P9 describe the same unresolved contradiction between the new cross-chain cap and the published withdrawal limits; it is reported once against the latest prior finding.
Findings
-
MAJOR · src/i18n/app/messages/en.json:3185 · Published limits still contradict the new cap
The app now tells capped users that withdrawals to other networks are limited and Arbitrum is exempt, while product/kyc.md and product/quick-ref.md still say crypto has no limits or no hard limit, and the published withdrawal/help pages still promise no maximum. Once the API flag is enabled, a user capped on Base or Polygon will receive a 429 while the help site says the same withdrawal is unlimited. Land and regenerate the product and public-content updates before enabling the flag, or keep rollout blocked until that dependency is guaranteed. -
MINOR · src/features/payments/shared/hooks/useCrossChainTransfer.ts:465 · [claude-opus] Cap copy calls a blocked request payment a "withdrawal"
This line brings the bridge path'spay-requestcontext under the same per-user cross-chain cap the SDA path already fed (peanut-api-ts#1497gateUserFundedProvisioncounts request payments against the caller's cap, not just withdrawals). But the only copy this PR ships for the 429 is withdrawal-framed:errors.xchainWithdrawLimit/xchainWithdrawLimitRetry= "You reached the limit for withdrawals to other networks. Withdrawals on Arbitrum have no limit, or contact support to raise yours."
That string reaches the payer: useCrossChainTransfer.calculate now does setError(toFriendlyError(err)) (src/features/payments/shared/hooks/useCrossChainTransfer.ts:363), useSemanticRequestFlow exposes it as routeError (line 83/645), and SemanticRequestConfirmView.tsx:142 renders it verbatim. So a user paying someone else's cross-chain request — never withdrawing — is told they hit a withdrawal limit and is advised to use Arbitrum, on a destination chain the request author fixed and they cannot change.
Which side is wrong: the copy. product/request-links.md:104 ("Requests do not expire and have no amount limits") and product/networks.md:329 ("No per-transaction or daily limits from Peanut except as noted") document no cap on paying a request at all, so there is no product truth the withdrawal wording can be reconciled with; the cap on request payments is a deliberate backend behaviour and the FE just mislabels it.
Fix: give the pay-request surface its own key (e.g. errors.xchainPaymentLimit — "You've made too many cross-chain payments recently. Try again in about {…}") and drop the Arbitrum advice there, or make one shared key context-neutral ("cross-chain transfers") across all four locales. This is separate from the already-reported en.json cap-vs-published-limits contradiction, which is about the withdrawal number itself.
Checked clean
- Confirmed the detached worktree head, supplied base SHA, merge base, trusted author, base ref, and PR metadata match the review request.
- P1 and P4 are fixed: Retry reads current route state, recomputes the failed route, and has a confirm-flow regression test; the in-flight calculation guard prevents a second provision after rerender.
- P2 is fixed: minute, hour, and day waits round upward at their tested boundaries so the displayed retry time does not under-promise.
- P6 is fixed: legal-versions.generated.ts is byte-identical to the supplied base, so this PR no longer advances the privacy version.
- P7 is fixed: the bridge-path hook test covers withdraw context and contextId plus omission for claim-xchain.
- P8 is fixed: both Rhino bridge POST and GET failures now use apiErrorFromResponse, preserving code and retryAfterSec for the friendly mapping.
- Reviewed API error parsing, friendly-error precedence, localized ICU branches, retry state transitions, bridge charge binding, ownership assumptions, and all changed call sites without another actionable defect.
- All exact-head GitHub checks completed successfully, including typecheck, format, eslint, unit, e2e, CodeQL, analysis, and the deploy preview.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
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: f2e511f7c1a1 · Context: repo, product, sibling-repo, notion, ci · Took 22m
Chip (f2e511f): the cap counts request payments too, and a payer blocked on the semantic-request flow saw "withdrawals to other networks … withdraw on Arbitrum instead" — advice they cannot act on, the request fixed the destination. friendlyError takes a crossChainSurface hint; the cross-chain hook passes 'payment' for pay-request, which selects xchainPaymentLimit* copy (no Arbitrum advice) in en, es-419, es-AR, pt-BR. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
GitHub Actions
Inactive
There was a problem hiding this comment.
Chip review — changes requested
One major rollout blocker remains: the cross-chain cap still contradicts the published product and help limits. The error parsing, localized wait calculation, retry path, surface-specific copy, and bridge charge binding are otherwise sound at this head.
Findings
-
MAJOR · src/i18n/app/messages/en.json:3185 · Published limits still contradict the cross-chain cap
Whenxchain_withdraw_capis enabled, a user who reaches the cap is told that cross-chain withdrawals are limited and that support can raise the limit, while the current product source and live transaction-limit/withdraw pages still say crypto withdrawals have no limits. That gives users contradictory promises in the same journey. Land the product and generated-content update covering the cap rungs, Arbitrum exemption, and support path before enabling the flag, or enforce that rollout dependency so the cap cannot go live first. -
MAJOR · src/app/(mobile-ui)/withdraw/crypto/page.tsx:351 · [claude-opus] Retry's double-tap guard is untested
f2e511f7caddedif (isCalculating) returnto the new Retry branch specifically so a double tap cannot start a second route calculation, and shipped it with no test — that commit's own message says it added theuseCrossChainTransferbridge-binding test, and the only new page test (crypto-withdraw-confirm.test.tsx:493) clicks confirm exactly once and assertscalculatewas called one more time. Nothing covers the second tap.
This is a shared-state mutation, not a pure UI guard: calculateCurrentRoute → calculateRoute → provisionSdaTransfer (POST /rhino/sda-transfer) or getBridgeQuote+commitBridgeQuote (POST /rhino/bridge/quote, /commit). Two in-flight calculations mean two Rhino commitments for one charge; the second overwrites transactions/commitmentId while the user may already be signing the first, so the deposit that gets signed and the commitment that gets polled can be different ones. Delete the guard line and every existing test still passes.
The fix is cheap — the page's useCrossChainTransfer mock already exposes isCalculating (line 233): set { transactions: null, error: <cap message>, isCalculating: true }, render, click confirm-withdraw, and assert calculate.mock.calls.length is unchanged and that no errors.txNotPrepared was set. A second case with isCalculating: false asserting exactly one extra calculate call pins both sides of the branch.
Checked clean
- Pinned head, base SHA, merge base, trusted author, PR title, and description match the supplied review target.
- Prior P1/P4 retry issue is fixed: the callback sees the current route error, recomputes the route, guards recalculation, and has a regression test.
- Prior P2 wait issue is fixed: minutes, hours, and days round up without selecting a unit before its threshold, with boundary coverage.
- Prior P6 legal-version issue no longer applies because this diff does not change the generated legal versions file.
- Prior P7 bridge binding gap is fixed: withdraw and pay-request quotes carry charge context, claim-xchain omits it, and both branches are tested.
- Prior P9 payment-copy issue is fixed: request payers receive payment-framed copy without Arbitrum advice, while withdrawals retain withdrawal guidance.
- Correctness and failure paths checked across ApiError response parsing, SDA and bridge services, route state reset, confirm retry, and semantic-request retry.
- Security pass found no new auth-header, injection, secret-exposure, or privilege-boundary regression in the changed client code.
- All reported CI checks at the exact head completed successfully, including format, typecheck, eslint, unit, e2e, build analysis, preview deployment, CodeQL, and human-authors.
- A local focused Jest rerun was unavailable because the detached review worktree has no installed dependencies; exact-head CI supplies the test evidence instead.
- The live Product Lexicon and linked task were checked; current product/network/KYC sources and published help/withdraw content still contain the conflicting unlimited-crypto claims.
Second opinion: did not run — the model did not answer in time. 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: 384403617793 · Context: repo, product, notion, api-sibling · Took 22m
Chip (3844036): the isCalculating guard on the route-error Retry branch had no test. A second tap while a recalculation is in flight must not call calculate again or set 'not prepared'. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
GitHub Actions
Inactive
There was a problem hiding this comment.
Chip review — changes requested
The code fixes the previously raised retry, countdown, legal-version, bridge-binding, and payment-copy issues, and exact-head CI is green. One acknowledged rollout blocker remains: published product and help content still promises unlimited crypto withdrawals while this UI introduces a per-user cross-chain cap.
Findings
- MAJOR · src/i18n/app/messages/en.json:3185 · Published crypto limits still contradict this cap
Ifxchain_withdraw_capis enabled, users can be blocked after a finite number of cross-chain withdrawals while the canonical product files and live transaction-limits help content still say crypto withdrawals are unlimited and limits are regional rather than individual. Land the documented cap rungs, Arbitrum exemption, and support-increase path before enabling the flag, and keep the flag off until that content is published.
Checked clean
- Supplied detached worktree HEAD and merge base matched the exact head and base SHAs; the worktree remained clean.
- Prior P1 is fixed: the confirm callback reads current
routeErrorand depends oncalculateCurrentRoute; P4's route-retry regression test is present. - Prior P2 is fixed: displayed minutes, hours, and days round up without selecting a larger unit before its threshold.
- Prior P6 no longer applies:
src/constants/legal-versions.generated.tshas no diff from the supplied base. - Prior P7 is fixed: bridge quotes are tested with withdraw
contextandcontextId, while claim-xchain omits both. - Prior P9 is fixed: pay-request errors select payment-framed localized copy without Arbitrum advice.
- Prior P11 is fixed: the in-flight recalculation guard and its regression test are present.
- All changed locale JSON parsed successfully, and app/marketing cap messages match across en, es-419, es-AR, and pt-BR.
- The Rhino service error conversion, friendly-error classification, cross-chain hook state transitions, retry path, and bridge quote boundary were reviewed for correctness and security.
- Exact-head CI completed green, including unit, typecheck, eslint, format, e2e, CodeQL, analysis, and preview checks.
- A local focused Jest rerun was unavailable because the detached worktree has no installed Jest binary; exact-head CI's unit check passed.
- The canonical Notion Lexicon does not define a cross-chain cap; current mono product/network/KYC/quick-reference files and transaction-limits help content still state that crypto withdrawals are unlimited.
Second opinion: did not run — the model did not answer in time. This review is one reviewer short.
Third opinion by claude-opus: 0 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: 11f59bf14414 · Context: repo, product, api-sibling, notion, ci · Took 22m
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No blocking defect survives at the exact head. The retry, countdown, legal-version, bridge-binding, payment-copy, and in-flight guard findings are fixed. The remaining published-limit mismatch is major and explicitly deferred to separate product/content work before the cap is enabled.
Answered by a human, not raised again
These are still present as far as this review can tell. You resolved the thread or deferred the fix, so Chip files them instead of repeating them.
- MAJOR · src/i18n/app/messages/en.json:3185 · Published crypto limits still contradict this cap — task
Checked clean
- The supplied detached worktree HEAD and merge base match the exact head and base SHAs, and the worktree is clean.
- P1 is fixed: the confirm callback depends on current
routeErrorandcalculateCurrentRoute. - P2 is fixed: displayed minutes, hours, and days round up without selecting a larger unit before its threshold.
- P4 is fixed: the route-error Retry branch has a regression test that recomputes without broadcasting or reporting an unprepared transaction.
- P6 no longer applies:
src/constants/legal-versions.generated.tshas no diff from the supplied base. - P7 is fixed: bridge quotes are tested with withdraw
contextandcontextId, while claim-xchain omits both. - P9 is fixed: pay-request errors select payment-framed localized copy without Arbitrum advice.
- P11 is fixed: the in-flight recalculation guard and its double-tap regression test are present.
- P3, P5, P8, P10, and P12 describe the same surviving product-content mismatch; it is reported once as deferred P12.
- The Rhino service error conversion, friendly-error classification, cross-chain hook state transitions, Retry path, and bridge quote boundary were reviewed for correctness, security, adversarial failure paths, and slop.
- Exact-head CI is fully completed with no failing or pending checks.
- The canonical Lexicon does not define a cross-chain cap; current product/network/KYC sources and transaction-limits help content still state that crypto withdrawals are unlimited.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: 11f59bf14414 · Context: repo, product, api-sibling, notion, ci · Took 7m (queued 9m)
Superseded: Chip reviewed 11f59bf and found nothing blocking. Dismissed so the pull request is not held by a review that no longer applies.
Chip on peanut-api-ts#1497 (6e815a66): only the quote sent context/contextId; with the cap on, POST /rhino/bridge/commit refuses commits without them, so a non-stablecoin withdrawal would quote and then fail at commit. The commit now carries the same charge (none for claim-xchain). Hook test asserts both. Claude-Session: https://claude.ai/code/session_017idXbJRcFgbugvA8Xxr5YC
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean at the pinned head: route retry, cap-error localization, and Rhino bridge charge binding are coherent and covered; the published product limits now match the UI copy.
Checked clean
- Verified the trusted author, main base ref and exact base/head SHAs; the supplied worktree matched the head before the review pipeline removed it, and the same pinned commit objects were used for the remaining read-only inspection.
- Exact-head CI is green, including unit, e2e, typecheck, eslint, format, CodeQL and JavaScript/TypeScript analysis.
- Checked route-error Retry recomputation, callback dependencies, the in-flight guard, and regression coverage; Retry neither broadcasts nor emits transaction-not-prepared when route construction failed.
- Checked ApiError code/retryAfterSec parsing, upward wait rounding, ICU unit selection, and withdrawal-versus-request-payment copy across all four locales.
- Checked Rhino SDA and bridge quote/commit charge binding, including omission for claim-xchain and the focused hook tests.
- Checked the current Lexicon mirror, product/networks.md, product/quick-ref.md and generated withdrawal pages; they publish the same 10/hour, 20/day and 30/30-days cap, Arbitrum exemption and support escalation path.
- Checked security and privacy boundaries: authentication headers are unchanged, backend error passthrough remains filtered, and charge context is sent only for charge-backed flows.
Security review by moonshotai/kimi-k3: 0 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 by claude-opus: 0 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: d81184d44cf2 · Context: repo, product, sibling · Took 8m (queued 6m)
1 participant
Summary
Pairs with peanutprotocol/peanut-api-ts#1497 (per-user cap on cross-chain withdrawals). That API answers
POST /rhino/sda-transferwith429 { error, code: "XCHAIN_WITHDRAW_LIMIT_REACHED", retryAfterSec }once a user is over the cap. On the current UI that surfaces as the raw stringFailed to provision SDA transfer: 429 {…}, and the confirm view's Retry callsonConfirmwith no prepared transactions and dead-ends on "transaction not prepared".postRhinothrows anApiError(message = the backend'serror, pluscodeandretryAfterSec);apiErrorFromResponsenow carriesretryAfterSecfor any route that sends one.friendlyErrormapsXCHAIN_WITHDRAW_LIMIT_REACHEDto localized copy (en, es-419, es-AR, pt-BR) that states the wait in the coarsest unit and points at Arbitrum (no limit) and support: "You reached the limit for withdrawals to other networks. Try again in about 3 hours. Withdrawals on Arbitrum have no limit, or contact support to raise yours."useCrossChainTransferruns every route error throughuseFriendlyErrorinstead of echoingerr.message, so other backend errors on that path get the same treatment.context+contextIdonPOST /rhino/bridge/quote, so the API's cap covers non-stablecoin withdrawals too (peanut-api-ts#1497 round 3).Design notes / accepted trade-offs
useFriendlyError, so other backend errors on the cross-chain path lose their rawFailed to …: <status> <json>shape too. Intended.Task
TASK-22154 — https://app.notion.com/p/Limit-cross-chain-withdrawals-per-user-3cf83811757981afb2d7f43e0bcbc086
Risks / breaking changes
main→ back-merge debt main → dev.app.configurationsrowxchain_withdraw_cap.enabled); it is switched on once this UI is live.useFriendlyErrorinsideuseCrossChainTransferadds auseTranslations('errors')dependency to that hook. Its consumers (withdraw crypto page, semantic request flow) already render inside the intl provider.QA
src/utils/__tests__/friendly-error.utils.test.tsx: wait rendering per unit, missing-wait fallback, copy present. Full suite green (4257).Screenshots
Sandbox at 375×667, real API (peanut-api-ts#1497 worktree, cap enabled, user seeded at the hourly cap), real
429onPOST /rhino/sda-transfer, USDC → Base. Only the Rhino public quote is stubbed (unreachable from the sandbox).Before (form, for context):
Assets live on branch
pr-assets-2931; delete it after merge.Summary by CodeRabbit
Bug Fixes
New Features