fixes: pesos currency #854
Conversation
|
""" WalkthroughThis update refactors the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TokenAmountInput
participant FormattingUtils
User->>TokenAmountInput: Types or edits input
TokenAmountInput->>TokenAmountInput: Updates inputValue state
TokenAmountInput->>FormattingUtils: Format input for display
FormattingUtils-->>TokenAmountInput: Returns formatted string
TokenAmountInput-->>User: Displays formatted value
User->>TokenAmountInput: Clicks currency toggle or max button
TokenAmountInput->>TokenAmountInput: Toggles mode or sets max, updates inputValue and tokenValue
TokenAmountInput->>FormattingUtils: Format new value
FormattingUtils-->>TokenAmountInput: Returns formatted string
TokenAmountInput-->>User: Updates input display
Possibly related PRs
Suggested reviewers
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/Global/TokenAmountInput/index.tsx (3)
46-68: Avoid extra renders by memoising formatted value
formatNumberForDisplayis executed on every render even iftokenValue,currencyMode, etc. did not actually change the formatted result. Since the result is deterministic for the given dependency tuple, consider wrapping the formatting block inuseMemoand only updatingdisplayValuewhen the memoised value changes.
This reduces re-renders/DOM writes when the parent ofTokenAmountInputitself re-renders frequently.
208-216: Key filtering blocks common editing/navigation keysThe current filter prevents
Enter,Escape,Home,End, and comma – the latter makes pasting formatted numbers impossible. Users on macOS will also expectCmd+A / Cmd+Vto work in all cases (already mostly covered, but still brittle).Consider switching to
inputMode="decimal"+ pattern validation and drop manualonKeyDownfiltering, or at least whitelist the additional keys:-!['Backspace','Delete','ArrowLeft','ArrowRight','ArrowUp','ArrowDown','Tab'].includes(key) +!['Backspace','Delete','ArrowLeft','ArrowRight','ArrowUp','ArrowDown','Tab','Enter','Escape','Home','End',','].includes(key)
233-238: Potential rounding issues when showing wallet balance
walletBalanceis cast toNumber, which loses precision for large token balances (common with 18-dec tokens). Displayed balances may be off by fractions of a cent.Prefer the same arbitrary-precision utility suggested above or, at minimum,
BigInt/BigNumberfor the intermediate multiplication:-Number(walletBalance) * currency.price +new BigNumber(walletBalance).multipliedBy(currency.price).toString()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Global/TokenAmountInput/index.tsx(5 hunks)src/components/Payment/Views/Status.payment.view.tsx(1 hunks)src/components/TransactionDetails/TransactionCard.tsx(3 hunks)src/components/TransactionDetails/transactionTransformer.ts(1 hunks)src/utils/general.utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/components/Global/TokenAmountInput/index.tsx (3)
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.
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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 `''`.
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.
🧬 Code Graph Analysis (1)
src/components/TransactionDetails/TransactionCard.tsx (4)
src/components/Global/Badges/StatusBadge.tsx (1)
StatusType(4-4)src/components/Global/Card/index.tsx (1)
CardPosition(4-4)src/components/TransactionDetails/transactionTransformer.ts (1)
TransactionDetails(15-41)src/utils/general.utils.ts (1)
formatNumberForDisplay(248-259)
🔇 Additional comments (8)
src/utils/general.utils.ts (1)
244-259: Well-designed utility function for consistent number formattingThe new
formatNumberForDisplayfunction provides a clean approach to standardize number formatting across the application. I appreciate the handling of edge cases (undefined, empty, NaN), the configurable decimal places, and the forward-looking comment about internationalization.This function will help address the missing thousands separators in ARS input fields mentioned in the PR objectives.
src/components/Payment/Views/Status.payment.view.tsx (1)
57-66: Good UX improvement by removing automatic redirectThe removal of automatic redirect after payment completion is a positive change. It allows users (especially those with external wallets) to properly view their payment confirmation before manually proceeding to the home screen.
This addresses one of the key objectives in the PR regarding disrupted user experience after payment completion.
src/components/TransactionDetails/transactionTransformer.ts (1)
81-81: Improved clarity in transaction labelingChanging the fallback text from "Payment via public link" to "Requested via Link" provides a more accurate description of the transaction type, aligning with the PR objective to improve transaction labels.
src/components/TransactionDetails/TransactionCard.tsx (4)
9-12: Appropriate imports for the new implementationGood choice importing
formatNumberForDisplayand the necessary enums to support the refined transaction amount display logic.
27-31: Useful helper function for currency symbol standardizationThe new
getDisplayCurrencySymbolhelper function centralizes the logic for currency symbol display, making it easier to maintain and extend for additional currencies in the future.
58-87: Comprehensive fix for currency display issuesThis overhaul of the currency display logic properly addresses the issue with incorrect plus/minus signs for ARS transactions mentioned in the PR objectives. The approach is thorough and considers:
- Different currencies (especially ARS vs USD)
- Transaction direction based on type and user role
- Proper formatting with thousands separators
The conditional logic is clear and well-structured, making the code maintainable.
119-119: Simplified rendering logicUsing the precomputed
finalDisplayAmountvariable simplifies the JSX rendering and makes the code more readable.src/components/Global/TokenAmountInput/index.tsx (1)
100-108: Hard-coding 6 decimals can under-represent many ERC-20 tokens
parseFloat(usdEquivalent.toFixed(6))limits the on-chain value to 6 d.p. Tokens such as USDC (6) are fine, but 18-dec tokens (ETH, WBTC, etc.) lose precision and may round to zero for small transfers.Prefer using the token’s native decimals:
-const usdEquivalent = customCurrencyNumber / currency.price -setTokenValue(parseFloat(usdEquivalent.toFixed(6)).toString()) +const usdEquivalent = customCurrencyNumber.div(currency.price) +setTokenValue(usdEquivalent.toFixed(selectedTokenData?.decimals ?? 18))
| const onChange = (rawInputValue: string) => { | ||
| const numberStrWithoutCommas = rawInputValue.replace(/,/g, '') | ||
|
|
||
| if (numberStrWithoutCommas === '') { | ||
| setDisplayValue('') | ||
| setTokenValue('') | ||
| if (setCurrencyAmount) setCurrencyAmount('') | ||
| return | ||
| } | ||
|
|
||
| if (/^\d*\.?\d*$/.test(numberStrWithoutCommas)) { | ||
| let formattedForDisplay | ||
| const currentMaxDecimals = currency ? 2 : (selectedTokenData?.decimals ?? 6) | ||
|
|
||
| if (numberStrWithoutCommas.endsWith('.') && (numberStrWithoutCommas.match(/\./g) || []).length === 1) { | ||
| const partBeforeDecimal = numberStrWithoutCommas.slice(0, -1) | ||
| formattedForDisplay = | ||
| formatNumberForDisplay(partBeforeDecimal, { maxDecimals: currentMaxDecimals }) + '.' | ||
| } else { | ||
| formattedForDisplay = formatNumberForDisplay(numberStrWithoutCommas, { | ||
| maxDecimals: currentMaxDecimals, | ||
| }) | ||
| } | ||
|
|
||
| if (formattedForDisplay === '' && numberStrWithoutCommas === '.') { | ||
| setDisplayValue('.') | ||
| } else { | ||
| setDisplayValue(formattedForDisplay) | ||
| } | ||
|
|
||
| if (currency && currencyMode === 'CUSTOM') { | ||
| if (setCurrencyAmount) setCurrencyAmount(formattedForDisplay) | ||
| const customCurrencyNumber = Number(numberStrWithoutCommas) | ||
| if (!isNaN(customCurrencyNumber) && currency.price > 0) { | ||
| const usdEquivalent = customCurrencyNumber / currency.price | ||
| setTokenValue(parseFloat(usdEquivalent.toFixed(6)).toString()) | ||
| } else { | ||
| setTokenValue('') | ||
| } | ||
| } else { | ||
| setTokenValue(numberStrWithoutCommas) | ||
| if (currency && currency.price > 0) { | ||
| const customEquivalent = Number(numberStrWithoutCommas) * currency.price | ||
| if (setCurrencyAmount) | ||
| setCurrencyAmount(formatNumberForDisplay(customEquivalent.toString(), { maxDecimals: 2 })) | ||
| } else if (!currency && setCurrencyAmount && selectedTokenData?.price && selectedTokenData.price > 0) { | ||
| const usdEquivalent = Number(numberStrWithoutCommas) * selectedTokenData.price | ||
| setCurrencyAmount(formatNumberForDisplay(usdEquivalent.toString(), { maxDecimals: 2 })) | ||
| } else if (setCurrencyAmount) { | ||
| setCurrencyAmount('') | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Number + floating-point math introduces precision loss for currency values
All conversions here (Number(…), / currency.price, parseFloat(...).toFixed(6), * currency.price, etc.) rely on IEEE-754 number, which will truncate/round amounts above 2¹⁵³ or with >15 significant digits – typical with on-chain 18-dec tokens.
Consider switching to an arbitrary-precision library (e.g. bignumber.js or ethers.BigNumber) for every monetary path:
-import { ... } from '@/utils'
+import { ... } from '@/utils'
+import BigNumber from 'bignumber.js'-const customCurrencyNumber = Number(numberStrWithoutCommas)
-...
-const usdEquivalent = customCurrencyNumber / currency.price
-setTokenValue(parseFloat(usdEquivalent.toFixed(6)).toString())
+const customCurrencyNumber = new BigNumber(numberStrWithoutCommas)
+...
+const usdEquivalent = customCurrencyNumber.div(currency.price)
+setTokenValue(usdEquivalent.toFixed(selectedTokenData?.decimals ?? 18))This ensures ARS ↔ USD and token conversions do not silently lose sub-cent amounts.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/Global/TokenAmountInput/index.tsx between lines 70 and 122,
the code uses JavaScript's Number type and floating-point arithmetic for
currency conversions, which causes precision loss for large or highly precise
token amounts. Replace all Number conversions and arithmetic operations
involving currency and token values with an arbitrary-precision library like
bignumber.js or ethers.BigNumber. Update calculations such as division,
multiplication, and toFixed formatting to use the library's methods to maintain
precision in all monetary conversions.
There was a problem hiding this comment.
can we use viem bignum or js native bigint
There was a problem hiding this comment.
🧩 Analysis chain
@Hugo0 Yes, you can use viem's bigint utilities for this case since they're designed specifically for handling crypto token amounts with proper decimal precision. Viem provides formatters like parseUnits and formatUnits that work well with decimal values.
Native JavaScript BigInt would be problematic here because it can only represent integers (no decimal points), which doesn't work for currency values with cents or token amounts with decimals.
I recommend using viem if it's already in your project dependencies, as it will properly handle the conversions while maintaining precision for both fiat currencies (like ARS) and tokens.
🏁 Script executed:
#!/bin/bash
# Check if viem is a dependency in package.json
grep -E '"viem":' package.json || echo "viem not found in package.json"
# Check if viem is imported anywhere in the codebase
rg "import .* from ['\"]viem['\"]" --type ts --type tsx
# Look for any bigint usage in the codebase
rg "BigInt" --type ts --type tsx
Length of output: 239
@Hugo0 Yes, you can use viem's bigint utilities since viem is already in your project dependencies ("viem": "^2.21.48"). This would be my recommended approach because:
- Viem provides
parseUnitsandformatUnitsfunctions specifically designed for handling token decimal precision - These functions properly handle the conversion between human-readable decimals and on-chain integer representations
Native JavaScript BigInt would be problematic for your currency conversions because it can only represent integers (no decimal points), which doesn't work well for amounts with fractional components.
For implementation, you could replace the current Number-based conversions with something like:
// Instead of:
const customCurrencyNumber = Number(numberStrWithoutCommas)
const usdEquivalent = customCurrencyNumber / currency.price
setTokenValue(parseFloat(usdEquivalent.toFixed(6)).toString())
// Using viem:
import { parseUnits, formatUnits } from 'viem'
const decimals = selectedTokenData?.decimals ?? 6
const customCurrencyBigInt = parseUnits(numberStrWithoutCommas, 2) // ARS uses 2 decimals
const priceBigInt = parseUnits(currency.price.toString(), decimals)
const usdEquivalent = formatUnits((customCurrencyBigInt * BigInt(10**decimals)) / priceBigInt, decimals)
setTokenValue(usdEquivalent)This approach will maintain precision throughout all your currency conversions.
🧬 Code Graph Analysis Results
Relevant snippets for understanding src/components/Global/TokenAmountInput/index.tsx and the user comment about using viem bignum or JS native bigint:
File: src/constants/zerodev.consts.ts (line 20)
export const PEANUT_WALLET_TOKEN = '0xaf88d065e77c8cc2239327c5edb3a432268e5831' // USDC Arbitrum address- Defines the constant address for the Peanut Wallet token (USDC on Arbitrum).
- Used in
TokenAmountInputto check if the selected token address matches this token.
File: src/context/tokenSelector.context.tsx (lines 25-45)
export const tokenSelectorContext = createContext({
selectedTokenAddress: '',
selectedChainID: '',
selectedTokenDecimals: 0 as number | undefined,
setSelectedTokenDecimals: (decimals: number | undefined) => {},
setSelectedTokenAddress: (address: string) => {},
setSelectedChainID: (chainID: string) => {},
updateSelectedChainID: (chainID: string) => {},
selectedTokenPrice: 0 as number | undefined,
setSelectedTokenPrice: (price: number | undefined) => {},
inputDenomination: 'TOKEN' as inputDenominationType,
setInputDenomination: (denomination: inputDenominationType) => {},
refetchXchainRoute: false as boolean,
setRefetchXchainRoute: (value: boolean) => {},
resetTokenContextProvider: () => {},
isXChain: false as boolean,
setIsXChain: (value: boolean) => {},
selectedTokenData: undefined as ITokenPriceData | undefined,
isFetchingTokenData: false as boolean,
supportedSquidChainsAndTokens: {} as Record<string, interfaces.ISquidChain & { tokens: interfaces.ISquidToken[] }>,
})- Provides context for token selection state, including selected token address, decimals, price, and input denomination.
selectedTokenDatacontains token metadata such as decimals and price, used inTokenAmountInputfor formatting and conversions.
File: src/utils/general.utils.ts (lines 248-259)
export const formatNumberForDisplay = (valueStr: string | undefined, options?: { maxDecimals?: number }): string => {
if (valueStr === undefined || valueStr === null || valueStr.trim() === '') return ''
const num = Number(valueStr)
if (isNaN(num)) return ''
const maxDecimals = options?.maxDecimals ?? 2 // Default to 2 if not specified
return num.toLocaleString('en-US', {
minimumFractionDigits: 0,
maximumFractionDigits: maxDecimals,
})
}- Formats a numeric string for display with a configurable maximum number of decimals.
- Used extensively in
TokenAmountInputto format token and currency amounts for user display.
File: src/utils/general.utils.ts (lines 322-343)
export function formatTokenAmount(amount?: number, maxFractionDigits?: number) {
if (amount === undefined) return undefined
maxFractionDigits = maxFractionDigits ?? 6
// floor the amount
const flooredAmount = Math.floor(amount * Math.pow(10, maxFractionDigits)) / Math.pow(10, maxFractionDigits)
// Convert number to string to count significant digits
const amountString = flooredAmount.toFixed(maxFractionDigits)
const significantDigits = amountString.replace(/^0+\./, '').replace(/\.$/, '').replace(/0+$/, '').length
// Calculate the number of fraction digits needed to have at least two significant digits
const fractionDigits = Math.max(2 - significantDigits, 0)
// Format the number with the calculated fraction digits
const formattedAmount = flooredAmount.toLocaleString('en-US', {
minimumFractionDigits: fractionDigits,
maximumFractionDigits: maxFractionDigits,
})
return formattedAmount
}- Formats a token amount number with a floor operation and dynamic fraction digits to ensure at least two significant digits.
- Used in
TokenAmountInputto display token amounts with appropriate precision.
File: src/utils/general.utils.ts (lines 704-711)
export const estimateIfIsStableCoinFromPrice = (tokenPrice: number) => {
// if the tokenprice is between .995 and 1.005, return 1
if (tokenPrice >= 0.995 && tokenPrice <= 1.005) {
return true
} else {
return false
}
}- Utility to estimate if a token is a stablecoin based on its price being close to 1.
- Used in
TokenAmountInputto adjust display logic for stablecoins.
File: src/hooks/wallet/useWallet.ts (lines 20-82)
export const useWallet = () => {
const dispatch = useAppDispatch()
const { address, isKernelClientReady, handleSendUserOpEncoded } = useZeroDev()
const { balance } = useWalletStore()
const sendTransactions = useCallback(
async (unsignedTxs: peanutInterfaces.IPeanutUnsignedTransaction[], chainId?: string) => {
const params = unsignedTxs.map((tx: peanutInterfaces.IPeanutUnsignedTransaction) => ({
to: tx.to! as Hex,
value: tx.value?.valueOf() ?? 0n,
data: (tx.data as Hex | undefined) ?? '0x',
}))
chainId = chainId ?? PEANUT_WALLET_CHAIN.id.toString()
let receipt = await handleSendUserOpEncoded(params, chainId)
return receipt
},
[handleSendUserOpEncoded]
)
const fetchBalance = useCallback(async () => {
if (!address) {
console.warn('Cannot fetch balance, address is undefined.')
return
}
const balance = await peanutPublicClient.readContract({
address: PEANUT_WALLET_TOKEN,
abi: erc20Abi,
functionName: 'balanceOf',
args: [address as Hex],
})
dispatch(walletActions.setBalance(balance))
}, [address, dispatch])
const getRewardWalletBalance = useCallback(async () => {
if (!address) {
console.warn('Cannot fetch reward balance, address is undefined.')
return ''
}
const balance = await pintaPublicClient.readContract({
address: PINTA_WALLET_TOKEN,
abi: erc20Abi,
functionName: 'balanceOf',
args: [getAddress(address)],
})
const formatedBalance = formatAmount(formatUnits(balance, PINTA_WALLET_TOKEN_DECIMALS))
dispatch(walletActions.setRewardWalletBalance(formatedBalance))
}, [address, dispatch])
useEffect(() => {
if (!address) return
fetchBalance()
getRewardWalletBalance()
}, [address, fetchBalance, getRewardWalletBalance])
return {
address: address!,
balance: BigInt(balance),
isConnected: isKernelClientReady,
sendTransactions,
getRewardWalletBalance,
fetchBalance,
}
}- Custom React hook managing wallet state and interactions.
- Uses
BigIntfor balance values (balance: BigInt(balance)). - Shows usage of native JS
BigIntfor balances. - Relevant to the user comment about using viem bignum or JS native bigint.
Summary of relevance:
- The
TokenAmountInputcomponent currently usesstringandnumbertypes for token amounts and conversions, with formatting utilities that rely on JSNumberand string formatting. - The wallet hook
useWalletuses native JSBigIntfor balance values. - No direct usage of viem bignum or other big number libraries is visible in
TokenAmountInput. - Utilities for formatting numbers (
formatNumberForDisplay,formatTokenAmount) use JSNumberandtoLocaleString. - The user comment about using viem bignum or JS native bigint relates to how token amounts and balances are handled and formatted in this component and possibly the wallet hook.
If further investigation or code changes are needed regarding big number handling, these snippets provide the context of current usage of number types and formatting in the relevant files.
🧠 Learnings used
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.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Global/TokenAmountInput/index.tsx (1)
70-122:⚠️ Potential issue
Number+ floating-point math introduces precision loss for currency valuesAll conversions here (
Number(…),/ currency.price,parseFloat(...).toFixed(6),* currency.price, etc.) rely on IEEE-754number, which can lead to precision issues with currency values.Consider using a decimal math library like
bignumber.jsfor all currency conversions:import { estimateIfIsStableCoinFromPrice, formatTokenAmount, formatNumberForDisplay, formatAmount } from '@/utils' +import BigNumber from 'bignumber.js'- const customCurrencyNumber = Number(numberStrWithoutCommas) - if (!isNaN(customCurrencyNumber) && currency.price > 0) { - const usdEquivalent = customCurrencyNumber / currency.price - setTokenValue(parseFloat(usdEquivalent.toFixed(6)).toString()) + const customCurrencyNumber = new BigNumber(numberStrWithoutCommas) + if (!customCurrencyNumber.isNaN() && currency.price > 0) { + const usdEquivalent = customCurrencyNumber.dividedBy(currency.price) + setTokenValue(usdEquivalent.toFixed(6))
🧹 Nitpick comments (2)
src/components/Global/TokenAmountInput/index.tsx (2)
46-68: UseuseMemoinstead of recalculating inuseEffectThe display value calculation logic would benefit from being moved to a
useMemohook instead of recalculating inuseEffect, which would better follow React best practices and potentially reduce rerenders.-useEffect(() => { +const newDisplayValueBasedOnTokenValue = useMemo(() => { let newDisplayValueBasedOnTokenValue = '' if (currency && currencyMode === 'CUSTOM') { if (tokenValue && currency.price > 0) { const customCurrencyValue = Number(tokenValue) * currency.price newDisplayValueBasedOnTokenValue = formatNumberForDisplay(customCurrencyValue.toString(), { maxDecimals: 2, }) } else { newDisplayValueBasedOnTokenValue = '' } } else { newDisplayValueBasedOnTokenValue = tokenValue ? formatNumberForDisplay(tokenValue, { maxDecimals: primaryDisplayMaxDecimals }) : '' } + return newDisplayValueBasedOnTokenValue +}, [tokenValue, currencyMode, currency, primaryDisplayMaxDecimals]) +useEffect(() => { if (displayValue.endsWith('.') && newDisplayValueBasedOnTokenValue === displayValue.slice(0, -1)) { // Do nothing; preserve user's partial decimal input } else if (newDisplayValueBasedOnTokenValue !== displayValue) { setDisplayValue(newDisplayValueBasedOnTokenValue) } -}, [tokenValue, currencyMode, currency, displayValue, primaryDisplayMaxDecimals]) +}, [newDisplayValueBasedOnTokenValue, displayValue])
125-139: Add error handling for invalid max valueThe max click handler should protect against NaN or invalid inputs that could result from an invalid
maxValue.const handleMaxClick = (e: React.MouseEvent) => { e.preventDefault() e.stopPropagation() if (maxValue) { + if (isNaN(Number(maxValue))) { + console.warn('Invalid maxValue provided to TokenAmountInput') + return + } setTokenValue(maxValue) if (currency && currency.price > 0) { const customEquivalent = Number(maxValue) * currency.price
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Global/TokenAmountInput/index.tsx(5 hunks)src/components/TransactionDetails/TransactionCard.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/TransactionDetails/TransactionCard.tsx
🧰 Additional context used
🧠 Learnings (1)
src/components/Global/TokenAmountInput/index.tsx (3)
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.
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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 `''`.
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.
🔇 Additional comments (5)
src/components/Global/TokenAmountInput/index.tsx (5)
38-42: Input handling improvements are excellent!The switch to always using
type="text"withinputMode="decimal"combined with a separatedisplayValuestate provides better control over formatting while maintaining a good mobile experience. This decoupling allows for proper thousands separators without disrupting the editing experience.Also applies to: 70-122
83-92: Good handling of decimal point during inputThe special case handling for decimal points during input preserves the user's input state when typing, which greatly improves the user experience. This is a thoughtful addition.
167-174: Clean currency mode toggle refactoringExtracting the toggle logic into a separate function eliminates duplication and centralizes the mode switching logic. This is a good refactoring that improves maintainability.
208-228: Excellent input filtering with detailed commentsThe key filtering logic is thorough and well-documented. It properly handles numeric input, single decimal points, and essential control keys while preventing invalid characters.
258-310: Comprehensive converted amount display with clear documentationThe secondary display logic handles all conversion scenarios clearly with detailed comments. This makes the code much more maintainable and easier to understand.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/components/Global/TokenAmountInput/index.tsx (1)
127-150: Floating-point conversions still introduce precision lossAll arithmetic here (
parseFloat,* currency.price,/ currency.price,toFixed(...)) uses IEEE-754number.
Values such as0.1 * 1e18or 18-dec token amounts lose sub-cent precision, the exact bug reported in past reviews (see previous comment thread).Please switch to an arbitrary-precision approach (e.g.
viem’sparseUnits/formatUnitsorbignumber.js) for every monetary calculation.
🧹 Nitpick comments (4)
src/utils/format.utils.ts (1)
80-85: Fallback formatting should respectmaximumFractionDigitsWhen
Intl.NumberFormatthrows, the fallback always usesminDigits.
UsingtoFixed(minDigits)can show too many decimals if the caller deliberately set a lowermaxDigits.-return `${currencyCode} ${numericValue.toFixed(minDigits)}` +return `${currencyCode} ${numericValue.toFixed(maxDigits)}`src/components/Global/TokenAmountInput/index.tsx (3)
217-224:onBlurmutates state twice – potential race with async React updatesCalling
setInputValueand immediatelyonChange(which itself callssetInputValue&setTokenValue) may cause stale closures or double renders.Instead, compute
cleanedonce and call onlyonChange:-if (inputValue.endsWith('.')) { - const cleaned = inputValue.slice(0, -1) || '' - setInputValue(cleaned) - onChange(cleaned) -} +if (inputValue.endsWith('.')) { + onChange(inputValue.slice(0, -1)) +}
327-340: Key-filter logic blocks ‘E’/‘+’/‘-’ but allows Ctrl/Meta variationsThe intent is to prevent scientific notation. However, users can still paste
"1e6"or"1e+6".
Consider sanitising pasted content inonPasteinstead of relying solely ononKeyDown.
355-375: Wallet balance formatting duplicates complex branchingThis 40-line ternary forest is hard to read and error-prone. Extract a helper:
const formatBalance = () => { if (isNaN(numericWalletBalance)) return walletBalance ... }Then call
{formatBalance()}in JSX.
Improves readability and unit-testability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Global/TokenAmountInput/index.tsx(3 hunks)src/components/Payment/PaymentForm/index.tsx(2 hunks)src/utils/format.utils.ts(1 hunks)src/utils/general.utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/general.utils.ts
🧰 Additional context used
🧠 Learnings (2)
src/components/Payment/PaymentForm/index.tsx (3)
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#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.
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.
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.
src/components/Global/TokenAmountInput/index.tsx (2)
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.
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#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 `''`.
🧬 Code Graph Analysis (1)
src/components/Global/TokenAmountInput/index.tsx (3)
src/redux/slices/send-flow-slice.ts (1)
setTokenValue(48-50)src/utils/format.utils.ts (1)
formatCurrencyWithIntl(39-87)src/utils/general.utils.ts (1)
estimateIfIsStableCoinFromPrice(735-742)
| const numericValue = typeof value === 'string' ? parseFloat(value) : value | ||
| if (isNaN(numericValue)) return '' // Return empty if not a valid number | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strip thousands separators before parseFloat to avoid silent mis-parsing
parseFloat('1,234.56') stops at the comma and returns 1, which will silently corrupt values entered with thousands separators – exactly the problem this PR tries to fix elsewhere.
Sanitise the string first:
-const numericValue = typeof value === 'string' ? parseFloat(value) : value
+const numericValue = typeof value === 'string'
+ ? parseFloat(value.replace(/,/g, ''))
+ : value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const numericValue = typeof value === 'string' ? parseFloat(value) : value | |
| if (isNaN(numericValue)) return '' // Return empty if not a valid number | |
| const numericValue = typeof value === 'string' | |
| ? parseFloat(value.replace(/,/g, '')) | |
| : value | |
| if (isNaN(numericValue)) return '' // Return empty if not a valid number |
🤖 Prompt for AI Agents
In src/utils/format.utils.ts around lines 48 to 50, the code uses parseFloat
directly on strings that may contain thousands separators like commas, causing
incorrect parsing. To fix this, sanitize the input string by removing all
thousands separators (e.g., commas) before calling parseFloat, ensuring the
numeric value is correctly parsed without silent corruption.
| // Ensure minDigits is not greater than maxDigits | ||
| minDigits = Math.min(minDigits, maxDigits) | ||
|
|
There was a problem hiding this comment.
minDigits = Math.min(minDigits, maxDigits) risks NaN when both are undefined
If the caller passes both minimumFractionDigits and maximumFractionDigits as undefined and style !== 'currency', the earlier branch leaves them undefined; Math.min(undefined, undefined) returns NaN, later triggering RangeError in Intl.NumberFormat.
Guard explicitly:
-if (minDigits === undefined) minDigits = 0
-if (maxDigits === undefined) maxDigits = 20
+minDigits = minDigits ?? 0
+maxDigits = maxDigits ?? 20Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/format.utils.ts around lines 69 to 71, the assignment minDigits =
Math.min(minDigits, maxDigits) can result in NaN if both minDigits and maxDigits
are undefined, causing a RangeError later. To fix this, add explicit checks to
ensure minDigits and maxDigits are defined before calling Math.min, and provide
default values or handle the undefined case to prevent NaN from being assigned.
| const maxAllowedDecimals = currency || inputDenomination === 'USD' ? 2 : (selectedTokenData?.decimals ?? 6) | ||
|
|
||
| // Convert to actual token value for payment processing | ||
| if (currency && currencyMode === 'CUSTOM') { | ||
| // Convert from custom currency to token value | ||
| const actualTokenValue = formatTokenAmount(Number(inputValue) / currency.price)! | ||
| setCurrencyAmount?.(inputValue) | ||
| setTokenValue(actualTokenValue.toString()) | ||
| if (maxAllowedDecimals === 0 && sanitizedValue.includes('.')) { | ||
| sanitizedValue = sanitizedValue.split('.')[0] | ||
| } else if (parts.length === 2 && parts[1].length > maxAllowedDecimals) { | ||
| sanitizedValue = `${parts[0]}.${parts[1].substring(0, maxAllowedDecimals)}` | ||
| } | ||
|
|
There was a problem hiding this comment.
maxAllowedDecimals can exceed the 20-digit limit of Intl.NumberFormat
When a token has decimals > 20 (some have 24+), the later call to formatCurrencyWithIntl will throw a RangeError.
Clamp the value:
-const maxAllowedDecimals = currency || inputDenomination === 'USD'
- ? 2
- : (selectedTokenData?.decimals ?? 6)
+const DECIMAL_LIMIT = 20
+const maxAllowedDecimals = currency || inputDenomination === 'USD'
+ ? 2
+ : Math.min(selectedTokenData?.decimals ?? 6, DECIMAL_LIMIT)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const maxAllowedDecimals = currency || inputDenomination === 'USD' ? 2 : (selectedTokenData?.decimals ?? 6) | |
| // Convert to actual token value for payment processing | |
| if (currency && currencyMode === 'CUSTOM') { | |
| // Convert from custom currency to token value | |
| const actualTokenValue = formatTokenAmount(Number(inputValue) / currency.price)! | |
| setCurrencyAmount?.(inputValue) | |
| setTokenValue(actualTokenValue.toString()) | |
| if (maxAllowedDecimals === 0 && sanitizedValue.includes('.')) { | |
| sanitizedValue = sanitizedValue.split('.')[0] | |
| } else if (parts.length === 2 && parts[1].length > maxAllowedDecimals) { | |
| sanitizedValue = `${parts[0]}.${parts[1].substring(0, maxAllowedDecimals)}` | |
| } | |
| const DECIMAL_LIMIT = 20 | |
| const maxAllowedDecimals = currency || inputDenomination === 'USD' | |
| ? 2 | |
| : Math.min(selectedTokenData?.decimals ?? 6, DECIMAL_LIMIT) | |
| if (maxAllowedDecimals === 0 && sanitizedValue.includes('.')) { | |
| sanitizedValue = sanitizedValue.split('.')[0] | |
| } else if (parts.length === 2 && parts[1].length > maxAllowedDecimals) { | |
| sanitizedValue = `${parts[0]}.${parts[1].substring(0, maxAllowedDecimals)}` | |
| } |
🤖 Prompt for AI Agents
In src/components/Global/TokenAmountInput/index.tsx around lines 137 to 144, the
maxAllowedDecimals value can exceed 20, which causes a RangeError in
Intl.NumberFormat. Fix this by clamping maxAllowedDecimals to a maximum of 20
before using it in formatting functions. This ensures the decimal precision does
not exceed Intl.NumberFormat's supported limit.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/TransactionDetails/TransactionCard.tsx (3)
27-32: Consider moving the currency symbol helper to utilsThe helper function
getDisplayCurrencySymbolis well-implemented but as noted in the comment, it would be better placed in a utils file for reuse across the application. This would promote code reuse and maintainability.- // Helper function to get currency symbol based on code - can be moved to utils if used elsewhere - const getDisplayCurrencySymbol = (code?: string, fallbackSymbol: string = '$'): string => { - if (code === 'ARS') return 'AR$' - if (code === 'USD') return '$' - return fallbackSymbol - }And add to an appropriate utils file:
// In src/utils/currency.utils.ts or similar export const getDisplayCurrencySymbol = (code?: string, fallbackSymbol: string = '$'): string => { if (code === 'ARS') return 'AR$' if (code === 'USD') return '$' return fallbackSymbol }
94-99: Consider extracting token decimal logic to a helper functionThe decimal precision determination is a bit complex and could be extracted to a helper function for better readability and maintainability.
- const decimalsForDisplay = actualCurrencyCode // If it's a known currency (USD, ARS) - ? defaultDisplayDecimals - : transaction.extraDataForDrawer?.originalType === EHistoryEntryType.SEND_LINK // Example: check token specific decimals if available - ? ((transaction.extraDataForDrawer as any)?.tokenDecimalsForDisplay ?? 6) // Fallback to 6 for tokens - : 6 // General fallback for other tokens + const decimalsForDisplay = getDisplayDecimals(transaction, actualCurrencyCode, defaultDisplayDecimals)And add a helper function:
const getDisplayDecimals = ( transaction: TransactionDetails, currencyCode?: string, defaultCurrencyDecimals: number = 2 ): number => { if (currencyCode) { return defaultCurrencyDecimals; } // For tokens, check if we have specific decimals if (transaction.extraDataForDrawer?.originalType === EHistoryEntryType.SEND_LINK) { return (transaction.extraDataForDrawer as any)?.tokenDecimalsForDisplay ?? 6; } // General fallback for other tokens return 6; }
97-97: Avoid type casting withas anywhen possibleUsing
as anybypasses TypeScript's type checking, which can lead to runtime errors. Consider adding proper type definitions for the extraDataForDrawer object to include tokenDecimalsForDisplay.- ? ((transaction.extraDataForDrawer as any)?.tokenDecimalsForDisplay ?? 6) // Fallback to 6 for tokens + ? (transaction.extraDataForDrawer?.tokenDecimalsForDisplay ?? 6) // Fallback to 6 for tokensYou would need to update the TransactionDetails interface to include this property:
// In the TransactionDetails interface extraDataForDrawer?: { originalType: EHistoryEntryType originalUserRole: EHistoryUserRole link?: string isLinkTransaction?: boolean transactionCardType?: TransactionCardType tokenDecimalsForDisplay?: number // Add this property }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/TransactionDetails/TransactionCard.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/TransactionDetails/TransactionCard.tsx (4)
src/components/Global/Badges/StatusBadge.tsx (1)
StatusType(4-4)src/components/Global/Card/index.tsx (1)
CardPosition(4-4)src/components/TransactionDetails/transactionTransformer.ts (1)
TransactionDetails(15-41)src/utils/general.utils.ts (1)
formatNumberForDisplay(248-290)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (5)
src/components/TransactionDetails/TransactionCard.tsx (5)
9-9: Updated imports to support improved currency formattingThe imports now include
formatNumberForDisplayand transaction history enums which are essential for the new currency formatting logic. This is a good change that aligns with the PR's objective to standardize currency formatting.Also applies to: 12-12
60-63: Good initialization for currency display logicSetting up the display variables and handling special cases like JPY's zero decimal places is a good practice. The code now correctly prepares for the different currency formatting requirements.
64-85: Improved sign handling for ARS transactionsThis change properly addresses the incorrect plus/minus signs issue mentioned in the PR objectives. The sign is now determined based on the transaction type and user role, ensuring consistent financial notation (debits as negative, credits as positive).
86-105: Enhanced currency and token amount formattingThe refactored code appropriately handles different scenarios:
- Token transactions without fiat values
- Currency transactions with appropriate symbols
- Different decimal precision for different currency types
This implementation addresses the precision and formatting issues mentioned in the PR objectives.
140-140: Simplified amount display in JSXUsing the pre-computed
finalDisplayAmountvariable in the JSX makes the template cleaner and more maintainable compared to having inline formatting logic.
Issues this PR tackles
Currency Input Issues
Missing Thousands Separators in ARS Input
Currency Conversion Precision Loss
History View Sign Issues
Automatic Redirect After Payment
Home Screen Transaction Labels
Summary of Changes
This PR significantly polishes currency handling, focusing on robust input behavior, correct decimal precision for both fiat (ARS, USD) and cryptocurrencies, and accurate display formatting in inputs and transaction history.
Key Enhancements & Fixes:
TokenAmountInput.tsx(Currency Input Component):currencyprop) correctly display with 2 decimal places. Direct token inputs (e.g., ETH, BTC) now respect their nativeselectedTokenData.decimalsfor display in the input field (defaulting to 6), allowing for necessary high precision.type="text"withinputMode="decimal"for all currency inputs. This is a common fintech practice to enable custom formatting (like thousands separators) while suggesting a numeric keyboard on mobile devices.tokenValuemaintains appropriate precision (e.g., 6 decimals for USD equivalents from ARS, full precision for direct token inputs).TransactionCard.tsx(Transaction History List Item):+or-signs for both ARS and USD amounts based on transaction type and user role.formatNumberForDisplay) for thousands separators and appropriate decimal places (2 for fiat in this context).general.utils.ts(Utility Functions):formatNumberForDisplayenhanced to accept amaxDecimalsoption, providing flexibility for fiat (default 2) vs. crypto display needs. It standardizes onen-USlocale for consistent, parsable output (comma-thousands, period-decimal) suitable for input field display and simple list views.Status.payment.view.tsx(Payment Flow):existing bugs or things we didnt think about:
/pay/hugotest124/10?currency=ARSleads to AR$ 11,660 payment (base is 10usd). But it should be 10 ars and 0.000xx usd. Else merchants cant make a specific ARS payment
BEFORE:
Screencast from 2025-05-14 23-46-51.webm
AFTER:
Screencast from 2025-05-15 00-26-43.webm