Fix quai_getBlockByNumber rejecting standard 2-param format from dApps - #474
Fix quai_getBlockByNumber rejecting standard 2-param format from dApps#4740xalank wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical compatibility issue where dApps using standard Ethereum libraries (quais.js, ethers.js, viem, web3.js) would fail when calling getBlockByNumber or getBlockByHash through the wallet provider. The issue stemmed from parameter format mismatch between the standard 2-parameter Ethereum format and Quai's 3-parameter format that includes an explicit shard parameter.
Changes:
- Modified
quai_blockNumber/eth_blockNumberhandlers to dynamically derive the shard from the selected account address instead of hardcoding"0x00" - Updated
quai_getBlockByNumberandquai_getBlockByHashhandlers to detect and support both standard 2-parameter format[blockTag, includeTransactions]and Quai 3-parameter format[shard, blockTag, includeTransactions] - Added import for
getZoneForAddressutility function to enable dynamic shard derivation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // quais.js and standard dApps send 2 params; detect by checking | ||
| // if the second param is a boolean (includeTransactions flag). | ||
| let shard: Shard | ||
| let blockTag: BlockTag |
There was a problem hiding this comment.
The variable name blockTag is used for both block numbers (in quai_getBlockByNumber) and block hashes (in quai_getBlockByHash). While both are technically BlockTag types, using a more generic name like blockIdentifier or having separate handling for each case would improve code clarity and make it clearer that the parameter can represent different things depending on the RPC method.
There was a problem hiding this comment.
Low-value nit. BlockTag is the actual type from quais used for both block numbers and hashes. The variable matches the type system. Renaming to blockIdentifier creates a disconnect with the quais API.
| case "quai_getBlockByNumber": { | ||
| // Support both standard format [blockTag, includeTransactions] | ||
| // and Quai format [shard, blockTag, includeTransactions]. | ||
| // quais.js and standard dApps send 2 params; detect by checking | ||
| // if the second param is a boolean (includeTransactions flag). | ||
| let shard: Shard | ||
| let blockTag: BlockTag | ||
| let includeTx: boolean | ||
| if (params.length <= 2 || typeof params[1] === "boolean") { | ||
| // Standard format: derive shard from the selected account | ||
| const { address } = await this.preferenceService.getSelectedAccount() | ||
| shard = (getZoneForAddress(address) || "0x00") as Shard | ||
| blockTag = params[0] as BlockTag | ||
| includeTx = (params[1] as boolean) ?? false | ||
| } else { | ||
| shard = params[0] as Shard | ||
| blockTag = params[1] as BlockTag | ||
| includeTx = (params[2] as boolean) ?? false | ||
| } | ||
| return this.chainService.jsonRpcProvider.getBlock( | ||
| params[0] as Shard, | ||
| params[1] as BlockTag, | ||
| params[2] as boolean | ||
| shard, | ||
| blockTag, | ||
| includeTx | ||
| ) | ||
| } |
There was a problem hiding this comment.
The new parameter format detection logic for quai_getBlockByNumber and quai_getBlockByHash lacks test coverage. Given that this is a critical fix for dApp compatibility (as described in the PR), consider adding tests that verify both the standard 2-parameter format and the Quai 3-parameter format are handled correctly. Tests should cover scenarios like: standard format with true/false for includeTransactions, Quai format with all three parameters, and edge cases like single parameter calls.
f657da0 to
1a4a1ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (params.length <= 2) { | ||
| // Standard format: derive shard from the selected account | ||
| const { address } = await this.preferenceService.getSelectedAccount() | ||
| shard = (getZoneForAddress(address) || "0x00") as Shard | ||
| blockTag = params[0] as BlockTag | ||
| includeTx = (params[1] as boolean) ?? false |
There was a problem hiding this comment.
When params.length is 0, this code will enter the standard format branch and attempt to access params[0] which will be undefined. This will result in blockTag being undefined and likely cause an error when calling jsonRpcProvider.getBlock(). The RPC methods quai_getBlockByNumber and quai_getBlockByHash require at least one parameter (the block number/hash). Consider adding validation to ensure at least one parameter is provided before proceeding, for example: if (params.length === 0) throw new Error("Missing required parameter").
| if (params.length <= 2) { | ||
| // Standard format: derive shard from the selected account | ||
| const { address } = await this.preferenceService.getSelectedAccount() | ||
| shard = (getZoneForAddress(address) || "0x00") as Shard |
There was a problem hiding this comment.
The fallback to "0x00" when getZoneForAddress returns null/undefined is inconsistent with how other parts of the codebase handle this situation. In background/services/transactions/index.ts:164-169, when getZoneForAddress returns null/undefined, an explicit error is thrown: "Invalid address shard: Unable to determine the zone for the given 'to' address." Silently falling back to "0x00" could mask issues with invalid addresses and lead to operations being performed on the wrong shard. Consider either throwing an error or logging a warning when the zone cannot be determined.
| params[0] as Shard | ||
| ) | ||
| const { address } = await this.preferenceService.getSelectedAccount() | ||
| blockNumShard = (getZoneForAddress(address) || "0x00") as Shard |
There was a problem hiding this comment.
The fallback to "0x00" when getZoneForAddress returns null/undefined is inconsistent with how other parts of the codebase handle this situation. In background/services/transactions/index.ts:164-169, when getZoneForAddress returns null/undefined, an explicit error is thrown: "Invalid address shard: Unable to determine the zone for the given 'to' address." Silently falling back to "0x00" could mask issues with invalid addresses and lead to operations being performed on the wrong shard. Consider either throwing an error or logging a warning when the zone cannot be determined.
| if (params.length <= 2) { | ||
| // Standard format: derive shard from the selected account | ||
| const { address } = await this.preferenceService.getSelectedAccount() | ||
| shard = (getZoneForAddress(address) || "0x00") as Shard | ||
| blockTag = params[0] as BlockTag | ||
| includeTx = (params[1] as boolean) ?? false | ||
| } else { | ||
| shard = params[0] as Shard | ||
| blockTag = params[1] as BlockTag | ||
| includeTx = (params[2] as boolean) ?? false | ||
| } |
There was a problem hiding this comment.
The parameter detection logic assumes that Quai format calls will always provide exactly 3 parameters. However, if a caller uses Quai format with only 2 parameters (omitting the optional includeTransactions), the code will incorrectly interpret it as standard format and derive the shard from the selected account instead of using the provided shard parameter. For example, ["0x00", "0x4f670e"] would be misinterpreted as [blockTag, includeTransactions] instead of [shard, blockTag]. Consider either documenting that Quai format requires all 3 parameters, or using a more robust detection mechanism such as checking the type of params[1] (boolean for standard format, string/number for Quai format).
Summary
quai_getBlockByNumberandquai_getBlockByHashto accept both standard Ethereum format ([blockTag, includeTransactions]) and Quai format ([shard, blockTag, includeTransactions])quai_blockNumberto derive shard from the selected account instead of hardcoding0x00getZoneForAddress()so it works for all shards, not just Cyprus 1Problem
Any dApp using quais.js (or ethers.js, viem, web3.js, etc.) that calls
getBlockByNumberthrough the wallet provider hits a silent failure. The library sends thestandard 2-param format:
{ "method": "quai_getBlockByNumber", "params": ["0x4f670e", true] }But the handler interprets this as:
This causes jsonRpcProvider.getBlock() to throw. The error is then caught by handleRPCErrorResponse() which can't parse it and defaults to returning EIP-1193 error
code 4001 (userRejectedRequest). To the dApp, it looks like the user rejected the request — no useful error, no way to debug.
This affects any dApp where quais.js calls getBlockNumber() internally (e.g., during signer.sendTransaction() for gas estimation).
Fix
Detect the parameter format by checking params.length and typeof params[1]: