Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Inconsistent behavior of web3.eth.sign between ganache-cli and hdwallet-provider #972

Closed
fedgiac opened this issue May 28, 2020 · 6 comments · Fixed by #1716
Closed

Inconsistent behavior of web3.eth.sign between ganache-cli and hdwallet-provider #972

fedgiac opened this issue May 28, 2020 · 6 comments · Fixed by #1716

Comments

@fedgiac
Copy link

fedgiac commented May 28, 2020

Expected Behavior

The output of web3.eth.sign should be consistent when using different Ethereum clients, including Ganache.

Current Behavior

The output of web3.eth.sign when using Ganache is different from that of other Ethereum client implementations in that the last byte of the signature (v) is either 00 or 01, while in other implementations it's 1b or 1c. Notably, the behavior of web3.eth.sign when only using Ganache is inconsistent with that of @truffle/hdwallet-provider.

Note that this inconsistency is not limited to Truffle projects.

  • The current example in the Ethereum Wiki however shows that the last output byte resulting from an RPC call to eth_sign is 1b (Ganache would return 00 instead). The behavior was apparently different in previous versions of this documentation (source).
  • The current examples in the documentation of Web3.js show that the last output byte of a call to web3.eth.sign is 00/01.
  • Calling eth_sign in Parity/OpenEthereum returns 1b/1c, as well as calling web3.eth.sign with Truffle using Parity/OpenEthereum as its client.
  • Calling eth_sign in Geth returns 1b/1c.

Possible Solution

Ganache and @truffle/hdwallet-provider should settle on a single standard output of web3.eth.sign.
I would recommend changing Ganache's behavior so that the final byte of an RPC call to eth_sign is 1b or 1c for consistency with Geth and Parity/OpenEthereum. This change would modify the last byte returned by web3.eth.sign accordingly, since it's just a wrapper around eth_sign.

Steps to Reproduce

Install truffle, ganache-cli, @truffle/hdwallet-provider.

Run ganache-cli --mnemonic="shove shoulder neutral steak day correct neither girl alcohol modify bacon fee"

Create a new Truffle project with the following network configuration:

var HDWalletProvider = require("@truffle/hdwallet-provider");
var mnemonic = "shove shoulder neutral steak day correct neither girl alcohol modify bacon fee";
var provider = new HDWalletProvider(mnemonic, "http://localhost:8545");
module.exports = {
  networks: {
    ganache: {
      host: "127.0.0.1",
      port: 8545,
      network_id: "*"
    },
    hdwalletprovider: {
      provider,
      network_id: "*"
    }
  }
};

Create Truffle script script.js with the following content:

module.exports = async (callback, b) => {
  const accounts = await web3.eth.getAccounts();
  console.log(await web3.eth.sign("test string", accounts[0]));
  callback();
};

Test the script on both networks:

$ npx truffle exec script.js --network=ganache
Using network 'ganache'.

0x0bef479e20b9186de969a6b1054892d007b870e8086cc162e0b72508a08742ba3830b16c42816dfb0420ae1f509086587971b0c0ada01fff6b17a91e2fddad8a00
$ npx truffle exec script.js --network=hdwalletprovider
Using network 'hdwalletprovider'.

0x0bef479e20b9186de969a6b1054892d007b870e8086cc162e0b72508a08742ba3830b16c42816dfb0420ae1f509086587971b0c0ada01fff6b17a91e2fddad8a1b

Observe that the final bytes differs (00 vs. 1b).

Context

This issue was found out while debugging an inconsistent behavior between production and testing in a script relying on web3.eth.sign, see this PR for details.

Your Environment

Truffle v5.1.27 (core: 5.1.27)
Solidity v0.5.16 (solc-js)
Node v12.16.3
Web3.js v1.2.1
Ganache-cli 6.9.1
@truffle/hdwallet-provider 1.0.35

If the changes I propose in this issue were greenlighted, I'd try to submit a PR implementing them.

@davidmurdoch davidmurdoch transferred this issue from trufflesuite/ganache-cli-archive Aug 19, 2021
@kevinbluer
Copy link
Member

Hey @fedgiac, apologies for the slight delay! We think this might be a dupe of #556 and is fixed as part of ganache@7.0.0-alpha.0.

Let us know if this resolves this.

@fedgiac
Copy link
Author

fedgiac commented Sep 10, 2021

Hi Kevin, I tried to run the steps to reproduce the issue and found that while Ganache behaves differently from version six, this time the final byte is replaced by two bytes (0a95), which I think might be a bug in the implementation of signing.

$ npx truffle exec script.js --network=ganache
Using network 'ganache'.

0x0bef479e20b9186de969a6b1054892d007b870e8086cc162e0b72508a08742ba3830b16c42816dfb0420ae1f509086587971b0c0ada01fff6b17a91e2fddad8a0a95
$ npx truffle exec script.js --network=hdwalletprovider
Using network 'hdwalletprovider'.

0x0bef479e20b9186de969a6b1054892d007b870e8086cc162e0b72508a08742ba3830b16c42816dfb0420ae1f509086587971b0c0ada01fff6b17a91e2fddad8a1b
$ npx ganache-cli --version
ganache v7.0.0-alpha.0 (@ganache/cli: 0.1.1-alpha.0, @ganache/core: 0.1.1-alpha.0)

@davidmurdoch
Copy link
Member

@fedgiac 0x0a95 (and 0x0a96) is the v value for a chain with an id of 1337, which is ganache's default chainId. If you want it the signature to match mainnet start ganache with a chainId of 1: ganache --chain.chainId 1

Note: starting ganache with a chainId that matches any other chain effectively removes the replay protection provided by EIP-155. So if you do this make absolutely sure you never use a real account/mnemonic with ganache (you shouldn't do this already... but now you REALLY shouldn't!)!

Let me know if this clears things up for you so I can close this issue or continue investigating.

@fedgiac
Copy link
Author

fedgiac commented Nov 25, 2021

As far as I know message signatures do not depend on the chain id like it is the case for Ganache.
To test this I run the signing code on a Rinkeby Geth node (version 1.10.12-stable, chainId 4) and the returned signature is still ending in 1b (and 1c). If the implementation depended on the chain id I'd expect the v value to somehow include the chain id 4.

In particular, eth_sign signatures can easily be replayed across networks, this is why other signing implementations like EIP-712 recommend to include the chain id in the signed data. Changing just the v value in any case wouldn't help as we can just replace it with the corresponding v value for mainnet and get a valid mainnet signature.

Note that here we are talking about message signing and not transaction signing, so even if likely related the issue that was just linked to this is different.

@davidmurdoch
Copy link
Member

Oh wow, I believe your are correct. Well get this fixed asap (which will be next week)!

@sylar123abc
Copy link

Hello it seems that the problem is here again, I have signatures ending with 01 instead of 1c using eth_sign calls, the problem seems to be coming from the 7.7.3 and up version (maybe even from 7.7.0)

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

Successfully merging a pull request may close this issue.

4 participants