-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
devop: new networks #603
devop: new networks #603
Conversation
Release: v2.0.1
Release: v2.0.2
Release 2.1.0
Signed-off-by: dung5ire <[email protected]>
feat: add 5ireChain
WalkthroughThis pull request introduces updates to the Ethereum network providers, focusing on network configurations for Syscoin NEVM, Rollux, and 5ire Chain. The changes include modifying network names, updating import paths, adding new network configurations, and enhancing token balance retrieval mechanisms. Key modifications involve replacing Syscoin with Syscoin NEVM, introducing 5ireChain, updating token lists and balance handling, and restructuring network-related files and imports. Changes
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (1)
✨ Finishing Touches
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 |
Update Syscoin and Rollux integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts (1)
Missing network configuration file for Sanko network
All networks added to
SupportedNetworkNames
have their corresponding configuration files except for Sanko network. While Sanko is referenced in the network index, it appears to be missing its dedicated configuration file.The following networks are properly configured:
- Degen (
degen.ts
)- Blast (
blast.ts
)- XLayer (
xlayer.ts
)- ProofOfPlayApex (
pop-apex.ts
)- Godwoken (
godwoken.ts
)- Linea (
linea.ts
)- MantaPacific (
manta-pacific.ts
)- Mode (
mode.ts
)- OpBNB (
op-bnb.ts
)- Scroll (
scroll.ts
)- Rari (
rari.ts
)🔗 Analysis chain
Line range hint
54-68
: Verify network configuration files exist.Multiple networks have been added to
SupportedNetworkNames
. Please ensure that corresponding network configuration files exist for each new network.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for network configuration files echo "Searching for network configuration files..." fd -e ts -e js "sanko|degen|blast|xlayer|proofofplayapex|godwoken|linea|mantapacific|mode|opbnb|scroll|rari"Length of output: 847
Script:
#!/bin/bash # Search for missing network configs with flexible patterns echo "Searching for missing network configs..." rg -l "sanko|proof.*play|manta|opbnb|pacific" packages/extension/src/providers/ethereum/networks/ echo -e "\nVerifying network configuration structure..." for file in packages/extension/src/providers/ethereum/networks/*.ts; do echo -e "\n=== $file ===" # Look for standard network configuration exports rg "export const \w+Config = {" "$file" || rg "export default {" "$file" doneLength of output: 37960
Script:
#!/bin/bash # Search for Sanko network configuration with more variations echo "Searching for Sanko network configuration..." rg -l "sanko|sanc?ko" packages/extension/src/providers/ethereum/networks/Length of output: 292
🧹 Nitpick comments (5)
packages/extension/src/providers/ethereum/networks/5ire.ts (1)
6-20
: Add buyLink property for consistency.Other network configurations include a
buyLink
property. Consider adding it for consistency if there's a faucet or exchange available for 5IRE tokens.packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts (2)
34-37
: Consider implementing rate limiting.Multiple concurrent requests to the Blockscout API might trigger rate limiting. Consider implementing request queuing or rate limiting.
79-87
: Enhance error handling specificity.The catch-all error handler returns a zero balance without distinguishing between different types of errors (network, API, parsing). Consider implementing more specific error handling.
.catch(error => { console.error('Error fetching balances:', error); + // Log specific error types for better debugging + if (error instanceof TypeError) { + console.error('Network error:', error); + } else if (error instanceof SyntaxError) { + console.error('JSON parsing error:', error); + } return [ { contract: NATIVE_TOKEN_ADDRESS, balance: '0x0', }, ]; });packages/extension/src/providers/ethereum/networks/index.ts (1)
65-65
: Consider renaming the 5ire import and key.While the implementation works, consider renaming
_5ireNode
tofireNode
and usingfire
as the key instead of'5ire'
for consistency with other network naming patterns.-import _5ireNode from './5ire'; +import fireNode from './fire'; export default { - '5ire': _5ireNode, + fire: fireNode, };Also applies to: 141-141
packages/extension/src/ui/action/App.vue (1)
365-384
: Consider adding error handling.The function should handle cases where:
- Selected account is undefined
- Network display address fails
- Buy link is undefined
const buyLink = (() => { + if (!accountHeaderData.value.selectedAccount) { + throw new Error('No account selected'); + } switch (currentNetwork.value.name) { case NetworkNames.KadenaTestnet: - return (currentNetwork.value as KadenaNetwork).options.buyLink; + const kadenaLink = (currentNetwork.value as KadenaNetwork).options.buyLink; + if (!kadenaLink) throw new Error('Buy link not configured'); + return kadenaLink; // ... rest of the cases default: return `https://ccswap.myetherwallet.com/?to=${currentNetwork.value.displayAddress( accountHeaderData.value.selectedAccount!.address, )}&network=${currentNetwork.value.name}&crypto=${ currentNetwork.value.currencyName }&platform=enkrypt`; } })(); +if (!buyLink) throw new Error('Failed to generate buy link'); Browser.tabs.create({ url: buyLink, });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/extension/src/providers/ethereum/networks/icons/5ire.svg
is excluded by!**/*.svg
📒 Files selected for processing (19)
README.md
(2 hunks)packages/extension/src/providers/ethereum/libs/activity-handlers/providers/etherscan/configs.ts
(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts
(3 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts
(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/token-lists.ts
(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts
(2 hunks)packages/extension/src/providers/ethereum/networks/5ire.ts
(1 hunks)packages/extension/src/providers/ethereum/networks/bsc.ts
(0 hunks)packages/extension/src/providers/ethereum/networks/form.ts
(1 hunks)packages/extension/src/providers/ethereum/networks/index.ts
(4 hunks)packages/extension/src/providers/ethereum/networks/sys.ts
(0 hunks)packages/extension/src/providers/ethereum/networks/syscoin/nevm-testnet.ts
(1 hunks)packages/extension/src/providers/ethereum/networks/syscoin/nevm.ts
(1 hunks)packages/extension/src/providers/ethereum/networks/syscoin/rollux-testnet.ts
(2 hunks)packages/extension/src/providers/ethereum/networks/syscoin/rollux.ts
(2 hunks)packages/extension/src/providers/ethereum/networks/tsys.ts
(0 hunks)packages/extension/src/providers/ethereum/types/evm-network.ts
(2 hunks)packages/extension/src/ui/action/App.vue
(1 hunks)packages/types/src/networks.ts
(2 hunks)
💤 Files with no reviewable changes (3)
- packages/extension/src/providers/ethereum/networks/bsc.ts
- packages/extension/src/providers/ethereum/networks/sys.ts
- packages/extension/src/providers/ethereum/networks/tsys.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: buildAll
- GitHub Check: test
🔇 Additional comments (21)
packages/extension/src/providers/ethereum/networks/5ire.ts (1)
16-16
: Consider using WSS endpoint for enhanced security.The current HTTP RPC endpoint could be vulnerable to man-in-the-middle attacks. Consider using a secure WebSocket (WSS) endpoint if available.
packages/extension/src/providers/ethereum/networks/form.ts (2)
15-16
: Verify currency configuration.The network is Form Mainnet but uses ETH as currency. This seems inconsistent. Please verify if this is correct or if Form has its own native token.
19-19
: Review activity handler configuration.The network uses
EtherscanActivity
handler but Form doesn't appear to use Etherscan. Consider implementing a custom activity handler or using a more appropriate one.packages/extension/src/providers/ethereum/networks/syscoin/rollux-testnet.ts (1)
1-1
: LGTM!The changes look good:
- Import paths are correctly updated
- Added buyLink property pointing to the correct faucet
- Configuration maintains security best practices with WSS endpoint
Also applies to: 3-4, 19-19
packages/extension/src/providers/ethereum/networks/syscoin/nevm-testnet.ts (1)
7-21
: LGTM!The network configuration is complete and follows best practices:
- Uses secure WSS endpoint
- Includes faucet link
- Chain ID matches official documentation
packages/extension/src/providers/ethereum/networks/syscoin/rollux.ts (2)
1-1
: LGTM! Import paths updated correctly.The import paths have been properly updated to reflect the new directory structure.
Also applies to: 3-5
23-23
: Verify the buyLink functionality.The buyLink includes multiple cryptocurrencies (SYSROLLUX, SYSEVM, SYS). Please ensure that:
- The Coinify trade URL is accessible
- All listed cryptocurrencies are available for purchase
packages/extension/src/providers/ethereum/networks/syscoin/nevm.ts (2)
19-20
: Potential conflict in coingeckoID configuration.Both Rollux and Syscoin NEVM networks use the same
coingeckoID: "syscoin"
. This might cause issues with price tracking as both networks would fetch the same price data.✅ Verification successful
Configuration is correct - no conflict in price tracking
The shared
coingeckoID: "syscoin"
is not problematic because each network uses a differentcoingeckoPlatform
value:
- NEVM:
CoingeckoPlatform.Syscoin
- Rollux:
CoingeckoPlatform.Rollux
The combination of both values ensures unique price tracking for each network.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of coingeckoID: "syscoin" rg -l 'coingeckoID.*syscoin' --type tsLength of output: 176
Script:
#!/bin/bash # Get the complete coingecko configuration from both files echo "=== NEVM Configuration ===" rg "coingecko[A-Z][a-zA-Z]+.*" packages/extension/src/providers/ethereum/networks/syscoin/nevm.ts -A 1 echo -e "\n=== Rollux Configuration ===" rg "coingecko[A-Z][a-zA-Z]+.*" packages/extension/src/providers/ethereum/networks/syscoin/rollux.ts -A 1Length of output: 510
8-25
: Verify network configuration parameters.Please ensure:
- The WebSocket endpoint is operational
- The block explorer endpoints are accessible
- The chain ID is correctly registered with chainlist.org
✅ Verification successful
Network configuration parameters verified successfully
All critical network parameters are correctly configured:
- Chain ID 0x39 is officially registered on chainlist.org
- Block explorer endpoints are operational and correctly formatted
- WebSocket endpoint is officially listed and matches chainlist.org data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify WebSocket endpoint curl --include --no-buffer \ --header "Connection: Upgrade" \ --header "Upgrade: websocket" \ --header "Sec-WebSocket-Version: 13" \ "wss://rpc.syscoin.org/wss" # Verify block explorer endpoints curl -I "https://explorer.syscoin.org/" # Verify chain ID registration curl -s "https://chainid.network/chains.json" | jq '.[] | select(.chainId == 57)'Length of output: 2975
Script:
#!/bin/bash # Install wscat for WebSocket testing npm install -g wscat # Test WebSocket connection timeout 5 wscat -c wss://rpc.syscoin.org/wss # Test specific explorer endpoints with a known transaction and address curl -I "https://explorer.syscoin.org/tx/0x0000000000000000000000000000000000000000000000000000000000000000" curl -I "https://explorer.syscoin.org/address/0x0000000000000000000000000000000000000000"Length of output: 5824
packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts (1)
8-8
: Interface changes look good but need verification.The optional properties
tbName
andbsEndpoint
have been added correctly. However, please verify that all implementations handle these optional properties appropriately.Also applies to: 10-10
✅ Verification successful
Optional properties are properly implemented and handled
The verification shows that both
tbName
andbsEndpoint
optional properties are correctly implemented inassetinfo-mew.ts
with proper fallbacks and conditional checks. The code handles undefined cases appropriately.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of tbName and bsEndpoint ast-grep --pattern 'interface $_ { $$$ tbName$_ $$$ }' ast-grep --pattern 'interface $_ { $$$ bsEndpoint$_ $$$ }'Length of output: 129
Script:
#!/bin/bash # First, let's find the interface name and its usages echo "=== Interface Definition ===" rg -A 5 "interface.*\{" "packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts" echo -e "\n=== Interface Usages ===" # Search for any imports of this file rg -l "tokenbalance-mew" packages/extension/src/providers/ethereum/ echo -e "\n=== Property Access Patterns ===" # Search for tbName and bsEndpoint usage rg "tbName|bsEndpoint" packages/extension/src/providers/ethereum/Length of output: 5992
packages/types/src/networks.ts (2)
69-69
: LGTM! Network names follow consistent naming conventions.The new network entries maintain consistent naming patterns and preserve backward compatibility with existing identifiers.
Also applies to: 72-73
100-100
: Add CoingeckoPlatform entry for Fire network.The Fire network (5ire) is missing its corresponding entry in the CoingeckoPlatform enum, which might affect token list functionality.
Run this script to check if the network requires token list support:
packages/extension/src/providers/ethereum/networks/index.ts (1)
38-41
: LGTM! Well-organized network node structure.The Syscoin and Rollux networks are properly organized under the syscoin directory, indicating good code organization.
packages/extension/src/providers/ethereum/libs/activity-handlers/providers/etherscan/configs.ts (1)
52-52
: LGTM! Secure explorer endpoints configured.The new network explorer endpoints:
- Use HTTPS for secure communication
- Follow consistent naming patterns for test/main networks
- Point to official block explorers
Also applies to: 55-56
packages/extension/src/providers/ethereum/types/evm-network.ts (2)
37-37
: LGTM! Well-typed interface extension.The addition of the optional
buyLink
property maintains backward compatibility while extending the network configuration options.
102-102
: LGTM! Good practice storing options.Storing the options in the class enables access to network configuration after initialization, which is necessary for the new buyLink functionality.
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (3)
23-23
: LGTM! Clean import.The import follows the project's conventions and supports the new balance fetching functionality.
124-130
: LGTM! Clear network configurations.The configurations for SyscoinNEVM and Rollux are well-structured and consistently use the blockscout endpoint.
199-200
: LGTM! Clean integration of blockscout support.The addition of blockscout endpoint support is well-integrated into the existing control flow.
packages/extension/src/ui/action/App.vue (1)
365-384
: LGTM! Well-structured network handling.The switch statement provides clear, network-specific logic for buy links.
Verify the network configurations with:
✅ Verification successful
Verified: Network-specific buy links are properly handled
The switch statement correctly handles all networks with specific buyLink configurations, including:
- Syscoin NEVM and Rollux networks with Coinify integration
- Test networks with appropriate fallbacks
- Default case for other networks using ccswap
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all networks with buyLink option are handled in the switch statement ast-grep --pattern 'case NetworkNames.$_: return $_'Length of output: 54
Script:
#!/bin/bash # Find NetworkNames enum definition and network configurations echo "=== NetworkNames enum ===" rg "enum NetworkNames" -A 10 echo -e "\n=== Network configurations with buyLink ===" rg "buyLink.*=" -B 2 -A 2 echo -e "\n=== Network type definitions ===" ast-grep --pattern 'interface $_ extends Network { $$$ }'Length of output: 2891
README.md (1)
Line range hint
102-111
: LGTM! Documentation aligned with code changes.The supported chains list accurately reflects the network configuration changes in the codebase.
🧰 Tools
🪛 LanguageTool
[duplication] ~99-~99: Possible typo: you repeated a word.
Context: ...ck - Sanko Chain - Scroll - Shibarium - Shiden - Shiden EVM - Sepolia - Syscoin NEVM - Telos EV...(ENGLISH_WORD_REPEAT_RULE)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts
Show resolved
Hide resolved
[NetworkNames.SyscoinNEVM]: `https://tokens.coingecko.com/${CoingeckoPlatform.Syscoin}/all.json`, | ||
[NetworkNames.Rollux]: `https://tokens.coingecko.com/${CoingeckoPlatform.Rollux}/all.json`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate Rollux entry.
The Rollux network has two entries in the TokenList:
- Line 7:
[NetworkNames.Rollux]
- Line 27:
[NetworkNames.Rollux]
Please remove one of the duplicate entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension/package.json
(1 hunks)packages/extension/src/providers/common/libs/new-features.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/extension/package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: buildAll
- GitHub Check: test
🔇 Additional comments (1)
packages/extension/src/providers/common/libs/new-features.ts (1)
7-7
: Verify if "Fire" should be "5ire".The PR objective mentions "5ire" but the code uses "Fire". This might be a naming inconsistency.
Let's check for any references to both names:
Summary by CodeRabbit
New Features
Network Updates
Documentation
Version Update