fix: user added chains price id #1571
Conversation
WalkthroughThe changes introduce a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
packages/extension-polkagate/src/hooks/useBalances.ts (1)
103-103: Approve: Improved robustness in retrieving existential depositThe change enhances the code's resilience by adding a safety check before accessing
api.consts['balances']['existentialDeposit']. This prevents potential runtime errors if the 'balances' module is not available in the API.Consider using optional chaining for slightly improved readability:
const ED = api.consts?.['balances']?.['existentialDeposit'] as unknown as BN ?? BN_ZERO;This achieves the same result with a more concise syntax.
packages/extension-polkagate/src/util/utils.ts (2)
379-385: Avoid deep nesting and improve readability in conditionalsThe nested optional chaining and destructuring in the
findmethod make the code harder to read and maintain. Consider breaking down the logic into smaller, clearer steps or using intermediate variables for better readability.Refactored example:
if (useAddedChains) { const entries = Object.entries(useAddedChains); const matchedEntry = entries.find(([_, chainData]) => { const chainNameCleaned = chainData.chain?.replace(/\s/g, '')?.toLowerCase(); return chainNameCleaned === chainName.toLowerCase(); }); const maybeUserAddedPriceId = matchedEntry?.[1]?.priceId; if (maybeUserAddedPriceId) { return maybeUserAddedPriceId; } }
Line range hint
388-402: Prevent mutation of input arrays inareArraysEqualfunctionThe
areArraysEqualfunction currently sorts the input arrays in place usingarray.sort(), which mutates the original arrays. This side effect can lead to unexpected behavior elsewhere in the code where these arrays are used.To fix this, create copies of the arrays before sorting:
- const sortedArrays = arrays.map((array) => array.sort()); + const sortedArrays = arrays.map((array) => [...array].sort());Additionally, be aware that
Array.prototype.sort()without a compare function may not sort numbers correctly and won't work as expected for complex types. If the arrays contain numbers or custom objects, consider providing a compare function:const sortedArrays = arrays.map((array) => [...array].sort((a, b) => a - b));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (2 hunks)
- packages/extension-polkagate/src/hooks/useBalances.ts (1 hunks)
- packages/extension-polkagate/src/hooks/useTokenPrice.ts (4 hunks)
- packages/extension-polkagate/src/util/utils.ts (2 hunks)
- packages/extension-polkagate/src/util/workers/getAssetOnRelayChain.js (1 hunks)
- packages/extension-polkagate/src/util/workers/utils/getChainEndpoints.js (1 hunks)
🔇 Additional comments (9)
packages/extension-polkagate/src/util/workers/utils/getChainEndpoints.js (1)
19-19: 🛠️ Refactor suggestionVerify the impact of replacing
sanitizeChainNamewith direct string manipulation.The change from
sanitizeChainName(chain)?.toLowerCase()tochain?.replace(/\s/g, '')?.toLowerCase()might have some implications:
- It could potentially improve performance by avoiding an external function call.
- The new method removes all whitespace, which might change behavior in edge cases compared to the previous
sanitizeChainNamefunction.- The inline regex might be less readable than a named function.
To ensure consistency and prevent potential issues:
- Verify that this change doesn't break consistency with other parts of the codebase that might still be using
sanitizeChainName.- Consider extracting this logic into a new named function for better readability and reusability.
Run the following script to check for other usages of
sanitizeChainNamein the codebase:Consider extracting the string manipulation into a new function for better readability and consistency:
const sanitizeChainName = (chain) => chain?.replace(/\s/g, '')?.toLowerCase(); // Then use it in the function: const maybeEndpoint = Object.entries(userAddedEndpoints).find(([_, { chain }]) => sanitizeChainName(chain) === chainName.toLowerCase());This approach maintains the performance improvement while enhancing code readability and maintainability.
packages/extension-polkagate/src/util/workers/getAssetOnRelayChain.js (1)
98-98: LGTM! Verify thegetPriceIdByChainNamefunction update.The addition of the
userAddedEndpointsparameter to thegetPriceIdByChainNamefunction call enhances flexibility by allowing custom endpoints. This change aligns with the PR objective of fixing user-added chains price ID.Please run the following script to verify that the
getPriceIdByChainNamefunction has been updated to handle the new parameter:packages/extension-polkagate/src/hooks/useBalances.ts (1)
Line range hint
1-385: Overall impact: Localized improvement with no negative side effectsThe change to the existential deposit retrieval is a targeted improvement that enhances the robustness of the
useBalanceshook. It aligns well with the existing error-handling patterns in the file and doesn't introduce any new risks or alter the core functionality.To ensure the change is consistent with the rest of the file, let's verify the usage of
ED:This will help confirm that the change doesn't introduce any inconsistencies in how the existential deposit is used throughout the file.
packages/extension-polkagate/src/hooks/useTokenPrice.ts (3)
9-13: Imports are correctly updated and organized.The added imports for
useUserAddedPriceId,toCamelCase,ASSET_HUBS,NATIVE_TOKEN_ASSET_ID,useInfo, andusePricesare appropriate and the paths appear to be correct.
59-59: Dependency array inuseMemoupdated correctly.Including
userAddedPriceIdin the dependency array ensures that the memoized value updates whenuserAddedPriceIdchanges, preventing potential stale data issues.
50-50: Verify the logic for determiningpriceId.The updated logic prioritizes
_assetIdwhen it's defined and greater thanNATIVE_TOKEN_ASSET_ID. Otherwise, it usesuserAddedPriceIdor falls back togetPriceIdByChainName(chainName). Ensure that this order of precedence aligns with the intended functionality, especially in cases whereuserAddedPriceIdandgetPriceIdByChainName(chainName)might return different results.To confirm that
userAddedPriceIdis correctly fetched and utilized, you can run the following script:✅ Verification successful
Let's run an updated verification to ensure
userAddedPriceIdandgenesisHashare correctly utilized.
Let's run an updated verification to ensure
userAddedPriceIdandgenesisHashare correctly utilized.
Price ID Logic Verified
The
userAddedPriceIdandgenesisHashare correctly fetched and utilized, ensuring the order of precedence aligns with the intended functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that user-added price IDs are correctly associated with the genesisHash. # Test: Search for all occurrences of useUserAddedPriceId to ensure it's implemented correctly. rg --type javascript 'useUserAddedPriceId\((.*?)\)' -A 5 # Check if genesisHash values are consistently defined and used. rg --type javascript 'genesisHash' -A 2Length of output: 168
Script:
#!/bin/bash # Description: Verify that user-added price IDs are correctly associated with the genesisHash. # Test: Search for all occurrences of useUserAddedPriceId to ensure it's implemented correctly. rg --type typescript 'useUserAddedPriceId\((.*?)\)' -A 5 # Check if genesisHash values are consistently defined and used. rg --type typescript 'genesisHash' -A 2Length of output: 168
Script:
#!/bin/bash # Description: Verify that user-added price IDs are correctly associated with the genesisHash. # Test: Search for all occurrences of useUserAddedPriceId to ensure it's implemented correctly within TypeScript files. rg 'useUserAddedPriceId\((.*?)\)' -A 5 -g '*.ts' # Check if genesisHash values are consistently defined and used within TypeScript files. rg 'genesisHash' -A 2 -g '*.ts'Length of output: 103623
packages/extension-polkagate/src/util/utils.ts (1)
11-11: ImportingUserAddedChainstype is appropriateThe addition of
UserAddedChainsto the import statement ensures that the new type is available for use in the updated function. This is necessary for the changes made below.packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (2)
18-18: ImportinguseTokenPriceHookThe
useTokenPricehook is correctly imported from'../../hooks', allowing you to access token price data within the component.
72-72: ObtainingcurrentPriceUsinguseTokenPriceThe
currentPriceis now obtained by destructuring thepricefrom theuseTokenPrice(address, assetId)hook. This simplifies the logic for retrieving the current price based on the account address and asset ID.
closes #1569
Summary by CodeRabbit
New Features
Bug Fixes
Documentation