Initial trezor support

[brunobar79]

Fixes #605, #2638

TODO:

• eth-trezor-keyring
  • eth-trezor-keyring tests with full coverage
  • move eth-trezor-keyring inside metamask org and publish as npm package
• Keyring Controller
  • Keyring Controller PR must be merged
  • Release new version via npm
• UI
  • Hardcoded strings needs to be moved to localization files
  • Add message for unsupported browsers
  • Refactor UI code
• Improvements
  • Ability to forget imported accounts (Allow removing loose accounts #2638)
  • Show account list by default if already unlocked
  • Abllity to "forget" your TREZOR device
  • Improve remove account modal based on Allow removing loose accounts #2638
  • Add tooltip to the X on the account list dropdown (also from Allow removing loose accounts #2638)
  • Added Global alert (Used with "hardware wallet connected")
• Bugs
  • Account balance is not working correctly in the TREZOR account list
• Compatibility
  • Define supported browsers and update code
• Unit Tests
• Latest design: New Trezor Design Design#35
• e2e tests

DEMO: https://youtu.be/AEPySP2ThaM

[ghost]

@brunobar79 , does the interfacing work?

e.g.: https://github.com/brunobar79/eth-trezor-keyring/blob/master/trezor-connect.js, accessed from within an extension, works fine?

[brunobar79]

@lazaridiscom yeah it works. They open a popup pointing to connect.trezor.io.
We are not dealing with any low level USB stuff. Check this out: https://github.com/trezor/connect

[brunobar79]

Went through most of the UX / UI stuff with @cjeria.

• We'll keep the "Connect" tab as it is
• Be able to forget / disconnect would be nice to have (probably a must)
• We will keep the tab for all platforms and for browsers where it's disable we will add a message explaining the current situation.
• The connect feature will force full screen mode in order to avoid the popup focus issue. This is not required for signing transactions when the extension is running in an external popup.

Christian will create some design specific tickets improve the experience
In the meantime I'll be writing some tests for the eth-trezor-keyring lib and I'll address some of the previously mentioned UX decision that are not blocked by design.

[ghost]

@brunobar79 , good to know. I've added the "Trezor Connect Way" as a solution path for Ledger (although my personal preference to access hw-wallets is still the (in-)direct USB access).

One sidenote: your eth-trezor-keyring contains GPL code, this possibly means that the whole module (if not whole MetaMask) must go form MIT to GPL (not sure though). You should verify this, and possibly ask Trezor to release the relevant code additionally under the MIT license.

Edit:

Your file uses GPLv3
https://github.com/brunobar79/eth-trezor-keyring/blob/master/trezor-connect.js#L7

Actual code seems to be under LGLPv3 (which should be fine for MIT-mix)
https://github.com/trezor/connect/blob/v4/COPYING

[cjeria]

Here's the design issue for UI/UX fixes MetaMask/Design#33

[brunobar79]

@lazaridiscom Thanks for the info! As you pointed the license situation is a bit weird...
The entire repo is LGPLv3 but the file I took from there (https://github.com/trezor/connect/blob/v4/connect.js#L4) is still GPLv3 I'll ping Trezor to get some clarification on that.

Btw, the reason I copy pasted this file is because the distributed npm package doesn't match the code on the repo at all.

And yeah, I agree that direct (in-)direct USB access is the desired approach and also eliminates de dependency on the trezor website (ex. if they are down, you can't use your wallet) but after looking at the existing ledger discussion and blockers I decided to not go that route for now.

With that said, once this is done, migrating to a more "low-level" approach will require changing the eth-trezor-keyring and probably nothing else on the extension code.

[ghost]

@brunobar79 , to clarify: your solution-path has a major benefit: it works, now, on code-level. So you went definitely the right way.

With that said, once this is done, migrating to a more "low-level" approach will require changing the eth-trezor-keyring and probably nothing else on the extension code.

Sounds promising!

Possibly you should keep additionally those options in mind:
(EDIT: those are 2 different solutions)

• MetaMask depends anyway on external services (Infura), so it could self-host the relevant code.
• your keyring-module could spawn a mini-server (Important: I've no idea if this is doable/allowed), which hosts the trezor-connect-code (stripped down to the minimum needed for MetaMask).

[metamaskbot]

Builds ready [e5512c3]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [de4265c]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [e89350b]: mascara, chrome, firefox, edge, opera

[ghost]

@brunobar79

There's no need to buy one!

If finally ordered a Trezor, need to see a real HW-Wallet working with MetaMask, thus I get my closure.

Do you know when your code goes into production?

[metamaskbot]

Builds ready [cb97517]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [cb53d51]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [cbb14f1]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [514148f]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [df19163]: mascara, chrome, firefox, edge, opera