Skip to content
This repository has been archived by the owner on Apr 20, 2021. It is now read-only.

[ECDSA] Add toEthSignedMessageHash #82

Open
nrryuya opened this issue Jan 28, 2019 · 1 comment
Open

[ECDSA] Add toEthSignedMessageHash #82

nrryuya opened this issue Jan 28, 2019 · 1 comment
Labels
enhancement New feature or request

Comments

@nrryuya
Copy link
Member

nrryuya commented Jan 28, 2019

From @vignesh-msundaram.


I also have been trying to use your ecrecover implementation from here : https://github.com/LayerXcom/verified-vyper-contracts/blob/master/contracts/ecdsa/ECDSA.vy

Since I'm using web3.eth.sign(), I realized the ecrecover didn't return the correct signer address.
So, we need to replace Line 23 :

ecrecover(_hash, convert(v, uint256), r, s)

with

ecrecover(sha3(concat("\x19Ethereum Signed Message:\n32", _hash)), convert(v, uint256), r, s)

Also, it's better to let user send r,s,v from web 3 using web3._extend.utils.toBigNumber() instead of parsing the bytes[65] input.

Eg,
In web3,

let _signature = web3.eth.sign(_signer, _message)
_signature = _signature.substr(2)
let _v = _signature.slice(128, 130) === '00' ? web3.utils.toBigNumber(27) : web3.utils.toBigNumber(28)
let _r = web3._extend.utils.toBigNumber("0x"+_signature.slice(0, 64))
let _s = web3._extend.utils.toBigNumber("0x"+_signature.slice(64, 128))
console.log(await contract.VerifySignature(_message, _v, _r, _s) === _signer)

Then in contract,

def VerifySignature(_message: bytes32, _sig_data: uint256[3]):
    return ecrecover(sha3(concat("\x19Ethereum Signed Message:\n32", _message)), _sig_data[0], _sig_data[0], _sig_data[0])

Reference : https://ethereum.stackexchange.com/questions/15364/ecrecover-from-geth-and-web3-eth-sign

@nrryuya nrryuya changed the title [ECDSA] [ECDSA] Add toEthSignedMessageHash Jan 28, 2019
@nrryuya
Copy link
Member Author

nrryuya commented Jan 28, 2019

About the prefix \x19Ethereum Signed Message:\n32, we can consider adding this function.

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/cryptography/ECDSA.sol#L54

@nrryuya nrryuya added the enhancement New feature or request label Jan 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant