Arbitrary type analysis and casting in soliditySha3 function

[4skinSkywalker]

Is there an existing issue for this?

• [x] I have searched the existing issues

Current Behavior

HI,

sorry if I've opened this as a bug, feel free to re-label it.

I migrated from Web3.js to ethers and I've noticed some functionality were substantially different, so I digged into the code and noticed in your code you make a lot of assumptions on what the consumer passes into that function:

// Web3.js
Web3.utils.soliditySha3(Web3.utils.soliditySha3("abcd"))
// "0x370bca323502d5b0ae64c7ef1450f856f7c7954cb6596812dec27970f3ec8c62"

// ethers OK
e.utils.solidityKeccak256(["uint"], [e.utils.solidityKeccak256(["string"], ["abcd"])])
// "0x370bca323502d5b0ae64c7ef1450f856f7c7954cb6596812dec27970f3ec8c62"



// Web3.js
Web3.utils.soliditySha3(Web3.utils.soliditySha3("1234"))
// "0x4a99c238973ace5ad0b6ebf34feeb1eee19e58d9aeaad7ddf02e52c9f5cd2360"

// ethers KO
e.utils.solidityKeccak256(["uint"], [e.utils.solidityKeccak256(["string"], ["1234"])])
// "0x1e84a4b9cbdb7af415f0f3f94189bc971e69ea100ee19e87e5429ba52eb360ed"

// ethers OK
e.utils.solidityKeccak256(["uint"], [e.utils.solidityKeccak256(["uint"], ["1234"])])
// "0x4a99c238973ace5ad0b6ebf34feeb1eee19e58d9aeaad7ddf02e52c9f5cd2360"



// Web3.js
Web3.utils.soliditySha3(Web3.utils.soliditySha3("0xb2d4e1E20Dd3B5Aad39E0B9e372Be85FA36F126f"))
// "0x3eaee8f757468330c144b4867a7529a21beb0fc9b59b26d7a7156878dff51f5c"

// ethers KO
e.utils.solidityKeccak256(["uint"], [e.utils.solidityKeccak256(["string"], ["0xb2d4e1E20Dd3B5Aad39E0B9e372Be85FA36F126f"])])
// "0xcef5b191f64e8a8363605a459f6b235a381d42731aa36101776f9c7a0f6e4e22"

// ethers KO
e.utils.solidityKeccak256(["uint"], [e.utils.solidityKeccak256(["uint"], ["0xb2d4e1E20Dd3B5Aad39E0B9e372Be85FA36F126f"])])
// "0x40ddde96a307e488ea8c72221dac2347982f16db3dbff157045ac8b0cacc0832"

You are taking strings and running analysis on them to see what's in there. You already have a way to define and grab types: Why do you also have force strings to be casted in a weird way?

Why did you do that?

I've also noticed the FIXME comment that hints to future changes: If you change this function's output, then devs could experience problems: in our case we have objects in our dapp that are redeemable by providing a correct double hash. If you change the output of the function by further manipulation on the type you'll end up breaking functionalities and it's also difficult to track these kind of things down!

Thanks, Fredo

Expected Behavior

I would expect string to be hashed as strings, therefore I wouldn't expect your code to type cast strings before hashing.

Web3.js Version

Latest to this very date

Environment

• Operating System: Windows 10
• Browser: Chrome 94.0.4606.81
• Node.js Version: 14.18.1
• NPM Version: 6.14.15

Anything Else?

No response

[spacesailor24]

Hey there Fredo, thank you for bringing this to our attention and providing extensive examples - it's extremely helpful!

I can't answer why the code is written to make these assumptions, as the decisions were made way before me. As you've mentioned, we can't really do anything to fix this in 1.x as we'd break user's current implementations. We're in the middle of a rewrite of library (4.x) and we'll certainly keep this issue in mind when re-writing the hashing functions

Thank you again!

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.