Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for new Trezor and Ledger firmware #340

Merged
merged 17 commits into from
Jun 22, 2020

Conversation

achow101
Copy link
Member

@achow101 achow101 commented Jun 8, 2020

This is a general update to fix many issues related to newly released signing behavior in Trezors and Ledgers.

  • The PSBT module is updated to handle PSBT inputs that include both non_witness_utxo and witness_utxo
  • For Trezor, Ledger, and Bitbox, instead of checking for existence of witness_utxo, witness_script, and redeem_script to determine signing behavior, we instead inspect the scripts using existing template functions to determine their type and base our behavior on that.
  • For Trezor and Ledger, the signing behavior is updated to pass in the full previous transaction for segwit inputs if they are available.
  • For Trezor, all of the derivation paths are updated to fit within Trezor's "known paths".
  • In the tests, a patch to bitcoind is added that allows for PSBTs with inputs that have both UTXO types. This is mostly from psbt: Include and allow both non_witness_utxo and witness_utxo for segwit inputs bitcoin/bitcoin#19215 and should be removed once that PR is merged.

Note that the Ledger emulator has not been updated to include the Bitcoin app that has the segwit requirements. Also note that the updated app with those requirements seems to have a bug when a transaction has multiple segwit inputs.

Closes #338 and #337

@molnard
Copy link
Contributor

molnard commented Jun 10, 2020

Hi there,
What do you think, when can we start to work on this in Wasabi? In my mind the procedure as follows:

  1. Fix HWI
  2. Create Wasabi PR that uses the fixed HWI
  3. Add the fix to Wasabi
  4. Tests
  5. Release new HWI
  6. Release new Wasabi

Let me know your thoughts.

@achow101
Copy link
Member Author

Since I've started doing release candidates, I can publish an rc containing the fix that Wasabi can use and test with.

@molnard
Copy link
Contributor

molnard commented Jun 11, 2020

@achow101 thx, I will keep an eye on it.

@Sjors
Copy link
Member

Sjors commented Jun 17, 2020

Tested spending a native segwit input with 45dbd08 along with bitcoin/bitcoin#19215

  • Trezor T:
    • old firmware 2.1.8 still works with both these PRs
    • After upgrading to 2.3.1 it still works
    • signing a transaction without the prerequisite info causes rather ugly behavior: the screen gets stuck at "Signing transaction" and 1/4th progress, and returns a cryptic error: {"error": "b'\\t\\xef=Gy\\x1e\\xc4E\\x86\\xe2-\\x0f?\\x8bw\\xdd\\xf6=\\xc2\\xe3_\\xd2\\xe6\\xcb\\xcfmG\\x0f=\\xfc\\x84A'", "code": -13}.
  • Ledger Nano S:
    • old firmware still works (firmware 1.5.5, bitcoin app version 1.3.9)
    • new firmware 1.6.0, bitcoin app 1.4.2
  • Ledger Nano X, firmware 1.2.4-1:
    • old bitcoin app version 1.3.21 works
    • new bitcoin app version 1.4.2 works

I suggest we clean up the cryptic error when tx info is missing, because users of older Bitcoin Core versions will encounter it. If Trezor firmware version can be obtained, maybe we can even pre-empt the error.

@achow101
Copy link
Member Author

I added a commit which should give a better error message

@Sjors
Copy link
Member

Sjors commented Jun 17, 2020

Now it says "Previous transaction ... not available", which is at least a bit better.

@Sjors
Copy link
Member

Sjors commented Jun 17, 2020

Tested and rather cursory code review ACK 1613a33. I didn't test the BitBox changes.

@jmacato
Copy link

jmacato commented Jun 22, 2020

I can confirm that fixes Wasabi's issues with Trezor (at least for Mainnet, Testnet is a bit wonky... probably a firmware fault). I compiled this via docker and used that binary in linux. Thanks for this @achow101 :D

@achow101 achow101 merged commit 2ce8734 into bitcoin-core:master Jun 22, 2020
@achow101
Copy link
Member Author

@molnard 1.1.2-rc.2 is available for testing now: https://github.com/bitcoin-core/HWI/releases/tag/1.1.2-rc.2 (I messed up rc.1, so rc.2 is the first release candidate)

@molnard
Copy link
Contributor

molnard commented Jun 24, 2020

Thank you @achow101 !

@KayBeSee
Copy link
Contributor

KayBeSee commented Jul 9, 2020

I'm not sure if this is the appropriate place to post this, but I am struggling to get this working on a Trezor One running 1.9.1. I am getting the same ""Previous transaction ... not available" that @Sjors posted earlier.

Where am I supposed to include the previous transaction in the PSBT?

EDIT:
Nevermind, I found the technical blog post about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect script type on SegWit input with non_segwit_utxo
5 participants