Add ledger hardware wallet support

[danfinlay]

Would be cool to support this hardware wallet:
https://www.ledgerwallet.com

Currently blocked by #3249

[danfinlay]

Most of the JS integration has already been written by the MyEtherWallet team here!

[izqui]

This would be really cool indeed! Happy to help make it possible

Now that this milestone is done https://github.com/MetaMask/metamask-plugin/milestone/14?closed=1, would it be possible to implement Ledger support?

[danfinlay]

Hi @izqui, thanks for the interest!

Our architecture will now support it, but we still haven't settled on what the UI/UX is going to be. If you know the constraints of the Ledger well, maybe you could help propose an account adding / signing flow.

One proposal:

1. We're planning to add an "Import Account" menu item to the top-right menu.
2. That menu will allow selection of different types of accounts to support.
3. One of those account types could be Ledger.

What does that UI look like? What code can we use under the hood?

The other big question for Ledger support:

What does transaction signing look like? I generally like the idea that reviewing a transaction can continue to have a common interface, and as we improve that, it improves all signing strategies, but when the user clicks "Accept", some types of accounts (like Ledger) will need to show another view, and instruct the user how to complete the interaction. What does that look like?

[kumavis]

@izqui also - how does a ledger wallet integration work? do you start a webserver stored on the USB device? Haven't looked into it yet but certainly is possible.

[danfinlay]

Last time I asked, @kumavis, I was told to basically copy MEW's code, but it didn't look quite as neatly abstracted as I'd hoped. I'm hoping there'd be a Ledger.js lib that we could drop in for getting listed accounts & requesting signatures.

[izqui]

@FlySwatter My Ledger Nano S is arriving on Friday. Will think about what a good flow could be then.

Regarding signing transactions, i think showing the Metamask confirmation screen but without buttons, indicating to perform the confirmation using Ledger's hardware could be an option.

[izqui]

I have been doing some research and it seems that the Ledger only allows to sign transactions and not arbitrary data.

AFAIK, this would make deploying contracts or signing proofs impossible.

Also, regarding My Ether Wallet implementation:

• MEW doesn't allow using Ledger for signing messages (https://www.myetherwallet.com/#sign-message)
• MEW offers the option to deploy a contract using the Ledger, but I cannot get it to work (https://www.myetherwallet.com/#contracts)

I have talked to Ledger support to see if they can give me an status update on this.

[danfinlay]

Good initial research.

Since Ledger has apps, I wonder if we could convince them to add message signing support.

In addition to removing buttons on the approval screen, I'd want a message, and then there is still the question of what the UI to pair should look like.

[izqui]

Quick update: I emailed Ledger support regarding arbitrary data signing, and referenced me to their public trello (https://trello.com/b/5nQ1mdzt/ledger-roadmap) where there is no reference of this feature/bug.

I was invited to join their Slack to continue the conversation (EDIT: Was just told that "we're already looking at that for other integrations, should happen shortly" 🙌)

Regarding confirmation screen, this is what they have in their Ethereum Wallet implementation (MEW doesn't show any UI related to the ledger)

The 'pair' should offer a list of all Ethereum addresses derived from the Ledger seed (MEW solves this well) and when the user selects one, save that address and the fact that is a Ledger address (no private key) that requires Ledger interaction to work with.

[recmo]

@LogvinovLeon and I implemented a LedgerWalletProvider that connects to the Ledger wallet over U2F. This should be an easier starting point than the MyEtherWallet code.

[recmo]

The Ledger wallets are based on a HD wallets and use derivation paths. This is a bit of a mismatch with Ethereuem/web3 that is based on a flat list of accounts.

The Ledger supports (as far as I have tested) any derivation path of the form m/44'/60'/*where * can be empty. (61 for ETC)

Their own Ethereum Wallet currently only uses the address available on m/44'/60'/0'/0. From their slack channel future versions will support multiple addresses, of the form m/44'/60'/${account}'/${address}. But this is not final and will be given further thought before implementation.

MyEtherWallet uses addresses available on m/44'/60'/0'/${address}.

For the LedgerWalletProvider we currently only implement m/44'/60'/0'/0 (our goal is to be compatible rather than flexible).

[danfinlay]

@recmo That is fantastic stuff! The linked code is a perfect starting point.

I'll need to either fork it to break out the core functionality or wrap it strangely though, since we don't use signing subproviders directly.

Maybe once I break that out into a small module, we could share that piece to share improvements and fixes.

Anyways, thanks a lot, I hope to work off this soon!

[recmo]

Thanks! Feel free to fork it, we are very open to pull-requests. It implements the Web3-providers API, but has no dependencies on it. You should be able to use it on it's own just as easily:

const Ledger = require('ledger-wallet-provider')
ledger = new Ledger()
ledger.getAccounts(console.log)
ledger.signTransaction(...

The happy-paths are working, we will soon be adding more error handling and feedback (ie. "Browser unsupported").

[danfinlay]

Oh that surface is actually closer to what I wanted than I thought! Very cool!

[chafey]

+1, would love to see this feature done. I have a ledger nano s and ledger blue so can help test, maybe code if given some guidance

[linagee]

Just looking at ledger-wallet-provider mentioned above, nice! https://github.com/Neufund/ledger-wallet-provider

[danfinlay]

Noting another nice looking ledger client available: https://github.com/LedgerHQ/ledger-node-js-api

[MicahZoltu]

EDIT @FlySwatter helped me find out that MetaMask is currently following BIP 44 spec. As long as you all keep that up then I'm happy with your implementation. :) Leaving this here for future readers that end up in this space, but you can mostly ignore this unless there is a plan to move away from BIP 44.

Cross posting my comment here. Please don't do what MEW and the Ledger app did (rumors suggest it was an accident that made it to release and now change is hard) and use a m/44'/* path without actually following the rest of the (BIP 44 spec). When you all get around to implementing this, please follow the spec and either create a new purpose (first segment) or use purpose 44' and follow the spec associated with that : m / purpose' / coin_type' / account' / change / address_index

Alternatively, do something totally different with regards to the path, just don't have it start with 44'. :)

Soapbox aside, what are your plans for this? As a dApp engineer working on Ledger integration, compatibility with MetaMask is important to us and we would like to make sure we are on the same page with the various wallet/integration providers.

You can see what other wallets are doing over in that thread (I believe you all already have some representation there) but it is more even more important to follow the standard when integrating with a HW wallet because users will expect the hardware wallet to show the same accounts no matter which tool they use to interact with the wallet. It is unfortunate that a bug in an early release of the Ledger app has resulted in this much community disconnect, but I think getting back on track with current standards until there is an official Ethereum one is critical for good UX for all of our users.

[GramDev1]

Do you have an ETA on this? It would really enhance the functionality of the ledger to use smart contracts.

[WestonReed]

I think this would be an excellent feature, and add a lot of security and usability to MetaMask. +1

[danfinlay]

For the people here asking for an update: At MetaMask, we also want this feature incredibly badly, and are currently doing some UX preparatory work to pave the way for a good integration.

Things have been a little slow to progress lately, we've had to put more time on fixes and support than usual, which has been a healthy expression of a still-exponential growth rate.

That said, after a few more core tuneups around how we manage nonces, and maybe a little token support, this is one of my top goals for MetaMask.

I think you can probably expect this by the end of the summer.

[jamespic]

The WebUSB support in extensions doesn't actually look usable. It's got the APIs, but always fails to find any devices, whereas the Ledger shows up when I call them from a regular browser tab.

[gre]

@jamespic you can try this snippet in the devtool console, it should work if you have a ledger device connected & unlocked

const ledgerDevices = [
 { vendorId: 0x2581, productId: 0x3b7c },
 { vendorId: 0x2c97 }
]
await navigator.usb.requestDevice({ filters: ledgerDevices })
devices = await navigator.usb.getDevices()
console.log(devices)
devices[0].open()

NB: however you won't be able to communicate yet with the device as we don't expose an interface from webusb yet but we're experimenting on it these days. You can see latest experiment here but it does not work with the current Eth ledger app, hopefully if we can figure it out, we will add support to it soon :)

There are some interesting UI questions about this requestDevice() call. it needs to be called at least once to being able to see the device in getDevices. however, requestDevice() seems to remember a device was accepted previously, so i'm not sure what should be done in term of UI. probably can try to getDevices and if not found, do a requestDevice (but what if multiple devices are connected?:D)

[jamespic]

@gre Trying a snippet much like that one was what persuaded me that WebUSB doesn't work from extensions, but maybe there's something I've missed, so I'll look again

[gre]

@jamespic oh ok. maybe WebUSB in context of an extensions is different. API is here but not seeing the device?

and does requestDevice popup open with the permission UI ? on a normal webpage:

[jamespic]

@gre Yes, if I do that on a normal page, I get the same thing you do. However if I open up a developer console for one of the MetaMask pages (the background page is where all the signing happens, but any page will do), requestDevice throws an exception without displaying the device selection screen.

[ghost]

There is currently no clean way of determining an appropriate origin of an extension for the purposes of populating clientData in the U2F protocol

@jamespic , looking quickly at all this, it seems like solving the above (finding a clean way, so the "wontfix" is revised) would solve this within chrome, and thus the interfacing problem?

Does it work with e.g firefox?

[jamespic]

@lazaridiscom It's conceivable that Chromium devs would be satisfied with some sort of solution where extensions can effectively act like any origin, since they already do this with the fetch API in extensions. Although even a simple fix would be a code change to Chrome.

I'll check what Firefox does with U2F and WebUSB requests from extensions when I get chance.

[jamespic]

Firefox doesn't seem to have working U2F in extensions either - the window.u2f object is there, unlike Chrome, but requests always fail, possibly due to the domain being ill-defined, as the Chromium devs alluded to.

I can't seem to get WebUSB working in Firefox at all, so that doesn't look promising either.

[ghost]

@jamespic , Sounds like bad news from the "interfacing side". News from the "project interfacing side" are not good either. But ok, a bit increased difficulty should not scare us. I should receive my Ledger tomorrow and will start working on this (starting May). I'm really relieved that you've done all this detail work, so that I'm not starting from scratch here!

@danfinlay , after overseeing the situation: you should try to allocate 5 to 10 ETH to place on some "heavy bounties". Don't ask for details or clarifications, i don't have them yet. I just sense that this will be needed.

[ghost]

Ledger arrived, follow-up: #4060

[ghost]

@gre, is there any way to set the ledger in a kind of development-mode, thus one does not have to unlock it again and again?

[ghost]

@gre, @jamespic - can you please verify the new solution paths mentioned within #4204 (extension-to-extension communication, extensions-messaging)?

@danfinlay the "heavy bounties" mentioned above would go exclusively to the #4205 issue. A "high-profile" bounty (I guess minimum 5 ETH, if not 10) could generate enough visibility and interest, thus a chrome dev is attracted to a) find a solution and b) ensures that it is finally merged (which can be the most difficult thing).

[danfinlay]

I'll look into a heavy bounty for that task, in hopes of getting a Chrome dev's interest.

[ghost]

Bounty for #4205 makes now even more sense (WebUSB path, chrome devs).

[ghost]

@gre, is there any way to set the ledger in a kind of development-mode, thus one does not have to unlock it again and again?

Answering myself: its a standard-option in the ledger-device menu.

[ghost]

superseded by #4060

(similar/related: #4625, Trezor)

( @danfinlay , not 100% sure here, but this should be possibly closed, too)

[ghost]

@ all

you can help by starring a relevant issue:

https://bugs.chromium.org/p/chromium/issues/detail?id=770896

[matkam]

Now that Firefox has U2F support in Firefox 67, can you enable Ledger support?

[matkam]

Made a pull request to enable Leger wallets in Firefox #6659