feat: limits cards in payment flows#1621
Conversation
kushagrasarathe
commented
Jan 15, 2026
kushagrasarathe
commented
- contributes to TASK-17858
- also contributes to TASK-17860
- also contributes to TASK-17859
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a centralized limits system: new useLimitsValidation hook, consolidated limits utilities and QR-country derivation, LimitsWarningCard UI, flag/currency helpers, integration of limits checks into add-money, qr-pay, and withdraw flows, removal of legacy limits.utils, and a small icon addition. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review in depth. Evaluate:
Also, write a very concise testing guide for QA |
🧩 Analysis chain🏁 Script executed: Length of output: 43838
🔴 Risk Assessment: MEDIUM-HIGHCritical Risks1. Loss of Max Amount Validation (HIGH) - } else if (paymentAmount > parseUnits(MAX_MANTECA_WITHDRAW_AMOUNT.toString()...)) {
- setError(`Deposit amount exceeds maximum limit of $${MAX_MANTECA_WITHDRAW_AMOUNT}`)Now relies entirely on 2. Complex Conditional Logic (HIGH)
3. Error Message Hiding (MEDIUM) {error && !showLimitsCard && <ErrorAlert description={error} />}If 4. Tight Coupling in QR Pay (MEDIUM) const isLocalUser = isUserMantecaKycApproved && paymentProcessor === 'MANTECA'This couples user location to payment processor selection. Changes to processor logic could break limit validation. 5. Race Conditions (LOW-MEDIUM) Breaking Scenarios to Test
⚡ Performance Analysis: LOW IMPACTPositive✅ Proper use of Concerns
const daysUntilMonthlyReset = useMemo(() => {
const now = new Date()
const nextMonth = new Date(now.getFullYear(), now.getMonth() + 1, 1)
const diffTime = nextMonth.getTime() - now.getTime()
return Math.ceil(diffTime / (1000 * 60 * 60 * 24))
}, [])Empty dependency array is good, but this could be computed once globally. Verdict: Performance impact is negligible. The calculations are lightweight. 📊 Code Quality & DRY: NEEDS IMPROVEMENT✅ Excellent Patterns
❌ DRY Violations1. Currency Mapping Duplicated (3 locations) Repeated in const limitsCurrency = useMemo<LimitCurrency>(() => {
const currency = selectedCountry?.currency?.toUpperCase()
if (currency === 'ARS' || currency === 'BRL') return currency as LimitCurrency
return 'USD'
}, [selectedCountry?.currency])Fix: Create utility export const mapToLimitCurrency = (currency?: string): LimitCurrency => {
const upper = currency?.toUpperCase()
if (upper === 'ARS' || upper === 'BRL') return upper as LimitCurrency
return 'USD'
}2. Card Integration Code Duplicated (6 locations) Every flow has nearly identical code: {showLimitsCard && (
<LimitsWarningCard
type={limitsValidation.isBlocking ? 'error' : 'warning'}
title={
limitsValidation.isBlocking
? 'Amount too high, try a smaller amount.'
: "You're close to your limit."
}
items={[...]} // Similar but slightly different
/>
)}Fix: Create wrapper hook or component: export function useLimitsCardProps(validation: LimitValidationResult, currency: string) {
return useMemo(() => ({
type: validation.isBlocking ? 'error' : 'warning',
title: validation.isBlocking ? 'Amount too high...' : "You're close...",
items: [/* construct items */]
}), [validation, currency])
}3. Magic Strings (not constants)
Should be in constants file. 4. Button Disable Logic Pattern disabled={
!parseFloat(rawTokenAmount) ||
parseFloat(rawTokenAmount) < minimumAmount ||
error.showError ||
isCreatingOnramp ||
limitsValidation.isBlocking
}5. Error Conditional Pattern {error.showError && !!error.errorMessage && !showLimitsCard && (
<ErrorAlert description={error.errorMessage} />
)}Minor Issues
🧪 QA Testing GuideTest MatrixSetup Required
1. Onramp (Add Money) - Bridge FlowPath:
Verify:
2. Onramp - Manteca Flow (ARS/BRL)Path:
Verify:
3. QR PaymentsPath: Foreign User (No Manteca KYC)
Local User (Manteca KYC + PIX/MercadoPago)
Verify:
4. Offramp (Withdraw) - MantecaPath:
Critical: Verify max limit still enforced (was removed from local validation) 5. Offramp (Withdraw) - Generic FlowPath: For Bank Withdrawal:
For Crypto Withdrawal:
Verify:
6. Edge Cases (ALL FLOWS)
7. Visual/UX Testing
8. Accessibility
📋 RecommendationsBefore Merge
Follow-up Refactoring (Optional)
Would you like me to elaborate on any specific risk area or create additional test scenarios? 🧠 Learnings used✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/app/`(mobile-ui)/withdraw/manteca/page.tsx:
- Line 483: The button's disabled condition must also prevent clicks while
limits are still loading; update the expression that sets disabled (currently
referencing usdAmount, balanceErrorMessage, and limitsValidation.isBlocking) to
include limitsValidation.isLoading so the button is disabled when
limitsValidation.isLoading is true (e.g., add a clause combining
limitsValidation.isLoading with the existing conditions).
In `@src/app/`(mobile-ui)/withdraw/page.tsx:
- Around line 327-329: The continue button's disabled prop only checks
limitsValidation.isBlocking for non-crypto methods and can be clicked while
limits validation is still loading; update the disabled expression used where
disabled={isContinueDisabled || (selectedMethod?.type !== 'crypto' &&
limitsValidation.isBlocking)} to also include limitsValidation.isLoading for
non-crypto methods (e.g. disabled={isContinueDisabled || (selectedMethod?.type
!== 'crypto' && (limitsValidation.isBlocking || limitsValidation.isLoading))})
so the button is disabled while validation is loading or blocking.
In `@src/features/limits/components/LimitsWarningCard.tsx`:
- Around line 40-47: LimitsWarningCard renders InfoCard with a redundant ternary
for the variant prop (type === 'error' ? 'warning' : 'warning'), so update the
InfoCard call in the LimitsWarningCard component to either use a fixed variant
(variant="warning") if errors should look the same, or change the ternary to
return an appropriate error variant (e.g., variant={type === 'error' ? 'error' :
'warning'}) so the 'type' prop actually affects the InfoCard styling; adjust
only the variant prop on the InfoCard invocation.
🧹 Nitpick comments (4)
src/features/limits/components/LimitsWarningCard.tsx (1)
51-64: Consider: Link items use hardcoded yellow color even for error type.The link text and icon use
text-yellow-11regardless of whethertypeis'error'or'warning'. If blocking errors should have distinct styling (e.g., red tones), consider conditionally applying color classes based ontype.If the unified yellow styling is intentional for consistency, this is fine as-is.
src/app/(mobile-ui)/qr-pay/page.tsx (1)
393-411: Consider usingmapToLimitCurrencyutility for consistency.The
limitsCurrencylogic duplicates the mapping that exists inmapToLimitCurrencyfromlimits.utils.ts. While the QR-specific mapping is slightly different (it maps by QR type rather than currency string), consider whether this could be unified to reduce duplication.Current approach works correctly, but note the
mapToLimitCurrencyutility is available if the mapping logic needs to be shared.src/components/AddMoney/components/InputAmountStep.tsx (1)
17-22: Consider extractingLimitsValidationPropsto a shared location.This interface duplicates fields from the
useLimitsValidationhook's return type. While prop isolation is reasonable, consider exporting a subset type from the hook file to ensure consistency.src/app/(mobile-ui)/withdraw/page.tsx (1)
334-337: Minor: Consider usingshowLimitsCardfor consistency.The error suppression uses
!limitsValidation.isBlockingdirectly, while the card display uses theshowLimitsCardvariable which already accounts for the crypto exclusion. UsingshowLimitsCardwould make the logic more self-documenting.Optional refactor for consistency
- {error.showError && !!error.errorMessage && !limitsValidation.isBlocking && ( + {error.showError && !!error.errorMessage && !(showLimitsCard && limitsValidation.isBlocking) && ( <ErrorAlert description={error.errorMessage} /> )}Or more simply, since
showLimitsCardalready implies blocking/warning for non-crypto:- {error.showError && !!error.errorMessage && !limitsValidation.isBlocking && ( + {error.showError && !!error.errorMessage && !(showLimitsCard && limitsValidation.isBlocking) && (
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/components/AddMoney/components/InputAmountStep.tsxsrc/components/AddMoney/components/MantecaAddMoney.tsxsrc/features/limits/components/LimitsWarningCard.tsxsrc/features/limits/utils/limits.utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/AddMoney/components/MantecaAddMoney.tsx
🧰 Additional context used
🧠 Learnings (31)
📓 Common learnings
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 420
File: src/components/Offramp/Offramp.consts.ts:27-28
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In `src/components/Offramp/Offramp.consts.ts`, the `MIN_CASHOUT_LIMIT` is set to $10 because smaller amounts are impractical due to approximately $1 fee per cashout.
📚 Learning: 2025-08-14T14:42:54.411Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/utils/withdraw.utils.ts:181-191
Timestamp: 2025-08-14T14:42:54.411Z
Learning: The countryCodeMap in src/components/AddMoney/consts/index.ts uses uppercase 3-letter country codes as keys (like 'AUT', 'BEL', 'CZE') that map to 2-letter country codes, requiring input normalization to uppercase for proper lookups.
Applied to files:
src/features/limits/utils/limits.utils.tssrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 420
File: src/components/Offramp/Offramp.consts.ts:27-28
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In `src/components/Offramp/Offramp.consts.ts`, the `MIN_CASHOUT_LIMIT` is set to $10 because smaller amounts are impractical due to approximately $1 fee per cashout.
Applied to files:
src/features/limits/utils/limits.utils.tssrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-10-29T12:19:41.968Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 495
File: src/components/Global/TokenAmountInput/index.tsx:23-30
Timestamp: 2024-10-29T12:19:41.968Z
Learning: In the `TokenAmountInput` component (`src/components/Global/TokenAmountInput/index.tsx`), when the 'Max' button is clicked, we intentionally set the input denomination to 'TOKEN' because we are setting the value as token.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2025-10-24T13:44:39.473Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1332
File: src/components/Global/TokenAmountInput/index.tsx:141-150
Timestamp: 2025-10-24T13:44:39.473Z
Learning: In the `TokenAmountInput` component (`src/components/Global/TokenAmountInput/index.tsx`), the slider feature (controlled by `showSlider` prop) is only shown for USD input mode. When the slider is used with `maxAmount`, the `selectedAmount` is computed in USD and `isInputUsd` is always `true`, so the conversion in `onChange` handles it correctly.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-10-29T12:20:47.207Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 495
File: src/components/Create/Link/Input.view.tsx:244-248
Timestamp: 2024-10-29T12:20:47.207Z
Learning: In the `TokenAmountInput` component within `src/components/Global/TokenAmountInput/index.tsx`, when `balance` is undefined, the `maxValue` prop should be set to an empty string `''`.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsx
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2025-11-06T08:19:05.201Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1407
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:459-475
Timestamp: 2025-11-06T08:19:05.201Z
Learning: In src/components/AddWithdraw/DynamicBankAccountForm.tsx, the street address field has a 35-character maximum length limit because the backend provider enforces this constraint, applicable to both US and Mexican addresses.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-12-11T10:13:22.806Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 564
File: src/components/Request/Pay/Views/Initial.view.tsx:430-430
Timestamp: 2024-12-11T10:13:22.806Z
Learning: In the React TypeScript file `src/components/Request/Pay/Views/Initial.view.tsx`, when reviewing the `InitialView` component, do not flag potential issues with using non-null assertion `!` on the `slippagePercentage` variable, as handling undefined values in this context is considered out of scope.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsx
📚 Learning: 2025-08-22T07:25:59.304Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1104
File: src/components/Payment/PaymentForm/index.tsx:596-600
Timestamp: 2025-08-22T07:25:59.304Z
Learning: The `TokenAmountInput` component in `src/components/Global/TokenAmountInput/` always returns decimal strings (e.g., "1,234.56"), not base units. When passing these values to external APIs like Daimo's `toUnits` prop, simply stripping commas with `.replace(/,/g, '')` is sufficient.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsx
📚 Learning: 2025-10-29T11:27:59.248Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1368
File: src/components/Common/ActionList.tsx:109-111
Timestamp: 2025-10-29T11:27:59.248Z
Learning: In `src/components/Common/ActionList.tsx`, the `balance` from `useWallet()` hook is always in USDC (as a formatted string), making it directly comparable to USD amounts without conversion. The comparison `Number(balance) >= amountInUsd` is intentional and correct.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2025-11-07T11:51:58.900Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1419
File: src/components/Global/BalanceWarningModal/index.tsx:94-94
Timestamp: 2025-11-07T11:51:58.900Z
Learning: In `src/components/Global/BalanceWarningModal/index.tsx`, the casual and congratulatory phrase "You're rich! Congrats on having a high balance." is intentional and by design.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/features/limits/components/LimitsWarningCard.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2025-08-26T15:25:53.328Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1132
File: src/app/[...recipient]/client.tsx:394-397
Timestamp: 2025-08-26T15:25:53.328Z
Learning: In `src/components/Common/ActionListDaimoPayButton.tsx`, the `handleCompleteDaimoPayment` function should not display error messages to users when DB update fails because the Daimo payment itself has succeeded - showing errors would be confusing since the payment was successful.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-10-23T09:38:27.670Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 469
File: src/app/request/pay/page.tsx:32-64
Timestamp: 2024-10-23T09:38:27.670Z
Learning: In `src/app/request/pay/page.tsx`, if `linkRes` is not OK in the `generateMetadata` function, the desired behavior is to use the standard title and preview image without throwing an error.
Applied to files:
src/components/AddMoney/components/InputAmountStep.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2025-09-18T09:30:42.901Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1230
File: src/app/(mobile-ui)/withdraw/page.tsx:92-97
Timestamp: 2025-09-18T09:30:42.901Z
Learning: In src/app/(mobile-ui)/withdraw/page.tsx, the useEffect that calls setShowAllWithdrawMethods(true) when amountFromContext exists is intentionally designed to run only on component mount (empty dependency array), not when amountFromContext changes. This is the correct behavior for the withdraw flow where showing all methods should only happen on initial load when an amount is already present.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 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/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2025-10-02T15:23:01.513Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1266
File: src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx:46-57
Timestamp: 2025-10-02T15:23:01.513Z
Learning: In the withdraw flow at src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx, the points calculation query intentionally uses crypto.randomUUID() in the queryKey dependency array to bypass React Query caching, ensuring fresh points estimates on every render. This is the intended behavior.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-10-07T15:25:45.170Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-10-07T15:25:45.170Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsx
📚 Learning: 2026-01-02T13:14:32.635Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 0
File: :0-0
Timestamp: 2026-01-02T13:14:32.635Z
Learning: In the peanut-ui codebase, the add-money flows (bank and Manteca) use nuqs library for URL-based state management (step, amount, currency via useQueryStates), while RequestFulfillmentFlowContext still uses enum-based context state. This creates intentional partial migration where URL state is used for user-facing flows that benefit from deep linking and bookmarking.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 413
File: src/context/tokenSelector.context.tsx:118-123
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In the `TokenContextProvider` component within `src/context/tokenSelector.context.tsx`, in the TypeScript React application, when data changes and before calling `fetchAndSetTokenPrice`, it is necessary to reset `selectedTokenData`, `selectedTokenPrice`, `selectedTokenDecimals`, and `inputDenomination` to discard stale data.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsx
📚 Learning: 2025-06-18T19:56:55.443Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 919
File: src/components/Withdraw/views/Initial.withdraw.view.tsx:87-87
Timestamp: 2025-06-18T19:56:55.443Z
Learning: In withdraw flows for Peanut Wallet, the PeanutActionDetailsCard should always display "USDC" as the token symbol because it shows the amount being withdrawn from the Peanut Wallet (which holds USDC), regardless of the destination token/chain selected by the user. The TokenSelector is used for choosing the withdrawal destination, not the source display.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsx
📚 Learning: 2025-05-19T19:40:43.138Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 868
File: src/components/Payment/PaymentForm/index.tsx:284-293
Timestamp: 2025-05-19T19:40:43.138Z
Learning: When converting between USD and token amounts, always check if the token price (divisor) is valid and non-zero before performing the division to prevent Infinity, NaN, or errors. Implementing validation like `if (!tokenPrice || isNaN(tokenPrice) || tokenPrice === 0)` before division operations is crucial for handling cases where price data might be unavailable.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 551
File: src/components/Request/Create/Views/Initial.view.tsx:151-156
Timestamp: 2024-12-02T17:19:18.532Z
Learning: In the `InitialView` component at `src/components/Request/Create/Views/Initial.view.tsx`, when setting the default chain and token in the `useEffect` triggered by `isPeanutWallet`, it's acceptable to omit the setters from the dependency array and not include additional error handling for invalid defaults.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsx
📚 Learning: 2024-10-04T13:10:49.199Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 413
File: src/components/Request/Pay/Views/Initial.view.tsx:71-72
Timestamp: 2024-10-04T13:10:49.199Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, it's acceptable to use the `!` operator in TypeScript to assert that `selectedTokenData` is not `null` or `undefined`, and potential runtime errors from accessing its properties without checks can be disregarded.
Applied to files:
src/app/(mobile-ui)/withdraw/page.tsxsrc/app/(mobile-ui)/add-money/[country]/bank/page.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Pay.tsx:103-111
Timestamp: 2024-10-08T20:13:42.967Z
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/app/(mobile-ui)/withdraw/page.tsx
📚 Learning: 2025-08-20T09:08:19.266Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1112
File: src/components/Claim/Link/views/BankFlowManager.view.tsx:336-343
Timestamp: 2025-08-20T09:08:19.266Z
Learning: In the KYC flow implementation, `setJustCompletedKyc` must be called after `await fetchUser()` in the `handleKycSuccess` callback. Setting `justCompletedKyc` before fetching the user would cause a re-fetching loop because `handleKycSuccess` is set in a useEffect inside the KYC hook, which would cause the UI flow to get stuck in one view.
Applied to files:
src/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsx
📚 Learning: 2025-08-11T10:35:02.715Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1078
File: src/hooks/useKycFlow.ts:129-141
Timestamp: 2025-08-11T10:35:02.715Z
Learning: In the KYC flow implementation in `src/hooks/useKycFlow.ts`, when Terms of Service (ToS) is accepted, there will always be a KYC link available in the `apiResponse`. The system ensures this invariant, so defensive checks for missing KYC links after ToS acceptance are unnecessary.
Applied to files:
src/app/(mobile-ui)/withdraw/manteca/page.tsx
📚 Learning: 2024-10-07T15:28:25.280Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-10-07T15:28:25.280Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.
Applied to files:
src/app/(mobile-ui)/qr-pay/page.tsx
📚 Learning: 2025-09-08T03:13:09.111Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 1190
File: src/app/(mobile-ui)/qr-pay/page.tsx:156-176
Timestamp: 2025-09-08T03:13:09.111Z
Learning: In the peanut-ui mobile app, the `/qr-pay` route is only accessed through the DirectSendQR component which always includes the qrCode parameter in the URL when redirecting users to the QR pay page.
Applied to files:
src/app/(mobile-ui)/qr-pay/page.tsx
📚 Learning: 2025-09-08T03:13:09.111Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 1190
File: src/app/(mobile-ui)/qr-pay/page.tsx:156-176
Timestamp: 2025-09-08T03:13:09.111Z
Learning: In the peanut-ui mobile app, the `/qr-pay` route is only accessed through the DirectSendQR component which always includes the qrCode parameter in the URL when redirecting users to the QR pay page after scanning MERCADO_PAGO or PIX QR codes.
Applied to files:
src/app/(mobile-ui)/qr-pay/page.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 422
File: src/components/Request/Pay/Pay.tsx:113-123
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In the `PayRequestLink` component (`src/components/Request/Pay/Pay.tsx`), when resolving ENS names, handle errors by displaying an appropriate error message to the user if the ENS cannot be resolved.
Applied to files:
src/app/(mobile-ui)/qr-pay/page.tsx
🧬 Code graph analysis (5)
src/features/limits/utils/limits.utils.ts (1)
src/features/limits/hooks/useLimitsValidation.ts (1)
LimitCurrency(27-27)
src/app/(mobile-ui)/withdraw/page.tsx (4)
src/features/limits/hooks/useLimitsValidation.ts (1)
useLimitsValidation(41-279)src/features/limits/components/LimitsWarningCard.tsx (1)
LimitsWarningCard(31-84)src/features/limits/utils/limits.utils.ts (1)
LIMITS_COPY(6-11)src/utils/general.utils.ts (1)
formatExtendedNumber(708-756)
src/app/(mobile-ui)/withdraw/manteca/page.tsx (7)
src/features/limits/utils/limits.utils.ts (2)
mapToLimitCurrency(17-21)LIMITS_COPY(6-11)src/features/limits/hooks/useLimitsValidation.ts (1)
useLimitsValidation(41-279)src/constants/payment.consts.ts (1)
MIN_MANTECA_WITHDRAW_AMOUNT(12-12)src/constants/zerodev.consts.ts (1)
PEANUT_WALLET_TOKEN_DECIMALS(16-16)src/features/limits/components/LimitsWarningCard.tsx (1)
LimitsWarningCard(31-84)src/utils/general.utils.ts (1)
formatExtendedNumber(708-756)src/components/0_Bruddle/Button.tsx (1)
Button(80-283)
src/features/limits/components/LimitsWarningCard.tsx (3)
src/components/Global/Icons/Icon.tsx (2)
IconProps(151-154)Icon(292-301)src/context/ModalsContext.tsx (1)
useModalsContext(81-87)src/features/limits/utils/limits.utils.ts (1)
LIMITS_COPY(6-11)
src/app/(mobile-ui)/qr-pay/page.tsx (6)
src/hooks/useKycStatus.tsx (1)
useKycStatus(12-36)src/features/limits/hooks/useLimitsValidation.ts (1)
useLimitsValidation(41-279)src/features/limits/components/LimitsWarningCard.tsx (1)
LimitsWarningCard(31-84)src/features/limits/utils/limits.utils.ts (1)
LIMITS_COPY(6-11)src/utils/general.utils.ts (1)
formatExtendedNumber(708-756)src/constants/payment.consts.ts (1)
MAX_QR_PAYMENT_AMOUNT_FOREIGN(16-16)
🔇 Additional comments (21)
src/features/limits/utils/limits.utils.ts (2)
5-11: LGTM! Centralized copy for consistent user messaging.Good practice extracting UI strings into a centralized constant for consistency and maintainability across flows.
13-21: LGTM! Clean currency mapping implementation.The function correctly normalizes to uppercase and safely defaults unsupported currencies to USD. The type assertion is valid since the conditional guards ensure only valid values are returned.
src/features/limits/components/LimitsWarningCard.tsx (1)
66-79: LGTM! Support link integration.The support link is well implemented with proper modal integration and clear call-to-action text.
src/app/(mobile-ui)/qr-pay/page.tsx (3)
69-74: LGTM! Clean imports for limits integration.All necessary imports are properly added for the limits validation feature.
1565-1593: LGTM! Well-structured limits warning card integration.The implementation correctly:
- Hides balance errors when limits are blocking (avoiding duplicate error messages)
- Shows appropriate messaging for local vs foreign users
- Includes optional
daysUntilResetmessaging- Provides navigation to limits page
- Controls support link visibility based on user type
1618-1619: LGTM! Button correctly disabled when limits are blocking.The pay button is now properly disabled when
limitsValidation.isBlockingis true, preventing users from attempting transactions that exceed their limits.src/components/AddMoney/components/InputAmountStep.tsx (2)
11-13: LGTM! Appropriate imports for limits feature.Clean import additions for the limits warning card functionality.
112-119: LGTM! Correct disabled and error display logic.The button correctly disables when limits are blocking, and errors are appropriately hidden when the limits card is displayed to avoid redundant messaging.
src/app/(mobile-ui)/add-money/[country]/bank/page.tsx (4)
29-32: LGTM! Clean imports for limits integration.Appropriate imports added for the limits validation feature in the bank onramp flow.
104-110: LGTM! Correct limits validation setup for bridge flow.The hook is correctly configured:
flowType: 'onramp'matches this add-money flowcurrency: 'USD'is appropriate for bridge transactionsisLocalUser: falsecorrectly identifies this as a non-Manteca (bridge) flow
414-425: LGTM! Correct button disable and error display logic.The implementation properly:
- Disables the Continue button when limits are blocking
- Suppresses the ErrorAlert when the limits blocking card is shown to avoid duplicate error messaging
381-399: Bridge limits are per-transaction and intentionally excludedaysUntilReset.The omission is correct by design. The
useLimitsValidationhook explicitly documents this on line 101 with the comment// per-tx limit doesn't reset, returningdaysUntilReset: nullfor all bridge flow scenarios. Unlike Manteca's monthly limits (which includedaysUntilMonthlyReset), bridge limits have no reset period, so the field is appropriately absent from the card.src/app/(mobile-ui)/withdraw/manteca/page.tsx (5)
48-52: LGTM!Imports are correctly organized for the limits validation feature integration.
107-118: LGTM!The limits currency mapping and validation initialization correctly handles the manteca-specific flow with
isLocalUser: trueand the appropriate currency based on the selected country.
308-310: Good use of constants for minimum amount validation.Using
MIN_MANTECA_WITHDRAW_AMOUNTconstant improves maintainability. The comment clarifies the responsibility split between local validation (min + balance) and limits validation (max).Note: If the limits hook fails to load or returns no data, there's no fallback maximum check. The balance check (line 311) provides some protection, but consider whether a defensive local max fallback is warranted for resilience.
446-468: LGTM!The LimitsWarningCard integration is well-structured with:
- Appropriate type mapping (blocking → error, warning → warning)
- Dynamic content including remaining limit, reset days, and limits page link
- Support link visibility tied to manteca user status
488-491: LGTM!The conditional error rendering prevents duplicate error states while allowing warnings to coexist with balance errors. This is a reasonable UX decision.
src/app/(mobile-ui)/withdraw/page.tsx (4)
17-19: LGTM!Imports are correctly added for the limits validation feature.
85-92: LGTM!The limits validation is correctly initialized with:
flowType: 'offramp'for withdrawal flows- Dynamic
isLocalUserdetermination based on selected method type- USD currency appropriate for this general withdraw page
262-264: LGTM!The
showLimitsCardcomputation correctly excludes crypto withdrawals which don't have fiat limits.
299-321: LGTM!The LimitsWarningCard implementation is well-suited for this flow:
- USD formatting with
$prefix is appropriate since currency is hardcoded to USD- Conditional "per transaction" text helps clarify bridge limits vs. monthly limits
showSupportLink={false}is reasonable for a page serving multiple withdrawal methods
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Hugo0
left a comment
Hugo0
left a comment
There was a problem hiding this comment.
@kushagrasarathe i'm blocking this; i think this and the limit settings page both PRs are not up to par in quality. They have a lot of repetition, and imo are contributing inelegant bugprone code
| initialDenomination, | ||
| setDisplayedAmount, | ||
| limitsValidation, | ||
| limitsCurrency = 'USD', |
There was a problem hiding this comment.
Why is this one special? where does this default come from?? this will be a nightmare to debug!
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(mobile-ui)/qr-pay/page.tsx (1)
1582-1597: Block payments while limits are still loading.
If limits data hasn’t loaded yet,isBlockingstays false and users can pay before validation completes.🐛 Suggested fix
- limitsValidation.isBlocking + limitsValidation.isBlocking || limitsValidation.isLoading
🤖 Fix all issues with AI agents
In `@src/features/limits/hooks/useLimitsValidation.ts`:
- Around line 65-68: The current logic treats hasMantecaLimits as true whenever
mantecaLimits exists, causing code paths to use Manteca validation even when
relevantMantecaLimit is null for the current currency; update the
hasMantecaLimits/usage in useLimitsValidation so it only returns true (or takes
the Manteca path) when a matching relevantMantecaLimit exists (i.e., derive
hasMantecaLimits from relevantMantecaLimit !== null or update downstream
conditionals to check relevantMantecaLimit directly); ensure you adjust both the
initial computation around relevantMantecaLimit and the similar checks in the
later block referenced (around the 249-275 logic) to avoid skipping bridge
validation when no currency-specific Manteca limit is found.
- Around line 58-62: The numeric parsing in useLimitsValidation's numericAmount
useMemo incorrectly parses comma-formatted strings (e.g., "1,200") because
parseFloat stops at the comma; update the normalization to strip commas from the
string before parsing: in the numericAmount useMemo, when amount is a string
remove all commas (e.g., replace(/,/g, '')) then call parseFloat on the
sanitized string, keep existing branch for numeric input, and continue to return
0 for NaN; this change touches the numericAmount useMemo and the amount handling
logic to ensure limit checks are accurate.
♻️ Duplicate comments (2)
src/app/(mobile-ui)/withdraw/manteca/page.tsx (1)
468-468: Consider adding limitsValidation.isLoading to the disabled gate (already noted).src/app/(mobile-ui)/withdraw/page.tsx (1)
314-316: Consider disabling while limits are loading (already noted).
🧹 Nitpick comments (1)
src/features/limits/consts.ts (1)
11-20: Normalizeiso2casing when filteringcountryData.
MANTECA_COUNTRY_ISO2is uppercase, so a lowercasecountryData.iso2would result in an empty QR list. Normalizing avoids silent mismatches.♻️ Suggested change
-const derivedQrCountries = countryData - .filter((c) => c.type === 'country' && c.iso2 && MANTECA_COUNTRY_ISO2.includes(c.iso2)) +const derivedQrCountries = countryData + .filter( + (c) => + c.type === 'country' && + c.iso2 && + MANTECA_COUNTRY_ISO2.includes(c.iso2.toUpperCase()) + )Based on learnings, existing country code maps rely on uppercase ISO codes, so normalizing here is safer.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/(mobile-ui)/qr-pay/page.tsx (1)
1580-1602: Pay button doesn't respect limits loading state — potential bypass risk.The button's
disabledcondition includeslimitsValidation.isBlockingbut notlimitsValidation.isLoading. If the limits API is slow or the hook hasn't finished fetching, users can click "Pay" before validation completes, potentially bypassing limits.The PR objectives explicitly recommend: "disable actions while limitsValidation.isLoading is true."
🔒 Suggested fix
disabled={ !!errorInitiatingPayment || isBlockingError || !amount || isLoading || !!balanceErrorMessage || shouldBlockPay || !usdAmount || usdAmount === '0.00' || isWaitingForWebSocket || - limitsValidation.isBlocking + limitsValidation.isBlocking || + limitsValidation.isLoading }src/components/AddMoney/components/InputAmountStep.tsx (1)
1-110: Prettier check failed for this file.
Runpnpm prettier --write src/components/AddMoney/components/InputAmountStep.tsxto satisfy the pipeline warning.
🤖 Fix all issues with AI agents
In `@src/app/`(mobile-ui)/qr-pay/page.tsx:
- Around line 1555-1568: Currently balanceErrorMessage is hidden when
limitsValidation.isBlocking is true and LimitsWarningCard is rendered via an
inline IIFE; change this to surface both issues and make limits props computed
with useMemo: always render ErrorAlert when balanceErrorMessage exists (or
combine balanceErrorMessage into the limits card message) instead of skipping it
when limitsValidation.isBlocking is true, and replace the IIFE that calls
getLimitsWarningCardProps with a useMemo hook (e.g., compute limitsCardProps =
useMemo(() => getLimitsWarningCardProps({...}), [limitsValidation,
limitsCurrency]) ) and then render {limitsCardProps ? <LimitsWarningCard
{...limitsCardProps} /> : null}; use the existing symbols balanceErrorMessage,
limitsValidation.isBlocking, ErrorAlert, getLimitsWarningCardProps,
LimitsWarningCard, and useMemo.
In `@src/components/AddMoney/components/InputAmountStep.tsx`:
- Around line 106-107: The current conditional in InputAmountStep hides
ErrorAlert whenever limitsValidation?.isBlocking is true, which can mask
non-limit errors; change the rendering logic so ErrorAlert is shown for
non-limit errors even when a limits blocking card is displayed—for example,
render ErrorAlert unconditionally when error is truthy or split error sources by
showing ErrorAlert when error exists and the limits-related message only when
limitsValidation?.isBlocking is true; locate the ErrorAlert usage and the error
and limitsValidation?.isBlocking checks in InputAmountStep to implement this
separation.
- Around line 79-82: The code sets primaryDenomination using
currencyData.symbol! and currencyData.price!.buy which can throw if useCurrency
returns null/undefined; update the InputAmountStep logic to only assign
primaryDenomination when currencyData and both currencyData.symbol and
currencyData.price?.buy are present (guard against null), e.g., check
currencyData !== null and typeof currencyData.symbol === 'string' and
currencyData.price?.buy != null before creating the object, otherwise skip
setting primaryDenomination or provide a safe fallback; update any dependent
code in the component (e.g., wherever primaryDenomination is used) to handle the
absent case safely.
♻️ Duplicate comments (1)
src/app/(mobile-ui)/qr-pay/page.tsx (1)
391-401: Currency mapping concern already flagged; loading state not respected.The currency-to-limit mapping duplication was already noted in previous reviews. However, there's an additional concern:
limitsValidation.isLoadingis not checked before allowing payment actions. If the limits API is slow or fails, users could potentially bypass limits validation.Per the PR objectives, actions should be disabled while
limitsValidation.isLoadingis true to prevent race conditions.Verify the loading state handling and consider adding
limitsValidation.isLoadingto the button's disabled condition at line 1594.
🧹 Nitpick comments (2)
src/app/(mobile-ui)/qr-pay/page.tsx (1)
125-125: Unused destructured variable.
isUserMantecaKycApprovedis extracted fromuseKycStatus()but never used in this file. TheuseLimitsValidationhook already internally callsuseKycStatus()to determine user type. This redundant call can be removed.♻️ Suggested fix
- const { isUserMantecaKycApproved } = useKycStatus()If this was intentionally added for future use, consider adding a comment or removing until needed.
src/app/(mobile-ui)/withdraw/page.tsx (1)
260-308: Keep limits card currency in sync with validation.Use the normalized
limitsValidation.currencyto ensure the warning text matches the currency used by the validation logic.♻️ Proposed tweak
- currency: 'USD', + currency: limitsValidation.currency,
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(mobile-ui)/qr-pay/page.tsx (1)
1583-1595: Disable Pay while limits validation is loading.Right now Pay is only blocked on
limitsValidation.isBlocking. If limits are still loading, users can submit before validation is ready. Line 1583-1595.🐛 Proposed fix
- isWaitingForWebSocket || - limitsValidation.isBlocking + isWaitingForWebSocket || + limitsValidation.isLoading || + limitsValidation.isBlocking
♻️ Duplicate comments (2)
src/features/limits/hooks/useLimitsValidation.ts (2)
58-61: Fix numeric parsing for comma-formatted amounts (limits can be bypassed).
parseFloat("1,200")becomes1, which weakens limit enforcement. Normalize commas before parsing. Line 58-61.🐛 Proposed fix
- const numericAmount = useMemo(() => { - if (!amount) return 0 - const parsed = typeof amount === 'string' ? parseFloat(amount) : amount - return isNaN(parsed) ? 0 : parsed - }, [amount]) + const numericAmount = useMemo(() => { + if (amount === null || amount === undefined || amount === '') return 0 + const normalized = typeof amount === 'string' ? amount.replace(/,/g, '') : amount + const parsed = typeof normalized === 'string' ? parseFloat(normalized) : normalized + return isNaN(parsed) ? 0 : parsed + }, [amount])Based on learnings, TokenAmountInput returns decimal strings with commas, so normalization is required before parsing.
240-246: Avoid skipping QR limits when Manteca KYC exists but no matching limit.In the QR path, local users always take
mantecaValidation, which returns neutral whenrelevantMantecaLimitis null—so foreign QR limits never apply. This can allow bypassing limits for unsupported currencies. Line 240-246.🐛 Proposed fix
- if (flowType === 'qr-payment') { - // local users (manteca kyc) use manteca limits - if (isLocalUser && isUserMantecaKycApproved) { - return mantecaValidation - } - // foreign users have fixed per-tx limit - return foreignQrValidation - } + if (flowType === 'qr-payment') { + // local users (manteca kyc) use manteca limits only when a matching limit exists + if (isLocalUser && isUserMantecaKycApproved && relevantMantecaLimit) { + return mantecaValidation + } + // fallback to foreign per-tx limit when no relevant manteca limit exists + return foreignQrValidation + }
|
@coderabbitai review in depth |
|
Was there something specific from that review you'd like me to:
Or would you like me to trigger a fresh automated review of all the changes? If so, I can do that with the `` action tag. Please let me know what additional depth or focus area would be most helpful! 🔍 🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(mobile-ui)/withdraw/manteca/page.tsx (1)
301-308: Removal of local max-amount check creates single point of failure.The AI-generated summary and PR comments note this as a risk. If
useLimitsValidationfails to load, returns an error, or is slow, there's no fallback max-amount validation. The previous explicit max check provided defense-in-depth.Consider keeping a fallback local max check alongside the limits hook, as recommended in the PR objectives.
🛡️ Suggested fallback check
+const MAX_MANTECA_WITHDRAW_AMOUNT = 5000 // Fallback max if limits fail to load + // only check min amount and balance here - max amount is handled by limits validation if (paymentAmount < parseUnits(MIN_MANTECA_WITHDRAW_AMOUNT.toString(), PEANUT_WALLET_TOKEN_DECIMALS)) { setBalanceErrorMessage(`Withdraw amount must be at least $${MIN_MANTECA_WITHDRAW_AMOUNT}`) +} else if (!limitsValidation.remainingLimit && paymentAmount > parseUnits(MAX_MANTECA_WITHDRAW_AMOUNT.toString(), PEANUT_WALLET_TOKEN_DECIMALS)) { + // Fallback max check when limits validation hasn't loaded + setBalanceErrorMessage(`Withdraw amount cannot exceed $${MAX_MANTECA_WITHDRAW_AMOUNT}`) } else if (paymentAmount > balance) { setBalanceErrorMessage('Not enough balance to complete withdrawal.') } else { setBalanceErrorMessage(null) }
🤖 Fix all issues with AI agents
In `@src/app/`(mobile-ui)/qr-pay/page.tsx:
- Around line 1589-1590: The button's disabled logic currently uses
isWaitingForWebSocket || limitsValidation.isBlocking but misses the loading
state, so add limitsValidation.isLoading to the condition (i.e., disable when
limitsValidation.isLoading is true) so users can't proceed while limits are
still loading; update the component that renders the button (referencing
isWaitingForWebSocket, limitsValidation.isBlocking, limitsValidation.isLoading)
to include the loading flag in the disabled expression and any related aria-busy
or spinner logic if present.
In `@src/components/AddMoney/components/MantecaAddMoney.tsx`:
- Around line 71-77: The submission flow doesn't wait for limits data and
doesn't block when limits indicate restriction; update handleAmountSubmit to
first check limitsValidation.isLoading and limitsValidation.isBlocking and
prevent proceeding (show appropriate user feedback) while isLoading is true or
isBlocking is true, and ensure you re-run validation by including
limitsValidation (or its isLoading/isBlocking/validation result) in the
handleAmountSubmit dependency array so the handler uses the latest limits state;
reference the useLimitsValidation hook, the limitsValidation object, and the
handleAmountSubmit function when making these changes.
♻️ Duplicate comments (4)
src/features/limits/utils.ts (2)
77-79: Duplicate currency symbol mapping logic.This function duplicates logic already available in
SYMBOLS_BY_CURRENCY_CODE. While the fallback to uppercase currency code is reasonable, the existinguseCurrency.tsalready has this exact pattern. Based on learnings, there's an existing comment about code smell regarding repeated currency maps.
179-186: UX copy concern: message doesn't explain why the limit exists.Based on past review comments, the copy was flagged as not clearly communicating to the user that they are exceeding their limit. The current messages say "You can add/withdraw/pay up to X" but don't explain why they're seeing this message or that they've exceeded their limit.
Consider adding context like "You've reached your monthly limit" or "This amount exceeds your per-transaction limit" for better user understanding.
src/app/(mobile-ui)/qr-pay/page.tsx (1)
1551-1564: Error masking and IIFE pattern concerns persist.Two issues from past review that should be addressed:
Error masking: When
limitsValidation.isBlockingis true, balance errors are hidden. Users with both issues will only see limits errors first.IIFE pattern: The inline IIFE for rendering
LimitsWarningCardis non-idiomatic. AuseMemowould be cleaner.♻️ Suggested refactor using useMemo
+ // Compute limits card props once + const limitsCardProps = useMemo(() => { + return getLimitsWarningCardProps({ + validation: limitsValidation, + flowType: 'qr-payment', + currency: limitsValidation.currency, + }) + }, [limitsValidation]) {/* only show balance error if limits blocking card is not displayed (warnings can coexist) */} {balanceErrorMessage && !limitsValidation.isBlocking && ( <ErrorAlert description={balanceErrorMessage} /> )} {/* Limits Warning/Error Card */} - {(() => { - const limitsCardProps = getLimitsWarningCardProps({ - validation: limitsValidation, - flowType: 'qr-payment', - currency: limitsValidation.currency, - }) - return limitsCardProps ? <LimitsWarningCard {...limitsCardProps} /> : null - })()} + {limitsCardProps && <LimitsWarningCard {...limitsCardProps} />}src/app/(mobile-ui)/withdraw/manteca/page.tsx (1)
463-463: Button doesn't disable during limits loading - addressed in past review.This was flagged in a past review comment. The fix should include
limitsValidation.isLoadingin the disabled condition.🔧 Suggested fix
-disabled={!Number(usdAmount) || !!balanceErrorMessage || limitsValidation.isBlocking} +disabled={!Number(usdAmount) || !!balanceErrorMessage || limitsValidation.isBlocking || limitsValidation.isLoading}
🧹 Nitpick comments (4)
src/components/AddMoney/components/MantecaAddMoney.tsx (1)
206-219: Limits integration looks correct; minor redundancy noted.The
limitsValidationobject is properly passed toInputAmountStep. ThelimitsCurrencyprop is derived fromlimitsValidation.currency, which works but is slightly redundant sinceInputAmountStepcould access it directly fromlimitsValidation.currency.If
InputAmountSteponly needs the currency for display purposes independent of the full validation object, consider whether both props are necessary or if one could be derived internally.src/features/limits/utils.ts (2)
67-71: Currency mapping may have unintended fallback behavior.The function defaults any unrecognized currency to
'USD'. This is noted in the AI-generated summary as a potential edge case concern. Consider whether this silent fallback could hide bugs when an unexpected currency is passed (e.g., 'MXN', 'EUR'). Adding a console warning in development mode could help surface such cases.
179-186: Complex ternary expressions reduce readability.The currency symbol formatting logic is repeated three times with identical complex ternary expressions. This is hard to read and maintain.
♻️ Suggested refactor to extract helper
+ // Helper to format currency amount consistently + const formatCurrencyDisplay = (symbol: string, amount: string) => { + if (symbol === '$') return `$${amount}` + return `${symbol} ${amount}` + } + // build the limit message based on flow type let limitMessage = '' if (flowType === 'onramp') { - limitMessage = `You can add up to ${currencySymbol === '$' ? '' : currencySymbol + ' '}${currencySymbol === '$' ? '$' : ''}${formattedLimit}${validation.daysUntilReset ? '' : ' per transaction'}` + limitMessage = `You can add up to ${formatCurrencyDisplay(currencySymbol, formattedLimit)}${validation.daysUntilReset ? '' : ' per transaction'}` } else if (flowType === 'offramp') { - limitMessage = `You can withdraw up to ${currencySymbol === '$' ? '' : currencySymbol + ' '}${currencySymbol === '$' ? '$' : ''}${formattedLimit}${validation.daysUntilReset ? '' : ' per transaction'}` + limitMessage = `You can withdraw up to ${formatCurrencyDisplay(currencySymbol, formattedLimit)}${validation.daysUntilReset ? '' : ' per transaction'}` } else { // qr-payment - limitMessage = `You can pay up to ${currencySymbol === '$' ? '' : currencySymbol + ' '}${currencySymbol === '$' ? '$' : ''}${formattedLimit} per transaction` + limitMessage = `You can pay up to ${formatCurrencyDisplay(currencySymbol, formattedLimit)} per transaction` }src/app/(mobile-ui)/withdraw/manteca/page.tsx (1)
439-448: IIFE pattern for limits card - same issue as QR pay flow.This duplicates the non-idiomatic IIFE pattern. Consider using
useMemofor consistency.♻️ Suggested refactor using useMemo
+ const limitsCardProps = useMemo(() => { + return getLimitsWarningCardProps({ + validation: limitsValidation, + flowType: 'offramp', + currency: limitsValidation.currency, + }) + }, [limitsValidation]) - {/* limits warning/error card - uses centralized helper for props */} - {(() => { - const limitsCardProps = getLimitsWarningCardProps({ - validation: limitsValidation, - flowType: 'offramp', - currency: limitsValidation.currency, - }) - return limitsCardProps ? <LimitsWarningCard {...limitsCardProps} /> : null - })()} + {/* limits warning/error card - uses centralized helper for props */} + {limitsCardProps && <LimitsWarningCard {...limitsCardProps} />}
| isWaitingForWebSocket || | ||
| limitsValidation.isBlocking |
There was a problem hiding this comment.
Consider disabling button while limits are loading.
Similar to the concern raised in the withdraw flow, the disabled condition doesn't account for limitsValidation.isLoading. If limits data hasn't loaded yet, isBlocking will be false, potentially allowing the user to proceed before validation completes.
🔧 Suggested fix
isWaitingForWebSocket ||
- limitsValidation.isBlocking
+ limitsValidation.isBlocking ||
+ limitsValidation.isLoading🤖 Prompt for AI Agents
In `@src/app/`(mobile-ui)/qr-pay/page.tsx around lines 1589 - 1590, The button's
disabled logic currently uses isWaitingForWebSocket ||
limitsValidation.isBlocking but misses the loading state, so add
limitsValidation.isLoading to the condition (i.e., disable when
limitsValidation.isLoading is true) so users can't proceed while limits are
still loading; update the component that renders the button (referencing
isWaitingForWebSocket, limitsValidation.isBlocking, limitsValidation.isLoading)
to include the loading flag in the disabled expression and any related aria-busy
or spinner logic if present.
| // validates deposit amount against user's limits | ||
| // currency comes from country config - hook normalizes it internally | ||
| const limitsValidation = useLimitsValidation({ | ||
| flowType: 'onramp', | ||
| amount: usdAmount, | ||
| currency: selectedCountry?.currency, | ||
| }) |
There was a problem hiding this comment.
Consider guarding submission on limits validation loading and blocking state.
The useLimitsValidation hook provides isLoading and isBlocking properties, but handleAmountSubmit (lines 146-185) doesn't check these before proceeding with the deposit. This creates a potential race condition where a user could submit while limits data is still loading, or submit an amount that exceeds their limits.
Suggested fix in handleAmountSubmit
const handleAmountSubmit = useCallback(async () => {
if (!selectedCountry?.currency) return
if (isCreatingDeposit) return
+ if (limitsValidation.isLoading) return
+ if (limitsValidation.isBlocking) return
// check if we still need to determine KYC statusAlso update the dependency array:
-}, [currentDenomination, selectedCountry, displayedAmount, isMantecaKycRequired, isCreatingDeposit, setUrlState])
+}, [currentDenomination, selectedCountry, displayedAmount, isMantecaKycRequired, isCreatingDeposit, setUrlState, limitsValidation.isLoading, limitsValidation.isBlocking])🤖 Prompt for AI Agents
In `@src/components/AddMoney/components/MantecaAddMoney.tsx` around lines 71 - 77,
The submission flow doesn't wait for limits data and doesn't block when limits
indicate restriction; update handleAmountSubmit to first check
limitsValidation.isLoading and limitsValidation.isBlocking and prevent
proceeding (show appropriate user feedback) while isLoading is true or
isBlocking is true, and ensure you re-run validation by including
limitsValidation (or its isLoading/isBlocking/validation result) in the
handleAmountSubmit dependency array so the handler uses the latest limits state;
reference the useLimitsValidation hook, the limitsValidation object, and the
handleAmountSubmit function when making these changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/app/`(mobile-ui)/add-money/[country]/bank/page.tsx:
- Around line 132-138: The Continue button can be clicked before limits
validation finishes; update the button's disabled condition to also check
limitsValidation.isLoading so actions are disabled while validation is pending.
Locate the Continue button render (the element that currently checks things like
canContinue or any existing disabled flag around the Continue/submit action) and
add limitsValidation.isLoading to the boolean expression (e.g., disabled={... ||
limitsValidation.isLoading}), ensuring you reference the useLimitsValidation
result (limitsValidation) used earlier in this file; no other behavior changes
needed besides preventing clicks while loading.
- Around line 118-130: The usdEquivalent calculation and the payment button need
to account for exchange-rate fetch failures: destructure isError from
useExchangeRate where exchangeRate is obtained, update the useMemo for
usdEquivalent (function named usdEquivalent that reads rawTokenAmount,
localCurrency, exchangeRate) to treat a failed fetch (isError true) as an error
case (e.g., return NaN or throw/flag) instead of silently returning 0, and
update the button disabled logic to include isError alongside isRateLoading so
the button is disabled (or an error UI is shown) when the rate fetch failed;
reference the useExchangeRate hook, the usdEquivalent useMemo, rawTokenAmount,
localCurrency, exchangeRate, and the button's disabled prop to find all places
to change.
🧹 Nitpick comments (1)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsx (1)
409-418: Consider memoizing limits card props.The IIFE pattern works but recalculates
getLimitsWarningCardPropson every render when the card is shown. For slightly cleaner code and consistency with other memoized values, consider extracting this to auseMemo.Suggested refactor
+ const limitsCardProps = useMemo(() => { + if (!limitsValidation.isBlocking && !limitsValidation.isWarning) return null + return getLimitsWarningCardProps({ + validation: limitsValidation, + flowType: 'onramp', + currency: 'USD', + }) + }, [limitsValidation]) + if (urlState.step === 'inputAmount') { - const showLimitsCard = limitsValidation.isBlocking || limitsValidation.isWarning - return ( // ... - {showLimitsCard && - (() => { - const limitsCardProps = getLimitsWarningCardProps({ - validation: limitsValidation, - flowType: 'onramp', - currency: 'USD', - }) - return limitsCardProps ? <LimitsWarningCard {...limitsCardProps} /> : null - })()} + {limitsCardProps && <LimitsWarningCard {...limitsCardProps} />}
| // convert input amount to USD for limits validation | ||
| // bridge limits are always in USD, but user inputs in local currency | ||
| const usdEquivalent = useMemo(() => { | ||
| if (!rawTokenAmount) return 0 | ||
| const numericAmount = parseFloat(rawTokenAmount.replace(/,/g, '')) | ||
| if (isNaN(numericAmount)) return 0 | ||
|
|
||
| // for USD, no conversion needed | ||
| if (localCurrency === 'USD') return numericAmount | ||
|
|
||
| // convert local currency to USD | ||
| return exchangeRate > 0 ? numericAmount * exchangeRate : 0 | ||
| }, [rawTokenAmount, localCurrency, exchangeRate]) |
There was a problem hiding this comment.
Handle exchange rate fetch errors.
When exchangeRate is 0 due to a fetch error (not just loading), usdEquivalent will be 0, potentially bypassing limits validation for non-USD currencies. The button's disabled state only guards against isRateLoading, not isError.
Consider also destructuring isError from useExchangeRate and either disabling the button or showing an error when the rate fetch fails.
Suggested approach
// get exchange rate: local currency → USD (for limits validation)
// skip for USD since it's 1:1
- const { exchangeRate, isLoading: isRateLoading } = useExchangeRate({
+ const { exchangeRate, isLoading: isRateLoading, isError: isRateError } = useExchangeRate({
sourceCurrency: localCurrency,
destinationCurrency: 'USD',
enabled: localCurrency !== 'USD',
})Then in the button disabled state (line 437):
- (localCurrency !== 'USD' && isRateLoading)
+ (localCurrency !== 'USD' && (isRateLoading || isRateError))🤖 Prompt for AI Agents
In `@src/app/`(mobile-ui)/add-money/[country]/bank/page.tsx around lines 118 -
130, The usdEquivalent calculation and the payment button need to account for
exchange-rate fetch failures: destructure isError from useExchangeRate where
exchangeRate is obtained, update the useMemo for usdEquivalent (function named
usdEquivalent that reads rawTokenAmount, localCurrency, exchangeRate) to treat a
failed fetch (isError true) as an error case (e.g., return NaN or throw/flag)
instead of silently returning 0, and update the button disabled logic to include
isError alongside isRateLoading so the button is disabled (or an error UI is
shown) when the rate fetch failed; reference the useExchangeRate hook, the
usdEquivalent useMemo, rawTokenAmount, localCurrency, exchangeRate, and the
button's disabled prop to find all places to change.
| // validate against user's bridge limits | ||
| // uses USD equivalent to correctly compare against USD-denominated limits | ||
| const limitsValidation = useLimitsValidation({ | ||
| flowType: 'onramp', | ||
| amount: usdEquivalent, | ||
| currency: 'USD', | ||
| }) |
There was a problem hiding this comment.
Guard against limits loading state.
The limitsValidation.isLoading state is not used to disable the Continue button. This creates a race condition where users could proceed before limits validation completes, potentially bypassing blocking limits.
Per the PR review comments recommendation: "Disable actions while limitsValidation.isLoading is true."
Suggested fix in button disabled state (around line 431)
disabled={
!parseFloat(rawTokenAmount) ||
parseFloat(rawTokenAmount) < minimumAmount ||
error.showError ||
isCreatingOnramp ||
limitsValidation.isBlocking ||
- (localCurrency !== 'USD' && isRateLoading)
+ (localCurrency !== 'USD' && isRateLoading) ||
+ limitsValidation.isLoading
}🤖 Prompt for AI Agents
In `@src/app/`(mobile-ui)/add-money/[country]/bank/page.tsx around lines 132 -
138, The Continue button can be clicked before limits validation finishes;
update the button's disabled condition to also check limitsValidation.isLoading
so actions are disabled while validation is pending. Locate the Continue button
render (the element that currently checks things like canContinue or any
existing disabled flag around the Continue/submit action) and add
limitsValidation.isLoading to the boolean expression (e.g., disabled={... ||
limitsValidation.isLoading}), ensuring you reference the useLimitsValidation
result (limitsValidation) used earlier in this file; no other behavior changes
needed besides preventing clicks while loading.
|
@coderabbitai review latest changes, especially in regard to hugos commetns |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3142 🏁 Script executed: Length of output: 1072 🏁 Script executed: Length of output: 28748
✅ Issues Addressed1. DRY Violations Fixed (Comments
|
| Issue | Status | Notes |
|---|---|---|
| DRY violations | ✅ Fixed | getLimitsWarningCardProps centralized |
| Currency mapping | ✅ Fixed | mapToLimitCurrency utility |
| isLocalUser code smell | ✅ Fixed | Now internal to hook |
| Currency display (ARS/USD) | ✅ Fixed | Exchange rate conversion added |
| Copy quality | Still doesn't explain limit context well | |
| limitsCurrency default | Required prop with comment, but could be optional |
🔧 Remaining Recommendations
- Improve copy messaging - explain the "why" to users
- Make limitsCurrency optional - derive from validation when available
- Add loading state checks - ensure buttons don't enable prematurely while limits load
The major architectural concerns (DRY, currency handling, validation logic) have been addressed well. The remaining issues are UX polish and minor API design improvements.
🧠 Learnings used
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 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.
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 594
File: src/components/Request/Pay/Views/Initial.view.tsx:188-213
Timestamp: 2024-12-31T15:08:15.640Z
Learning: Always resolve coderabbit comments when reviewing code, since they can contain useful suggestions.
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 823
File: src/context/kernelClient.context.tsx:79-86
Timestamp: 2025-04-29T19:36:38.121Z
Learning: When Hugo0 asks to "resolve coderabbit comments", they want to acknowledge the comment without necessarily implementing the suggested changes, as the current implementation might be intentional for their specific use case.
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 942
File: src/components/AddMoney/consts/index.ts:2151-2162
Timestamp: 2025-06-30T10:44:08.048Z
Learning: Hugo0 often agrees with refactoring suggestions but defers implementation due to time constraints, preferring to track improvements as follow-up issues when they're part of larger architectural changes.
2 participants
