fix(card): route spends on live on-chain smart balance (fixes #2230 incident, keeps smart-first)#2234
Merged
Merged
Conversation
…value Follow-up to the #2230 incident. Smart-first routing is correct, but it trusted the FE's cached smartBalance (useBalance, 30s cache / read before the smart->collateral sweep). For card users the smart account is swept ~empty into collateral, so a stale balance >= amount routed smart-only to an empty account and the USDC transfer reverted on-chain (ERC20: transfer amount exceeds balance). Both routers (useSpendBundle.spend + useSignSpendBundle.signSpend) now read the LIVE balanceOf the exact kernel account that will send the UserOp (fetchLiveSmartUsdcBalance) before computeSpendStrategy, and use it for routing and the mixed shortfall. getClientForChain already asserts the client belongs to the logged-in user. Removes the now-redundant smartBalance input from both hooks and all call sites. Keeps smart-first so users with smart-account USDC avoid Rain's withdrawal-signature cooldown.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Walkthrough
ChangesLive USDC Balance Routing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Code-analysis diffPainscore total: 5786.03 → 5719.2 (-66.83) 🆕 New findings (51)
…and 31 more. ✅ Resolved (50)
…and 30 more. 📈 Painscore deltas (top movers)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/wallet/useSpendBundle.ts`:
- Around line 191-192: In the useSpendBundle hook, replace the non-null
assertion on kernelClient.account!.address with an explicit guard check that
verifies kernelClient.account is initialized before accessing the address
property. Follow the same pattern used in useSignSpendBundle (lines 117-121) by
checking if the account exists and throwing a descriptive error when it is
uninitialized, rather than relying on the non-null assertion which would produce
a cryptic "Cannot read property 'address' of undefined" error if the kernel
context hasn't hydrated yet.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58ccb6fb-abc5-442e-a53f-51478526a3c9
📒 Files selected for processing (9)
src/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/components/Card/CancelCardModal.tsxsrc/components/Card/LockCardModal.tsxsrc/hooks/wallet/__tests__/useSpendBundle.test.tssrc/hooks/wallet/useSendMoney.tssrc/hooks/wallet/useSignSpendBundle.tssrc/hooks/wallet/useSpendBundle.tssrc/hooks/wallet/useWallet.ts
💤 Files with no reviewable changes (4)
- src/components/Card/CancelCardModal.tsx
- src/components/Card/LockCardModal.tsx
- src/app/(mobile-ui)/qr-pay/page.tsx
- src/hooks/wallet/useWallet.ts
Comment on lines
+191
to
+192
| const kernelClient = getClientForChain(chainIdStr) | ||
| const smartBalance = await fetchLiveSmartUsdcBalance(kernelClient.account!.address) |
Contributor
There was a problem hiding this comment.
Add a guard for kernelClient.account before accessing .address.
useSignSpendBundle (lines 117-121) explicitly checks kernelClient.account and throws a descriptive error when uninitialized. This hook uses a non-null assertion that could produce a cryptic Cannot read property 'address' of undefined if the kernel context hasn't hydrated yet.
Suggested fix
const kernelClient = getClientForChain(chainIdStr)
- const smartBalance = await fetchLiveSmartUsdcBalance(kernelClient.account!.address)
+ const kernelAccount = kernelClient.account
+ if (!kernelAccount) {
+ throw new Error('useSpendBundle: kernel account not initialized')
+ }
+ const smartBalance = await fetchLiveSmartUsdcBalance(kernelAccount.address)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const kernelClient = getClientForChain(chainIdStr) | |
| const smartBalance = await fetchLiveSmartUsdcBalance(kernelClient.account!.address) | |
| const kernelClient = getClientForChain(chainIdStr) | |
| const kernelAccount = kernelClient.account | |
| if (!kernelAccount) { | |
| throw new Error('useSpendBundle: kernel account not initialized') | |
| } | |
| const smartBalance = await fetchLiveSmartUsdcBalance(kernelAccount.address) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/wallet/useSpendBundle.ts` around lines 191 - 192, In the
useSpendBundle hook, replace the non-null assertion on
kernelClient.account!.address with an explicit guard check that verifies
kernelClient.account is initialized before accessing the address property.
Follow the same pattern used in useSignSpendBundle (lines 117-121) by checking
if the account exists and throwing a descriptive error when it is uninitialized,
rather than relying on the non-null assertion which would produce a cryptic
"Cannot read property 'address' of undefined" error if the kernel context hasn't
hydrated yet.
… races Two follow-ups to #2234, both on the smart-first spend path. 1. Kill the duplicated balance read. #2234 added a standalone fetchLiveSmartUsdcBalance — a second copy of useBalance's readContract(balanceOf). Routing now force-refetches the SAME TanStack query (smartUsdcBalanceQueryOptions, staleTime:0): one source of truth, one readContract, and the displayed balance refreshes in the same call instead of lying until the next 30s poll. 2. Recoverable routing. A smart-only spend can still lose the read -> sweep -> submit race (card funds get swept smart->collateral after we route). A thrown smart-only UserOp never broadcast — handleSendUserOpEncoded's receipt-timeout path returns, only a failed sendUserOperation throws — so we re-read the live balance and, if it dropped below the amount, retry on collateral/mixed. Provably no double-spend. Real failures (balance still covers, stale key) rethrow untouched. Tests: pure rerouteAfterSmartOnlySweep cases + a renderHook test proving a swept smart-only retries on collateral exactly once (and a non-sweep failure rethrows without touching collateral).
`import/first` isn't registered in this repo's eslint config, so the
`// eslint-disable-next-line import/first` directive errors ("rule not
found"). The late import it guarded isn't flagged either (no such rule), so
the comment is pure dead weight. Removing it from the new reroute test (net
0) and the sibling unit test (−1 from the standing eslint count).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/wallet/__tests__/useSpendBundle.reroute.test.tsx`:
- Line 78: The ESLint suppression comment on line 78 of the
useSpendBundle.reroute.test.tsx file disables the `import/first` rule, but this
rule is not defined in the active ESLint configuration, causing the suppression
itself to fail linting. Remove the entire eslint-disable-next-line comment line
that references the non-existent import/first rule. If mocks truly need to
register before imports, verify the actual ESLint rule name in your
configuration and update the suppression accordingly, or restructure the code to
comply with active linting rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b236469d-245d-4a31-abe2-cdcd01316c2d
📒 Files selected for processing (5)
src/hooks/wallet/__tests__/useSpendBundle.reroute.test.tsxsrc/hooks/wallet/__tests__/useSpendBundle.test.tssrc/hooks/wallet/useBalance.tssrc/hooks/wallet/useSignSpendBundle.tssrc/hooks/wallet/useSpendBundle.ts
Review found the smart-only reroute could double-spend. `client.sendUserOperation` is a network RPC that can throw AFTER the bundler accepts the UserOp (chronic on mobile ZeroDev RPC), so "thrown => never broadcast" does not hold. The reroute would then re-read the already-debited balance, classify it as a sweep, and pay a second time from collateral — strictly worse than the revert it was healing. Keep fix #1 only: routing reads the live balance through the SHARED smartUsdcBalanceQueryOptions query (force-refetch, staleTime:0) instead of a duplicated readContract — one source of truth, and the displayed balance refreshes in the same call. This already collapses the 30s stale-cache window that caused incident #2230. Residual (accepted): a sweep landing in the sub-second window between the live read and on-chain execution still reverts the payment (terminal, manual retry — safe, no double-spend). The general fix is backend-authoritative routing, filed as a follow-up.
…ed its test) Completes the previous commit: removes rerouteAfterSmartOnlySweep, the runStrategy extraction, and the smart-only reroute wrappers from useSpendBundle.spend and useSignSpendBundle.signSpend, plus the reroute unit tests. Routing keeps fix #1 only (shared smartUsdcBalanceQueryOptions, force-refetch staleTime:0).
This was referenced Jun 17, 2026
Hugo0
added a commit
that referenced
this pull request
Jun 24, 2026
…op settling-at-input) Gating money-flows at input on an "available-now" subset was wrong: the FE balance is only ~30s-polled while the spend routing reads the chain live at submit (#2234), so an input-time available-now gate blocks spends that would actually succeed — and the ~15s flow naturally lets the ~10-45s collateral rebalance settle. So gate on the DISPLAYED balance (block only a true shortfall) and let in-transit spends fail late on live execution. - useWallet: hasSufficientSpendableBalance now gates on the displayed spendableBalance. Deletes spendBlockReason / availableSpendableBalance / SPEND_BLOCK_MESSAGE and the whole "settling at input" apparatus (−55 net). - Refetch on failure (TanStack): on InsufficientSpendableError the routing already re-read live smart balance, so the two bundle hooks (useSpendBundle / useSignSpendBundle) invalidate [RAIN_CARD_OVERVIEW_QUERY_KEY] before throwing — the displayed balance + a retry de-stale immediately instead of waiting out the 30s poll. - Informative failure copy: the post-gate in-transit failure now says "Your balance isn't fully available yet — try again in a few seconds" (one shared BALANCE_SETTLING_MESSAGE) across send (ErrorHandler), qr-pay, manteca, useSendMoney — instead of a misleading "add funds". - Keeps the display-field fix + the formattedSpendableBalance formatting consolidation. Net -39 lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚑 Incident fix — keeps smart-first, fixes the on-chain revert
Follow-up to #2230. Smart-first routing is correct, but it trusted the FE's cached
smartBalance(useBalance, 30s cache, often read before the smart→collateral sweep). Card funds are swept smart→collateral to back the card, so the smart account is normally ~empty — a stale balance>= amountroutedsmart-onlyto an empty account and the USDC transfer reverted on-chain (ERC20: transfer amount exceeds balance).Fix: both routers —
useSpendBundle.spendanduseSignSpendBundle.signSpend— now read the livebalanceOfof the exact kernel account that will send the UserOp (fetchLiveSmartUsdcBalance), right beforecomputeSpendStrategy, and use it for routing and themixedshortfall.getClientForChainalready asserts the client belongs to the logged-in user, so sender + balance are an authoritative pair (also closes the stale-key/account-switch angle).Removes the now-redundant
smartBalanceinput from both hooks and every call site (useSendMoney,useWallet.sendTransactions, qr-pay, withdraw/manteca, Lock/CancelCardModal) so a cached value can never drive routing again.Net: keeps Konrad's smart-first win (smart-account USDC → no Rain cooldown), and collateral-funded users route collateral-only/mixed correctly.
Replaces
Supersedes the revert PR #2233 (we keep smart-first instead of reverting).
Risk
balanceOfread per spend (negligible;useBalancealready polls every 30s). If the read throws, the spend errors clearly instead of reverting on-chain.QA
npx jest src/hooks/wallet— 21 pass, incl. newfetchLiveSmartUsdcBalancetests (readsbalanceOfof the sender; returns chain value).next build✅.🤖 Generated with Claude Code