Wrong signature for segwit transactions with multiple inputs #154
Comments
achow101
added a commit
to bitcoin-core/HWI
that referenced
this issue
Jun 22, 2020
1613a33 trezorlib: Raise a more specific error when a prevtx is not provided (Andrew Chow) 45dbd08 ledger: For app v1.4.0+, do getTrustedInput on segwit inputs (Andrew Chow) fe0f82a btchip: Give the major, minor, and patch versions separately (Andrew Chow) ce5e095 btchip: prefer trustedInput over witness (Andrew Chow) 9800899 ledger: Determine segwit based on scripts and allow both UTXOs (Andrew Chow) c085077 tests: increase signtx keypool (Andrew Chow) a47f7e5 trezor: Determine segwit based on scripts instead of provided info (Andrew Chow) 662eab0 bitbox: Update to inspect scriptcode for signature type (Andrew Chow) 249825d Make imports multiline (Andrew Chow) 9f8e03c trezorlib: Update sign_tx from upstream (Andrew Chow) a264d84 test: Do legacy, then segwit, then mixed (Andrew Chow) 4ba72f3 trezor: handle when psbt has both types of UTXOs (Andrew Chow) 2f40d0f trezor: Use standard derivation paths (Andrew Chow) f862135 trezor: Set coin name as testnet when --testnet (Andrew Chow) d662fad serializations: Refactor script type matching into separate functions (Andrew Chow) 3b6131c test: Patch bitcoind to give out PSBTs with both UTXOs (Andrew Chow) 65c6de6 psbt: Allow PSBTs to have both non_witness_utxo and witness_utxo (Andrew Chow) Pull request description: 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 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](LedgerHQ/app-bitcoin#154) when a transaction has multiple segwit inputs. Closes #338 and #337 ACKs for top commit: Sjors: Tested and rather cursory code review ACK 1613a33. I didn't test the BitBox changes. Tree-SHA512: 18e238748e4e17cc692c2725881b62dc13499d127b532922b550cf41bd3c128927520f91b560706c70b4f4059efefccc3e1fe103168340ecb7eb5bb0060f1bfc
|
1.4.0 introduced a regression if parsing an input sequence together with an empty script - the input sequence is to be sent in a diffeernt APDU. You can see a simple fix in LedgerHQ/btchip-python@6c1d4e4 |
achow101
added a commit
to bitcoin-core/HWI
that referenced
this issue
Jul 2, 2020
d0de7b1 Update bitcoind patch (Andrew Chow) 55f9c7f Update speculos patch (Andrew Chow) bd60b74 Update btchip library (Andrew Chow) Pull request description: A workaround for the [multiple segwit input signing issue](LedgerHQ/app-bitcoin#154) was added to btchip-python. So we update our copy of btchip-python to include this fix. Also updates the speculos and bitcoind patches so that travis works. Note that speculos doesn't have a 1.4.0+ bitcoin app. ACKs for top commit: instagibbs: tACK d0de7b1 tested multiple input segwit tx on ledger nano s with *non-trusted inputs*, bitcoin app 1.4.2, so at least nothing is broken for old flow. Tree-SHA512: f0a7364b4342c1ca619e2119747a0a85062ac8cdab7bddbe78584a283cb62c21bf7c2e760a70b234a41fcc20fc776ca81b31c1ac9c03ca91ef9a89eaa71ca0a3
|
Linking my issue on ledgerjs for relevance; incorrect signature produced for tx with one segwit input. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
and others
In trying to update HWI to get trusted inputs for segwit inputs, it seems like version 1.4.2 is producing incorrect signatures for transactions containing multiple segwit inputs.
I've largely been following the steps given in the documentation. Using btchip-python, I've been calling
startUntrustedTransactionwith inputs that have been trusted and using the scriptcodes for those inputs. Note that this is contrary to the documentation which says to use blank scriptcodes for this step in segwit transaction signing. However in my testing, using a blank scriptcode resulted in a bad signature. Then finalizeInput is done to set the outputs. Then for each input, startUntrustedTransaction is done again with only that input specified and its scriptcode. Then untrustedHashSign is done for just that input.This works for segwit transactions with a single segwit input. But does not work for such transactions containing multiple inputs. For those transactions, a signature is produced, but the signature is not valid for that transaction.
The same code works on app version 1.3.14 (the default provided by speculos) with the change that getTrustedInput is not called for those segwit inputs.
The text was updated successfully, but these errors were encountered: