eth_sign behavior differences between Parity and other clients

[fumecow]

Hello, we were having issues with a script running between different nodes using eth_sign and on-chain ecrecover(). Tracked down the differences and created a diagram illustrating the differences we observed here:

[5chdn]

I'm not sure how to deal with this issue. This needs a dialogue between all client's developers. Maybe you can push things forward by stating which way you prefer and drafting a proposal?

[fumecow]

I'm not particularly vested in one versus the other. The documentation on the ethereum wiki for Web3 gives a format for the signature byte order so I would personally lean toward that format. However, v should probably be returned as 27 or 28 and not 00 or 01 as it states in the documentation.

https://github.com/ethereum/wiki/wiki/JavaScript-API#web3ethsign

I'd be inclined to change the byte ordering for Parity to match TestRPC and Geth, fix TestRPC to accept the hex encoded messages and prepend "Ethereum Signed Message" as the other clients do. Finally, TestRPC should return the correct value for v (27 or 28).

I've posted under an existing bug under TestRPC as well, here's that link: https://github.com/ethereumjs/testrpc/issues/243#issuecomment-296295847

[5chdn]

Adding #5568:

I'm trying to sign a text message with the private key of my ethereum wallet, but the function seems to be broken.

reproduce (web3 console):

> address='0x00106fE095f556e6E4d18BE78CfC263D27c1D2C2'
'0x00106fE095f556e6E4d18BE78CfC263D27c1D2C2'
> plain="hallo"
'hallo'
> msg=web3.sha3(plain)
'0xd123f0a85b26264de171387e5503b75ee1f33737c297fdba8c43a3191ff2f8d0'
> web3.eth.sign(address,msg)

now parity trusted signer asks me for the passphrase. Then I get:

'0x1bb640d5f69ba246fa909f06d5ac1b343b4c241d2485daab1d4aa53feb950eddbe0fe6fa42c6f0dd3a0b5d70153089268aa58b1c64f24d7626a14d34fd1e46e4d2'

now, this signature doesn't verify on https://etherchain.org/verify/signature nor manually (using ethereumjs-utils):

sign='0x1bb640d5f69ba246fa909f06d5ac1b343b4c241d2485daab1d4aa53feb950eddbe0fe6fa42c6f0dd3a0b5d70153089268aa58b1c64f24d7626a14d34fd1e46e4d2'
r = utils.toBuffer(sgn.slice(0,66))
s = utils.toBuffer('0x' + sgn.slice(66,130))
v = utils.toBuffer('0x' + sgn.slice(130,132))
m = utils.toBuffer(msg)
pub = utils.ecrecover(m, v, r, s)

ecrecover fails:

Error: Invalid signature v value
    at Object.exports.ecrecover (/home/brenzi/node_modules/ethereumjs-util/index.js:362:11)
    at repl:1:13
    at sigintHandlersWrap (vm.js:22:35)
    at sigintHandlersWrap (vm.js:96:12)
    at ContextifyScript.Script.runInThisContext (vm.js:21:12)
    at REPLServer.defaultEval (repl.js:313:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:513:10)
    at emitOne (events.js:101:20)

This seems to work for others (geth users?): https://www.reddit.com/r/ethereum/comments/4yamv4/signing_a_simple_message/ that's why I wonder if parity users can sign and verify messages

thanks for your help

[gavofyork]

At the time of writing, eth_sign was written to be consistent with Ethereum's RPC documentation as provided in the example on the wiki.

There should either be an EIP process to change the RPC spec on the wiki or geth should respect that wiki.

[5chdn]

Closing as intended as per comment above.

[tomusdrw]

The wiki examples now state differently (I think they never mentioned vrs actually): https://github.com/ethereum/wiki/wiki/JSON-RPC#example-21 https://gist.github.com/bas-vk/d46d83da2b2b4721efb0907aecdb7ebd#file-ercevocer-js-L42

I'm in favour of returning geth-compatible result (r,s,v where v is [27,28])

[5chdn]

Ok, are we going to include this in 1.7 milestone or rather later?

[tomusdrw]

I think would be sensible to do it in 1.7 and also backport the change to 1.6

[bwheeler96]

@fumecow you are a champion