Conversation
WalkthroughThese changes add logic to account for wallet deployment costs when users interact with token amounts in various components. A new hook, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SwapReceiveCard
participant useDeployWallet
participant EnterAmount
User->>SwapReceiveCard: Selects token or account
SwapReceiveCard->>useDeployWallet: getWalletDeploymentCost(account, chainId)
useDeployWallet-->>SwapReceiveCard: Returns deploymentCost, isDeploymentCostLoading
SwapReceiveCard->>EnterAmount: Passes adjusted balance, deploymentCost, isDeploymentCostLoading
EnterAmount-->>User: Displays input, error messages based on deploymentCost and loading state
sequenceDiagram
participant User
participant SendModalTokensTabView
participant useDeployWallet
User->>SendModalTokensTabView: Opens send modal, selects asset
SendModalTokensTabView->>useDeployWallet: getWalletDeploymentCost(account, chainId)
useDeployWallet-->>SendModalTokensTabView: Returns deploymentCost, isDeploymentCostLoading
SendModalTokensTabView-->>User: Adjusts max/sendable amount, disables actions while loading
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 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 (3)
✨ 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 (
|
Deploying x with
|
| Latest commit: |
abd09d7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ed51f217.x-e62.pages.dev |
| Branch Preview URL: | https://fix-pro-3382-undeployed-wall.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/hooks/useDeployWallet.tsx (2)
50-83: Consider adding timeout and retry logic for API calls.The
getGasPricefunction makes external API calls without timeout or retry mechanisms, which could lead to poor user experience if the API is slow or temporarily unavailable.Consider adding timeout and retry logic:
const getGasPrice = async (chainId: number): Promise<string | undefined> => { const apiKey = process.env.REACT_APP_ETHERSPOT_DATA_API_KEY; if (!chainId) { console.error('getGasPrice: chainId is required'); return undefined; } if (!apiKey) { console.error('getGasPrice: API key is missing'); return undefined; } const url = `https://rpc.etherspot.io/v2/${chainId}?api-key=${apiKey}`; try { const response = await axios.post( url, { method: 'skandha_getGasPrice', }, { headers: { 'Content-Type': 'application/json', }, + timeout: 10000, // 10 second timeout } ); return response.data.result.maxFeePerGas; } catch (error) { console.error('getGasPrice: Failed to get gas price', error); return undefined; } };
113-116: Consider potential race condition in deployment status caching.If multiple components call
isWalletDeployedsimultaneously before the first call completes, they might all make redundant API calls since the localStorage cache won't be updated until the first call finishes.Consider implementing in-memory caching with Promise memoization to prevent redundant API calls:
+ const deploymentChecks = new Map<string, Promise<boolean | undefined>>(); const isWalletDeployed = async ( accountAddress: string, chainId: number ): Promise<boolean | undefined> => { + const cacheKey = `${accountAddress}_${chainId}`; + // Checking if this wallet has been deployed before using localStorage const localStorageStatus = getWalletDeployedLocalStorageStatus( accountAddress, chainId ); // If wallet has been deployed no further calls will be made if (localStorageStatus === true) { return true; } + // Check if there's already a pending request for this wallet + if (deploymentChecks.has(cacheKey)) { + return deploymentChecks.get(cacheKey); + } // If wallet has not deployed, then we will check on chain + const checkPromise = (async () => { // ... existing API call logic + })(); + + deploymentChecks.set(cacheKey, checkPromise); + + try { + const result = await checkPromise; + deploymentChecks.delete(cacheKey); + return result; + } catch (error) { + deploymentChecks.delete(cacheKey); + throw error; + } };src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)
175-189: Optimize deployment cost fetching to avoid unnecessary API calls.The deployment cost is fetched every time
selectedAssetchanges, which could result in redundant API calls if the user rapidly switches between assets on the same chain.Consider memoizing by chainId instead of the entire selectedAsset:
React.useEffect(() => { const getDeploymentCost = async () => { - if (!accountAddress || !selectedAsset?.chainId) return; + if (!accountAddress || !selectedAsset?.chainId) { + setDeploymentCost(0); + setIsDeploymentCostLoading(false); + return; + } setIsDeploymentCostLoading(true); const cost = await getWalletDeploymentCost({ accountAddress, chainId: selectedAsset.chainId, }); setDeploymentCost(cost); setIsDeploymentCostLoading(false); }; getDeploymentCost(); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [accountAddress, selectedAsset]); + }, [accountAddress, selectedAsset?.chainId]);
579-579: Improve accessibility and user feedback for disabled max button.The max button is conditionally disabled based on
isDeploymentCostLoading, but users might not understand why it's disabled.Consider adding a tooltip or loading indicator:
- {!isDeploymentCostLoading && maxAmountAvailable > 0 && ( + {maxAmountAvailable > 0 && ( <TextInputButton onClick={() => setAmount(`${maxAmountAvailable}`)} + disabled={isDeploymentCostLoading} + title={isDeploymentCostLoading ? 'Loading deployment cost...' : 'Set maximum amount'} > {t`helper.max`} + {isDeploymentCostLoading && <span>⏳</span>} <span> <IconFlash size={16} variant="Bold" /> </span> </TextInputButton> )}src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx (1)
121-136: Consistent optimization opportunity as in SendModal.Similar to the SendModal component, the deployment cost is fetched every time
swapTokenchanges rather than just when the chain changes, which could lead to unnecessary API calls.Consider optimizing the dependency to focus on chainId:
useEffect(() => { const getDeploymentCost = async () => { - if (!accountAddress || !swapToken?.blockchain) return; + if (!accountAddress || !swapToken?.blockchain) { + setDeploymentCost(0); + setIsDeploymentCostLoading(false); + return; + } setIsDeploymentCostLoading(true); const cost = await getWalletDeploymentCost({ accountAddress, chainId: chainNameToChainIdTokensData(swapToken.blockchain), }); setDeploymentCost(cost); setIsDeploymentCostLoading(false); }; getDeploymentCost(); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [accountAddress, swapToken]); + }, [accountAddress, swapToken?.blockchain]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx(3 hunks)src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx(4 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(7 hunks)src/hooks/useDeployWallet.tsx(1 hunks)src/services/tokensData.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx (4)
src/apps/the-exchange/hooks/useReducerHooks.tsx (1)
useAppSelector(6-6)src/services/tokensData.ts (3)
Token(11-21)chainNameToChainIdTokensData(244-265)chainNameDataCompatibility(51-73)src/apps/token-atlas/types/types.tsx (1)
SelectedTokenType(14-23)src/apps/the-exchange/utils/wrappedTokens.ts (1)
isNativeToken(29-30)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (1)
src/apps/the-exchange/utils/wrappedTokens.ts (1)
isNativeToken(29-30)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (5)
src/services/tokensData.ts (1)
52-53: Good defensive programming practice!Adding the early return for falsy
chainNamevalues prevents potential runtime errors from calling.toLowerCase()on undefined, null, or empty values. This improves the robustness of the function and aligns well with the broader deployment cost integration where chain names are processed.src/hooks/useDeployWallet.tsx (1)
184-185:Details
❓ Verification inconclusive
Verify the static gas unit estimate across different chains.
The fixed gas unit estimate of 271,000 may not be accurate across all supported chains, as deployment costs can vary significantly between different networks.
Please verify if this gas estimate is appropriate for all supported chains:
🏁 Script executed:
#!/bin/bash # Description: Check what chains are supported and their typical deployment costs # Find supported chains in the codebase echo "=== Supported Chains ===" rg -A 5 "CompatibleChains|chainId.*=" --type ts echo -e "\n=== Chain ID mappings ===" rg -A 10 "chainIdToChainNameTokensData|chainNameToChainIdTokensData" --type tsLength of output: 66334
🏁 Script executed:
#!/bin/bash # Extract the CompatibleChains array to list supported chain IDs awk '/export const CompatibleChains/,/\];/' src/utils/blockchain.tsLength of output: 483
Verify static gas limit for wallet deployment across supported chains
We currently hardcode a gas limit of
271000for deploying wallets, but gas requirements can differ by network. Based on the code, the supported chains are:
- Ethereum (chainId 1)
- Polygon (137)
- Base (8453)
- Gnosis (100)
- BNB Smart Chain (56)
- Optimism (10)
- Arbitrum (42161)
Please verify that
BigInt(271000)is sufficient (and not overly generous) for deploying the wallet contract on each network. You can use an RPC call (e.g.eth_estimateGasvia Ethers.js or the Etherspot RPC endpoint) to measure actual deployment gas. If estimates vary, consider:
- Mapping
chainId→ tailoredgasUnits- Falling back to a dynamic
estimateGascall at runtimesrc/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx (1)
184-192:Details
✅ Verification successful
Verify EnterAmount component can handle the new props correctly.
The component is passing new props (
tokenChain,isDeploymentCostLoading,deploymentCost) to theEnterAmountcomponent. Ensure these props are properly handled to prevent runtime errors.Let's verify that the
EnterAmountcomponent properly handles these new props:
🏁 Script executed:
#!/bin/bash # Description: Check EnterAmount component prop definitions and usage # Find the EnterAmount component and check its prop types echo "=== EnterAmount Component Props ===" ast-grep --pattern $'type $_ = { $$$ tokenChain$$$?: $_, $$$ isDeploymentCostLoading$$$?: $_, $$$ deploymentCost$$$?: $_, $$$ }' # Alternative search for interface or props definition rg -A 20 -B 5 "EnterAmount.*Props|interface.*EnterAmount" --type tsx --type tsLength of output: 333
🏁 Script executed:
#!/bin/bash echo "=== Searching for EnterAmount usage in SwapReceiveCard.tsx ===" grep -n "EnterAmount" src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx || echo "No direct references found in SwapReceiveCard.tsx" echo echo "=== Locating EnterAmount.tsx in the repository ===" fd -e tsx | grep -i "EnterAmount.tsx" || echo "No EnterAmount.tsx file found" echo echo "=== Searching for imports of EnterAmount across .tsx files ===" rg -n "import .*EnterAmount" --glob "*.tsx" || echo "No imports of EnterAmount found"Length of output: 954
🏁 Script executed:
#!/bin/bash echo "=== Displaying EnterAmount.tsx header (first 200 lines) ===" sed -n '1,200p' src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx echo echo "=== Checking for tokenChain usage ===" grep -R "tokenChain" -n src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx || echo "tokenChain not found" echo echo "=== Checking for isDeploymentCostLoading usage ===" grep -R "isDeploymentCostLoading" -n src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx || echo "isDeploymentCostLoading not found" echo echo "=== Checking for deploymentCost usage ===" grep -R "deploymentCost" -n src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx || echo "deploymentCost not found"Length of output: 7877
EnterAmount Component Already Supports New Props
The
EnterAmountPropstype (lines 39–41) includestokenChain,isDeploymentCostLoading, anddeploymentCost, and these props are used in:
- the
tokenBalanceLimiterror message (line 157)- the conditional rendering based on
isDeploymentCostLoading(around line 213)No further changes are needed—runtime safety for these props is already ensured.
src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx (2)
39-41: LGTM! Well-structured props for deployment cost feature.The new optional props are appropriately typed and named, clearly supporting the deployment cost limitation feature for undeployed wallets.
Also applies to: 44-51
212-220: LGTM! Proper loading state handling for deployment cost.The conditional rendering correctly prevents showing error messages while deployment cost data is loading, ensuring users don't see potentially incorrect balance limit messages. The improved JSX formatting also enhances readability.
| const swapTokenBalance = useMemo(() => { | ||
| if (!swapToken || !swapToken.balance) return 0; | ||
|
|
||
| const adjustedBalance = isNativeToken(swapToken.contract) | ||
| ? swapToken.balance - deploymentCost | ||
| : swapToken.balance; | ||
|
|
||
| return adjustedBalance; | ||
| }, [swapToken, deploymentCost]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add bounds checking for negative balance scenarios.
Similar to the SendModal, the swapTokenBalance calculation should handle cases where deployment cost exceeds the available balance.
Add protection against negative balances:
const swapTokenBalance = useMemo(() => {
if (!swapToken || !swapToken.balance) return 0;
const adjustedBalance = isNativeToken(swapToken.contract)
? swapToken.balance - deploymentCost
: swapToken.balance;
- return adjustedBalance;
+ return Math.max(0, adjustedBalance);
}, [swapToken, deploymentCost]);📝 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 swapTokenBalance = useMemo(() => { | |
| if (!swapToken || !swapToken.balance) return 0; | |
| const adjustedBalance = isNativeToken(swapToken.contract) | |
| ? swapToken.balance - deploymentCost | |
| : swapToken.balance; | |
| return adjustedBalance; | |
| }, [swapToken, deploymentCost]); | |
| const swapTokenBalance = useMemo(() => { | |
| if (!swapToken || !swapToken.balance) return 0; | |
| const adjustedBalance = isNativeToken(swapToken.contract) | |
| ? swapToken.balance - deploymentCost | |
| : swapToken.balance; | |
| return Math.max(0, adjustedBalance); | |
| }, [swapToken, deploymentCost]); |
🤖 Prompt for AI Agents
In src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx around
lines 138 to 146, the calculation of swapTokenBalance does not handle cases
where deploymentCost exceeds swapToken.balance, potentially resulting in a
negative balance. Modify the calculation to check if the adjusted balance is
negative and if so, return zero instead. This ensures the balance never goes
below zero, preventing negative values.
github-actions
bot
temporarily deployed
to
Preview (fix/PRO-3382/undeployed-wallet-cost)
3 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes