Repository navigation
fix: right-pad short bytesN values in solidity_keccak - #3912
Open
haybinalim wants to merge 1 commit into
Open
haybinalim wants to merge 1 commit into
haybinalim wants to merge 1 commit into
Conversation
Solidity's abi.encodePacked encodes a bytesN value as exactly N bytes. hex_encode_abi_type returned standalone bytesN values unpadded, so a value shorter than N bytes (accepted by validate_abi_value since ApeWorX#3882) produced a hash that differs from Solidity and eth_abi.encode_packed. Right-pad standalone bytesN values to N bytes. Values longer than N bytes are still rejected by validate_abi_value. bytesN array elements are left unchanged (see ApeWorX#3886). Closes ApeWorX#3906
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 I did
Web3.solidity_keccaknow right-pads a standalonebytesNvalue that is shorter thanNbytes toNbytes, the same way Solidity'sabi.encodePackedandeth_abi.packed.encode_packedencode it. Before this change,solidity_keccak(["bytes2"], [b"a"])hashed0x61instead of0x6100. Since #3882,validate_abi_valueaccepts these short values, so they passed validation and gave a hash that differs from Solidity.Closes #3906
How I did it
hex_encode_abi_type(web3/_utils/encoding.py): for a standalonebytesN(noforce_size), right-pad the hex value with zero bytes toNbytes. Dynamicbytesis unchanged.Nbytes still raiseWeb3TypeError.validate_abi_valuealready rejects them (since fix: reject out-of-range integers and junk hex in validate_abi_value #3882), before the encoder runs, and this PR keeps that. I did not truncate them, because there's no single right way to do it: Solidity won't implicitly convert a longer value tobytesN, an explicitbytesN(x)keeps the leftmost bytes, andeth_abi'sencode_packedraisesValueOutOfBounds. Hashing a cut-down value without warning would bring back the same kind of mismatch this PR fixes. The new tests cover this.bytesNarray elements are left alone. Fix solidity_keccak for bytesN array elements #3886 fixes those (they have to be padded to 32 bytes) in the same branch ofhex_encode_abi_type. This change only touches theforce_size is Nonepath, so the two PRs don't overlap in behavior. Whichever one lands second is a one-line rebase (anif force_size is not None/elif abi_type != "bytes"pair), and I'm happy to do it.["bytes3"], ["0x5402"]case intest_solidity_keccak. It expected the same hash asbytes2(0x5402), but Solidity hashesbytes3(0x540200)to0xed37fb74…. I also updated thehex_encode_abi_typeunit case forbytes3(0x5402→0x540200).Web3.solidity_keccak(docs/web3.main.rst), with abytes4(hex"1234")example.There's no newsfragment because the repo now uses GitHub-generated release notes, per the contributing guide.
Note: #3907 and #3911 also target #3906, but they produce different hashes from Solidity for
bytes2(b"a"). #3907 pads to 32 bytes (0x294587bf…), and #3911 left-pads and encodes hex strings as UTF-8 (0x97222015…). Solidity gives0xa3fe1181…. #3911 also includes unrelatedWei/Gweichanges.How to verify it
I took every new expected hash from
keccak256(abi.encodePacked(...))in a contract compiled with solc 0.8.26 and called on py-evm (EthereumTesterProvider). Each one also matcheskeccak(eth_abi.packed.encode_packed(...)):bytes2b"a"/"0x61"0xa3fe1181…bytes3"0x5402"0xed37fb74…bytes4"0x1234"0x83cdd5d8…bytes32"0x01"0x48078cfe…bytes4, bytes1b"\x01\x02", b"\x03"0x84afedaa…string, bool, uint16, bytes4, address"testing a string!", False, 299, "0x5402", 0x49Ed…56140x9f22f3fb…Local runs (Python 3.12):
pytest tests/core/utilities(604 passed) andpytest tests/core/web3-module(all passed)pytest tests/integration/test_ethereum_tester.py -k "solidity_keccak or Web3Module": 42 passed, 1 skipped (batch requests, as onmain)main(10 failures) and pass with this changeruff check .,ruff format --check .andmdformat --check *.mdpass.mypy -p web3 -p ensreports nothing in the touched files.Checklist