-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Adding private RPC on transport for all supported chains #4863
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce two new functions: Changes
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 (
|
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
🧹 Outside diff range and nitpick comments (2)
src/lib/network.ts (2)
23-24
: Enhance function documentationWhile the function signature is well-typed, consider adding a JSDoc comment to provide more context about:
- The purpose of DRPC (Distributed RPC)
- Parameter constraints or valid chainId ranges
- Return value meaning (null case implications)
-// Network mapping for DRPC +/** + * Maps chain IDs to their corresponding DRPC (Distributed RPC) network identifiers. + * @param chainId - The blockchain network identifier + * @returns The DRPC network name if supported, null otherwise + */
42-43
: Add input validation for chainIdThe current implementation doesn't validate the chainId parameter. Consider adding checks for:
- Invalid numbers (negative values, decimals)
- Commonly mistyped chainIds
- Type coercion issues
export const getDrpcNetwork = (chainId: number): string | null => { + if (!Number.isInteger(chainId) || chainId < 0) { + throw new Error('Invalid chainId: must be a positive integer'); + } const networkMap: { [key: number]: string } = { // ... network mappings }; return networkMap[chainId] || null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/lib/network.ts
(1 hunks)src/wagmiConfigs.ts
(3 hunks)
🔇 Additional comments (3)
src/lib/network.ts (1)
25-40
: Consider externalizing network configuration
The hardcoded network mapping could be challenging to maintain as new networks are added or network identifiers change. Consider:
- Moving this configuration to a dedicated config file
- Aligning it with the existing
config.NETWORKS_CONFIG
- Adding validation for supported networks
Let's check if these networks align with the existing configuration:
src/wagmiConfigs.ts (2)
49-52
: Consider rate limiting and monitoring strategy for DRPC usage
The client configuration now uses DRPC for all chains when available. This has important operational implications:
- Rate limiting needs to be considered across all chains collectively
- Transport failures should be monitored for fallback scenarios
- Usage metrics would be valuable for capacity planning
Consider implementing:
- A monitoring solution to track DRPC usage per chain
- Circuit breakers for automatic fallback on rate limit exhaustion
- Metrics collection for transport performance
Let's check which chains are supported:
#!/bin/bash
# Check configured chains and their usage
echo "Checking chain configurations..."
rg -A 5 "EVM_CHAINS"
23-29
: 🛠️ Refactor suggestion
Enhance error handling and security in DRPC transport creation
The function needs additional safeguards for production readiness:
- The fallback to
http()
when DRPC is unavailable should be logged for monitoring - The environment variable access should be validated earlier
- The URL construction should be more secure
Consider this improved implementation:
const createDrpcTransport = (chainId: number) => {
+ const drpcKey = process.env.NEXT_PUBLIC_DRPC_KEY;
+ if (!drpcKey) {
+ console.warn(`DRPC key not found, falling back to public RPC for chain ${chainId}`);
+ return http();
+ }
+
const network = getDrpcNetwork(chainId);
- const drpcKey = process.env.NEXT_PUBLIC_DRPC_KEY;
- return network && drpcKey
- ? http(`https://lb.drpc.org/ogrpc?network=${network}&dkey=${drpcKey}`)
- : http();
+ if (!network) {
+ console.warn(`Unsupported network for DRPC: ${chainId}`);
+ return http();
+ }
+
+ const drpcUrl = new URL('https://lb.drpc.org/ogrpc');
+ drpcUrl.searchParams.append('network', network);
+ drpcUrl.searchParams.append('dkey', drpcKey);
+ return http(drpcUrl.toString());
};
Let's verify the DRPC key is properly set in the environment:
export const getDrpcNetwork = (chainId: number): string | null => { | ||
const networkMap: { [key: number]: string } = { | ||
1: 'ethereum', | ||
137: 'polygon', | ||
10: 'optimism', | ||
42161: 'arbitrum', | ||
8453: 'base', | ||
42220: 'celo', | ||
100: 'gnosis', | ||
1101: 'polygon-zkevm', | ||
11155111: 'sepolia', | ||
11155420: 'optimism-sepolia', | ||
421614: 'arbitrum-sepolia', | ||
84532: 'base-sepolia', | ||
44787: 'celo-alfajores', | ||
1442: 'polygon-zkevm-cardona', | ||
}; | ||
|
||
return networkMap[chainId] || null; | ||
}; |
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.
💡 Codebase verification
Based on the search results, I can now generate the final response:
DRPC key implementation needs error handling improvements
The current implementation in wagmiConfigs.ts
uses DRPC key from environment variables but lacks:
- Error handling for invalid or missing DRPC keys
- Rate limit handling mechanisms
- Fallback strategy when DRPC requests fail
Specific locations that need attention:
src/wagmiConfigs.ts
: The transport configuration simply falls back to default HTTP transport without any error handling when DRPC key is missing- Consider adding validation similar to the
projectId
check that's already implemented (if (!projectId) throw new Error('Project ID is not defined')
)
🔗 Analysis chain
Verify DRPC key handling
The PR objectives mention a rate-limited key requirement for production. While this function handles network mapping, ensure that:
- The DRPC key is properly validated in the transport configuration
- Rate limiting is properly implemented
- Error handling exists for cases when the key is invalid or rate limit is exceeded
Let's check how the DRPC key is handled:
Let me gather more context about the transport configuration and error handling:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check DRPC key usage and validation
# Look for DRPC key handling in the codebase
rg -g '!node_modules' -g '*.ts' -g '*.js' -A 5 'drpcKey|DRPC_KEY'
Length of output: 458
Script:
#!/bin/bash
# Check the complete transport configuration and error handling
rg -g '*.ts' -g '*.js' -A 10 'createConfig|http\('
# Look for any error handling related to RPC
rg -g '*.ts' -g '*.js' -A 5 'RpcError|rpcError|transport.*error'
Length of output: 3142
@geleeroyale we set the key on vercel for staging, we'd need one rate limited before releasing to production 💯
Summary by CodeRabbit
New Features
chainId
, enhancing network mapping capabilities.chainId
, improving flexibility in network connections.Bug Fixes