Metamask-extension: Add KeepKey hardware wallet support

[danfinlay]

A new contender in the ethereum hardware wallet space:

http://ethnews.com/keepkey-hard-wallet-supports-ethereum

Blocked by #328

[SCBuergel]

328 is closed and references https://github.com/MetaMask/metamask-plugin/milestone/14 which is also closed. Does that mean this issue is basically ready to progress?

[danfinlay]

This is now blocked by UI work that we're doing, for how we're going to fit extra account types into MetaMask.

[keepkeyjon]

Is this still blocked?

We've published https://github.com/keepkey/keepkey.js, which should make it reasonably straightforward. I'm also happy to provide a couple of development devices to help with integration / testing, if you're interested.

[danfinlay]

Feel free to email me dan [at] metamask.io to continue the discussion. Some development devices would help, but you could also make some headway by following the lead of some of our other hardware wallet integrations.

https://github.com/MetaMask/metamask-extension/pulls?q=is%3Apr+Trezor+is%3Aclosed
https://github.com/MetaMask/metamask-extension/pulls?utf8=%E2%9C%93&q=is%3Apr+Ledger+is%3Aclosed+

Right now this is a bit lower priority than our mobile client, so it might be another quarter or two before we have the bandwidth to fully pursue this, but you could help us line up the effort.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__This issue now has a funding of 0.3 ETH (62.2 USD @ $207.33/ETH) attached to it.__

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
  
  
  
  • Want to chip in? Add your own contribution here.
  
  
  • Questions? Checkout Gitcoin Help or the Gitcoin Slack
  
  
  • $95,179.15 more funded OSS Work available on the Gitcoin Issue Explorer
  
  

[frankchen07]

@abrahamh08 - frank from gitcoin here - any progress on this task, or should we release it to the general public?

[keepkeyjon]

"Fun" thing I learned over the weekend: it turns out that the chrome.hid and chrome.usb APIs are only available to Chrome Apps, but not Chrome Extensions. That means you can't talk to a KeepKey from the background page in Metamask, since you need access to window.navigator.usb to make it work.

[keepkeyjon]

https://bugs.chromium.org/p/chromium/issues/detail?id=352257#c7

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__Work has been started__.

These users each claimed they can complete the work by 4 months, 2 weeks from now.
Please review their action plans below:

1) deepsi43 has started work.

Hi,
I am interested in working on this project .

Learn more on the Gitcoin Issue Details page.

[deepsi43]

Is this issue still open ??

[danfinlay]

It appears so!

[deepsi43]

I have been looking at the trezor integration but couldnt find any trace of keepkey modules added !!
Did check with the keepkey installation process and modules.

[keepkeyjon]

Any KeepKey integration with Metamask will be a wildly different beast than the Trezor integration, since we don't integrate with TrezorConnect. Don't expect it to be a simple copy-paste level of effort.

[Abrahamh08]

@frankchen Sorry I didn't see you mention me earlier, but no I decided way too late that I didn't have time to work on it, sorry to put a hold on it

[roncoejr]

Good to see that there is an open feature request on this one. Understand that the mobile client is taking priority. Looking forward to seeing Keepkey support in MetaMask sometime in September.

All the best to the team.

[step21]

It appears this is still blocked by chrome not allowing use of usb by extensions, right?

[keepkeyjon]

I believe extensions have access to window.navigator.usb, but not chrome.usb, which means device communication has to happen from the popup, and not the background page.

[revonoc]

Any updates? Would love this integration but sadly don’t have the skills to help out

[frankchen07]

@deepsi43, frank from gitcoin here, it looks like you've started work. Do you have updated results? if not, I'll release this bounty back to the public.

[deepsi43]

Will get the update by the today evening@Frank

[revonoc]

Good to hear this is still being worked on!

[revonoc]

Bumping this again to see if it's still open - would love the Keepkey support for sure, but not sure how hard it is to integrate!

[danfinlay]

I have a feeling if you wrote a module like eth-trezor-keyring for KeepKey, you could model it into a plugin for MetaMask using our newly released plugin branch beta.

You'd need to use this branch until it's merged (to add custom account type), but this could be the model for future hardware wallet integrations on MetaMask to be more easily contributed.

[deepsi43]

I have been looking into the same aspect and have been working on it

Sent from my iPhone

On Nov 10, 2019, at 12:24 PM, Dan Finlay [e-mail] wrote:

I have a feeling if you wrote a module like eth-trezor-keyring for KeepKey, you could model it into a plugin for MetaMask using our newly released plugin branch beta.

You'd need to use this branch until it's merged, but this could be the model for future hardware wallet integrations on MetaMask to be more easily contributed.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

[keepkeyjon]

FYI, we've replaced keepkey.js with the more general https://github.com/shapeshift/hdwallet

[deepsi43]

Yes I did have seen it.just wondering which would be a better approach like using keepkeyjs directly or to work based on the new hdwallet shapeshitftoss webusbadapter

Sent from my iPhone

On Nov 13, 2019, at 11:33 PM, keepkeyjon [e-mail] wrote:

FYI, we've replaced keepkey.js with the more general https://github.com/shapeshift/hdwallet

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

[keepkeyjon]

keepkey.js is a dead project. I wouldn't recommend building new things on it.

[keepkeyjon]

I highly recommend building this on top of @shapeshiftoss/hdwallet-keepkey-webusb and the other related packages.

[revonoc]

@deepsi43 so glad you're working on it! Thanks man

[deepsi43]

Seems like I cannot continue to work on this module anymore!!!...

Sent from my iPhone

On Nov 17, 2019, at 12:45 AM, revonoc [e-mail] wrote:

@deepsi43 so glad you're working on it! Thanks man

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

[keepkeyjon]

You'll need some small MetaMask UI tweaks, since PIN/Passphrase on Trezor are handled through TrezorConnect UI, which is not provided by @shapeshiftoss/hdwallet-keepkey nor keepkey.js.

[walidmujahid]

If webusb is the way to go, would it be more worthwhile to re-implement things using packages from shapeshift/hdwallet?

[keepkeyjon]

@walidmujahid yes

[walidmujahid]

I thought so. I am not very comfortable with the quick dev solution of making a "fake" keyring and hardcoding the pin.

Perhaps more discussion can illuminate on different but still actionable solution paths.

I think it is worth discussing whether avoiding design changes would be counterproductive in both the short term and long term, or just the long term.

It might be worth discussing whether shifting to building on shapeshift/hadwallet packages could lead to value that could be added to design discussions at #731 and help in part in achieving the idea of multiple keychains.

[keepkeyjon]

Using @shapeshiftoss/hdwallet-keepkey{-webusb} for this is absolutely the correct decision... there is no better library for communicating with a keepkey. However, moving metamask's ledger or trezor support over to hdwallet-{ledger,trezor} is probably not the way to go through, since metamask already has support for those, and that would tightly couple metamask to the design decisions we made in hdwallet, which is probably not helpful for this project.

This cannot be do as side-projects based on $50 bounties

Yeah, not sure what Virgil was aiming for there. It's pretty clear to me that this is not a quick/easy project, which is why I haven't taken it on as a hobby-thing over some weekend.

[walidmujahid]

I assume that the @shapeshiftoss/hdwallet-keepkey-webusb has the potential to build cross browser support unlike the hdwallet-keepkey-chromeusb package.

I personally am convinced by @keepkeyjon about going with using shapeshift hadwallet-keepkey-{*} packages. That and I had read through all their code, so I feel a bit biased towards them.

Regarding design needs, I assume the main and perhaps only thing would be in regards to handling the pin matrix. So, on UI/UX side, perhaps something similar to how shapeshift.com handles it?

Something for the design team?

[keepkeyjon]

hdwallet-keepkey-chromeusb is meant for use in a chrome app. hdwallet-keepkey-webusb is meant for browsers (requires webusb support, which currently only exists in chrome/brave).

[keepkeyjon]

perhaps only thing would be in regards to handling the pin matrix

Also bip39 passphrases, the input of which gets triggered by a similar event.

[walidmujahid]

@lazaridiscom I just noticed this comment in my email but it seems to have
been deleted from the thread -and now that I check, your other comments to
the thread as well. It would have been nice to have seen your comments on
technical debt and your link to #7601 when I had checked early this morning.

Also, with your explanation, I understand better why a "fake" keyring route
was proposed. I was never against it, it just seemed a bit uncomfortable taking the route.

On Fri, 13 Dec 2019 at 00:17, Lazaridis [e-mail] wrote:

If webusb is the way to go, would it be more worthwhile to re-implement
things using packages from shapeshift/hdwallet?
@walidmujahid https://github.com/walidmujahid yes

@walidmujahid https://github.com/walidmujahid , @keepkeyjon
https://github.com/keepkeyjon , you are both wrong.

"Getting things done" on a code-base full off technical debt and
in-development (metamask) needs some "higher grade abstract processing",
which usually cannot be discussed much, as you cannot teach decades-xp and
ability to discussion partners.

This task here is "peanuts", the "fake" implementation becomes quickly a
"real" one. Doing "the right implementation" on a code-base like MetaMask,
is not always possible. You need to adapt, and many times, "doing a bad
implementation, compatible to existent code" is the logical way to go.

As for issue like #731
https://github.com/MetaMask/metamask-extension/issues/731 and the likes
(there are many of them), this need a bottom-up redesign of MM, to
eliminate technical-debt and create a high-quality code-base, well, this
cannot be do as side-projects based on $50 bounties (which are not even
worth the metal overhead they introduce).

This redesign was attempted during #4060
https://github.com/MetaMask/metamask-extension/issues/4060, and is
again attempted here #7601
https://github.com/MetaMask/metamask-extension/issues/7601 (with a
different starting point).

In fact "achieving the idea of multiple keychains." would not even exist
for me. It would be a "natural by-product" of an elegant implementation (or
a "higher-grade-redesign").

Enough talk, me out here

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MetaMask/metamask-extension/issues/889?email_source=notifications&email_token=AALLSSHIPC2JWVIUR5KZWXTQYMLFLA5CNFSM4CXMQNW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGY5VAI#issuecomment-565303937,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AALLSSD5DEU5UR6XCWCOIPTQYMLFLANCNFSM4CXMQNWQ
.

[keepkeyjon]

off-topic meta-discussion

Enough talk, me out here

I delete my messages usually myself after a weak, sometimes faster. Different reasons, here it is mainly that it hurts my brain to discuss stuff which is "obvious grade" for me.

Github leaves no trace if the original author deletes messages (only if the repo-owner deletes them).

So, I deleted my messages myself, so there is no need for you to reproduce them.

( And now I really really out )

@lazaridiscom IMO that revisionist attitude is detrimental to open source projects such as this one. It may be "obvious grade" to you, but not to others... and deleting messages hurts those who come to look at these threads after the fact.

[walidmujahid]

The "fake" eth-trezor-keyring could provide this initially by mock-functionality (even with a hardcoded PIN/Passphrase).

Since reading the previous explanation by @lazaridiscom, I have been thinking a bit harder on the "fake" keyring and mock-functionality solution. However, it is the second part, mock-functionality, that I remain uncertain about.

Was there a method in mind when the idea was proposed as to how to provide that mock-functionality when comes to the Keepkey pin/passphrase?

I am not sure how a "hardcoded" solution would be implemented and have people's Keepkeys work with MM. Though, I feel perhaps that "hardcoded" comment was a solution available to hardware such as Trezor or Ledger? I have neither, but I do have a Keepkey.

I still feel the other solution path, if you wish to call it so, of utilising @shapeshiftoss/hdwallet-keepkey{-webusb} packages and not avoiding initial UI changes seems the most straightforward -even if it may not the best- when it comes to making things work within a reasonable amount of time.

Another question, perhaps for @keepkeyjon: Would I be correct to assume that hdwallet packages could be used within an implementation of MM's Keyring interface and still be able to use hdwallet's webusb functionality? Or does MM have to use the hdwallet's keyring in order to use @shapeshiftoss/hdwallet-keepkey-webusb? Would MM would have to make our own version of hdwallet-keepkey-webusb and any relevant packages?

On Mon, 9 Dec, 22:57, Lazaridis [e-mail] wrote:

See #4060 https://github.com/MetaMask/metamask-extension/issues/4060 for
more relate info re ledger-implementation (though outdated, as today,
webusb would be the way to go).

My estimation for KeepKey is 3 to 6 weeks for production-grade.
On Mon, 9 Dec, 12:00, Lazaridis [e-mail] wrote:

The "fake" eth-trezor-keyring could provide this initially by
mock-functionality (even with a hardcoded PIN/Passphrase).
On Mon, 9 Dec, 11:51, keepkeyjon [e-mail] wrote:

You'll need some small MetaMask UI tweaks, since PIN/Passphrase on
Trezor are handled through TrezorConnect UI, which is not provided by
@shapeshiftoss/hdwallet-keepkey nor keepkey.js.

On Sun, 8 Dec, 08:36, Lazaridis [e-mail] wrote:

if this keepkey hw-wallet is similar (api/functionality) to
trezor/ledger, then the easiest (dev) way would be to produce a 100%
compatible eth-trezor-keyring (or ledger) as a drop-in replacement. Thus no
need to change the UI of MM initially.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/MetaMask/metamask-extension/issues/889?email_source=notifications&email_token=AALLSSC5VBODCHEBKUFUWQTQXTZ6ZA5CNFSM4CXMQNW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGG6ZCI#issuecomment-562949257,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AALLSSHK5BMQILDHPKPARDTQXTZ6ZANCNFSM4CXMQNWQ
.

[keepkeyjon]

Was there a method in mind when the idea was proposed as to how to provide
that mock-functionality when comes to the Keepkey pin/passphrase?

Mocking passphrase input is possible, but doing so for the PIN is not (because of the cipher pad).

I don't think it makes sense to cut corners the way lazaridiscom was suggesting, though you could use the command-line tools for KeepKey [1] to turn off the PIN for testing.

Another question, perhaps for @keepkeyjon:
Would I be correct to assume that hdwallet packages could be used within an
implementation of MM's Keyring interface and still use hdwallet's webusb
functionality? Or does MM have to use the hdwallet's keyring in order to
use @shapeshiftoss/hdwallet-keepkey-webusb?

You'll need to use both. To make MM happy, it needs a package that exposes the same interface as eth-trezor-keyring, and that should be implemented on top of hdwallet-keepkey-webbusb + hdwallet-core + hdwallet-keepkey through hdwallet's keyring.

In hdwallet, the keyring is a global singleton for managing all connected wallets (that hdwallet knows about). If you do not register other transports with it, it won't pay attention to the other kinds of devices that hdwallet supports... which is great for this use case, since in MM we don't want hdwallet's keyring to try to manage Trezors/Ledgers, but we do want that where we use it in the ShapeShift platform.

Would MM would have to make our own version of hdwallet-keepkey-webusb and any relevant packages?

No, don't duplicate the functionality of that... just import it and use it.... that's what it's there for.

1: www.github.com/solipsis/go-keepkey, www.github.com/keepkey/python-keepkey

[walidmujahid]

@keepkeyjon Thank you very much for the clarification and links.

On Wed, 18 Dec 2019 at 17:16, keepkeyjon [e-mail] wrote:

Was there a method in mind when the idea was proposed as to how to provide
that mock-functionality when comes to the Keepkey pin/passphrase?

Mocking passphrase input is possible, but doing so for the PIN is not
(because of the cipher pad).

I don't think it makes sense to cut corners the way lazaridiscom was
suggesting, though you could use the command-line tools for KeepKey [1] to
turn off the PIN for testing.

Another question, perhaps for @keepkeyjon https://github.com/keepkeyjon:
Would I be correct to assume that hdwallet packages could be used within an
implementation of MM's Keyring interface and still use hdwallet's webusb
functionality? Or does MM have to use the hdwallet's keyring in order to
use @shapeshiftoss/hdwallet-keepkey-webusb?

You'll need to use both. To make MM happy, it needs a package that exposes
the same interface as eth-trezor-keyring, and that should be implemented on
top of hdwallet-keepkey-webbusb + hdwallet-core + hdwallet-keepkey through
hdwallet's keyring.

In hdwallet, the keyring is a global singleton for managing all connected
wallets (that hdwallet knows about). If you do not register other
transports with it, it won't pay attention to the other kinds of devices
that hdwallet supports... which is great for this use case, since in MM we
don't want hdwallet's keyring to try to manage Trezors/Ledgers, but we do
want that where we use it in the ShapeShift platform.

Would MM would have to make our own version of hdwallet-keepkey-webusb and
any relevant packages?

No, don't duplicate the functionality of that... just import it and use
it.... that's what it's there for.

1: www.github.com/solipsis/go-keepkey,
www.github.com/keepkey/python-keepkey

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MetaMask/metamask-extension/issues/889?email_source=notifications&email_token=AALLSSD4F3EPU4JRQOYSGS3QZKONJA5CNFSM4CXMQNW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHVV7Y#issuecomment-567237375,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AALLSSANY6L7FGMJSCWBSALQZKONJANCNFSM4CXMQNWQ
.