descriptor: Expose key expression parsing #35918

pull pseudoramdom wants to merge 3 commits into bitcoin:master from pseudoramdom:parse-key-expression-v2 changing 3 files +310 −95
  1. pseudoramdom commented at 6:44 PM on August 6, 2026: contributor

    This PR exposes a utility function to parse and validate a standalone descriptor key expression. This is particularly useful in GUI multisig setup flows where the UI accepts a pasted key expression. The GUI can use this parser to validate input, inspect the parsed key, and apply setup-specific policy i.e. reject private keys, uncompressed keys etc. (Multisig Wizard tracking issue: #35645)

    PubkeyProvider is the existing internal representation of a parsed key expression. This PR exposes its interface in descriptor.h and adds a public ParsePubkey() overload that returns the providers produced by the existing parser.

    Key changes

    • public overload uses P2TR key rules, permitting compressed, x-only, extended, and MuSig key expressions while rejecting uncompressed keys.
    • Add GetOriginInfo() for retrieving explicitly provided key origin information.

    Also, while I'm here, I noticed that musig parser accepted trailing characters apart from /.

    • Add a fix & test to reject unexpected trailing characters after a musig() expression
  2. DrahtBot added the label Descriptors on Aug 6, 2026
  3. DrahtBot commented at 6:44 PM on August 6, 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.

    Type Reviewers
    ACK w0xlt

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #36648 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36648.svg"></sub> (descriptor, rpc, wallet: do not echo key material in error messages by fametrano)
    • #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. DrahtBot closed this on Aug 7, 2026
  5. DrahtBot reopened this on Aug 7, 2026
  6. DrahtBot added the label Needs rebase on Aug 11, 2026
  7. pseudoramdom force-pushed on Aug 11, 2026
  8. pseudoramdom marked this as ready for review on Aug 11, 2026
  9. DrahtBot removed the label Needs rebase on Aug 11, 2026
  10. w0xlt commented at 8:32 PM on August 13, 2026: contributor

    ACK 93600f9f0360bf48a1f616f0f5db3c9ce985ce4b

  11. DrahtBot added the label Needs rebase on Aug 24, 2026
  12. in src/script/descriptor.cpp:2871 in 93600f9f03 outdated 
    2866 | +std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(std::string_view key_expression, FlatSigningProvider& out, std::string& error)
2867 | +{
2868 | +    uint32_t key_exp_index{0};
2869 | +    const std::span<const char> expression{key_expression};
2870 | +    return ParsePubkey(key_exp_index, expression, ParseScriptContext::P2TR, out, error);
2871 | +}
    uqlidi commented at 5:05 PM on August 28, 2026: 
    std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(std::string_view key_expression, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error)
{
    uint32_t key_exp_index{0};
    const std::span<const char> expression{key_expression};
    return ParsePubkey(key_exp_index, expression, ctx, out, error);
}

    I see you hardcoded it for p2tr. shouldn't be for all script types? Why are you making the overload follow the p2tr rules only?

    sedited commented at 7:02 AM on October 13, 2026:

    @pseudoramdom can you follow up here?

    pseudoramdom commented at 7:30 AM on October 14, 2026:

    Using P2TR here is intentional, as mentioned in the PR description. Its key parsing rules match what we want for standalone key expressions: accept compressed, x-only, extended, and musig() expressions, while rejecting uncompressed keys.

  13. sedited commented at 5:53 AM on October 14, 2026: contributor

    Ping for rebase @pseudoramdom

  14. descriptor: Reject trailing text after musig()
    The musig() parser split the input at the closing parenthesis without
validating the remaining text. This allowed unexpected characters after
the expression to be ignored.
Add a requirement for nonempty char after musig() to begin with '/'.
    7835e0ef70
  15. descriptor: move PubkeyProvider to descriptor.h
    Expose the abstract interface by moving to descriptor.h to allow other
modules to consume parsed key expressions.
Mostly a move-only commit. Implementation of operator< is retained in
descriptor.cpp
    c17a6ab8b7
  16. descriptor: Add public ParsePubKey overload
    Add an overload for parsing standalone key expressions using P2TR key
rules.
Also, expose explicitly provided origin information through GetOriginInfo(),
allowing callers to inspect the fingerprint and derivation path without
re-parsing the expression.
    17ddfdc0ed
  17. pseudoramdom force-pushed on Oct 14, 2026
  18. pseudoramdom commented at 7:35 AM on October 14, 2026: contributor

    Rebased & force-pushed to https://github.com/bitcoin/bitcoin/pull/35918/commits/17ddfdc0edc3ac35b1a87c884d61db2a58941bce. No other changes.

  19. DrahtBot removed the label Needs rebase on Oct 14, 2026
  20. fametrano commented at 6:25 AM on October 18, 2026: none

    The first commit, 7835e0ef70, fixes a bug I also found on v31.1: tr(musig(X)xyz) is accepted and xyz is dropped. The worse case is a typo such as tr(musig(X)0/*), missing the /, which v31.1 turns into a non-ranged tr(musig(X)) without an error. Since it is a parser fix and the rest of the PR adds a new API, would you split it into its own PR? It could then be reviewed and backported on its own.

  21. pseudoramdom requested review from w0xlt on Oct 20, 2026
  22. w0xlt commented at 6:05 PM on October 20, 2026: contributor

    reACK 17ddfdc0edc3ac35b1a87c884d61db2a58941bce

Contributors
pseudoramdomDrahtBotw0xltuqlidiseditedfametrano
Labels
Descriptors
Linked (view graph)
#35645 Multisig Wizard tracking issue#36325 Contrib: add multisig wizard
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-30 09:40 UTC