-
Notifications
You must be signed in to change notification settings - Fork 232
feat: add conflux network #683
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
feat: add conflux network #683
Conversation
WalkthroughSupport for the Conflux eSpace network has been added across multiple modules. This includes new network configuration, NFT handler logic, type definitions, asset handler support, and integration with existing network registries and enums. The changes enable recognition, querying, and processing of Conflux NFTs and assets within the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant ConfluxAPI
User->>App: Requests NFT collections for Conflux network
App->>ConfluxAPI: Fetch collections owned by address
ConfluxAPI-->>App: Returns collection list
loop For each collection
App->>ConfluxAPI: Fetch NFT items for collection/address
ConfluxAPI-->>App: Returns NFT items
end
App-->>User: Returns aggregated NFT collections and items
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
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
🧹 Nitpick comments (6)
packages/extension/src/libs/nft-handlers/types/conflux.ts (3)
3-13: RefineConfluxNFTItemproperty types for better type safety.
- Replace the generic
rawData: objectwith a more specific type orunknown/Record<string, unknown>to avoidobjectpitfalls.- Consider using the existing
NFTType(or a union) instead oftype: stringto enforce valid NFT types.
15-24: Improve nullability and reuse existing types inNFTBalanceItem.
websiteandiconUrlare nullable—consider using aURLtype or customMaybeURLalias.- For
type: string, reuseNFTTypeif applicable.
26-29: MakeListResponsegeneric.Current
list: object[]is too coarse. You can introduce a type parameter:export interface ListResponse<T> { total: number; list: T[]; }This will enable strong typing of API results.
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (1)
279-289: Code formatting improvement while maintaining logic.The code reformatting maintains the original logic while improving readability. It correctly:
- Filters out the native token address from contracts
- Uses the appropriate Coingecko platform for the specified network
However, there is a performance issue flagged by static analysis:
- : tokens.reduce( - (obj, cur) => ({ ...obj, [cur.contract]: null }), - {} as Record<string, CoinGeckoTokenMarket | null>, - ); + : tokens.reduce( + (obj, cur) => { + obj[cur.contract] = null; + return obj; + }, + {} as Record<string, CoinGeckoTokenMarket | null>, + );This avoids using spread syntax on accumulators, which can lead to O(n²) time complexity.
🧰 Tools
🪛 Biome (1.9.4)
[error] 287-287: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/extension/src/libs/nft-handlers/conflux.ts (2)
36-37: Type casting could be improved.The double casting pattern
as unknown as NFTBalanceItem[]suggests potential type mismatch. Consider revising the type definitions to more accurately match the API response structure.If the type definitions can't be changed, at least add a comment explaining why this casting approach is necessary.
Also applies to: 72-73
58-84: Helper function looks good but has room for improvement.The collection item retrieval function is well implemented with:
- Proper error handling
- Correct mapping to standardized NFT item format
- Good use of caching
Consider adding:
- Logging for specific API errors to aid debugging
- Handling for API rate limiting if Conflux API enforces it
- Pagination support for collections with more than 100 items
Also, the URL construction:
- url: `https://evm.confluxscan.org/nft/${item.contract}/${item.tokenId}`, + url: `https://evm.confluxscan.org/nft/${item.contract}/${encodeURIComponent(item.tokenId)}`,This ensures tokenIds with special characters are properly encoded in URLs.
📜 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/conflux.pngis excluded by!**/*.png
📒 Files selected for processing (8)
packages/extension/src/libs/nft-handlers/conflux.ts(1 hunks)packages/extension/src/libs/nft-handlers/types/conflux.ts(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(2 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts(1 hunks)packages/extension/src/providers/ethereum/networks/conflux.ts(1 hunks)packages/extension/src/providers/ethereum/networks/index.ts(2 hunks)packages/types/src/networks.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (2)
packages/swap/src/configs.ts (1)
NATIVE_TOKEN_ADDRESS(123-123)packages/extension/src/libs/market-data/types.ts (1)
CoinGeckoTokenMarket(23-37)
packages/extension/src/providers/ethereum/networks/conflux.ts (2)
packages/extension/src/providers/ethereum/types/evm-network.ts (2)
EvmNetworkOptions(25-56)EvmNetwork(58-293)packages/extension/src/providers/ethereum/libs/activity-handlers/index.ts (1)
EtherscanActivity(10-10)
packages/extension/src/libs/nft-handlers/types/conflux.ts (1)
packages/extension/src/types/nft.ts (1)
NFTItem(9-16)
🪛 Biome (1.9.4)
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts
[error] 287-287: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (11)
packages/extension/src/libs/nft-handlers/types/conflux.ts (2)
31-35:ConfluxResponseshape looks correct.No further changes needed here; the interface aligns with the other handlers.
37-39: ExtendingNFTItemto includedescriptionis appropriate.This ensures downstream code treats descriptions uniformly across networks.
packages/extension/src/providers/ethereum/libs/activity-handlers/providers/etherscan/configs.ts (1)
74-74: Confirm the ConfluxScan EVM API endpoint.The URL
https://evmapi.confluxscan.org/matches the Etherscan-compatible pattern. Please verify it supports the same query parameters as other Etherscan endpoints (module, action, etc.).packages/extension/src/providers/ethereum/networks/index.ts (2)
74-74: Import Conflux network configuration.The new import aligns with the file structure and makes the Conflux config available for registration.
158-159: Registerconfluxin the network exports.Including
conflux: confluxensures the Conflux network is part of the default export and can be selected like other networks.packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts (1)
76-77: Include Conflux in supported network names.Adding
| NetworkNames.Confluxextends token balance handling to Conflux. Ensure assetinfo-mew.ts also configures Conflux undersupportedNetworks.packages/types/src/networks.ts (2)
108-110: AddConfluxtoNetworkNamesenum.The new entry
Conflux = "Conflux"aligns with other network identifiers. Ensure any consumers ofNetworkNameshandle this new key.
174-176: AddConfluxtoCoingeckoPlatformenum.The platform ID
confluxmatches the lowercase convention. Consider verifying on CoinGecko that the slugconfluxis correct.packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (1)
220-223: LGTM! Conflux network support added properly.The Conflux network integration follows the established pattern for adding network support, with appropriate token balance API name (
tbName: 'cfx') and Coingecko platform identifier.packages/extension/src/providers/ethereum/networks/conflux.ts (2)
9-25: Network configuration looks good and follows established patterns.The Conflux eSpace network configuration is properly defined with:
- Correct chain ID (0x406 = 1030 in decimal)
- Appropriate explorer URLs
- Proper currency names
- Integration with relevant handlers (NFT, assets, activity)
The configuration properties follow the expected structure for an EVM network.
27-29: Network instantiation and export follows project conventions.The instantiation of the EvmNetwork with the defined configuration and its default export matches the pattern used for other networks in the project.
Summary by CodeRabbit