fix: accept hex-encoded block number as contract call block_identifier - #3868
Open
cristianizzo wants to merge 2 commits into
Open
fix: accept hex-encoded block number as contract call block_identifier#3868cristianizzo wants to merge 2 commits into
cristianizzo wants to merge 2 commits into
Conversation
A hex-encoded block number string (e.g. "0x1510FA4") is documented as a valid block_identifier, and the codebase already exposes an is_hex_encoded_block_number helper. However parse_block_identifier (and its async twin) never used it, so such values fell through to BlockNumberOutOfRange. Route them through the helper and resolve to the integer block number, matching the documented behavior. Fixes ApeWorX#3646
There was a problem hiding this comment.
Pull request overview
Fixes contract call block identifier parsing so hex-encoded block numbers (e.g. "0x1510FA4") are accepted and converted to an integer BlockNumber, aligning runtime behavior with the documented accepted block_identifier formats and avoiding a BlockNumberOutOfRange error.
Changes:
- Add handling for hex-encoded block-number strings in
parse_block_identifierandasync_parse_block_identifier. - Add sync + async parametrized tests for common and edge-case hex block-number inputs (including odd-length and mixed-case).
- Add a release note fragment for the bugfix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
web3/_utils/contracts.py |
Extends block identifier parsing to recognize hex-encoded block numbers and convert them to BlockNumber. |
tests/core/contracts/test_contract_util_functions.py |
Adds sync/async tests validating hex-string block numbers are parsed to the expected integers. |
newsfragments/3646.bugfix.rst |
Documents the bugfix in release notes. |
Comments suppressed due to low confidence (1)
web3/_utils/contracts.py:379
- Same concern as the sync variant:
is_hex_encoded_block_number()accepts non-0x-prefixed strings (e.g. "10"), so this will silently reinterpret such inputs as hex block numbers. Consider requiring an explicit0x/0Xprefix to avoid surprising behavior changes and to match JSON-RPC quantity formatting.
elif is_hex_encoded_block_number(block_identifier):
return BlockNumber(int(block_identifier, 16))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
is_hex_encoded_block_number is a routing predicate for requests that forward the raw string to the node, so it does not require a 0x prefix. Reusing it here was wrong: this branch converts to an int locally, so loose input silently changed meaning -- the decimal string "20" resolved to block 0x20 (32), and "abc" to 2748, rather than raising. That also regressed the existing invalid-block-identifier caller tests. Gate the branch on a stricter local predicate requiring 0x-prefixed canonical hex, and cover the loose forms with negative tests.
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
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.
What was wrong?
Related to Issue #3646
Closes #3646
Passing a hex-encoded block number string (e.g.
"0x1510FA4") as a contract callblock_identifierraisedBlockNumberOutOfRange:parse_block_identifier(and its async twin) handledint, the named blocks, and block hashes, but had no case for a hex-encoded block number, so those values fell through toelse: raise BlockNumberOutOfRange— even thoughHexStris part of theBlockIdentifierunion andselect_method_for_block_identifieralready accepts this form.How was it fixed?
Added a branch to both
parse_block_identifierandasync_parse_block_identifierthat resolves a0x-prefixed hex block number to its integerBlockNumber— the same output type the plain-intbranch produces, so it flows througheth_callunchanged with no extra RPC round-trip.The branch is gated on a private
_is_hex_block_numberhelper rather than the existingis_hex_encoded_block_number. That distinction is deliberate and is the important review point:is_hex_encoded_block_numberis a routing predicate.select_method_for_block_identifieruses it to pick an endpoint and then forwards the raw string to the node, which performs final validation — so it does not require a0xprefix (it just attemptsint(value, 16))."20"would resolve to block0x20(32),"abc"to 2748, and" 0x1a "/"0x1_a"/"+0x1a"would all be accepted.So
_is_hex_block_numberadditionally requires text, a0x/0Xprefix, and canonical hex. I kept it local rather than tighteningis_hex_encoded_block_number, because that shared predicate feeds 12Methoddeclarations acrossweb3/eth/eth.pyandweb3/eth/async_eth.py(eth_getBlockBy*,eth_getUncle*, etc.); tightening it would be a behavior change to those public methods and belongs in its own PR. Happy to go that route instead if you'd prefer.Ordering is preserved: the new branch sits after the block-hash check, so a 64-character hex string is still treated as a block hash.
Tests: parametrized sync + async cases for
"0x0", mixed-case"0XABC", and the odd-length"0x1510FA4"from the issue; plus negative cases ("abc","20","1000000"," 0x1a ","0x1_a","+0x1a","0b11","0x") asserting those still raise.Not changed: a block hash still resolves via
eth_getBlockByHash. Per your note on the issue, passing a hash straight through toeth_callis a breaking change better suited to the next major version, so it's out of scope here.Possible follow-up (not included, to keep this focused):
raise BlockNumberOutOfRangeis raised bare, so the error names neither the rejected value nor the accepted formats. Happy to add a message in a separate PR.Todo:
Cute Animal Picture