descriptor: Reconstruct a multipath descriptor from its expansion #36443

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

#36442 allows multipath descriptors to be represented in a single Descriptor object. GetMultipathExpansion returns the multiple descriptors represented by a multipath descriptor. This expansion is reversible - given the expansion in the order of the multipath elements, it is possible to determine what the original multipath descriptor was. This PR adds ReconstructMultipath to produce a new Descriptor representing the multipath descriptor that produced the expansion.

Depends on #36442

DrahtBot added the label Descriptors on Oct 5, 2026

DrahtBot commented at 10:16 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/36443
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36443.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36443#author-ralph

Reviews

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

Type	Reviewers
Concept ACK	w0xlt

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)
#36442 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36442.svg"> (descriptor: Represent multipath descriptors in a single Descriptor object 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: for a give multipath position → for a given multipath position [“give” is a typo]
src/test/descriptor_tests.cpp: Failed reconstrcted to private string → Failed reconstructed to private string [“reconstrcted” is misspelled]

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/scriptpubkeyman.cpp: Parse(desc_str, provider, error, false) (combo descriptor)
src/scriptpubkeyman.cpp: Parse(desc_str, provider, error, false) (chain descriptor)
src/scriptpubkeyman.cpp: WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0)
src/scriptpubkeyman.cpp: WalletDescriptor w_desc(std::move(desc), 0, 0, chain_counter, 0)
src/wallet/rpc/spend.cpp: Parse(desc_str, desc_out, error, true)
src/wallet/test/fuzz/scriptpubkeyman.cpp: Parse(desc_str.value(), keys, error, false)
src/wallet/test/util.cpp: Parse(desc_str, keys, error, false)
src/wallet/walletutil.cpp: Parse(desc_str, keys, error, false)
src/wallet/walletutil.cpp: WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0)

2026-10-07 18:03:03

achow101 marked this as a draft on Oct 5, 2026

achow101 force-pushed on Oct 5, 2026

DrahtBot added the label CI failed on Oct 5, 2026

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

🚧 At least one of the CI tasks failed. Task macOS-cross to arm64: https://github.com/bitcoin/bitcoin/actions/runs/37381386585/job/112003972868 LLM reason (✨ experimental): CI failed to compile walletload_tests.cpp: Parse() returns a std::unique_ptr<Descriptor>, but the code assigns it to a vector of those 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

util: Change all key path operations to operate with std::vector<KeyPathElement> 9353f7986c

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.

611b2b9f16

util: Store hardened character in KeyPathElement 192c38bb11

util: Make KeyPathElement handle multipaths

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

2f7d95b319

descriptor: Get the multipath expansion from a multipath descriptor 010a986ad3

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.

fa1b3002b4

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.

b7980165b2

descriptor: Implement multipath reconstruction 5c0b1e77a6

test: Add multipath reconstruction round trip checks e2d78b3edd

fuzz: Check multipath descriptor reconstruction 8aba47abf1

test: Invalid multipath reconstruction tests 62e70eb6f0

achow101 force-pushed on Oct 7, 2026

DrahtBot removed the label Needs rebase on Oct 7, 2026

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

Concept ACK

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 334e6a6388 on Oct 20, 2026

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

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

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

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 6:18 AM on October 23, 2026: contributor

ReconstructMultipath accepts relatives that repeat a path value, and then returns a descriptor that is not their multipath form.

I tested it with a small harness against btclib-wallet, a BIP389 implementation I maintain. Its expansions of 24 multipath descriptors each came back as the original. Thirteen mismatched sets (different keys, function, threshold, timelock, key order, tree shape, origin, wildcard or range, two changed steps in one key, musig steps at both levels) return null, as they should.

Repeated values are not caught. wpkh(X/0/*), wpkh(X/1/*), wpkh(X/0/*) gives wpkh(X/<0;1;0>/*), which Parse refuses as a duplicate. wpkh(X/0/*) twice and then wpkh(X/1/*) gives wpkh(X/<0;1>/*): two elements for three inputs. Two identical inputs give the single-path descriptor back. In BIP32PubkeyProvider::ReconstructMultipath, each relative is compared with p after AddMultipathIndex has already extended it. So a repeat is skipped while p still holds one index, and appended once it holds more. Nothing checks the indexes already in p, or that the step ends with one index per input. The inline suggestions add both checks, with a test for the repeated cases, musig included. I can share the harness and its outputs.

in src/script/descriptor.cpp:699 in 62e70eb6f0

 695 | +                if (p != rp) {
 696 | +                    if (multipath_pos.has_value() && *multipath_pos != i) {
 697 | +                        return false;
 698 | +                    }
 699 | +                    multipath_pos = i;
 700 | +                    p.AddMultipathIndex(rp.Index(0));

fametrano commented at 6:18 AM on October 23, 2026:

A relative whose index is already in the step is a repeat, so reject it.

                    if (std::ranges::find(p.Indexes(), rp.Index(0)) != p.Indexes().end()) return false;
                    p.AddMultipathIndex(rp.Index(0));

in src/script/descriptor.cpp:959 in 62e70eb6f0

 954 | +                if (p != rp) {
 955 | +                    if (multipath_pos.has_value() && *multipath_pos != i) {
 956 | +                        return false;
 957 | +                    }
 958 | +                    multipath_pos = i;
 959 | +                    p.AddMultipathIndex(rp.Index(0));

fametrano commented at 6:18 AM on October 23, 2026:

Same check for the musig path.

                    if (std::ranges::find(p.Indexes(), rp.Index(0)) != p.Indexes().end()) return false;
                    p.AddMultipathIndex(rp.Index(0));

in src/script/descriptor.cpp:1389 in 62e70eb6f0

1384 | +                    todo.back().second.push_back(rel->m_subdescriptor_args.at(i).get());
1385 | +                }
1386 | +            }
1387 | +        }
1388 | +
1389 | +        return out;

fametrano commented at 6:18 AM on October 23, 2026:

Fail unless the step ends with one index per input. This also rejects identical inputs.

        // Each relative must add one multipath index
        if (!relatives.empty() && multipath_len != relatives.size() + 1) return nullptr;

        return out;

in src/test/descriptor_tests.cpp:1157 in 62e70eb6f0

1152 | +        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)",
1153 | +    });
1154 | +    CheckFailedMultipathReconstruction({
1155 | +        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0/*,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/1))",
1156 | +        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/9/*h,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/1))",
1157 | +    });

fametrano commented at 6:18 AM on October 23, 2026:

Add the repeated-value cases, with and without musig.

    });
    CheckFailedMultipathReconstruction({
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)",
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/1)",
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)",
    });
    CheckFailedMultipathReconstruction({
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)",
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)",
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/1)",
    });
    CheckFailedMultipathReconstruction({
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)",
        "wpkh(xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)",
    });
    CheckFailedMultipathReconstruction({
        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)",
        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/2)",
        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)",
    });
    CheckFailedMultipathReconstruction({
        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)",
        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)",
        "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/2)",
    });