descriptor: Represent multipath descriptors in a single Descriptor object #36442

achow101 commented at 10:14 PM on October 5, 2026: member

Currently, multipath descriptor strings treat the multipath specifiers as syntactic sugar. Once we finish parsing such a string, the multipath elements are lost as the result is the multiple distinct descriptors that the multipath is a shorthand for. This means that we are unable to return the multipath string for such descriptors.

This PR allows descriptors to actually be multipath where the multipath specifiers are represented inside of the paths of BIP32PubkeyProviders. This allows for the multipath string to be produced.

In order to achieve this, a lot of derivation path parsing is refactored. KeyPath is promoted to being its own class (which subclasses std::vector), while the individual derivation indexes become a KeyPathElement object so that the multipath elements can be represented. Furthermore, hardened derivation characters are now being stored within KeyPathElements so each element knows what the original hardened character was. This also cleans up a little bit of the descriptor implementation as we no longer have to carry around bool apostrophe in a bunch of places.

Lastly, Parse now returns a single Descriptor object, rather than a vector of them. If the descriptor is a multipath descriptor, the expansion can be retrieved using GetMultipathExpansion(). The wallet uses this and still must not store multipath descriptors.

DrahtBot added the label Descriptors on Oct 5, 2026

DrahtBot commented at 10:15 PM on October 5, 2026: contributor

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

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36442
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36442.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36442#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	w0xlt, Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36579 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36579.svg"> (refactor: Enable some string performance tidy rules by maflcko)
#36443 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36443.svg"> (descriptor: Reconstruct a multipath descriptor from its expansion by achow101)
#36122 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"> (BIP460: CISA for Taproot key path spends by fjahr)
#36070 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36070.svg"> (wallet: Add deriveHDKey interface by PraneethGunas)
#35918 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35918.svg"> (descriptor: Expose key expression parsing by pseudoramdom)
#35377 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35377.svg"> (wallet: Allow importing of descriptors without private keys when the wallet has the private keys by achow101)
#35170 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35170.svg"> (test: Better test coverage for ParseHDKeypath() by optout21)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

src/util/bip32.h: give → given [misspelling]

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

src/qt/test/wallettests.cpp: WalletDescriptor(std::move(desc), 0, 0, 1, 1)
src/rpc/mining.cpp: Parse(descriptor, key_provider, error, false)
src/rpc/output_script.cpp: Parse(desc_str, key_provider, error, true)
src/script/descriptor.cpp: std::make_unique<ConstPubkeyProvider>(key_exp_index, pubkey, false)
src/script/descriptor.cpp: std::make_unique<ConstPubkeyProvider>(key_exp_index, pubkey, true)
src/script/descriptor.cpp: std::make_unique<ConstPubkeyProvider>(0, pubkey, false)
src/script/descriptor.cpp: std::make_unique<ConstPubkeyProvider>(0, pubkey, true)
src/wallet/scriptpubkeyman.cpp: WalletDescriptor(std::move(desc), creation_time, 0, 0, 0)
src/wallet/scriptpubkeyman.cpp: WalletDescriptor(std::move(desc), 0, 0, chain_counter, 0)
src/wallet/test/wallet_tests.cpp: WalletDescriptor{std::move(desc), 0, 0, 0, 0}
src/wallet/wallet.cpp: WalletDescriptor(std::move(desc), creation_time, 0, 0, 0)
src/wallet/wallet.cpp: Parse(desc_str, keys, desc_error, false)
src/wallet/walletutil.cpp: WalletDescriptor(std::move(desc), creation_time, 0, 0, 0)
src/wallet/test/util.cpp: Parse(desc_str, keys, error, false)

2026-10-07 21:48:25

achow101 force-pushed on Oct 5, 2026

DrahtBot added the label CI failed on Oct 5, 2026

DrahtBot commented at 10:35 PM on October 5, 2026: contributor

🚧 At least one of the CI tasks failed. Task ASan + LSan + UBSan + integer: https://github.com/bitcoin/bitcoin/actions/runs/37381217840/job/112003412636 LLM reason (✨ experimental): The build failed because walletload_tests.cpp assigns Parse()’s std::unique_ptr<Descriptor> result to a vector of unique pointers.

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

DrahtBot added the label Needs rebase on Oct 7, 2026

util: Make KeyPathElement class with private members 2ff1009dbe

achow101 force-pushed on Oct 7, 2026

DrahtBot removed the label Needs rebase on Oct 7, 2026

util: Change all key path operations to operate with std::vector<KeyPathElement> 9f3a86f4b1

util: Make KeyPath a subclass of std::vector and not only a typedef

We will want to have member functions on KeyPath later, so make it a
class now.

7851cfa16c

util: Store hardened character in KeyPathElement e29d6efd19

util: Make KeyPathElement handle multipaths

Moves all of the multipath parsing into ParseHDKeypath, and let
KeyPathElement be a multipath element.

b84641e4c6

descriptor: Get the multipath expansion from a multipath descriptor 177874ccd3

descriptor: Produce multipath descriptor in ParseScript

Internally, descriptor parsing will produce multipath descriptor objects
rather than std::vector<DescriptorImpl>. This single multipath
descriptor becomes std::vector<Descriptor> within Parse. Future commits
will return the single multipath descriptor object for all descriptor parsing.

d689425c0c

descriptor: Return multipath descriptor from Parse

Instead of a std::vector<std::unique_ptr<Descriptor>>, Parse can return
a single std::unique_ptr<Descriptor> that contains the multipath
information if the descriptor was multipath. Callers that
require the multipath expansion use GetMultipathExpansion() to get the
expanded descriptors.

74915efa6e

achow101 force-pushed on Oct 7, 2026

w0xlt commented at 6:05 PM on October 8, 2026: contributor

Concept ACK

Sjors referenced this in commit 95ad9d4ad5 on Oct 20, 2026

Sjors referenced this in commit de20c94b37 on Oct 20, 2026

Sjors referenced this in commit de8d85c4c6 on Oct 20, 2026

Sjors referenced this in commit 59eb880b15 on Oct 20, 2026

Sjors referenced this in commit 5c8268050b on Oct 20, 2026

Sjors referenced this in commit 334e6a6388 on Oct 20, 2026

Sjors referenced this in commit 80a95f5644 on Oct 20, 2026

Sjors referenced this in commit 53a458ed82 on Oct 21, 2026

Sjors referenced this in commit 2635883c46 on Oct 21, 2026

Sjors referenced this in commit 4ac892de26 on Oct 21, 2026

Sjors commented at 4:17 AM on October 21, 2026: member

Concept ACK.

Sorry for the pingback noise. I've now adjusted my merge skill to stop pinging the main repo from fork PRs.

DrahtBot added the label Needs rebase on Oct 21, 2026

DrahtBot commented at 2:01 PM on October 21, 2026: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

fametrano commented at 4:50 AM on October 23, 2026: contributor

On this head, migratewallet turns every step of an imported key's origin path into a hardened step.

SingleKeyPathElement::Unserialize assigns a bool to std::optional<char> m_hardened. The optional is then always engaged, so ChildNumber() sets the hardened bit on every step read from disk. It came with "util: Store hardened character in KeyPathElement". A v29.4 legacy watch-only wallet holding wpkh([d34db33f/84h/0h/0h/0/5]<pubkey>) migrates to 84h/0h/0h/0/5 on v31.1 and to 84h/0h/0h/0h/5h on this head. With the change suggested inline, the migrated descriptors match v31.1.

The multipath round trip agrees with btclib-wallet, a BIP389 implementation I maintain. On 25 descriptors (wpkh, sh-wsh multisig, tr trees, miniscript, musig, steps of three and five elements), getdescriptorinfo keeps each input's paths and markers, and its multipath_expansion and deriveaddresses match btclib-wallet. Both refuse the same 8 malformed ones. As the description says, markers are now kept as written; v31.1 printed one per key. A descriptor mixing h and ' thus prints, with its checksum, differently from v31.1, which may deserve a release note. I can share the scripts and outputs.

in src/util/bip32.h:58 in 74915efa6e

  57 | +    inline void Unserialize(Stream& s)
  58 | +    {
  59 | +        uint32_t num;
  60 | +        s >> num;
  61 | +        m_index = num & ~BIP32_HARDENED_FLAG;
  62 | +        m_hardened = bool(num & BIP32_HARDENED_FLAG);

fametrano commented at 4:50 AM on October 23, 2026:

Engaged only for a hardened step, so an unhardened step read from disk stays unhardened:

        m_hardened = (num & BIP32_HARDENED_FLAG) ? std::optional<char>{'h'} : std::nullopt;