feat: add support for chains #413
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Poem
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
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: 1
🧹 Outside diff range and nitpick comments (3)
src/services/nft.service.ts (3)
Line range hint
58-61: Add error handling for chain-specific failuresThe current implementation doesn't handle chain-specific failures gracefully. If one chain fails, the entire operation fails.
Consider this approach:
let profiles: Array<any> = []; const supportedChainIds = [84532, 42161]; for (let i = 0; i < supportedChainIds.length; i++) { + try { const chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]); profiles = profiles.concat(chainProfiles); + } catch (error) { + logger.error(`Failed to fetch profiles for chain ${supportedChainIds[i]}: ${error}`); + // Continue with other chains instead of failing completely + continue; + } } + if (profiles.length === 0) { + throw new ApiError(httpStatus.SERVICE_UNAVAILABLE, 'Failed to fetch profiles from any chain'); + } return profiles;
Line range hint
58-61: Consider performance optimization for parallel chain queriesThe current sequential chain querying could be optimized using Promise.all for parallel execution.
Consider this optimization:
let profiles: Array<any> = []; const supportedChainIds = [84532, 42161]; - for (let i = 0; i < supportedChainIds.length; i++) { - const chainProfiles = await ociService.getProfiles(address, supportedChainIds[i]); - profiles = profiles.concat(chainProfiles); - } + const chainPromises = supportedChainIds.map(chainId => + ociService.getProfiles(address, chainId) + .catch(error => { + logger.error(`Failed to fetch profiles for chain ${chainId}: ${error}`); + return []; + }) + ); + const results = await Promise.all(chainPromises); + profiles = results.flat(); return profiles;
Based on the gathered information, I can now provide a final response:
Mixing of testnet and mainnet chains confirmed as a critical issue
The codebase shows that chain IDs 84532 (Base Sepolia testnet) and 42161 (Arbitrum One mainnet) are hardcoded in
src/services/nft.service.tswithout any environment-based configuration. TheociService.getProfiles()function fetches and combines data from both chains indiscriminately, which could lead to:
- Mixing of test and production NFT profile data
- Inconsistent data states between environments
- Potential security risks when handling mainnet data in test environments
🔗 Analysis chain
Line range hint
57-61: Review mixing of testnet and mainnet chainsThe current implementation mixes Base Sepolia (testnet, 84532) with Arbitrum One (mainnet, 42161). This could lead to unexpected behavior by combining test and production data.
Let me gather more context about chain IDs usage in the codebase to better understand the environment setup.
Let me check for any environment-specific configurations or constants that might indicate intended chain usage.
Let me check for any configuration files or environment variables that might define the chain IDs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other instances of these chain IDs to understand the context rg -l "84532|42161" --type tsLength of output: 59
Script:
#!/bin/bash # Search for chain ID related configurations and constants rg -A 5 "chainId|ChainId|CHAIN_ID|Chain_ID" --type tsLength of output: 1184
Script:
#!/bin/bash # Look for environment configurations and chain-related constants rg -A 3 "production|mainnet|testnet|development|staging" --type tsLength of output: 3399
Script:
#!/bin/bash # Look for configuration files and environment variables related to chains rg -A 3 "chain.*config|CHAIN" --type-add 'config:*.{ts,js,json,env}' --type configLength of output: 84
| @@ -54,7 +54,7 @@ const getReputationScore = async (tokenId: string, address: string) => { | |||
|
|
|||
| async function getProfiles(address: string) { | |||
| let profiles: Array<any> = []; | |||
| const supportedChainIds = [11155111]; | |||
| const supportedChainIds = [84532, 42161]; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider moving chain IDs to configuration
The hard-coded chain IDs should be moved to a configuration file for better maintainability and environment-specific settings.
- const supportedChainIds = [84532, 42161];
+ const supportedChainIds = config.chains.supported;Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Bug Fixes