openzeppelin-contracts
openzeppelin-contracts copied to clipboard
Implement `0x00` version of EIP191 in ECDSA Library
🧐 Motivation
In the current ECDSA library, there is support to construct a message to sign according to the EIP191 Standard with the following:
- The version
0x45(E) withtoEthSignedMessage(..)function - The version
0x01with thetoTypedDataHash(..)function. But we are missing the0x00version.
📝 Details
After looking at this closed issue, saw a comment from @frangio stating that 0x00 version is not that used, which is true because people are misusing the standard, they are using the 0x45 version for everything: signing message for offchain verification, for smart contract execution based on signatures, which is dangerous.
As people could be easily tricked to sign a normal message, we should have a different mechanism, then different handling for execution based on signatures, and that's why we should make people use the 0x00 version for this case. + some projects are starting to use it like xenium and the lsp-smart-contracts (planning to use it).
I am suggesting to add a new function toDataWithIntendedValidator(...) to be compatible with the version 0x00 taking 2 parameters, <address validator> and <bytes dataToSign>.
The function would be:
function toDataWithIntendedValidator(address validator, bytes memory dataToSign) internal pure returns (bytes32) {
return keccak256(abi.encodePacked("\x19\x00", validator, dataToSign));
}
If you're okay with it I am happy to open the PR, otherwise you can close the issue 😄
Hello @YamenMerhi
That is an interesting proposal. Do you know any wallet that include support for version 0x00 signing ?
Hey @Amxx
As I stated in my message, we are planning to use it in the LSP6KeyManager contract, also when I met nick.eth in DevCon he mentioned that he uses it for xenium. Also when I checked argent, their old multisig use it with a weird combination of 0x45 version.
With the rise of smart contract-based accounts and wallets, I think it will start to be used more and more, and we should push to have it as anyone could be tricked to sign a 0x45 message and end up having a smart contract execution based on this signature where people thinks its just for offchain or SIWE purposes.
I made a PR to the EIP191 standard to make it more comprehensible and the LUKSO team is also developing a library EIP191-Signer that allows signing different versions of EIP191 messages (0x45 and 0x00). Hopefully these steps and tools will make 0x00 version more adopted.
Thanks for all this information!
I was only saying that because many devs still make the error of not using the "preparation" function, and not understanding the difference between rawsign and personalsign. I hope adding this new function doesn't add to the confusion.
Also, I want us to test it, and for that we'll need to produce the corresponding signature, hopefully without using some low level rawsign.
@Amxx Definitely we want to avoid confusion as much as possible. If you want to create a list of things to test, change, do please do, and let's sync on it 😀
Any updates @Amxx @frangio 🙏
Sorry for the silence. This is simple enough, I'm not opposed to a PR implementing it.
Okay will do a PR, thanks! @frangio