EIP-712: eth_signTypedData as a standard for machine-verifiable and human-readable typed data signing


#1

For on-going discussion and feedback for EIP712.

Draft is available here

Left old, new right


#2

Just a minor point that should probably be cleaned up.

In the draft spec…

Returns

DATA: Signature. As in eth_sign it is a hex encoded 129 byte array starting with 0x. It encodes the r, s and v parameters from appendix F of the yellow paper in big-endian format. Bytes 0…64 contain the r parameter, bytes 64…128 the s parameter and the last byte the v parameter. Note that the v parameter includes the chain id as specified in EIP-155.

But EIP-155 Chain IDs can be larger than a byte (elsewhere in the spec it is encoded as uint256).

Clients probably should not rely on a fixed length response of 129 bytes. Perhaps “a hex encoded byte array of at least 129 bytes…”


#3

This is copied almost verbatim from the official Ethereum JSON RPC. Personally, I think packing the three values into a hex string is a bad idea and I would rather have a structure with v, r, s attributes.

Given the elsewhere mentioned confusion about v,r,s vs. r,s,v order and the above mentioned assumptions on the size of v I’m inclined to go for the object approach instead of packed binary.

What do you think?


#4

I agree that it’d probably be clearer to return a JSON object. Labeled fields eliminate a lot of ambiguity.

If you don’t do this, you’ll have to keep it in r,s,v order given the variable length of v. (And that’s a bit weird, since transactions are signed in v,r,s order.)


#5

What is the purpose of the domain separator?

The domain separator prevents collision of otherwise identical structures. It is possible that two DApps come up with an identical structure like Transfer(address from,address to,uint256 amount) that should not be compatible. By introducing a domain separator the DApp developers are guaranteed that there can be no signature collision.

What I mean is, should it be verified “against” anything?

Great work by the way, @Recmo!


#6

Hello, I don’t exactly know where to ask questions about EIP’s, so I’m trying my luck here …

At the end of EIP-712’s draft, there is a link toward example files (.sol for the smart contract and .js for the tester). In the smart contract, hashes are computed using keccak256(abi.encode(...)). I am used to doing keccak256(abi.encodePacked(...)), which is slightly different when dealing with addresses and correspond to web3.utils.soliditySha3.

using encodePacked and soliditySha3 I managed to have both part of my application compute hashes the same way, but in order to benefit from the EIP full potential I have to make sure other entities (like metamask) will be able to reproduce my hashes.

The thing is, encodePacked can be nested, which is a feature I need due to my struct having to much members → too much local variable to do a single abi.encode.

I saw in the EIP that this is discussed as “alternatives” … and that the encoding should be feaseable in place … but not being fluent in assembly, I’m not very confident in my ability to do the hashStruct myself …


#7

Hi,
I have just heard that this EIP is getting finalized. I would like to propose an important change:

It has been discussed briefly before and was the reason domain separator came into being in the first place but I would like to bring back the possibility again to add the “origin” back into the data being signed.
see https://github.com/ethereum/EIPs/pull/712#issuecomment-330501545
and https://github.com/ethereum/EIPs/pull/712#issuecomment-364495160

This would allow smart contract to ensure the user has signed the data in a set of allowed origin.

The web3 signer would be in charge to add the origin data. This is not a data that can be set independently so that the user can’t be cheated in thinking the data is for another purpose.

Note that this extra data can only increase the security of the proposal.