devop: switch solana asset handler to helius #578
Conversation
WalkthroughThis pull request introduces modifications to token information retrieval across Ethereum and Solana providers. The changes primarily focus on enhancing token metadata handling, specifically by adding or modifying the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (5)
🔇 Additional comments (8)packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-alert.vue (2)
The conditional rendering logic has been enhanced to handle zero balance cases separately from other insufficient balance scenarios, providing clearer feedback to users.
The new prop enables more granular control over alert messages based on the balance state. packages/extension/src/providers/kadena/ui/send-transaction/index.vue (1)
The error message has been updated to "Not enough balance." to maintain consistency with other blockchain implementations in the codebase. packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (2)
The send-alert component now receives proper zero balance state through UTXOBalance.isZero(), enabling more precise error messaging.
The validation logic now properly handles numeric checks using BigNumber, preventing potential type coercion issues. packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (2)
The error message has been updated to be more user-friendly and consistent with other components.
The validation logic has been improved by explicitly checking if the send amount is a valid number using BigNumber. This helps prevent potential issues with invalid number inputs. packages/extension/src/providers/solana/ui/send-transaction/index.vue (1)
The error message has been updated to maintain consistency with the Ethereum implementation, providing a unified user experience across different blockchain networks. 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
Documentation and Community
|
|
💼 Build Files |
![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
🧹 Nitpick comments (3)
packages/extension/src/providers/solana/libs/api.ts (1)
26-26: Consider removing the emptyinitmethod if it's unnecessary.The
initmethod is currently empty and doesn't perform any initialization logic. If it's not required, consider removing it to keep the codebase clean.packages/extension/src/providers/ethereum/libs/assets-handlers/solanachain.ts (1)
Line range hint
40-44: Optimize balance aggregation logic.When accumulating balances, you can enhance performance by reducing the number of
toBNconversions and checking for existing contracts more efficiently.Apply this diff to optimize:
const contract = (acc.account.data as any).parsed.info.mint; - if (!balanceObj[contract]) balanceObj[contract] = toBN(0); - balanceObj[contract] = balanceObj[contract].add(toBN(balance)); + if (balanceObj[contract]) { + balanceObj[contract] = balanceObj[contract].add(toBN(balance)); + } else { + balanceObj[contract] = toBN(balance); + }packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (1)
338-341: Improved icon fallback chain enhances token display reliabilityThe enhanced icon resolution logic provides a more robust fallback chain:
- First attempts to use the known token list's logo URI
- Then falls back to the token info's icon (new addition)
- Finally defaults to the network icon
This improvement ensures better visual representation of tokens, especially for unknown or custom tokens.
Consider adding a default placeholder icon as the final fallback instead of using the network icon, to better distinguish between network and token icons.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/extension/src/providers/ethereum/libs/api.ts(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/solanachain.ts(1 hunks)packages/extension/src/providers/ethereum/types/index.ts(1 hunks)packages/extension/src/providers/solana/libs/api.ts(3 hunks)packages/extension/src/providers/solana/types/sol-token.ts(1 hunks)
🔇 Additional comments (5)
packages/extension/src/providers/solana/libs/api.ts (1)
85-92: Ensure the JSON-RPC request parameters are correctly structured.
In the cacheFetch request, verify that the JSON-RPC parameters match the API's expected format. The getAsset method may require specific parameters, and any mismatch could result in unexpected responses.
Run the following script to validate the API response structure:
Replace "$1" with a valid contract address and "$NODE_URL" with the node URL. Check if the response contains the expected token_info and content fields.
✅ Verification successful
JSON-RPC parameters for getAsset are correctly structured
Looking at the codebase, the implementation and usage of getAsset method is consistent. The response type is properly typed and handled in the code:
- In
api.ts, it's used to fetch token information with correct parameter structure{ id: contractAddress } - The same structure is used in
decode-tx.tsand the response fields match the expected type - The response handling shows the expected structure contains
result.token_infoandresult.contentfields, which aligns with the implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the getAsset method returns the expected structure.
curl -s -X POST -H "Content-Type: application/json" -d '{
"jsonrpc": "2.0",
"id": 1,
"method": "getAsset",
"params": {
"id": "'"$1"'"
}
}' "$NODE_URL" | jq '.result'
Length of output: 164
Script:
#!/bin/bash
# Let's check the API implementation and usage in the codebase
rg -A 10 "getAsset" --type ts
Length of output: 3625
Script:
#!/bin/bash
# Let's also check if there's any documentation or interface defining the expected parameters
fd -e ts -e md "interface|type" --exec grep -l "getAsset" {} \; | xargs cat
Length of output: 78
Script:
#!/bin/bash
# Check the entire file for context
cat packages/extension/src/providers/solana/libs/api.ts
Length of output: 3254
packages/extension/src/providers/solana/types/sol-token.ts (1)
6-6: Ensure compatibility with the updated ERC20TokenInfo interface.
By adding icon?: string to the ERC20TokenInfo interface, make sure that any implementations of SPLTokenInfo correctly handle the new optional icon property inherited from ERC20TokenInfo.
Check the codebase for any instances where SPLTokenInfo objects are created or manipulated to ensure that the icon property is appropriately handled.
packages/extension/src/providers/ethereum/types/index.ts (1)
63-63: Addition of optional icon property to ERC20TokenInfo.
The icon property allows tokens to include an icon URL for display purposes. Ensure that all implementations and usages of ERC20TokenInfo accommodate this new optional property.
packages/extension/src/providers/ethereum/libs/api.ts (1)
79-79: LGTM: Icon field addition maintains interface consistency
The addition of the icon: undefined field in the error case ensures consistent token information structure, properly handling cases where token information cannot be retrieved.
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (1)
338-341: Verify cross-provider icon handling compatibility
Since these changes are part of switching to Helius for Solana assets, we should verify that the icon handling logic is compatible across both Ethereum and Solana providers.
✅ Verification successful
Let me analyze the icon handling patterns across providers based on the search results. I'll generate additional scripts to verify the specific icon fallback implementation.
This will help us verify if the icon fallback chain is consistently implemented across providers and if the token info structures are compatible.
Based on the analysis of the search results, I can now provide a final response regarding the cross-provider icon handling compatibility:
Icon handling is consistently implemented across providers
The verification shows that:
- All providers (Ethereum, Solana, Bitcoin, Polkadot, Kadena) implement the same icon field structure in their network and token interfaces
- The icon fallback chain in the Ethereum MEW asset handler (
tokenInfo[unknownTokens[idx]]?.logoURI || tInfo.icon || network.icon) follows a standard pattern that's compatible with Solana's token info structure - Both providers use string-based icon URLs and have consistent icon field definitions in their base interfaces
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent icon handling across providers
# Check for icon-related code in Solana provider
echo "Checking Solana provider icon handling..."
rg -l "icon" packages/extension/src/providers/solana/
# Check for similar icon fallback patterns
echo "Checking icon fallback patterns..."
rg "icon(\s+)?[=:]" packages/extension/src/providers/
Length of output: 21433
Script:
#!/bin/bash
# Check icon fallback implementation in Solana and Ethereum providers
# Check Solana token info structure and icon handling
echo "Checking Solana token info structure..."
ast-grep --pattern 'interface $_ {
$$$
icon: $_
$$$
}'
# Check icon fallback patterns in asset handlers
echo "Checking icon fallback patterns in asset handlers..."
rg -A 3 "icon(\s+)?[:=]" packages/extension/src/providers/*/libs/assets-handlers/
Length of output: 23043
packages/extension/src/providers/ethereum/libs/assets-handlers/solanachain.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/extension/src/providers/solana/libs/api.ts (1)
98-109: Document the token info retrieval strategy.The code switches between Helius API and on-chain data, but this behavior isn't documented.
Add JSDoc comments to explain the fallback strategy:
+ /** + * Attempts to fetch token information in the following order: + * 1. Helius API for rich metadata including name, symbol, decimals, and icon + * 2. On-chain data for basic token information (decimals) + * 3. Falls back to default values if both attempts fail + */ if (tokenResponse.result) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/extension/src/providers/solana/libs/api.ts(3 hunks)
🔇 Additional comments (4)
packages/extension/src/providers/solana/libs/api.ts (4)
26-26: LGTM!
The empty implementation is appropriate as no initialization is required for the API wrapper.
36-36: LGTM!
Minor formatting change with no functional impact.
69-83:
Add null checks for nested properties.
The type definition suggests deep nesting, but the code doesn't safely handle cases where intermediate properties might be undefined.
Apply this diff to add safe property access:
if (tokenResponse.result) {
+ const { content, token_info } = tokenResponse.result;
+ if (!content?.metadata || !token_info) {
+ throw new Error('Invalid token metadata structure');
+ }
return {
- name: tokenResponse.result.content.metadata.name
- ? tokenResponse.result.content.metadata.name
- : tokenResponse.result.content.metadata.symbol,
- symbol: tokenResponse.result.content.metadata.symbol,
- decimals: tokenResponse.result.token_info.decimals,
+ name: content.metadata.name || content.metadata.symbol,
+ symbol: content.metadata.symbol,
+ decimals: token_info.decimals,Likely invalid or redundant comment.
69-109: Verify the impact of switching to Helius API.
The switch to Helius API is a significant change that warrants verification of its behavior across different token types.
Let's verify the token info retrieval for different types of tokens:
Consider implementing a caching strategy for frequently accessed tokens to reduce API calls and improve performance.
✅ Verification successful
Based on the search results, I can see that the Helius API integration is implemented with error handling in place, as shown in the decode-tx.ts file. However, let's verify the caching implementation and error handling in the main API file:
Based on the analysis of the codebase, I can now provide a final response:
The Helius API integration is properly implemented with adequate safeguards
The implementation includes:
- Proper caching mechanism through
cacheFetchwith a 6-hour TTL (6 * 60 * 60 * 1000) - Default fallback for decimals (9) if the API call fails
- Error handling through the optional chaining on
tokenResponse.result - Consistent response structure matching the Helius API format
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check token info retrieval for different token types
# Common SPL tokens
echo "Checking USDC token info..."
curl -s "$NODE_URL" \
-X POST \
-H "Content-Type: application/json" \
-d '{
"jsonrpc": "2.0",
"id": 1,
"method": "getAsset",
"params": {
"id": "EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v"
}
}'
# NFTs
echo "Checking NFT token info..."
curl -s "$NODE_URL" \
-X POST \
-H "Content-Type: application/json" \
-d '{
"jsonrpc": "2.0",
"id": 1,
"method": "getAsset",
"params": {
"id": "DezXAZ8z7PnrnRJjz3wXBoRgixCa6xjnB7YaB1pPB263"
}
}'
Length of output: 519
Script:
#!/bin/bash
# Let's analyze the codebase to understand the Helius API integration better
# Check for any error handling or fallback mechanisms
rg -A 5 "getAsset" --type ts
# Check for any test files related to token info retrieval
fd -e test.ts -e spec.ts -x cat {} \; | rg -A 10 "getAsset"
# Look for any configuration or constants related to Helius API
rg "helius" --type ts -i
# Check the implementation details around token info retrieval
ast-grep --pattern 'async function $FUNC($$$) {
$$$
getAsset
$$$
}'
Length of output: 2374
Script:
#!/bin/bash
# Check the cacheFetch implementation and error handling
rg -B 5 -A 10 "cacheFetch" --type ts
# Look for any token-related interfaces or types
ast-grep --pattern 'interface $NAME {
$$$
token_info
$$$
}'
# Check for any fallback mechanisms or default values
rg "decimals = \d+" --type ts
Length of output: 47328
Summary by CodeRabbit
New Features
iconproperty to the ERC20 token information structure for enhanced token representation.Bug Fixes
Refactor
Chores