wip: Support N keychains

[ValuedMammal]

Description

This PR introduces initial support for multi-keychain wallets in bdk_wallet, allowing a single wallet instance to manage and track potentially many descriptors ("keychains"). This allows more complex/flexible wallet constructions in contrast to the current 2-keychain design.

Summary of changes

• New multi_keychain module:
  • Implements KeyRing for managing multiple descriptors.
  • Implements Wallet that can index tx data for multiple keychains.
  • Adds a new ChangeSet type for managing persistence.
  • Note that the keychain K is generic throughout the multi_keychain module, allowing the user to define the keychain type at wallet creation time, provided that K implements core::cmp::Ord.
• SQLite persistence for multi-keychain wallets:
  • Serialization and migration logic for saving/loading multi-keychain wallet state and descriptors using rusqlite. This is currently only supported for Wallet<DescriptorId>, i.e. wallets where the keychain has the concrete type DescriptorId.
• API surface:
  • Exposes multi_keychain module in crate root (lib.rs).
  • No breaking changes: All new features are additive; existing 2-keychain wallet functionality is preserved.

For example usage see wallet/examples/keyring.rs for a demo of how to set up and persist a multi-keychain wallet.

Note: This is a work in progress and feedback is welcome on the design and API.

[thunderbiscuit]

If you bring in the idea of the KeyRing into 3.0, then two things we could offer on it to enable ease-of-migration between major versions would be something like KeyRing::from_v2_changeset and KeyRing::into_v2_changeset.

[coveralls]

Pull Request Test Coverage Report for Build 15786383238

Details

• 0 of 367 (0.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-3.5%) to 82.058%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
wallet/src/multi_keychain/keyring.rs	0	63	0.0%
wallet/src/multi_keychain/changeset.rs	0	137	0.0%
wallet/src/multi_keychain/wallet.rs	0	167	0.0%

Totals
Change from base Build 15719064446:	-3.5%
Covered Lines:	7377
Relevant Lines:	8990

---

💛 - Coveralls

[thunderbiscuit]

Wow this is taking shape! Quick comments/questions as I'm looking at the diff (sorry if it's jumping the gun and you're actually in the middle of adding new stuff as I review).

1. I like the renaming of APIs like Wallet::peek_address(KeychainKind) to Wallet::peek_default_address() and Wallet::next_unused_address(KeychainKind) to Wallet::next_default_unused_address().
2. The idea of grouping the descriptors is potentially neat, but I would like to flesh out the use case in my mind a bit more because of all the complexity it potentially adds. Could that be used for APIs that would do something like "build me a transaction using keychain group 4"? If so, then the descriptors within-groups might also need default/change flags. Is there something else you'd use them for?
3. APIs like Wallet::mark_used(keychain: u32, index: u32) feel less clear to me at the moment. Did you prefer the u32 over the DescriptorId for a reason? Why the u32 instead of something like Wallet::mark_used(keychain: DescriptorId, index: u32)? Also, I see the group is a u32, so in this case would you not also need to specify the group + the keychain number (to uniquely identify a keychain)? Something like Wallet::mark_used(keychain_group: u32, keychain: u32, index: u32)? I might be misunderstanding something here.
4. I see the comment on the KeychainGroup type: Descriptors in the same group share certain characteristics such as the descriptor type. Is that something you'd like to enforce? I'm not sure if there would be usecases for them not being of the same type, but it really depends on how people would use the idea of "groups". I guess I'm not yet clear on the groups.

Some APIs I could see being useful on the KeyRing:

• KeyRing::get_default_keychain
• KeyRing::get_change_keychain
• KeyRing::new_from_multipath
• KeyRing::from_v1_changeset
• KeyRing::from_v2_changeset

[nymius]

Some thoughts I don't want to miss about this:

• We could enforce some expected format for descriptors to be accepted in a KeyRing. Mainly to avoid ambiguity and the aggregated complexity of many border cases. I would suggest to only consider descriptors that have wildcards. For cases like multipath, I would split the descriptor first into multiple descriptors.

• I would prefer a flat structure rather than nested. For the case of Internal descriptors, and thinking in terms of a table, I would add a field to link through the descriptor Id the one used for change. That field would be optional, if the descriptor isn't expected to create change, can be elided. Also, multiple keychains could refer to the same change keychain.

• Continuing with the idea of a flat structure, the external/internal consideration could be reduced to a boolean. If a keychain is linked to another keychain to use as change by the descriptor id, the second keychain should have the boolean field "external" enabled.

• I think the behavior desired by the use of group ids could also be achieved just by looking at the properties of the descriptor. Getting balance from all the descriptors of the same type should be easily achieved by filtering by their variant. In the case of multipath descriptors the balance can be obtained by filtering by the descriptor fingerprint. In more complex query cases that don't necessarily share any of the common traits a descriptor can have, I would leave the implementation for higher layers.

Thanks for working on this! Consider these comments as mere suggestions.

[ValuedMammal]

Last time I talked to thunderbiscuit we agreed on possibly doing away with the extra "grouping" logic, and for simplicity go back to using DescriptorId as the keychain. I'm also exploring the "clean break" approach with some sort of upgrade path.

[thunderbiscuit]

I've also been playing around with the "group" idea in my head and I can't see a super strong case for it yet. I think those kinds of grouping would be built at the application layer in a structure that would map descriptor ids to groups.

I suggest we build without, and bring them back if more review brings to light more information.

[thunderbiscuit]

One more thing that needs some hashing out on is the idea of keychain "pairs", which would provide you with a way to link two keychains together, one being the external and one being the internal.

The more I think about this one, the less hot I am on it. Here's a breakdown of my thoughts.

Pros + Use Cases

After thinking about this one for a while, I honestly can't write a good paragraph on it. Why would people need the pairs? Note that the idea that you might want to use the same script type for change as you do for the recipient (for privacy reasons) is valid, but is not actually enabled by the pairs idea! In fact, what's needed for this to work is for the TxBuilder to recognize what script type it is paying to when building a transaction, and then be able to choose from a pool of keychains one that will match the script type (i.e., if you are paying to a P2SH address, pick a P2SH keychain for producing the change outputs). The pairs don't have anything to do with this, and I myself was confusing the 2 ideas at first.

Reasons why I'm not so sure about keychain pairs

1. Trying to keep track of pairs adds a lot of complexity (at least in the naive implementations in my mind?) for what appears to be unuseful properties.
2. It's not general enough. For many applications using N-number of keychains (so people leveraging these new APIs) I think users will either want to provide their change keychain in a fully custom way (a sort of per-transaction keychain choice for change, probably leaning on some heuristics derived at the application layer), or users will want to always use the same change keychain (the simple wallet/happy path), for those that even use change keychains, which I now mostly discourage in my personal projects.
3. More than that, change keychains are a really specific corner case use case (you have to have a need to give out an entire keychain to an Electrum server? who does that? when? why?). In all other cases you should basically have only one keychain. Conversations with Peiter Wuille and Antoine Poinsot have convinced me that change keychains are mostly... not great, and legacy ideas not required in most modern wallets.
4. As mentioned in my section above, if we want the feature (avaialable in Core) where when you pay a BIP-44 (P2PKH) address you also automatically produce P2PKH change, this is different and requires we basically have APIs that recognize the type of address being paid to and can choose from a number of keychains the most appropriate one. This PR does not address that, but of course enables the development of such an API in the future as well.
5. Not having keychain pairs doesn't preclude any of the APIs one might want them for. It simply requires users provide custom arguments for keychains when required. This keeps the API focused (happy/simply path would use your default keychains—external + internal if you have defined one) while allowing for fully customizable transactions that leverage any keychains available in the keyring.
6. The idea of pairing keychain is still fully available at the application layer, where structures/logic can live to define maps/groups of metadata to keychains, including pairs of course, which can then be leveraged in the Wallet APIs.

Final thoughts

All this being said, I'm looking forward to hearing from users if I have maybe overlooked a use case for them at the Wallet layer; feel free to correct me on anything I might have missed.

[nymius]

I'm convinced by thunderbiscuit arguments, let's move this logic up to the application layer, and keep the interface simpler.

I still hold the following argument:

We could enforce some expected format for descriptors to be accepted in a KeyRing.
Mainly to avoid ambiguity and the aggregated complexity of many border cases.
I would suggest to only consider descriptors that have wildcards
For cases like multipath, I would split the descriptor first into multiple descriptors.

[notmandatory]

Here are my high-level goals for N keychain support (originally called multi-descriptor support in #188).

1. support importing/exporting bitcoincore wallets with bdk_wallet ( see importdescriptors, listdescriptors)
2. enable improved wallet privacy per Wallet Privacy Improvements #28
3. make it easier to migrate from old to new descriptors

[ValuedMammal]

Updated the PR description after pushing a new change adding the multi_keychain module.