Support for EIP-712 message signing

[Mrtenz]

It seems to be not possible to sign EIP-712-based messages right now, because the message is prepended with \x19Ethereum Signed Message:\n here:

app-ethereum/src_features/signMessage/cmd_signMessage.c

Line 57 in 8d0544b

cx_hash((cx_hash_t *)&sha3, 0, (uint8_t*)SIGN_MAGIC, sizeof(SIGN_MAGIC) - 1, NULL, 0);

This is already implemented by MetaMask, and it would be nice to be able to sign EIP-712 messages with Ledger devices as well.

[TamtamHero]

We're currently testing it, an app update should be available soon®
(cf: https://github.com/LedgerHQ/app-ethereum/tree/eip712_v0)

[A2be]

What is current status on Ledger's implementation of EIP712 to support transaction signing?

MetaMask, Gnosis Safe multisig, and many others support it, and without it, the security case for using a Ledger in concert with these other wallets goes down significantly. Pinging @TamtamHero

[TamtamHero]

This is supported already: #105

[A2be]

@TamtamHero : It appears EIP-712 compatibility is also not working with the Gnosis Safe Multisig, per discussion on Gnosis #safe Discord ( [https://discord.com/channels/477106835862716416/477391201708802058/801466336873283594] ).

Maybe you and @tobias_s from Gnosis could chat to see if the Gnosis dev team is aware that Ledger think EIP-712 is correctly implemented.

[tschubotz]

This is supported already: #105

@TamtamHero You linked this exact same isse (#105) Did you mean to link another issue in your comment? :)

That would be great news if Ledger supports EIP712! With which Firmware update did that go live?

[TamtamHero]

indeed, my bad!
https://github.com/LedgerHQ/app-ethereum/blob/master/doc/ethapp.asc#sign-eth-eip-712
Available since 91e4b45
So, since Eth v1.6.0

[tschubotz]

Oh nice! Would a user actually be able to review the different EIP712 fields on their Ledger display?
I still just see the hash to sign, and not the human readable message fields. Now I'm wondering if I do anything wrong.

[Mrtenz]

Is this implemented in Ledger.js, or do we have to manually send the data to the device for now? Either way, sweet that this has been implemented.

[TamtamHero]

For implementation version 0, the domain hash and message hash are provided to the device, which displays them and returns the signature
So, for the time being, the data is not displayed on the device.

The call is available from ledger.js: https://github.com/LedgerHQ/ledgerjs/tree/master/packages/hw-app-eth#signeip712hashedmessage

[A2be]

With the data not displayed on the device, the user who wants to carefully validate their signature on, say, a Gnosis Safe Multisig transaction approval/signing, cannot actually validate that they are signing the correct transaction.

So my encouragement for you is to definitely fix it so that the user can use the Ledger display device to usefully, and securely, approve an EIP-712 transaction.

[SCBuergel]

Bump.

This is a serious concern for e.g. Gnosis Safe users (reminder: if you want to claim your wallet protects a large amount of ETH and Ethereum based value then you must support Gnosis Safe) who are currently unable to verify the tx idahoan on screen - not even with hex code. All you currently see us a hash. So right now you need a laptop to verify the hash and that laptop is the device you didn't trust in the first place and therefore got a hardware wallet.

[SCBuergel]

Ok the issue seems resolved, see above. I suggest to close this issue.

[Xeonus]

Now that Uniswap v3 uses EIP-712 for signing transactions like LP migration, can you update users on the status of this issue? Otherwise anyone using Metamask together with Ledger is stuck on Uniswap v2 for the time-being.

[pscott]

@Xeonus EIP-712 is already supported by the Ledger. Metamask has not updated its hw-app-eth dependency though.
You can track the actual issue on their repo.

Thanks! :D

[mocolicious]

Update to 2.0 firmware and works like a charm MetaMask/metamask-extension#10240 (comment)

[gorgos]

Still waiting for the support to show all data on the device. That's the whole point of the display not to sign random things.

[pscott]

Still waiting for the support to show all data on the device. That's the whole point of the display not to sign random things.

This is on our radar. Unfortunately we have higher priority issues :)

In the meantime, feel free to check out the already existing EIP712 interal plugin and contribute! :)

[mazurroman]

Bump. This is a serious concern to us. Is there an ETA on resolving this issue?

[ghost]

Unfortunately no, still WIP on our side, we can't give any ETA yet, just hoping for somewhere in Q1 2022 at the moment.

[Kalesberg]

Any updates on this issue?