Sjors
commented at 2:13 PM on July 2, 2025:
member
While testing MuSig2 integration in #29675 I ran into a Ledger bug https://github.com/LedgerHQ/app-bitcoin-new/issues/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_only option (default false for now) to:
the send RPC
the walletprocesspsbt RPC.
The resulting PSBT won't include taproot_script_path_sigs. It does still have taproot_bip32_derivs and musig2_participant_pubkeys.
The wallet_taproot.py tests are expanded to cover this behavior.
#35092 (wallet: stop materializing skipped descriptor addresses after high-index detection by takeshikurosawaa)
#34861 (wallet: Add importdescriptors interface by polespinasa)
#33392 (wallet, rpc: add UTXO set check and incremental rescan to importdescriptors by musaHaruna)
#32861 (Have createwalletdescriptor auto-detect an unused(KEY) by Sjors)
#32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
#31668 (Added rescan option for import descriptors by saikiran57)
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.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
LLM Linter (✨ experimental)
Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):
Parse(desc_str, keys, error, false) in src/wallet/rpc/wallet.cpp
WalletDescriptor(std::move(descs.at(0)), GetTime(), 0, 0, 0) in src/wallet/rpc/wallet.cpp
<sup>2026-05-01 10:44:00</sup>
DrahtBot added the label CI failed on Jul 2, 2025
DrahtBot
commented at 3:53 PM on July 2, 2025:
contributor
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed.
<sub>Task ARM, unit tests, no functional tests: https://github.com/bitcoin/bitcoin/runs/45219351756</sub>
<sub>LLM reason (✨ experimental): Compilation error due to passing a bool where an int* is expected in fillPSBT function.</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
fanquake
commented at 4:04 PM on July 2, 2025:
member
Sjors
commented at 4:35 PM on July 2, 2025:
member
@fanquake because it's in the fuzzer code, I incorrectly changed one of the function signatures there.
Sjors force-pushed on Jul 2, 2025
Sjors
commented at 4:53 PM on July 2, 2025:
member
From the original description:
Under the hood it adds m_hide_script_paths to the HidingSigningProvider which triggers an early return in GetTaprootSpendData() when set.
It's also missing taproot_scripts which might be a bit too much. At least in initial testing Ledger wouldn't sign it.
I probably need to implement this at the SigningProvider level rather than HidingSigningProvider. The early return needs to be inside SignTaproot() right before // Try script path spending rather than in GetTaprootSpendData(). The goal isn't to hide leaf scripts from co-signers, only to make sure we don't sign them ourselves.
Sjors force-pushed on Jul 2, 2025
Sjors
commented at 6:31 PM on July 2, 2025:
member
Shoehorning this into SigningProvider got ugly real fast. So instead I endup up passing avoid_script_path along the call stack from SignPSBTInput to SignTaproot. Which is also ugly...
Well, it compiles and works. Thoughts on how to implement this elegantly?
Sjors force-pushed on Jul 3, 2025
Sjors
commented at 12:48 PM on July 3, 2025:
member
The walletprocesspsbt RPC now also has this option. Added test coverage by expanding wallet_taproot.py.
Sjors force-pushed on Jul 3, 2025
Sjors
commented at 2:01 PM on July 3, 2025:
member
I split off a non-behavior-changing commit that adds avoid_script_path to FillPSBT. I think it makes sense to pass this option here, although in general we may want to refactor FillPSBT to take an options struct instead of this ever expanding argument list. That could be an independent or prerequisite PR.
I then did the same for SignPSBTInput, where I also think struct would be better.
Rinse and repeat for ::SignTransaction.
Will continue later to further split 09334ae0bd7939ed7476eb2b36f3bf8085885fe7. It might make sense to add a property to BaseSignatureCreator or it subclasses?
DrahtBot removed the label CI failed on Jul 3, 2025
Sjors referenced this in commit ea493972d2 on Jul 4, 2025
Sjors referenced this in commit edbb5d8df3 on Jul 4, 2025
Sjors referenced this in commit 2829cb78c5 on Jul 4, 2025
Sjors force-pushed on Jul 4, 2025
Sjors
commented at 1:45 PM on July 4, 2025:
member
I opened #32876 and rebased this PR on top of it. That allowed me to drop 1486f05c8eded02d72690bf34d983a03e4ad748b + efc34a514b59cc1bab6e4d0f80c197481561c429.
Sjors force-pushed on Jul 4, 2025
Sjors
commented at 4:02 PM on July 4, 2025:
member
I expanded #32876 with SignOptions. This shrinks 93fb607af3141ff1e22693588f498764401fc2c4 to just e2a151c3dc35997ac0f515feb292fc4543093593 which is very small ... but very ugly: casting BaseSignatureCreator to MutableTransactionSignatureCreator to access its avoid_script_path option.
Sjors
commented at 7:57 AM on July 7, 2025:
member
This may also be useful for Silent Payments, cc @josibake.
Sjors
commented at 12:59 PM on July 7, 2025:
member
Sidenote: currently it's unlikely you'll accidentally spend via an older() or after() script path. This is because you need to manually set the input sequence field in the send (etc.) RPC (and manually adjust the fee rate). Found out the hard way while testing 🤦 (there's currently no useful feedback when this happens).
DrahtBot added the label Needs rebase on Jul 7, 2025
Sjors referenced this in commit 2923087479 on Jul 8, 2025
Sjors force-pushed on Jul 8, 2025
DrahtBot removed the label Needs rebase on Jul 8, 2025
Sjors referenced this in commit 576da4c7dd on Jul 8, 2025
Sjors force-pushed on Jul 8, 2025
Sjors referenced this in commit 8660b55c88 on Jul 11, 2025
Sjors force-pushed on Jul 14, 2025
DrahtBot added the label Needs rebase on Jul 29, 2025
Sjors force-pushed on Jul 29, 2025
Sjors referenced this in commit 8c268ff1d7 on Jul 29, 2025
DrahtBot removed the label Needs rebase on Jul 29, 2025
Sjors referenced this in commit 6703cc7743 on Aug 1, 2025
DrahtBot added the label Needs rebase on Aug 19, 2025
Sjors
commented at 11:51 AM on August 25, 2025:
member
Holding off on rebasing until #32876 is merged or I have to touch it.
rkrux
commented at 12:39 PM on October 6, 2025:
contributor
Sidenote: currently it's unlikely you'll accidentally spend via an older() or after() script path. This is because you need to manually set the input sequence field in the send (etc.) RPC (and manually adjust the fee rate). Found out the hard way while testing 🤦 (there's currently no useful feedback when this happens).
Scenario that I have not tested but expect to behave in this way: 3 signers (A, B, C) form a MuSig (ABC) in the keypath and also have 3 non-Musig multi-sig script paths (AB, BC, AC) without any timelocks. They intend to MuSig sign in order from A to C to spend via keypath and start the signing flow first by producing nonces in the mentioned order, and then by adding partial signatures. By the time B adds its nonce, there'd would be a fully signed spendable script path? If yes, seems unusual to have this when the intent of the signers was for keypath spending, which makes a good case for this PR.
rkrux
commented at 12:42 PM on October 6, 2025:
contributor
If #32876 is not garnering reviews, can consider undrafting this PR itself because it contains 7 commits only.
Allowing skipping script path spending optionally seems reasonable to me and a ready-to-review PR is more inviting than a draft one.
Sjors
commented at 8:10 AM on October 7, 2025:
member
@rkrux I'm not too worried about reviews just yet, let's focus on getting #29675 in.
Sjors referenced this in commit 4c11b25943 on Oct 16, 2025
Sjors force-pushed on Oct 16, 2025
DrahtBot removed the label Needs rebase on Oct 16, 2025
Sjors referenced this in commit a2a20b97ca on Nov 4, 2025
Sjors force-pushed on Nov 4, 2025
DrahtBot added the label Needs rebase on Nov 18, 2025
Sjors referenced this in commit 643e6dc59a on Jan 5, 2026
Sjors force-pushed on Jan 5, 2026
DrahtBot removed the label Needs rebase on Jan 5, 2026
Sjors referenced this in commit e4a1397fea on Jan 22, 2026
Sjors force-pushed on Jan 22, 2026
DrahtBot added the label CI failed on Jan 22, 2026
DrahtBot added the label Needs rebase on Mar 25, 2026
Sjors referenced this in commit 66ce046ab6 on Mar 26, 2026
Sjors force-pushed on Mar 26, 2026
DrahtBot removed the label CI failed on Mar 26, 2026
DrahtBot removed the label Needs rebase on Mar 26, 2026
DrahtBot added the label Needs rebase on Apr 19, 2026
Sjors referenced this in commit b3788bf2fc on Apr 22, 2026
Sjors force-pushed on Apr 22, 2026
DrahtBot added the label CI failed on Apr 22, 2026
DrahtBot removed the label Needs rebase on Apr 23, 2026
Sjors referenced this in commit dc4a5d1270 on Apr 28, 2026
sedited referenced this in commit cef58341a0 on Apr 29, 2026
DrahtBot added the label Needs rebase on Apr 29, 2026
descriptor: Add unused(KEY) descriptor
unused() descriptors do not have scriptPubKeys. Instead, the wallet uses
them to store keys without having any scripts to watch for.
80c29bc6f1
test: Simple test for importing unused(KEY)82bc280de4
wallet: Add addhdkey RPCf3f8bcbd1d
wallet, rpc: Disallow import of unused() if key already exists35bbee6374
wallet, rpc: Disallow importing unused() to wallets without privkeys89b9a01b4e
doc: Release note for addhdkeya39cc16b43
Sjors force-pushed on Apr 30, 2026
Sjors
commented at 9:45 AM on April 30, 2026:
member
Rebased after #32876 landed. I need to check a few more things before marking this as ready for review.
DrahtBot removed the label CI failed on Apr 30, 2026
DrahtBot removed the label Needs rebase on Apr 30, 2026
wallet: add option to avoid script path spendsed89145afb
rpc: add keypath_only to walletprocesspsbt38885eddc4
test: cover keypath_only in wallet_taproot.py
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.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-05-03 21:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me