-
Notifications
You must be signed in to change notification settings - Fork 1.8k
soliditySha parameter encoding issue #1154
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
Comments
Can you provide the specific inputs? When I smoke test the same concept, I get a matching result:
This returns Same result in web3.py: w3.soliditySha3(
['bytes32', 'uint256', 'address'],
[b'\x12'*32, 3, "0x3232323232323232323232323232323232323232"],
) which returns |
I have a similar problem on web3=4.8.2, solidity=0.5.0: JS:
Python:
Maybe something to do with padding differences between the two libraries. The 'string' type gets handled the same by both |
Hm, actually both of those look wrong when the bytes32 value is short. My Remix run suggests that the result should be: Source:
(Note that remix won't let you compile with a short bytes32 value) If you pre-pad the value in web3.py, it looks right: >>>: Web3.soliditySha3(['bytes32'], ['0x000000000000000000000000000000008ddc1a359f25f52858404a15f1098181'])
HexBytes('0xc5c3c026c2d7e0acf858a8142a4664f2fef9d227a4a8e904282ef05c63e58012') Some obvious options are to:
I'm leaning toward # 1 |
Above answer was helpful and a bit misleading for me at the same time. I had to right-pad bytes32 to achieve the same hash. I'm using
As @carver mentioned this is not an issue if you fill 0s yourself. However, I often find myself storing short strings in bytes32 and filling it with 0s is kind of cumbersome. Perhaps coping solidity default behaviour is not the worst idea. Hopefully this will save someone debugging time. |
So it looks like web3.js is left-padding and solidity is right-padding. That's even more reason for us to not take a stance in web3.py until it is standardized (and require explicit padding by the caller). In order to generate the hash, the caller must know which tool created the "short bytes" value, and pad it appropriately. I agree that manual padding is a bit annoying, and would like to see a generally accepted standard so we can guess the correct padding direction. In the meantime, explicitly padding is a short-ish python built-in: # right pad:
filled_val = short_val.ljust(32, b'\0')
# left pad:
filled_val = short_val.rjust(32, b'\0') |
cc @njgheorghita @kclowes Can we get the strictness of bytes input into v5? Much easier to relax it later than constrain it. That means that any constant-length bytes value would have to be explicitly padded by the caller. |
Sure! To be clear, this is a request for option 1 in this comment, right? |
Yup! |
Anything new here..? |
I don't know if there have been any standardization discussions between Solidity and web3.js. As for web3.py, it looks like our answer will have to be that too-short values are rejected, and the caller must pad themselves using:
According to the summary on #1419 the too-short values are only rejected if you first call |
Uh oh!
There was an error while loading. Please reload this page.
pip freeze
outputWhat was wrong?
I am trying to implement EIP712 signatures in python. In order to do that, I need to follow a very strict hashing mechanism. I've got it working in web3.js and in solidity, but I fail to achieve the same hashes in python.
In order to compute the domain separator I use the following code:
when checked, all 5 elements are the same as what web3.js produces. However, the result of soliditySha3 is not the same. I noticed that when I change the description of verifyingContract from 'address' to 'bytes32', the resulting hash doesn't change. Is it possible python's soliditySha3 does not abi.encode correctly ?
Just for the record, here is the corresponding javascript code:
The text was updated successfully, but these errors were encountered: