Feature/multi keychain take 3 by thunderbiscuit · Pull Request #524 · bitcoindevkit/bdk_wallet

thunderbiscuit · 2026-08-04T19:48:54Z

Take 3 on the multi-keychain PR feature. I'm opening this as a new PR in order to not lose all the work done in #318, since it's a fairly new/reimagined way to work it (2 PRs together, etc.). See #523 for part 1 of this.

Note that this is made against the pre-multi-keychain branch.

Quite experimental at this point, but I wanted to share where I'm at so others can build upon it.

TODO

I have removed the APIs related to multipath descriptors as they were focused on 2-path descriptors, but we should now be able to handle all multipath. I think this part needed a bit more thinking and I wanted to punt on it for now to focus on the core of the feature. Multipath is still supported, you simply have to "unpack" them before giving them to the KeyRing.
Figure out how to deal with change in the Wallet::create_psbt API. To keep things simple here I require users always set the keychain explicitly, but it's the sort of thing that users would probably love to be able to set once and forget. The example of wallets that use 2 keychains and one of those is the change keychain is the most obvious one.
One of the issues left is the Wallet::balance method not being able to tell what's a change keychain and what's not, and so the concept of "trusted" is a bit lost in the current API (all defaults to untrusted until the UTXO is mined). But this is I believe fixed by refactor(wallet): derive balance trust from output ancestry #431 and would land hopefully before this, and so I choose to let this one sit as is for now.

Changelog notice

TODO

Before submitting

I followed the contribution guidelines
This PR breaks the existing API

codecov · 2026-08-04T19:50:45Z

Codecov Report

❌ Patch coverage is 79.74684% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.20%. Comparing base (9ad7073) to head (eaa0946).

Files with missing lines	Patch %	Lines
src/wallet/persisted.rs	58.82%	14 Missing ⚠️
src/keyring/error.rs	23.07%	10 Missing ⚠️
src/wallet/params.rs	50.00%	8 Missing ⚠️
src/types.rs	68.42%	6 Missing ⚠️
src/wallet/error.rs	0.00%	6 Missing ⚠️
src/wallet/changeset.rs	94.44%	4 Missing ⚠️

Additional details and impacted files

@@                  Coverage Diff                   @@
##           pre-multi-keychain     #524      +/-   ##
======================================================
+ Coverage               81.79%   82.20%   +0.40%     
======================================================
  Files                      25       27       +2     
  Lines                    6362     6371       +9     
  Branches                  289      281       -8     
======================================================
+ Hits                     5204     5237      +33     
+ Misses                   1058     1038      -20     
+ Partials                  100       96       -4

Flag	Coverage Δ
rust	`82.20% <79.74%> (+0.40%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

thunderbiscuit · 2026-08-05T14:32:22Z

A few ideas for interesting helper features to add to the KeyRing:

A KeyRing::standard method that takes two descriptors and slots them right into KeychainKind::External and KeychainKind::Internal. This is basically allowing anyone who builds very run-of-the-mill wallets to do it quickly and simply, using the current approach of KeychainKind as their K keychain identifier.
A more elegant API for dealing with multipath descriptors. This has been a request for a while and we currently only have so-so methods to work with them, mostly because we only could work with 2 keychains at a time. With the multi-keychain paradigm, really any number of keychains that the multpath descriptors expand to can be added to the wallet. We just need a way to deal with this elegantly (and minimize footguns).

thunderbiscuit · 2026-08-05T17:12:05Z

TODO: Figure out how to deal with change in the Wallet::create_psbt API. To keep things simple here I require users always set the keychain explicitly, but it's the sort of thing that users would probably love to be able to set once and forget. The example of wallets that use 2 keychains and one of those is the change keychain is the most obvious of those, where clearly the user intends for one of those keychains to be used for change for all transactions.

My current naive idea to deal with this would be to add a field on the wallet change: K where users can define a keychain that the Wallet::create_psbt could always use in case no change keychain is set in the psbt creation.

@ValuedMammal happy to hear your thoughts on this.

One thought on this comment: by default the wallet would currently use UTXOs from all keychains to build transactions (maybe that needs work too; kind of like locking UTXOs, you might want to lock keychains or at least prevent the wallet mixing up all your keychains for privacy reasons), but in any case the base workflow would use all keychains. If each of those had defined a sister change keychain, then the wallet is back at square 1, not really knowing which one to use for change. Here again, explicitly setting the keychain to use for change is preferable IMO.

thunderbiscuit

Leaving here for review for tomorrow.

thunderbiscuit · 2026-08-05T17:18:18Z

+    ///
+    /// Two keychains sharing a descriptor cannot be told apart when attributing discovered
+    /// outputs, so the indexer rejects it.
+    DescriptorAlreadyAssigned(Box<Descriptor<DescriptorPublicKey>>),


Need to confirm where exactly this check will succeed/fail given corner cases, because I know we've discussed this in the past and I need a refresh on it. Posting here for self-review.

Won't we encounter this, if we try to add a descriptor to the keyring but it already exists under a different keychain?

Won't we encounter this, if we try to add a descriptor to the keyring but it already exists under a different keychain?

Yes and no. The equality check is not perfect if I remember correctly, and we ended up deciding that a hash of the first script derived by the descriptor was a more robust (if not perfect) way to check that two descriptors were basically "the same".

The wallet creation does that check using the descriptor_id() method (what I'm referring to above), so I'll use that instead of the equality used previously.

110CodingP · 2026-08-07T14:17:34Z

+        let tx = &psbt.unsigned_tx;
+        if psbt.inputs.len() < tx.input.len() {
+            return Err(IndexOutOfBoundsError::new(
+                psbt.inputs.len(),


tx.input.len() ?

110CodingP · 2026-08-07T14:18:20Z

        let index = &mut self.tx_graph.index;
        let stage = &mut self.stage;

        let ((index, spk), index_changeset) = index
-            .reveal_next_spk(keychain)
+            .reveal_next_spk(keychain.clone())
            .expect("keychain must exist");


We should return an error instead?

110CodingP · 2026-08-07T14:27:00Z

+            let psbt_input = &psbt
+                .inputs
+                .get(n)
+                .ok_or(IndexOutOfBoundsError::new(n, psbt.inputs.len()))?;


We have already ensured this won't happen?

110CodingP · 2026-08-07T17:11:16Z


-        fee_amount += fee_rate * tx.weight();
+    /// The derivation index of this wallet. It will return `None` if it has not derived any


We need to handle the case when keychain doesn't exist here too.

110CodingP · 2026-08-08T10:13:00Z

My current naive idea to deal with this would be to add a field on the wallet change: K where users can define a keychain that the Wallet::create_psbt could always use in case no change keychain is set in the psbt creation.

If convenience is the only reason for this change can we avoid persisting this? Can we have change_keychains: Vec<K>? The user could supply the value whenever we create/load the wallet. We could the choose the first one if we cannot decide which one to use? If the no change-keychain is specified and also no change_keychain is provided during tx-building we could throw up an error.

CHEF-SAVY · 2026-08-11T07:04:41Z

Run cargo audit locally to see the full details of the 4 securities issues.

thunderbiscuit · 2026-08-11T13:36:54Z

I'll follow up on the comments here in this thread! @ValuedMammal

The privacy focused approach is to choose the change keychain based on the output type of the outputs of the transaction to be created, not the keychain of the utxos pulled in as inputs.

You're correct I got this backwards there for a second.

the cleanest way to define the change keychain is at wallet creation time, because it's tied directly to a wallet descriptor. Supplying the change keychains another way means you'll have to handle a possibly non-existing keychain.

Yeah I see your point there. It conflicts with @110CodingP's idea above ("If convenience is the only reason for this change can we avoid persisting this?"), but if we really value this idea of "set it and forget it" then I think adding it to persistence could be worth the trouble.

On having multiple change keychains

I don't personally like the idea of defining more than 1 change keychain wallet-wide at the Wallet level (and even then! But I don't like change keychains at all to be honest so I understand I come at this a bit biased). Overall, I think it implies the wallet can do more than what I think it can reasonably do (picking correctly between multiple change is just a hard problem that's better handled outside of the wallet IMO, see next section). Defining them app-layer within your keychain identifiers K is of course perfect for this, and can be used powerfully.

I think a reasonable feature would be to be able to define a single global change keychain, which would come into play when going down the order of preference for change keychain selection in the psbt creation process as you defined in the linked post. This would also facilitate and keep simple the "basic case" of 2-keychain wallets as we have them now in Wallet.

Order of preference for change

So for PSBT creation the order of precedence is

User provided change-script/descriptor via the params

Auto derive from a change keychain the Wallet/KeyRing knows about

A third option could be a single globally configurable K value, but the real benefit of this comes from persisting it, so you can truly set it and forget it

Thanks for laying this out. I would do exactly this, but remove step 2. Or maybe I can be convinced, or it could come as a new feature at some point?

On choosing the change based on payment scripts

For me, this lives better at the application layer than the bdk_wallet layer. The basic privacy workflow is this:

Figure out what type of script I'm paying to
Pick a change script of the same type to ensure alll outputs are of the same script type

Attempting to do this on the bdk_wallet side is probably not impossible, but feels messy to me and very hard to do well, because the wallet would need to know so much about the user/app layer, i.e. carry a lot of metadata on all keychains and when to use which, which is exactly what the K approach is powerful for: letting the user hold keychain metadata with the descriptors, and leverage this for transaction building.

Even for the base case, the wallet would need to be able to parse the script type being paid to (what if it's a few different ones?), then see if it has keychains that are marked as change and have the same script type (again what if you pay to a tapscript address and all your 7 keychains are tapscript keychains?), and then pick one of those (which one? random? me don't like random). Again not impossible by any means, but this feels perfect for business-logic added on the application layer (at least to me at the moment).

thunderbiscuit · 2026-08-11T14:03:07Z

This was a long post but here is what I suggest I focus on for the next round of this:

Make sure the current 2-keychains External/Internal flow is clean and simple to use, with similar features to what we have now.
Keep complete flexibility for what I expect is more advanced + custom users of the library (those who define many keychains).

110CodingP · 2026-08-13T08:15:24Z

Do we want to allow the change_keychain or internal marker to be modified after wallet creation? Then this would also require a new field on the ChangeSet !

110CodingP · 2026-08-13T13:48:57Z

-/// change descriptor should be defined at wallet creation time and respected for the life of the
-/// wallet, meaning that if a change descriptor is originally defined, it must also be present in
-/// the aggregate change set.
+/// A keychain may be introduced by a later change set — a wallet can start tracking a new one at


Are we planning to allow addition of descriptors in bdk_wallet 4.0 itself?

110CodingP · 2026-08-13T17:34:34Z

-    pub change_descriptor: Option<Descriptor<DescriptorPublicKey>>,
+#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
+pub struct ChangeSet<K: Ord> {
+    /// The descriptor tracked by each keychain.


Mammal brought this up on #318 : Since we need to follow the ChangeSet's field-deprecation policy we would need to keep the change_descriptor and descriptor fields around for v4.

110CodingP · 2026-08-14T16:36:46Z

Also ig we could just let the user specify the change_keychain in this PR and have another PR (fixing the way user specifies the change keychain) when we have an agreement over the final API?

thunderbiscuit requested review from ValuedMammal and oleonardolima as code owners August 4, 2026 19:48

github-project-automation Bot added this to BDK Wallet Aug 4, 2026

thunderbiscuit force-pushed the feature/multi-keychain-take-3 branch from b2f34b5 to dd4d5d0 Compare August 5, 2026 15:30

feat(keyring): add KeyRing<K>

5ad0f13

thunderbiscuit force-pushed the feature/multi-keychain-take-3 branch from dd4d5d0 to 9e43360 Compare August 5, 2026 15:31

thunderbiscuit mentioned this pull request Aug 5, 2026

Pre multi-keychain work (do not merge) #523

Open

thunderbiscuit force-pushed the feature/multi-keychain-take-3 branch from c9f4922 to 09c9c5d Compare August 5, 2026 18:14

thunderbiscuit commented Aug 5, 2026

View reviewed changes

thunderbiscuit added 6 commits August 5, 2026 15:51

Add multi-keychain feature

13224ae

fix(keyring) relax KeyRingError bound from Display to Debug

6fafa92

feat(persist)! make the persisters generic over the keychain type

ceacfc4

docs(examples): add multi-keychain examples

5207231

feat(wallet)remove LoadParams::two_path_descriptor

76a0a9d

feat(wallet)! users must supply change keychain explicitly

eaa0946

thunderbiscuit force-pushed the feature/multi-keychain-take-3 branch from 09c9c5d to eaa0946 Compare August 5, 2026 19:52

110CodingP reviewed Aug 7, 2026

View reviewed changes

110CodingP mentioned this pull request Aug 8, 2026

Multi-Keychain-Redo 110CodingP/bdk_wallet#1

Closed

110CodingP reviewed Aug 13, 2026

View reviewed changes


		fee_amount += fee_rate * tx.weight();
		/// The derivation index of this wallet. It will return `None` if it has not derived any

Uh oh!

Conversation

thunderbiscuit commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Changelog notice

Before submitting

Uh oh!

codecov Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thunderbiscuit commented Aug 5, 2026

Uh oh!

thunderbiscuit commented Aug 5, 2026

Uh oh!

thunderbiscuit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

110CodingP Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

110CodingP commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CHEF-SAVY commented Aug 11, 2026

Uh oh!

thunderbiscuit commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

On having multiple change keychains

Order of preference for change

On choosing the change based on payment scripts

Uh oh!

thunderbiscuit commented Aug 11, 2026

Uh oh!

110CodingP commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

110CodingP commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thunderbiscuit commented Aug 4, 2026 •

edited

Loading

codecov Bot commented Aug 4, 2026 •

edited

Loading

110CodingP Aug 6, 2026 •

edited

Loading

110CodingP commented Aug 8, 2026 •

edited

Loading

thunderbiscuit commented Aug 11, 2026 •

edited

Loading

110CodingP commented Aug 13, 2026 •

edited

Loading

110CodingP commented Aug 14, 2026 •

edited

Loading