wallet: store multipath descriptor #36133

Sjors commented at 6:12 PM on August 31, 2026: member

HWI recently added a registerdescriptor command (https://github.com/bitcoin-core/HWI/pull/842) which some hardware wallets require in order to (more safely) support multisig. This registration can then be used with displayaddress and signtx (using the --registration argument, see https://github.com/bitcoin-core/HWI/pull/841 and https://github.com/bitcoin-core/HWI/pull/841). Internally HWI converts the descriptor to a BIP388 policy. The only catch is that we need to provide a multipath descriptor, rather than separate receive and change descriptors. This PR makes that possible.

Our wallet already supported importing multipath descriptors, but upon import they are irreversibly turned into change and receive descriptors. We don't overhaul this fundamental descriptor wallet design. Rather (as per approach 5 from #36075) we store an additional record to hold on to the multipath descriptor strings and point to its matching receive and change descriptor.

Existing wallets are not impacted (and not "upgraded").

The main commits are:

refactor: introduce descriptor ParseState: the parser first collects keys, generates replacements (not yet normalized, just converting to public keys) and then applies those replacements (in reverse order).
descriptor: normalize keys in the multipath descriptor string: no record is generated if hardened derivation blocks normalization (those would be useless).
descriptor: let Parse return the multipath descriptor string
wallet: store multipath descriptor record on import

These commits make wallet handling consistent and expose the record through RPC:

wallet: store multipath descriptor record on wallet creation: covers default wallets and createwalletdescriptor.
wallet: expand multipath descriptors with 'h' hardened marker: we don't preserve the exact imported string anyway, so might as well make it consistent
wallet: copy multipath record in exportwatchonlywallet: remaps descriptor IDs to the destination wallet.
rpc: return multipath descriptor from getaddressinfo
rpc: return multipath descriptor from listdescriptors

The remaining commits provide parser plumbing, tests, and refactors that make the main changes easier to follow and keep the descriptor code maintainable:

miniscript: let FromString take a string_view
test: add MULTIPATH flag to descriptor test vectors: tags existing vectors
refactor: use util::Expected for descriptor parsers: replaces &error and introduces ParsePubkeyResult and other aliases that are expanded later (with less churn).
Four small helper extractions, kept as separate commits to make each change easier to review:
- refactor: extract LastHardenedIndex helper
- refactor: extract OriginKeyString helper
- refactor: extract MergeNormalizedOrigin helper
- refactor: extract HardenedPrefix helper
refactor: setup wallet descriptors per output type pair (dropping the loop over internal)

miniscript: let FromString take a string_view

This lets the next commit record key positions in the descriptor string
while its miniscript expression is parsed in place.

5198e3980d

test: add MULTIPATH flag to descriptor test vectors

Use it to check whether parsing expands a descriptor into multiple descriptor.

ad24e1d98f

DrahtBot added the label Wallet on Aug 31, 2026

DrahtBot commented at 6:12 PM on August 31, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36133.

Reviews

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

Type	Reviewers
Concept ACK	jeanpablojp

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:

#36143 (descriptor: add CreateMultisigDescriptor() by rxbryan)
#36013 (test: Descriptor roundtrip and raw()/ addr() coverage by pablomartin4btc)
#36005 (rpc, descriptor: warn when multipath expansion clones single-path key expressions by pythcoiner)
#35998 (wallet: Handle or explicitly ignore WalletBatch write failures by achow101)
#35760 (wallet: make corrupted transaction records fail wallet loading instead of forcing a rescan by achow101)
#35445 (wallet, descriptor: Revert StringType::COMPAT for Miniscript expressions and drop the concept of a Descriptor ID that can be validated by achow101)
#35440 (wallet: check descriptor cache xpub length before decoding by alhudz)
#35436 (wallet: Add addHDkey interface by pseudoramdom)
#35377 (wallet: Allow importing of descriptors without private keys when the wallet has the private keys by achow101)
#34909 (wallet, refactor: modularise wallet by extracting out legacy wallet migration by rkrux)
#34861 (wallet: Add importdescriptors interface by polespinasa)
#32861 (Have createwalletdescriptor auto-detect an unused(KEY) by Sjors)

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

std::make_unique<ConstPubkeyProvider>(state.key_exp_index, pubkey, false) in src/script/descriptor.cpp
std::make_unique<ConstPubkeyProvider>(state.key_exp_index, pubkey, true) in src/script/descriptor.cpp

2026-09-02 13:54:19

jeanpablojp commented at 2:53 PM on September 1, 2026: contributor

Concept ACK

The matching approaches in #36075 work off descriptors a wallet already has, and storing a record instead means wallets created before this get none on their own.

Those get no record, and createwalletdescriptor on a pair that exists just answers Descriptor already exists. You can still rebuild one by hand, dumping the private descriptor, turning /0/* into /<0;1>/* and re-importing, which stores exactly what the new code would have. That needs the xprv though, since the public form is rejected on a wallet with private keys. Is that the intended path for existing wallets?

in src/wallet/walletdb.cpp:955 in 0948b92435 outdated

 951 | +            if (!pwallet->GetScriptPubKeyMan(desc_id)) {
 952 | +                strErr = strprintf("Error: Multipath descriptor record references unknown descriptor id '%s' in wallet %s.", desc_id.ToString(), pwallet->GetName());
 953 | +                return DBErrors::CORRUPT;
 954 | +            }
 955 | +        }
 956 | +        pwallet->LoadMultipathDescriptor(std::move(multipath_desc));

jeanpablojp commented at 2:53 PM on September 1, 2026:

The two checks above reject a record whose id doesn't match its own string, and one that names a descriptor the wallet doesn't have. The string itself is never checked against those descriptors, so a record saying wpkh(record/<0;1>/*) next to an unrelated descriptor loads fine and both RPCs serve it. Is this deliberate?

Sjors commented at 4:13 PM on September 1, 2026:

IIUC you're asking if we should Parse() the multipath string at load time and verify that it expands as expected. That's more heavy-lifting than we typically do at wallet load.

It could make sense to introduce a bitcoin-wallet command to perform a more thorough integrity check..

in src/wallet/wallet.cpp:3897 in 0948b92435 outdated

3892 | +std::optional<std::string> CWallet::GetMultipathDescriptor(const uint256& desc_id) const
3893 | +{
3894 | +    AssertLockHeld(cs_wallet);
3895 | +    for (const auto& [id, multipath_desc] : m_multipath_descriptors) {
3896 | +        if (std::find(multipath_desc.desc_ids.begin(), multipath_desc.desc_ids.end(), desc_id) != multipath_desc.desc_ids.end()) {
3897 | +            return multipath_desc.descriptor;

jeanpablojp commented at 2:53 PM on September 1, 2026:

<0;1> and <0;2> are different policies that share their /0/* expansion, so one descriptor can sit in two records and this hands back whichever the map reaches first. Would it be worth returning all of them, or refusing the ambiguity?

Sjors commented at 4:05 PM on September 1, 2026:

It doesn't seem realistic for someone to import a <0;1> and <0;2> descriptor, so I'm not too worried about supporting that. Is there a more realistic scenario?

jeanpablojp commented at 6:27 PM on September 1, 2026:

Fair enough, I don't have a more realistic one. The idea was to avoid an arbitrary pick, but I take the point that it's over-protective for a case nobody hits.

Sjors commented at 6:31 PM on September 1, 2026:

This is now prevented. But what you can do is import a multipath descriptor into a pre-existing wallet that lacked it; this then adds the record. Not a documented feature, but it's tested.

in src/wallet/wallet.cpp:3655 in 0948b92435

3657 | +
3658 | +    // Store a record with the multipath descriptor, once the whole pair
3659 | +    // exists and if this call created at least one of its members
3660 | +    const bool pair_complete{GetScriptPubKeyMan(receive_id) && GetScriptPubKeyMan(change_id)};
3661 | +    if (!new_descs.empty() && pair_complete) {
3662 | +        AddMultipathDescriptor(batch, MultipathDescriptorRecord(std::move(*multipath_normalized), {receive_id, change_id}));

jeanpablojp commented at 2:53 PM on September 1, 2026:

The other two callers check this return, one warns and the other errors out. Dropped here, a failed write leaves the wallet created and reporting success with no record, and the transaction wrapping wallet setup never learns of it. Is this intended?

Sjors commented at 6:31 PM on September 1, 2026:

Added the missing handling and a [[nodiscard]].

in src/script/descriptor.cpp:3068 in 0948b92435 outdated

3065 | +                Assume(pos <= next_pos);
3066 | +                Assume(kr.text.size() <= next_pos - pos);
3067 | +                multipath_str.replace(pos, kr.text.size(), kr.replacement);
3068 | +                next_pos = pos;
3069 | +            }
3070 | +            *multipath = AddChecksum(multipath_str);

jeanpablojp commented at 2:53 PM on September 1, 2026:

The origin comes through as typed here when the key is an xpub, and normalized when it's the xprv, so desc and multipath can end up disagreeing on 84h versus 84' for the same key. Is this intended?

Sjors commented at 6:32 PM on September 1, 2026:

Good catch, fixed multipath normalization to always uses h.

Additionally, 620bd8ffa57dbda36d0e3a21d0eba952e2406e09 now uses h when expanding multipath, so the expanded descriptors are consistent with the parent.

Sjors commented at 4:00 PM on September 1, 2026: member

wallets created before this get none on their own.

Correct, I'll clarify in the description that this doesn't "upgrade" existing wallets. I think that's fine, because any multisig wallet using BIP388 registration is going to be new - since we don't support it yet.

since the public form is rejected on a wallet with private keys

See #35377.

Sjors force-pushed on Sep 1, 2026

DrahtBot added the label CI failed on Sep 1, 2026

DrahtBot commented at 7:37 PM on September 1, 2026: contributor

🚧 At least one of the CI tasks failed. Task macOS native, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/33544094392/job/99977225999 LLM reason (✨ experimental): Fuzz test failure: descriptor_parse hit an assertion in script/descriptor.cpp (canonical_result && sp_canonical.empty()) and exited with code 1.

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

achow101 commented at 2:36 AM on September 2, 2026: member

Concept NACKish

My suggestion was not to store an extra record, but rather an extra field in WalletDescriptor which contains the ids of the other descriptors in the multipath. Reconstructing the multipath can be done by doing a pairwise traversal in Descriptor::ToString(), i.e. traverse all of the descriptors at the same time, entering the sub descriptors and PubkeyProviders located at the same indexes. The only difference should be a single derivation path element, so it should not be that hard to figure out where the multipath is. I'm pretty sure it would be less code too to do that. Currently on vacation, but I can look at implementing this method once I'm back.

Additionally, multipath descriptors do not inherently have a concept of receive and change descriptors. They are not limited to 2 descriptors once expanded. There should be no mention of "receive" or "change" when discussing multipath descriptors in the code.

wallet: expand multipath descriptors with 'h' hardened marker

The descriptors that a multipath descriptor expands to are new, derived
descriptors rather than user input, so give them canonical form.

When a multipath descriptor contains an apostrophe, replace them all
with 'h' and parse again.

4431da11ac

refactor: use util::Expected for descriptor parsers

Use util::Expected for ParsePubkeyInner, ParsePubkey, and ParseScript, so errors are returned directly instead of passed through an output argument.

The only behavior change is that a preexisting error string is cleared when nested descriptor parsing fails without producing an error message. Previously, the stale error was left intact.

932b809ded

refactor: introduce descriptor ParseState

Group the key expression index and signing provider shared by ParsePubkeyInner, ParsePubkey, and ParseScript in ParseState.

A later commit extends this state with key replacement collection.

ae6c2b217a

descriptor: let Parse return the multipath descriptor string

Add an optional Parse() output for the unexpanded multipath descriptor. This is always a public descriptor, so ParseState optionally collects KeyReplacement entries tracking key locations in the input for private-to-public conversion.

The replacement sink is null unless the caller requests this output.

83e7b71488

refactor: extract LastHardenedIndex helper

Split finding the last hardened derivation step out of
BIP32PubkeyProvider::ToNormalizedString. No behavior change. A later
commit reuses it when normalizing the keys of a multipath descriptor
string.

4331511afb

refactor: extract OriginKeyString helper

Split formatting the normalized public form of a derived key, i.e. its
origin followed by the extended public key at the last hardened step,
out of BIP32PubkeyProvider::ToNormalizedString. No behavior change. A
later commit reuses it when normalizing the keys of a multipath
descriptor string.

ec970958b2

refactor: extract MergeNormalizedOrigin helper

Split merging an outer key origin into an already normalized key
expression out of OriginPubkeyProvider::ToNormalizedString. No behavior
change. A later commit reuses it when normalizing the keys of a
multipath descriptor string.

f44022a547

refactor: extract HardenedPrefix helper a14f99d53c

descriptor: normalize keys in the multipath descriptor string

Private extended keys followed by a fixed hardened prefix are now replaced by their key origin and the extended public key at the last hardened step, merging any explicit origin. This matches Descriptor::ToNormalizedString and produces BIP 388 wallet policy key expressions suitable for registering the multipath descriptor on a signing device.

A hardened multipath element or wildcard cannot be represented by a single publicly derivable multipath descriptor. Leave the Parse multipath output unset in those cases.

ParsePubkeyInner now returns an optional replacement alongside parsed providers so ParsePubkey can merge explicit key origin information before collecting it.

004fd02930

Sjors force-pushed on Sep 2, 2026

Sjors commented at 11:23 AM on September 2, 2026: member

IIUC you're objecting to two separate things:

Storing the multipath record. You prefer to reconstruct it on the fly.
The method of deriving the (normalized) multipath descriptor. You prefer to reconstruct it from individual descriptors, rather than have Parse yield it based on the original.

Given that we know the actual intended multipath descriptor at import (and wallet creation) time, it seems better to store it then. But doing so is indeed more involved, and storing the multipath becomes pointless if we do (1).

I'm pretty sure it would be less code too to do that.

The meat of the Parse change is not big, i.e. descriptor: let Parse return the multipath descriptor string and descriptor: normalize keys in the multipath descriptor string. Most of the churn in this PR comes from prep refactors.

The easiest implementation is just a regex. But having a mix of parser strategies in the already hard to understand Descriptor class, does not sound appealing.

My approach tries to stick to the existing recursive parser design.

Reconstructing the multipath can be done by doing a pairwise traversal [...] so it should not be that hard to figure out where the multipath is.

So we give a multipath descriptor to Parse, it gets split apart and then we reconstruct it? It feels wrong to throw away information this way, and essentially treat Parse as a black box. It's a sign we need to reconsider the entire Descriptor class.

I tried the approach in 2026/09/multipath-reconstruct, commit descriptor: reconstruct a multipath descriptor from its expansion 5694a99992fbe314779b1ce2c127dbcb50644025 (unpolished), with one additional prep refactor descriptor: let ToNormalizedString reject forms that are not publicly derivable fcd48f8a31e34bf6cbb653aeae98ce19d9573f7f. For that branch, I dropped the refactor commits it doesn't need, but I did keep the approach of storing a wallet record.

multipath descriptors do not inherently have a concept of receive and change descriptors.

Indeed, the Descriptor class should be agnostic, so I adjusted MultipathExtKeyNormalizationPrefix to handle more than two paths.

Also fixed the CI fuzzer failure.

wallet: store multipath descriptor record on import

When importdescriptors expands a BIP 389 multipath descriptor into
separate receive and change descriptors, additionally store a record
containing the multipath descriptor string, as reported by the parser
in public form, along with the IDs of the wallet descriptors it
expanded to.

Older wallet software ignores the new record.

7937add08d

rpc: return multipath descriptor from getaddressinfo

Add an optional multipath field to getaddressinfo, containing the
multipath descriptor that this address' parent descriptor was expanded
from, in public form.

11f73f315d

rpc: return multipath descriptor from listdescriptors

Add an optional multipath field to each listdescriptors entry that was
expanded from a multipath descriptor, containing the stored multipath
descriptor record. Every descriptor from the same multipath descriptor
returns the same value, always in public form.

df07c8aa0a

wallet: copy multipath record in exportwatchonlywallet

The descriptor IDs in the record are translated to those of the
watchonly wallet.

e6026977bc

refactor: setup wallet descriptors per output type pair

Add SetupDescriptorScriptPubKeyManPair, used by default wallet creation
and createwalletdescriptor, which derives the receive and change
descriptors of an output type from a single multipath descriptor
string, built by a new GenerateMultipathDescriptorString.

This commit does not change behavior.

722a77452b

wallet: store multipath descriptor record on wallet creation

When SetupDescriptorScriptPubKeyManPair creates a descriptor pair, store
a record with the normalized multipath descriptor they derive from.

When createwalletdescriptor is used with the internal option, the record
is stored by whichever call completes the pair.

1cdbfecb87

Sjors force-pushed on Sep 2, 2026

DrahtBot removed the label CI failed on Sep 2, 2026