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

Bring web3 sha3 API in line with Solidity sha3 #445

Closed
jtremback opened this issue May 5, 2016 · 19 comments
Closed

Bring web3 sha3 API in line with Solidity sha3 #445

jtremback opened this issue May 5, 2016 · 19 comments
Assignees

Comments

@jtremback
Copy link

jtremback commented May 5, 2016

I had to spend a lot of time figuring out how to get sha3 to produce the same hashes in web3 and in my contracts. I think we should consider giving them both the same API. Here is a function that should do it:

function solSha3 (...args) {
    args = args.map(arg => {
        if (typeof arg === 'string') {
            if (arg.substring(0, 2) === '0x') {
                return arg.slice(2)
            } else {
                return web3.toHex(arg).slice(2)
            }
        }

        if (typeof arg === 'number') {
            return leftPad((arg).toString(16), 64, 0)
        } else {
          return ''
        }
    })

    args = args.join('')

    return '0x' + web3.sha3(args, { encoding: 'hex' })
}
@jtremback
Copy link
Author

Needs something for BigNumber.js, probably.

@ethers
Copy link
Contributor

ethers commented May 11, 2016

related #226

@ethers
Copy link
Contributor

ethers commented Jun 3, 2016

If web3.sha3 is going to start returning the "0x", perhaps it could be done by an extra parameter to web3.sha3, to avoid breaking existing stuff (which add the "0x" themselves already).

@jtremback
Copy link
Author

That's a valid concern, but honestly at some point you just have to correct blatant errors with a breaking update.

@graup
Copy link

graup commented Jul 4, 2016

This change is VERY needed! I just spent hours trying to find out how to do this.

The function given is good, but to make it work with negative numbers, I had to change the 'number' handling to this:

        if (typeof arg === 'number') {
            if (arg < 0) {
              return leftPad((arg >>> 0).toString(16), 64, 'F');
            }
            return leftPad((arg).toString(16), 64, 0);
        }

@raineorshine
Copy link

I took the suggestions from this issue and wrapped them all up into an npm library:
https://github.com/raineorshine/solidity-sha3

Here's the source code in its entirety:

import Web3 from 'web3'
import leftPad from 'left-pad'
const web3 = new Web3()

// the size of a character in a hex string in bytes
const HEX_CHAR_SIZE = 4

// the size to hash an integer if not explicity provided
const DEFAULT_SIZE = 256

const isHex = val => val.toString().slice(0, 2) === '0x'

/** Encodes a values in hex and adds padding to the given size if needed. Curried args. */
const encodeWithPadding = size => value => {
  return typeof value === 'string' && !isHex(value)
    // non-hex string
    ? web3.toHex(value)
    // numbers, big numbers, and hex strings
    : leftPad(web3.toHex(value >>> 0).slice(2), size / HEX_CHAR_SIZE, value < 0 ? 'F' : '0')
}

/** Hashes one or more arguments, using a default size for numbers. */
export default (...args) => {
  const paddedArgs = args.map(encodeWithPadding(DEFAULT_SIZE)).join('')
  return web3.sha3(paddedArgs, { encoding: 'hex' })
}

/** Hashes a single value at the given size. */
export const sha3withsize = (value, size) => {
  const paddedArgs = encodeWithPadding(size)(value)
  return web3.sha3(paddedArgs, { encoding: 'hex' })
}

@u2
Copy link

u2 commented Jul 28, 2016

Great!

@axic
Copy link
Contributor

axic commented Jul 29, 2016

This is actually implemented in ethereumjs-abi too:

var abi = require('ethereumjs-abi')
var BN = require('bn.js')

abi.soliditySHA3(
    [ "address", "address", "uint", "uint" ],
    [ new BN("43989fb883ba8111221e89123897538475893837", 16), 0, 10000, 1448075779 ]
).toString('hex')

@raineorshine
Copy link

@axic Thanks... kind of a bummer to have done duplicate work, but I'm glad to know. I'll make a note to myself to find what docs this should go in so that it is not so secret. None of us on the gitter channel were aware of this.

@axic
Copy link
Contributor

axic commented Jul 31, 2016

@raineorshine It was no secret, it was there since last November :)

I think I've linked to it on stackexchange.

@raineorshine
Copy link

If I ask about sha3 on gitter, StackExchange, google it, write a library
for it, publish it, and THEN hear about this from one person, then yes, for
the purpose of OSS, it is basically a secret :). So, I respectfully
disagree.
On Sun, Jul 31, 2016 at 4:33 AM Alex Beregszaszi notifications@github.com
wrote:

@raineorshine https://github.com/raineorshine It was no secret, it was
there since last November :)

I think I've linked to it on stackexchange.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#445 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAtyxEmztJzmLwSSlAtugJf3TawgIY5qks5qbHn6gaJpZM4IYO5G
.

@Schaeff
Copy link

Schaeff commented Dec 9, 2016

Hey, why can't we merge this into web3? 😃

@frozeman frozeman self-assigned this Jan 11, 2017
@frozeman
Copy link
Contributor

frozeman commented Jan 11, 2017

Can someone explain me the issue better?
So currently sha3 is simply taking anything and hashes it with no special treatment.
You guys say we should convert input data before to EVM abi before having it?

@frozeman frozeman added this to the 1.0 milestone Jan 12, 2017
@raineorshine
Copy link

Let's think about the use cases:

  1. Developer needs to hash some values that get used in their app. They happen to have web3 installed, so they use the sha3 available on web3.
  2. Developer needs to hash values off-chain and verify them on-chain.

I don't know for sure, but I imagine that #2 would be more common when it comes to dapp development. It is at least common enough that myself and several others have been bit by this issue. The problem is that sha3(12345) produces a different hash in solidity than in web3. Given that it looks exactly the same, this is quite confusing for the average developer. It wasn't until I did research and experimentation that I discovered that I needed to pad the value to 32 bytes to account for the byte representation of integers in Solidity. It is made more difficult by the fact that if a hash is off by any amount, it is off completely. This makes it very hard to know why the function is producing something different from what you expect.

Similar counterintuitive results occur when you try to hash tightly packed arguments, arrays, and BigNumber instances.

If this is considered special treatment, I would suggest two functions:

  • web3.sha3 which behaves the same as Solidity's sha3
  • web3.sha3Raw which hashes the input without any special handling of types, padding, tightly packed arguments, or arrays (current behavior).

Or vice versa:

  • web3.sha3 (current behavior)
  • web3.soliditySha3 which behaves the same as Solidity's sha3

Personally I think the latter is the correct decision and the more common sticking point for developers.

Look forward to hearing your thoughts!

@frozeman frozeman moved this from Backlog to Sprint 04 - 03-03-2017 in 1.0 Feb 23, 2017
@frozeman frozeman moved this from Sprint 04 - 03-03-2017 to In progress in 1.0 Mar 14, 2017
@frozeman frozeman moved this from In progress to Done in 1.0 Mar 16, 2017
@frozeman
Copy link
Contributor

frozeman commented Mar 16, 2017

Here it is: http://web3js.readthedocs.io/en/1.0/web3-utils.html#soliditysha3

Will be in the 1.0 once its out

@raineorshine
Copy link

Thanks Fabian!

@frozeman frozeman moved this from Done to Done Sprints in 1.0 Mar 20, 2017
@elenadimitrova
Copy link

@frozeman I noticed the soliditySha3 reuses ethjs-sha3 library which is marked defunct
https://www.npmjs.com/package/ethjs-sha3
Is this intentional?

@saveriocastellano
Copy link

saveriocastellano commented Feb 6, 2018

hi all, has anybody taken a look at this case? solidity-sha3 is still failing for me with negative big number... can anybody check ?

raineorshine/solidity-sha3#9

@blueto01h
Copy link

blueto01h commented Aug 26, 2018

@saveriocastellano try the solution with ethereumjs-abi, works like a charm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

12 participants