Metamask-extension: Support current merged EIP-712 specification

[leroldary]

ethereum/EIPs#712 is merged. When/How do you plan to update from your current implementation of
eth_signTypedData to the new one specified in EIP-712?

[bdresser]

We're looking forward to updating our implementation @leroldary, no timeline yet but will update here when we have one.

[bdresser]

latest version of spec might support some features that require a tad bit of design @cjeria, we'll follow up

[bdresser]

@cjeria PR is ready above, as mentioned just need a little love in formatting the domain section as was done in the old UI version

[shaharsol]

with v4.9.0 it breaks my dapp #5014

[cjeria]

Here's the suggested new design for this screen

It's consistent with the old UI version accept for the balance which doesn't seem necessary to display for this use case. If the balance is an important piece of data in this context (signing data), we can implement a tool tip upon hovering on the accounts address.

And here's an second option with a bit more color for a bit of contrast.

@danfinlay @bdresser

[bdresser]

@cjeria these look great! I prefer the first option, it's a little more in line with our other color treatments.

• Wondering if we need an etherscan link next to the address requesting the signature (below the domain) - lots of feedback coming our way about these links in the New UI makes me think people use them regularly.
• The "domain" section up top looks good - our old UI has labels for each section which makes it feel sorta official, do you think we should add or is it just overkill? (Included screenie below for reference)
• As @bitpshr shows in the PR, there's a good chance lots of this data will be nested in an object. Paul/Dan have suggested we keep the console-like object representation shown in the GIF above for the non-domain data, is that okay with you?
• From the EIP, there's the possibility for
  
  
  
  • string name ✅
  
  
  • uint256 chainId ✅ (probably taken care of by the "Mainnet" top-right)
  
  
  • address verifyingContract ✅
  
  
  • string version "the current major version of the signing domain" - probably just like a 2.0.1 next to the "Cryptokitties"
  
  
  • bytes32 salt "a disambiguating salt for the protocol." @bitpshr is this necessary to include?
  
  

In general, I think we could launch the first version of 712 support without this fresh coat of paint. Depends on bandwidth on the UI side.

For those following along, timing & rollout plan is still TBD but we'll update here shortly.

[cjeria]

Here's an updated signature req. screen design illustrating the JSON data as per the spec.

The labels + icons in the old UI adds a lot more unnecessary noise to the screen which makes it harder to read IMO that's why I intentionally left them out. I think we can add tool tips to show the labels as well as making them clickable and link to the URL and etherscan.

I'm going to push back a little on the decision to display raw JSON data because to me it's very much catered to technical users who understand what some of these pieces of data actually mean. As user experience designers, we need to ask the question -- what's the minimum amount of data the user needs to make an informed decision? This can be done with some lite usability testing. Then bubble up that data and design an easy to read screen based on findings. Not to leave out the technical users, we should make the granular JSON data available under an advanced tab for those user who care to dig deeper.

[danfinlay]

@cjeria That looks nice! I almost didn't notice the domain section because it's formatted so nicely. I also like adding the site first letter to enrich the icon.

I'm going to push back a little on the decision to display raw JSON data because to me it's very much catered to technical users who understand what some of these pieces of data actually mean.

Unfortunately, I don't think this is an option for us here.

We don't have control over the shape of the Message data, and there are no "technical users who understand what some of these pieces of data actually mean", because every signature could look different, and the fields will be chosen by applications _specifically so that they are easier to understand for average users_. We don't know what data will be presented, so we can't assume that it's overly technical, or irrelevant to users. The problem EIP 712 was solving was multi-faceted:

• Reviewing signature proposals is often completely opaque and hard to read (just binary/hex data)
• When signatures are easy to review (like the CryptoKitty log in, or uPort attestations), it's expensive to verify on-chain.

The compromise they came up with is this sort of JSON-looking object. It actually isn't JSON, it's "structured" (don't worry, it doesn't matter for design).

This data may look ugly in this example mock, but what EIP 712 does here is it takes the very messy/difficult question of "which data do we display?" and pushes it back to the app. The Dapps are able to display the information that is important to them here.

A few examples, to clarify how this might be used:

• In an auction, it might include "max bid" and "quantity to buy".
• In a state channel, it might list an amount of money to send to another person recipient: 0x.., amountToSend: 5.
• In a uPort-like context, it might make a claim about someone else, like drivingRatingStars: 5.

All of these fields would be provided by the application, and without showing the user these values, there is no way for us to prove to the user that the application is suggesting an honest transaction to them.

We could hide more of these by doing advanced things, like permitting "a spending limit" for a site, where we don't re-ask for signing permissions, but that would have to come later, as an enhancement for the signing interface. Some signatures still need to be shown.

Since all of these fields are provided by the app, we have no way of telling which parts of this data are relevant, but we have to assume that at least some parts are (since signatures could potentially be permitting the spending of funds).

Since these signatures could send funds, we can't _not_ show the data being signed. Since we can't tell which parts are important, we have to show all of it.

We can still make it a bit prettier: We don't need to use the word "JSON" at all. We could probably even par down that title from "Message Data (JSON)" to just "Message", or "Message to Sign". The one thing we can't reduce for this particular view, unfortunately, is this structured data (as ugly as it might be). That data is actually is serving an important role, and is a big step forward for the legibility of signatures.

Over time, we can start recognizing some types of signatures, and providing custom/friendly views for them, but at the end of the day, when we don't recognize the signature method, we need a default, and I think that default should err on the side of keeping users _over_-informed.

[danfinlay]

Another way we could make it prettier: We could de-camelCase, so it would turn drivingRatingStars into Driving Rating Stars: I think this would be a pretty acceptable protocol, and I think devs would appreciate this.

[bdresser]

@cjeria I think we could probably drop colons and use color to make it a little friendlier (while still being technically accurate) like in the old UI.

[leroldary]

@danfinlay de-camelCaseing would be great!
@cjeria The domain.url parameter was removed, so www.cryptokitties.com shouldn't be shown in the domain section. The version (behind the name? E.g. CryptoKitties 2.0.1) and salt (just put it below the contract address?) should be somehow added to the domain section.

For the message, I would prefer something like this (But how to render arrays?):
Mail:
     From Person:
         Name: "Alice"
         Wallet: 0x...
    To Person:
         Name: "Bob"

@bdresser Any news on timing & rollout plan?

[danfinlay]

The `domain.url parameter was removed, so www.cryptokitties.com shouldn't be shown in the domain section.

It may not be part of the signed message, but the domain still informs the user the origin of the signature proposal, so I think it still has value to the UI here.

[shaharsol]

When you complete the breaking changes, can you publish a post similar to this one: https://medium.com/metamask/scaling-web3-with-signtypeddata-91d6efc8b290 so we all know how to modify our dapps?

[bitpshr]

@shaharsol Definitely! see https://github.com/MetaMask/metamask-extension/pull/4803#issuecomment-418135053 for more information.

[bdresser]

Hey @leroldary @shaharsol and others - this is rolling out slowly over the next week.

To read more about the change and how to implement, check out https://medium.com/metamask/eip712-is-coming-what-to-expect-and-how-to-use-it-bb92fd1a7a26