MetaMask / MetaMask/metamask-extension

Make signTypedData domain headers more consistent/less confusing

Open
#13,703 4 comments 0 reactions 1 assignee Claimed by @jpuri View on GitHub
area-signTypedData
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

In the OpenSea migration to the Wyvern contracts, they've switched to using EIP-712 signTypedData, which improves signature readability, but the current header is confusing to their users, because it shows info about the contract instead of their site (like their users are used to).

![image](https://user-images.githubusercontent.com/542863/155000147-d0d31318-8fc7-4a24-8c86-3d89168203a5.png)

This has led to [OpenSea requesting a special header](https://github.com/MetaMask/metamask-extension/pull/13685) on EIP-712 signatures when requested from their site. A hard-coded special case is a huge code smell imo, and we should be looking at the root of the problem and addressing it in a way that alleviates this future problem for other sites as well.

First off, @Gudahtt notes that the contract name shown here is dapp-provided, and is part of the `domain separator` part of the signed payload [as shown here](https://eips.ethereum.org/EIPS/eip-712#definition-of-domainseparator), so it can't be trivially updated from the client side.

## Fundamental fix

Why did this suddenly surprise them? Probably because EIP-712 signatures currently have a header that is inconsistent with our other signature views. Those views currently show data purely reflecting the requesting site (the page `title`, and the page metadata defined preview image). So that means `personal_sign` (what they were previously using) and `signTypedData` (what they are now using) are both using different ways of rendering data provided by the dapp, and so the simplest fix would be to simply change the `signTypedData` header to behave the same way the `personal_sign` header works, to totally side-step the inconsistency confusion.

## Longer term questions

Is it safe to show arbitrary data from an untrustworthy dapp? Well, it doesn't help users recognize a phishing site, that's for sure, and we have work to do in that direction, but I think we can treat it as a separate issue for now. This issue is just about header consistency, and then we can talk about improving header safety.

## Acceptance criteria

- EIP-712 signTypedData signatures should use the same header rendering logic as `personal_sign`.
- These changes should also be reflected on mobile.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.