fix: x-chain payments (daimo + peanut)#1447
Merged
kushagrasarathe merged 4 commits intopeanut-wallet-devfrom Nov 12, 2025
Merged
fix: x-chain payments (daimo + peanut)#1447kushagrasarathe merged 4 commits intopeanut-wallet-devfrom
kushagrasarathe merged 4 commits intopeanut-wallet-devfrom
Conversation
Contributor
kushagrasarathe
commented
Nov 12, 2025
kushagrasarathe
commented
- fixes TASK-16439 : add cross chain token selector for peanut wallet payments to semantic requests
- also fixes TASK-16803 : fix daimo x-chain destination chain issue
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughRemoves the external-wallet fulfillment UI and related context state, deletes associated components, and adds token/chain customization and external-invocation support for Daimo Pay plus TokenSelector integration for external ADDRESS/ENS recipient flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Payment/PaymentForm/index.tsx (1)
181-207: Fix default token initialization for external recipientsOn the first render
selectedChainIDis still'', so the USDC fallback inside theelse if (isExternalRecipient && !selectedTokenAddress && selectedChainID)branch never executes. We then callsetInitialSetupDone(true), the effect short-circuits on the next pass, and external ADDRESS/ENS flows end up without a default token (canInitiatePaymentstaysfalseuntil the user manually picks one). Please derive the chain id for defaults before the state updates (or delay flippinginitialSetupDone) so the token fallback can run in the same pass. One option is:- if (chain) { - setSelectedChainID((chain.chainId || requestDetails?.chainId) ?? '') - if (!token && !requestDetails?.tokenAddress) { - const defaultToken = chain.tokens.find((t) => t.symbol.toLowerCase() === 'usdc') - if (defaultToken) { - setSelectedTokenAddress(defaultToken.address) - // Note: decimals automatically derived by useTokenPrice hook - } - } - } else if (isExternalRecipient && !selectedChainID) { - // default to arbitrum for external recipients if no chain specified - setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString()) - } - - if (token) { - setSelectedTokenAddress((token.address || requestDetails?.tokenAddress) ?? '') - // Note: decimals automatically derived by useTokenPrice hook - } else if (isExternalRecipient && !selectedTokenAddress && selectedChainID) { - // default to USDC for external recipients if no token specified - const chainData = supportedSquidChainsAndTokens[selectedChainID] - const defaultToken = chainData?.tokens.find((t) => t.symbol.toLowerCase() === 'usdc') - if (defaultToken) { - setSelectedTokenAddress(defaultToken.address) - } - } + const effectiveChainId = + chain?.chainId?.toString() ?? + requestDetails?.chainId?.toString() ?? + (isExternalRecipient ? PEANUT_WALLET_CHAIN.id.toString() : undefined) + + if (effectiveChainId && effectiveChainId !== selectedChainID) { + setSelectedChainID(effectiveChainId) + } + + if (token?.address || requestDetails?.tokenAddress) { + setSelectedTokenAddress((token?.address || requestDetails?.tokenAddress) ?? '') + } else if (isExternalRecipient && !selectedTokenAddress && effectiveChainId) { + const defaultToken = + chain?.tokens?.find((t) => t.symbol.toLowerCase() === 'usdc') ?? + supportedSquidChainsAndTokens[effectiveChainId]?.tokens.find( + (t) => t.symbol.toLowerCase() === 'usdc' + ) + if (defaultToken) { + setSelectedTokenAddress(defaultToken.address) + } + }This keeps the default behavior working for both URL-provided chains and the Arbitrum fallback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/app/[...recipient]/client.tsx(0 hunks)src/components/Common/ActionList.tsx(5 hunks)src/components/Common/ActionListDaimoPayButton.tsx(4 hunks)src/components/Global/DaimoPayButton/index.tsx(3 hunks)src/components/Payment/PaymentForm/index.tsx(10 hunks)src/components/Request/views/ExternalWalletFulfilManager.tsx(0 hunks)src/components/Request/views/ExternalWalletFulfilMethods.tsx(0 hunks)src/context/RequestFulfillmentFlowContext.tsx(0 hunks)
💤 Files with no reviewable changes (4)
- src/context/RequestFulfillmentFlowContext.tsx
- src/components/Request/views/ExternalWalletFulfilManager.tsx
- src/app/[...recipient]/client.tsx
- src/components/Request/views/ExternalWalletFulfilMethods.tsx
🧰 Additional context used
🧠 Learnings (26)
📓 Common learnings
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 919
File: src/components/Withdraw/views/Initial.withdraw.view.tsx:87-87
Timestamp: 2025-06-18T19:56:55.443Z
Learning: In withdraw flows for Peanut Wallet, the PeanutActionDetailsCard should always display "USDC" as the token symbol because it shows the amount being withdrawn from the Peanut Wallet (which holds USDC), regardless of the destination token/chain selected by the user. The TokenSelector is used for choosing the withdrawal destination, not the source display.
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Pay.consts.ts:34-34
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In `src/components/Request/Pay` components, the `tokenPrice` property in the `IPayScreenProps` interface is only relevant to these views. Other components using `IPayScreenProps` do not need to handle `tokenPriceData` when it's updated in these components.
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Pay.consts.ts:34-34
Timestamp: 2024-10-07T15:50:29.173Z
Learning: In `src/components/Request/Pay` components, the `tokenPrice` property in the `IPayScreenProps` interface is only relevant to these views. Other components using `IPayScreenProps` do not need to handle `tokenPriceData` when it's updated in these components.
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1104
File: src/components/Payment/PaymentForm/index.tsx:522-545
Timestamp: 2025-08-22T07:28:32.281Z
Learning: In `src/components/Payment/PaymentForm/index.tsx`, the `handleCompleteDaimoPayment` function is only for updating payment status in the backend after a successful Daimo payment. Payment success/failure is handled by Daimo itself, so try/catch error handling and error display are not needed for backend sync failures - users shouldn't see errors if payment succeeded but database update failed.
📚 Learning: 2025-08-26T15:25:53.328Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1132
File: src/app/[...recipient]/client.tsx:394-397
Timestamp: 2025-08-26T15:25:53.328Z
Learning: In `src/components/Common/ActionListDaimoPayButton.tsx`, the `handleCompleteDaimoPayment` function should not display error messages to users when DB update fails because the Daimo payment itself has succeeded - showing errors would be confusing since the payment was successful.
Applied to files:
src/components/Common/ActionListDaimoPayButton.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2025-08-22T07:28:32.281Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1104
File: src/components/Payment/PaymentForm/index.tsx:522-545
Timestamp: 2025-08-22T07:28:32.281Z
Learning: In `src/components/Payment/PaymentForm/index.tsx`, the `handleCompleteDaimoPayment` function is only for updating payment status in the backend after a successful Daimo payment. Payment success/failure is handled by Daimo itself, so try/catch error handling and error display are not needed for backend sync failures - users shouldn't see errors if payment succeeded but database update failed.
Applied to files:
src/components/Common/ActionListDaimoPayButton.tsxsrc/components/Global/DaimoPayButton/index.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2025-08-26T17:38:37.055Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1132
File: src/components/Common/ActionList.tsx:153-156
Timestamp: 2025-08-26T17:38:37.055Z
Learning: In ActionList.tsx, when there are circular dependency concerns with ACTION_METHODS being imported by other components, the preferred solution is to move ACTION_METHODS to a separate constants file (like src/constants/actionlist.consts.ts) rather than using prop drilling. This centralizes constants management and creates a cleaner dependency graph.
Applied to files:
src/components/Common/ActionListDaimoPayButton.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Pay.consts.ts:34-34
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In `src/components/Request/Pay` components, the `tokenPrice` property in the `IPayScreenProps` interface is only relevant to these views. Other components using `IPayScreenProps` do not need to handle `tokenPriceData` when it's updated in these components.
Applied to files:
src/components/Common/ActionListDaimoPayButton.tsxsrc/components/Global/DaimoPayButton/index.tsxsrc/components/Payment/PaymentForm/index.tsx
📚 Learning: 2024-10-07T13:42:00.443Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Pay.tsx:103-111
Timestamp: 2024-10-07T13:42:00.443Z
Learning: When the token price cannot be fetched in `src/components/Request/Pay/Pay.tsx` within the `PayRequestLink` component, set `tokenPriceData.price` to 0 to ensure the UI remains functional. Since Squid uses their own price engine for x-chain fulfillment transactions, this approach will not affect the transaction computation.
Applied to files:
src/components/Common/ActionListDaimoPayButton.tsxsrc/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-10-24T13:44:39.473Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1332
File: src/components/Global/TokenAmountInput/index.tsx:141-150
Timestamp: 2025-10-24T13:44:39.473Z
Learning: In the `TokenAmountInput` component (`src/components/Global/TokenAmountInput/index.tsx`), the slider feature (controlled by `showSlider` prop) is only shown for USD input mode. When the slider is used with `maxAmount`, the `selectedAmount` is computed in USD and `isInputUsd` is always `true`, so the conversion in `onChange` handles it correctly.
Applied to files:
src/components/Common/ActionListDaimoPayButton.tsxsrc/components/Payment/PaymentForm/index.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 551
File: src/components/Request/Create/Views/Initial.view.tsx:151-156
Timestamp: 2024-12-02T17:19:18.532Z
Learning: In the `InitialView` component at `src/components/Request/Create/Views/Initial.view.tsx`, when setting the default chain and token in the `useEffect` triggered by `isPeanutWallet`, it's acceptable to omit the setters from the dependency array and not include additional error handling for invalid defaults.
Applied to files:
src/components/Global/DaimoPayButton/index.tsxsrc/components/Payment/PaymentForm/index.tsx
📚 Learning: 2024-10-29T12:19:41.968Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 495
File: src/components/Global/TokenAmountInput/index.tsx:23-30
Timestamp: 2024-10-29T12:19:41.968Z
Learning: In the `TokenAmountInput` component (`src/components/Global/TokenAmountInput/index.tsx`), when the 'Max' button is clicked, we intentionally set the input denomination to 'TOKEN' because we are setting the value as token.
Applied to files:
src/components/Global/DaimoPayButton/index.tsxsrc/components/Payment/PaymentForm/index.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2024-10-11T01:14:15.489Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 424
File: src/components/Global/TokenSelector/TokenSelector.tsx:197-211
Timestamp: 2024-10-11T01:14:15.489Z
Learning: In `src/components/Global/TokenSelector/TokenSelector.tsx`, when the calculation within functions like `byChainAndText` is not computationally expensive, it's acceptable to avoid using `useCallback` for memoization.
Applied to files:
src/components/Global/DaimoPayButton/index.tsxsrc/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-06-18T19:56:55.443Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 919
File: src/components/Withdraw/views/Initial.withdraw.view.tsx:87-87
Timestamp: 2025-06-18T19:56:55.443Z
Learning: In withdraw flows for Peanut Wallet, the PeanutActionDetailsCard should always display "USDC" as the token symbol because it shows the amount being withdrawn from the Peanut Wallet (which holds USDC), regardless of the destination token/chain selected by the user. The TokenSelector is used for choosing the withdrawal destination, not the source display.
Applied to files:
src/components/Global/DaimoPayButton/index.tsxsrc/components/Payment/PaymentForm/index.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 413
File: src/context/tokenSelector.context.tsx:118-123
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In the `TokenContextProvider` component within `src/context/tokenSelector.context.tsx`, in the TypeScript React application, when data changes and before calling `fetchAndSetTokenPrice`, it is necessary to reset `selectedTokenData`, `selectedTokenPrice`, `selectedTokenDecimals`, and `inputDenomination` to discard stale data.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2024-10-04T13:10:49.199Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 413
File: src/components/Request/Pay/Views/Initial.view.tsx:71-72
Timestamp: 2024-10-04T13:10:49.199Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, it's acceptable to use the `!` operator in TypeScript to assert that `selectedTokenData` is not `null` or `undefined`, and potential runtime errors from accessing its properties without checks can be disregarded.
Applied to files:
src/components/Payment/PaymentForm/index.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2024-10-07T15:25:45.170Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-10-07T15:25:45.170Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-01-16T13:14:40.363Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 631
File: src/components/Create/Create.tsx:108-112
Timestamp: 2025-01-16T13:14:40.363Z
Learning: In the Peanut UI codebase, the `resetTokenContextProvider` function from `tokenSelectorContext` is a stable function reference that doesn't change, so it doesn't need to be included in useEffect dependencies.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2024-10-03T09:57:43.885Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 404
File: src/context/tokenSelector.context.tsx:121-121
Timestamp: 2024-10-03T09:57:43.885Z
Learning: In `TokenContextProvider` within `tokenSelector.context.tsx`, when token data is loaded from preferences, it's acceptable to set `isTokenPriceFetchingComplete` to `true` because the token data is already available.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-09-18T09:30:42.901Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1230
File: src/app/(mobile-ui)/withdraw/page.tsx:92-97
Timestamp: 2025-09-18T09:30:42.901Z
Learning: In src/app/(mobile-ui)/withdraw/page.tsx, the useEffect that calls setShowAllWithdrawMethods(true) when amountFromContext exists is intentionally designed to run only on component mount (empty dependency array), not when amountFromContext changes. This is the correct behavior for the withdraw flow where showing all methods should only happen on initial load when an amount is already present.
Applied to files:
src/components/Payment/PaymentForm/index.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-05-23T19:26:58.220Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 873
File: src/components/Withdraw/views/Initial.withdraw.view.tsx:95-95
Timestamp: 2025-05-23T19:26:58.220Z
Learning: The GeneralRecipientInput component supports username validation and resolution through the validateAndResolveRecipient function in src/lib/validation/recipient.ts. The function automatically detects usernames (inputs that don't contain '.' for ENS and don't start with '0x' for addresses), validates them via API HEAD request, fetches user data, and resolves them to Ethereum addresses from the user's PEANUT_WALLET account.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-10-29T11:27:59.248Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1368
File: src/components/Common/ActionList.tsx:109-111
Timestamp: 2025-10-29T11:27:59.248Z
Learning: In `src/components/Common/ActionList.tsx`, the `balance` from `useWallet()` hook is always in USDC (as a formatted string), making it directly comparable to USD amounts without conversion. The comparison `Number(balance) >= amountInUsd` is intentional and correct.
Applied to files:
src/components/Payment/PaymentForm/index.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2025-09-16T17:27:39.840Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1201
File: src/components/Payment/Views/MantecaFulfillment.view.tsx:56-56
Timestamp: 2025-09-16T17:27:39.840Z
Learning: In the TRequestResponse interface (src/services/services.types.ts), recipientAccount is a required field, not optional. When requestDetails is not null, recipientAccount and its nested user.username properties are guaranteed to exist. Only requestDetails itself can be null (typed as TRequestResponse | null).
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-07-07T20:22:11.092Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 958
File: src/app/actions/tokens.ts:266-266
Timestamp: 2025-07-07T20:22:11.092Z
Learning: In `src/app/actions/tokens.ts`, within the `fetchWalletBalances` function, using the non-null assertion operator `!` on `process.env.MOBULA_API_KEY!` is intentional and correct, and should not be flagged for replacement with explicit validation.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-05-19T19:40:43.138Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 868
File: src/components/Payment/PaymentForm/index.tsx:284-293
Timestamp: 2025-05-19T19:40:43.138Z
Learning: When converting between USD and token amounts, always check if the token price (divisor) is valid and non-zero before performing the division to prevent Infinity, NaN, or errors. Implementing validation like `if (!tokenPrice || isNaN(tokenPrice) || tokenPrice === 0)` before division operations is crucial for handling cases where price data might be unavailable.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-09-05T07:31:11.396Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1185
File: src/components/Claim/useClaimLink.tsx:14-0
Timestamp: 2025-09-05T07:31:11.396Z
Learning: In the peanut-ui codebase, `window.history.replaceState` is preferred over `router.replace` when immediate/synchronous URL parameter updates are required, as `router.replace` is asynchronous and doesn't guarantee instant URL changes that subsequent code can rely on. This pattern is used consistently across usePaymentInitiator.ts, Confirm.payment.view.tsx, and useClaimLink.tsx.
Applied to files:
src/components/Payment/PaymentForm/index.tsxsrc/components/Common/ActionList.tsx
📚 Learning: 2024-12-11T10:13:22.806Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 564
File: src/components/Request/Pay/Views/Initial.view.tsx:430-430
Timestamp: 2024-12-11T10:13:22.806Z
Learning: In the React TypeScript file `src/components/Request/Pay/Views/Initial.view.tsx`, when reviewing the `InitialView` component, do not flag potential issues with using non-null assertion `!` on the `slippagePercentage` variable, as handling undefined values in this context is considered out of scope.
Applied to files:
src/components/Payment/PaymentForm/index.tsx
📚 Learning: 2025-11-04T17:47:06.328Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1396
File: src/app/(mobile-ui)/home/page.tsx:295-304
Timestamp: 2025-11-04T17:47:06.328Z
Learning: In src/app/(mobile-ui)/home/page.tsx, when closing the KycCompletedModal, updateUserById is called without awaiting to provide instant feedback to the user. This fire-and-forget pattern for modal dismissals and UI preference updates is intentional and consistent across the codebase—user experience with instant UI feedback takes priority over waiting for backend sync operations.
Applied to files:
src/components/Common/ActionList.tsx
🧬 Code graph analysis (2)
src/components/Global/DaimoPayButton/index.tsx (1)
src/constants/zerodev.consts.ts (1)
PEANUT_WALLET_TOKEN(20-20)
src/components/Payment/PaymentForm/index.tsx (3)
src/context/RequestFulfillmentFlowContext.tsx (1)
useRequestFulfillmentFlow(104-110)src/constants/zerodev.consts.ts (2)
PEANUT_WALLET_CHAIN(18-18)PEANUT_WALLET_TOKEN(20-20)src/utils/general.utils.ts (1)
areEvmAddressesEqual(423-430)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (9)
src/components/Global/DaimoPayButton/index.tsx (2)
16-19: LGTM!The new optional props are well-documented and appropriately typed for enabling cross-chain payment customization.
148-151: The review comment is incorrect—defensive validation is already implemented.The code at line 151 already includes defensive handling:
toToken={getAddress(toTokenAddress ?? PEANUT_WALLET_TOKEN)}. IftoTokenAddressis undefined or null, the expression safely falls back toPEANUT_WALLET_TOKEN, preventing an invalid address from being passed togetAddress(). While viem'sgetAddressdoes throw when address is invalid, the??operator mitigates this by ensuring a fallback constant is used when the primary value is falsy. No additional validation or error handling is necessary.Likely an incorrect or invalid review comment.
src/components/Common/ActionListDaimoPayButton.tsx (3)
20-20: LGTM!The new
clickHandlerRefprop enables external control of the Daimo button click handler, which aligns with the PR's integration pattern for triggering payments after state transitions.
156-157: LGTM!The props are correctly passed through with appropriate null/undefined handling using optional chaining and conditional logic.
185-191: LGTM!The ref assignment pattern correctly stores the onClick handler in both the internal ref and the parent-provided ref (when available), enabling external control of the Daimo button.
src/components/Common/ActionList.tsx (4)
111-112: LGTM!The ref is properly typed and clearly documented for storing the Daimo button click handler for external triggering from the balance modal.
259-259: LGTM!The removal of the 'exchange-or-wallet' case from the switch statement is appropriate since this method is now exclusively handled by the
ActionListDaimoPayButtoncomponent (lines 315-335).
331-331: LGTM!The
clickHandlerRefis correctly passed to enable external triggering of the Daimo button from the parent component.
430-439: Verify the setTimeout pattern handles all edge cases.The
setTimeout(fn, 0)pattern defers execution until after state updates complete, which is appropriate here. The optional chaining on line 434 safely handles the case where the ref might be null.However, consider if there are edge cases where the ref might not be populated yet or where state updates take longer than expected. The pattern should work for the typical event loop timing, but it's worth testing thoroughly.
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: 111081 🏁 Script executed: Length of output: 346 🏁 Script executed: Length of output: 4533 🏁 Script executed: Length of output: 755 🏁 Script executed: Length of output: 279 🏁 Script executed: Length of output: 10045 🏁 Script executed: Length of output: 7047 🏁 Script executed: Length of output: 1385 🏁 Script executed: Length of output: 2987 🏁 Script executed: Length of output: 4313 @kushagrasarathe, I've completed an in-depth review of this PR. Here's my analysis: 🚨 Critical Issues1. Chain ID Fallback Logic Bug (High Priority)Location: destinationchainId: Number(chargeDetails.chainId) ?? Number(daimoPaymentResponse.payment.destination.chainId)Problem: The nullish coalescing operator ( Fix: destinationchainId: chargeDetails.chainId
? Number(chargeDetails.chainId)
: Number(daimoPaymentResponse.payment.destination.chainId)2. Race Condition with setTimeout (High Priority)Location: setTimeout(() => {
daimoButtonClickRef.current?.()
}, 0)Problem: Using Recommendation: Use React's state update callbacks or // Instead of setTimeout, trigger from useEffect
useEffect(() => {
if (shouldTriggerDaimo && daimoButtonClickRef.current) {
daimoButtonClickRef.current()
setShouldTriggerDaimo(false)
}
}, [shouldTriggerDaimo])3. Missing Null Safety in Division Operations (Medium Priority)Location: if (isNaN(requestedTokenPriceData.price) || requestedTokenPriceData.price === 0) {
// error handling
}
const usdAmount = parseFloat(inputUsdValue)
tokenAmount = (usdAmount / requestedTokenPriceData.price).toString()Problem: While there's validation, the division still happens outside the validated scope. If Fix: Add explicit null checks before any property access.
|
kushagrasarathe
commented
Nov 12, 2025
| // for exchange-or-wallet, trigger daimo button after state updates | ||
| if (selectedPaymentMethod.id === 'exchange-or-wallet' && daimoButtonClickRef.current) { | ||
| // use setTimeout to ensure state updates are processed before triggering daimo | ||
| setTimeout(() => { |
Contributor
Author
There was a problem hiding this comment.
not using a useeffect here cuz it will just add unnecessary complexity, setimeout a bit hacky but it makes sure state is updated properly
Contributor
|
@coderabbitai do a thorough review and risk analysis. What bugs could this introduce in existing payment flows? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 13592 🏁 Script executed: Length of output: 8525 🏁 Script executed: Length of output: 3746 🏁 Script executed: Length of output: 3049 🏁 Script executed: Length of output: 4089 🏁 Script executed: Length of output: 4612 🏁 Script executed: Length of output: 1716 @Hugo0, I've conducted a thorough risk analysis. Here are the specific bugs this could introduce in existing payment flows: 🚨 Critical Risk: Existing Payment Flows1. Bank/Pix/Mercado Pago Broken for External Recipients (HIGH SEVERITY)Affected Flow: Request payment to ADDRESS/ENS with bank transfer Bug Scenario:
Root Cause: Token selector is shown for external recipients, but bank/pix/mercado methods don't support cross-chain. No validation prevents incompatible chain selection. Evidence from code: // PaymentForm line 842: Token selector rendered for external recipients
(recipient?.recipientType === 'ADDRESS' || recipient?.recipientType === 'ENS') && (
<TokenSelector viewType="payment" />
)
// But bank methods are still available in ActionList
case 'bank': // No check that selected chain is compatible!Who's affected: Any payer trying to use bank/pix/mercado for ADDRESS/ENS recipients. 2. Balance Modal -> Daimo Flow Broken (HIGH SEVERITY)Affected Flow: Request payment with Peanut balance modal Bug Scenario:
Root Cause: Line 432-435 in ActionList.tsx relies on ref being set, but doesn't handle case where ref is null: if (
selectedPaymentMethod?.id === 'exchange-or-wallet' &&
daimoButtonClickRef.current
) {
setTimeout(() => {
daimoButtonClickRef.current?.() // What if this is null?
}, 0)
}Additional Risk: The 'exchange-or-wallet' method was removed from Who's affected: Users with Peanut wallet balance trying to pay via Daimo. 3. Request Pot + External Recipients = Conflict (MEDIUM SEVERITY)Affected Flow: Request pot to ADDRESS recipient Bug Scenario:
Root Cause: Token selector logic doesn't check if it's a request pot scenario: // Line 181-206: External recipient gets token selector
if (isExternalRecipient && !selectedChainID) {
setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString())
}
// But no check: if (isRequestPotLink) { /* don't allow token changes */ }Who's affected: Request pot creators using ADDRESS/ENS recipients. 4. Username Payments Get Token Selector (Logic Leak) (MEDIUM SEVERITY)Affected Flow: Regular username payment Bug Scenario:
Root Cause: Token selector rendering has duplicate checks (line 842): (recipient?.recipientType === 'ADDRESS' || recipient?.recipientType === 'ENS')But if Who's affected: All username payments if recipient parsing fails. 5. Invite Flow Breaks for External Recipients (MEDIUM SEVERITY)Affected Flow: Guest user paying ADDRESS request from inviter Bug Scenario:
Root Cause: Invite flow doesn't preserve token/chain selection through redirect: // ActionList.tsx line 383-389: After invite modal
handleMethodClick(selectedMethod) // Re-triggers method
// But token selector state might have changed during invite flowWho's affected: Guests accepting invites for ADDRESS/ENS payments. 6. Bank Fulfillment Incomplete (CRITICAL - Per TODO)Affected Flow: Request fulfillment via bank transfer Evidence: Line 214 in ActionList.tsx: // @dev TODO: Fix req fulfillment with bank properly post devconnect
// if (method.id === 'bank') {Risk: The bank fulfillment flow for requests is commented out and marked TODO. This PR doesn't address it. If users try bank fulfillment for requests, behavior is undefined:
Who's affected: Anyone using bank transfer to fulfill a request (not claim). 7. Daimo Chain/Token No Validation (MEDIUM SEVERITY)Affected Flow: Cross-chain Daimo payment Bug Scenario:
Root Cause: No validation that selected chain/token are supported by Daimo: // ActionListDaimoPayButton.tsx line 181-190
<DaimoPayButton
toChainId={parsedPaymentData.chain.chainId} // No validation!
toTokenAddress={parsedPaymentData.token.address} // No validation!
/>Who's affected: Users selecting non-Daimo-supported chains. 🔴 State Synchronization Risks8. Race Condition: Balance Check vs Token Selection (MEDIUM SEVERITY)Bug Scenario:
Root Cause: No debouncing on balance validation, fires on every keystroke/change: // Line 223: Effect runs on ANY dependency change
useEffect(() => {
// Complex validation...
dispatch(paymentActions.setError(...))
}, [
selectedTokenBalance, // Changes on every token switch
inputTokenAmount, // Changes on every keystroke
// ...18 more dependencies
])9. setTimeout Race in Balance Modal (Already Identified, Expanded)Additional Risk Beyond Previous Review:
Then Daimo modal opens with:
Evidence: Line 432-435: setTimeout(() => {
daimoButtonClickRef.current?.() // Assumes state is ready
}, 0)No check that 📊 Risk Matrix by Flow Type
🧪 Missing Test CoverageCritical Gap: Zero integration tests found for:
Risk: All bugs above will only be caught in production. ✅ Recommended Pre-Merge ActionsMust Fix Before Merge:
if (isExternalRecipient && selectedChainID !== PEANUT_WALLET_CHAIN.id) {
// Hide bank/pix/mercado from action list
}
if (daimoButtonClickRef.current) {
daimoButtonClickRef.current()
} else {
dispatch(paymentActions.setError('Payment method unavailable'))
setShowUsePeanutBalanceModal(false)
}
destinationchainId: chargeDetails.chainId
? Number(chargeDetails.chainId)
: Number(daimoPaymentResponse.payment.destination.chainId)
if (isRequestPotLink) {
// Don't render token selector
}
Should Add Soon:
🎯 Testing Priority (For QA)Test these flows FIRST (highest risk):
Automated test scenarios (should write): describe('PaymentForm - External Recipients', () => {
it('should hide bank methods when non-default chain selected', () => {
// Render with ADDRESS recipient
// Change chain to Base
// Assert bank/pix/mercado not in action list
})
it('should preserve token selection through invite flow', () => {
// Select Polygon + USDC
// Trigger invite modal
// Complete invite
// Assert chain/token still Polygon/USDC
})
})Bottom Line: This PR introduces 7 high-severity bugs in existing flows due to insufficient validation, missing null checks, and complex state interactions. The lack of tests means these will likely surface in production. Do not merge until at least the 5 "Must Fix" items are addressed. Would you like me to create follow-up issues for the medium-priority items, or should we block this PR until all critical issues are resolved? 🧠 Learnings used |
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
3 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.