Try   HackMD

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.
  6. 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.
  7. create_tx, build_fee_bump, policies, sign, derivation_index, next_derivation_index.
  8. start_sync_with_revealed_spks, start_full_scan, wallet_name_from_descriptor, new_local_utxo, create_indexer
  9. update tests in wallet/mod.rs and tests directory
  10. Some Keychain specific methods will return Options now simply because they might not have descriptors associated with them.
  11. 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 this will allow users to operate with multiple descriptors instead of just 2. IMO will also help in #57. It will also help users who just want the wallet to control a single descriptor. Related to privacy improvements in #1824.

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), evanlinjin(#1539, #647), rajarshimaitra(#966), vladimirfomene(#486)

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!

Last changed by 
 
codingp110
0
36
Published on HackMD