Conversation
Separated payment flow hooks that simplify the payment flows and handle all the necessary logic.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughReplaces legacy, multi-view payment UIs with new orchestrated flow components/hooks (DirectSendFlow, CryptoWithdrawFlow, RequestPayFlow), adds a Direct Send client page, centralizes flow types/hooks/exports, integrates TanStack Query-based route preparation, and updates page routing to use the new flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (1)
src/components/Payment/flows/views/CryptoWithdrawInitial.tsx (1)
17-23: Consider centralizing form data types.As noted in other components, this form data interface is likely duplicated. Consider using shared types from the central types file.
🧹 Nitpick comments (19)
src/components/Payment/flows/views/DirectSendInitial.tsx (1)
89-91: Consider adding loading state for wallet balanceThe wallet balance is displayed immediately but may not be fetched yet, potentially showing "0" initially which could confuse users.
Consider using the
isFetchingBalancestate fromuseWalletto show a loading indicator:+ const { balance, isFetchingBalance } = useWallet() // In the TokenAmountInput: - walletBalance={formatAmount(formatUnits(balance, PEANUT_WALLET_TOKEN_DECIMALS))} + walletBalance={isFetchingBalance ? 'Loading...' : formatAmount(formatUnits(balance, PEANUT_WALLET_TOKEN_DECIMALS))}src/components/Payment/flows/views/CryptoWithdrawStatus.tsx (1)
9-15: Consider centralizing form data types.The
CryptoWithdrawFormDatainterface is defined locally but likely duplicates types used in other flow components. Consider moving shared types to a central location.Based on the relevant code snippets, there might be shared types in
src/hooks/payment/types.ts. Consider importing from there:-interface CryptoWithdrawFormData { - amount: string - selectedToken: ITokenPriceData | null - selectedChain: (peanutInterfaces.ISquidChain & { tokens: peanutInterfaces.ISquidToken[] }) | null - recipientAddress: string - isValidRecipient: boolean -} +import { WithdrawPayload } from '@/hooks/payment/types' + +// Extend the base payload if needed for UI-specific fields +interface CryptoWithdrawFormData extends Omit<WithdrawPayload, 'recipient'> { + selectedToken: ITokenPriceData | null + selectedChain: (peanutInterfaces.ISquidChain & { tokens: peanutInterfaces.ISquidToken[] }) | null + recipientAddress: string + isValidRecipient: boolean +}src/components/Payment/flows/views/DirectSendConfirm.tsx (1)
13-17: Consider centralizing form data types.Similar to the CryptoWithdrawStatus component, this form data interface is likely duplicated across flow components.
Import shared types from the central types file:
+import { DirectSendPayload } from '@/hooks/payment/types' + -interface DirectSendFormData { - amount: string - message: string - recipient: ParsedURL['recipient'] | null -} +// Extend base payload for UI-specific needs +interface DirectSendFormData extends Omit<DirectSendPayload, 'recipient' | 'tokenAmount'> { + amount: string + message: string + recipient: ParsedURL['recipient'] | null +}src/components/Payment/FlowSelector.tsx (1)
11-16: Consider memoizing hook calls to prevent unnecessary re-renders.All four hooks are called unconditionally on every render, even when only one is needed. This could cause performance issues.
Consider using a factory pattern or lazy initialization:
export const usePaymentFlow = (flowType: PaymentFlowType, chargeId?: string) => { - const directSendFlow = useDirectSendFlow() - const addMoneyFlow = useAddMoneyFlow() - const withdrawFlow = useCryptoWithdrawFlow() - const requestPayFlow = useRequestPayFlow(chargeId) - + const getFlow = useMemo(() => { switch (flowType) { case 'direct_send': + const directSendFlow = useDirectSendFlow() return { ...directSendFlow, execute: directSendFlow.sendDirectly, type: 'direct_send' as const, } case 'add_money': + const addMoneyFlow = useAddMoneyFlow() return { ...addMoneyFlow, execute: addMoneyFlow.addMoney, type: 'add_money' as const, } case 'withdraw': + const withdrawFlow = useCryptoWithdrawFlow() return { ...withdrawFlow, execute: withdrawFlow.withdraw, type: 'withdraw' as const, } case 'request_pay': + const requestPayFlow = useRequestPayFlow(chargeId) return { ...requestPayFlow, execute: requestPayFlow.payRequest, type: 'request_pay' as const, } default: throw new Error(`Unknown flow type: ${flowType}`) } + }, [flowType, chargeId]) + + return getFlow }However, note that this approach violates the Rules of Hooks, so an alternative approach would be to create separate hook functions for each flow type.
src/hooks/payment/types.ts (1)
17-21: Consider making recipientType required for better type safety.The
recipientTypefield is optional but appears to be used consistently across the codebase. Making it required would improve type safety.export interface PaymentRecipient { identifier: string resolvedAddress: string - recipientType?: 'ADDRESS' | 'ENS' | 'USERNAME' + recipientType: 'ADDRESS' | 'ENS' | 'USERNAME' }src/app/(mobile-ui)/withdraw/crypto/page.tsx (1)
13-20: Consider improving error handling and navigation consistencyThe error is logged when
amountToWithdrawis missing but the component continues to render briefly before redirecting. This could cause a flash of loading state.Consider handling the redirect more immediately:
// Redirect if no amount is set useEffect(() => { if (!amountToWithdraw) { console.error('Amount not available for crypto withdrawal, redirecting.') router.push('/withdraw') - return } }, [amountToWithdraw, router]) + +// Return early if no amount (avoid flash of loading) +if (!amountToWithdraw) { + router.push('/withdraw') + return null +}Or even better, handle this check before mounting:
- // Redirect if no amount is set - useEffect(() => { - if (!amountToWithdraw) { - console.error('Amount not available for crypto withdrawal, redirecting.') - router.push('/withdraw') - return - } - }, [amountToWithdraw, router]) - - const handleComplete = () => { - // Clean up the context state and go home - resetWithdrawFlow() - router.push('/home') - } - - if (!amountToWithdraw) { - return <PeanutLoading /> - } + const handleComplete = () => { + // Clean up the context state and go home + resetWithdrawFlow() + router.push('/home') + } + + // Redirect if no amount is set + useEffect(() => { + if (!amountToWithdraw) { + console.error('Amount not available for crypto withdrawal, redirecting.') + router.push('/withdraw') + } + }, [amountToWithdraw, router]) + + if (!amountToWithdraw) { + return <PeanutLoading /> + }src/components/Payment/flows/DirectSendFlow.tsx (1)
55-81: Verify recipient resolution before accessing propertiesThe code accesses
formData.recipient!.identifierwith a non-null assertion operator after validatingformData.recipient?.resolvedAddress. While the validation checks for the presence ofresolvedAddress, it's safer to avoid the non-null assertion.Replace non-null assertions with safer access:
// Execute the direct send transaction immediately (no confirmation needed) const result = await directSendHook.sendDirectly({ recipient: { - identifier: formData.recipient!.identifier, - resolvedAddress: formData.recipient!.resolvedAddress, + identifier: formData.recipient.identifier, + resolvedAddress: formData.recipient.resolvedAddress, }, tokenAmount: formData.amount, attachmentOptions: formData.message ? { message: formData.message } : undefined, })src/hooks/payment/useAddMoneyFlow.ts (1)
163-182: Consider adding transaction status feedbackThe transaction execution loop doesn't provide user feedback between multiple transactions. For cross-chain operations with multiple steps, users might be confused about progress.
Consider exposing transaction progress state:
+const [currentTransactionIndex, setCurrentTransactionIndex] = useState(0) +const [totalTransactions, setTotalTransactions] = useState(0) // 4. Execute transactions console.log(`💸 Executing ${transactions.length} transaction(s)...`) +setTotalTransactions(transactions.length) let finalReceipt: TransactionReceipt | null = null for (let i = 0; i < transactions.length; i++) { const tx = transactions[i] console.log(`📤 Sending transaction ${i + 1}/${transactions.length}`) + setCurrentTransactionIndex(i + 1) const hash = await sendTransactionAsync({ to: tx.to, data: tx.data as `0x${string}`, value: tx.value, chainId: sourceChainId, }) const receipt = await waitForTransactionReceipt(config, { hash, chainId: sourceChainId, confirmations: 1, }) finalReceipt = receipt console.log(`✅ Transaction ${i + 1} confirmed:`, hash) }Then expose these states in the return object for UI components to display progress.
src/components/Payment/flows/views/DirectSendStatus.tsx (2)
28-28: Remove unused prop from interfaceThe
onSendAnotherActionprop is defined in the interface but never used in the component implementation.Remove the unused prop:
interface DirectSendStatusProps { formData: DirectSendFormData directSendHook: ReturnType<typeof useDirectSendFlow> onCompleteAction: () => void - onSendAnotherAction: () => void }
36-36: Update function signature to match interfaceThe component destructures only three props but the interface defines four. Also, the unused
onSendAnotherActionis passed from the parent component.Either use the prop or remove it from both the interface and parent component call:
-export const DirectSendStatus = ({ formData, directSendHook, onCompleteAction }: DirectSendStatusProps) => { +export const DirectSendStatus = ({ formData, directSendHook, onCompleteAction, onSendAnotherAction }: DirectSendStatusProps) => {Or if not needed, update the parent component to not pass it (Line 121 in DirectSendFlow.tsx).
src/app/send/[...username]/client.tsx (1)
47-67: Remove console.log statements from production codeThe console.log statements on lines 47, 63-66 should be removed or wrapped in a development-only condition for production builds.
} else { // It's a username - resolve it to an address - console.log('🔍 Resolving username:', recipientIdentifier) + if (process.env.NODE_ENV === 'development') { + console.log('🔍 Resolving username:', recipientIdentifier) + } const user = await usersApi.getByUsername(recipientIdentifier) // Find the Peanut wallet account (should be the primary one) const peanutAccount = user.accounts.find((account) => account.type === AccountType.PEANUT_WALLET) if (!peanutAccount) { throw new Error(`User ${recipientIdentifier} does not have a Peanut wallet`) } resolvedRecipient = { identifier: recipientIdentifier, resolvedAddress: peanutAccount.identifier, // This should be the wallet address recipientType: 'USERNAME', } - console.log('✅ Username resolved:', { - username: recipientIdentifier, - address: peanutAccount.identifier, - }) + if (process.env.NODE_ENV === 'development') { + console.log('✅ Username resolved:', { + username: recipientIdentifier, + address: peanutAccount.identifier, + }) + } }src/components/Payment/flows/CryptoWithdrawFlow.tsx (1)
79-106: Remove console.log statements from production codeMultiple console.log statements are present that should be removed or wrapped in development-only conditions for production builds.
const handleNext = useCallback(async () => { if (currentView === 'INITIAL') { // Validate form if (!currentPayload) { - console.error('Form validation failed - missing required fields') + if (process.env.NODE_ENV === 'development') { + console.error('Form validation failed - missing required fields') + } return } // Prepare route (synchronous now with TanStack Query!) - console.log('Preparing route for withdrawal...') + if (process.env.NODE_ENV === 'development') { + console.log('Preparing route for withdrawal...') + } cryptoWithdrawHook.prepareRoute(currentPayload) // Move to confirm immediately - route preparation happens in background - console.log('Moving to confirm view, route will prepare automatically') + if (process.env.NODE_ENV === 'development') { + console.log('Moving to confirm view, route will prepare automatically') + } setCurrentView('CONFIRM') } else if (currentView === 'CONFIRM') { // Execute the crypto withdraw transaction with cached route - console.log('Executing withdrawal with cached route...') + if (process.env.NODE_ENV === 'development') { + console.log('Executing withdrawal with cached route...') + } const result = await cryptoWithdrawHook.withdraw() if (result.success) { - console.log('Withdrawal successful, moving to status view') + if (process.env.NODE_ENV === 'development') { + console.log('Withdrawal successful, moving to status view') + } setCurrentView('STATUS') } else { - console.error('Withdrawal failed:', result.error) + if (process.env.NODE_ENV === 'development') { + console.error('Withdrawal failed:', result.error) + } // Error will be shown by the hook } } }, [currentView, currentPayload, cryptoWithdrawHook])src/components/Payment/flows/views/RequestPayConfirm.tsx (1)
253-301: Extract TokenChainInfoDisplay to a shared componentThe
TokenChainInfoDisplaycomponent appears to be a reusable UI component that could be used across different payment flows. Consider extracting it to a shared components directory for better reusability.Consider moving this component to a shared location like
src/components/Global/TokenChainInfoDisplay.tsx:// src/components/Global/TokenChainInfoDisplay.tsx export interface TokenChainInfoDisplayProps { tokenIconUrl?: string chainIconUrl?: string resolvedTokenSymbol?: string fallbackTokenSymbol: string resolvedChainName?: string fallbackChainName: string } export function TokenChainInfoDisplay({ ... }: TokenChainInfoDisplayProps) { // existing implementation }Then import it in this file:
+import { TokenChainInfoDisplay } from '@/components/Global/TokenChainInfoDisplay' - -interface TokenChainInfoDisplayProps { - // ... remove interface -} - -function TokenChainInfoDisplay({ ... }: TokenChainInfoDisplayProps) { - // ... remove implementation -}src/components/Payment/flows/RequestPayFlow.tsx (1)
127-127: Pass requestId as optional prop to allow null valuesThe
requestIdprop is passed directly fromsearchParams.get('id')which can return null, but the component expects it to be string or undefined.- requestId={requestId} + requestId={requestId || undefined}src/hooks/payment/useCryptoWithdrawFlow.ts (1)
133-156: Consider caching token details to avoid repeated lookupsThe
minReceivedcalculation fetches token details on every recalculation. Since token details rarely change, consider caching them to improve performance.You could memoize the token details lookup:
const minReceived = useMemo(() => { if (!xChainRoute || !currentPayload) return null - const tokenDetails = getTokenDetails({ - tokenAddress: currentPayload.toTokenAddress as Address, - chainId: currentPayload.toChainId, - }) + // Consider adding a separate useMemo for token details + // or using React Query to cache token details globally + const tokenDetails = getTokenDetails({ + tokenAddress: currentPayload.toTokenAddress as Address, + chainId: currentPayload.toChainId, + }) if (!tokenDetails) return null const minReceivedAmount = formatUnits( BigInt(xChainRoute.rawResponse.route.estimate.toAmountMin), tokenDetails.decimals ) return isStableCoin(tokenDetails.symbol) ? `$ ${formatAmount(minReceivedAmount)}` : `${formatAmount(minReceivedAmount)} ${tokenDetails.symbol}` }, [xChainRoute, currentPayload])src/hooks/payment/useRequestPayFlow.ts (4)
134-143: Inconsistent error message between createCharge implementationsThe error message says "Unable to get token details for payment" but this is in the charge creation function. Compare this with Line 211 in
useCryptoWithdrawFlow.tswhich uses "Unable to get token details for withdrawal".if (!tokenDetails) { - throw new Error('Unable to get token details for payment') + throw new Error('Unable to get token details for charge creation') }
268-269: Consider implementing same-chain external wallet paymentsThe function throws an error for same-chain external wallet payments, which seems like a missing feature that could impact users.
Would you like me to help implement the same-chain external wallet payment logic? This would involve:
- Detecting the token type (native vs ERC20)
- Building the appropriate transaction
- Sending via wagmi's sendTransactionAsync
I can open an issue to track this enhancement if needed.
322-328: Complex nested ternary operator reduces readabilityThe error state aggregation uses deeply nested ternary operators which makes it hard to read and maintain.
- error: paymentMutation.error - ? ErrorHandler(paymentMutation.error) - : chargeQuery.error - ? ErrorHandler(chargeQuery.error) - : routeQuery.error - ? ErrorHandler(routeQuery.error) - : null, + error: (() => { + if (paymentMutation.error) return ErrorHandler(paymentMutation.error) + if (chargeQuery.error) return ErrorHandler(chargeQuery.error) + if (routeQuery.error) return ErrorHandler(routeQuery.error) + return null + })(),Or even better, extract to a helper function:
+ const getDisplayError = useCallback(() => { + if (paymentMutation.error) return ErrorHandler(paymentMutation.error) + if (chargeQuery.error) return ErrorHandler(chargeQuery.error) + if (routeQuery.error) return ErrorHandler(routeQuery.error) + return null + }, [paymentMutation.error, chargeQuery.error, routeQuery.error]) + return { // ... other returns - error: paymentMutation.error - ? ErrorHandler(paymentMutation.error) - : chargeQuery.error - ? ErrorHandler(chargeQuery.error) - : routeQuery.error - ? ErrorHandler(routeQuery.error) - : null, + error: getDisplayError(),
97-98: Remove empty lines in query functionThere are unnecessary empty lines in the query function that don't add clarity.
// Only get route if cross-chain/cross-token is needed if (!isXChain && !isDiffToken) { return null } - - return await getRoute({
| onComplete={() => { | ||
| // Handle completion - could navigate or reset state | ||
| console.log('Request payment flow completed') | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement proper completion handler
The onComplete callback only logs to console. Consider implementing actual navigation or state cleanup logic.
onComplete={() => {
- // Handle completion - could navigate or reset state
- console.log('Request payment flow completed')
+ dispatch(paymentActions.resetPaymentState())
+ router.push('/home')
}}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/[...recipient]/client.tsx around lines 402 to 405, the onComplete
handler only logs to the console; replace that stub with real completion
behavior such as navigating to a confirmation page or resetting/closing the
payment UI and refreshing relevant state. Update the component to use the
Next.js router (useRouter) or a passed-in navigation callback to route to a
success/receipt page (e.g., router.push('/payment/success') with any needed
params), and/or call state setters to clear form data and close any modal and
trigger a data refresh (invalidate queries or call a prop refresh function).
Ensure imports and any required props/hooks are added and error handling is
applied for the navigation/cleanup steps.
| useEffect(() => { | ||
| const resolveRecipient = async () => { | ||
| try { | ||
| if (!recipient || recipient.length === 0) { | ||
| setError('No recipient specified') | ||
| return | ||
| } | ||
|
|
||
| const recipientIdentifier = recipient[0] | ||
|
|
||
| let resolvedRecipient: ParsedURL['recipient'] | ||
|
|
||
| if (isAddress(recipientIdentifier)) { | ||
| // It's already a valid address | ||
| resolvedRecipient = { | ||
| identifier: recipientIdentifier, | ||
| resolvedAddress: recipientIdentifier, | ||
| recipientType: 'ADDRESS', | ||
| } | ||
| } else { | ||
| // It's a username - resolve it to an address | ||
| console.log('🔍 Resolving username:', recipientIdentifier) | ||
| const user = await usersApi.getByUsername(recipientIdentifier) | ||
|
|
||
| // Find the Peanut wallet account (should be the primary one) | ||
| const peanutAccount = user.accounts.find((account) => account.type === AccountType.PEANUT_WALLET) | ||
|
|
||
| if (!peanutAccount) { | ||
| throw new Error(`User ${recipientIdentifier} does not have a Peanut wallet`) | ||
| } | ||
|
|
||
| resolvedRecipient = { | ||
| identifier: recipientIdentifier, | ||
| resolvedAddress: peanutAccount.identifier, // This should be the wallet address | ||
| recipientType: 'USERNAME', | ||
| } | ||
|
|
||
| console.log('✅ Username resolved:', { | ||
| username: recipientIdentifier, | ||
| address: peanutAccount.identifier, | ||
| }) | ||
| } | ||
|
|
||
| setParsedRecipient(resolvedRecipient) | ||
| } catch (err) { | ||
| console.error('Error resolving recipient:', err) | ||
| setError('Failed to resolve recipient') | ||
| } finally { | ||
| setIsLoading(false) | ||
| } | ||
| } | ||
|
|
||
| resolveRecipient() | ||
| }, [recipient]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding cleanup and error state handling
The useEffect hook doesn't handle cleanup for the async operation, which could lead to state updates after component unmount. Also, the error messages could be more specific to help users understand what went wrong.
Apply this diff to add proper cleanup and improve error handling:
useEffect(() => {
+ let isMounted = true
+
const resolveRecipient = async () => {
try {
if (!recipient || recipient.length === 0) {
setError('No recipient specified')
return
}
const recipientIdentifier = recipient[0]
let resolvedRecipient: ParsedURL['recipient']
if (isAddress(recipientIdentifier)) {
// It's already a valid address
resolvedRecipient = {
identifier: recipientIdentifier,
resolvedAddress: recipientIdentifier,
recipientType: 'ADDRESS',
}
} else {
// It's a username - resolve it to an address
console.log('🔍 Resolving username:', recipientIdentifier)
const user = await usersApi.getByUsername(recipientIdentifier)
// Find the Peanut wallet account (should be the primary one)
const peanutAccount = user.accounts.find((account) => account.type === AccountType.PEANUT_WALLET)
if (!peanutAccount) {
- throw new Error(`User ${recipientIdentifier} does not have a Peanut wallet`)
+ throw new Error(`User @${recipientIdentifier} does not have a Peanut wallet. Please ask them to create one.`)
}
resolvedRecipient = {
identifier: recipientIdentifier,
resolvedAddress: peanutAccount.identifier, // This should be the wallet address
recipientType: 'USERNAME',
}
console.log('✅ Username resolved:', {
username: recipientIdentifier,
address: peanutAccount.identifier,
})
}
- setParsedRecipient(resolvedRecipient)
+ if (isMounted) {
+ setParsedRecipient(resolvedRecipient)
+ }
} catch (err) {
console.error('Error resolving recipient:', err)
- setError('Failed to resolve recipient')
+ const errorMessage = err instanceof Error ? err.message : 'Failed to resolve recipient'
+ if (isMounted) {
+ setError(errorMessage)
+ }
} finally {
- setIsLoading(false)
+ if (isMounted) {
+ setIsLoading(false)
+ }
}
}
resolveRecipient()
+
+ return () => {
+ isMounted = false
+ }
}, [recipient])📝 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.
| useEffect(() => { | |
| const resolveRecipient = async () => { | |
| try { | |
| if (!recipient || recipient.length === 0) { | |
| setError('No recipient specified') | |
| return | |
| } | |
| const recipientIdentifier = recipient[0] | |
| let resolvedRecipient: ParsedURL['recipient'] | |
| if (isAddress(recipientIdentifier)) { | |
| // It's already a valid address | |
| resolvedRecipient = { | |
| identifier: recipientIdentifier, | |
| resolvedAddress: recipientIdentifier, | |
| recipientType: 'ADDRESS', | |
| } | |
| } else { | |
| // It's a username - resolve it to an address | |
| console.log('🔍 Resolving username:', recipientIdentifier) | |
| const user = await usersApi.getByUsername(recipientIdentifier) | |
| // Find the Peanut wallet account (should be the primary one) | |
| const peanutAccount = user.accounts.find((account) => account.type === AccountType.PEANUT_WALLET) | |
| if (!peanutAccount) { | |
| throw new Error(`User ${recipientIdentifier} does not have a Peanut wallet`) | |
| } | |
| resolvedRecipient = { | |
| identifier: recipientIdentifier, | |
| resolvedAddress: peanutAccount.identifier, // This should be the wallet address | |
| recipientType: 'USERNAME', | |
| } | |
| console.log('✅ Username resolved:', { | |
| username: recipientIdentifier, | |
| address: peanutAccount.identifier, | |
| }) | |
| } | |
| setParsedRecipient(resolvedRecipient) | |
| } catch (err) { | |
| console.error('Error resolving recipient:', err) | |
| setError('Failed to resolve recipient') | |
| } finally { | |
| setIsLoading(false) | |
| } | |
| } | |
| resolveRecipient() | |
| }, [recipient]) | |
| useEffect(() => { | |
| let isMounted = true | |
| const resolveRecipient = async () => { | |
| try { | |
| if (!recipient || recipient.length === 0) { | |
| setError('No recipient specified') | |
| return | |
| } | |
| const recipientIdentifier = recipient[0] | |
| let resolvedRecipient: ParsedURL['recipient'] | |
| if (isAddress(recipientIdentifier)) { | |
| // It's already a valid address | |
| resolvedRecipient = { | |
| identifier: recipientIdentifier, | |
| resolvedAddress: recipientIdentifier, | |
| recipientType: 'ADDRESS', | |
| } | |
| } else { | |
| // It's a username - resolve it to an address | |
| console.log('🔍 Resolving username:', recipientIdentifier) | |
| const user = await usersApi.getByUsername(recipientIdentifier) | |
| // Find the Peanut wallet account (should be the primary one) | |
| const peanutAccount = user.accounts.find( | |
| (account) => account.type === AccountType.PEANUT_WALLET | |
| ) | |
| if (!peanutAccount) { | |
| throw new Error( | |
| `User @${recipientIdentifier} does not have a Peanut wallet. Please ask them to create one.` | |
| ) | |
| } | |
| resolvedRecipient = { | |
| identifier: recipientIdentifier, | |
| resolvedAddress: peanutAccount.identifier, // This should be the wallet address | |
| recipientType: 'USERNAME', | |
| } | |
| console.log('✅ Username resolved:', { | |
| username: recipientIdentifier, | |
| address: peanutAccount.identifier, | |
| }) | |
| } | |
| if (isMounted) { | |
| setParsedRecipient(resolvedRecipient) | |
| } | |
| } catch (err) { | |
| console.error('Error resolving recipient:', err) | |
| const errorMessage = | |
| err instanceof Error ? err.message : 'Failed to resolve recipient' | |
| if (isMounted) { | |
| setError(errorMessage) | |
| } | |
| } finally { | |
| if (isMounted) { | |
| setIsLoading(false) | |
| } | |
| } | |
| } | |
| resolveRecipient() | |
| return () => { | |
| isMounted = false | |
| } | |
| }, [recipient]) |
🤖 Prompt for AI Agents
In src/app/send/[...username]/client.tsx around lines 26 to 79, the useEffect
spawns an async resolver but doesn't cancel or prevent state updates after
unmount and it uses generic error text; add a mounted flag (or AbortController)
inside the effect and check it before calling setParsedRecipient, setError, or
setIsLoading so no state updates occur after unmount, and ensure finally also
checks mounted; when catching errors, populate setError with a clearer message —
prefer err.message or a mapped user-facing string (e.g., "Username not found",
"User has no Peanut wallet", or "Network error") so users see more specific
feedback; also ensure to clear the mounted flag in the cleanup function and
include any used external references in the dependency array if necessary.
| const handleInitialSubmit = async (payload: RequestPayPayload) => { | ||
| try { | ||
| let finalPayload = payload | ||
|
|
||
| // For dynamic scenarios (no existing charge), create charge before moving to confirm | ||
| if (!payload.chargeId && payload.recipient && payload.selectedTokenAddress && payload.selectedChainID) { | ||
| setIsCreatingCharge(true) | ||
|
|
||
| const newCharge = await createCharge(payload) | ||
|
|
||
| // Update payload with the new charge ID | ||
| finalPayload = { | ||
| ...payload, | ||
| chargeId: newCharge.uuid, | ||
| } | ||
| } | ||
|
|
||
| setPaymentPayload(finalPayload) | ||
| setCurrentView('confirm') | ||
| } catch (error) { | ||
| console.error('❌ Failed to create charge:', error) | ||
| // Error will be handled by the hook and displayed in the UI | ||
| } finally { | ||
| setIsCreatingCharge(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for edge cases in charge creation
The function assumes that createCharge will always succeed when the conditions are met, but doesn't handle potential edge cases like network failures or invalid payload data that could cause the charge creation to fail silently.
Add validation and proper error state management:
const handleInitialSubmit = async (payload: RequestPayPayload) => {
try {
let finalPayload = payload
// For dynamic scenarios (no existing charge), create charge before moving to confirm
if (!payload.chargeId && payload.recipient && payload.selectedTokenAddress && payload.selectedChainID) {
setIsCreatingCharge(true)
+
+ // Validate payload before creating charge
+ if (!payload.tokenAmount || parseFloat(payload.tokenAmount) <= 0) {
+ throw new Error('Invalid token amount')
+ }
const newCharge = await createCharge(payload)
+
+ if (!newCharge?.uuid) {
+ throw new Error('Failed to create charge: Invalid response')
+ }
// Update payload with the new charge ID
finalPayload = {
...payload,
chargeId: newCharge.uuid,
}
}
setPaymentPayload(finalPayload)
setCurrentView('confirm')
} catch (error) {
console.error('❌ Failed to create charge:', error)
// Error will be handled by the hook and displayed in the UI
} finally {
setIsCreatingCharge(false)
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/Payment/flows/RequestPayFlow.tsx around lines 61-86, the
handler assumes createCharge always succeeds and proceeds to the confirm view
even if charge creation fails; add input validation before calling createCharge
(ensure recipient, selectedTokenAddress, selectedChainID are non-empty and
payload shape is valid), wrap createCharge call to explicitly handle rejected
promises and unexpected returns (check that newCharge exists and newCharge.uuid
is a valid string), set and surface an error state/message when creation fails
(do not call setPaymentPayload or setCurrentView('confirm') on failure), and
ensure setIsCreatingCharge(false) runs in finally; optionally log contextual
error and return early so the UI shows the error instead of moving forward.
| const handleConfirmSubmit = async () => { | ||
| if (!paymentPayload) return | ||
|
|
||
| const result = await payRequest(paymentPayload) | ||
|
|
||
| if (result.success) { | ||
| setCurrentView('status') | ||
| } | ||
| // Error handling is done by the hook via TanStack Query | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check before proceeding with payment request
While there's a guard clause for paymentPayload, the function should also validate that the payload contains the required fields before attempting the payment.
const handleConfirmSubmit = async () => {
- if (!paymentPayload) return
+ if (!paymentPayload || !paymentPayload.tokenAmount) {
+ console.error('Invalid payment payload')
+ return
+ }
const result = await payRequest(paymentPayload)
if (result.success) {
setCurrentView('status')
}
// Error handling is done by the hook via TanStack Query
}📝 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.
| const handleConfirmSubmit = async () => { | |
| if (!paymentPayload) return | |
| const result = await payRequest(paymentPayload) | |
| if (result.success) { | |
| setCurrentView('status') | |
| } | |
| // Error handling is done by the hook via TanStack Query | |
| } | |
| const handleConfirmSubmit = async () => { | |
| if (!paymentPayload || !paymentPayload.tokenAmount) { | |
| console.error('Invalid payment payload') | |
| return | |
| } | |
| const result = await payRequest(paymentPayload) | |
| if (result.success) { | |
| setCurrentView('status') | |
| } | |
| // Error handling is done by the hook via TanStack Query | |
| } |
🤖 Prompt for AI Agents
In src/components/Payment/flows/RequestPayFlow.tsx around lines 88 to 97, the
handler only checks for a non-null paymentPayload but does not validate that
required fields are present; before calling payRequest, verify that required
properties (for example amount, recipientId, and currency — or the actual
required keys used by payRequest) are present and valid (non-empty/positive
types), otherwise short-circuit (return) or set an error state and avoid calling
payRequest; if validation fails provide a clear user-facing error or set the
existing form error state and only call payRequest when all required fields pass
validation.
| case 'confirm': | ||
| return ( | ||
| <RequestPayConfirm | ||
| payload={paymentPayload!} |
There was a problem hiding this comment.
Non-null assertion on paymentPayload could cause runtime error
Using the non-null assertion operator ! on Line 138 could lead to runtime errors if paymentPayload is null. While there's a guard in handleConfirmSubmit, the component could still render with null payload in edge cases.
case 'confirm':
+ if (!paymentPayload) {
+ // This shouldn't happen, but handle gracefully
+ setCurrentView('initial')
+ return null
+ }
return (
<RequestPayConfirm
- payload={paymentPayload!}
+ payload={paymentPayload}
chargeDetails={chargeDetails}
route={route}
estimatedFees={estimatedFees}
isProcessing={isProcessing}
isPreparingRoute={isPreparingRoute || isLoadingRoute}
error={error}
onConfirm={handleConfirmSubmit}
onBack={handleGoBack}
/>
)📝 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.
| payload={paymentPayload!} | |
| case 'confirm': | |
| if (!paymentPayload) { | |
| // This shouldn't happen, but handle gracefully | |
| setCurrentView('initial') | |
| return null | |
| } | |
| return ( | |
| <RequestPayConfirm | |
| payload={paymentPayload} | |
| chargeDetails={chargeDetails} | |
| route={route} | |
| estimatedFees={estimatedFees} | |
| isProcessing={isProcessing} | |
| isPreparingRoute={isPreparingRoute || isLoadingRoute} | |
| error={error} | |
| onConfirm={handleConfirmSubmit} | |
| onBack={handleGoBack} | |
| /> | |
| ) |
🤖 Prompt for AI Agents
In src/components/Payment/flows/RequestPayFlow.tsx around line 138, the JSX uses
a non-null assertion payload={paymentPayload!} which can throw if paymentPayload
is null; remove the non-null assertion and instead pass a safe value or avoid
rendering the child until payload exists — e.g. conditionally render the
component only when paymentPayload is defined, or pass payload={paymentPayload
?? undefined} (and handle undefined in the child), or add a lightweight fallback
payload; ensure the component's props are typed to accept undefined if you
choose the latter.
|
|
||
| {/* Transaction Details Drawer */} | ||
| <TransactionDetailsDrawer | ||
| isOpen={isDrawerOpen && selectedTransaction?.id === transactionForDrawer?.id} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential issue with drawer visibility condition
The condition for isOpen on line 209 checks both isDrawerOpen and whether the selected transaction ID matches. However, if transactionForDrawer is null, accessing transactionForDrawer?.id will return undefined, which could cause unexpected behavior.
Consider simplifying the condition to be more defensive:
<TransactionDetailsDrawer
- isOpen={isDrawerOpen && selectedTransaction?.id === transactionForDrawer?.id}
+ isOpen={isDrawerOpen && !!transactionForDrawer && selectedTransaction?.id === transactionForDrawer.id}
onClose={closeTransactionDetails}
transaction={selectedTransaction}
/>📝 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.
| isOpen={isDrawerOpen && selectedTransaction?.id === transactionForDrawer?.id} | |
| <TransactionDetailsDrawer | |
| isOpen={isDrawerOpen && !!transactionForDrawer && selectedTransaction?.id === transactionForDrawer.id} | |
| onClose={closeTransactionDetails} | |
| transaction={selectedTransaction} | |
| /> |
🤖 Prompt for AI Agents
In src/components/Payment/flows/views/RequestPayStatus.tsx around line 209, the
current isOpen condition uses selectedTransaction?.id ===
transactionForDrawer?.id which can evaluate against undefined when
transactionForDrawer is null; make the check defensive by first ensuring
transactionForDrawer exists (and optionally selectedTransaction has an id)
before comparing IDs so the drawer only opens when isDrawerOpen is true and both
transactionForDrawer and the selected transaction id are present and equal.
| export interface RequestPayPayload { | ||
| // For existing charges/requests | ||
| chargeId?: string | ||
| requestId?: string | ||
| tokenAmount: string | ||
| recipient?: PaymentRecipient | ||
| attachmentOptions?: AttachmentOptions | ||
|
|
||
| // For dynamic charge creation (like CryptoWithdrawFlow) | ||
| selectedTokenAddress?: string | ||
| selectedChainID?: string | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate RequestPayPayload design for dual usage patterns.
The payload supports both existing charges (chargeId, requestId) and dynamic creation (selectedTokenAddress, selectedChainID). Ensure this doesn't create ambiguous states.
🏁 Script executed:
#!/bin/bash
# Check how RequestPayPayload is used to ensure the dual pattern is handled correctly
echo "Checking RequestPayPayload usage patterns:"
rg -B 3 -A 10 "RequestPayPayload" src/hooks/payment/
echo -e "\nChecking if validation exists for the dual usage pattern:"
ast-grep --pattern $'chargeId && selectedTokenAddress'Length of output: 5653
Enforce exclusive RequestPayPayload patterns
Although the runtime guards in useRequestPayFlow.ts separate the “existing charge” and “dynamic creation” flows, the flat RequestPayPayload interface still allows both sets of properties to coexist—leading to ambiguous states. To eliminate this ambiguity at compile time, please:
• In src/hooks/payment/types.ts, refactor RequestPayPayload into a discriminated union with two strict variants:
– Existing charge:
• chargeId: string
• requestId?: string
• tokenAmount: string
• recipient?: PaymentRecipient
• attachmentOptions?: AttachmentOptions
• selectedTokenAddress?: never
• selectedChainID?: never
– Dynamic creation:
• selectedTokenAddress: string
• selectedChainID: string
• tokenAmount: string
• recipient: PaymentRecipient
• attachmentOptions?: AttachmentOptions
• chargeId?: never
• requestId?: never
• Update all usages in src/hooks/payment/useRequestPayFlow.ts to narrow on the discriminant and remove any redundant runtime checks for mutually exclusive fields.
Example replacement in types.ts:
export type RequestPayPayload =
| {
chargeId: string
requestId?: string
tokenAmount: string
recipient?: PaymentRecipient
attachmentOptions?: AttachmentOptions
selectedTokenAddress?: never
selectedChainID?: never
}
| {
chargeId?: never
requestId?: never
tokenAmount: string
recipient: PaymentRecipient
attachmentOptions?: AttachmentOptions
selectedTokenAddress: string
selectedChainID: string
}This change guarantees at compile time that you can never accidentally pass both sets (or neither), and keeps your runtime logic simple and error-free.
🤖 Prompt for AI Agents
In src/hooks/payment/types.ts around lines 45-56, the flat RequestPayPayload
interface allows both "existing charge" and "dynamic creation" fields to
coexist; refactor it into a discriminated union with two strict variants
(Existing charge variant: required chargeId:string, optional requestId,
tokenAmount:string, optional recipient and attachmentOptions, and
selectedTokenAddress/selectedChainID set to never; Dynamic creation variant:
required selectedTokenAddress:string and selectedChainID:string, required
tokenAmount:string and recipient:PaymentRecipient, optional attachmentOptions,
and chargeId/requestId set to never). Then update
src/hooks/payment/useRequestPayFlow.ts to narrow on the discriminant (presence
of chargeId vs selectedTokenAddress) and remove redundant runtime checks for
mutually exclusive fields so flow logic relies on the type union instead of
overlap-prone conditionals.
| const transferData = isNativeCurrency(payload.fromTokenAddress) | ||
| ? '0x' // Native token transfer | ||
| : '0x' // ERC20 transfer (would need proper encoding) | ||
|
|
||
| transactions = [ | ||
| { | ||
| to: PEANUT_WALLET_TOKEN as Address, | ||
| data: transferData, | ||
| value: isNativeCurrency(payload.fromTokenAddress) | ||
| ? parseUnits(payload.tokenAmount, 18) | ||
| : 0n, | ||
| }, | ||
| ] | ||
| } |
There was a problem hiding this comment.
Fix incorrect logic for ERC20 transfers
The code for same-chain, same-token transfers has issues:
- For ERC20 transfers, the
toaddress should be the recipient wallet, not the token contract - The transfer data is set to
'0x'which is incorrect for ERC20 transfers - it needs proper encoding of thetransferfunction
Fix the transfer logic for same-chain, same-token transfers:
} else {
// Same chain, same token - simple transfer
- const transferData = isNativeCurrency(payload.fromTokenAddress)
- ? '0x' // Native token transfer
- : '0x' // ERC20 transfer (would need proper encoding)
-
- transactions = [
- {
- to: PEANUT_WALLET_TOKEN as Address,
- data: transferData,
- value: isNativeCurrency(payload.fromTokenAddress)
- ? parseUnits(payload.tokenAmount, 18)
- : 0n,
- },
- ]
+ if (isNativeCurrency(payload.fromTokenAddress)) {
+ // Native token transfer
+ transactions = [
+ {
+ to: wagmiAddress as Address, // Send to user's wallet
+ data: '0x',
+ value: parseUnits(payload.tokenAmount, 18),
+ },
+ ]
+ } else {
+ // ERC20 transfer - needs proper encoding
+ // This should encode: transfer(address to, uint256 amount)
+ const transferInterface = new ethers.utils.Interface([
+ 'function transfer(address to, uint256 amount)',
+ ])
+ const transferData = transferInterface.encodeFunctionData('transfer', [
+ wagmiAddress,
+ parseUnits(payload.tokenAmount, 6), // USDC has 6 decimals
+ ])
+
+ transactions = [
+ {
+ to: PEANUT_WALLET_TOKEN as Address, // ERC20 contract address
+ data: transferData as `0x${string}`,
+ value: 0n,
+ },
+ ]
+ }
}Note: You'll need to import the appropriate encoding utility (ethers or viem's encodeFunctionData).
📝 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.
| const transferData = isNativeCurrency(payload.fromTokenAddress) | |
| ? '0x' // Native token transfer | |
| : '0x' // ERC20 transfer (would need proper encoding) | |
| transactions = [ | |
| { | |
| to: PEANUT_WALLET_TOKEN as Address, | |
| data: transferData, | |
| value: isNativeCurrency(payload.fromTokenAddress) | |
| ? parseUnits(payload.tokenAmount, 18) | |
| : 0n, | |
| }, | |
| ] | |
| } | |
| } else { | |
| // Same chain, same token - simple transfer | |
| if (isNativeCurrency(payload.fromTokenAddress)) { | |
| // Native token transfer | |
| transactions = [ | |
| { | |
| to: wagmiAddress as Address, // Send to user's wallet | |
| data: '0x', | |
| value: parseUnits(payload.tokenAmount, 18), | |
| }, | |
| ] | |
| } else { | |
| // ERC20 transfer - needs proper encoding | |
| // This should encode: transfer(address to, uint256 amount) | |
| const transferInterface = new ethers.utils.Interface([ | |
| 'function transfer(address to, uint256 amount)', | |
| ]) | |
| const transferData = transferInterface.encodeFunctionData('transfer', [ | |
| wagmiAddress, | |
| parseUnits(payload.tokenAmount, 6), // USDC has 6 decimals | |
| ]) | |
| transactions = [ | |
| { | |
| to: PEANUT_WALLET_TOKEN as Address, // ERC20 contract address | |
| data: transferData as `0x${string}`, | |
| value: 0n, | |
| }, | |
| ] | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/hooks/payment/useAddMoneyFlow.ts around lines 137-150, the same-chain
same-token transfer logic is wrong: for ERC20 you must set the transaction "to"
to the recipient wallet and supply properly encoded ERC20 transfer calldata (not
'0x'), and for native transfers you should keep sending native value to the
recipient; update the branch so that if
isNativeCurrency(payload.fromTokenAddress) you create a tx with to =
payload.toWalletAddress (or PEANUT_WALLET_TOKEN if that constant represents the
recipient) and value = parseUnits(payload.tokenAmount, 18), data = '0x';
otherwise (ERC20) set to = payload.toWalletAddress (recipient), value = 0n, and
data = encodeFunctionData for transfer(recipient, amount) (import
encodeFunctionData from ethers or viem) using the correct token decimals when
encoding/parsing amount. Ensure imports are added and ERC20 branch uses encoded
calldata and zero value.
| chainId: PEANUT_WALLET_CHAIN.id.toString(), | ||
| hash: receipt.transactionHash, | ||
| tokenAddress: PEANUT_WALLET_TOKEN, | ||
| payerAddress: peanutWalletAddress ?? '', |
There was a problem hiding this comment.
Handle edge case when peanutWalletAddress is null
Using the nullish coalescing operator with an empty string could lead to issues if the wallet address is actually required by the API.
- payerAddress: peanutWalletAddress ?? '',
+ payerAddress: peanutWalletAddress!,Or better yet, add a validation check earlier:
const sendDirectly = useCallback(
async (payload: DirectSendPayload): Promise<DirectSendResult> => {
+ if (!peanutWalletAddress) {
+ return {
+ success: false,
+ error: 'Wallet not connected',
+ }
+ }
+
setIsProcessing(true)
setError(null)🤖 Prompt for AI Agents
In src/hooks/payment/useDirectSendFlow.ts around line 130, the code uses
payerAddress: peanutWalletAddress ?? '' which masks a missing wallet address;
validate peanutWalletAddress earlier and fail fast instead of supplying an empty
string—add a check before constructing the payload that ensures
peanutWalletAddress is non-null/non-empty (throw an error or return a rejected
Promise/validation result) and only build the request when it is present, or
propagate a clear validation error to the caller so the API never receives an
empty payerAddress.
| queryClient.getQueryData(['charge-details', payload.chargeId]) || | ||
| (await chargesApi.get(payload.chargeId)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential null reference when accessing query cache
The code uses || operator which could return a falsy value from the cache. Consider using nullish coalescing for safer access.
fullChargeDetails =
- queryClient.getQueryData(['charge-details', payload.chargeId]) ||
- (await chargesApi.get(payload.chargeId))
+ queryClient.getQueryData(['charge-details', payload.chargeId]) ??
+ (await chargesApi.get(payload.chargeId))📝 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.
| queryClient.getQueryData(['charge-details', payload.chargeId]) || | |
| (await chargesApi.get(payload.chargeId)) | |
| fullChargeDetails = | |
| queryClient.getQueryData(['charge-details', payload.chargeId]) ?? | |
| (await chargesApi.get(payload.chargeId)) |
🤖 Prompt for AI Agents
In src/hooks/payment/useRequestPayFlow.ts around lines 197 to 198, the
expression uses the logical OR (||) to fall back to fetching charge details
which can mis-handle valid falsy cache values; replace the || with the nullish
coalescing operator (??) so only null or undefined trigger the API call,
preserving legitimately falsy cache entries and avoiding unintended fetches.
jjramirezn
changed the title
1 participant
Separate State and reusable UI for the different payment flows.
Each flow logic and state must be separated from eachother and also reset properly on unmount (ie: the dev shoulndt have to remember to call a resetState() ) For this each payment flow will be its own hook, we are removing redux stores for state and storing directly in the hook (and on unmount it dissapear, that is intended). Also using tanstack query for some calls (we are using in other parts of the application and they are working well for us)
UI should not contain flow logic and as much as possible just receive the necesary props.
After all this we should be able to have the same flows with the same UI, with less bug and developer overhead
For more information read: https://www.notion.so/peanutprotocol/PaymentForm-and-flow-refactor-workshop-22b838117579800c8074d649e6bc565f?source=copy_link