bdk
bdk copied to clipboard
BDK Descriptor Improvements: Add multi descriptor capability in wallet
Description
In order for BDK to extend beyond just a Bitcoin wallet, it needs a more powerful descriptor capability. This project aims at improving the descriptors used in the library. Currently BDK uses only a single descriptor for a single wallet. Giving a new descriptor to BDK will create a brand new wallet.
The goal of this project is to create a wallet that can handle multiple descriptors within itself (a "super wallet"), and can send funds from any or all of them together.
Expected Outcomes
- Through understanding of descriptors.
- Basic understanding of the rust-miniscript library.
- Multi descriptor capability in BDK.
Resources
Skills Required
- Experience with git. Guide
- Basic familiarity with rust. First seven chapters of the book
- Familiarity with BDK’s descriptor usage.
Mentor(s)
Difficulty: Hard
Competency Test (optional)
- Install rust, compile and run all bdk examples and tests.
- Read through the BDK docs.
- Make a dummy wallet with BDK with different blockchain backends.
- Familiarity with basic rust, should be able to write basic custom trait implementations on foreign structures.
Note that the main goal for this task is not really to add more descriptors to the Wallet structure, but mainly to find a way to group multiple wallets into a "super wallet" that can operate on all of them at the same time, for instance by creating a transaction that spends utxos from all of them
Noted. Paraphrased the above in the description. Let me know if it can be elaborated more..
Note that the main goal for this task is not really to add more descriptors to the
Walletstructure, but mainly to find a way to group multiple wallets into a "super wallet" that can operate on all of them at the same time, for instance by creating a transaction that spends utxos from all of them
I think the simplest solution is create a API that allows you to create a PSBT using the databases of multiple wallets. Could even be a TxBuilder method add_utxos_from_wallet(wallet). Internally this could just take list_utxos do a weak version of add_foreign_utxo where the UTXO would be a non-mandatory spend. This would give you all the functionality that isn't available in BDK at the moment.
Of course this leaves you to manually add the balances of the individual wallets to find the joint balance etc. But more sophisticated things can be done later. In my mind wallets already have two descriptors with different semantics. I don't see why they couldn't have three or four or twenty four all in a Vec.
I think once we converge on an approach, the description can be elaborated further. I feel this will be a little difficult project for the students and they would need as much context as possible to tackle this issue.
Thanks for all the inputs @LLFourn @afilini , I will keep following this thread and update the project template accordingly.
In my mind wallets already have two descriptors with different semantics. I don't see why they couldn't have three or four or twenty four all in a Vec.
Yes, what worries me is how complex it can get to define all the different semantics for n descriptors inside a wallet. If this "super-wallet" thing turns out well we could also consider changing the wallets to only have one descriptor (which would simplify a ton of stuff) and then handling the semantics outside (for instance the super-wallet create_tx knows that it needs to generate the change address from one specific sub-wallet that is designated as the "internal" one).
I would be interested in having a go at this issue.
Per our discussion from bitcoindevkit/.github#51 the new bdk module now has a Wallet using the new KeychainTracker which can track keychains for multiple descriptors; it currently only uses two: External and optional Internal. My initial thought was to keep this as is to not complicate the bdk::Wallet API. But with prompting from @rajarshimaitra I've taken a deeper look and think supporting multiple descriptors in Wallet can simplify the wallet internals and improve the user facing API. The biggest down side I see is this is a fair bit of work, but if we're going to make major API changes now is the time.
Below is my high-level take on what would need to be done, what am I missing? Any reasons not to make this change?
Wallet
Becomes a collection of descriptor signers with corresponding keychain_tracker, persist, network, secp. Provides typical wallet functions across multiple descriptors for getting addresses, balances, listing transactions and utxos, creating new transactions via TxBuilder, and signing and finalizing PSBTs.
- add a new
Ktype param with traits needed byKeychainTracker's K type - remove
change_signersfield, only havesigners - modify
newto take aVecof tuple (K,E: IntoWalletDescriptor) - modify address functions to use a
Kparam to specify which keychain to use see #898 - modify balances, listing transactions and utxos functions to use optional
VecofKparam for filtered results - update examples for a simple
KeychainKind.ExternalandKeychainKind.InternaldescriptorWallet - add examples for a more than two descriptor
Wallet
TxBuilder
Create spending and fee bump transactions for a multi descriptor Wallet.
- add required
VecofKkeychains allowed to spend UTXOs from, with optional policy paths - add optional
VecofKkeychains not allowed to spend UTXOs from - add optional
Kkeychain to use for change output - remove
change_policy, it can be represented with above
Per my chat with @evanlinjin I've done some more digging into how the Core wallet works, in particular regarding privacy and multiple descriptors, see https://github.com/bitcoindevkit/bdk/issues/918#issuecomment-1491221046. I also want to incorporate @rajarshimaitra suggestion for keeping simplified wallet functions for users who only have external + (optional) internal descriptors. My original ideas above will need to be updated.
From the above research plus goal of making Wallet easier to use for the basic two descriptor wallet scenario I propose a few modifications to above:
Wallet
- add a
DefaultKeychainKindtrait requirement onKto set a keychain kind (External or Internal) and is_defaultbool - add address functions that take a script type and uses first default external keychain for that script type if one exists, if more than one take the first?
- add function to list all the wallet keychains
TxBuilder
- make the spend keychains optional, if none given use the default external and/or internal keychains of appropriate type (ie. avoid mixing different utxo output types), should probably be an error if none found
- don't remove
change_policysince it would still be needed to decide which kind of keychains to get utxos from - If no keychain specified for change use default internal keychain, if none then use first default external keychain, in either case use the appropriate type (ie. try to match tx change type output type to improve privacy)
Examples
The basic example should be setup similar to what Core does for a new single key descriptor wallet:
- 4 external default single key descriptors, one for each script type,
pkh(),sh(wpkh()),tr()andwpkh() - 4 internal default single descriptors, one for each script type
- get new deposit addresses from default external descriptor for requested script type
- get balance, list transactions and utxos for all wallet descriptors
- use appropriate internal descriptor for change, depending on the script type of the transaction output
An advanced example would be similar to above but also include using non-default descriptors. A scenario I'm thinking of is migrating utxos from an old wallet to a new wallet. We can do this by adding the old wallet descriptors as non-default descriptors to our new wallet. Over time old wallet utxos will be spent, but wallet continues to monitor old chains for any received utxos to old descriptors and spend them when needed with change back to new descriptors:
- 4 external default descriptors for new wallet, one for each script type
- 4 internal default descriptors for new wallet, one for each script type
- 4 external non-default descriptors for old wallet, one for each script type
- 4 internal non-default descriptors for old wallet, one for each script type
- get new deposit addresses only from new wallet external descriptors
- get balance, list transactions and utxos from old and new wallet descriptors
- spend utxos from all wallet keychains
- use only appropriate new wallet default internal descriptor for change, depending on the script type of the transaction output
EDIT: changed above examples from 3+3 to 4+4 descriptors per @vladimirfomene comment below.
From the above research plus goal of making
Walleteasier to use for the basic two descriptor wallet scenario I propose a few modifications to above:Wallet
1. add a `DefaultKeychainKind` trait requirement on `K` to set a keychain kind (External or Internal) and is_default `bool`
What's wrong with the usual Default trait here?
add a DefaultKeychainKind trait requirement on K to set a keychain kind (External or Internal) and is_default bool
I was thinking of
struct Wallet<D = (), K = KeychainKind>, In that way we can keep all the existing APIs in
impl <D : PeristentBackend> Wallet<D> {}
And all the new multi descriptor API in
impl<D: PersistentBackend, K: Debug + Clone + Ord> Wallet <D, K> {} scope.
In this way all the users who don't care about keychains and used bdk in previous way can use the Wallet::new() constructor without specifying anything related to keychains and the current implementation of new will take care of using the KeychainKind enum internally to manage the two keychains.
I think @vladimirfomene tried this pattern out and and it seemed to work. Vlad can you confirm on this?
1. add a `DefaultKeychainKind` trait requirement on `K` to set a keychain kind (External or Internal) and is_default `bool`What's wrong with the usual
Defaulttrait here?
I'm not thinking of a Default as in the default variant of K. A better name for the trait I have in mind is ActiveKeychainKind to filter K for ones that are "active" and "external" or "internal" when I need to pick a 'K' for new addresses.
If we don't want to impose any additional traits on K, the other way I can think to do it is for a non-default K, multi-descriptor Wallet (as @rajarshimaitra suggests above) we add a K param to the functions that need it, like getting a new receive address, or for change address in the TxBuilder.
Can we just make the user be explicit about which addresses they want rather than it being implicitly defined by a trait? It seems to me like the application should always know when requesting one. When creating a tx we could force the user to tell us about which keychain they want to use to derive a change address too.
add a DefaultKeychainKind trait requirement on K to set a keychain kind (External or Internal) and is_default bool
I was thinking of
struct Wallet<D = (), K = KeychainKind>, In that way we can keep all the existing APIs inimpl <D : PeristentBackend> Wallet<D> {}And all the new multi descriptor API in
impl<D: PersistentBackend, K: Debug + Clone + Ord> Wallet <D, K> {}scope.In this way all the users who don't care about keychains and used bdk in previous way can use the
Wallet::new()constructor without specifying anything related tokeychainsand the current implementation of new will take care of using theKeychainKindenum internally to manage the two keychains.I think @vladimirfomene tried this pattern out and and it seemed to work. Vlad can you confirm on this?
I find this strategy workable. Let me sync with Steve to better understand how he thinks about this.
Can we just make the user be explicit about which addresses they want rather than it being implicitly defined by a trait? It seems to me like the application should always know when requesting one. When creating a tx we could force the user to tell us about which keychain they want to use to derive a change address too.
If I understand correctly, that's exactly the plan. Force the user to specify a keychain to get addresses and create transactions from them. And this should not require any extra traits. We just need to manage the generics right while implementing multi-desc APIs, and provide a default for users who don't care about keychains/multi-descs.
Maybe eventually, we should not have the "default" wallet behavior and treat everything as multi-desc and always force users to specify keychains. But it will be a smoother transition to keep the default alive for now, as that makes BDK wallet really easy to spawn as a personal wallet setup.
I'm not thinking of a Default as in the default variant of K. A better name for the trait I have in mind is ActiveKeychainKind to filter K for ones that are "active" and "external" or "internal" when I need to pick a 'K' for new addresses.
I think this will be a pretty massive break for the API. The concept of keychain wasn't present before.
I think
- We can keep the original API without users ever knowing about keychains. Just internal/external apis that they used before.
- We can add the new APIs where users are forced to specify keychains, and can add their custom keychains of any kind that they like.
- All this while not creating another new trait.
Vlad is close to coming up with the concept code, (waiting on my review). At this point, it would be helpful to have the code to talk over.
Examples
The basic example should be setup similar to what Core does for a new single key descriptor wallet:
- 3 external default single key descriptors, one for each script type
- 3 internal default single descriptors, one for each script type
- get new deposit addresses from default external descriptor for requested script type
- get balance, list transactions and utxos for all wallet descriptors
- use appropriate internal descriptor for change, depending on the script type of the transaction output
@notmandatory , I think you meant to say 4 external and 4 internal default. The core wallet has 4 script types for its DescriptorScriptPubKeyMan. 3 descriptors is for the LegacyScriptPubKeyMan.
OK, I agree with above that we don't need any new trait on K, and should go with struct Wallet<D = (), K = KeychainKind> and impl <D : PeristentBackend> Wallet<D> {} with functions that let the users not worry about multiple descriptors. Then for impl<D: PersistentBackend, K: Debug + Clone + Ord> Wallet <D, K> {} have equivalent functions for advanced users where they will need to specify which K for some functions.
@notmandatory , I think you meant to say 4 external and 4 internal default. The core wallet has 4 script types for its DescriptorScriptPubKeyMan. 3 descriptors is for the LegacyScriptPubKeyMan.
@vladimirfomene ah yes, you are correct, I must have been looking at an old pre-TR PR. I did a test just now to confirm and for a new default descriptor wallet Core creates 8 descriptors, internal + external for: pkh(), sh(wpkh()), tr() and wpkh(). I'll edit my above comment example.
With @rajarshimaitra and @notmandatory, we have settled on an initial approach to implementing a multi-descriptor wallet in BDK. Here are the highlights:
- We will make the wallet struct generic over a certain
K(Khas to be Ord + Clone + Debug as the generic keychain identifier in TxoutIndex) and give it a default type ofKeychainKindso that we can have a default wallet that just supports two descriptors (internal and external). - The multi-descriptor wallet will take in a
Vec<K, E: IntoWalletDescriptor>in its constructor and initialize the wallet. - While spending with a multi-descriptor wallet, we need to specify the keychains we want to spend utxos from and also specify the keychain to use for change outputs. In a more advanced version of this wallet, you can give it a list of keychains for spending and a list of potential keychains to use for change. The wallet will be smart enough to pick which keychain to use for change and will do so by picking one that will help your wallet preserve its privacy. The wallet here will pick the keychain that will allow it produce a change output script that is same as the payment output script.