Skip to content

fix(fees): one fee source — Rhino's quoted fee verbatim, one NetworkFeeRow (TASK-21991) - #2902

Open
abalinda wants to merge 8 commits into
devfrom
fix/21991-network-fee-one-source
Open

fix(fees): one fee source — Rhino's quoted fee verbatim, one NetworkFeeRow (TASK-21991)#2902
abalinda wants to merge 8 commits into
devfrom
fix/21991-network-fee-one-source

Conversation

@abalinda

@abalinda abalinda commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Requires peanut-api-ts#1480 deployed first — the preview call sends depositor/recipient and expects the normalized quote shape. Merge together on the same dev→main train; API first, UI second, in one release window.

Summary

Every cross-chain crypto withdrawal showed a "Network fee" Rhino never charges (60 days to 2026-09-01: $415.81 shown and collected, $0.14 actually deducted — plan and evidence in mono ops/rhino-fee-display-fix.md). The API side (peanut-api-ts#1480) switches to Rhino's authenticated, account-bound quote, which prices our 1:1 zero-fee config. This side makes the UI show that quote verbatim and stops doing fee arithmetic:

  • useCrossChainTransfer exposes feeUsd / payAmount / receiveAmount exactly as quoted on every path. The bridge path used to set feeUsd + gasFeeUsd (born in 8cf16749f) — a double count, since feeUsd is already Rhino's total. The SDA preview now sends depositor (kernel) and recipient, which the account-bound quote requires; the claim-link route preview does the same.
  • formatNetworkFee(feeUsd, isCrossChain) in cross-chain-fee.utils.ts is the one formatter: null (sponsored) when nothing is charged on top, < $0.01 below a cent, else $X.XX.
  • Global/NetworkFeeRow is the one row (the PaymentInfoRow fee variant the DS audit asks for — "Fee row 'Sponsored by Peanut!' recipe"). Adopted in the withdraw confirm, the pay-request confirm and the on-chain claim confirm. It also carries the paymaster strike-through the pay-request screen had, so behavior is unchanged there.
  • Copy: withdraw.confirm.networkFeeInfo / recipientReceivesInfo → "Peanut covers the bridge cost. A small network cost may be deducted on delivery." (en, es-419, pt-BR; the es-AR override is dropped so it inherits es-419). The old text named a fee nobody pays and said "paid on top", which was wrong for the bridge path.
  • src/services/rhino-*.ts consume the normalized RhinoQuote (amountIn/amountOut/fee/gasFeeUsd are gone); api.openapi.json + api.generated.ts regenerated for the two new required preview fields.

Quoting cost (one Rhino flow credit per authenticated quote) is accepted — no caching or debouncing, by decision (plan §6.1).

Task

TASK-21991 — https://app.notion.com/p/3cc838117579814cb884f9b9b9ec5955

Risks / breaking changes

  • ⚠️ Cross-repo, deploy order. Against the current API this UI sends two extra fields the old preview rejects (400) and reads payAmount/receiveAmount the old bridge quote does not return. Must ship after peanut-api-ts#1480.
  • Visible change on the confirm screens: the fee row shows "Sponsored by Peanut!" where it showed $0.51-class amounts; "You pay" now equals "Recipient receives" on stablecoin routes. The pay-request confirm additionally shows a Rhino fee if a route ever quotes one (it never did before — it only showed gas); with the authenticated quote that is $0.
  • ReceiptDetailsCard is not adopted, deliberately: it is a DataRow card and design.md forbids mixing PaymentInfoRow and DataRow in one card. Its network-fee value already comes from the success views' display string; nothing there reads a quote.
  • The pre-sign affordability gate and the 5% "high fee" heads-up stay as insurance — they read the quote verbatim, so a future Rhino config change shows up with no code change.

Design notes / accepted trade-offs

  • New component, justified: Global/NetworkFeeRow (shared non-primitive → Global/; code-only, no figma board — it composes PaymentInfoRow, which is itself "receipt-row board pending"). Replaces three hand-rolled rows; nothing in 0_Bruddle/Global covered the strike-plus-label pattern.
  • formatNetworkFee returns null for "sponsored" rather than the plan's 'sponsored' | string — that union collapses to string in TS and cannot be told apart from a formatted fee.
  • The pay-request confirm's paymaster gas strike-through is kept as a sponsoredGasUsd prop on the row (behavior parity per design.md "building or migrating a screen").
  • Claim route cache is keyed by quote recipient (Chip finding on 39bdcf7e2): ClaimXChainPreview.quotedFor + findClaimRoute / resolveClaimQuoteRecipient in src/utils/claim-route.utils.ts (unit-tested: a route quoted for A is not reused for B); switching the external address drops the selected route and re-quotes. The view-level effect is exercised manually — Initial.view.tsx has no render harness today (a pre-existing gap, not added here).
  • The SDA quote expires, and the confirm screen respects it (Chip finding on fac56691a): the hook carries expiresAt for the SDA path too; the withdraw confirm re-quotes instead of signing an expired quote (never on the record-only retry). The SDA primitive has no commit step, so the numbers stay Rhino's estimate by construction — accounting books executed actuals (api#1480) and the tooltip says a small network cost may be deducted on delivery.
  • A failed cross-chain quote shows - in the fee row (quoteFailedestimationFailed), never the sponsored label.
  • Expiry is decided at the tap (isQuoteNearExpiry(quoteExpiresAt) with the signing lead), not from a render-time flag — fake-timer test covers a screen left open past expiry. Claim routes carry expiresAt (expired = cache miss → re-quote) and a generation stamp, so a slow quote for recipient A is never selected after the user moved to B. The claim view has no render harness (pre-existing); the rules are unit-tested in claim-route.utils.test.ts.
  • Expiry is enforced at the tap on every consumer: withdraw (re-quote before signing), pay-request (executePayment re-quotes instead of broadcasting an expired route — fake-timer hook test), and the on-chain claim confirm (an expired route is dropped and the initial view re-quotes; never executed on expired numbers).
  • Claim quotes carry a generation: a result that is no longer current — success or failure — never selects a route, installs an error, or clears loading; concurrent quotes merge into the cache (setRoutes((prev) => …)). No render harness exists for Initial.view.tsx (pre-existing), so these rules are guarded by construction and the pure parts are unit-tested.
  • A chain/token switch retires any claim quote still in flight (generation advanced synchronously in the selection-change effect); the claim cache honours the same 15 s signing lead the confirm screens use (QUOTE_SIGNING_LEAD_MS), so a near-expiry entry re-quotes instead of bouncing between screens; a withdraw tap with nothing prepared re-quotes instead of dead-ending on "not prepared".
  • The claim preview prices with the link sender's address as depositor: display-only, the claim's execution is not quote- or depositor-bound (relayer deposits, Rhino settles at delivery, the API books actuals); server-side relayer substitution would be a larger API change for no numeric difference today.
  • Flagged, not changed: the Manteca and qr-pay "Peanut fee" rows (not quote-fed) still hand-assemble tCommon('sponsoredByPeanut'); out of scope here.

QA

  • Unit: cross-chain-fee.utils.test.ts (formatter), NetworkFeeRow.test.tsx (all four states), useCrossChainTransfer.test.ts (feeUsd verbatim on the SDA and bridge paths; depositor/recipient forwarded), render tests for the three adopted confirm views, i18n catalog tests (duplicate-value drift passes: both keys share one string per locale).
  • Full suite: 4998 pass; ResidenceChangeModal.test.tsx flakes only under the full parallel run and passes alone on this branch and on origin/dev.
  • Local: the sandbox RHINO_API_KEY has no account config, so the zero-fee state only reproduces with the prod key. Acceptance (plan §5) post-deploy: $10 → Solana USDC shows the sponsored label, "You pay" $10.00, "Recipient receives" $10.00; same for Tron/Base/Ethereum; $10 → ETH on Ethereum shows the sponsored label (was $3.01).

Screenshots: ⚠️ NONE — the sandbox RHINO_API_KEY can call the authenticated quote but has no account config, so it prices the generic schedule (probe: $10 → Solana quoted feeUsd 0.51); a local capture would show a $0.51 row, not the sponsored state this PR exists for. Visual verification = the post-deploy acceptance checks above, on prod with the prod key. Reviewer: the row's four states are pinned in NetworkFeeRow.test.tsx.

…m screen

Every cross-chain withdrawal showed a "Network fee" Rhino never charges: the
preview priced the public schedule, the bridge path added gas on top of a
fee that already included it, and three confirm screens each assembled the
row by hand. Now the hook exposes feeUsd, payAmount and receiveAmount
exactly as the authenticated quote returns them, formatNetworkFee is the
one formatter, and NetworkFeeRow is the one row (DS audit "Fee row
'Sponsored by Peanut!' recipe"). The preview sends depositor and recipient,
which the account-bound quote requires.

Requires peanut-api-ts#1480 deployed first.

TASK-21991
@vercel

vercel Bot commented Sep 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
peanut-wallet Ready Ready Preview Sep 1, 2026 3:21pm UTC

Request Review

@notion-workspace

Copy link
Copy Markdown

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: 36ecb60c-d2c4-483f-a2cd-19c789e1723a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Code-analysis diff

Painscore total: 7152.21 → 7167.14 (+14.93)
Findings: +6 net (+143 new, -137 resolved)

🆕 New findings (143)

  • critical complexity — src/components/Claim/Link/Initial.view.tsx — CC 220, MI 50.41, SLOC 774
  • critical complexity — src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts — CC 109, MI 48.96, SLOC 468
  • critical complexity — src/app/(mobile-ui)/withdraw/crypto/page.tsx — CC 99, MI 49.5, SLOC 456
  • critical complexity — src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx — CC 60, MI 56.79, SLOC 136
  • high structural-dup — types/api.generated.ts:10279 — 83 duplicate lines / 388 tokens with types/api.generated.ts:10867
  • high structural-dup — types/api.generated.ts:10289 — 74 duplicate lines / 348 tokens with types/api.generated.ts:10877
  • high hotspot — src/app/(mobile-ui)/withdraw/crypto/page.tsx — 55 commits, +626/-324 lines since 6 months ago
  • high hotspot — src/components/Claim/Link/Initial.view.tsx — 46 commits, +374/-195 lines since 6 months ago
  • high complexity — src/components/Withdraw/views/Confirm.withdraw.view.tsx — CC 45, MI 57.13, SLOC 68
  • high complexity — src/components/Claim/Link/Onchain/Confirm.view.tsx — CC 33, MI 48.11, SLOC 156
  • high complexity — src/features/payments/shared/hooks/useCrossChainTransfer.ts — CC 33, MI 45.67, SLOC 334
  • high method-complexity — src/components/Withdraw/views/Confirm.withdraw.view.tsx:73 — ConfirmWithdrawView CC 32 SLOC 50
  • high method-complexity — src/components/Claim/Link/Initial.view.tsx:63 — CC 30 SLOC 215
  • medium react-long-component — src/app/(mobile-ui)/withdraw/crypto/page.tsx:49 — WithdrawCryptoPage is 698 lines — split it
  • medium high-mdd — src/components/Claim/Link/Initial.view.tsx:63 — InitialClaimLinkView: MDD 241.3 (uses across many lines from declarations)
  • medium high-mdd — src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts:34 — useSemanticRequestFlow: MDD 198.1 (uses across many lines from declarations)
  • medium high-mdd — src/app/(mobile-ui)/withdraw/crypto/page.tsx:49 — WithdrawCryptoPage: MDD 149.4 (uses across many lines from declarations)
  • medium high-mdd — src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx:36 — SemanticRequestConfirmView: MDD 119.7 (uses across many lines from declarations)
  • medium high-dlt — src/components/Claim/Link/Initial.view.tsx:63 — InitialClaimLinkView: DLT 108 (calls 108 distinct functions — high context load)
  • medium high-mdd — src/components/Claim/Link/Onchain/Confirm.view.tsx:33 — ConfirmClaimLinkView: MDD 89.1 (uses across many lines from declarations)

…and 123 more.

✅ Resolved (137)

  • src/components/Claim/Link/Initial.view.tsx — CC 214, MI 50.64, SLOC 733
  • src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts — CC 106, MI 49.12, SLOC 462
  • src/app/(mobile-ui)/withdraw/crypto/page.tsx — CC 95, MI 49.41, SLOC 439
  • src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx — CC 64, MI 56.71, SLOC 149
  • types/api.generated.ts:10277 — 83 duplicate lines / 388 tokens with types/api.generated.ts:10865
  • types/api.generated.ts:10287 — 74 duplicate lines / 348 tokens with types/api.generated.ts:10875
  • src/app/(mobile-ui)/withdraw/crypto/page.tsx — 52 commits, +566/-291 lines since 6 months ago
  • src/components/Withdraw/views/Confirm.withdraw.view.tsx — CC 48, MI 57.55, SLOC 76
  • src/components/Claim/Link/Initial.view.tsx — 40 commits, +288/-168 lines since 6 months ago
  • src/features/payments/shared/hooks/useCrossChainTransfer.ts — CC 33, MI 45.85, SLOC 330
  • src/components/Claim/Link/Onchain/Confirm.view.tsx — CC 31, MI 48.47, SLOC 152
  • src/components/Withdraw/views/Confirm.withdraw.view.tsx:68 — ConfirmWithdrawView CC 31 SLOC 53
  • src/components/Claim/Link/Initial.view.tsx:62 — CC 30 SLOC 211
  • src/app/(mobile-ui)/withdraw/crypto/page.tsx:48 — WithdrawCryptoPage is 672 lines — split it
  • src/components/Claim/Link/Initial.view.tsx:62 — InitialClaimLinkView: MDD 227.2 (uses across many lines from declarations)
  • src/features/payments/flows/semantic-request/useSemanticRequestFlow.ts:33 — useSemanticRequestFlow: MDD 194.4 (uses across many lines from declarations)
  • src/app/(mobile-ui)/withdraw/crypto/page.tsx:48 — WithdrawCryptoPage: MDD 144.8 (uses across many lines from declarations)
  • src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx:35 — SemanticRequestConfirmView: MDD 124.3 (uses across many lines from declarations)
  • src/components/Claim/Link/Initial.view.tsx:62 — InitialClaimLinkView: DLT 105 (calls 105 distinct functions — high context load)
  • src/components/Claim/Link/Onchain/Confirm.view.tsx:31 — ConfirmClaimLinkView: MDD 84.3 (uses across many lines from declarations)

…and 117 more.

📈 Painscore deltas (top movers)

File Before After Δ
src/components/Global/NetworkFeeRow/index.tsx 0.0 6.7 +6.7
src/utils/claim-route.utils.ts 0.0 3.9 +3.9
src/components/Claim/Link/Onchain/Confirm.view.tsx 13.5 14.4 +0.9
src/components/Claim/Link/Initial.view.tsx 19.6 20.4 +0.8

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🧪 UI test report — ✅ all green

Suites

  • unit: 5022 ran, 0 failed, 0 skipped, 1.7m

📊 Coverage (unit)

metric %
statements 73.3%
branches 58.6%
functions 64.7%
lines 74.4%
⏱ 10 slowest test cases
time test
4.3s src/components/Card/share-asset/__tests__/shareAssetLayout.test.ts › never places two stickers in heavy overlap (broad seed sweep)
2.1s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › the change cooldown shows its date and blocks changing to another country, not re-saving
1.6s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › saves the declared residence, refetches, and closes
1.6s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › moving to a country in neither slot leaves the second document alone
1.6s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › prefers the server value over a stale device mirror
1.5s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › swaps from the server value with no device mirror at all
1.4s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › promoting the second document country swaps the pair instead of dropping one
1.1s src/components/Setup/Views/__tests__/Residence.test.tsx › shows the per-country availability comparison with the truth-first guidance
1.0s src/hooks/query/__tests__/user.test.tsx › does NOT clear a token that rotated mid-request (stale 401 racing a fresh login)
1.0s src/components/Setup/Views/__tests__/Residence.test.tsx › reveals the second selector via the multi-doc link
📍 Inline annotations are in the **Unit test report** check above. Coverage artifact: `coverage-unit`. Generated by `.github/workflows/tests.yml`.

A bank claim's recipient.address is an IBAN or account number, which the
account-bound quote rejects. Use the link sender's address for pricing (the
relayer makes the real deposit). Also folds the formatNetworkFee cases into
the existing colocated test file and documents payAmount/receiveAmount in
tokenIn/tokenOut units — the bridge path parses payAmount as USDC.

TASK-21991

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

Request changes: the new recipient-bound claim quote can be reused after the recipient changes, so confirmation can show economics for the wrong address.

Findings

  • MAJOR · src/components/Claim/Link/Initial.view.tsx:693 · Invalidate claim quotes when the recipient changes
    fetchRoute now sends the recipient into an account-bound quote, but the routes cache is still keyed only by chain and token, and selectedRoute is not cleared when an external address changes. Quote address A, replace it with address B while keeping the same chain/token, then click Review: the cached route for A can reach confirmation while execution targets B, so the displayed fee and receive amount are not B's quote. Key or invalidate cached routes by the effective quote recipient, clear selectedRoute on recipient changes, and add an address-switch regression test.

Checked clean

  • Verified the detached worktree head, trusted PR metadata, exact base, and 21-file merge-base comparison.
  • Checked SDA, bridge, and same-chain quote normalization, transaction sizing, funding amounts, reset, and failure paths.
  • Checked withdraw, pay-request, and on-chain claim fee rows, formatter behavior, translations, and added regression tests.
  • Exact-head CI completed successfully, including unit, typecheck, lint, format, design-system, backend-baseline, preview, and analyze gates.
  • The referenced mono ops/rhino-fee-display-fix.md was absent from the current mirror; generated OpenAPI and current backend source were checked as dependency context.
  • Targeted local Jest could not run because the detached worktree has no node_modules; exact-head unit CI succeeded.

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.

Exact head: 39bdcf7e2d2b · Context: repo, mono

Comment thread src/components/Claim/Link/Initial.view.tsx Outdated
… was quoted for

The quote is now account- and address-bound, but the route cache was keyed
by chain and token only: quote for address A, switch to B on the same chain,
tap Review, and A's route reached confirmation while execution targeted B.
Routes remember quotedFor, the lookup keys on it, and a recipient switch
drops the stale selection and re-quotes. Bank claims and the Peanut wallet
keep their route (their effective quote recipient does not change).

TASK-21991

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

The recipient-keyed claim-route cache fix is sound, but the SDA flow still presents an expiring, uncommitted preview as exact execution.

Findings

  • MAJOR · src/features/payments/shared/hooks/useCrossChainTransfer.ts:331 · Do not present an expiring SDA estimate as execution
    This branch still discards preview.expiresAt and never commits or refreshes the quote, yet its payAmount, receiveAmount, and feeUsd drive the transaction and exact-looking confirm rows. If a user opens a zero-fee quote, waits past expiry, then confirms after fees or route costs change, the transfer still sends the stale pay amount while the UI says the recipient gets the quoted amount and the fee is sponsored. Rhino's Fees & Limits docs describe SDA fees as estimates and only guarantee a quote until expiry. Requote before signing and bind execution to it where possible; otherwise label SDA receive/fee values as estimates and reserve final values for executed webhook actuals.

  • MINOR · src/components/Withdraw/views/Confirm.withdraw.view.tsx:187 · [moonshotai/kimi-k3] Withdraw confirm shows 'Sponsored by Peanut!' when the Rhino quote failed
    ConfirmWithdrawView renders <NetworkFeeRow feeUsd={networkFee} isCrossChain={isCrossChain} ...> without the estimationFailed prop that NetworkFeeRow was explicitly built to accept (and that SemanticRequestConfirmView wires up via isFeeEstimationError). On a cross-chain withdraw where the Rhino preview/provision call errors, useCrossChainTransfer leaves feeUsd undefined, formatNetworkFee(undefined, true) returns null, and the row falls through to the sponsored label — telling the user Peanut covers the fee when in reality the fee is unknown (and, if a re-quote fails after a prior success, a stale fee from the previous quote is shown instead). Execution is gated elsewhere (no transactions/payAmount on error), so this is display-only, but it contradicts the PR's own 'show the quote verbatim' contract in exactly the failure case the row was designed for. Fix: thread the hook's isFeeEstimationError (or error) state into ConfirmWithdrawView and pass estimationFailed to NetworkFeeRow so the row shows '-' on quote failure.

Checked clean

  • Earlier recipient-change finding: FIXED — ClaimXChainPreview carries quotedFor, cache lookup keys chain/token/recipient, and the view invalidates and requotes when the effective recipient changes.
  • Earlier SDA exact-execution finding: STILL PRESENT — re-reported above after rechecking every confirm-screen consumer.
  • Bridge path now uses Rhino feeUsd verbatim without adding the gas component twice; pay and receive amounts follow the normalized quote shape.
  • Correctness, security, adversarial, and slop passes covered the 24-file merge-base diff, authenticated service wrappers, amount construction, failure states, translations, and shared fee-row rendering.
  • Exact supplied base integration has no overlapping changed paths or merge-tree conflicts.
  • Exact-head CI: unit, typecheck, eslint, format, analyze, and preview deploy succeeded; ds-shots was still in progress.
  • Focused local hook, formatter, and claim-route tests passed. Component reruns could not resolve next-intl from the shared dependency checkout; exact-head CI's full unit job passed.
  • All changed JSON catalogs and the OpenAPI document parse successfully; git diff --check is clean.

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.

Exact head: fac56691a885 · Context: repo, CI, Rhino provider documentation

Comment thread src/features/payments/shared/hooks/useCrossChainTransfer.ts
Comment thread src/components/Withdraw/views/Confirm.withdraw.view.tsx
… failed quote as unknown

The SDA path dropped the quote's expiresAt, so the confirm screen could sit
on stale numbers and sign them. The hook now carries the expiry for the SDA
path as it did for the bridge path, and the withdraw confirm refreshes the
quote instead of signing when it has expired (never on the record-only
retry, where funds already moved). A failed quote shows "-" in the fee row
instead of the sponsored label.

TASK-21991

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

Both earlier quote-validity findings remain reachable: expiry is checked only at render time, and an out-of-order claim quote can still replace the route after the recipient changes. The authenticated fee normalization and shared display path otherwise match the intended contract.

Findings

  • MAJOR · src/app/(mobile-ui)/withdraw/crypto/page.tsx:359 · Check quote expiry at confirmation time
    isQuoteExpired is computed from Date.now() only when the hook renders. After a fresh quote renders this as false, leaving the confirm screen open past expiresAt causes no render, so this handler still executes the stale transactions/pay amount. The new test starts with isQuoteExpired: true and misses the aging case. Compare quoteExpiresAt with the current time inside the click handler (with an execution lead time), or make expiry reactive, and add a fake-timer test that advances after render before clicking.

  • MAJOR · src/components/Claim/Link/Initial.view.tsx:724 · Discard out-of-order recipient quotes
    quotedFor prevents reuse of a completed cached route, but an in-flight fetch is never cancelled or generation-checked. If recipient A's request is slow, the user switches to B, B resolves first and the user opens confirm, then A resolves, this callback still calls the parent setSelectedRoute(route) after the initial view unmounts; confirm now shows A's receive amount and fee for a claim going to B. Only apply a result when its recipient/request generation is still current (or abort superseded requests), and test deferred A/B responses resolving out of order.

  • MAJOR · src/components/Claim/Link/Initial.view.tsx:673 · [moonshotai/kimi-k3] Claim-link route cache ignores quote expiry — stale fee/receive shown at confirm
    The PR's own invariant is that displayed fees are only valid until the quote's expiresAt (the withdraw flow re-quotes before signing when isQuoteExpired). The claim flow drops that invariant: previewSdaTransfer returns a RhinoQuote with expiresAt, but the route built at src/components/Claim/Link/Initial.view.tsx (~line 716) keeps only receiveAmount/feeUsd/quotedFor, and findClaimRoute (line 673) reuses any cached entry matching chain/token/recipient with no freshness check. Worse, the new recipient-invalidation effect makes stale reuse more likely: switching recipient A→B→A hits the cached route priced for A earlier, however old it is. A claimer who selects a route, dwells past the Rhino quote TTL (or switches recipients and back), then confirms sees 'Sponsored by Peanut!' / a receive amount from a dead quote, while the actual claim executes at Rhino's execution-time actuals — exactly the shown-fee ≠ charged-fee divergence this PR exists to fix, and the PR description's 'future Rhino config change shows up with no code change' silently fails on this path. Fix: add expiresAt to ClaimXChainPreview, store preview.expiresAt when building the route, and treat expired entries as a miss in findClaimRoute (forcing a re-preview); ideally also re-quote when entering the confirm view, mirroring the withdraw page.

  • MINOR · src/components/Claim/Link/Initial.view.tsx:710 · [moonshotai/kimi-k3] Claim preview priced for a surrogate, attacker-influenced depositor
    previewSdaTransfer is called with depositor: claimLinkData.senderAddress (link data the claimer did not choose — the link creator controls it) while the code comment admits the real SDA deposit comes from the Peanut claim relayer. The quote is explicitly account- and address-bound, so the numbers shown (feeUsd, receiveAmount) are Rhino's price for (sender address, recipient), not for the (relayer, recipient) pair that will actually execute. If the API or Rhino ever validates or re-prices per depositor at execution, the confirm screen's fee/receive are not honored; the client also supplies an unvalidated arbitrary string into an authenticated endpoint that the openapi schema only constrains to minLength 1. Fix: have the API substitute the relayer's address server-side for claim previews (it knows the relayer) instead of trusting a client-supplied surrogate depositor, or make depositor optional for claim-context previews and price account-only.

Inline anchors unavailable for 1 finding(s); the findings remain in this summary.

Checked clean

  • Exact head, trusted author, base ref/base SHA, and merge base were verified; review diff is the 26-file merge-base-to-head change.
  • Authenticated SDA and bridge quote fields are forwarded and consumed verbatim without frontend fee arithmetic.
  • NetworkFeeRow covers sponsored, nonzero, sub-cent, failed-estimation, and sponsored-gas display states.
  • Security pass found no new secret exposure, injection path, or privilege change.
  • Exact-head CI completed successfully for unit, typecheck, eslint, format, design-system, analyze, and preview checks.

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.

Exact head: 815092c472cb · Context: repo, ops, product

Comment thread src/app/(mobile-ui)/withdraw/crypto/page.tsx Outdated
Comment thread src/components/Claim/Link/Initial.view.tsx
Comment thread src/components/Claim/Link/Initial.view.tsx
…r expired claim quote

Withdraw: the expiry check now reads the quote's expiresAt when the user
taps, with the bridge path's signing lead time — a render-time flag went
stale on a screen left open (fake-timer test covers the aging case).
Claim: each route quote carries its generation and expiry; a result for a
recipient the user has since left is cached but never selected, and an
expired cached route is a miss that re-quotes.

TASK-21991
@abalinda

abalinda commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Re the finding without an inline anchor ("Discard out-of-order recipient quotes"): fixed in fe5de92. fetchRoute stamps each selecting quote with a generation (quoteGenerationRef); the recipient-change effect bumps it, and a result whose generation is no longer current is cached but never selected — a slow quote for A cannot land on a confirm screen for B. The view has no render harness today (pre-existing), so this is covered by the generation rule plus the findClaimRoute unit tests rather than a deferred A/B view test; noted in the PR body.

@abalinda
abalinda marked this pull request as ready for review September 1, 2026 14:12
@abalinda
abalinda requested a review from Hugo0 as a code owner September 1, 2026 14:12
Copilot AI lite review requested due to automatic review settings September 1, 2026 14:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

Request changes: confirmation-time expiry is still unhandled in the pay-request and claim flows, and a stale failed claim preview can still overwrite a newer successful route. The earlier withdraw confirmation expiry bug is fixed.

Findings

  • MAJOR · src/features/payments/flows/semantic-request/views/SemanticRequestConfirmView.tsx:232 · Refresh pay-request quotes before execution
    This row now shows an expiring Rhino quote, but useSemanticRequestFlow never carries quoteExpiresAt and executePayment broadcasts the cached routeTransactions unconditionally. Repro: open a cross-chain request with a quote expiring in 60 seconds, leave this screen open for 61 seconds, then press Send; the user accepts fee and receive numbers Rhino no longer guarantees, while the stale prepared route is still used. Carry quoteExpiresAt through the flow and re-run prepareRoute at the tap, requiring confirmation of the refreshed values before broadcasting; add fake-timer coverage like the withdraw flow.

  • MAJOR · src/components/Claim/Link/Onchain/Confirm.view.tsx:239 · Requote an expired claim preview before execution
    selectedRoute.expiresAt is checked only when Initial fetches or reuses a route. Repro: open this confirmation, wait past expiresAt, then press Receive now; handleOnClaim calls claimLinkXchain with no quote id, expiry, or minimum-receive bound while this row still shows the expired fee and the preceding row shows the expired Min Received. Requote (or return to Initial) at the tap before executing, and cover a confirm screen aged past expiry.

  • MAJOR · src/components/Claim/Link/Initial.view.tsx:732 · Discard out-of-order quote failures too
    The new generation check gates only successful route selection. Repro: quote A for recipient A is slow, switch to B, quote B succeeds, then A rejects. The catch below still unconditionally clears selectedRoute, marks the fetch complete, and installs A's error; finally also clears loading. That can remove the valid B route or leave its confirmation with a stale error. Apply the generation guard to every completion mutation (success, catch, and finally), or abort superseded requests, and test failure-after-newer-success ordering.

  • MINOR · src/components/Claim/Link/Initial.view.tsx:677 · [moonshotai/kimi-k3] Quote-generation guard doesn't cover token/chain switches, so a slow stale quote can still be selected
    In fetchXchainRoute, the generation counter is only incremented for arg-less (auto) calls: const generation = toToken || toChain ? quoteGenerationRef.current : (quoteGenerationRef.current += 1). Scenario: an auto quote for token A is in flight (generation 1); the user switches destination token/chain via the explicit path, which starts a quote for B without bumping the counter; the slow quote for A then resolves and passes generation === quoteGenerationRef.current, so setSelectedRoute(routeA) selects a route whose chain/token/fee no longer match the selection. ConfirmClaimLinkView renders selectedRoute?.feeUsd and selectedRoute?.receiveAmount verbatim next to the newly selected token, so the confirm screen can show A's fee/receive numbers for a claim that will execute on B — exactly the stale-quote landing the generation mechanism was added to prevent (the recipient-change effect bumps the counter, so only token/chain switches are exposed). Execution itself uses the selected token/chain server-side, so the impact is wrong fee display that self-corrects when B's quote is selected. Fix: increment on every new quote (const generation = ++quoteGenerationRef.current) — harmless for the explicit branch, since its selection happens via the later cache lookup.

  • MINOR · src/components/Claim/Link/Initial.view.tsx:733 · [moonshotai/kimi-k3] Concurrent quotes lose cache entries via setRoutes([...routes, route]) stale-closure overwrite
    When two quotes are in flight concurrently (now more likely: entering the view auto-quotes, and the new recipient-change effect can trigger a re-quote while another is still running), both async continuations captured the same routes array. The second to resolve runs setRoutes([...routes, route]) and overwrites the first's entry. The dropped route is then a cache miss the next time that chain/token/recipient is picked, forcing a fresh authenticated preview — each of which costs one Rhino flow credit per the PR's own costing note. Fix: use the functional updater so concurrent resolutions merge: setRoutes((prev) => [...prev, route]).

Checked clean

  • Earlier withdraw quote-expiry finding: FIXED; confirmation checks expiry at the tap and re-quotes before broadcast, with fake-timer coverage.
  • Earlier recipient quote ordering finding: STILL PRESENT in the failure path; successful stale responses are now generation-gated.
  • Earlier expiring SDA estimate finding: STILL PRESENT in pay-request and claim confirmation consumers.
  • Fee source and formatting: bridge and SDA feeUsd values are passed verbatim into the shared row with no component-side fee arithmetic.
  • Recipient cache keying: reusable claim routes must match chain, token, recipient, and an unexpired expiresAt.
  • Security and API boundary: the new depositor and recipient fields remain within authenticated API calls; no new client-side secret or unsafe sink was introduced.
  • Integration and CI: the supplied dev base merges cleanly and all 19 checks completed (17 successful, 2 skipped).

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.

Exact head: fe5de927bacf · Context: repo, api, ops, design

Comment thread src/components/Claim/Link/Onchain/Confirm.view.tsx
Comment thread src/components/Claim/Link/Initial.view.tsx Outdated
Comment thread src/components/Claim/Link/Initial.view.tsx
Comment thread src/components/Claim/Link/Initial.view.tsx
…seded claim quotes never mutate state

Pay-request: executePayment re-quotes instead of broadcasting a route whose
quote expired while the confirm sat open (fake-timer hook test). Claim
confirm: an expired route is dropped and the initial view re-quotes, never
executed. Claim initial view: every quote gets a generation, and a result
that is no longer current — success or failure — neither selects a route,
installs an error, nor clears loading; concurrent quotes merge into the
cache instead of overwriting each other.

TASK-21991

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

The fee-source change is coherent and all exact-head CI checks pass, but claim quote invalidation can still pair one route's displayed economics with another destination, so the PR needs changes before merge.

Findings

  • MAJOR · src/components/Claim/Link/Initial.view.tsx:735 · Invalidate in-flight quotes when the destination changes
    The generation advances only when another fetchRoute call starts. If quote A is in flight and the user switches chain or token, the selection-change effect clears the route but does not advance the generation; while selectedTokenData for B is still loading, A can resolve as current, select route A here, and clear loading. The CTA can then reach confirmation with A's fee/receive amount while claimLinkXchain executes the current chain/token B. Advance the generation synchronously when the selection changes, verify the captured chain/token before selecting, and cover this ordering with a render-level regression test.

  • MINOR · src/utils/claim-route.utils.ts:36 · Use the confirmation lead time for cached claim quotes
    This cache accepts every quote whose expiry is still in the future, but the confirm screen rejects the same quote once it is within the 15-second signing lead. With 10 seconds remaining, returning from confirm immediately reselects the cached route, so Receive now sends the user back again instead of obtaining a fresh quote; this repeats until the timestamp passes. Treat near-expiry entries as cache misses using the same helper/lead time and add the 10-second case to the utility test.

  • MINOR · src/app/(mobile-ui)/withdraw/crypto/page.tsx:364 · Let a failed expiry refresh retry the quote
    On an expired quote this awaits quoteRoute and returns. calculate clears transactions before requesting and records failures in routeError, so a transient refresh failure leaves no transactions. The retry button calls handleConfirmWithdrawal again, which exits at the no-transactions guard and replaces the error with txNotPrepared instead of re-quoting; the user cannot retry in place. Route the retry/no-transactions state back through quoteRoute and cover an expired-then-failed-then-successful refresh.

Checked clean

  • Earlier withdraw confirmation-time expiry finding: FIXED; the tap re-quotes before signing.
  • Earlier semantic pay-request expiry finding: FIXED; executePayment refreshes an aged quote before broadcast.
  • Earlier claim expired-preview execution finding: FIXED; confirmation drops an aged route before claim execution.
  • Earlier recipient quote success/failure ordering findings: FIXED for superseded fetches; both result and failure mutations are generation-guarded.
  • Rhino normalized quote mapping preserves feeUsd, payAmount, and receiveAmount verbatim on SDA and bridge paths.
  • NetworkFeeRow states and localized fee-copy changes were checked across withdraw, pay-request, and claim confirmations.
  • Security pass found no new authorization bypass, secret exposure, or unsafe client trust boundary in the changed code.
  • All exact-head GitHub checks completed successfully, including unit, typecheck, eslint, format, design-system, backend-baseline, and preview checks.
  • Local targeted Jest execution was unavailable because the detached worktree has no node_modules; exact-head CI unit passed.

Second opinion skipped: daily-budget-reached.

Exact head: 9ba89cc9317e · Context: repo, product

Comment thread src/components/Claim/Link/Initial.view.tsx
Comment thread src/utils/claim-route.utils.ts Outdated
Comment thread src/app/(mobile-ui)/withdraw/crypto/page.tsx
…onour the signing lead in the cache; re-quote when nothing is prepared

A chain/token switch now advances the quote generation synchronously, so a
quote still in flight for the old destination can never select a route for
it. The claim route cache treats an entry inside the 15 s signing lead as
a miss — the same rule the confirm screen applies — instead of bouncing
the user between the two screens until the clock passes. On withdraw, a
tap with nothing prepared (an expiry refresh that failed) quotes again
instead of dead-ending on "not prepared".

TASK-21991

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — no blocking findings — this is not an approval

Clean at the pinned head. All six findings from the two earlier reviews are fixed, and no new actionable defect survived verification.

Checked clean

  • Pinned worktree HEAD, trusted author, dev base ref, supplied base SHA, and merge-base branch delta matched the review scope.
  • Earlier pay-request tap-time quote refresh finding: FIXED; expired quotes re-run prepareRoute before broadcast.
  • Earlier on-chain claim expiry finding: FIXED; near-expiry routes are dropped and returned to the quoting view before execution.
  • Earlier out-of-order claim failure finding: FIXED; superseded failures cannot clear or overwrite current route state.
  • Earlier in-flight destination-change finding: FIXED; chain/token changes advance the claim quote generation before clearing selection.
  • Earlier cached-claim lead-time finding: FIXED; cache reuse applies the shared 15-second signing lead.
  • Earlier failed withdrawal expiry-refresh retry finding: FIXED; a tap with no prepared transaction quotes again.
  • Authenticated Rhino quote inputs and normalized fee/pay/receive fields were traced through every frontend call site; feeUsd is no longer combined with a gas component.
  • Shared NetworkFeeRow states, claim recipient-keyed caching, withdraw affordability/minimum guards, and the internal Rhino fee implementation plan were checked.
  • Exact-head CI passed unit, typecheck, eslint, format, analyze, deploy preview, and aggregate ci-success; ds-shots was still in progress. A local focused Jest run was unavailable because the detached worktree has no node_modules.

Second opinion skipped: daily-budget-reached.

Exact head: 9330432df2c6 · Context: repo, ops, ci

@notion-workspace

Copy link
Copy Markdown

1 similar comment
@notion-workspace

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants