wallet: allow skipping script paths #32857
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32857. ReviewsSee the guideline for information on the review process. 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. |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
|
Why is one of the native Windows jobs failing to compile: https://github.com/bitcoin/bitcoin/actions/runs/16027562790/job/45219347987?pr=32857#step:10:553, but not the other: https://github.com/bitcoin/bitcoin/actions/runs/16027562790/job/45219348012?pr=32857 ? |
|
@fanquake because it's in the fuzzer code, I incorrectly changed one of the function signatures there. |
967931f to
22013ba
Compare
|
From the original description:
I probably need to implement this at the |
22013ba to
370081a
Compare
|
Shoehorning this into Well, it compiles and works. Thoughts on how to implement this elegantly? |
370081a to
1c4dfcb
Compare
|
The |
1c4dfcb to
5542074
Compare
|
I split off a non-behavior-changing commit that adds I then did the same for Rinse and repeat for Will continue later to further split 09334ae. It might make sense to add a property to |
Replace the sign, finalize , bip32derivs and sighash_type arguments that are passed to FillPSBT() and SignPSBTInput() with a PSBTOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
Replace the sign, finalize , bip32derivs and sighash_type arguments that are passed to FillPSBT() and SignPSBTInput() with a PSBTOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
Replace the sign, finalize , bip32derivs and sighash_type arguments that are passed to FillPSBT() and SignPSBTInput() with a PSBTOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
5542074 to
4c6a1c7
Compare
4c6a1c7 to
5192880
Compare
|
Sidenote: currently it's unlikely you'll accidentally spend via an |
Replace the sign, finalize , bip32derivs and sighash_type arguments that are passed to FillPSBT() and SignPSBTInput() with a PSBTOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
5192880 to
b0a162f
Compare
Replace the sign, finalize , bip32derivs and sighash_type arguments which are passed to FillPSBT() and SignPSBTInput() with a PSBTFillOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
b0a162f to
33eaf87
Compare
Replace the sign, finalize , bip32derivs and sighash_type arguments which are passed to FillPSBT() and SignPSBTInput() with a PSBTFillOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
33eaf87 to
50023a9
Compare
Replace the sign, finalize , bip32derivs and sighash_type arguments which are passed to FillPSBT() and SignPSBTInput() with a PSBTFillOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
Expand taproot tests to cover avoid_script_path in walletprocesspsbt. When avoiding script paths, there's no need for the workaround that increases fee_rate to compensate for the wallet's inability to estimate fees for script path spends. We use this to indirectly test that key path was used. We also check that taproot_script_path_sigs is not set. Finally, for transactions that can't be signed using their key path, we try again by allowing the script path. Additional test extended private keys were extracted from other tests.
50023a9 to
db07d06
Compare
Replace the sign, finalize , bip32derivs and sighash_type arguments which are passed to FillPSBT() and SignPSBTInput() with a PSBTFillOptions struct. This makes it easier to add additional options later without large code churn, such as avoid_script_path proposed in bitcoin#32857. It also makes the use of default boolean options safer compared to positional arguments that can easily get mixed up.
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
Holding off on rebasing until #32876 is merged or I have to touch it. |
While testing MuSig2 integration in #29675 I ran into a Ledger bug LedgerHQ/app-bitcoin-new#329 that was triggered because we signed for a script path in addition to the key path. The bug itself will be fixed, but it seems useful in general to be able to skip script path signing.
For example, you may have a 2-of-2 musig() descriptor with some sort of fallback after coins are N blocks old. For both on chain privacy and fee reasons, you don't want to use this path unless absolutely necessary. But our wallet can't know if our co-signer still has their key, so we sign the script path at all times. This could lead to accidental broadcast.
This PR introduces a
keypath_onlyoption (defaultfalsefor now) to:sendRPCwalletprocesspsbtRPC.The resulting PSBT won't include
taproot_script_path_sigs. It does still havetaproot_bip32_derivsandmusig2_participant_pubkeys.The
wallet_taproot.pytests are expanded to cover this behavior.It can also be tested with Sjors#91 and a slightly modified Moosig. See LedgerHQ/app-bitcoin-new#329 for the rough steps.
Based on:
TODO:
MutableTransactionSignatureCreatorsendPotential followups:
musig()(after wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys #29675)