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

Allow binary message for wallet.signMessage. #80

Closed
jennazenk opened this issue Nov 29, 2017 · 11 comments
Closed

Allow binary message for wallet.signMessage. #80

jennazenk opened this issue Nov 29, 2017 · 11 comments
Labels
enhancement New feature or improvement.

Comments

@jennazenk
Copy link

jennazenk commented Nov 29, 2017

hello @ricmoo , what is the equivalent of web.eth.sign() (https://github.com/ethereum/wiki/wiki/JavaScript-API#web3ethsign) in ethers.js ?
I am trying to sign a piece of data in order to retrieve s, r and v from the signature. I tried with Wallet.sign() and also SigningKey.signDigest, but I don't get the same output as when I call web3.eth.sign().

The purpose of that is that I need to call the solidity ecrecover function to verify that my address has signed the data.

Thanks a lot!

@jennazenk
Copy link
Author

Just saw this issue was opened #78 . It is a bit related and I've tried both way (with wallet instance and signing key) and I can't find the right output. Also, should signing the same piece of data through the wallet instance or through the signing key output the same data?

@ricmoo
Copy link
Member

ricmoo commented Nov 29, 2017

Hey Jenna!

Can you provide an example of a privateKey (obviously newly created for this sample) as well as data and the output you expect?

Also keep in mind that Web3 treats strings inconsistently. So the string "0x12". is 1 byte long, but the string "12" is two bytes long. And as a result there is no easy way to represent the 4 byte string "0x12", except to expand it to "0x30793132".

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Nov 29, 2017
@ricmoo ricmoo changed the title Signing data Discussion: How to match Web3 signing data Nov 29, 2017
@ricmoo
Copy link
Member

ricmoo commented Nov 29, 2017

by the way, have you checked out:

var w = new ethers.Wallet('0x123456789012345678901234567890123456789012345678901234567890123');

var sig = w.signMessage("Hello World");
//'0xe0ed34fbbe927a58267ce2e8067a611c69869e20e731bc99187a8bc97058664c16de07f7660f06ce0985d1d8e063726783033fda59b307897f26a21392d62b3a1c'

var address = ethers.Wallet.verifyMessage("Hello World", sig);
// address === w.address

@jennazenk
Copy link
Author

Thanks @ricmoo for your answer! I tried this indeed, and verifyMessage does return my account address. Still, its not the same output as the one I have when signing with api.eth.sign in parity.js .Pinged you on gitter to send you an example

@jennazenk
Copy link
Author

jennazenk commented Dec 1, 2017

Hey @ricmoo , here is some code to reproduce. I used ethereumjs-util (think its easier to reproduce than with parity.js). As you can see, signing with ethers.js and signing with ethereumjs-util doesn't give the same output.

const ethers = require("ethers");
const sigUtil = require("eth-sig-util");
const ethUtil = require("ethereumjs-util");

const hash =
  "0x47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad";

const wallet = new ethers.Wallet.fromMnemonic(
  "loan million aware switch length candy artwork finish comfort eyebrow mimic estate"
);
const privateKey = wallet.privateKey;

//SIGN WITH ETHERS-WALLET
const signEthWallet = () => {
  const rawSignature = wallet.signMessage(hash);
  const walletSignature = rawSignature.substr(2, rawSignature.length);
  return {
    fullSignature: rawSignature,
    r: `0x${walletSignature.substr(0, 64)}`,
    s: `0x${walletSignature.substr(64, 64)}`,
    v: parseFloat(walletSignature.substr(128, 2)) + 27
  };
};

//SIGN WITH ETH-SIG-UTIL
const signEthSig = () => {
  const sigUtilSignature = sigUtil.personalSign(ethUtil.toBuffer(privateKey), {
    data: hash
  });
  const personalSignature = sigUtilSignature.substr(2, sigUtilSignature.length);
  return {
    fullSignature: sigUtilSignature,
    r: `0x${personalSignature.substr(0, 64)}`,
    s: `0x${personalSignature.substr(64, 64)}`,
    v: parseFloat(personalSignature.substr(128, 2)) + 27
  };
};

const resultWithEthersJs = signEthWallet();
console.log(resultWithEthersJs);
// { fullSignature: '0x9476cf9425e8cb30f0589915e2900152aa6fcfeb15c097a090db3f43484f1c3d3c3d006896fcedd56b0d574da440347886f0d95a73f2ff013d621d200ab9dee11b',
//   r: '0x9476cf9425e8cb30f0589915e2900152aa6fcfeb15c097a090db3f43484f1c3d',
//   s: '0x3c3d006896fcedd56b0d574da440347886f0d95a73f2ff013d621d200ab9dee1',
//   v: 28 }
const resultWithEthSigUtil = signEthSig();
console.log(resultWithEthSigUtil);
// { fullSignature: '0x546f0c996fa4cfbf2b68fd413bfb477f05e44e66545d7782d87d52305831cd055fc9943e513297d0f6755ad1590a5476bf7d1761d4f9dc07dfe473824bbdec751b',
//   r: '0x546f0c996fa4cfbf2b68fd413bfb477f05e44e66545d7782d87d52305831cd05',
//   s: '0x5fc9943e513297d0f6755ad1590a5476bf7d1761d4f9dc07dfe473824bbdec75',
//   v: 28 }
```

@ricmoo
Copy link
Member

ricmoo commented Dec 2, 2017

Ok, I've found the problem.

I have updated the signMessage API to allow binary data to be passed in. Before it only allowed UTF-8 strings. You will have to make a small change to the above code, since "0x1234" is a valid string (of 6 bytes); the eth-sig-util is using the message as a binary payload.

var hash = "0x47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad"

// To use the 66 byte string (not what you want)
wallet.signMessage(hash);

// To use the 32 byte binary data (matches the above eth-sig-util)
var hashData = ethers.utils.arrayify(hash);
wallet.signMessage(hashData);

I've also added your example to the test-cases.

Once Travis-CI has completed, I'll publish to npm and close this issue. You'll have to delete your package-lock.json and node_modules and redo an npm install, but then you should be off to the races. :)

@ricmoo ricmoo added enhancement New feature or improvement. and removed discussion Questions, feedback and general information. labels Dec 2, 2017
@jennazenk
Copy link
Author

Awesome thanks @ricmoo

@ricmoo ricmoo changed the title Discussion: How to match Web3 signing data Allow binary message for wallet.signMessage. Dec 2, 2017
@ricmoo
Copy link
Member

ricmoo commented Dec 2, 2017

Just published to npm.

Thanks @jennazenk!

@Perseverance
Copy link

Thank you @jennazenk and @ricmoo ! This saved me (after spending whole Sunday trying to make it work). I would love for this to be included in the cookbook part of the docs as the most common case would be off-chain wallet.signMessage with on-chain ecrecover (or atleast this is what I would need if I need to implement state chanels). I can provide example if you need!

@ricmoo
Copy link
Member

ricmoo commented Mar 25, 2018

Excellent Point! I will add it to my todo list for the Cookbook.

@Perseverance
Copy link

Here is my playground: https://gitlab.com/limelabs/ecrecover-test/tree/master

I feel that in order for this to be a fully comprehensive example the Smart Contract part might be needed too. The meaty part is in scripts/sign.js

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

No branches or pull requests

3 participants