EIPs
EIPs copied to clipboard
Update EIP-191: Improve Comprehensibility
What does this PR introduce?
- Updating the example to be more comprehensive + more updated.
- Simplifying the EIP191 Standard, by providing more explanation of the different versions.
- Re-ordering the text so we start by: - The formula for the standard - Why we have 0x19 - What are the different bytes version we have
- Explicitly mentioning that
Eis version0x45andthereum Signed Message:\n" + len(message)is version specific data.
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):
(fail) eip-191.md
| classification |
|---|
updateEIP |
- eip-191.md is in state final at the head commit, not draft or last call or review; an EIP editor needs to approve this change
- This PR requires review from one of [@axic, @samwilsn, @pandapip1]
- eip-191.md requires approval from one of (@holiman, @arachnid)
I know this is an old standard and missing some sections, I am happy to add them. Also was wondering if we should mention in a Security Consideration section that it's advised to use 0x00 for any execution based on signature as people could be easily tricked to sign normal 0x45 messages as it's normally used for SIWE and for offchain verification
Ok thank you I truly appreciate all the help and wisdom I am learning thank you for your care...I will send more gracious gifts in the near future.
On Fri, Oct 28, 2022, 1:14 PM Sam Wilson @.***> wrote:
@.**** requested changes on this pull request.
In EIPS/eip-191.md https://github.com/ethereum/EIPs/pull/5804#discussion_r1008321635:
+0x19 <0x00>
+```
+The version
0x00has<intended validator address>for the version specific data. In the case of a Multisig wallet that perform an execution based on a passed signature, the validator address is the address of the Multisig itself. The data to sign could be any arbitrary data. + +#### Version0x01+ +The version0x01is for structured data as defined in [EIP-712] + +#### Version0x45(E) + ++0x19 <0x45 (E)> <thereum Signed Message:\n" + len(message)> <data to sign> ++ +The version0x45(E) has<thereum Signed Message:\n" + len(message)>for the version specific data. The data to sign could be any arbitrary data.So this treats the E as the version? I would explicitly mention that the E is consumed, and thereum is not a typo :P
In EIPS/eip-191.md https://github.com/ethereum/EIPs/pull/5804#discussion_r1008326879:
// Arguments when calculating hash to validate// 1: byte(0x19) - the initial 0x19 byte // 2: byte(0) - the version byte
- // 3: this - the validator address
- // 4-7 : Application specific data
- transactionHash = keccak256(abi.encodePacked(byte(0x19),byte(0),address(this),destination, value, data, nonce));
- sender = ecrecover(transactionHash, v, r, s);
- // ...
- // 3: address(this) - the validator address
- // 4-6 : Application specific data
- bytes32 hash = keccak256(abi.encodePacked(byte(0x19), byte(0), address(this), msg.value, nonce, payload));
- // recovering the signer from the hash and the signature
- addressRecovered = ECDSA.recover(hash, signature);
Hm, I'm leery about changing this from plain ecrecover to what I assume is supposed to be OZ's. Their implementation has additional restrictions that plain ecrecover doesn't have. Changes to final EIPs are supposed to be backwards compatible.
In EIPS/eip-191.md https://github.com/ethereum/EIPs/pull/5804#discussion_r1008327763:
-function submitTransactionPreSigned(address destination, uint value, bytes data, uint nonce, uint8 v, bytes32 r, bytes32 s)
- public
- returns (bytes32 transactionHash) -{ +function signatureBasedExecution(address target, uint256 nonce, bytes memory payload, bytes memory signature)
- public payable {
- // Arguments when calculating hash to validate // 1: byte(0x19) - the initial 0x19 byte // 2: byte(0) - the version byte
- // 3: this - the validator address
- // 4-7 : Application specific data
- transactionHash = keccak256(abi.encodePacked(byte(0x19),byte(0),address(this),destination, value, data, nonce));
- sender = ecrecover(transactionHash, v, r, s);
- // ...
- // 3: address(this) - the validator address
- // 4-6 : Application specific data
- bytes32 hash = keccak256(abi.encodePacked(byte(0x19), byte(0), address(this), msg.value, nonce, payload));
- // recovering the signer from the hash and the signature
- addressRecovered = ECDSA.recover(hash, signature);
- // logic of the wallet
- if (addressRecovered == owner) executeOnTarget(target,payload);
This is not the minimal change required to fix errata. Since this is a final EIP, we only allow clarifications and correcting errors. The minimal change would likely be just changing the 7 to a 6, unless there's something else I missed.
— Reply to this email directly, view it on GitHub https://github.com/ethereum/EIPs/pull/5804#pullrequestreview-1160440212, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3H6XK6YKMDAJ2TAF3FJBPDWFQJZ3ANCNFSM6AAAAAARJPOIPU . You are receiving this because you are subscribed to this thread.Message ID: @.***>
@SamWilsn @Pandapip1 Applied the changes 👍
@Pandapip1 Done 👍