[TASK-6758] Hide token selector on peanut wallet and default token #551
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request involves the removal of the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
docs/high_level_overview.md
Outdated
| ### Hooks | ||
| The `/hooks` folder contains two custom hooks to help manage wallet functionality: | ||
| The `/hooks` folder contains a custom hook to help manage wallet functionality: |
There was a problem hiding this comment.
Q:
- why remove this section?
- Do we even need these docs? I feel like they're prone to getting outdated / potentially misleading
There was a problem hiding this comment.
I think these docs are outdated and frankly I've never read them before. I removed this because ir removed the useBalance hook
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (8)
src/utils/balance.utils.ts (1)
29-29: Specify radix inparseIntto avoid unexpected resultsWhen using
parseInt, it's good practice to specify the radix parameter to avoid unexpected parsing behavior.Apply this diff:
-decimals: parseInt(item.quantity.decimals), +decimals: parseInt(item.quantity.decimals, 10),src/components/Cashout/Components/Initial.view.tsx (1)
219-225: IncludesetSelectedChainIDandsetSelectedTokenAddressinuseEffectdependenciesThe
useEffectdepends onsetSelectedChainIDandsetSelectedTokenAddressfunctions, but they are not included in the dependency array. This might lead to stale closures or unintended behavior. Include these functions in the dependency array to ensure the effect runs correctly when they change.Apply this diff to include the dependencies:
-useEffect(() => { +useEffect(() => { if (isPeanutWallet) { setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString()) setSelectedTokenAddress(PEANUT_WALLET_TOKEN) } -}, [isPeanutWallet]) +}, [isPeanutWallet, setSelectedChainID, setSelectedTokenAddress])src/components/Claim/Link/Initial.view.tsx (1)
Line range hint
459-473: Setautocompleteattribute for bank account inputs as per previous learningTo enhance user experience and align with best practices, set
autocomplete="bank-account-number"on theGeneralRecipientInputcomponent when therecipientTypeis'us'or'iban', as advised in the previous learning.This applies the learning:
Learning: For bank account input fields, use
autocomplete="bank-account-number"when the recipient type is'us'or'iban'.Apply this diff:
<GeneralRecipientInput className="" placeholder="wallet address / ENS / IBAN / US account number" recipient={recipient} onUpdate={(update: GeneralRecipientUpdate) => { // existing code }} + autocomplete={recipientType === 'us' || recipientType === 'iban' ? 'bank-account-number' : undefined} infoText={TOOLTIPS.CLAIM_RECIPIENT_INFO} />src/components/Global/TokenSelector/Components/AdvancedButton.tsx (1)
99-102: Consider improving readability of conditional renderingWhile the logic is correct, the nested ternary with null could be more readable.
Consider restructuring like this:
- (tokenBalance ? ( - <p className="text-xs text-gray-1"> - Balance: {utils.formatTokenAmount(tokenBalance ?? 0, 4)} - </p> - ) : null)} + tokenBalance && ( + <p className="text-xs text-gray-1"> + Balance: {utils.formatTokenAmount(tokenBalance, 4)} + </p> + )}This approach:
- Eliminates the nested ternary
- Removes redundant null check since we're already checking for tokenBalance
- Maintains the same functionality with cleaner syntax
src/components/Global/ChainSelector/index.tsx (1)
34-37: Consider adding error boundary for balance calculationsWhile the memoization is good for performance, consider adding error handling for edge cases where balance calculations might fail.
const valuePerChain = useMemo( - () => calculateValuePerChain(selectedWallet?.balances ?? []), + () => { + try { + return calculateValuePerChain(selectedWallet?.balances ?? []) + } catch (error) { + console.error('Failed to calculate chain values:', error) + return [] + } + }, [selectedWallet?.balances] )src/components/Claim/Link/Onchain/Confirm.view.tsx (1)
28-28: LGTM! Consider handling potential refetch errors.The addition of
refetchBalancesensures wallet balances are updated after a successful claim. However, since this is an async operation, consider handling potential errors to prevent them from being silently ignored.-refetchBalances(address ?? '') +try { + await refetchBalances(address ?? '') +} catch (error) { + console.error('Failed to refresh balances:', error) + // Non-blocking error - don't show to user as the claim was successful +}Also applies to: 83-83
src/components/Create/Link/Confirm.view.tsx (1)
75-75: LGTM! Consider handling potential refetch errors for consistency.The addition of
refetchBalancesensures wallet balances are updated after successful link creation. For consistency with other components, consider handling potential errors.-refetchBalances(address ?? '') +try { + await refetchBalances(address ?? '') +} catch (error) { + console.error('Failed to refresh balances:', error) + // Non-blocking error - don't show to user as the link creation was successful +}Also applies to: 220-220
src/components/Create/Link/Input.view.tsx (1)
331-344: Consider adding loading state for balances.The conditional rendering for external wallet token selector is good, but consider adding a loading state while balances are being fetched to prevent UI flicker.
-{isExternalWallet && ( +{isExternalWallet && ( <> <TokenSelector classNameButton="w-full" /> - {selectedWallet!.balances!.length === 0 && ( + {selectedWallet?.balances ? ( + selectedWallet.balances.length === 0 && ( <div onClick={() => { open() }} className="cursor-pointer text-h9 underline" > ( Buy Tokens ) </div> + ) + ) : ( + <div className="text-h9">Loading balances...</div> )} </> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (23)
docs/high_level_overview.md(1 hunks)src/app/(mobile-ui)/wallet/page.tsx(2 hunks)src/components/Cashout/Components/Initial.view.tsx(7 hunks)src/components/Claim/Link/Initial.view.tsx(6 hunks)src/components/Claim/Link/Onchain/Confirm.view.tsx(2 hunks)src/components/Create/Create.tsx(0 hunks)src/components/Create/Link/Confirm.view.tsx(2 hunks)src/components/Create/Link/Input.view.tsx(6 hunks)src/components/Create/useCreateLink.tsx(7 hunks)src/components/Global/ChainSelector/index.tsx(3 hunks)src/components/Global/TokenSelector/Components/AdvancedButton.tsx(1 hunks)src/components/Global/TokenSelector/TokenSelector.tsx(3 hunks)src/components/Home/WalletCard.tsx(1 hunks)src/components/Home/WalletToggleButton.tsx(2 hunks)src/components/Request/Create/Views/Initial.view.tsx(5 hunks)src/components/Request/Pay/Views/Initial.view.tsx(5 hunks)src/constants/general.consts.ts(0 hunks)src/constants/zerodev.consts.ts(1 hunks)src/context/walletContext/walletContext.tsx(7 hunks)src/hooks/useBalance.tsx(0 hunks)src/interfaces/wallet.interfaces.ts(1 hunks)src/utils/balance.utils.ts(1 hunks)src/utils/index.ts(1 hunks)
💤 Files with no reviewable changes (3)
- src/constants/general.consts.ts
- src/hooks/useBalance.tsx
- src/components/Create/Create.tsx
🧰 Additional context used
📓 Learnings (2)
src/components/Global/TokenSelector/TokenSelector.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#424
File: src/components/Global/TokenSelector/TokenSelector.tsx:197-211
Timestamp: 2024-11-12T09:39:20.720Z
Learning: In `src/components/Global/TokenSelector/TokenSelector.tsx`, when the calculation within functions like `byChainAndText` is not computationally expensive, it's acceptable to avoid using `useCallback` for memoization.
src/components/Claim/Link/Initial.view.tsx (1)
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#545
File: src/components/Global/GeneralRecipientInput/index.tsx:118-126
Timestamp: 2024-11-26T12:06:11.603Z
Learning: For bank account input fields, use `autocomplete="bank-account-number"` when the recipient type is `'us'` or `'iban'`.
🪛 Gitleaks (8.21.2)
src/constants/zerodev.consts.ts
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (20)
src/utils/balance.utils.ts (1)
86-88:
Avoid unnecessary chain ID splitting in calculateValuePerChain
In the calculateValuePerChain function, balance.chainId has already been processed and does not require splitting. The line const chainId = balance?.chainId ? balance.chainId.split(':')[1] : '1' may lead to incorrect chain IDs. Consider using const chainId = balance.chainId directly.
Apply this diff to fix the issue:
-const chainId = balance?.chainId ? balance.chainId.split(':')[1] : '1'
+const chainId = balance.chainIdLikely invalid or redundant comment.
src/utils/index.ts (1)
5-5: LGTM!
The new export makes the utility functions in balance.utils.ts accessible, and the change is appropriate.
src/constants/zerodev.consts.ts (1)
15-16: LGTM! Verify USDC contract address.
The addition of PEANUT_WALLET_TOKEN constant aligns with the PR objective. The address appears to be the USDC contract on Arbitrum.
Let's verify the contract address:
✅ Verification successful
USDC Arbitrum address verified across codebase
The USDC contract address (0xaf88d065e77c8cc2239327c5edb3a432268e5831) is consistently used across the codebase:
- Used in
zerodev.consts.tsasPEANUT_WALLET_TOKEN - Referenced in
cashout.consts.ts - Properly associated with Arbitrum network (PEANUT_WALLET_CHAIN)
The address matches the official USDC contract on Arbitrum and is used consistently throughout the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other occurrences of this USDC address to ensure consistency
rg -i "0xaf88d065e77c8cc2239327c5edb3a432268e5831"
Length of output: 265
Script:
#!/bin/bash
# Let's check the context of USDC address usage in cashout.consts.ts
rg -B 3 -A 3 "0xaf88d065e77c8cc2239327c5edb3a432268e5831" src/constants/cashout.consts.ts
# Also check if there are any other USDC-related constants or comments
rg -i "usdc" src/constants/
Length of output: 740
Script:
#!/bin/bash
# Let's verify this address against a known source
# First, let's check if there's any documentation or config mentioning Arbitrum USDC
rg -i -B 2 -A 2 "arbitrum.*usdc|usdc.*arbitrum"
# Also check for any test files or documentation that might validate this address
fd -e md -e test.ts -e spec.ts | xargs rg -i "usdc.*arbitrum|arbitrum.*usdc"
Length of output: 438
🧰 Tools
🪛 Gitleaks (8.21.2)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
src/components/Home/WalletToggleButton.tsx (1)
Line range hint 8-17: LGTM! Good performance optimization.
The conversion of toggleWallet to use useCallback is a good optimization since the function is used in the onClick event handler. The dependency array is complete and correctly includes all referenced values.
src/interfaces/wallet.interfaces.ts (1)
32-32: LGTM! Verify IUserBalance interface.
The addition of the optional balances property to IWallet interface is well-structured and maintains backward compatibility.
Let's verify the IUserBalance interface exists:
✅ Verification successful
IUserBalance interface exists and is well-defined
The IUserBalance interface is properly defined in src/interfaces/interfaces.ts with all necessary fields for representing wallet balances including chain ID, token details, amounts and values.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for IUserBalance interface definition
ast-grep --pattern 'interface IUserBalance {
$$$
}'
Length of output: 666
src/components/Home/WalletCard.tsx (1)
39-45: LGTM: Card content structure simplified
The removal of the Link wrapper while maintaining the onClick handler aligns with the PR's objective to streamline wallet interactions. The content remains clear and actionable.
src/app/(mobile-ui)/wallet/page.tsx (1)
12-12: LGTM: Improved balance display with proper formatting
The balance display now properly uses formatUnits for USDC (6 decimals) with appropriate fallback handling and consistent decimal places.
Also applies to: 43-45
src/components/Global/ChainSelector/index.tsx (2)
10-11: LGTM! Good refactor to use centralized wallet context
The change from useBalance hook to useWallet context with utility functions improves code organization and maintainability.
150-150: LGTM! Proper formatting of token amounts
Good use of the formatTokenAmount utility for consistent display of values.
docs/high_level_overview.md (1)
130-132: Consider expanding wallet management documentation
While removing useBalance documentation aligns with the code changes, consider adding more context about the wallet context and its role in balance management.
src/components/Request/Create/Views/Initial.view.tsx (2)
31-40: LGTM! Good use of wallet context
Clean implementation of wallet context with proper destructuring of required properties.
186-186: LGTM! Proper conditional rendering of TokenSelector
Good implementation of hiding token selector for peanut wallet, aligning with PR objectives.
src/components/Create/Link/Input.view.tsx (2)
268-273: LGTM! Robust balance calculation implementation.
The maxValue calculation is well implemented with proper null checks and formatting.
290-295: LGTM! Verify chain/token defaults for peanut wallet.
The automatic setting of chain and token for peanut wallet improves UX, but let's verify the constants are correctly defined.
✅ Verification successful
Let me gather more information about the arbitrum constant to ensure it's correctly defined.
Let me modify the search pattern to find the arbitrum chain definition.
Let me try one more search to find where arbitrum is imported or defined.
Let me verify the USDC token address on Arbitrum.
Chain and token defaults for peanut wallet are correctly configured
The verification confirms:
- PEANUT_WALLET_CHAIN is set to Arbitrum mainnet (imported from viem/chains)
- PEANUT_WALLET_TOKEN is set to the official USDC contract address on Arbitrum (0xaf88d065e77c8cc2239327c5edb3a432268e5831)
- The chain configuration is properly used in the viem client setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PEANUT_WALLET constants are defined correctly
# Check for constant definitions
rg -A 1 "PEANUT_WALLET_CHAIN|PEANUT_WALLET_TOKEN" "src/constants/"
Length of output: 645
Script:
#!/bin/bash
# Check arbitrum chain definition and any imports
rg -A 3 "^export const arbitrum = {" src/constants/
Length of output: 169
Script:
#!/bin/bash
# Check arbitrum chain definition and any imports
rg -A 5 "const arbitrum =" src/constants/
Length of output: 43
Script:
#!/bin/bash
# Search for arbitrum in any context
rg -B 2 -A 2 "arbitrum" src/constants/
Length of output: 1699
Script:
#!/bin/bash
# Search for any other references to the USDC token address
rg "0xaf88d065e77c8cc2239327c5edb3a432268e5831" -B 2 -A 2
Length of output: 422
src/components/Global/TokenSelector/TokenSelector.tsx (2)
Line range hint 177-245: LGTM! Improved state management by using wallet context.
The changes correctly migrate from the useBalance hook to accessing balances directly from the wallet context, with proper null coalescing for the empty array fallback.
Line range hint 403-405: LGTM! Successfully implements token selector visibility control.
The conditional rendering effectively hides the token selector for peanut wallet users while maintaining it for external wallets with cross-chain support.
src/components/Request/Pay/Views/Initial.view.tsx (2)
341-349: LGTM! Well-structured wallet-specific token/chain reset logic.
The useCallback implementation correctly handles different defaults for peanut and external wallets, with proper memoization dependencies.
351-353: LGTM! Proper handling of wallet type changes.
The useEffect correctly triggers token/chain reset when the wallet type changes to peanut wallet.
src/components/Create/useCreateLink.tsx (2)
50-55: LGTM! Robust balance checking implementation.
The balance check logic properly handles null cases and uses the balanceByToken utility effectively.
744-744: LGTM! Proper balance refresh after link creation.
The refetchBalances call ensures the wallet state remains in sync after the transaction.
| .map((item: any) => ({ | ||
| chainId: item?.chainId ? item.chainId.split(':')[1] : '1', | ||
| address: item?.address ? item.address.split(':')[2] : '0x0000000000000000000000000000000000000000', | ||
| name: item.name, | ||
| symbol: item.symbol, | ||
| decimals: parseInt(item.quantity.decimals), | ||
| price: item.price, | ||
| amount: parseFloat(item.quantity.numeric), | ||
| currency: 'usd', | ||
| logoURI: item.iconUrl, | ||
| value: item.value.toString(), | ||
| })) |
There was a problem hiding this comment.
Ensure type safety and handle potential undefined properties during balance processing
The current implementation assumes that properties like item.name, item.quantity.decimals, and item.price are always defined. This could lead to runtime errors if any of these properties are undefined or null. Consider adding checks or default values to handle missing properties, and define proper TypeScript interfaces for item to ensure type safety.
| .map((balance: any) => | ||
| balance.chainId === '8508132' | ||
| ? { ...balance, chainId: '534352' } | ||
| : balance.chainId === '81032' | ||
| ? { ...balance, chainId: '81457' } | ||
| : balance.chainId === '59160' | ||
| ? { ...balance, chainId: '59144' } | ||
| : balance | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor chain ID remapping logic for better readability
The chain ID remapping logic uses nested ternary operators, which can reduce code readability and increase complexity. Consider refactoring this segment using a mapping object or switch statements for clarity.
Apply this diff to refactor the chain ID remapping:
-.map((balance: any) =>
- balance.chainId === '8508132'
- ? { ...balance, chainId: '534352' }
- : balance.chainId === '81032'
- ? { ...balance, chainId: '81457' }
- : balance.chainId === '59160'
- ? { ...balance, chainId: '59144' }
- : balance
-)
+const chainIdMapping: { [key: string]: string } = {
+ '8508132': '534352',
+ '81032': '81457',
+ '59160': '59144',
+};
+
+.map((balance: any) => {
+ const mappedChainId = chainIdMapping[balance.chainId] || balance.chainId;
+ return { ...balance, chainId: mappedChainId };
+})Committable suggestion skipped: line range outside the PR's diff.
| onClick={() => { | ||
| open() | ||
| }} | ||
| className="cursor-pointer text-h9 underline" | ||
| > | ||
| ( Buy Tokens ) | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Undefined function open will cause runtime error
In the click handler for the "Buy Tokens" link, the function open() is called, but open is not defined in the current scope, which will lead to a runtime error when the user clicks the link. Ensure that open is properly imported or defined, or replace it with the correct function.
If the intent is to open a modal for buying tokens, ensure that the appropriate function is called, such as signInModal.open() or another defined function.
Apply this diff to fix the issue:
-onClick={() => {
- open()
-}}
+onClick={() => {
+ signInModal.open()
+}}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@jjramirezn, glad to hear you've made the change!
(^_^)/
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/Cashout/Components/Initial.view.tsx (2)
287-301: Consider using optional chaining for safer null checksWhile the code works, using the non-null assertion operator (
!) could lead to runtime errors if the assumptions about nullability are wrong.Consider this safer approach:
-{selectedWallet!.balances!.length === 0 && ( +{selectedWallet?.balances?.length === 0 && (
221-226: Add missing dependencies to useEffectThe effect hook is missing
setSelectedChainIDandsetSelectedTokenAddressin its dependencies array. While these are stable references from context, it's best practice to include them.-}, [isPeanutWallet]) +}, [isPeanutWallet, setSelectedChainID, setSelectedTokenAddress])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Cashout/Components/Initial.view.tsx(7 hunks)
🔇 Additional comments (3)
src/components/Cashout/Components/Initial.view.tsx (3)
12-12: LGTM: New imports and context usage align with PR objectives
The added imports and context values provide the necessary functionality to manage default token selection and chain ID.
Also applies to: 21-21, 35-42
76-77: LGTM: Wallet type differentiation is properly implemented
The addition of wallet type flags (isExternalWallet, isPeanutWallet) and wallet state management enables proper handling of different wallet types, which is crucial for the token selector visibility logic.
199-204: LGTM: Balance calculation is properly implemented
The maxValue calculation correctly handles null checks and uses appropriate formatting with floorFixed. The implementation properly integrates with the wallet-specific balance handling.
|
why are all my coommetns pending |
| <Card.Content className="flex h-full flex-row items-center justify-center"> | ||
| <div className="text-5xl">{'$ 420.69'}</div> | ||
| <div className="text-5xl"> | ||
| $ {Number(formatUnits(selectedWallet?.balance ?? 0n, 6)).toFixed(2)} |
There was a problem hiding this comment.
Nit: balance is bigint. Might be a rlly large number with 21 decimal places (js formates as scientific notation). Not sure toFixed(2) would help here
There was a problem hiding this comment.
formatUnit handles part of that, it will return the USD(C) amount. Unless the balance is too big, in that case you are right and we would have problems with scientific notation.
I have this task https://www.notion.so/peanutprotocol/Bug-X-chain-from-amount-formatted-wrongly-to-scientific-notation-141838117579807abd12ee396cbfee97?pvs=4 to deal with that
| export function balanceByToken( | ||
| balances: IUserBalance[], | ||
| chainId: string, | ||
| tokenAddress: string | ||
| ): IUserBalance | undefined { | ||
| if (!chainId || !tokenAddress) return undefined | ||
| return balances.find((balance) => balance.chainId === chainId && areAddressesEqual(balance.address, tokenAddress)) | ||
| } |
There was a problem hiding this comment.
nit: this fetches an individual token balance - I think the name should reflect that better. E.g. getTokenBalance
|
|
||
| const maxValue = useMemo(() => { | ||
| const balance = balanceByToken(selectedChainID, selectedTokenAddress) | ||
| if (!selectedWallet?.balances) return '' |
There was a problem hiding this comment.
nit: wouldnt it be better to add the following check to balanceByToken:
if (!chainId || !tokenAddress || !balances) return undefined
There was a problem hiding this comment.
I think the balances missing case must be handled outside. Also I noticed a bug here, fixing
|
|
||
| useEffect(() => { | ||
| if (isPeanutWallet) { | ||
| setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString()) |
| useEffect(() => { | ||
| if (isPeanutWallet) { | ||
| setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString()) | ||
| setSelectedTokenAddress(PEANUT_WALLET_TOKEN) |
There was a problem hiding this comment.
thought: we miiiiiiiiiiight want to support other tokens in the future. Depends on demand and situation. E.g. maybe we find out ppinta token is great for onboarding and we run a promotion campaign partnering with them. Not sure what the best to do in that case would be here
There was a problem hiding this comment.
This is a great point and we have to think how we handle it. The idea of PW is absolute abstraction, but that comes with reduced functionality, like you mentioned (and PNT or MORFI are great onboarding tools).
One inmediate issue I can see is what do we do with claim links that contains tokens not supported for x-chain
| return valueB - valueA | ||
| }) | ||
|
|
||
| const totalBalance = processedBalances.reduce((acc: number, balance: any) => acc + Number(balance.value), 0) |
There was a problem hiding this comment.
Q: Number instead of bigint? 🤔
| ...createDefaultDBWallet(wagmiAddress), | ||
| connected: isWalletConnected(createDefaultDBWallet(wagmiAddress)), | ||
| balances, | ||
| balance: BigInt(Math.floor(totalBalance * 1e6)), |
There was a problem hiding this comment.
Q: why Math.floor(totalBalance * 1e6)? This seems arbitrary and would benefit either of a comment or from implementing in fetchWalletbalances
There was a problem hiding this comment.
balance is expected to be expressed as the base units amount of USDC (in the peanut wallet case) so this converts from the usd amount to that.
There was a problem hiding this comment.
imo this type of stuff should be commented in the code since it's very idiosyncratic
| > | ||
| ( Buy Tokens ) | ||
| </div> | ||
| {isExternalWallet && ( |
docs/high_level_overview.md
Outdated
| ### Hooks | ||
| The `/hooks` folder contains two custom hooks to help manage wallet functionality: | ||
| The `/hooks` folder contains a custom hook to help manage wallet functionality: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/context/walletContext/walletContext.tsx (1)
177-216: Ensure error handling provides meaningful feedbackWhile the
console.errorstatement logs errors during therefetchBalancesoperation, consider implementing more robust error handling to inform the user or trigger retries if appropriate. This will enhance the reliability and user experience of the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
docs/high_level_overview.md(0 hunks)src/components/Cashout/Components/Initial.view.tsx(7 hunks)src/components/Create/Link/Input.view.tsx(6 hunks)src/context/walletContext/walletContext.tsx(7 hunks)
💤 Files with no reviewable changes (1)
- docs/high_level_overview.md
🧰 Additional context used
📓 Learnings (1)
src/context/walletContext/walletContext.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#551
File: src/context/walletContext/walletContext.tsx:87-88
Timestamp: 2024-12-02T17:21:45.515Z
Learning: When converting `totalBalance` (in USD) to a `BigInt` balance in `src/context/walletContext/walletContext.tsx`, multiplying by `1e6` is intentional to maintain compatibility with USDC's 6 decimal places. The application displays only 2 decimal places, so this level of precision is sufficient.
🔇 Additional comments (2)
src/components/Create/Link/Input.view.tsx (1)
338-340:
Undefined function open will cause runtime error
The open() function used in the onClick handler is not defined in the current scope, which will lead to a runtime error when the user clicks the link. Ensure that open is properly defined or replace it with the correct function.
Apply this diff to fix the issue:
- onClick={() => {
- open()
- }}
+ onClick={() => {
+ signInModal.open()
+ }}src/components/Cashout/Components/Initial.view.tsx (1)
292-293: 🛠️ Refactor suggestion
Potential null reference when accessing selectedWallet.balances
Using non-null assertions ! with selectedWallet!.balances!.length might cause a runtime error if selectedWallet or balances is undefined. Consider using optional chaining or adding null checks to prevent potential crashes.
Apply this diff to fix the issue:
-{selectedWallet!.balances!.length === 0 && (
+{selectedWallet?.balances?.length === 0 && (| {selectedWallet!.balances!.length === 0 && ( | ||
| <div |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential null reference when accessing selectedWallet.balances
Using non-null assertions ! with selectedWallet!.balances!.length might cause a runtime error if selectedWallet or balances is undefined. Consider using optional chaining or adding null checks to prevent potential crashes.
Apply this diff to fix the issue:
-{selectedWallet!.balances!.length === 0 && (
+{selectedWallet?.balances?.length === 0 && (📝 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.
| {selectedWallet!.balances!.length === 0 && ( | |
| <div | |
| {selectedWallet?.balances?.length === 0 && ( | |
| <div |
When creating a claim link, paying a request or cashing out and using the peanut wallet, the user doesn't need to choose a token, the token is already chosen for them
Summary by CodeRabbit
Release Notes
New Features
PEANUT_WALLET_TOKEN.Bug Fixes
Chores
useBalancehook and associated logic.Documentation