HWI
HWI copied to clipboard
Improve support for the coming Ledger app 2.1.0
Intro and context
The upcoming version of the Ledger bitcoin app (version 2.1.0) will remove support to the legacy API (1.*).
A number of limitations of the 2.* protocol have been addressed since the first release:
- Each returned signature is now accompanied by the corresponding pubkey, instead of just the input index.
- Key origin information is no longer required for external xpubs.
- Multiple internal xpubs are supported (the device will sign for each internal xpub).
- Introduces a modified wallet policy language matching this proposal on bitcoin-dev, which addresses some drawbacks of the earlier version (most notably, removing the change/address-index from key information, and adding it to the policy's descriptor template instead).
- Support for message signing was added.
- OP_RETURN outputs are now supported.
Since there are incompatibilities, the modified protocol from version 2.1.0 is opt-in (signaled by using 01
instead of 00
in the p1 field of the APDU), and the protocol of version 2.0.* (using 00
in p1) will still be supported in future versions for some time. Nevertheless, the p1 == 0
protocol should is to be considered deprecated from the moment the 2.1.0 app goes live (expected in early November).
All 2.* versions before 2.1.0 support the legacy 1.x protocol; therefore, in order to keep things simple, this PR uses the legacy protocol for any version prior to 2.1.0). I believe disruption will be minimal: users of the 2.0.* app will downgrade their multisig experience to the legacy protocol (which doesn't recognize change addresses), and are therefore recommended to upgrade to 2.1.0. Nothing changes for users that are still on the 1.* app series (upgrade is, of course, still recommended!).
Moreover, the 2.1.0 adds full support to miniscript within wsh
descriptors.
Changes in this PR
This is the list of changes in this PR:
- Recognize "Bitcoin Legacy" and "Bitcoin Testnet Legacy" as a valid app name for the legacy protocol (regardless of the version). It will be deployed and available as an app in the Ledger app store.
- Completely switch to the updated version (p1 = 1) of the new protocol starting from version 2.1.0; use legacy (1.*) protocol below version 2.1.0. Never retry with the legacy protocol on failure.
- Remove checks for the limitations that were addressed.
- Add support for message signing using the new protocol.
- Add support for displaying multisig addresses.
- Support multisig wallets with 16 keys (a previous version of the python library was limiting to 15 keys by mistake).
Miniscript support is left for a separate PR (more comments below).
Drawbacks
Signing a psbt and displaying an address requires registering the wallet policy on the device. The current version of HWI does that right before signing (and this PR adds the same behavior for displaying addresses).
This is not secure in a compromised machine if the device participates to multiple multisignature/script wallet accounts, as the adversary could trick the user into spending from a different wallet account.
Moreover, miniscript support in HWI is not in this PR, since it requires larger changes to HWI.
I'm working on a BIP for miniscript wallet policies, and will propose a backwards-compatible and vendor-agnostic way to add native support for wallet policies in a separate PR.
@achow101, I didn't work on the tests − I'm not yet familiar with the test suite. Please advise accordingly if there are improvements you'd like me to make.
I managed to run the test suite locally, but it seems to run with an old binary taken from the speculos repository. Not sure how to fix it.
Meanwhile, I rebased to fix a mistake I made in display_multisig_address
, and added a few more commits to fix the remaining issues I'm aware of:
-
signmessage
failing with the new app because of the requirement of'
instead ofh
for the hardened character − solved by always normalizing to'
; - update speculos automation file for some slight UX changes;
- enabled test for
displaymultisigaddress
if not legacy, and re-enabled all the multisig tests that were disabled.
Some work is still needed on test_display_address_multisig
which is failing. The reason seems to be that HWI tests multisigs with:
- xpubs like
[f5acc2fd/48h/1h/0h/0h/0]<xpub>/0
- bare pubkeys like
[f5acc2fd/48h/1h/1h/0h/0/0]023f9fe73519d0bc86658151ef95d7844ed41033fc2051bf8291a1cf1153fcdf03
Both are not supported, as the app expects xpubs to be in the form: [f5acc2fd/48h/1h/0h/0h]<xpub>/<change>/<addr_index>
− which is compatible with the needs of any current multisig software wallet that I'm aware of.
I managed to run the test suite locally, but it seems to run with an old binary taken from the speculos repository. Not sure how to fix it.
It should be using a self built app, which overwrites the speculos provided binary. I'm working on fixing CI in general right now.
@achow101 I disabled again in 00a8c1b32e02d80fb72dc4fbfaae44b37474ece6 the tests for multisig address display, since I'm not too sure how best to adapt the existing tests. Maybe we could leave that to a separate PR and get the other improvements in?