Update Syscoin and Rollux integration #601
Conversation
WalkthroughThis pull request introduces changes to the codebase related to the Syscoin network, specifically transitioning from "Syscoin" to "Syscoin NEVM." Modifications span multiple files within the extension's provider ecosystem, updating network configurations, type definitions, and supporting infrastructure. The changes involve renaming network entries, updating import paths, and adding new network-specific configurations for both the main Syscoin NEVM and its testnet variant. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
![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 (1)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts (1)
1-24: Consider adding JSDoc documentation for interfaces.Adding JSDoc documentation for the interfaces would improve code maintainability and help other developers understand the expected data structure.
Add documentation like this:
+/** + * Represents a token balance with its contract address and quantity + */ interface TokenBalanceType { token: string; quantity: string; error?: unknown; } +/** + * Represents a single token item in the Blockscout API response + */ interface TokenResponseItem { token: { address: string; decimals: string; name: string; symbol: string; }; value: string; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
README.md(1 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/index.ts(2 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(1 hunks)packages/extension/src/providers/ethereum/networks/syscoin/rollux.ts(1 hunks)packages/extension/src/providers/ethereum/networks/tsys.ts(0 hunks)packages/types/src/networks.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/extension/src/providers/ethereum/networks/tsys.ts
- packages/extension/src/providers/ethereum/networks/sys.ts
🔇 Additional comments (14)
packages/extension/src/providers/ethereum/networks/syscoin/rollux-testnet.ts (1)
1-1: LGTM! Import paths correctly updated.The import paths have been properly updated to reflect the new directory structure while maintaining the same functionality.
Also applies to: 3-4
packages/extension/src/providers/ethereum/networks/syscoin/nevm.ts (1)
8-24: LGTM! Network configuration is properly implemented.The Syscoin NEVM network configuration is complete with:
- Correct chain ID (0x39)
- Secure WebSocket endpoint
- Proper integration with activity and asset handlers
- Appropriate block explorer URLs
packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts (1)
8-8: LGTM! Interface changes are backward compatible.The optional fields
tbNameandbsEndpointhave been properly added to support new functionality while maintaining compatibility with existing code.Also applies to: 10-10
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts (1)
80-80: LGTM!The export statement is correct.
packages/types/src/networks.ts (1)
71-72: Verify the impact of network renaming.The renaming from Syscoin to SyscoinNEVM maintains the same network identifiers, which is good for backward compatibility. However, we should verify that all dependent systems and documentation are updated accordingly.
Run this script to check for any remaining references to the old network names:
✅ Verification successful
Network renaming verification completed successfully
The renaming from Syscoin to SyscoinNEVM has been consistently implemented across the codebase while maintaining backward compatibility through unchanged network identifiers (SYS, TSYS). All relevant configurations, including network definitions, hardware wallet support, and asset handlers, have been properly updated to reflect the new naming.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for old network names in the codebase echo "Searching for old network names..." rg -i "syscointest|syscoin(?!nevm)" --type ts --type md # Search for network configuration files echo "Checking network configuration files..." fd -e ts -e json syscoinLength of output: 502
Script:
#!/bin/bash # Search for network-related files and their contents echo "=== Searching for network-related files ===" fd -e ts -e md -e json . | grep -i "network" echo -e "\n=== Searching for Syscoin references ===" rg -i "syscoin" --type ts --type md -A 2 -B 2 echo -e "\n=== Checking for network enum usage ===" ast-grep --pattern 'enum $_ { $$$ Syscoin $$$ }' echo -e "\n=== Checking network type definitions ===" ast-grep --pattern 'type $_ = $$$Syscoin$$$'Length of output: 22080
packages/extension/src/providers/ethereum/libs/activity-handlers/providers/etherscan/configs.ts (1)
54-55: Verify explorer endpoints are active and responding.The explorer URLs remain unchanged, which is good for continuity. However, we should verify that these endpoints are active and responding correctly with the new network names.
Run this script to check the endpoints:
✅ Verification successful
Explorer endpoints verified and operational
Both explorer endpoints are active and responding with proper blockchain explorer configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Test explorer endpoints endpoints=( "https://explorer.tanenbaum.io/" "https://explorer.syscoin.org/" ) for endpoint in "${endpoints[@]}"; do echo "Testing $endpoint..." curl -I "$endpoint" doneLength of output: 4051
packages/extension/src/providers/ethereum/networks/index.ts (2)
111-112: LGTM!The network exports are correctly updated to match the new import paths.
37-40: Verify the new import paths exist.The network nodes have been reorganized under the syscoin directory, which improves code organization. Let's verify that all the new import paths exist and are correctly structured.
Run this script to check the import paths:
✅ Verification successful
All network node imports verified successfully
The network node files are properly organized under the syscoin directory with the correct file names and extensions:
syscoin/nevm-testnet.tssyscoin/nevm.tssyscoin/rollux-testnet.tssyscoin/rollux.ts🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the new network node files exist files=( "syscoin/nevm-testnet" "syscoin/nevm" "syscoin/rollux-testnet" "syscoin/rollux" ) for file in "${files[@]}"; do echo "Checking for $file..." fd -e ts "$file" doneLength of output: 625
Script:
#!/bin/bash # Search for network node files in the correct directory cd packages/extension/src/providers/ethereum/networks 2>/dev/null || exit 1 echo "Checking files in networks directory:" ls -la syscoin/Length of output: 517
packages/extension/src/providers/ethereum/libs/assets-handlers/token-lists.ts (1)
6-7: LGTM! Token list entries added correctly.The new entries for SyscoinNEVM and Rollux follow the established pattern and are properly configured to fetch token data from Coingecko.
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (3)
23-23: LGTM! Blockscout integration added.The import of getBlockscoutBalances is correctly placed and aligns with the new functionality.
124-127: LGTM! Network configurations added correctly.Both SyscoinNEVM and Rollux are properly configured to use Blockscout for balance retrieval.
Also applies to: 130-130
199-200: LGTM! Blockscout integration logic implemented correctly.The conditional logic for using Blockscout is properly implemented and maintains the existing flow.
packages/extension/src/providers/ethereum/networks/syscoin/nevm-testnet.ts (1)
7-20: Verify the network configuration parameters.The network configuration appears correct, but please verify:
- Chain ID 0x1644 (5700 in decimal) matches the official Syscoin NEVM Testnet
- WebSocket endpoint is stable and production-ready
- Block explorer URLs are correct for the testnet
Run this script to verify the chain ID:
✅ Verification successful
Configuration verified and consistent across the codebase
The Syscoin NEVM Testnet configuration is correctly implemented:
- Block explorer URLs and WebSocket endpoint consistently use the Tanenbaum domain
- Network is properly integrated with activity handlers and network registry
- Configuration follows the standard testnet network pattern used across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify chain ID matches official documentation # Expect: Chain ID 0x1644 (5700) mentioned in official docs rg -i "chain.*id.*5700|chain.*id.*0x1644|syscoin.*testnet" README.md docs/Length of output: 122
Script:
#!/bin/bash # Find all network configuration files fd -e ts -e js networkLength of output: 1079
Script:
#!/bin/bash # Search for Syscoin-related files and configurations rg -i "syscoin|nevm" --type ts --type jsLength of output: 6480
Script:
#!/bin/bash # Search for chain ID references rg -i "0x1644|5700" --type ts --type jsLength of output: 1349
Script:
#!/bin/bash # Check block explorer and node configurations ast-grep --pattern 'const $_ = { $$$ blockExplorerTX: $explorer, $$$ node: $node, $$$ }'Length of output: 100
Script:
#!/bin/bash # Search for any Syscoin testnet specific configurations or documentation rg -i "tanenbaum|syscoin.*test" --type ts --type js -A 2 -B 2Length of output: 6381
README.md (1)
102-102: LGTM! Documentation updated correctly.The network name has been updated to reflect the new "Syscoin NEVM" branding, maintaining alphabetical order in the list.
packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts
Outdated
Show resolved
Hide resolved
| const getBlockscoutBalances = ( | ||
| chain: SupportedNetworkNames, | ||
| address: string | ||
| ): Promise<TokenBalance[]> => { | ||
| const nativeTokenUrl = `${NetworkEndpoints[chain]}api/v2/addresses/${address}`; | ||
| const tokenBalancesUrl = `${NetworkEndpoints[chain]}api/v2/addresses/${address}/tokens?type=ERC-20`; | ||
|
|
||
| return Promise.all([ | ||
| fetch(nativeTokenUrl).then((res) => res.json()), | ||
| fetch(tokenBalancesUrl).then((res) => res.json()), | ||
| ]) | ||
| .then(([nativeResponse, tokenResponse]: [any, TokenResponse]) => { | ||
| if (!nativeResponse || !tokenResponse) { | ||
| return Promise.reject("Error fetching balance data"); | ||
| } | ||
|
|
||
| // Map native token balance | ||
| const nativeBalance: TokenBalanceType = { | ||
| token: NATIVE_TOKEN_ADDRESS, | ||
| quantity: nativeResponse.coin_balance, | ||
| }; | ||
|
|
||
| // Map token balances | ||
| const tokenBalances: TokenBalanceType[] = Array.isArray( | ||
| tokenResponse?.items | ||
| ) | ||
| ? tokenResponse.items | ||
| .filter( | ||
| (item) => | ||
| item?.token?.address && | ||
| typeof item.token.address === "string" && | ||
| item.value !== undefined | ||
| ) | ||
| .map((item) => ({ | ||
| token: item.token.address.toLowerCase(), | ||
| quantity: item.value, | ||
| })) | ||
| : []; | ||
|
|
||
| // Merge native token and token balances | ||
| const allBalances = [nativeBalance, ...tokenBalances]; | ||
|
|
||
| // Convert to TokenBalance format | ||
| return allBalances.map((tb) => ({ | ||
| contract: tb.token, | ||
| balance: numberToHex(tb.quantity), // Convert to hex format | ||
| })); | ||
| }) | ||
| .catch((error) => { | ||
| console.error("Error fetching balances:", error); | ||
| throw error; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for invalid API responses.
The function should handle cases where the API response is malformed or contains unexpected data structures.
Consider these improvements:
return Promise.all([
fetch(nativeTokenUrl).then((res) => res.json()),
fetch(tokenBalancesUrl).then((res) => res.json()),
])
.then(([nativeResponse, tokenResponse]: [any, TokenResponse]) => {
- if (!nativeResponse || !tokenResponse) {
+ if (!nativeResponse?.coin_balance || !tokenResponse?.items) {
return Promise.reject("Error fetching balance data");
}
+ // Validate native token balance is a valid number
+ if (isNaN(Number(nativeResponse.coin_balance))) {
+ return Promise.reject("Invalid native token balance");
+ }
// Map native token balance
const nativeBalance: TokenBalanceType = {Also, consider implementing retry logic for failed API calls:
const fetchWithRetry = async (url: string, retries = 3): Promise<any> => {
try {
const response = await fetch(url);
return response.json();
} catch (error) {
if (retries > 0) {
await new Promise(resolve => setTimeout(resolve, 1000));
return fetchWithRetry(url, retries - 1);
}
throw error;
}
};
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts (2)
33-36: Add timeout handling for fetch requests.Network requests should have timeouts to prevent hanging in case of slow responses.
+const TIMEOUT_MS = 10000; + +const fetchWithTimeout = async (url: string) => { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), TIMEOUT_MS); + try { + const response = await fetch(url, { signal: controller.signal }); + const data = await response.json(); + clearTimeout(timeout); + return data; + } catch (error) { + clearTimeout(timeout); + throw error; + } +}; + return Promise.all([ - fetch(nativeTokenUrl).then((res) => res.json()), - fetch(tokenBalancesUrl).then((res) => res.json()), + fetchWithTimeout(nativeTokenUrl), + fetchWithTimeout(tokenBalancesUrl), ])
78-81: Enhance error logging with context.Include the chain and address in error messages to aid debugging.
- console.error("Error fetching balances:", error); + console.error(`Error fetching balances for chain ${chain} and address ${address}:`, error);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts(2 hunks)packages/extension/src/providers/ethereum/networks/syscoin/rollux.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension/src/providers/ethereum/networks/syscoin/rollux.ts
- packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts
[error] 42-42: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (2)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts (2)
1-24: Well-structured type definitions!The interfaces provide good type safety for the API responses and internal data structures.
46-77: Clean implementation of balance processing!The code properly handles:
- Filtering of invalid token data
- Case normalization for addresses
- Conversion to hex format
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts
Outdated
Show resolved
Hide resolved
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts (3)
12-20: Consider usingnumbertype fordecimals.The
decimalsfield inTokenResponseIteminterface is typed asstring, but it represents a numeric value. Consider changing it tonumberfor better type safety and to avoid unnecessary string-to-number conversions.token: { address: string; - decimals: string; + decimals: number; name: string; symbol: string; };
34-37: Add timeout handling for fetch calls.The fetch calls could hang indefinitely. Consider adding a timeout to ensure responsive error handling:
const fetchWithTimeout = async (url: string, timeout = 5000) => { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), timeout); try { const response = await fetch(url, { signal: controller.signal }); clearTimeout(timeoutId); return response.json(); } catch (error) { clearTimeout(timeoutId); throw error; } };Then use it in the Promise.all:
return Promise.all([ - fetch(nativeTokenUrl).then((res) => res.json()), - fetch(tokenBalancesUrl).then((res) => res.json()), + fetchWithTimeout(nativeTokenUrl), + fetchWithTimeout(tokenBalancesUrl), ])
79-82: Improve error context in catch block.The error handling could provide more context about which operation failed:
.catch((error) => { - console.error("Error fetching balances:", error); + const errorMessage = error instanceof Error ? error.message : String(error); + console.error(`Failed to fetch balances for address ${address} on chain ${chain}:`, errorMessage); throw error; });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts(1 hunks)
🔇 Additional comments (1)
packages/extension/src/providers/ethereum/libs/assets-handlers/blockscout.ts (1)
85-85: LGTM!Clean and clear export statement.
| return allBalances.map((tb) => ({ | ||
| contract: tb.token, | ||
| balance: numberToHex(tb.quantity), // Convert to hex format | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for hex conversion.
The numberToHex conversion could fail with invalid inputs. Consider adding error handling:
return allBalances.map((tb) => ({
contract: tb.token,
- balance: numberToHex(tb.quantity), // Convert to hex format
+ balance: (() => {
+ try {
+ return numberToHex(tb.quantity);
+ } catch (error) {
+ console.error(`Failed to convert balance to hex for token ${tb.token}:`, error);
+ return '0x0';
+ }
+ })(),
}));📝 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.
| return allBalances.map((tb) => ({ | |
| contract: tb.token, | |
| balance: numberToHex(tb.quantity), // Convert to hex format | |
| })); | |
| return allBalances.map((tb) => ({ | |
| contract: tb.token, | |
| balance: (() => { | |
| try { | |
| return numberToHex(tb.quantity); | |
| } catch (error) { | |
| console.error(`Failed to convert balance to hex for token ${tb.token}:`, error); | |
| return '0x0'; | |
| } | |
| })(), | |
| })); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/extension/src/providers/ethereum/types/evm-network.ts (1)
102-102: Add JSDoc for the new options property.The addition of
this.optionsis good for accessing all network options. Consider adding documentation to explain its purpose and usage.+ /** All network options passed during instantiation. */ this.options = options;packages/extension/src/ui/action/App.vue (2)
366-366: Remove debug console.log statement.Production code should not contain console.log statements.
- console.log([currentNetwork.value, (currentNetwork.value as EvmNetwork).options]);
365-385: Refactor the buyLink logic for better maintainability.The current implementation could be improved by:
- Extracting the default URL construction to a separate function
- Combining similar cases
- Using early returns instead of a switch statement
- const buyLink = (() => { - switch (currentNetwork.value.name) { - case NetworkNames.KadenaTestnet: - return (currentNetwork.value as KadenaNetwork).options.buyLink; - case NetworkNames.SyscoinNEVM: - case NetworkNames.Rollux: - return `${(currentNetwork.value as EvmNetwork).options.buyLink}&address=${currentNetwork.value.displayAddress( - accountHeaderData.value.selectedAccount!.address - )}`; - case NetworkNames.SyscoinNEVMTest: - case NetworkNames.RolluxTest: - return (currentNetwork.value as EvmNetwork).options.buyLink; - default: - return `https://ccswap.myetherwallet.com/?to=${currentNetwork.value.displayAddress( - accountHeaderData.value.selectedAccount!.address, - )}&network=${currentNetwork.value.name}&crypto=${ - currentNetwork.value.currencyName - }&platform=enkrypt`; - } - })(); + const getDefaultBuyLink = () => { + const address = currentNetwork.value.displayAddress( + accountHeaderData.value.selectedAccount!.address + ); + return `https://ccswap.myetherwallet.com/?to=${address}&network=${currentNetwork.value.name}&crypto=${currentNetwork.value.currencyName}&platform=enkrypt`; + }; + + const network = currentNetwork.value; + const options = (network as EvmNetwork).options; + + if (network.name === NetworkNames.KadenaTestnet) { + return (network as KadenaNetwork).options.buyLink; + } + + if ([NetworkNames.SyscoinNEVM, NetworkNames.Rollux].includes(network.name)) { + const address = network.displayAddress( + accountHeaderData.value.selectedAccount!.address + ); + return `${options.buyLink}&address=${address}`; + } + + if ([NetworkNames.SyscoinNEVMTest, NetworkNames.RolluxTest].includes(network.name)) { + return options.buyLink; + } + + return getDefaultBuyLink();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
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/types/evm-network.ts(2 hunks)packages/extension/src/ui/action/App.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/extension/src/providers/ethereum/networks/syscoin/rollux-testnet.ts
- packages/extension/src/providers/ethereum/networks/syscoin/nevm-testnet.ts
- packages/extension/src/providers/ethereum/networks/syscoin/nevm.ts
- packages/extension/src/providers/ethereum/networks/syscoin/rollux.ts
🔇 Additional comments (1)
packages/extension/src/providers/ethereum/types/evm-network.ts (1)
37-37: LGTM! Well-defined optional property.The
buyLinkproperty is correctly defined as optional with appropriate types, maintaining backward compatibility.
kvhnuke
merged commit 1d753c8
into
enkryptcom:devop/new-networks-2
Summary by CodeRabbit
Network Updates
Technical Improvements