5afe / safe-react

Deprecated! New repo – https://github.com/safe-global/web-core
MIT License
332 stars 363 forks source link

Error: Ledger: Only version 4 of typed data signing is supported #2531

Closed PaulRBerg closed 3 years ago

PaulRBerg commented 3 years ago

Description

I used to be able to interact with the third-party apps listed in the "Apps" section using my Ledger physical wallet. Not anymore. I am encountering the following errors.

When using Ledger via MetaMask on Firefox:

MetaMask - RPC Error: MetaMask Message Signature: Error: Ledger: Only version 4 of typed data signing is supported

When using Ledger directly:

SafeConnector: A message was received from origin https://gnosis-safe.io with NO message id provided.

Nothing happens in the UI, and no transaction is emitted from my account. It looks like the error is that Gnosis prompts users for signing EIP-712 typed data, but the integration is broken for Ledger wallets.

To ensure that I am not interacting with a particularly broken app, I used Sablier, Compound, as well as a local version of the Sablier app that I upgraded to the latest version of the safe-apps-sdk. Across all apps, I encountered the same errors.

Environment

I have tried using the apps with two different browsers, and two different wallet integrations from the modal (MetaMask and Ledger). I have tried using the Ledger Live integration (since Chrome v91 changed how U2F transport works), but that didn't work either.

Steps to reproduce

  1. Go to https://gnosis-safe.io
  2. Open the Compound app
  3. Try to make a deposit with a Ledger wallet

Screenshots

Capture d’écran 2021-07-13 à 12 44 15 Capture d’écran 2021-07-13 à 12 44 54
francovenica commented 3 years ago

I only have a ledger Nano S. I'm working on Win 10, in chrome, with direct ledger connection to the app (I cannot use the imported ledger acc into the MM extension since Win10 still has issues with that) I tried to use the compound app in the Prod Rinkeby env and it worked fine for me. https://rinkeby.gnosis-safe.io/app/#/safes/0x150Fd684E8506C1287a22aA494D7E1BABe578dB7/transactions image

We should see if any of us has a Nano X to give it a try

PaulRBerg commented 3 years ago

I'll try to test later with my Ledger Nano S to check if this applies to Nano X in particular. It would be surprising (at least to me) if it did.

What version does your Ledger Ethereum app have?

francovenica commented 3 years ago

What version does your Ledger Ethereum app have?

1.7.0, is the latest one.

PaulRBerg commented 3 years ago

Interesting. The latest version for Nano X is 1.8.7

francovenica commented 3 years ago

Just checked and I yes, for Nano S the latest version of the Eth app is 1.7.0 and the latest firmware is 1.6.1

image

Question, did you connect directly with the ledger into the gnosis app? image

tschubotz commented 3 years ago

I just tested with a Nano X on Firmware 1.3.0 and Ledger Etheruem app version 1.8.7.

tschubotz commented 3 years ago

@liliya-soroka you have a Ledger nano X too, right? Could you please try to reproduce this as well?

PaulRBerg commented 3 years ago

I confirm that now the direct Ledger connection worked. MetaMask still doesn't.

liliya-soroka commented 3 years ago

Tested using Nano X with Firmware 1.3.0 and Ledger Etheruem app version 1.8.7.,FF , MM (9.7.1)

  1. No issue with direct Ledger connection

  2. I have also got MetaMask - RPC Error: MetaMask Message Signature: Error: Ledger: Only version 4 of typed data signing is supported error in the console, but it doesn't block me in the transaction creation. Current result: Sign dialogue in MM appears twice. -When the "Sign" button is clicked the first time the error appears in the console ( no reaction on Ledger Nano) -Then the second sign dialogue appears immediately and it can be signed without any issue ( sign message appears on Ledger Nano X and can be sign without any issues) -The same behaviour with double signing dialogue in MM I see for send funds too

https://user-images.githubusercontent.com/338622/125779248-8f7356ac-0438-4121-810f-081bcccf7b70.mov

tschubotz commented 3 years ago

That's exactly what I'm observing as well!

I wonder what's different in your scenario @paulrberg 🤔

Could you share your Safe address with us for debugging? Or if you wouldn't want to share it for privacy reasons let us know:

Shouldn't make any difference, but I'm lacking other ideas at the moment, unfortunately.

mmv08 commented 3 years ago

I think the key error is in this screenshot Paul shared image

So either the ledger device is still locked when he tries to sign the second time or there's a problem with data (params.data error)

What OS do you use @paulrberg so we can emulate your environment as close as possible?

@liliya-soroka it also looked like you tested on Firefox and Paul used Brave, could you please check if it changes something? Brave is known for problems 🤷‍♂️

PaulRBerg commented 3 years ago

hey folks, I tried again, and I couldn't manage to reproduce the Ledger Device is busy error, nor the params.data must be an array one.

Now I'm observing the same behaviour as @liliya-soroka - that is, two pop-ups appear in MetaMask, the first emits a console error (which looks harmless), the seconds prompts for me a signature on the Ledger. This is with Firefox.

Case solved then, but I think that this issue should remain open to investigate the Only version 4 of typed data signing is supported error.

tschubotz commented 3 years ago

Glad it's resolved. Agree that we should check out the 2 metamask popups still and this version 4 typed signed data error.

Btw. thanks a lot for the detailed original error report, we appreciate this a lot!

tschubotz commented 3 years ago

I'd leave this as major since Ledger via Metamask is a quite populat setup and 2 Metamask popups are probably rather suspicious for the user, hence I think we should check this out in this or the next release cycle. cc @dasanra

mmv08 commented 3 years ago

@tschubotz This is quite a popular problem, the reason we have multiple pop-ups is that we don't know what wallets support what signing method and we just execute them subsequently until we find success.

They're executed in this order: EOA - Eip712_v3, Eip712_v4, Eip712, eth_sign if safe version is >= 1.1.0 Hardware wallet - eth_sign if safe version is >= 1.1.0. It works only if you connect hw wallet with onboard.js, otherwise, EOA flow is used

If you connect metamask via ledger, we cannot know it, metamask doesn't expose such information. What we can do to fix the issue is to try to move EIP712_v4 to the first place assuming it found more support among wallets.

mmv08 commented 3 years ago

We can also try to reach to the metamask team to see if they plan to fix it or would accept a contribution

dasanra commented 3 years ago

There are no further actions that we can do. When a hardware wallet is connected through Metamask we just can send different signing methods, which cause opening several Metamask windows until we select one that is compatible with the hardware wallet. Any other failure in behavior should be fixed on Metamask side

oed commented 2 years ago

Is there an issue tracking this on the Metamask repo?