fix: update rpcUrl handling in Eip1193Provider and TurnkeyWalletStrategy#670
Merged
Conversation
📝 WalkthroughWalkthroughThis PR refactors RPC URL configuration across the wallet packages to support per-chain RPC endpoints. The base type 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/wallets/wallet-turnkey/src/strategy/Eip1193Provider.ts`:
- Around line 11-21: The parseChainId helper can return NaN for malformed
inputs; update parseChainId (and its callers) to validate the parsed result
(e.g., return null/undefined or throw on invalid parsing) and then guard the
place where you assign to this.chainId (the assignment in the Eip1193Provider
code around the existing this.chainId update) so you only mutate provider state
when the parsed chainId is a finite number (Number.isFinite or !Number.isNaN).
If parsing fails, do not set this.chainId — instead surface an error or reject
the request so eth_chainId and RPC client creation never persist an invalid
value. Ensure you reference parseChainId and the this.chainId assignment when
making the change.
In `@packages/wallets/wallet-turnkey/src/strategy/strategy.ts`:
- Around line 173-174: The error message that validates presence of an RPC
endpoint should be updated to reflect support for both options.rpcUrl and
options.rpcUrls (used in the selection `const url = options.rpcUrls?.[chainId]
|| options.rpcUrl`); find the places that throw when `url` is falsy (including
the other occurrence around lines 380-381) and change the thrown message to
mention "rpcUrl or rpcUrls (indexed by chainId)" or similar so callers using
either option see an accurate error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d256294-c6f3-446c-af8a-584d7730389f
📒 Files selected for processing (3)
packages/wallets/wallet-base/src/types/strategy.tspackages/wallets/wallet-turnkey/src/strategy/Eip1193Provider.tspackages/wallets/wallet-turnkey/src/strategy/strategy.ts
Comment on lines
+11
to
+21
| const parseChainId = (chainId: number | string) => { | ||
| if (typeof chainId === 'number') { | ||
| return chainId | ||
| } | ||
|
|
||
| if (chainId.startsWith('0x')) { | ||
| return parseInt(chainId.replace('0x', ''), 16) | ||
| } | ||
|
|
||
| return parseInt(chainId, 10) | ||
| } |
There was a problem hiding this comment.
Validate parsed chain IDs before mutating provider state.
Line 20 can yield NaN for malformed input, and Line 133 then persists that invalid value into this.chainId. That can break eth_chainId and RPC client creation.
💡 Proposed fix
const parseChainId = (chainId: number | string) => {
- if (typeof chainId === 'number') {
- return chainId
- }
-
- if (chainId.startsWith('0x')) {
- return parseInt(chainId.replace('0x', ''), 16)
- }
-
- return parseInt(chainId, 10)
+ const parsed =
+ typeof chainId === 'number'
+ ? chainId
+ : /^0x/i.test(chainId.trim())
+ ? Number.parseInt(chainId.trim().slice(2), 16)
+ : Number.parseInt(chainId.trim(), 10)
+
+ if (!Number.isInteger(parsed) || parsed < 0) {
+ throw new Error('Invalid chainId')
+ }
+
+ return parsed
}Also applies to: 133-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/wallets/wallet-turnkey/src/strategy/Eip1193Provider.ts` around lines
11 - 21, The parseChainId helper can return NaN for malformed inputs; update
parseChainId (and its callers) to validate the parsed result (e.g., return
null/undefined or throw on invalid parsing) and then guard the place where you
assign to this.chainId (the assignment in the Eip1193Provider code around the
existing this.chainId update) so you only mutate provider state when the parsed
chainId is a finite number (Number.isFinite or !Number.isNaN). If parsing fails,
do not set this.chainId — instead surface an error or reject the request so
eth_chainId and RPC client creation never persist an invalid value. Ensure you
reference parseChainId and the this.chainId assignment when making the change.
Comment on lines
+173
to
174
| const url = options.rpcUrls?.[chainId] || options.rpcUrl | ||
|
|
There was a problem hiding this comment.
Update error messaging to reflect rpcUrls support.
After changing selection precedence, the thrown message still asks only for rpcUrl, which is now inaccurate for callers using rpcUrls.
💡 Proposed fix
- new Error('Please pass rpcUrl within the evmOptions'),
+ new Error('Please pass rpcUrl or rpcUrls within the evmOptions'),Also applies to: 380-381
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/wallets/wallet-turnkey/src/strategy/strategy.ts` around lines 173 -
174, The error message that validates presence of an RPC endpoint should be
updated to reflect support for both options.rpcUrl and options.rpcUrls (used in
the selection `const url = options.rpcUrls?.[chainId] || options.rpcUrl`); find
the places that throw when `url` is falsy (including the other occurrence around
lines 380-381) and change the thrown message to mention "rpcUrl or rpcUrls
(indexed by chainId)" or similar so callers using either option see an accurate
error.
ThomasRalee
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request improves the flexibility and correctness of how RPC URLs are handled for EVM chains in the wallet packages. The main changes standardize the use of chain IDs as numbers (instead of enums) for RPC URL mapping, add utility functions for parsing chain IDs, and ensure that the correct RPC URL is selected based on the current chain context. These updates make it easier to support multiple networks and improve compatibility with various chain ID formats.
EVM Chain ID and RPC URL Handling Improvements:
rpcUrlsinWalletStrategyEvmOptionsand related code to usePartial<Record<number, string>>, ensuring chain IDs are consistently treated as numbers, which improves flexibility for multi-chain support.parseChainIdutility to handle both string (including hex) and number chain ID formats, ensuring robust parsing throughout the codebase. [1] [2]CustomEip1193Providerand its instantiation to accept and correctly prioritizerpcUrlandrpcUrlsparameters, allowing dynamic selection of the appropriate RPC URL for the active chain. [1] [2] [3] [4] [5] [6] [7] [8]Strategy Logic Updates:
TurnkeyWalletStrategyto always prioritizerpcUrls[chainId]overrpcUrlfor selecting the RPC endpoint, ensuring the correct URL is used for each chain. [1] [2]EIP-1193 Compliance and Consistency:
eth_chainIdmethod, as required by the EIP-1193 specification.Summary by CodeRabbit