feat(rain): init→passkey→prepare flow for all collateral spend callers (TASK-19573)#2027
Open
jjramirezn wants to merge 3 commits into
Open
feat(rain): init→passkey→prepare flow for all collateral spend callers (TASK-19573)#2027jjramirezn wants to merge 3 commits into
jjramirezn wants to merge 3 commits into
Conversation
Pairs with peanut-api-ts feat/rain-withdraw-init-flow. Rain locks out the user for ~7min when they cancel a passkey because /prepare already burned a signature. Now we call POST /rain/cards/withdraw/init first (no Rain call), pass the resulting EIP-712 fields to the passkey, then call /prepare with the pre-signed admin sig. /prepare verifies the sig BEFORE calling Rain. Cancelled passkey → zero Rain sigs burned. Scoped to useSpendBundle's collateral-only branch (BE-broadcasted, the most common path). Mixed-strategy + useSignSpendBundle (QR pay, manteca offramp, lock/cancel card modals) stay on the legacy /prepare-first flow — they need their own restructure since the FE broadcasts the kernel UserOp itself. Follow-up. TASK-19573
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughAdds a Rain withdrawal init step and types, exposes rainApi.initWithdrawal, and refactors both useSignSpendBundle and useSpendBundle (collateral-only and mixed) to call init, derive admin EIP-712 signing inputs from init, then call prepare with initId and adminSignature before submission. ChangesRain withdrawal init-prepare flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Code-analysis diffPainscore total: 5649.23 → 5610.88 (-38.35) 🆕 New findings (16)
✅ Resolved (16)
📈 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/wallet/useSpendBundle.ts (1)
209-227:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSign against the chain returned by
/init.The init response is now the authoritative EIP-712 bundle, but this still signs with
PEANUT_WALLET_CHAIN.idand looks up the client withchainIdStr. If frontend config ever drifts from the backend-issued init chain, the admin signature is invalid and every collateral-only withdrawal fails. Useinit.chainIdhere, and ideally assert it matches the local constant before proceeding.🔧 Suggested fix
- const kernelClient = getClientForChain(chainIdStr) + const initChainId = Number(init.chainId) + if (initChainId !== chainIdNum) { + throw new Error(`useSpendBundle: init chain mismatch (${init.chainId})`) + } + const kernelClient = getClientForChain(init.chainId) const adminSignature = (await kernelClient.account!.signTypedData({ domain: { name: RAIN_WITHDRAW_EIP712_DOMAIN_NAME, version: RAIN_WITHDRAW_EIP712_DOMAIN_VERSION, - chainId: chainIdNum, + chainId: initChainId, verifyingContract: init.collateralProxy as Address, salt: init.adminSalt as Hex, },🤖 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 209 - 227, The admin signature is being produced using the local chain lookup (getClientForChain / chainIdStr / PEANUT_WALLET_CHAIN.id) instead of the authoritative chain from the init response; change the client lookup and signing to use init.chainId (e.g., call getClientForChain(String(init.chainId)) and use init.chainId for the EIP-712 domain chainId) and add a runtime assertion that init.chainId matches the expected PEANUT_WALLET_CHAIN.id constant before proceeding (throw or log/abort if it does not) so adminSignature and kernelClient are derived from the backend-provided chain.
🧹 Nitpick comments (1)
src/services/rain.ts (1)
91-96: ⚡ Quick winMake the init-flow fields all-or-none in the request type.
PrepareRainWithdrawalInputcurrently allowsinitIdwithoutadminSignature(and vice versa), but the contract described here is “both or neither”. Encoding that as a union prevents future callers from compiling an invalid/preparerequest and only discovering it via a runtime 4xx.♻️ Proposed type shape
-export interface PrepareRainWithdrawalInput { +type PrepareRainWithdrawalBase = { amount: string recipientAddress: string directTransfer: boolean kind: RainCollateralKind totalAmountCents?: string chargeId?: string - initId?: string - adminSignature?: string -} +} + +type PrepareRainWithdrawalLegacy = { + initId?: never + adminSignature?: never +} + +type PrepareRainWithdrawalInitFlow = { + initId: string + adminSignature: string +} + +export type PrepareRainWithdrawalInput = + PrepareRainWithdrawalBase & (PrepareRainWithdrawalLegacy | PrepareRainWithdrawalInitFlow)🤖 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/services/rain.ts` around lines 91 - 96, PrepareRainWithdrawalInput currently permits initId and adminSignature independently but needs to be an all-or-none pair; change the type definition for PrepareRainWithdrawalInput to a union that enforces either both fields present (e.g., { initId: string; adminSignature: string }) or neither present (e.g., { initId?: undefined; adminSignature?: undefined }) so callers cannot compile with only one set. Update any functions handling PrepareRainWithdrawalInput (references to PrepareRainWithdrawalInput, prepareRainWithdrawal, validate/handler code) to match the new shape (use type narrowing or runtime checks where necessary) so code compiles and handles both-unset and both-set cases.
🤖 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.
Outside diff comments:
In `@src/hooks/wallet/useSpendBundle.ts`:
- Around line 209-227: The admin signature is being produced using the local
chain lookup (getClientForChain / chainIdStr / PEANUT_WALLET_CHAIN.id) instead
of the authoritative chain from the init response; change the client lookup and
signing to use init.chainId (e.g., call getClientForChain(String(init.chainId))
and use init.chainId for the EIP-712 domain chainId) and add a runtime assertion
that init.chainId matches the expected PEANUT_WALLET_CHAIN.id constant before
proceeding (throw or log/abort if it does not) so adminSignature and
kernelClient are derived from the backend-provided chain.
---
Nitpick comments:
In `@src/services/rain.ts`:
- Around line 91-96: PrepareRainWithdrawalInput currently permits initId and
adminSignature independently but needs to be an all-or-none pair; change the
type definition for PrepareRainWithdrawalInput to a union that enforces either
both fields present (e.g., { initId: string; adminSignature: string }) or
neither present (e.g., { initId?: undefined; adminSignature?: undefined }) so
callers cannot compile with only one set. Update any functions handling
PrepareRainWithdrawalInput (references to PrepareRainWithdrawalInput,
prepareRainWithdrawal, validate/handler code) to match the new shape (use type
narrowing or runtime checks where necessary) so code compiles and handles
both-unset and both-set cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a0d1730-52ce-4c68-b955-40b96e9f1597
📒 Files selected for processing (2)
src/hooks/wallet/useSpendBundle.tssrc/services/rain.ts
Same change as the collateral-only branch — call /init first (no Rain), let the user sign the admin EIP-712, then call /prepare with the pre-signed sig. /prepare verifies the sig before touching Rain. Mixed and signSpend each have TWO passkey prompts (admin sig + kernel UserOp). The init flow saves a Rain burn on the FIRST one; the second still burns if cancelled because Rain has to be called between the two prompts (its executor sig is part of the UserOp's calldata). ~half the burn cases solved — same fix shape as collateral-only, no reason to ship it later. useSpendBundle.spend() mixed branch + useSignSpendBundle.signSpend() collateral-only + mixed branches all migrated. No BE changes needed (peanut-api-ts feat/rain-withdraw-init-flow already accepts the init flow optionally).
jjramirezn
changed the title
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/wallet/useSpendBundle.ts (1)
209-227:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse one checked kernel account for both Rain signatures and the mixed-path admin address.
Lines 210 and 310 still dereference
kernelClient.account!, while Lines 287-290 prepare the mixed withdrawal withuser.accountsinstead of the signer. If auth and kernel state drift,/initcan be created for one address and the typed data signed by another, or either branch can throw before the passkey prompt. Hoisting a guardedkernelAccountonce and reusingkernelAccount.addresshere keeps the init payload and signature aligned.🛠️ Proposed fix
const chainIdNum = PEANUT_WALLET_CHAIN.id const chainIdStr = chainIdNum.toString() + const kernelClient = getClientForChain(chainIdStr) + const kernelAccount = kernelClient.account + if (!kernelAccount) { + throw new Error('useSpendBundle: kernel account not initialized') + } // ─── collateral-only ────────────────────────────────────────────── // init → passkey → prepare → submit. /init does no Rain call, // so if the user cancels the passkey at step 2 we never burned @@ - const kernelClient = getClientForChain(chainIdStr) - const adminSignature = (await kernelClient.account!.signTypedData({ + const adminSignature = (await kernelAccount.signTypedData({ domain: { name: RAIN_WITHDRAW_EIP712_DOMAIN_NAME, version: RAIN_WITHDRAW_EIP712_DOMAIN_VERSION, chainId: chainIdNum, @@ - const adminAddress = user?.accounts.find((a) => a.type === AccountType.PEANUT_WALLET)?.identifier as - | Address - | undefined - if (!adminAddress) throw new Error('useSpendBundle: missing peanut-wallet address for mixed spend') + const adminAddress = kernelAccount.address as Address @@ - const kernelClient = getClientForChain(chainIdStr) - const adminSignature = (await kernelClient.account!.signTypedData({ + const adminSignature = (await kernelAccount.signTypedData({ domain: { name: RAIN_WITHDRAW_EIP712_DOMAIN_NAME, version: RAIN_WITHDRAW_EIP712_DOMAIN_VERSION, chainId: chainIdNum,Also applies to: 287-337
🤖 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 209 - 227, Dereference and validate the kernel account once and reuse it for both EIP-712 admin signing and the mixed-path admin address assembly to avoid signer/state drift: call getClientForChain(chainIdStr), assert kernelClient.account exists into a guarded local (e.g., kernelAccount) and use kernelAccount.address wherever init.adminAddress/mixed withdrawal user signer and signTypedData use kernelClient.account! (affecting adminSignature creation and the mixed withdrawal path around the code that builds the init payload and uses user.accounts); ensure any early errors are thrown before user prompts so the same checked kernelAccount is used for signing and for populating init.adminAddress.
🤖 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.
Outside diff comments:
In `@src/hooks/wallet/useSpendBundle.ts`:
- Around line 209-227: Dereference and validate the kernel account once and
reuse it for both EIP-712 admin signing and the mixed-path admin address
assembly to avoid signer/state drift: call getClientForChain(chainIdStr), assert
kernelClient.account exists into a guarded local (e.g., kernelAccount) and use
kernelAccount.address wherever init.adminAddress/mixed withdrawal user signer
and signTypedData use kernelClient.account! (affecting adminSignature creation
and the mixed withdrawal path around the code that builds the init payload and
uses user.accounts); ensure any early errors are thrown before user prompts so
the same checked kernelAccount is used for signing and for populating
init.adminAddress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2efb27c9-1444-4481-b142-d2a9eb441ec1
📒 Files selected for processing (2)
src/hooks/wallet/useSignSpendBundle.tssrc/hooks/wallet/useSpendBundle.ts
Contributor
Author
|
@coderabbitai review in depth. Evaluate:
Also, write a very concise testing guide for QA |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 53939
Risk of Introducing New Bugs🔴
|
| Path | Before | After | Delta |
|---|---|---|---|
| collateral-only | 2 API calls (prepare, submit) + 1 passkey | 3 API calls (init, prepare, submit) + 1 passkey | +1 RTT |
| mixed | 2 API calls (prepare) + passkey + UserOp | 3 API calls (init, prepare) + passkey + UserOp | +1 RTT |
| smart-only | unchanged | unchanged | 0 |
The +1 RTT is the explicit trade-off for never burning a Rain signature on cancellation. One minor sequencing inefficiency: getClientForChain is called after the await initWithdrawal(...) call in both useSpendBundle.ts paths. Since getClientForChain is synchronous, moving it before the init await would pipeline marginally better and also fail fast if the kernel client is unavailable before a network call is made.
// Move these lines above the initWithdrawal call:
const kernelClient = getClientForChain(chainIdStr)
if (!kernelClient.account) throw new Error('...')
const amountCents = ...
const init = await rainApi.initWithdrawal(...)Code Quality & DRY
🔴 EIP-712 signing block duplicated 4×
The following block appears identically in 4 places (collateral-only + mixed in each of the two hooks):
const adminSignature = (await <account>.signTypedData({
domain: {
name: RAIN_WITHDRAW_EIP712_DOMAIN_NAME,
version: RAIN_WITHDRAW_EIP712_DOMAIN_VERSION,
chainId: chainIdNum,
verifyingContract: init.collateralProxy as Address,
salt: init.adminSalt as Hex,
},
types: rainWithdrawEip712Types,
primaryType: 'Withdraw',
message: {
user: init.adminAddress as Address,
asset: init.tokenAddress as Address,
amount: BigInt(init.amount),
recipient: init.recipientAddress as Address,
nonce: BigInt(init.adminNonce),
},
})) as HexThis should be extracted into a shared utility — e.g. in utils/rain.utils.ts or co-located in constants/rain.consts.ts:
export async function signRainAdminWithdraw(
account: KernelAccount,
chainId: number,
init: InitRainWithdrawalResponse
): Promise<Hex> {
return (await account.signTypedData({
domain: {
name: RAIN_WITHDRAW_EIP712_DOMAIN_NAME,
version: RAIN_WITHDRAW_EIP712_DOMAIN_VERSION,
chainId,
verifyingContract: init.collateralProxy as Address,
salt: init.adminSalt as Hex,
},
types: rainWithdrawEip712Types,
primaryType: 'Withdraw',
message: {
user: init.adminAddress as Address,
asset: init.tokenAddress as Address,
amount: BigInt(init.amount),
recipient: init.recipientAddress as Address,
nonce: BigInt(init.adminNonce),
},
})) as Hex
}This also eliminates the kernelClient.account! vs kernelAccount discrepancy at the same time, since the guard would live at the call site once.
🟡 prepareWithdrawal call pattern repeated
The initWithdrawal → signTypedData → prepareWithdrawal trio for collateral-only is near-identical between useSpendBundle.ts and useSignSpendBundle.ts. The difference is only what happens to prep afterward (submit vs. return). A shared helper initAndPrepareCollateralWithdrawal(account, chainId, params) returning { prep, adminSignature } would cut duplication further and ensure both hooks stay in sync when the API contract evolves.
Concise QA Testing Guide
Prerequisites
- A wallet with known Rain card in good standing
- Test amounts: one < smart balance, one > smart balance but < smart + collateral, one requiring collateral only
Test Matrix
| # | Scenario | Expected |
|---|---|---|
| 1 | Collateral-only, happy path — send USDC where Rain covers 100% | Single passkey prompt → tx submitted → receipt shows collateral-only strategy |
| 2 | Collateral-only, passkey cancel — tap cancel at the passkey sheet | No Rain signature burned; user can immediately retry; CARD_WITHDRAW_FAILED event fired |
| 3 | Collateral-only, cancel then re-attempt | Second attempt succeeds normally (no 7-min lockout) |
| 4 | Mixed strategy — send amount requiring both smart + Rain | Two prompts (admin EIP-712 then UserOp); funds arrive at recipient; intent stamped |
| 5 | Mixed, cancel admin passkey | No Rain sig burned; retry succeeds |
| 6 | Mixed, cancel UserOp passkey | Rain sig IS burned (known limitation per PR scope); confirm 7-min cooldown behavior |
| 7 | Smart-only — send amount covered entirely by smart balance | Single UserOp passkey; no /init call in network tab |
| 8 | Session-key grant missing | Extra grant passkey fires before the spend passkey; flow continues normally after |
| 9 | chargeId flow (collateral-only + charge) | chargeId visible in /init request body; history shows correct category; caller must NOT call recordPayment |
| 10 | QR Pay / Manteca (useSignSpendBundle) — collateral-only | initWithdrawal called with directTransfer: true; signed artifact returned; backend broadcasts |
| 11 | QR Pay / Manteca — mixed | initWithdrawal called with directTransfer: false; signed UserOp returned |
| 12 | Slow passkey (>init TTL) — delay >init expiry between prompt and confirmation | Graceful error, not a raw "Request failed: 410" |
Network tab checks: confirm
/rain/cards/withdraw/initis called before any passkey prompt for all collateral-touching flows, and that/rain/cards/withdraw/preparecarriesinitId+adminSignaturein the request body.
- useSpendBundle (collateral-only + mixed): resolve kernel account + admin address synchronously BEFORE the init call. Mirrors useSignSpendBundle's pattern. Fails fast if auth is still hydrating (was silent crash via bare `kernelClient.account!.signTypedData` non-null assertion) and pipelines the sync setup with the async init call's start. - friendly-error.utils: surface a retry-friendly 'Withdrawal session expired — please try again.' for /prepare's 410 (init TTL expiry, race with a parallel consume, or foreign initId). Otherwise the raw 'Request failed: 410' / 'Unknown or expired init' leaked through the generic error handler — a UX cliff on low-end devices where the passkey prompt can stall past the 5min TTL.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/friendly-error.utils.tsx (1)
40-44: ⚡ Quick winMatch init-expiry substrings against both
textandmessage.Line 40-44 currently checks only
text; if the backend phrase lands inmessageinstead, users can still see generic/raw errors. Checking both fields makes this mapper more reliable.Proposed diff
export const rainCollateralErrorMessage = (error: unknown): string | null => { const { text, message } = extractErrorParts(error) + const searchable = `${text} ${message ?? ''}` if ( text.includes('A previous withdrawal is still active for this card') || text.includes('A previous withdrawal signature is still active') || text.includes('Insufficient collateral balance for this withdrawal') ) { return message ?? text } @@ - if ( - text.includes('Unknown or expired init') || - text.includes('Init was just consumed') || - text.includes('Init does not belong to this user') - ) { + if ( + searchable.includes('Unknown or expired init') || + searchable.includes('Init was just consumed') || + searchable.includes('Init does not belong to this user') + ) { return 'Withdrawal session expired — please try again.' }🤖 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/utils/friendly-error.utils.tsx` around lines 40 - 44, Update the init-expiry substring checks in friendly-error.utils.tsx to test both the existing `text` variable and the `message` variable (where backend errors may appear); inside the if-condition that currently inspects `text.includes('Unknown or expired init') || text.includes('Init was just consumed') || text.includes('Init does not belong to this user')`, extend each check to also OR `message.includes(...)` so the same three substrings are matched against both `text` and `message`, ensuring the same friendly mapping logic in the function that uses these variables.
🤖 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.
Nitpick comments:
In `@src/utils/friendly-error.utils.tsx`:
- Around line 40-44: Update the init-expiry substring checks in
friendly-error.utils.tsx to test both the existing `text` variable and the
`message` variable (where backend errors may appear); inside the if-condition
that currently inspects `text.includes('Unknown or expired init') ||
text.includes('Init was just consumed') || text.includes('Init does not belong
to this user')`, extend each check to also OR `message.includes(...)` so the
same three substrings are matched against both `text` and `message`, ensuring
the same friendly mapping logic in the function that uses these variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3cc6f389-7a5c-4582-8a2e-2a7a83737e82
📒 Files selected for processing (2)
src/hooks/wallet/useSpendBundle.tssrc/utils/friendly-error.utils.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/wallet/useSpendBundle.ts
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
1 participant
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.
Pairs with peanut-api-ts #785 — deploy together.
Why
Rain locks a user out for up to ~7min when they cancel a passkey because the legacy `/prepare` already burned a Rain signature. The init flow defers the Rain call to AFTER the user commits, so passkey-cancel = zero Rain sig burn.
Coverage
Migrated all four collateral-touching callsites:
Mixed paths have two prompts because the kernel UserOp's calldata embeds Rain's executor signature — Rain must be called between the two prompts. We can't avoid burning on the second cancel without restructuring the on-chain coordinator interface (out of scope).
Affected user flows
All ship in this one PR.
Tests
QA
🤖 Generated with Claude Code