Hardware wallets support

[tonymorony]

It would be a huge booster to add hardware wallets (Trezor/Ledger) support into

• step1 adex-api wallet functionality
• step2 adex-api swaps functionality

[artemii235]

• [ ] Search for Rust libs that support HW wallets. Hopefully, such libs do exist.
• [ ] Implement a PoC of generating e.g. KMD address and receiving some funds to it.
• [ ] Implement a PoC of activating the coin using MM2 + HW wallet. Display balance.

@sergeyboyko0791 Please post the info about libs and documentation you discover to this issue.

[artemii235]

https://crates.io/crates/usb-device https://docs.rs/usbd-webusb/1.0.2/usbd_webusb/

[sergeyboyko0791]

I started researching how to integrate Ledger Nano S/X first into MarketMaker in WASM. Here's what I learned about it:

• Ledger requires to develop a Nano app https://developers.ledger.com/docs/nano-app/introduction/ to work with an asset, token, etc. Also Ledger allows running only one app at the moment. It means, if you want to sign Bitcoin and Eth transactions, you should open the Bitcoin app first and sign a UTXO tx, after that open the Ethereum app and sign an Eth tx. This doesn't suit us. I see only the one way - to develop an AtomicDEX app.
  • Fortunately, there is [Ledger Nano S SKD]https://github.com/LedgerHQ/ledger-nanos-sdk, an SDK ported in Rust. Also a simple example. I've not checked yet if the SDK works on Ledger Nano X.
• I found some crates which communicate with Ledger: bitcoins-rs (native and WASM), Tezedge client (works with Trezor, native only). It looks like we can only reuse the Ledger/Trezor transport, and get inspired by how they work with HW.
• As far as browser support is concerned, there are several ways to communicate with HW from WASM:
  • Import the ledgerhq/hw-transport-u2f npm package like bitcoins-rs/ledger/transports/WASM does. As we discussed with @artemii235, it may lead to security issues;
  • Do not import the whole ledgerhq/hw-transport-u2f npm package, but import only u2f-api.js, google-u2f-api.js files
  • Port u2f-api.js and google-u2f-api.js to Rust

[sergeyboyko0791]

A few comments regarding integration Ledger/Trezor into MarketMaker in WASM

• bitcoin-rs uses u2f-api as a transport protocol, but as the Ledger team mentioned in this article, U2F is deprecated and they stopped support it in favor of the webusb protocol.
• 3rd-party wallets can interact with Ledger using Ledger Live Bridge, but as I found this they seem to be deprecating it.
• Trezor supports Chrome (via WebUSB or Trezor Bridge) and Firefox (via Trezor Bridge only). Read more this. I guess it's possible to use U2F to interact with Trezor, but it may take some time to check this.

In summary, we can try to integrate Ledger and/or Trezor using WebUSB for Chrome only and continue looking for a solution to support every browser.

[pbca26]

I think running one app at a time is a security feature rather than "inconvenience" because a user is required to select a specific app. This adds an extra isolation layer to prevent malicious attempts to retrieve data from other supported coins. If I remember correctly on Trezor all you have to do is specify derivation path and/or coin name and get desired data from a device.

U2F over browser in my experience is highly unstable, constant connection issues, timeouts etc.

[artemii235]

@sergeyboyko0791 Please consider this idea https://github.com/KomodoPlatform/atomicDEX-API/issues/928#issuecomment-940962013 while implementing abstractions for transaction signing. We should have the possibility to delegate this to a remote node that has access to an actual device.

[ca333]

adding my edited comment from the "global todos list" (spreadsheet) from March 2021 for transparency and vision clarification:

I see two good options for the UX of HW wallet support:

(1) use it from HW only - HW can log in with pubkey - mm2 can create transactions and passes the unsigned tx to hw device, hw device signs and returns signed tx. Benefit: this is ultra secure. negative: each swap transaction needs manual transaction confirmation and user interaction - user experience is suboptimal and require too much interaction. However, in regards to security this is the most secure option. (2) use HW wallet for swap funding. i.e. when user attempts swap he signs a tx of full swap volume plus fees which goes to a "local" mm2 address. And mm2 will handle swap from this key without user-interaction requirement. When swap is complete mm2 transfers received coins to HW wallet pubkey. This is quite safe as well and the risk is reduced to the volume amount (in case user-device is infiltrated).

personally I prefer (1) but I understand it turn user experience poor so we likely stick to option (2) anyway

One other factor and very high priority item is https://github.com/KomodoPlatform/atomicDEX-API/issues/740

With both HD and HW wallet support two of the most popular and wanted wallet features would get coverage. It would make sense to keep #740 in mind for the path derivation handlers, et cetera.

[sergeyboyko0791]

At the first iteration, I'd prefer not to change the consensus protocol, but to implement the (1) option. Then we can continue improving HW support by implementing the (2) option and https://github.com/KomodoPlatform/atomicDEX-API/issues/740. As I see, these are 3 different tasks we should solve separately.

[sergeyboyko0791]

In the PR mentioned above I implemented requesting balances of HD accounts and addresses like it does hw-kmd-wallet:

1. Check the transaction history of external, internal addresses for every activated HD account m/44'/coin_type'/account_id'/change/i until we get 20 (configurable gap limit) empty addresses. Remember the number of non-empty addresses for every account to avoid the checking of at least already known addresses.
2. Request and return balances of every non-empty address.

These steps are performed on coin activation and every time user calls wallet_balance. This can lead to a significant increase in network traffic. I found an alternate solution as it's done in MetaMask: User chooses the set of addresses that should be indexed by the app and can add new addresses at any time. The other side is that this behaviour is different from common practice.

@artemii235 @tonymorony what do you think?

[sergeyboyko0791]

I'd like to share my thoughts on how to activate coins with Trezor and provide HD wallet functionality. I propose to add a duplicate of each coin with the HW suffix like KMD-HW, KMD-BEP20-HW etc. It'll allow the user to activate coins with Iguana private key and Trezor device, transfer funds between them.

Cons:

• Users can swap funds with other peers even if HW coins are wallets only by transferring funds from <COIN>-HW to <COIN>
• GUI displays the same orderbooks for both <COIN>-HW and <COIN> coins https://github.com/KomodoPlatform/atomicDEX-API/issues/1089

@artemii235 @tonymorony @ca333

[tonymorony]

We've just had a quite productive call with @sergeyboyko0791 @Milerius @yurii-khi and concluded that HW suffix solution should be sufficient to find the way to make UX comfortable for the user Thank you for your time guys! :)

[sergeyboyko0791]

I'd like to share the integration progress and the what we need to do (in my opinion) to complete the implementation of the wallet functionality:

• [x] PoC integration https://github.com/KomodoPlatform/atomicDEX-API/pull/1150 : spawning RPC tasks, connecting to a Trezor device, withdraw with a Trezor device;
• [x] Coin activation with a Trezor device https://github.com/KomodoPlatform/atomicDEX-API/pull/1190
• [x] HD wallet sqlite, IndexedDb storages https://github.com/KomodoPlatform/atomicDEX-API/pull/1223. The storage is used to cache the information of the user HD accounts including XPUBs;
• [x] Implement the ability to activate coins with Iguana priv key and Hardware Wallet at the same time https://github.com/KomodoPlatform/atomicDEX-API/pull/1227. In other words, launch mm2 with Iguana priv key and activate KMD and KMD-HW at the same time;
• [x ] Implement transaction history for HD accounts;
• [x] Optimize init_utxo, account_balance, scan_for_new_addresses RPC calls by using batch requests to Electrum services. Also consider refactoring account_balance and scan_for_new_addresses by using RpcTask to allow GUI to request the whole account balance instead of using paging options.
• [ ] Integrate trezor/connect https://github.com/KomodoPlatform/atomicDEX-API/issues/1478

[sergeyboyko0791]

Hardware Wallet integration is not definitely finished, but we have reached the point that it can be integrated into GUIs and used in cli. @smk762 could we please arrange work on this? Related issue https://github.com/KomodoPlatform/developer-docs/issues/392

[smk762]

Can we please add a method for returning a user's xpub key? This is available in some competing wallets:

• https://insights.blockonomics.co/how-to-find-your-xpub-key-with-these-8-popular-bitcoin-wallets-ce8ea665ffdc/
• https://trezor.io/learn/a/trezor-suite-app-public-keys-xpub

[smk762]

Can we please add a method for returning a user's xpub key? This is available in some competing wallets:

• https://insights.blockonomics.co/how-to-find-your-xpub-key-with-these-8-popular-bitcoin-wallets-ce8ea665ffdc/
• https://trezor.io/learn/a/trezor-suite-app-public-keys-xpub

Looks like blockbook could help with this https://github.com/trezor/blockbook/blob/master/docs/api.md#get-xpub

[ca333]

Can we please add a method for returning a user's xpub key? This is available in some competing wallets:

• https://insights.blockonomics.co/how-to-find-your-xpub-key-with-these-8-popular-bitcoin-wallets-ce8ea665ffdc/
• https://trezor.io/learn/a/trezor-suite-app-public-keys-xpub

yes definitely. Its a trivial addition and actually a requirement for efficient HD key/account management (enables mm2, GUIs and 3rd party implementations to derivate entire key-tree for any-pre-checks/pre-syncs/et cetera) - we will add this during the next iterations.