Implement Ethereum Sign/Verify message once EIP712 is widely adopted

[Ivshti]

In Ethereum, signed messages are usually prefixed with "Ethereum signed message:\n" in order to avoid being able to trick the user into signing transactions.

However, dapps such as decentralized exchanges and others that use signed messages, usually make the user sign a hash. This is not very user friendly, since the user still does not know what they're allowing.

An EIP exists to solve that: ethereum/EIPs#712

This essentially means that the user will know what they're signing, but it requires implementation in wallets. Since the Model T has a large-ish touchscreen, this EIP makes a lot of sense in order to enhance the UX of dapps in the future.

Another consideriation is that since this is a standard, it will remove issues like the one where Trezor uses a bitcoin variant length integer after "Ethereum signed message", vs the ASCII formatted used in Metamask and geth: ethereum/go-ethereum#14794 ( https://github.com/0xProject/0x.js/pull/376/files#diff-0dcbf3991e702af6b8de8208658c581aR111 and https://github.com/AdExBlockchain/adex-core/blob/signed/contracts/ADXExchange.sol#L275 )

[Ivshti]

Also, we (AdEx Network) would like to implement this in trezor-core. Is there a way we can obtain a dev kit?

[prusnak]

@Ivshti No, we only have production devices or you can develop using the emulator.

[marc0olo]

@prusnak when do you expect this feature to be implemented? seems to be necessary for many ethereum dapps and hinders people being able to use them with the new trezor-integration of metamask

[prusnak]

@marc0olo Trezor is a community project and so far no one from the community is working on this feature at the moment. Thus I can't give you any estimates.

Maybe @Ivshti is up for the task?

[kellerassel007]

would love to use this feature for signing up with meta mask for Neufund.org platform :)

[rmeissner]

@marc0olo you were asking for how to implement this.

I create a python implementation for EIP-712 (https://github.com/rmeissner/py-eth-sig-utils) you could start by making this code compatible with micropython.

Once that step is done you would have to eliminate the dependencies (requirements.txt).

The last and easiest step would be to wire up a new protobuf event.

After that you should add tests to show that everything works.

If I find time I would try to implement this myself, but not sure before devcon.

[marc0olo]

currently very busy ... hopefully me or someone else finds time to implement this soon

[kellerassel007]

Any updates on this? Many people can't wait for using xxxxxxxxx platform with MetaMask+Trezor for signup & secure funds.

[prusnak]

No updates. Feel free to send a pull request.

[Ivshti]

@prusnak I don't think implementing EIP 712 is a good idea at the moment.

This EIP has been changed in significant ways a few times already, so I'd wait until it's stable. I don't think the latest iteration is even implemented in metamask.

Edit: it's just now rolling out: MetaMask/metamask-extension#4752

[rmeissner]

I wouldn't agree. The eup has not really changed in the last weeks and the only thing that is not 100% defined is how to handle arrays in the data. If you wait until it is cobsidered final before you consider implementing this you will be late.

Not saying that this has to merged immediately, but having this in would be a huge step forward for ux of signing and with the model t gives a lot of possibilities to display the data. So starting to work on it early so that it can be merged when the eip has been reviewed would make sense for me.

[jamesmorgan]

Hi guys, after reading this blog post https://medium.com/metamask/eip712-is-coming-what-to-expect-and-how-to-use-it-bb92fd1a7a26 I am just checking if its the right time to start using this i.e. is it supported by metamask yet?

[prusnak]

@jamesmorgan Yes, you can start implementing this for Trezor firmware. I will gladly review a pull request adding this feature.

[rmeissner]

@jamesmorgan do you want to implement this or are you just asking if this can be used now?

[jamesmorgan]

@rmeissner I was just checking to see if its supported yet, mainly as one of our app is going to start using ERC-712

[rmeissner]

@jamesmorgan ahh ok, so afaik trezor is not supporting this yet. But we also want to use this and will probably put some work into this soon ;)

[rmeissner]

@prusnak Hey, wanted to check what the state on this is. We would love to add Trezor support for the Gnosis Safe, but we depend on the EIP-712. We are aware that some parts are not finalized in the EIP (namely array support), but it would good to know how we could help out on pushing this forward. :)

[prusnak]

@rmeissner You can push this forward by contributing a pull request which adds the functionality.

[rmeissner]

We can do this, just wanted to check if there is already something in progress, to avoid duplicated work ;)

[prusnak]

Thanks! AFAIK there is no-one working on that.

[prusnak]

We switch to the monorepo, please send PR there: https://github.com/trezor/trezor-firmware