psbt: classify missing Taproot script-path signatures as signer #36039

pull btcpavao wants to merge 1 commits into bitcoin:master from btcpavao:fix-taproot-psbt-analyze-role changing 3 files +23 −3
  1. btcpavao commented at 2:30 PM on August 20, 2026: none

    This is my first Bitcoin Core PR.

    I originally reported #36035 after running into this while experimenting with Taproot 2-of-3 multisig on Signet. After reporting it, I wanted to see whether I could actually trace the problem in the source code and try to fix it myself.

    I used ChatGPT Pro as a tool to help me navigate the codebase, understand the PSBT analysis/signing flow, and test different hypotheses. I followed the changes myself, built Bitcoin Core locally, and ran the tests rather than just submitting generated code.

    The issue appears to come from the Taproot script-path signing path not recording a missing signature in missing_sigs when a Schnorr signature is not available. AnalyzePSBT() uses missing_sigs to decide whether the next role should be signer. Without it, the PSBT was classified as updater, which is also why the GUI displayed “Transaction is missing some information about inputs.”

    My first attempt at the fix did not work. The regression test still returned updater, which led me to notice that SignMuSig2() can return successfully without actually producing a Taproot script-path signature. I adjusted the change so that, after the Schnorr/MuSig2 attempts, a missing signature is recorded when no script-path signature was produced.

    I also added a regression test that checks that this Taproot script-path case is classified as signer.

    I tested the change locally with:

    test/functional/rpc_psbt.py test/functional/wallet_musig.py test_bitcoin --run_test=psbt_tests test_bitcoin --run_test=miniscript_tests git diff --check

    All of these passed with the current patch.

    This appears to fix the behavior I reported in #36035. Since this is my first contribution, feedback on both the approach and the implementation is very welcome.

    Fixes #36035

  2. DrahtBot added the label PSBT on Aug 20, 2026
  3. DrahtBot commented at 2:30 PM on August 20, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    External sites

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #36122 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"></sub> (BIP460: CISA for Taproot key path spends by fjahr)

    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-->

  4. psbt: classify missing Taproot script-path signatures as signer 641b52828a
  5. in src/script/sign.cpp:398 in 775e2040d8
     392 | @@ -393,8 +393,13 @@ static bool CreateTaprootScriptSig(const BaseSignatureCreator& creator, Signatur
     393 |  
     394 |      if (creator.CreateSchnorrSig(provider, sig_out, pubkey, &leaf_hash, nullptr, sigversion)) {
     395 |          sigdata.taproot_script_sigs[lookup_key] = sig_out;
     396 | -    } else if (!SignMuSig2(creator, sigdata, provider, sig_out, pubkey, /*merkle_root=*/nullptr, &leaf_hash, sigversion)) {
     397 | -        return false;
     398 | +    } else {
     399 | +        if (!SignMuSig2(creator, sigdata, provider, sig_out, pubkey, /*merkle_root=*/nullptr, &leaf_hash, sigversion)) {
     400 | +            return false;
    


    achow101 commented at 6:16 PM on August 20, 2026:

    This results in the analysis still being incorrect for transactions involving a musig.


    btcpavao commented at 7:32 PM on August 20, 2026:

    Thanks, I reproduced this with a Taproot script-path MuSig2 case. My previous change was incorrectly treating the aggregate key as an ordinary missing Schnorr signature.

    I updated the patch to distinguish when SignMuSig2() actually matched a MuSig2 aggregate, and only add missing_sigs for the non-MuSig case. I also added a regression check for the MuSig2 script-path case. wallet_musig.py, rpc_psbt.py, psbt_tests, and miniscript_tests pass locally.

  6. btcpavao force-pushed on Aug 20, 2026
  7. sedited requested review from achow101 on Oct 2, 2026
  8. in src/script/sign.cpp:283 in 641b52828a
     279 | @@ -280,9 +280,10 @@ static bool CreateSig(const BaseSignatureCreator& creator, SignatureData& sigdat
     280 |      return false;
     281 |  }
     282 |  
     283 | -static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigdata, const SigningProvider& provider, std::vector<unsigned char>& sig_out, const XOnlyPubKey& script_pubkey, const uint256* merkle_root, const uint256* leaf_hash, SigVersion sigversion)
     284 | +static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigdata, const SigningProvider& provider, std::vector<unsigned char>& sig_out, const XOnlyPubKey& script_pubkey, const uint256* merkle_root, const uint256* leaf_hash, SigVersion sigversion, bool* is_musig = nullptr)
    


    achow101 commented at 9:24 PM on October 2, 2026:

    In 641b52828ad93ed5ff33f5789908802f1e4fbfde "psbt: classify missing Taproot script-path signatures as signer"

    Adding an output parameter here is entirely unnecessary. SignMuSig2 has access to sigdata, it can update the fields itself if no signature was made.

  9. fametrano commented at 8:33 PM on October 3, 2026: contributor

    Tested 641b52828ad93ed5ff33f5789908802f1e4fbfde: with a tr(H,pk(K)) script-path input carrying witness UTXO, leaf script, control block, internal key and merkle root but no signature, analyzepsbt says updater on the parent commit and signer with this PR, with missing.signatures naming K; a wallet holding only K signs and finalizes that PSBT unchanged, so a signature was all it lacked. psbt_tests, psbt_wallet_tests, miniscript_tests, script_tests and rpc_psbt.py, wallet_musig.py, wallet_taproot.py pass.

    One thing to keep in mind for the restructure achow101 asked for: once SignMuSig2 records the missing signature itself, the key path gets the same treatment, because make_keypath_sig in SignTaproot calls SignMuSig2 too. Today a tr(K) input with PSBT_IN_TAP_INTERNAL_KEY and PSBT_IN_TAP_BIP32_DERIVATION and no signature is updater on both binaries, and the same wallet signs it; after the move it becomes signer, which is right. Two details follow from it. make_keypath_sig runs twice, with the internal key and then with the output key, so an unsigned tr(K) input would list both keys unless the second push is skipped once the first has listed a key; CreateSig already lists every key of a multisig, so either is defensible. And a key-path case in rpc_psbt.py would then belong in this PR, since its behaviour changes.


github-metadata-mirror

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-10-14 02:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me