Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughThe changes standardize the token data model across the codebase and replace several hook-based asset retrieval methods with service-based calls. The token structure has shifted from using properties such as Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component (e.g., DropdownTokenList)
participant TS as tokensData Service
participant VW as react-window List
UI->>TS: Call queryTokenData()/searchTokensData(query)
TS-->>UI: Return Token[] (with contract, blockchain, logo, etc.)
UI->>VW: Pass token list as itemData for virtualization
VW-->>UI: Render token rows via TokenRow component
sequenceDiagram
participant User as End User
participant Search as TokenSearchInput
participant TS as tokensData Service
User->>Search: Enters search query
Search->>TS: Call searchTokensData(query)
TS-->>Search: Return matched token results
Search->>User: Display updated search results via FixedSizeList
Suggested reviewers
Poem
🪧 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: 4
🔭 Outside diff range comments (1)
src/apps/the-exchange/components/EnterAmount/test/EnterAmount.test.tsx (1)
138-138:⚠️ Potential issueFix typo in tokenSymbol property
There's a typo in the property access -
symbollshould besymbol.- tokenSymbol={store.getState().swap.swapToken.symboll} + tokenSymbol={store.getState().swap.swapToken.symbol}
🧹 Nitpick comments (5)
src/apps/token-atlas/components/TokensSearchResult/TokenRow.tsx (1)
10-36: Use token unique identifier as key instead of indexThe component uses the
indexas akeyprop (line 27), which can lead to inefficient DOM updates if the list order changes. React recommends using a stable unique identifier for keys.<div style={style}> <TokenResultCard - key={index} + key={`${token.contract}-${token.blockchain}`} onClick={() => handleChooseToken(token)} tokenName={token.name} tokenSymbol={token.symbol} tokenChain={chainNameToChainIdTokensData(token.blockchain)} tokenLogo={token.logo} /> </div>src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (1)
34-74: Consider extending mock coverage
The mockedchainNameToChainIdTokensDatadictionary only includes Ethereum and Polygon. If more blockchains are expected, consider adding them for broader test coverage.src/components/BottomMenuModal/AccountModal.tsx (1)
65-65: Remove commented out code.Clean up the codebase by removing the commented out line.
- // const assets = useAssets();src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx (1)
58-67: Property naming inconsistency.The token object dispatched to
setSelectedTokenusesiconas the property name but assigns the value fromtoken.logo. This creates a naming inconsistency that could confuse other developers.Apply this fix to make property naming consistent:
- icon: token.logo, + logo: token.logo,Alternatively, if you need to maintain the
iconproperty for backward compatibility:- icon: token.logo, + icon: token.logo, // Using logo as the icon source for backward compatibilitysrc/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (1)
177-242: Complex token selection logic could be refactored.While the virtualized list implementation is excellent, the token selection logic in
handleClickis quite lengthy and handles both swap and receive scenarios. Consider extracting this into separate functions for better readability and maintainability.You could refactor to separate functions:
const handleSwapTokenClick = (token: Token) => { dispatch(setSwapToken(token)); dispatch( setSwapChain({ chainId: chainNameToChainIdTokensData(token.blockchain), chainName: token.blockchain, }) ); recordPresence({ address: accountAddress, action: 'app:theExchange:sourceTokenSelect', value: { chainId: chainNameToChainIdTokensData(token.blockchain), address: token.contract, symbol: token.symbol, name: token.name, }, }); dispatch(setSearchTokenResult([])); dispatch(setIsSwapOpen(false)); }; const handleReceiveTokenClick = (token: Token) => { dispatch(setReceiveToken(token)); dispatch( setReceiveChain({ chainId: chainNameToChainIdTokensData(token.blockchain), chainName: token.blockchain, }) ); recordPresence({ address: accountAddress, action: 'app:theExchange:destinationTokenSelect', value: { chainId: chainNameToChainIdTokensData(token.blockchain), address: token.contract, symbol: token.symbol, name: token.name, }, }); dispatch(setSearchTokenResult([])); dispatch(setIsReceiveOpen(false)); };Then simplify the
handleClicklogic:handleClick: (token: Token) => { if (isSwapOpen) { handleSwapTokenClick(token); } else { handleReceiveTokenClick(token); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/DropdownTokensList/test/__snapshots__/DropdownTokensList.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/TokenListItem/test/__snapshots__/TokenListItem.test.tsx.snapis excluded by!**/*.snapsrc/apps/token-atlas/components/TokensSearchResult/test/__snapshots__/TokensSearchResult.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (29)
package.json(2 hunks)src/apps/the-exchange/components/CardsSwap/CardsSwap.tsx(2 hunks)src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx(2 hunks)src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx(6 hunks)src/apps/the-exchange/components/DropdownTokensList/TokenRow.tsx(1 hunks)src/apps/the-exchange/components/DropdownTokensList/test/DropdownTokensList.test.tsx(1 hunks)src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx(5 hunks)src/apps/the-exchange/components/EnterAmount/test/EnterAmount.test.tsx(1 hunks)src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx(5 hunks)src/apps/the-exchange/components/ExchangeAction/test/ExchangeAction.test.tsx(1 hunks)src/apps/the-exchange/components/SelectDropdown/test/SelectDropdown.test.tsx(1 hunks)src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx(2 hunks)src/apps/the-exchange/components/SwapSummary/SwapSummary.tsx(1 hunks)src/apps/the-exchange/components/SwitchCardsButton/SwitchCardsButton.tsx(1 hunks)src/apps/the-exchange/components/TokenListItem/TokenListItem.tsx(1 hunks)src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx(4 hunks)src/apps/the-exchange/hooks/usePillarSwapAssets.tsx(1 hunks)src/apps/the-exchange/reducer/theExchangeSlice.ts(1 hunks)src/apps/token-atlas/components/HeaderSearch/HeaderSeach.tsx(2 hunks)src/apps/token-atlas/components/SearchTokenModal/SearchTokenModal.tsx(2 hunks)src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx(4 hunks)src/apps/token-atlas/components/TokensSearchResult/TokenRow.tsx(1 hunks)src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx(4 hunks)src/apps/token-atlas/components/TokensSearchResult/test/TokensSearchResult.test.tsx(3 hunks)src/apps/token-atlas/reducer/tokenAtlasSlice.ts(1 hunks)src/components/BottomMenuModal/AccountModal.tsx(6 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(4 hunks)src/components/Form/AssetSelect/index.tsx(6 hunks)src/services/tokensData.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/apps/the-exchange/reducer/theExchangeSlice.ts
- src/apps/token-atlas/reducer/tokenAtlasSlice.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: unit-tests
- GitHub Check: Cloudflare Pages
🔇 Additional comments (86)
package.json (2)
94-94: Appropriate addition of react-window library.The addition of
react-windowlibrary is a good choice for efficiently rendering large lists of tokens. This virtualization library will help improve performance by only rendering the visible items in the viewport.
121-121: Good practice including type definitions.Including the TypeScript type definitions for the react-window library ensures proper type safety and better developer experience.
src/apps/the-exchange/components/TokenListItem/TokenListItem.tsx (2)
38-40: Improved text overflow handling with truncation.Adding
truncate max-w-[150px]classes to the token name display ensures long token names don't break the UI layout and appear with an ellipsis when they exceed the maximum width.
46-46: Good responsive text handling for chain names.The addition of
truncate xs:max-w-[150px]applies appropriate text truncation for chain names, with the responsive breakpoint ensuring proper display across different screen sizes.src/apps/the-exchange/components/SwitchCardsButton/SwitchCardsButton.tsx (1)
16-18: Standardization of Token type imports.Replacing the external Token class import with a locally defined one from the services directory aligns with the PR objective of standardizing token data models across the codebase.
src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx (3)
1-5: Proper encapsulation of token data handling.Importing
TokenandchainNameToChainIdTokensDatafrom a centralized service module is a good practice for maintaining consistency in token data handling across the application.
52-54: Updated token property access pattern.The code now properly uses the
chainNameToChainIdTokensDatahelper function to convert from the token'sblockchainproperty to chain ID, reflecting the new token data structure.Make sure that the
chainNameToChainIdTokensDatafunction properly handles null or undefined values to prevent runtime errors when tokens haven't been selected yet.
56-57: Updated token logo property reference.Changed from using
icontologoproperty in accordance with the new token data model.src/apps/the-exchange/components/ExchangeAction/test/ExchangeAction.test.tsx (2)
33-33: Adoption of centralized Token type from services.This change properly updates the token type import to use the centralized definition from the tokensData service, which improves consistency across the codebase.
36-54: Token model standardization aligns with Mobula Token Data.The mock token data structure has been updated to match the new token model:
address→idandcontractchainId→blockchainicon→logoThis change ensures test data consistency with the production code.
src/apps/token-atlas/components/HeaderSearch/HeaderSeach.tsx (2)
15-17: New import for tokensData service.The import for the centralized token data service has been added, replacing the previous hook-based approach.
45-46:Details
✅ Verification successful
Switch from async hook to synchronous service call.
The implementation has changed from an asynchronous call to
getPillarSwapAssets()to a synchronous call toqueryTokenData({}). This eliminates the need for async/await, but requires verification that the data is available when needed.
🏁 Script executed:
#!/bin/bash # Look for implementations of queryTokenData to verify it's synchronous rg -A 3 "queryTokenData\s*=\s*" --type typescriptLength of output: 86
Below is a revised verification step to re-run the search using proper file extensions (e.g. *.ts and *.tsx) instead of using the unrecognized file type flag. This will help verify that the new call to
queryTokenDatais indeed synchronous.Re-run verification using proper TypeScript file extensions:
Please execute this revised script to check the actual implementation of
queryTokenData. Once you confirm that it is synchronous and that the data required byHeaderSeach.tsxis immediately available, we can finalize the review comment.
🏁 Script executed:
[run_scripts] #!/bin/bash # Searching for implementations of queryTokenData to verify its synchronous behavior rg -A 3 "queryTokenData\s*=" -g "*.ts" -g "*.tsx"Length of output: 338
Update: Confirm Synchronous Data Access in HeaderSearch
The search confirms that
queryTokenDatais implemented insrc/services/tokensData.tswithout any asynchronous constructs—indicating a synchronous function. This aligns with the change from the previous async hook (getPillarSwapAssets()) to a synchronous service call. Please ensure that the token data is indeed available immediately whenHeaderSearch.tsxcallsqueryTokenData({}), as the removal of async/await assumes no latency issues.src/apps/the-exchange/components/SwapSummary/SwapSummary.tsx (1)
5-5: Updated Token type import source.The import source for the Token type has been changed from an external package to the local tokensData service. This aligns with the centralized token data model approach across the application.
src/apps/the-exchange/components/CardsSwap/CardsSwap.tsx (1)
12-14: New import for tokensData service.The import for the centralized token data service has been added, replacing the previous hook-based approach.
src/apps/token-atlas/components/SearchTokenModal/SearchTokenModal.tsx (1)
13-17: Change from direct import to service import looks goodThe Token type and chainNameToChainIdTokensData function are now being imported from a centralized service, which promotes better code organization and reusability.
src/apps/the-exchange/components/SelectDropdown/test/SelectDropdown.test.tsx (2)
27-29: Good type importProper import of the Token type from the centralized service.
34-53: Consistent token structure updateThe token structure has been appropriately updated to match the new model:
address→idandcontractchainId→blockchainicon→logoThis change aligns with the standardization of the token data model across the codebase.
src/apps/token-atlas/components/TokensSearchResult/TokenRow.tsx (1)
1-9: Good service imports and component organizationProper imports and clear component organization structure.
src/apps/the-exchange/components/EnterAmount/test/EnterAmount.test.tsx (1)
35-54: Token structure consistently updatedThe mock token data structure has been correctly updated to match the new token model with the proper properties.
src/apps/the-exchange/components/DropdownTokensList/TokenRow.tsx (1)
1-35: Well-structured component for virtualized list rendering.This new
TokenRowcomponent implements the interface required byreact-windowfor efficient virtualized rendering of token lists. The implementation looks clean and follows React best practices.src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (4)
10-15: Good refactor to centralize token data services.You've moved token-related functionality to a centralized service, which improves maintainability and consistency across the application.
59-61: Implementation simplified with service function.The token search functionality has been refactored to use a centralized service, simplifying this component's responsibility.
67-77: Token data structure standardized.Updated code to use the new standardized token data structure, replacing the previous nested format with direct property access.
100-101: Token search service applied consistently.The renamed function is used consistently throughout the component, maintaining the functionality while leveraging the centralized service.
Also applies to: 118-118
src/apps/the-exchange/components/DropdownTokensList/test/DropdownTokensList.test.tsx (3)
28-28: Updated import to use standardized Token type.Good job importing the Token type from the centralized service, ensuring test consistency with the application code.
34-53: Test data aligned with the new token structure.The mock token assets have been updated to match the new token data structure, using
contractinstead ofaddress,blockchaininstead ofchainId, andlogoinstead oficon.
55-95: Comprehensive mock of token data service.This mock implementation provides all necessary functions used by the component, making the tests accurate and reliable. Good job including chain mapping, query, and search functionality.
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (5)
106-107: Updated property access to use the new token structure.The code now correctly uses
selectedAsset.asset.contractinstead ofselectedAsset.asset.addressto align with the standardized token model.
123-123: Updated token address reference for price retrieval.This change maintains the functionality while using the new property name
contractinstead ofaddress.
532-535: Updated address checks with new token structure.Correctly updated the zero address checking logic to use the new
contractproperty instead ofaddress.
546-549: Token property name consistency maintained.Second instance of token property check correctly updated to use
contractinstead ofaddress.
553-553: Token address reference updated in transfer transaction.The EtherspotTokenTransferTransaction component now correctly receives the token address from the
contractproperty.src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (6)
32-32: Import of Token interface is consistent
No issues found with importing theTokeninterface fromtokensData.
76-76: Defining mockTokenAssets
Good approach to centralize test tokens in a single array.
78-78: Ethereum token structure
The new fields (id,blockchain,logo) align well with the updatedTokentype.Also applies to: 82-82, 84-84
87-87: Polygon token structure
All fields match the newTokenshape.Also applies to: 88-88, 91-91, 93-93
250-250: No functional code
This changed line appears to be only whitespace or formatting; skipping further review.
253-256: Deep equality check on token arrays
Ensuring the entire array matches is correct. This strengthens test reliability.src/apps/token-atlas/components/TokensSearchResult/test/TokensSearchResult.test.tsx (6)
20-22: Importing the Token type
The updated import fromservices/tokensDatais consistent with the new token model.
25-25: mockTokens[0]: Replacing legacy fields
Propertiesid,blockchain,logo, andcontractmatch the revisedTokeninterface.Also applies to: 28-28, 29-29, 31-31
34-34: mockTokens[1]: Polygon token structure
The object accurately follows the newTokentype.Also applies to: 37-37, 38-38, 40-40
46-46: mockTokenListData[0]: First token alignment
Transitioning to the new fields looks correct.Also applies to: 49-49, 50-50, 52-52
55-55: mockTokenListData[1]: Second token alignment
All fields align with the updatedTokenstructure.Also applies to: 58-58, 59-59, 61-61
119-119: Temporary mock token definition
This token’s structure is consistent with the newTokeninterface.Also applies to: 122-122, 123-123, 125-125
src/apps/the-exchange/hooks/usePillarSwapAssets.tsx (5)
3-7: New imports from tokensData
Switching toToken,chainIdToChainNameTokensData, andqueryTokenDatais aligned with the updated approach for token data retrieval.
13-16: Querying token data by resolved blockchain
UsingchainIdToChainNameTokensData(chainId)is a good approach. Verify that unexpected chain IDs return a fallback orundefinedif necessary.
21-22: Return type and array initialization
ReturningToken[]and initializingpillarSwapAssetsasToken[]is consistent with the updated data model.
30-30: Flattening assets
Confirmassetsis truly an object keyed by blockchain (or similar). IfqueryTokenDatareturns an array only,.flat()might be unnecessary.
33-33: Mapping final token shape
The propertiesid,logo,blockchain, andcontractcorrectly match the newTokeninterface.Also applies to: 36-36, 37-37, 38-38
src/components/Form/AssetSelect/index.tsx (5)
14-19: Good refactoring to standardize token data access.The introduction of imports from the centralized
tokensDataservice aligns with the broader refactoring effort to standardize token data access across the codebase.
32-37: Appropriate type update for assets.Updating the
assetproperty type in theTokenAssetSelectOptioninterface fromTokenListTokentoTokenproperly aligns with the new token structure.
72-74: Good simplification of asset retrieval.Replacing the
useAssetshook with a direct call toqueryTokenDatastreamlines the code and centralizes token data access.
84-95: Updated token mapping to use new token structure.The asset mapping has been properly updated to use the new token properties:
asset.idfor theidandasset.logofor theimageSrc.
129-129:Details
❓ Verification inconclusive
Verify dependency array for useEffect.
The
assetsdependency has been removed from the dependency array, but the effect usesassetsin its body. Consider whether this was intentional or ifassetsshould be added back to ensure the effect re-runs when assets change.
🏁 Script executed:
#!/bin/bash # Check if assets is derived within useEffect or if it needs to be in the dependency array rg -B 5 -A 5 "}, \[walletAddress, chainId\]);" src/components/Form/AssetSelect/index.tsxLength of output: 202
Ensure the useEffect dependency array includes all relevant dependencies
It appears that within the useEffect block in
src/components/Form/AssetSelect/index.tsxthe variableassetsis used, yet it’s currently omitted from the dependency array (which only includeswalletAddressandchainId). Please verify whether:
- The omission of
assetsis intentional (for example, if it’s derived or known to be stable) and properly documented.- Or if
assetsshould be added to ensure the effect correctly re-runs when its value changes.Once confirmed, update either the dependency array or add appropriate comments explaining the decision.
🏁 Script executed:
[run_scripts] #!/bin/bash # Re-run the search with corrected regex escapes for the useEffect dependency array in index.tsx rg -B 5 -A 5 'useEffect\([^)]*\}, \[walletAddress, chainId\]\);' src/components/Form/AssetSelect/index.tsxLength of output: 230
Verify the useEffect dependencies
The current dependency array only includes
walletAddressandchainId, yet the effect body usesassets. Our automated searches haven’t provided clear evidence on whetherassetsis derived or stable enough to be safely omitted. Please manually confirm that this omission is intentional. If not, addassetsto the dependency array or document why it’s intentionally excluded.src/components/BottomMenuModal/AccountModal.tsx (4)
49-53: Good implementation of token data centralization.Importing
Tokenand utility functions from thetokensDataservice aligns with the project's standardization efforts.
79-93: Good approach for token data retrieval.The refactoring to use
queryTokenDatawithin thegroupedTokensuseMemo calculation is cleaner than using a separate hook. The property access update froma.addresstoa.contractproperly aligns with the new token structure.
107-107: Appropriate dependency array update.Adding
addressesEqualandisZeroAddressto the dependency array is correct since they're used in the memoized computation.
305-306: Updated token property reference.The property access has been correctly updated from
logoURItologoto match the new token structure.src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx (4)
2-2: Great performance improvement with virtualized lists.Adding the
FixedSizeListfromreact-windowis an excellent optimization for rendering large token lists, reducing memory usage and improving performance.
16-20: Good token data centralization.Importing the standardized token model and utility functions from the central service improves consistency across the codebase.
49-52: Updated token filtering logic.The filtering logic has been correctly updated to use
chainNameToChainIdTokensData(token.blockchain)instead of directly comparing chain IDs.
79-91: Excellent list rendering optimization.The implementation of virtualized rendering with
FixedSizeListgreatly improves performance when dealing with large token lists. The conditional rendering for empty lists is also well implemented.src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (4)
3-3: Great performance optimization with virtualized lists.Using
FixedSizeListfromreact-windowsignificantly improves performance for rendering large token lists.
20-24: Good centralization of token data services.Importing from the central
tokensDataservice improves consistency across the codebase.
85-96: Well-implemented chain mapping logic.The chain mapping logic has been updated appropriately to work with the new token structure, using
blockchainandchainNameToChainIdTokensData.
107-110: Updated filtering logic aligns with new token structure.The filtering logic has been correctly updated to use
chainNameToChainIdTokensData(token.blockchain)for comparing chain IDs.Also applies to: 122-125
src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (4)
14-18: Proper modularization of token data services.Good job on moving to a centralized service for token data operations. The imports are clean and properly structured.
45-63: Clean implementation of search logic migration.The refactored search function effectively leverages the imported service. The implementation properly handles both the all-chains case (chainId === 0) and the specific chain filtering case.
The conversion from blockchain names to chain IDs using
chainNameToChainIdTokensDataensures consistent behavior across the application.
77-77: Consistent update to function call.The update to use the renamed function in the useEffect dependency is appropriate.
94-94: Consistent handling of search input.The update to use the renamed function for handling search input is consistent with other changes in the file.
src/services/tokensData.ts (6)
4-12: Well-defined Token type with standardized properties.The Token interface provides a clear and consistent structure for token data throughout the application, which will improve maintainability.
14-23: Comprehensive raw data structure definition.The TokenRawDataItem type efficiently handles tokens that exist on multiple blockchains with corresponding contracts and decimals.
37-52: Efficient token data loading with memoization.The implementation properly memoizes the token data to prevent unnecessary processing on subsequent calls. The flatMap approach effectively transforms the multi-blockchain tokens into individual token entries.
54-70: Clean and flexible token query implementation.The query function provides a flexible way to filter tokens based on different criteria with proper null checking on parameters.
73-84: Effective fuzzy search implementation.Good use of the Fuse.js library for fuzzy searching with appropriate configuration for search fields and threshold.
104-119:Details
❓ Verification inconclusive
Verify default chain ID behavior in chainNameToChainIdTokensData.
The function returns 1 (Ethereum's chain ID) as the default when an unrecognized blockchain name is provided. Ensure this is the intended behavior, as it could potentially cause confusion if users expect different behavior for unknown chains.
🏁 Script executed:
#!/bin/bash # Check where this function is used and how it handles unknown blockchain names echo "Checking usages of chainNameToChainIdTokensData:" rg -A 2 "chainNameToChainIdTokensData\\(" --type typescriptLength of output: 202
Action: Verify Default Chain ID Behavior in chainNameToChainIdTokensData
Please confirm that returning Ethereum's chain ID (1) by default for unrecognized blockchain names is the intended behavior. The current fallback value may be ambiguous if unexpected chain names are passed. Additionally, re-run the updated search (shown above) to verify if this function is used in contexts that might require a different error handling or default value.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching for usages of chainNameToChainIdTokensData in TypeScript files:" rg -A 2 "chainNameToChainIdTokensData\\(" -g "*.ts"Length of output: 331
Action Required: Confirm Default Chain ID Behavior
The function
chainNameToChainIdTokensDatareturns Ethereum’s chain ID (1) as a default when an unrecognized blockchain name is provided. Since our searches did not reveal any additional contexts or usage that clarify a different intended behavior for unknown chains, please verify that this fallback is deliberate. If the default value is meant to indicate an error or neutral state rather than implicitly favoring Ethereum, consider updating the logic or adding documentation to avoid confusion.
- Review Location:
src/services/tokensData.ts(Lines 104–119)- Potential Improvement: Clarify or adjust the return value for unrecognized chain names if a behavior other than defaulting to 1 is desired.
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx (4)
6-10: Proper integration with the centralized token service.Good migration from the external Token type to the locally defined one from the tokensData service.
100-102: Updated transaction description with proper token properties.The description now correctly references the token's blockchain property and uses the chainNameToChainIdTokensData function to derive the chain ID.
127-129: Consistent transaction handling.The same approach is properly applied to cross-chain transactions, maintaining consistency throughout the codebase.
169-169: Updated token logo property reference.Correctly updated from 'icon' to 'logo' to match the new Token type structure.
src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx (6)
19-23: Proper import of token service utilities.Correctly importing the Token type and chainNameToChainIdTokensData function from the centralized service.
77-79: Updated hook parameter to use blockchain name conversion.The useOffer hook now correctly uses the chainNameToChainIdTokensData function to derive the chain ID from the blockchain name.
85-88: Updated price fetching to use new token properties.Correctly modified to use token.contract and chainNameToChainIdTokensData conversion instead of direct token.address and token.chainId properties.
105-108: Consistent property usage for receive token.Same approach is properly applied for receive token price fetching, maintaining consistency.
125-131: Comprehensive update of offer parameters.All parameters in the getOffer function have been properly updated to match the new token property structure, with appropriate null handling.
165-173: Updated balance lookup with proper conversion.The tokenBalanceLimit function now correctly accesses balances using the chainNameToChainIdTokensData conversion and uses token.contract instead of token.address.
| // Error handled in usePillarSwapAssets hook | ||
| const assets = await getPillarSwapAssets(); | ||
| const assets = queryTokenData({}); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Removed error handling and switched to synchronous call.
The implementation has been changed from an asynchronous call with error handling to a synchronous call to queryTokenData({}). The comment about error handling in the hook remains, but is now irrelevant as that error handling mechanism was removed with the hook.
Consider:
- Removing the outdated comment about error handling
- Verifying that any potential errors from
queryTokenDataare properly handled elsewhere
- // Error handled in usePillarSwapAssets hook
const assets = queryTokenData({});🏁 Script executed:
#!/bin/bash
# Check if queryTokenData has any error handling mechanisms
rg -A 5 "queryTokenData\s*=\s*function" --type typescriptLength of output: 94
I'm issuing a revised shell script to further investigate the queryTokenData function since the initial command failed due to an unrecognized file type. This script will search within .ts and .tsx files to locate its definition and give us hints on whether any error handling (like try/catch) is implemented:
Action Required:
Please run the above script and verify if the implementation of queryTokenData includes any error handling mechanisms (e.g., a try/catch block or similar), and confirm whether any potential errors from this synchronous call are managed elsewhere. Based on that verification, the outdated comment in CardsSwap.tsx (lines 60–62) should be removed if error handling is no longer applicable.
🏁 Script executed:
#!/bin/bash
# Updated search for the definition of queryTokenData in .ts and .tsx files
rg -g "*.ts" -g "*.tsx" -A 10 "queryTokenData\s*="Length of output: 584
Remove outdated error handling comment in CardsSwap.tsx
After verifying the queryTokenData implementation in src/services/tokensData.ts, it’s clear that error handling (such as try/catch) is not present since the function is synchronous. The original comment stating "Error handled in usePillarSwapAssets hook" is now misleading and should be removed.
Please update the code as follows:
- Remove the comment about error handling from
CardsSwap.tsx(lines 60–62). - Ensure that any potential errors are managed appropriately at a higher level if needed in the future.
Revised diff:
- // Error handled in usePillarSwapAssets hook
const assets = queryTokenData({});📝 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.
| // Error handled in usePillarSwapAssets hook | |
| const assets = await getPillarSwapAssets(); | |
| const assets = queryTokenData({}); | |
| const assets = queryTokenData({}); |
There was a problem hiding this comment.
@RanaBug Thanks for confirming you've removed the outdated error handling comment! The code is now more accurate without the misleading comment about error handling in the usePillarSwapAssets hook.
| const allChains = tokenListData | ||
| ?.map((chain) => chain.blockchain) | ||
| .map((chain) => chainNameToChainIdTokensData(chain)); |
There was a problem hiding this comment.
Add null check for tokenListData
The optional chaining operator (?.) is used on tokenListData, but there's no null check before using allChains in the subsequent filter operation on line 44. If tokenListData is null or undefined, allChains will be undefined, and the filter operation will throw an error.
- const allChains = tokenListData
- ?.map((chain) => chain.blockchain)
- .map((chain) => chainNameToChainIdTokensData(chain));
+ const allChains = tokenListData
+ ?.map((chain) => chain.blockchain)
+ .map((chain) => chainNameToChainIdTokensData(chain)) || [];📝 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 allChains = tokenListData | |
| ?.map((chain) => chain.blockchain) | |
| .map((chain) => chainNameToChainIdTokensData(chain)); | |
| const allChains = tokenListData | |
| ?.map((chain) => chain.blockchain) | |
| .map((chain) => chainNameToChainIdTokensData(chain)) || []; |
There was a problem hiding this comment.
@IAmKio Great! I've noted your approval of the null check suggestion. This change will prevent errors when tokenListData is null or undefined, making the code more robust.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| expect(store.getState().tokenAtlas.selectedToken).toEqual({ | ||
| name: 'TestToken', | ||
| symbol: 'TTK', | ||
| chainId: 1, | ||
| icon: 'testLogo.png', | ||
| decimals: 6, | ||
| address: '0x06', | ||
| }); |
There was a problem hiding this comment.
Mismatch between stored token and assertion
The test expects legacy fields (chainId, icon, address) instead of blockchain, logo, contract. This will likely fail or not reflect the real data shape.
Consider updating the assertion to align with the new structure:
- expect(store.getState().tokenAtlas.selectedToken).toEqual({
- name: 'TestToken',
- symbol: 'TTK',
- chainId: 1,
- icon: 'testLogo.png',
- decimals: 6,
- address: '0x06',
- });
+ expect(store.getState().tokenAtlas.selectedToken).toEqual({
+ id: 5,
+ name: 'TestToken',
+ symbol: 'TTK',
+ blockchain: 'Ethereum',
+ logo: 'testLogo.png',
+ decimals: 6,
+ contract: '0x06',
+ });📝 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.
| expect(store.getState().tokenAtlas.selectedToken).toEqual({ | |
| name: 'TestToken', | |
| symbol: 'TTK', | |
| chainId: 1, | |
| icon: 'testLogo.png', | |
| decimals: 6, | |
| address: '0x06', | |
| }); | |
| expect(store.getState().tokenAtlas.selectedToken).toEqual({ | |
| id: 5, | |
| name: 'TestToken', | |
| symbol: 'TTK', | |
| blockchain: 'Ethereum', | |
| logo: 'testLogo.png', | |
| decimals: 6, | |
| contract: '0x06', | |
| }); |
| if (!assetOption.asset?.contract) return; | ||
|
|
||
| const assetAddress = assetOption.asset.address; | ||
| const assetAddress = assetOption.asset.contract; |
There was a problem hiding this comment.
Fix missing return value in conditional.
The conditional check for assetOption.asset?.contract is missing a return value when the condition is false. This could lead to unexpected filtering behavior.
Apply this fix:
- if (!assetOption.asset?.contract) return;
+ if (!assetOption.asset?.contract) return false;📝 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.
| if (!assetOption.asset?.contract) return; | |
| const assetAddress = assetOption.asset.address; | |
| const assetAddress = assetOption.asset.contract; | |
| if (!assetOption.asset?.contract) return false; | |
| const assetAddress = assetOption.asset.contract; |
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
Just a few comments but none of them are show stoppers - LGTM!
| fromChainId: chainNameToChainIdTokensData(swapToken?.blockchain) ?? 0, | ||
| fromTokenDecimals: swapToken?.decimals ?? 0, | ||
| toTokenAddress: receiveToken?.address ?? '', | ||
| toChainId: receiveToken?.chainId ?? 0, | ||
| toTokenAddress: receiveToken?.contract ?? '', | ||
| toChainId: chainNameToChainIdTokensData(receiveToken?.blockchain) ?? 0, |
There was a problem hiding this comment.
I know we're doing this elsewhere around this code block but should 0 actually be null? The reason i'm thinking is that 0 is something whereas null is definitely nothing... just a thought
There was a problem hiding this comment.
I know what you mean, but there is an error handling in place if something is off anyway. The params are all mandatory
| const allChains = tokenListData | ||
| ?.map((chain) => chain.blockchain) | ||
| .map((chain) => chainNameToChainIdTokensData(chain)); |
| const { logout } = useLogout(); | ||
| const [t] = useTranslation(); | ||
| const assets = useAssets(); | ||
| // const assets = useAssets(); |
There was a problem hiding this comment.
| // const assets = useAssets(); |
Deploying x with
|
| Latest commit: |
ae12a44
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://69bb0cf1.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3036-mobula-token-d.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3036/Mobula-Token-Data)
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3036/Mobula-Token-Data)
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3036/Mobula-Token-Data)
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3036/Mobula-Token-Data)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx(6 hunks)src/components/BottomMenuModal/AccountModal.tsx(6 hunks)src/components/Form/AssetSelect/index.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/BottomMenuModal/AccountModal.tsx
- src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
src/components/Form/AssetSelect/index.tsx (4)
76-78: Good implementation of service-based token data retrieval.The replacement of the useAssets hook with direct queryTokenData service calls aligns with the standardization efforts for token data modeling across the codebase. This approach is more maintainable and provides a centralized way to access token data.
163-165: Fix for missing return value in conditional is implemented correctly.The conditional checks for native assets and token addresses now properly return boolean values, fixing the previous issue where a return value was missing.
133-133:Details
✅ Verification successful
Verify if removing 'assets' from dependency array is intentional.
The 'assets' variable has been removed from the useEffect dependency array, which means the effect won't re-run when assets change. This could lead to stale data if assets are updated elsewhere. Ensure this is intentional behavior.
🏁 Script executed:
#!/bin/bash # Check if assets are expected to change during component lifecycle rg -A 5 "queryTokenData" src --type tsLength of output: 6195
Confirmed: The exclusion of 'assets' from the dependency array is intentional.
After reviewing the usage ofqueryTokenDataacross the codebase, it’s clear thatassetsis derived directly from thechainId(viachainIdToChainNameTokensData(chainId)) and is re-computed on every render. This pattern is consistently applied in similar components (e.g., in CardsSwap and usePillarSwapAssets), where includingassetsin the dependency array would lead to unnecessary re-executions or even potential infinite loops ifqueryTokenDatareturns a new reference every time. Therefore, the current implementation ensuring thatuseEffectruns only whenwalletAddressorchainIdchanges is both intentional and correct.
89-96:Details
✅ Verification successful
Verify token ID mapping compatibility.
The component now uses
asset.idfor the option ID instead of the previousasset.address. Ensure this is consistent with how tokens are identified elsewhere in the codebase, especially for defaultSelectedId matching.
🏁 Script executed:
#!/bin/bash # Check how token IDs are constructed elsewhere rg "id: `\${chainId}:" src --type tsLength of output: 116
I'm noticing that the original regex command produced an error due to quoting issues. To gather the necessary information on how token IDs are constructed elsewhere in the codebase, let's re-run the search with corrected quoting.
Please run the above script so we can verify how tokens are identified elsewhere in the repository before finalizing the review comment.
🏁 Script executed:
#!/bin/bash # Re-run token ID mapping verification with corrected query syntax: rg 'id: "\${chainId}:[^"]+"' -t tsLength of output: 150
Let's try a simpler search to locate any usage of the token ID construction pattern across the codebase. I'd like to run a new shell script that looks for the pattern
"${chainId}:"in TypeScript files:Please run the above script so we can further verify how token IDs are built elsewhere in the repository.
🏁 Script executed:
#!/bin/bash # Search for any token ID constructions that use the chainId pattern in TypeScript files rg -P '\$\{chainId\}:' -t tsLength of output: 373
Token ID mapping appears consistent.
The AssetSelect component now constructs IDs asid: `${chainId}:${asset.id}`,and our search shows this pattern is used in the same file without any conflicting references to
asset.address. There’s also a slightly different ID construction in AssetsList (using an extra token part), which is acceptable if it targets a different context (e.g., NFTs vs. fungible tokens). Please verify manually that the defaultSelectedId matching logic (if applicable) in your integration aligns with the updated${chainId}:${asset.id}format.
| const isNativeAsset = | ||
| assetOption.asset?.contract && | ||
| (nativeTokens.includes(assetOption.asset.contract) || | ||
| isZeroAddress(assetOption.asset.contract)); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure null checking for contract property.
The code checks if assetOption.asset?.contract exists before using it, but doesn't handle the case where it's undefined or null in the subsequent condition. Consider adding additional null checks or providing default values.
🏁 Script executed:
#!/bin/bash
# Check if Token interface enforces contract property as required
rg -A 10 "interface Token" src --type tsLength of output: 1761
Action Required: Explicitly Check for Null/Undefined Contract Values
The current logic relies on optional chaining to avoid errors when assetOption.asset or its contract is falsy. However, since the Token interface doesn’t enforce a non-null contract property (as confirmed by the grep search), it’s safer and clearer to explicitly extract and check the contract value before using it. For example:
- const isNativeAsset =
- assetOption.asset?.contract &&
- (nativeTokens.includes(assetOption.asset.contract) ||
- isZeroAddress(assetOption.asset.contract));
+ const contract = assetOption.asset?.contract;
+ const isNativeAsset =
+ contract != null &&
+ (nativeTokens.includes(contract) || isZeroAddress(contract));This change makes the null/undefined check explicit and improves code clarity and maintainability.
📝 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 isNativeAsset = | |
| assetOption.asset?.contract && | |
| (nativeTokens.includes(assetOption.asset.contract) || | |
| isZeroAddress(assetOption.asset.contract)); | |
| const contract = assetOption.asset?.contract; | |
| const isNativeAsset = | |
| contract != null && | |
| (nativeTokens.includes(contract) || isZeroAddress(contract)); |
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3036/Mobula-Token-Data)
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit