feat(card): return excess collateral to wallet on limit decrease#2452
Conversation
Lowering the per-transaction limit left the card holding more than its new target: the auto-balancer only ever tops up (rebalance-decision no-ops when collateral >= limit), so the excess sat in the collateral contract indefinitely. After the limit PATCH succeeds, sign a FORCED collateral-only withdrawal of the excess back to the user's smart wallet - the PATCH-first ordering keeps the auto-balancer from racing the withdrawal by topping the collateral back up. UX: the user only sees the passkey prompt. The unified displayed balance is identical before/after (both buckets are folded into one), and the intent kind AUTO_REBALANCE maps to INTERNAL_TRANSFER, which history hides. Failures are non-fatal by design: the limit change already stuck, and re-saving the limit retries the return. forceStrategy on useSignSpendBundle exists because live-balance routing would pick smart-only (a self-transfer no-op) whenever the smart wallet covers the amount - the exact trap the lock-card flow comments around.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughCard limit decreases now calculate eligible excess collateral, optionally return it through a forced collateral-only Rain withdrawal, refresh wallet data, and emit success or failure analytics. The flow is covered by hook and utility tests. ChangesExcess collateral return flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CardLimitEditModal
participant useReturnExcessCollateral
participant useSignSpendBundle
participant rainApi
CardLimitEditModal->>useReturnExcessCollateral: returnExcess(newLimitCents)
useReturnExcessCollateral->>useSignSpendBundle: sign collateral-only withdrawal
useSignSpendBundle-->>useReturnExcessCollateral: signed withdrawal
useReturnExcessCollateral->>rainApi: submitWithdrawal
rainApi-->>useReturnExcessCollateral: withdrawal result
useReturnExcessCollateral-->>CardLimitEditModal: returned excess cents
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Code-analysis diffPainscore total: 6223 → 6234.78 (+11.78) 🆕 New findings (17)
✅ Resolved (14)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/useSignSpendBundle.ts`:
- Around line 144-164: Update the forced collateral-only branch in
useSignSpendBundle to capture CARD_WITHDRAW_FAILED analytics before invalidating
the overview query and throwing InsufficientSpendableError, matching the
insufficient-funds handling in resolveSpendStrategy. Preserve the existing
collateral balance check and fail-closed behavior.
- Around line 144-164: Add direct tests for the forced collateral-only branch in
useSignSpendBundle: cover successful signing when rainSpendingPower meets
requiredUsdcAmount, and insufficient funds when it does not. Verify the
insufficient path invalidates RAIN_CARD_OVERVIEW_QUERY_KEY and throws
InsufficientSpendableError, while the success path proceeds without resolving
another strategy.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 894571ac-397f-4dfd-bda5-582a636e6479
📒 Files selected for processing (7)
src/components/Card/CardLimitEditModal.tsxsrc/constants/analytics.consts.tssrc/hooks/wallet/__tests__/useReturnExcessCollateral.test.tsxsrc/hooks/wallet/useReturnExcessCollateral.tssrc/hooks/wallet/useSignSpendBundle.tssrc/utils/__tests__/balance.utils.test.tssrc/utils/balance.utils.ts
CodeRabbit review: the forced-path insufficient branch skipped the CARD_WITHDRAW_FAILED capture resolveSpendStrategy emits, leaving a gap in the withdraw funnel for the excess-return flow; and the forced branch had no direct tests.
Summary
Lowering the per-transaction card limit left the card's collateral above its new target forever: the auto-balancer only tops up (
rebalance-decision.tsno-ops when collateral ≥ limit) and nothing on the decrease path ever moved funds back. This PR returns the excess (spendingPower − new limit) to the user's smart wallet right after a limit decrease.UX contract (deliberate): the user knows nothing about collateral and sees exactly one balance. So the flow is: tap Save changes → limit PATCH → (only if there's ≥ $1 excess) the passkey prompt → done. No explanatory copy, no extra modal. The unified displayed balance is identical before and after (both buckets are folded into one), and the withdrawal intent (
AUTO_REBALANCE→INTERNAL_TRANSFER) is hidden from history.Ordering matters: the withdrawal runs only after the limit PATCH succeeds. The auto-balancer targets the DB
cardLimit; withdrawing first would race it topping the collateral straight back up.Failure is non-fatal by design: if the user cancels the passkey or Rain's withdrawal-sig cooldown is active, the limit change has already stuck, the visible balance is unchanged either way (funds stay spendable through normal spend routing), and re-saving the limit retries the return. We capture
card_limit_excess_return_failedfor observability.How
useSignSpendBundlegainsforceStrategy?: 'collateral-only'— live-balance routing would picksmart-only(a self-transfer no-op) whenever the smart wallet covers the amount. Affordability is still enforced againstrainSpendingPower; the shared collateral preflight (session-key grant) is unchanged.useReturnExcessCollateralhook: computes the excess (pure helpercomputeExcessCollateralCents, $1 threshold mirroring the auto-balancer'sREBALANCE_THRESHOLD), signs the forced collateral-only withdrawal to the user's own wallet, submits via the existing/rain/cards/withdraw/submit, then invalidates both balance queries.CardLimitEditModalcalls it after a successfulperAuthorizationPATCH.No backend changes — prepare/submit, session-key relay, and the history-hiding all exist.
Risks / breaking changes
forceStrategyis additive/optional — all existingsignSpendcallers are untouched (default path identical).Design notes / accepted trade-offs
runCollateralSpendPreflightbehavior, identical to the lock-card flow.LockCardModal/CancelCardModalroute their "return everything before lock/cancel" withdrawal through strategy resolution and throw 'Unexpected withdrawal strategy' if the smart wallet balance ≥ spendingPower (routing picks smart-only). They should migrate toforceStrategy: 'collateral-only'— follow-up PR.QA
npm test— 2007/2008 pass; the one failure (add-money-states› "loaded EVM deposit shows QR code and address") reproduces on pristineorigin/dev(pre-existing, unrelated).computeExcessCollateralCents(7 cases) +useReturnExcessCollateralcontract tests (skip-below-threshold / forced-strategy args / fail-closed / no-submit-on-sign-fail).Summary by CodeRabbit
New Features
Bug Fixes
Tests