Parity-ethereum: Hardware wallets don't support signing arbitrary messages: "Unknown binary data"

Created on 18 Jul 2017  ยท  13Comments  ยท  Source: openethereum/parity-ethereum

I'm using Parity/1.6.8/1.17.0/ma
I'm using the latest Parity Chrome extension
I'm using a Nano Ledger S with Ethereum app v1.0.8
I'm browsing the Dapp https://etherdelta.github.io

If I try to place a buy or a sell order on a pair, I get the following error and I cannot confirm request (it doesn't even come up on the nano like it should do)
screen shot 2017-07-18 at 18 43 21

When I go to my parity UI, I can see in the signer this request and when I try to confirm it from there, it throws the error "[-32021] Account password is invalid or account does not exist." while the expected behaviour is to prompt on my hardware wallet to confirm transaction.
screen shot 2017-07-18 at 18 52 12

To be noted that I can fill existing buy or sell orders, but cannot place them. It works like a charm with the default parity wallet without using a hardware wallet.

F3-annoyance ๐Ÿ’ฉ M7-signer ๐Ÿ” P7-nicetohave ๐Ÿ•

All 13 comments

Just to make sure, is contract support enabled on your ledger?

Does your ledger show up in accounts list as colored identicon or gray?

Yes contract support is enabled. Ledger shows up as a colored icon.
Food for thoughts: I believe Etherdelta handles placing a buy/sell order differently than actually buying or selling into an existing order. It seems like it's not doing a transaction but rather authorizing Etherdelta to make this transaction later. @frenchhoudini @zackcoburn could help understand this.

Correct. Placing an order on EtherDelta involves signing a message, whereas
trading against an existing order involves sending a transaction.

On Wed, Jul 19, 2017 at 6:17 AM Romain Barissat notifications@github.com
wrote:

Yes contract support is enabled. Ledger shows up as a colored icon.

Food for thoughts: I believe Etherdelta handles placing a buy/sell order
differently than actually buying or selling into an existing order. It
seems like it's not doing a transaction but rather authorizing Etherdelta
to make this transaction later. @frenchhoudini
https://github.com/frenchhoudini @zackcoburn
https://github.com/zackcoburn could help understand this.

โ€”
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/paritytech/parity/issues/6083#issuecomment-316339884,
or mute the thread
https://github.com/notifications/unsubscribe-auth/APEqPWK7q0Ey8fGMkJFh7l-FPh-rCkOlks5sPdeegaJpZM4ObonT
.

I also have the same problem using Parity 1.7.0 and EtherDelta.
If anyone has any pointers to where the problem might lie I'll be glad to help out.

Looks like hardware support for signatures is missing, somewhere over here:
https://github.com/paritytech/parity/blob/604ea5d684b05f63ab2a32f68fdf50b8bbe38498/rpc/src/v1/helpers/dispatch.rs#L516

I'll have a look.

Since the hardware wallets don't support signing arbitrary messages yet it's not possible to implement that.
Please re-open if the hardware supports this.

It should support personal signing (see https://github.com/LedgerHQ/ledger-node-js-api/blob/master/src/ledger-eth.js#L113) - is this what you're looking for ?

@btchip Interesting, this could use some documentation at https://raw.githubusercontent.com/LedgerHQ/blue-app-eth/master/doc/ethapp.asc

What is the minimum required eth app version that supports this? Is the message presented on the device display hex-encoded?

that's right, I'll update the documentation. This message has been added into version 1.0.8 - the device displays a sha256 hash of the message sent to be signed (to handle the case where a long binary message is sent)

This is not limited to hardware wallets, isn't it?

normal

This is crucial to support using EtherDelta with Ledger Nano through Parity (I don't think there is any other alternative right now to use ED with a hardware wallet)!

๐Ÿ‘ @arkpar

@tomusdrw This isn't implemented on the client side yet right? I don't believe we necessarily want to do any UI changes, but we should probably have this ability on node-side. If it's not done then maybe @niklasad1 can pick it up?

Update, I got it working but it will take some time to get it PR ready!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

0x7CFE picture 0x7CFE  ยท  3Comments

barakman picture barakman  ยท  3Comments

stone212 picture stone212  ยท  3Comments

jacogr picture jacogr  ยท  4Comments

dukei picture dukei  ยท  3Comments