psbt: Taproot fields for PSBT #22558
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This was referenced Jul 27, 2021
|
Concept and Approach ACK. I was experimenting with a basic taproot n-of-m multisig using a single OP_CHECKSIGADD based script on top of this branch. Taproot fields in the PSBTs I was creating (and attempting to sign - although I haven't got that working yet) looked good when I inspected them. I did some very light testing of this, mostly sanity checks |
c6b66e2 to
81f38a3
Compare
81f38a3 to
6003b30
Compare
This was referenced Sep 22, 2021
6003b30 to
2e9031b
Compare
2e9031b to
3ec5cf9
Compare
|
Needs rebase due to silent merge conflict in |
3ec5cf9 to
4283e79
Compare
687eaaa to
58af891
Compare
TaprootSpendData can be gotten from TaprootBuilder, however for PSBT, we also need TaprootBuilders directly (for the outputs). So we store the TaprootBuilder in the FlatSigningProvider and when the TaprootSpendData is needed, we generate it on the fly using the stored builder.
GetSpendData needs to be finalized in order to be used. To avoid future bugs, assert `!m_output_key.IsNull()` as m_output_key is only set during Finalize.
If all inputs are segwit v1+, the non_witness_utxos can be removed.
Actually use pre-existing signatures in CreateTaprootScriptSig if a signature is found for the given key and leaf hash.
The taproot spenddata stored in a sigdata is the combination of data existing previously (e.g. in a PSBT) and the data stored in a SigningProvider. In order to use the external data when signing, we need to be using the sigdata's spenddata.
When filling a PSBT, we search the listed pubkeys in order to determine whether the current DescriptorScriptPubKeyMan could sign the transaction even if it is not watching the scripts. With Taproot, the taproot pubkeys need to be searched as well.
|
Code review ACK b80de4c |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 28, 2022
|
I am pretty sure the memory issue still persists: |
|
@MarcoFalke that's a different one than we found: #22558 (review) (not that that's great news) |
Sjors
reviewed
Jun 30, 2022
| s_tree >> depth; | ||
| s_tree >> leaf_ver; | ||
| s_tree >> script; | ||
| m_tap_tree->Add((int)depth, script, (int)leaf_ver, true /* track */); |
There was a problem hiding this comment.
This is the only place where we call TaprootBuilder::Add (in this PR). We're not checking leaf_ver here, so with a corrupt PSBT the leaf_version & ~TAPROOT_LEAF_MASK assert gets hit.
azuchi
added a commit
to chaintope/bitcoinrb
that referenced
this pull request
Jul 7, 2022
fanquake
added a commit
that referenced
this pull request
Jul 19, 2022
76fb300 psbt: Check Taproot tree depth and leaf versions (Andrew Chow) Pull request description: Since TaprootBuilder has assertions for the depth and leaf versions, the PSBT decoder should check these values before calling TaprootBuilder::Add so that the assertions are not triggered on malformed taproot trees. Fixes #22558 (comment) ACKs for top commit: Sjors: utACK 76fb300 sipa: utACK 76fb300 w0xlt: ACK 76fb300 Tree-SHA512: 94b288bc1453d10bce9a8a6389bc866f2c71c76579b7908e22d6b5770ac387086f6221af8597668e62977d4d6861fe2d72ec7b052002a2c36769d056b2e66360
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jul 20, 2022
76fb300 psbt: Check Taproot tree depth and leaf versions (Andrew Chow) Pull request description: Since TaprootBuilder has assertions for the depth and leaf versions, the PSBT decoder should check these values before calling TaprootBuilder::Add so that the assertions are not triggered on malformed taproot trees. Fixes bitcoin#22558 (comment) ACKs for top commit: Sjors: utACK 76fb300 sipa: utACK 76fb300 w0xlt: ACK bitcoin@76fb300 Tree-SHA512: 94b288bc1453d10bce9a8a6389bc866f2c71c76579b7908e22d6b5770ac387086f6221af8597668e62977d4d6861fe2d72ec7b052002a2c36769d056b2e66360
fanquake
added a commit
that referenced
this pull request
Oct 26, 2022
796b020 wallet: add taproot support to external signer (Sjors Provoost) Pull request description: Builds on #22558 (merged on 2022-06-28). [HWI 2.1.0](https://github.com/bitcoin-core/HWI/releases/tag/2.1.0) or newer is required to import and use taproot descriptors. Older versions will work, but won't import a taproot descriptor. Tested with HWI 2.1.1: * Trezor T (firmware v2.5.1) on Signet: signs, change detection works * Ledger Nano S (firmware 2.1.0, Bitcoin app 2.0.6): signs, change detection works Only the most basic `tr(key)` descriptor is supported, script path spending is completely untested (if it works at all). ACKs for top commit: jb55: utACK 796b020 achow101: ACK 796b020 Tree-SHA512: 6dcb7eeb45421a3bbf2bdabeacd29979867db69077d7bf192bb77faa4bfefe446487b8df07bc40f9457009a88e598bdc09f769e6106fed2833ace7ef205a157a
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 27, 2022
796b020 wallet: add taproot support to external signer (Sjors Provoost) Pull request description: Builds on bitcoin#22558 (merged on 2022-06-28). [HWI 2.1.0](https://github.com/bitcoin-core/HWI/releases/tag/2.1.0) or newer is required to import and use taproot descriptors. Older versions will work, but won't import a taproot descriptor. Tested with HWI 2.1.1: * Trezor T (firmware v2.5.1) on Signet: signs, change detection works * Ledger Nano S (firmware 2.1.0, Bitcoin app 2.0.6): signs, change detection works Only the most basic `tr(key)` descriptor is supported, script path spending is completely untested (if it works at all). ACKs for top commit: jb55: utACK 796b020 achow101: ACK 796b020 Tree-SHA512: 6dcb7eeb45421a3bbf2bdabeacd29979867db69077d7bf192bb77faa4bfefe446487b8df07bc40f9457009a88e598bdc09f769e6106fed2833ace7ef205a157a
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements the Taproot fields for PSBT described in BIP 371.