# Bringing Multidescriptor Wallets to BDK

### High Level Overview
We mainly introduce a generic parameter `K` to `Wallet` so that it can handle custom keychains like in `bdk_chain`. This would allow the wallet to control multiple descriptors.
For users who want the current behaviour we make `KeychainKind` the default type.

#### New APIs
1. (as suggested by vladimirfomene and afilini) `balance_per_keychain`, `unspent_outputs_per_keychain`, `outputs_per_keychain`

### Changes in Detail

[wallet/mod.rs]
`Wallet` will now look like this:

pub struct Wallet<K=KeychainKind> {
    signers: Some_Map<K,Arc<SignersContainer>>,
    chain: LocalChain,
    indexed_graph: IndexedTxGraph<ConfirmationBlockTime, KeychainTxOutIndex<K>>,
    stage: ChangeSet<K>,
    network: Network,
    secp: SecpCtx,
}

`Update` will look like:

pub struct Update<K=KeychainKind> {
  
    pub last_active_indices: BTreeMap<K, u32>,

    
    pub tx_update: TxUpdate<ConfirmationBlockTime>,

    
    pub chain: Option<CheckPoint>,
}

`AddressInfo` will need to be changed:

pub struct AddressInfo<K=KeychainKind> {
    
    pub index: u32,
   
    pub address: Address,
    
    pub keychain: K,
}

pub enum LoadMismatch<K=KeychainKind> {
    <!-- as it is --!>,
    Descriptor {
        
        keychain: K,
        
        loaded: Option<ExtendedDescriptor>,
        
        expected: Option<ExtendedDescriptor>,
    },
}

[wallet/changeset.rs]
`ChangeSet` looks like:

pub struct ChangeSet<K=KeychainKind> {
   
    pub descs: Some_Map<K,Option<Descriptor<DescriptorPublicKey>>>,
    
    pub network: Option<bitcoin::Network>,
   
    pub local_chain: local_chain::ChangeSet,
    
    pub tx_graph: tx_graph::ChangeSet<ConfirmationBlockTime>,
    
    pub indexer: keychain_txout::ChangeSet,
}

- `merge`, `is_empty`: apply merge logic on each desc.
- persistence functions would need to be also modified in wallet/changeset.rs

[wallet/params.rs]
`CreateParams` becomes(we need a K to pass on information about K to `create_with_params`):

pub struct CreateParams<K=KeychainKind> {
    pub(crate) descs: Some_Map<K,DescriptorToExtract>,
    pub(crate) descs_keymaps: Some_Map<K,KeyMap>,
    pub(crate) network: Network,
    pub(crate) genesis_hash: Option<BlockHash>,
    pub(crate) lookahead: u32,
}

`LoadParams` also needs to be changed.

- `new_single` in wallet/params.rs can be removed now.
- `keymap` function needs to be changed

[wallet/types.rs]
`LocalOutput` changes

pub struct LocalOutput<K=KeychainKind> {
    
    pub outpoint: OutPoint,
    
    pub txout: TxOut,
    
    pub keychain: K,
    
    pub is_spent: bool,
    
    pub derivation_index: u32,
    
    pub chain_position: ChainPosition<ConfirmationBlockTime>,
}

[wallet/persisted.rs]
`WalletPersister` and `AsyncWalletPersister` also need to be changed since they all take/give `ChangeSet`. `PersistedWallet` also needs to take an additional generic parameter `K`. `FileStoreError` also changes.

[wallet/tx_builder.rs]
`TxBuilder` will be changed to:

pub struct TxBuilder<'a, Cs, K=KeychainKind> {
    pub(crate) wallet: &'a mut Wallet<K>,
    pub(crate) params: TxParams<K>,
    pub(crate) coin_selection: Cs,
}

pub(crate) struct TxParams<K=KeychainKind> {
    <!-- as it is --!>,
    pub(crate) policy_paths: Some_Map<K,Option<BTreeMap<String, Vec<usize>>>>,
    <!--as it is--!>,
    pub(crate) change_policy: ChangeSpendPolicy,
    <!--as it is--!>,,
}

- `policy_path` fn needs to be changed.

[wallet/mod.rs]
Now getting back to impl for `Wallet<K>` :
1. `new` would now take a vec of descriptors and intialize wallet.
2. `create_tx` would require user to enter the keychains to spend from and the keychain to be used for change.
3. we would not require `create_single` function now.
4. check each descriptor in `load_with_params`. Also create signer for each keychain in `load_with_params`. change_descriptor case to be removed.
5. `keychains` fn will also be changed.
7. `peek_address`, `reveal_addresses_to` , `reveal_next_address`, `list_unused_addresses`, `derivation_of_spk`, `list_unspent`, `list_output`, `all_unbounded_spk_iters`, `unbounded_spk_iter`, `get_utxo`, `balance`, `add_signer` would need to be changed.
8. `create_tx`, `build_fee_bump`, `policies`, `sign`, `derivation_index`, `next_derivation_index`.
9. `start_sync_with_revealed_spks`, `start_full_scan`, `wallet_name_from_descriptor`, `new_local_utxo`, `create_indexer`
10. update tests in wallet/mod.rs and tests directory
11. Some `Keychain` specific methods will return Options now simply because they might not have descriptors associated with them.
12. If the user provides no descriptor then missing descriptor error. Gives a descriptor without keychain then also error.

### Questions
1. Which map to use for signers field of wallet?
2. Can we add a mechanism to group keychains in the wallet?
3. What map should go in ChangeSet descs field?
4. Default, Deserialize and Serialize would need to be implemented for `ChangeSet<K>`?
5. What should be trust_predicate in balance now? (maybe take as input trusted keychains?)
6. How should ChangeSpendPolicy be changed?
7. Should we ask the user for a change keychain in create_tx ? We can put the argument as an option if not specified we do some default behaviour.
8. Do we enforce change descriptor to be different in case of multi descriptor( >= 2)?

### Proposed Timeline
Subject to change if go on implementing TxBuilder decoupling
Week-1: wallet/changeset.rs + docs
Week-2: wallet/params.rs + docs
Week-3: Breather
Week-4: wallet/types.rs + docs
Week-5: wallet/tx_builder.rs + docs
Week-6: Breather
Week-7 and 8: wallet/mod.rs + docs
Week-9 and 10: wallet/persisted.rs + docs

### Motivation
As mentioned in [#486](https://github.com/bitcoindevkit/bdk/issues/486#issue-1066860189) this will allow users to operate with multiple descriptors instead of just 2. IMO will also help in [#57](https://github.com/bitcoindevkit/bdk/issues/57#issue-700190681). It will also help users who just want the wallet to control a single descriptor. Related to privacy improvements in [#1824](https://github.com/bitcoindevkit/bdk/issues/1824#issue-2834036731).

### Downsides/Alternatives
This approach breaks almost every part of `bdk_wallet` but is much easier to execute than an alternative approach involving creating a multi-wallet struct which calls subwallets which are each single-descriptor.

### Attribution
This doc derives from work done and GitHub discussions by afilini([#121](https://github.com/bitcoindevkit/bdk/issues/121#issuecomment-717103337)), evanlinjin([#1539](https://github.com/bitcoindevkit/bdk/pull/1539), [#647](https://github.com/bitcoindevkit/bdk/pull/647)), rajarshimaitra([#966](https://github.com/bitcoindevkit/bdk/pull/966)), vladimirfomene([#486](https://github.com/bitcoindevkit/bdk/issues/486#issuecomment-1527426267))

### Notes
1. How can we still keep the old Wallet and its methods around? (maybe use default type for generic parameters and let arguments to functions be options so that default behaviour occurs if we don't specify them.)
2. There is plan to replace policy stuff with corresponding thing from rust-bitcoin
3. TxBuilder redesigning is also going on...
4. New API section needs more thought.
5. Decoupling of TxBuilder also needs to be thought about.
6. Feel like the changes would be easier if the `TxBuilder` is first made independant of the Wallet. Like what if we do the following and change functions in wallet/tx_builder.rs accordingly:

pub struct TxBuilder<'a, Cs, K=KeychainKind> {
    pub(crate) indexed_graph: &'a IndexedTxGraph<ConfirmationBlockTime, KeychainTxOutIndex<K>>
    pub(crate) params: TxParams<K>,
    pub(crate) coin_selection: Cs,
}

IMO only `add_utxos`, `coin_selection` and `finish_with_aux_rand` need to be changed for this.
should we pass a mutable ref to IndexedTxGraph instead?
7. Need more tests
8. Persistence part would need a lot of work! The whole persistence design would need to change!