Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds TanStack Query hooks for wallet balances, token prices, and Squid chains/tokens; introduces usePendingTransactions and BALANCE_DECREASE-related mutation keys; integrates these hooks across token selector, payment, send, QR-pay, and withdraw flows to gate or replace previous balance/price fetching and validation logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/app/(mobile-ui)/qr-pay/page.tsx (1)
32-32: Pending-transactions gating looks correct; consider one small UX hardening.
- Skipping balance checks when hasPendingTransactions or isWaitingForWebSocket prevents the race you described. LGTM.
- Optional: also disable the Pay button while hasPendingTransactions to eliminate accidental double-submission taps during brief UI states. You already disable on isLoading/isWaitingForWebSocket, so this is purely defensive.
Please confirm that all balance-decreasing mutations consistently include mutationKey: [BALANCE_DECREASE, ...] so hasPendingTransactions always reflects in-flight sends across flows.
Also applies to: 79-79, 720-736
src/constants/query.consts.ts (1)
6-14: Mutation tracking constants — LGTM.Names are clear and align with usage (BALANCE_DECREASE as umbrella). Optionally export a frozen object or a union type to catch typos at compile time, but not required.
src/context/tokenSelector.context.tsx (1)
95-99: refetchXchainRoute is only ever set to true here.If downstream doesn’t reset it, the flag stays true permanently. Consider toggling (true → false) after consumers react, or document the contract clearly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/app/(mobile-ui)/qr-pay/page.tsx(4 hunks)src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx(5 hunks)src/app/(mobile-ui)/withdraw/manteca/page.tsx(4 hunks)src/components/Global/TokenSelector/TokenSelector.tsx(3 hunks)src/components/Payment/PaymentForm/index.tsx(4 hunks)src/components/Send/link/views/Initial.link.send.view.tsx(4 hunks)src/constants/query.consts.ts(1 hunks)src/context/tokenSelector.context.tsx(3 hunks)src/hooks/useSquidChainsAndTokens.ts(1 hunks)src/hooks/useTokenPrice.ts(1 hunks)src/hooks/useWalletBalances.ts(1 hunks)src/hooks/wallet/__tests__/usePendingTransactions.test.tsx(1 hunks)src/hooks/wallet/usePendingTransactions.ts(1 hunks)src/hooks/wallet/useSendMoney.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-09-18T09:30:42.901Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#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/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/Global/TokenSelector/TokenSelector.tsxsrc/context/tokenSelector.context.tsx
📚 Learning: 2024-10-11T01:14:15.489Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/TokenSelector/TokenSelector.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/Send/link/views/Initial.link.send.view.tsxsrc/context/tokenSelector.context.tsx
📚 Learning: 2024-10-07T13:42:00.443Z
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#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/hooks/useTokenPrice.tssrc/context/tokenSelector.context.tsx
📚 Learning: 2024-10-03T09:57:43.885Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/hooks/useTokenPrice.tssrc/context/tokenSelector.context.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
📚 Learning: 2025-01-16T13:14:40.363Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/context/tokenSelector.context.tsx
🧬 Code graph analysis (13)
src/app/(mobile-ui)/withdraw/manteca/page.tsx (1)
src/hooks/wallet/usePendingTransactions.ts (1)
usePendingTransactions(33-62)
src/hooks/useWalletBalances.ts (2)
src/interfaces/interfaces.ts (1)
IUserBalance(19-30)src/app/actions/tokens.ts (1)
fetchWalletBalances(261-318)
src/hooks/wallet/__tests__/usePendingTransactions.test.tsx (2)
src/hooks/wallet/usePendingTransactions.ts (1)
usePendingTransactions(33-62)src/constants/query.consts.ts (1)
BALANCE_DECREASE(7-7)
src/components/Global/TokenSelector/TokenSelector.tsx (1)
src/hooks/useWalletBalances.ts (1)
useWalletBalances(24-41)
src/app/(mobile-ui)/qr-pay/page.tsx (1)
src/hooks/wallet/usePendingTransactions.ts (1)
usePendingTransactions(33-62)
src/hooks/useSquidChainsAndTokens.ts (1)
src/app/actions/squid.ts (1)
getSquidChainsAndTokens(36-59)
src/components/Send/link/views/Initial.link.send.view.tsx (1)
src/hooks/wallet/usePendingTransactions.ts (1)
usePendingTransactions(33-62)
src/hooks/useTokenPrice.ts (4)
src/interfaces/interfaces.ts (1)
ITokenPriceData(80-83)src/constants/zerodev.consts.ts (6)
PEANUT_WALLET_TOKEN(20-20)PEANUT_WALLET_TOKEN_DECIMALS(19-19)PEANUT_WALLET_TOKEN_SYMBOL(21-21)PEANUT_WALLET_TOKEN_NAME(22-22)PEANUT_WALLET_CHAIN(18-18)PEANUT_WALLET_TOKEN_IMG_URL(23-24)src/constants/general.consts.ts (2)
STABLE_COINS(211-211)supportedMobulaChains(62-187)src/app/actions/tokens.ts (1)
fetchTokenPrice(106-149)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (3)
src/hooks/wallet/useWallet.ts (1)
useWallet(15-103)src/hooks/wallet/usePendingTransactions.ts (1)
usePendingTransactions(33-62)src/constants/zerodev.consts.ts (1)
PEANUT_WALLET_TOKEN_DECIMALS(19-19)
src/hooks/wallet/usePendingTransactions.ts (1)
src/constants/query.consts.ts (1)
BALANCE_DECREASE(7-7)
src/context/tokenSelector.context.tsx (2)
src/hooks/useSquidChainsAndTokens.ts (1)
useSquidChainsAndTokens(17-27)src/hooks/useTokenPrice.ts (1)
useTokenPrice(43-110)
src/components/Payment/PaymentForm/index.tsx (1)
src/hooks/wallet/usePendingTransactions.ts (1)
usePendingTransactions(33-62)
src/hooks/wallet/useSendMoney.ts (1)
src/constants/query.consts.ts (2)
BALANCE_DECREASE(7-7)SEND_MONEY(8-8)
⏰ 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 (10)
src/hooks/wallet/useSendMoney.ts (1)
6-6: LGTM! Mutation key addition enables tracking.The mutationKey addition correctly follows TanStack Query patterns for hierarchical mutation tracking. This enables
usePendingTransactionsto track all balance-decreasing operations via theBALANCE_DECREASEkey.Also applies to: 40-40
src/app/(mobile-ui)/withdraw/manteca/page.tsx (1)
43-43: LGTM! Race condition fix properly implemented.The
hasPendingTransactionscheck correctly gates balance validation during pending transactions, preventing false "insufficient balance" errors during optimistic updates. The dependency array is properly updated.Also applies to: 72-72, 270-291
src/components/Payment/PaymentForm/index.tsx (1)
21-21: LGTM! Consistent race condition fix.The
hasPendingTransactionsguard is correctly added to the existing view/processing checks, preventing balance validation during optimistic updates. Dependency array properly updated.Also applies to: 119-119, 193-290
src/components/Send/link/views/Initial.link.send.view.tsx (1)
21-21: LGTM! Proper race condition handling.The
hasPendingTransactionscheck correctly prevents balance validation during pending mutations. Implementation matches the pattern used consistently across other components.Also applies to: 33-33, 100-135
src/hooks/wallet/__tests__/usePendingTransactions.test.tsx (1)
1-87: LGTM! Comprehensive test coverage.The test suite properly verifies the hook's integration with TanStack Query's mutation tracking system. Tests cover initial state, mutation key filtering, and both pending/non-pending scenarios.
src/hooks/wallet/usePendingTransactions.ts (1)
33-62: LGTM! Synchronous mutation tracking prevents race conditions.The hook correctly leverages
queryClient.isMutating()for synchronous mutation state tracking, eliminating race conditions. The decision to avoid auto-invalidation on completion (lines 42-54) is well-reasoned: it prevents confusing "backwards" balance updates due to RPC staleness while optimistic updates already show the correct state.src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
10-10: LGTM! Balance validation properly implemented.The balance validation correctly:
- Gates checks during pending transactions (lines 238-240)
- Uses
parseUnitsfor accurate comparison (line 247)- Provides clear user feedback via error message and disabled button
- Follows the consistent pattern used in other withdrawal flows
Also applies to: 13-13, 29-29, 43-43, 50-51, 235-253, 351-351, 358-358
src/hooks/useSquidChainsAndTokens.ts (1)
17-27: 36-hour combined cache may be acceptable—confirm product requirements.Squid adds chains and tokens without fixed cadence as integrations complete, meaning new assets aren't published on a predictable schedule. The stacked 24-hour client + 12-hour server cache creates up to 36-hour staleness, but impact depends on whether your product requires faster availability. If new chains/tokens need to appear sooner (e.g., same-day launch announcements), add a manual refresh mechanism or shorten client cache.
src/components/Global/TokenSelector/TokenSelector.tsx (1)
66-70: Switch to useWalletBalances simplifies state — nice.Loading guard prevents stale selection while refetching. Once the API route change in the hook lands, this should be robust.
src/hooks/useWalletBalances.ts (1)
2-2: The original review comment is incorrect—no fix needed.fetchWalletBalances is exported from a file with
'use server'directive (src/app/actions/tokens.ts:1), making it a Next.js server action. When imported into client components, Next.js automatically generates proxy stubs on the client that call the server-side function. The actual implementation and secrets (MOBULA_API_KEY) remain server-only and are never bundled into the client JavaScript. There is no build failure risk or secret leak risk.The only minor point: useWalletBalances.ts should add
'use client'at the top since it uses React hooks (useQuery), though this is a best-practice clarification rather than a functional issue.Likely an incorrect or invalid review comment.
| @@ -0,0 +1,110 @@ | |||
| import { useQuery } from '@tanstack/react-query' | |||
| import { fetchTokenPrice } from '@/app/actions/tokens' | |||
There was a problem hiding this comment.
🧩 Analysis chain
Server-only import in a client hook (price) — same boundary issue as balances.
fetchTokenPrice uses unstable_cache and server credentials. Importing it here will pull server-only code into the client.
Fix: call a server route from the hook.
-import { fetchTokenPrice } from '@/app/actions/tokens'
...
- const tokenPriceResponse = await fetchTokenPrice(tokenAddress!, chainId!)
+ const res = await fetch(
+ `/api/token-price?tokenAddress=${encodeURIComponent(tokenAddress!)}&chainId=${encodeURIComponent(chainId!)}`
+ )
+ if (!res.ok) return undefined
+ const tokenPriceResponse: ITokenPriceData | undefined = await res.json()Add a minimal route handler:
// app/api/token-price/route.ts
import { NextResponse } from 'next/server'
import { fetchTokenPrice } from '@/app/actions/tokens'
export async function GET(req: Request) {
const { searchParams } = new URL(req.url)
const tokenAddress = searchParams.get('tokenAddress')
const chainId = searchParams.get('chainId')
if (!tokenAddress || !chainId) return NextResponse.json({ error: 'params required' }, { status: 400 })
const data = await fetchTokenPrice(tokenAddress, chainId)
return NextResponse.json(data ?? null)
}Use the same grep from the balances comment to verify server-only imports.
Also applies to: 89-90
🏁 Script executed:
#!/bin/bash
# Find and inspect useTokenPrice.ts
fd 'useTokenPrice.ts' --type f -x cat {}Length of output: 4353
🏁 Script executed:
#!/bin/bash
# Find tokens action file and inspect fetchTokenPrice
fd 'tokens.*\.(ts|tsx)' --type f --path '*/actions/*' -x cat {}Length of output: 233
🏁 Script executed:
#!/bin/bash
# Search for unstable_cache usage in the codebase
rg 'unstable_cache' -n -B2 -A5 --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check for 'use client' directive in useTokenPrice.ts
rg '"use client"' -n -B1 -A10 src/hooks/useTokenPrice.tsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look at lines around 89-90 in useTokenPrice.ts
sed -n '80,100p' src/hooks/useTokenPrice.tsLength of output: 843
🏁 Script executed:
#!/bin/bash
# Find tokens action file with correct syntax
fd 'tokens' --type f | grep -i 'actions'Length of output: 25
🏁 Script executed:
#!/bin/bash
# Search for fetchTokenPrice definition and unstable_cache
rg 'fetchTokenPrice|unstable_cache' -n -B3 -A8 --type tsLength of output: 26187
🏁 Script executed:
#!/bin/bash
# Check where useTokenPrice hook is imported from
rg 'from.*useTokenPrice|import.*useTokenPrice' -n -B2 -A2 --type tsLength of output: 496
🏁 Script executed:
#!/bin/bash
# Check if any file that imports useTokenPrice has 'use client'
rg 'useTokenPrice' -n --type ts -l | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -20 {}'Length of output: 1472
Server-only import in a client hook — valid architectural boundary concern.
The review correctly identifies a server/client boundary violation. useTokenPrice uses useQuery (client-side), while fetchTokenPrice wraps server-only unstable_cache and accesses process.env.MOBULA_API_KEY. Although Next.js server actions can be called from client components via RPC, proper architecture requires client code to route through API endpoints, not directly invoke server-only functions. The proposed API route handler is the correct fix.
-import { fetchTokenPrice } from '@/app/actions/tokens'
...
- const tokenPriceResponse = await fetchTokenPrice(tokenAddress!, chainId!)
+ const res = await fetch(
+ `/api/token-price?tokenAddress=${encodeURIComponent(tokenAddress!)}&chainId=${encodeURIComponent(chainId!)}`
+ )
+ if (!res.ok) return undefined
+ const tokenPriceResponse: ITokenPriceData | undefined = await res.json()Add route handler:
// app/api/token-price/route.ts
import { NextResponse } from 'next/server'
import { fetchTokenPrice } from '@/app/actions/tokens'
export async function GET(req: Request) {
const { searchParams } = new URL(req.url)
const tokenAddress = searchParams.get('tokenAddress')
const chainId = searchParams.get('chainId')
if (!tokenAddress || !chainId) return NextResponse.json({ error: 'params required' }, { status: 400 })
const data = await fetchTokenPrice(tokenAddress, chainId)
return NextResponse.json(data ?? null)
}Also applies to lines 89–90 and other client-side imports of fetchTokenPrice (see Payment/PaymentForm, Claim, [...recipient]/client).
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/context/tokenSelector.context.tsx (1)
47-55: Consider removing unused constant.The
peanutWalletTokenDataobject is defined but never referenced in this file. If it's not used elsewhere, consider removing it to reduce clutter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Payment/PaymentForm/index.tsx(9 hunks)src/components/Request/link/views/Create.request.link.view.tsx(2 hunks)src/context/tokenSelector.context.tsx(3 hunks)src/hooks/useTokenPrice.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useTokenPrice.ts
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2024-10-07T13:42:00.443Z
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#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/Payment/PaymentForm/index.tsxsrc/components/Request/link/views/Create.request.link.view.tsxsrc/context/tokenSelector.context.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/Payment/PaymentForm/index.tsxsrc/components/Request/link/views/Create.request.link.view.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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.tsxsrc/components/Request/link/views/Create.request.link.view.tsxsrc/context/tokenSelector.context.tsx
📚 Learning: 2024-10-03T09:57:43.885Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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.tsxsrc/context/tokenSelector.context.tsx
📚 Learning: 2025-10-24T13:44:39.443Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1332
File: src/components/Global/TokenAmountInput/index.tsx:141-150
Timestamp: 2025-10-24T13:44:39.443Z
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/Payment/PaymentForm/index.tsx
📚 Learning: 2024-10-29T12:19:41.968Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/Payment/PaymentForm/index.tsx
📚 Learning: 2025-05-19T19:40:43.138Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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: 2024-10-07T15:25:45.170Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/Request/link/views/Create.request.link.view.tsx
📚 Learning: 2025-01-16T13:14:40.363Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/context/tokenSelector.context.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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/context/tokenSelector.context.tsx
🧬 Code graph analysis (2)
src/components/Payment/PaymentForm/index.tsx (4)
src/hooks/useSquidChainsAndTokens.ts (1)
useSquidChainsAndTokens(17-27)src/hooks/useTokenPrice.ts (1)
useTokenPrice(43-110)src/hooks/wallet/usePendingTransactions.ts (1)
usePendingTransactions(33-62)src/utils/general.utils.ts (1)
formatAmount(393-433)
src/context/tokenSelector.context.tsx (2)
src/hooks/useSquidChainsAndTokens.ts (1)
useSquidChainsAndTokens(17-27)src/hooks/useTokenPrice.ts (1)
useTokenPrice(43-110)
⏰ 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 (6)
src/components/Payment/PaymentForm/index.tsx (3)
20-22: LGTM! Clean integration of TanStack Query hooks.The new imports align with the PR's migration strategy to replace manual state management with React Query patterns.
106-115: LGTM! Safe hook integration with proper query gating.The hooks are correctly integrated:
useSquidChainsAndTokensprovides token metadata with sensible empty object fallbackuseTokenPricewon't execute queries whentokenAddressorchainIdare undefined (gated byenabledflag in the hook)
199-296: LGTM! Effectively resolves the race condition.The addition of
hasPendingTransactionsto the balance check gating logic directly addresses the PR's core issue: preventing false "insufficient balance" validations when optimistic updates race with async React loading states. The dependency array is correctly updated.src/components/Request/link/views/Create.request.link.view.tsx (1)
34-35: LGTM! Clean migration to consolidated token data.The refactor correctly removes the separate
selectedTokenPricefield and consolidates price information intoselectedTokenData.price. The USD calculation properly handles undefined values and includes the correct dependency.Also applies to: 76-79
src/context/tokenSelector.context.tsx (2)
69-85: Excellent migration to hook-based state management.The integration of
useSquidChainsAndTokensanduseTokenPricehooks successfully:
- Eliminates manual
useEffectboilerplate for price fetching- Provides a single source of truth for token data via
selectedTokenData- Leverages React Query's caching, deduplication, and stale-while-revalidate patterns
- Properly exposes
isFetchingTokenDatafor consumers to handle loading statesThe approach aligns well with the PR's goal of reducing race conditions and API calls. Based on learnings.
Also applies to: 126-130
87-93: Reject original review comment — reset logic exists.The flag is properly reset to false in two places:
- After
fetchRoute()completes (line 611)- When the route matches the current selection (line 684)
The original concern about missing reset logic is incorrect. The flag-based pattern works as intended: set to true when a refetch is needed, reset to false once handled.
The underlying design question about whether
isFetching(background refetches) is the right trigger vs.isLoading(initial fetch only) is a separate concern and a matter of intended behavior, not a bug.Likely an incorrect or invalid review comment.
|
@coderabbitai review in depth. Pay attention to what flows might break or are at high risk. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2547 🏁 Script executed: Length of output: 27 🏁 Script executed: Length of output: 586 🏁 Script executed: Length of output: 27 🏁 Script executed: Length of output: 4355 🏁 Script executed: Length of output: 58610 🏁 Script executed: Length of output: 26857 🏁 Script executed: Length of output: 15944 🏁 Script executed: Length of output: 6701 🏁 Script executed: Length of output: 34868 🏁 Script executed: Length of output: 4213 🏁 Script executed: Length of output: 27 🏁 Script executed: Length of output: 4881 🏁 Script executed: Length of output: 339 🏁 Script executed: Length of output: 3127 🏁 Script executed: Length of output: 11329 🏁 Script executed: Length of output: 12940 🏁 Script executed: Length of output: 5154 🏁 Script executed: Length of output: 8585 🏁 Script executed: Length of output: 0 @Hugo0, I've completed an in-depth review focusing on flows that might break or are at high risk. Here are my findings: 🚨 CRITICAL: Incomplete Mutation TrackingThe race condition fix is incomplete. Only Missing Mutation Keys:
// src/components/Create/useCreateLink.tsx
// This is just an async function, NOT a mutation!
const createLink = useCallback(async (amount: bigint) => {
// ... sends transaction but isn't tracked
}, [handleSendUserOpEncoded])
Impact: Users can still hit "insufficient balance" errors when:
Recommendation: Wrap ALL balance-decreasing operations in // Example fix for createLink
export const useCreateLink = () => {
const queryClient = useQueryClient()
return useMutation({
mutationKey: [BALANCE_DECREASE, SEND_LINK],
mutationFn: async (amount: bigint) => {
// existing createLink logic
},
// Add optimistic updates like useSendMoney
})
}
|
| Risk Level | Issue | Files Affected | Fix Priority |
|---|---|---|---|
| 🚨 CRITICAL | createLink not tracked as mutation | useCreateLink.tsx, Initial.link.send.view.tsx | P0 - Before merge |
| 🚨 CRITICAL | Payment mutations missing BALANCE_DECREASE key | usePaymentInitiator, PaymentForm | P0 - Before merge |
| Token price zero-division potential | PaymentForm/index.tsx, useTokenPrice.ts | P1 - Before release | |
| SimpleFi timeout no retry | qr-pay/page.tsx | P2 - Post-release | |
| Aggressive price caching | useTokenPrice.ts | P2 - Monitor metrics |
Bottom Line: The race condition fix is a great architectural improvement, but incomplete mutation tracking leaves Send Link and potentially other flows vulnerable to the same race condition the PR aims to fix. This needs to be addressed before merge to achieve the PR's stated goals.
Would you like me to help generate the mutation wrappers for createLink and other missing operations?
🧠 Learnings used
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#852
File: src/hooks/useTransactionHistory.ts:177-239
Timestamp: 2025-05-14T21:16:06.369Z
Learning: When reviewing code, pay attention to and resolve CodeRabbit comments as they can contain useful refactoring suggestions and insights.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/hooks/useGetExchangeRate.tsx (1)
26-30: Avoid caching fallback '1' as a “successful” value; prefer placeholderData + enabled gating.Returning '1' on errors masks failures and caches it for 5 minutes. Consider:
- Gate non‑US calls:
enabled: enabled && accountType !== AccountType.US.- Provide immediate UX with
placeholderData: '1'.- On real errors: throw (to surface via devtools) or set an
isErrorflag (if you can extend the API later).const { data: exchangeRate, isFetching: isFetchingRate } = useQuery<string>({ queryKey: ['exchangeRate', accountType], queryFn: async () => { if (accountType === AccountType.US) { return '1' } try { const { data, error: rateError } = await getExchangeRate(accountType) if (rateError) { - console.error('Failed to fetch exchange rate:', rateError) - // Return default rate to 1 for error cases - return '1' + console.error('Failed to fetch exchange rate:', rateError) + // Prefer surfacing the error; placeholderData handles UX fallback. + throw new Error(rateError) } - return data?.sell_rate || '1' + return String(data?.sell_rate ?? '1') } catch (error) { console.error('An error occurred while fetching the exchange rate:', error) - return '1' + // Propagate to query error state; UX still sees '1' via placeholderData. + throw error } }, - enabled, + enabled: enabled && accountType !== AccountType.US, + placeholderData: '1', staleTime: 5 * 60 * 1000, gcTime: 10 * 60 * 1000, refetchOnWindowFocus: true, refetchInterval: 5 * 60 * 1000, retry: 3, retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000), })Verification suggestions:
- Confirm product decision: should errors be surfaced (devtools/isError) or fully masked?
- If masking is intentional, keep current behavior; otherwise adopt the pattern above.
Also applies to: 33-36, 39-45
src/hooks/useExchangeRate.ts (3)
86-90: Encode query params to avoid malformed URLs.Guard against unexpected currency strings with URLSearchParams.
- const res = await fetch(`/api/exchange-rate?from=${sourceCurrency}&to=${destinationCurrency}`) + const params = new URLSearchParams({ from: sourceCurrency, to: destinationCurrency }) + const res = await fetch(`/api/exchange-rate?${params.toString()}`)
85-96: Consider adding retry backoff for parity with the other hook.Add
retry: 3andretryDelayexponential backoff for transient API failures.} = useQuery<{ rate: number }>({ queryKey: ['exchangeRate', sourceCurrency, destinationCurrency], queryFn: async () => { … }, staleTime: 5 * 60 * 1000, gcTime: 10 * 60 * 1000, refetchOnWindowFocus: true, refetchInterval: 5 * 60 * 1000, enabled: enabled && !!sourceCurrency && !!destinationCurrency, + retry: 3, + retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000), })
91-96: Polling behavior is correct by default; consider making visibility gating explicit.TanStack Query v5 defaults to
refetchIntervalInBackground: false, which stops polling when the tab is hidden—the concern flagged in the review is already addressed. The current numericrefetchInterval: 5 * 60 * 1000will pause when hidden.To improve clarity for maintainers, consider either:
- Adding
refetchIntervalInBackground: falseexplicitly in the config- Or using a function-based
refetchInterval: () => (document.visibilityState === 'visible' ? 5 * 60 * 1000 : false)for explicit visibility gatingEither approach documents intent without changing runtime behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/TANSTACK_QUERY_OPPORTUNITIES.md(0 hunks)src/hooks/useExchangeRate.ts(2 hunks)src/hooks/useGetExchangeRate.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- docs/TANSTACK_QUERY_OPPORTUNITIES.md
🧰 Additional context used
🧬 Code graph analysis (1)
src/hooks/useGetExchangeRate.tsx (1)
src/app/actions/exchange-rate.ts (1)
getExchangeRate(25-61)
⏰ 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 (2)
src/hooks/useGetExchangeRate.tsx (1)
39-45: No action required — v5 defaults to safe background polling behavior.In TanStack Query v5,
refetchIntervalInBackgroundis supported and defaults tofalse, which stopsrefetchIntervalpolling when the tab is hidden. Since your code does not explicitly set this option, it automatically uses the safe default and will not poll in the background. No changes are needed.src/hooks/useExchangeRate.ts (1)
85-96: Query key structure prevents accidental collisions; concern is unfounded.Verified both hooks use structurally distinct keys:
['exchangeRate', accountType](src/hooks/useGetExchangeRate.tsx:16) vs['exchangeRate', sourceCurrency, destinationCurrency](src/hooks/useExchangeRate.ts:85). React Query requires exact key matching, so these won't collide. No targeted invalidation of 'exchangeRate' keys exists in the codebase; only broadqueryClient.invalidateQueries()calls appear in logout flows (intentional). No mix-ups detected.
| const { data: exchangeRate, isFetching: isFetchingRate } = useQuery({ | ||
| queryKey: ['exchangeRate', accountType], |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Strongly type the query and the hook’s return to prevent unknown leakage.
Without generics, data is unknown and the hook’s return type is implicit. Make types explicit and keep the return contract stable.
-export default function useGetExchangeRate({ accountType, enabled = true }: IExchangeRate) {
- const { data: exchangeRate, isFetching: isFetchingRate } = useQuery({
+export default function useGetExchangeRate(
+ { accountType, enabled = true }: IExchangeRate
+): { exchangeRate: string | null; isFetchingRate: boolean } {
+ const { data: exchangeRate, isFetching: isFetchingRate } = useQuery<string>({
queryKey: ['exchangeRate', accountType],
queryFn: async () => {
// US accounts have 1:1 exchange rate
if (accountType === AccountType.US) {
return '1'
}
…
- },
- enabled,
+ },
+ enabled,
staleTime: 5 * 60 * 1000, // 5 minutes
gcTime: 10 * 60 * 1000, // Garbage collect after 10 minutes
refetchOnWindowFocus: true, // Refresh rates when user returns to tab
refetchInterval: 5 * 60 * 1000, // Auto-refresh every 5 minutes
retry: 3,
retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000),
})
- return { exchangeRate: exchangeRate ?? null, isFetchingRate }
+ return { exchangeRate: exchangeRate ?? null, isFetchingRate }Also applies to: 47-48
🤖 Prompt for AI Agents
In src/hooks/useGetExchangeRate.tsx around lines 15-16 (and likewise at 47-48),
the useQuery call is untyped so data is inferred as unknown and the hook return
type is implicit; define a concrete ExchangeRate (or appropriate) interface, add
generics to useQuery (e.g., useQuery<ExchangeRate, Error>(...)) and change the
hook signature to explicitly return a typed object or
UseQueryResult<ExchangeRate, Error> so callers receive a stable, strongly-typed
return contract; ensure mapped fields (data/isFetching/error) use those types
and export the ExchangeRate type if needed.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/hooks/useTokenPrice.ts (1)
2-2: Server/client boundary violation — already flagged as critical.This issue has been thoroughly analyzed in previous reviews.
fetchTokenPriceusesunstable_cacheand server-only environment variables (process.env.MOBULA_API_KEY), making it incompatible with client-side code. The proposed solution is to create an API route (/api/token-price) and call it viafetchfrom this hook.See the detailed fix proposal in the previous review comments.
Also applies to: 89-90
🧹 Nitpick comments (2)
src/hooks/useTokenPrice.ts (2)
67-81: Consider adding defensive checks despite the enabled guard.The non-null assertions on lines 67-68 assume the
enabledguard (line 103) prevents execution whenchainIdortokenAddressare undefined. While technically safe, adding an early defensive check would improve robustness:// Case 2: Known stablecoin from supported tokens (always $1) +if (!chainId || !tokenAddress) return undefined + const token = supportedSquidChainsAndTokens[chainId!]?.tokens.find( (t) => t.address.toLowerCase() === tokenAddress!.toLowerCase() )This guards against future changes to the enabled condition or unexpected query behavior.
104-104: Consider shorter staleTime for volatile tokens to improve payment form accuracy.The 60-second cache is acceptable for stable/liquid assets but can lead to stale prices in USD value calculations for volatile tokens. Per DeFi best practices, UI caching should range from 5–30s for responsiveness to 10–60s for routine displays, with shorter intervals for low-liquidity or high-volatility pairs.
Currently, the hook mitigates some risk: stablecoins are hardcoded to $1, price validation prevents errors, and
refetchOnWindowFocusrefreshes when users return to the tab. However, volatile tokens cached for 60s could show outdated USD conversions in the PaymentForm, affecting both display accuracy and cross-chain token conversion calculations.Options to consider:
- Reduce staleTime to 30–45s as a practical middle ground, or
- Add a manual refresh trigger on PaymentForm focus, or
- Differentiate staleTime by querying token liquidity metadata (if available from Mobula or Squid), or
- Accept current behavior and document the trade-off (caching reliability vs price freshness).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Create/useCreateLink.tsx(1 hunks)src/hooks/usePaymentInitiator.ts(1 hunks)src/hooks/useTokenPrice.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/components/Create/useCreateLink.tsx
- src/hooks/usePaymentInitiator.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-07T13:42:00.443Z
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#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/hooks/useTokenPrice.ts
🧬 Code graph analysis (1)
src/hooks/useTokenPrice.ts (4)
src/interfaces/interfaces.ts (1)
ITokenPriceData(80-83)src/constants/zerodev.consts.ts (6)
PEANUT_WALLET_TOKEN(20-20)PEANUT_WALLET_TOKEN_DECIMALS(19-19)PEANUT_WALLET_TOKEN_SYMBOL(21-21)PEANUT_WALLET_TOKEN_NAME(22-22)PEANUT_WALLET_CHAIN(18-18)PEANUT_WALLET_TOKEN_IMG_URL(23-24)src/constants/general.consts.ts (2)
STABLE_COINS(211-211)supportedMobulaChains(62-187)src/app/actions/tokens.ts (1)
fetchTokenPrice(106-149)
⏰ 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 (2)
src/hooks/useTokenPrice.ts (2)
111-125: Excellent defensive validation — prevents zero-division across all consumers.The
selectfunction provides centralized validation that rejects invalid prices (NaN, non-numeric, or ≤ 0) before returning data to consumers. This directly mitigates the "token price zero-division" risk identified in the PR context. The warning log with context is also helpful for debugging.This is a significant improvement over the previous approach (noted in learnings) where failed fetches would set
price: 0, which could cause division errors.
27-126: Well-structured hook with clear separation of concerns.The four-case approach is logical and well-documented:
- Peanut Wallet USDC → instant $1 (no API call)
- Known stablecoins → instant $1 (no API call)
- Unsupported chains → early return (no wasted API call)
- API fetch with caching and validation
Returning
undefinedon failure (rather than0as noted in learnings) is a better design choice — it forces consumers to explicitly handle missing prices rather than risk silent zero-division errors.The TanStack Query integration provides automatic caching, deduplication, and retry logic, which should significantly reduce API calls as claimed in the PR objectives.
2 participants
Payment Optimization & Race Condition Fix
🎯 What This PR Does
Fixes race condition in payment flows and optimizes state management by migrating to TanStack Query patterns.
🐛 Problem & Solution
Race Condition in Balance Validation
Problem: Users saw false "insufficient balance" errors during payment processing.
Root Cause: Optimistic balance updates (synchronous) vs React loading states (asynchronous) → validation ran with new balance but old loading state.
Solution: Created
usePendingTransactionshook usingqueryClient.isMutating()- updates synchronously with mutations, eliminating race condition.Redundant API Calls
Problem: Manual
useEffects refetched data on every mount, wasting API calls.Solutions:
useSquidChainsAndTokenshookuseTokenPricehook (replaced 100+ lines of manual state)useWalletBalanceshook📊 Impact
Performance:
Code Quality:
useEffectboilerplateUX:
🔧 Technical Changes
New Hooks
usePendingTransactions- Tracks balance-decreasing mutationsuseSquidChainsAndTokens- Caches Squid config 24huseTokenPrice- Caches token prices 60suseWalletBalances- Caches balances 30s, auto-refresh 60sFiles Changed
Key Decision: No Auto-Refresh After Transactions
We do NOT force immediate balance refetch after transactions because:
✅ QA Testing Required
🔴 Critical - Race Condition Fixes (Must Test)
These flows had balance validation updated to prevent false errors:
Send Link (
Initial.link.send.view.tsx)QR Payment (⚠️ HIGH PRIORITY
qr-pay/page.tsx)Manteca Withdrawal (
withdraw/manteca/page.tsx)Bridge Withdrawal (
withdraw/[country]/bank/page.tsx) 🆕 NEW VALIDATIONRequest Payment (
Payment/PaymentForm/index.tsx)Pass Criteria: No "insufficient balance" errors during processing in any flow.
🟡 Medium - Caching Behavior (Should Test)
These features now use TanStack Query caching:
tokenSelector.context.tsx+TokenSelector.tsx)Pass Criteria:
🟢 Low Risk - Regression Check
Verify existing functionality still works:
Pass Criteria: No regressions, no console errors (except expected deprecation warnings).
🐛 Expected Warnings (Not Bugs)
Console may show deprecation warnings - this is harmless:
📝 Testing Time Estimate