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

soliditySHA3 multiple bytes32 encoding issue #43

Closed
marekkirejczyk opened this issue Jun 27, 2017 · 7 comments
Closed

soliditySHA3 multiple bytes32 encoding issue #43

marekkirejczyk opened this issue Jun 27, 2017 · 7 comments

Comments

@marekkirejczyk
Copy link

It seems soliditySHA3 does not handle properly case when multiple bytes32 are involved.
Steps to reproduce below:

abi.soliditySHA3([ "bytes32", "bytes32" ], [ "Volume", "Volume" ]).toString('hex');
returns:
8239dae94d559efaf979a8ffdb349ed1d13c6636d28ab641f43ae2bc0c0b985a

Yet,

contract TestTightHash {    
    function coded() constant returns (bytes32) {
        return keccak256(bytes32("Volume"), bytes32("Volume"));
    }
}

coded() returns 0x7ab276f38b269dc879fb051ca72ac238c1a03c2db62bbd7a499c1dd822c3e020

@marekkirejczyk
Copy link
Author

marekkirejczyk commented Jun 27, 2017

It works in code downloaded for the master. I think it might be a matter of releasing new version.

Checked: The code on master and in latest version (0.6.4) differ in function solidityPack.
The problem is in the solidityPack function, which in 0.6.4 returns only zeros after first bytes32 was encoded.
Version on master works fine.

@thisbejim
Copy link

Also confirming this issue, you must install from master for soliditySHA3 to work properly. I recommend publishing the latest version to npm.

@D-Nice
Copy link

D-Nice commented Oct 25, 2017

The issue is with any static bytes type, on the package published on npm.

It has to do with the solidityPack function, which in the npm package does a return

      return utils.setLengthRight(value, size)

while in this repo it seems fixed and actually pushes to the ret array instead:
https://github.com/ethereumjs/ethereumjs-abi/blob/master/lib/index.js#L453

Requesting for an update also here
#54

@gkucmierz
Copy link

Anyone can resolve that?

@D-Nice
Copy link

D-Nice commented Dec 4, 2017

pinging @axic

@marekkirejczyk
Copy link
Author

marekkirejczyk commented Dec 4, 2017

@holgerd77
Copy link
Member

New npm release is out: https://www.npmjs.com/package/ethereumjs-abi

Sorry for the delay, will close now.

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

5 participants