Skip to content
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

invalid bytes32 value #2256

Closed
zastrin opened this issue Jan 29, 2019 · 10 comments · Fixed by #2345
Closed

invalid bytes32 value #2256

zastrin opened this issue Jan 29, 2019 · 10 comments · Fixed by #2345
Labels
Bug Addressing a bug In Progress Currently being worked on

Comments

@zastrin
Copy link

zastrin commented Jan 29, 2019

Expected behavior

contract.deploy({
... data: bytecode,
... arguments: [stringArray.map(name => web3.utils.asciiToHex(name))]
... }).send({args ...})
should deploy the contract where argument to contract is an array of bytes32.

There is no issue with web3 1.0.0-beta.37, it happens with beta.41

Actual behavior

The deploy fails with Error: invalid bytes32 value (arg="candidateName", coderType="bytes32", value="0x52616d61", version=4.0.23)

Error Logs

(node:46067) UnhandledPromiseRejectionWarning: Error: invalid bytes32 value (arg="candidateNames", coderType="bytes32", value="0x52616d61", version=4.0.23)
at Object.throwError (node_modules/ethers/errors.js:76:17)
at CoderFixedBytes.encode (/node_modules/ethers/utils/abi-coder.js:473:20)
at /node_modules/ethers/utils/abi-coder.js:645:59
at Array.forEach ()
at pack (/node_modules/ethers/utils/abi-coder.js:644:12)
at CoderArray.encode (/node_modules/ethers/utils/abi-coder.js:744:40)
at /node_modules/ethers/utils/abi-coder.js:645:59

Versions

NodeJS 11.5.0
Web3js 1.0.0-beta.41

@zastrin
Copy link
Author

zastrin commented Jan 29, 2019

Looks like the recent versions are using ethers.js. When I convert string to bytes32 in ethers.js

ethers.utils.formatBytes32String('Nick')
'0x4e69636b00000000000000000000000000000000000000000000000000000000'

but in web3.js there are no 0 padding

web3.utils.asciiToHex('Nick')
'0x4e69636b'

I don't exactly know how/why ethers.js is being used but may be this will give some clues?

@patidarmanoj10
Copy link

I too facing this issue in new version of web3.js

@nivida nivida added the Bug Addressing a bug label Jan 31, 2019
@nivida
Copy link
Contributor

nivida commented Jan 31, 2019

Thanks for submitting this issue!

Will fix it asap. I think the former maintainer decided because of the lack of time to use the AbiCoder of ethers.js instead of updating the AbiCoder of Web3. The long term goal is to remove this dependency because of the importance of it in the Web3.js library.

@nivida nivida added the In Progress Currently being worked on label Feb 6, 2019
@nivida
Copy link
Contributor

nivida commented Feb 6, 2019

I've checked it and decided to write an ABICoder on my own because I would have to change anyways a lot of things. This means it will delay the fix for this issue. I apologize for the troubles you might have.

Edit:
Found a simpler an faster solution than writing of my own abi coder.

@ricmoo
Copy link
Contributor

ricmoo commented Feb 6, 2019

The ethers.js coder was used because it is robust and well tested and was the only implementation of ABIv2. The previous Web3 ABI coder had several bugs and did not support ABIv2.

Passing in the wrong length is actually ambiguous. If you had a bytes4, and passed in “0x12” you might expect “0x00000012” or “0x12000000”. I did informal polls, and the answer was more or less split (with slightly more thinking the wrong padding rule). So it makes sense to enforce. I think with a byes32 and a string we feel it isn’t ambiguous, but it’s not really that different from the coder’s point of view. This is why the ethers.utils.stringToBytes32 and ethers.utils.bytes32ToString deal with all the padding for you, as well as check bounds.

Is there any reason the asciiToHex couldn’t pad to 32 bytes? Or have a variant that did, or take in a second parameter on what bytesXX to align to? Are there times that it is used that you don’t want padding?

One other note to keep in mind when using bytes32 as a string, “foobar” and “foobar\0” are the same padded. Often it doesn’t matter, but just something to keep in mind, because it could. :)

If you write your own, please feel free to use the test cases in ethers.js; I think there are about 13,000 of them, half for ABIv1 and half for ABIv2, ish. Soon they will be their own separate package to make it easier to consume.

Anyhoo, hope that helps explain things, and feel free to bug me if you have any questions. :)

@ricmoo
Copy link
Contributor

ricmoo commented Feb 6, 2019

Another FYI; if this broke recently, I think it is because you updated the version in the package.json of ethers.js? If you put it back to point at the beta version, you will pick up an older version of ethers.js before the ABI coder enforces bytesXX lengths. As a quick fix.

@nivida
Copy link
Contributor

nivida commented Feb 8, 2019

Thanks @ricmoo for the additional information!

Is there any reason the asciiToHex couldn’t pad to 32 bytes? Or have a variant that did, or take in a second parameter on what bytesXX to align to? Are there times that it is used that you don’t want padding?

This is what I'm checking currently and will probably be the solution.

If you write your own, please feel free to use the test cases in ethers.js; I think there are about 13,000 of them, half for ABIv1 and half for ABIv2, ish. Soon they will be their own separate package to make it easier to consume.

If I really will write my own abi coder then I will definitely use your test cases! thanks!

Another FYI; if this broke recently, I think it is because you updated the version in the package.json of ethers.js? If you put it back to point at the beta version, you will pick up an older version of ethers.js before the ABI coder enforces bytesXX lengths. As a quick fix.

Yes, that's right but with a downgrade, I will just have other issues this is why I'm looking for a possible solution without a downgrade of ethers.js. If I can't solve these issues until the end of the day I will downgrade to an older version of ethers.js.

@melihbirim
Copy link

melihbirim commented Feb 12, 2019

web3.utils.fromAscii('Nick').padEnd(66, '0');

worked for me.

@KaloNK
Copy link

KaloNK commented May 24, 2019

There is still a problem with fixed bytes parameters in beta 55

Uncaught (in promise) Error: invalid bytes4 value (arg="", coderType="bytes4", value="VMOB", version=4.0.27)

myContract.methods.myMethod(param).call({}).then(console.log); == Error
myContract.methods.myMethod(web3.utils.fromAscii(param)).call({}).then(console.log); == Error
myContract.methods.myMethod(web3.utils.fromAscii(param).substring(0,10)).call({}).then(console.log); == OK

@nivida
Copy link
Contributor

nivida commented May 24, 2019

The AbiCoder from ethers we use has a strict validation of the passed values and it requires the padding to be added to the method parameters. There is currently no generic internal web3 solution for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Addressing a bug In Progress Currently being worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants