fix(wallet)!: Change method LoadParams::descriptors

to just `descriptor` that takes a `KeychainKind`,
`D: IntoWalletDescriptor` and a bool indicating whether to add
a signer to the loaded wallet.

crates/wallet/README.md

@@ -79,7 +79,8 @@ let network = Network::Testnet;
 let descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/0/*)";
 let change_descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/1/*)";
 let wallet_opt = Wallet::load()
-    .descriptors(descriptor, change_descriptor)
+    .descriptor(KeychainKind::External, Some(descriptor), true)
+    .descriptor(KeychainKind::Internal, Some(change_descriptor), true)
     .network(network)
     .load_wallet(&mut db)
     .expect("wallet");

crates/wallet/src/wallet/mod.rs

@@ -246,9 +246,9 @@ pub enum LoadMismatch {
         /// Keychain identifying the descriptor.
         keychain: KeychainKind,
         /// The loaded descriptor.
-        loaded: ExtendedDescriptor,
+        loaded: Option<ExtendedDescriptor>,
         /// The expected descriptor.
-        expected: ExtendedDescriptor,
+        expected: Option<ExtendedDescriptor>,
     },
 }
 
@@ -401,11 +401,15 @@ impl Wallet {
         ));
 
         let (change_descriptor, change_signers) = match params.change_descriptor {
-            Some(desc_fn) => {
-                let (descriptor, mut keymap) = desc_fn(&secp, network)?;
-                keymap.extend(params.change_descriptor_keymap);
-                let change_signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp));
-                (Some(descriptor), change_signers)
+            Some(make_desc) => {
+                let (change_descriptor, mut internal_keymap) = make_desc(&secp, network)?;
+                internal_keymap.extend(params.change_descriptor_keymap);
+                let change_signers = Arc::new(SignersContainer::build(
+                    internal_keymap,
+                    &change_descriptor,
+                    &secp,
+                ));
+                (Some(change_descriptor), change_signers)
             }
             None => (None, Arc::new(SignersContainer::new())),
         };
@@ -442,7 +446,7 @@ impl Wallet {
     /// Note that the descriptor secret keys are not persisted to the db. You can either add
     /// signers after-the-fact with [`Wallet::add_signer`] or [`Wallet::set_keymap`]. Or you can
     /// add keys when building the wallet using [`LoadParams::keymap`] and/or
-    /// [`LoadParams::descriptors`].
+    /// [`LoadParams::descriptor`].
     ///
     /// # Synopsis
     ///
@@ -467,7 +471,8 @@ impl Wallet {
     /// let mut conn = bdk_wallet::rusqlite::Connection::open(file_path)?;
     /// let mut wallet = Wallet::load()
     ///     // check loaded descriptors matches these values and extract private keys
-    ///     .descriptors(EXTERNAL_DESC, INTERNAL_DESC)
+    ///     .descriptor(KeychainKind::External, Some(EXTERNAL_DESC), true)
+    ///     .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC), true)
     ///     // you can also manually add private keys
     ///     .keymap(KeychainKind::External, external_keymap)
     ///     .keymap(KeychainKind::Internal, internal_keymap)
@@ -499,42 +504,6 @@ impl Wallet {
         let chain = LocalChain::from_changeset(changeset.local_chain)
             .map_err(|_| LoadError::MissingGenesis)?;
 
-        let mut descriptor_keymap = params.descriptor_keymap;
-        let descriptor = changeset
-            .descriptor
-            .ok_or(LoadError::MissingDescriptor(KeychainKind::External))?;
-        check_wallet_descriptor(&descriptor).map_err(LoadError::Descriptor)?;
-
-        let (change_descriptor, change_signers) = match changeset.change_descriptor {
-            Some(change_descriptor) => {
-                check_wallet_descriptor(&change_descriptor).map_err(LoadError::Descriptor)?;
-                let mut change_descriptor_keymap = params.change_descriptor_keymap;
-
-                // check params match loaded
-                if let Some(exp_change_descriptor) = params.check_change_descriptor {
-                    let (exp_change_descriptor, keymap) =
-                        (exp_change_descriptor)(&secp, network).map_err(LoadError::Descriptor)?;
-                    change_descriptor_keymap.extend(keymap);
-
-                    if change_descriptor.descriptor_id() != exp_change_descriptor.descriptor_id() {
-                        return Err(LoadError::Mismatch(LoadMismatch::Descriptor {
-                            keychain: KeychainKind::Internal,
-                            loaded: change_descriptor,
-                            expected: exp_change_descriptor,
-                        }));
-                    }
-                }
-                let change_signers = Arc::new(SignersContainer::build(
-                    change_descriptor_keymap,
-                    &change_descriptor,
-                    &secp,
-                ));
-                (Some(change_descriptor), change_signers)
-            }
-            None => (None, Arc::new(SignersContainer::new())),
-        };
-
-        // checks
         if let Some(exp_network) = params.check_network {
             if network != exp_network {
                 return Err(LoadError::Mismatch(LoadMismatch::Network {
@@ -551,25 +520,80 @@ impl Wallet {
                 }));
             }
         }
-        if let Some(exp_descriptor) = params.check_descriptor {
-            let (exp_descriptor, keymap) =
-                (exp_descriptor)(&secp, network).map_err(LoadError::Descriptor)?;
-            descriptor_keymap.extend(keymap);
 
+        let descriptor = changeset
+            .descriptor
+            .ok_or(LoadError::MissingDescriptor(KeychainKind::External))?;
+        check_wallet_descriptor(&descriptor).map_err(LoadError::Descriptor)?;
+        let mut external_keymap = params.descriptor_keymap;
+
+        if let Some((make_desc, extract_keys)) = params.check_descriptor {
+            let (exp_descriptor, exp_keymap) =
+                make_desc(&secp, network).map_err(LoadError::Descriptor)?;
             if descriptor.descriptor_id() != exp_descriptor.descriptor_id() {
                 return Err(LoadError::Mismatch(LoadMismatch::Descriptor {
                     keychain: KeychainKind::External,
-                    loaded: descriptor,
-                    expected: exp_descriptor,
+                    loaded: Some(descriptor),
+                    expected: Some(exp_descriptor),
                 }));
             }
+            if extract_keys {
+                external_keymap.extend(exp_keymap);
+            }
         }
+        let signers = Arc::new(SignersContainer::build(external_keymap, &descriptor, &secp));
+
+        let (change_descriptor, change_signers) =
+            match (changeset.change_descriptor, params.check_change_descriptor) {
+                // Incorrectly specified `None` for existing change descriptor
+                (Some(cs_desc), None) => {
+                    return Err(LoadError::Mismatch(LoadMismatch::Descriptor {
+                        keychain: KeychainKind::Internal,
+                        loaded: Some(cs_desc),
+                        expected: None,
+                    }))
+                }
+                // Params must match loaded
+                (changeset_descriptor, Some((make_desc, extract_keys))) => {
+                    let (exp_change_descriptor, exp_keymap) =
+                        make_desc(&secp, network).map_err(LoadError::Descriptor)?;
+                    match changeset_descriptor {
+                        None => {
+                            return Err(LoadError::Mismatch(LoadMismatch::Descriptor {
+                                keychain: KeychainKind::Internal,
+                                loaded: None,
+                                expected: Some(exp_change_descriptor),
+                            }));
+                        }
+                        Some(change_descriptor) => {
+                            check_wallet_descriptor(&change_descriptor)
+                                .map_err(LoadError::Descriptor)?;
+                            if change_descriptor.descriptor_id()
+                                != exp_change_descriptor.descriptor_id()
+                            {
+                                return Err(LoadError::Mismatch(LoadMismatch::Descriptor {
+                                    keychain: KeychainKind::Internal,
+                                    loaded: Some(change_descriptor),
+                                    expected: Some(exp_change_descriptor),
+                                }));
+                            }
+                            let mut internal_keymap = params.change_descriptor_keymap;
+                            if extract_keys {
+                                internal_keymap.extend(exp_keymap);
+                            }
+                            let change_signers = Arc::new(SignersContainer::build(
+                                internal_keymap,
+                                &change_descriptor,
+                                &secp,
+                            ));
+                            (Some(change_descriptor), change_signers)
+                        }
+                    }
+                }
+                // Create empty change signer
+                (None, None) => (None, Arc::new(SignersContainer::new())),
+            };
 
-        let signers = Arc::new(SignersContainer::build(
-            descriptor_keymap,
-            &descriptor,
-            &secp,
-        ));
         let index = create_indexer(descriptor, change_descriptor, params.lookahead)
             .map_err(LoadError::Descriptor)?;
 

crates/wallet/src/wallet/params.rs

@@ -144,8 +144,8 @@ pub struct LoadParams {
     pub(crate) lookahead: u32,
     pub(crate) check_network: Option<Network>,
     pub(crate) check_genesis_hash: Option<BlockHash>,
-    pub(crate) check_descriptor: Option<DescriptorToExtract>,
-    pub(crate) check_change_descriptor: Option<DescriptorToExtract>,
+    pub(crate) check_descriptor: Option<(DescriptorToExtract, bool)>,
+    pub(crate) check_change_descriptor: Option<(DescriptorToExtract, bool)>,
 }
 
 impl LoadParams {
@@ -174,14 +174,28 @@ impl LoadParams {
         self
     }
 
-    /// Checks that `descriptor` of `keychain` matches this, and extracts private keys (if
-    /// available).
-    pub fn descriptors<D>(mut self, descriptor: D, change_descriptor: D) -> Self
+    /// Checks that `descriptor` matches exactly the one loaded for `keychain`. Setting
+    /// `extract_keys` to `true` will add a signer to the wallet if and only if the given
+    /// `descriptor` contains secrets.
+    pub fn descriptor<D>(
+        mut self,
+        keychain: KeychainKind,
+        descriptor: Option<D>,
+        extract_keys: bool,
+    ) -> Self
     where
         D: IntoWalletDescriptor + 'static,
     {
-        self.check_descriptor = Some(make_descriptor_to_extract(descriptor));
-        self.check_change_descriptor = Some(make_descriptor_to_extract(change_descriptor));
+        match keychain {
+            KeychainKind::External => {
+                self.check_descriptor =
+                    descriptor.map(|d| (make_descriptor_to_extract(d), extract_keys));
+            }
+            KeychainKind::Internal => {
+                self.check_change_descriptor =
+                    descriptor.map(|d| (make_descriptor_to_extract(d), extract_keys));
+            }
+        }
         self
     }
 

crates/wallet/tests/wallet.rs

@@ -137,7 +137,8 @@ fn wallet_is_persisted() -> anyhow::Result<()> {
         {
             let mut db = open_db(&file_path).context("failed to recover db")?;
             let wallet = Wallet::load()
-                .descriptors(external_desc, internal_desc)
+                .descriptor(KeychainKind::External, Some(external_desc), false)
+                .descriptor(KeychainKind::Internal, Some(internal_desc), false)
                 .network(Network::Testnet)
                 .load_wallet(&mut db)?
                 .expect("wallet must exist");
@@ -240,7 +241,16 @@ fn wallet_load_checks() -> anyhow::Result<()> {
         );
         assert_matches!(
             Wallet::load()
-                .descriptors(internal_desc, external_desc)
+                .descriptor(KeychainKind::External, Some(internal_desc), false)
+                .load_wallet(&mut open_db(&file_path)?),
+            Err(LoadWithPersistError::InvalidChangeSet(LoadError::Mismatch(
+                LoadMismatch::Descriptor { .. }
+            ))),
+            "unexpected descriptors check result",
+        );
+        assert_matches!(
+            Wallet::load()
+                .descriptor(KeychainKind::External, Option::<&str>::None, false)
                 .load_wallet(&mut open_db(&file_path)?),
             Err(LoadWithPersistError::InvalidChangeSet(LoadError::Mismatch(
                 LoadMismatch::Descriptor { .. }
@@ -279,25 +289,37 @@ fn single_descriptor_wallet_persist_and_recover() {
         .network(Network::Testnet)
         .create_wallet(&mut db)
         .unwrap();
-
     let _ = wallet.reveal_addresses_to(KeychainKind::External, 2);
     assert!(wallet.persist(&mut db).unwrap());
 
     // should recover persisted wallet
     let secp = wallet.secp_ctx();
     let (_, keymap) = <Descriptor<DescriptorPublicKey>>::parse_descriptor(secp, desc).unwrap();
-    let wallet = LoadParams::new()
-        .keymap(KeychainKind::External, keymap.clone())
+    assert!(!keymap.is_empty());
+    let wallet = Wallet::load()
+        .descriptor(KeychainKind::External, Some(desc), true)
         .load_wallet(&mut db)
         .unwrap()
         .expect("must have loaded changeset");
-
     assert_eq!(wallet.derivation_index(KeychainKind::External), Some(2));
     // should have private key
     assert_eq!(
         wallet.get_signers(KeychainKind::External).as_key_map(secp),
         keymap,
     );
+
+    // should error on wrong internal params
+    let desc = get_test_wpkh();
+    let (exp_desc, _) = <Descriptor<DescriptorPublicKey>>::parse_descriptor(secp, desc).unwrap();
+    let err = Wallet::load()
+        .descriptor(KeychainKind::Internal, Some(desc), true)
+        .load_wallet(&mut db);
+    assert_matches!(
+        err,
+        Err(LoadWithPersistError::InvalidChangeSet(LoadError::Mismatch(LoadMismatch::Descriptor { keychain, loaded, expected })))
+        if keychain == KeychainKind::Internal && loaded == None && expected == Some(exp_desc),
+        "single descriptor wallet should refuse change descriptor param"
+    );
 }
 
 #[test]

example-crates/wallet_electrum/src/main.rs

@@ -26,7 +26,8 @@ fn main() -> Result<(), anyhow::Error> {
     let mut db = Store::<bdk_wallet::ChangeSet>::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?;
 
     let wallet_opt = Wallet::load()
-        .descriptors(EXTERNAL_DESC, INTERNAL_DESC)
+        .descriptor(KeychainKind::External, Some(EXTERNAL_DESC), true)
+        .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC), true)
         .network(NETWORK)
         .load_wallet(&mut db)?;
     let mut wallet = match wallet_opt {

example-crates/wallet_esplora_async/src/main.rs

@@ -23,7 +23,8 @@ async fn main() -> Result<(), anyhow::Error> {
     let mut conn = Connection::open(DB_PATH)?;
 
     let wallet_opt = Wallet::load()
-        .descriptors(EXTERNAL_DESC, INTERNAL_DESC)
+        .descriptor(KeychainKind::External, Some(EXTERNAL_DESC), true)
+        .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC), true)
         .network(NETWORK)
         .load_wallet(&mut conn)?;
     let mut wallet = match wallet_opt {

example-crates/wallet_esplora_blocking/src/main.rs

@@ -22,7 +22,8 @@ fn main() -> Result<(), anyhow::Error> {
     let mut db = Store::<bdk_wallet::ChangeSet>::open_or_create_new(DB_MAGIC.as_bytes(), DB_PATH)?;
 
     let wallet_opt = Wallet::load()
-        .descriptors(EXTERNAL_DESC, INTERNAL_DESC)
+        .descriptor(KeychainKind::External, Some(EXTERNAL_DESC), true)
+        .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC), true)
         .network(NETWORK)
         .load_wallet(&mut db)?;
     let mut wallet = match wallet_opt {

example-crates/wallet_rpc/src/main.rs

@@ -5,7 +5,7 @@ use bdk_bitcoind_rpc::{
 use bdk_wallet::{
     bitcoin::{Block, Network, Transaction},
     file_store::Store,
-    Wallet,
+    KeychainKind, Wallet,
 };
 use clap::{self, Parser};
 use std::{path::PathBuf, sync::mpsc::sync_channel, thread::spawn, time::Instant};
@@ -89,7 +89,12 @@ fn main() -> anyhow::Result<()> {
     let mut db =
         Store::<bdk_wallet::ChangeSet>::open_or_create_new(DB_MAGIC.as_bytes(), args.db_path)?;
     let wallet_opt = Wallet::load()
-        .descriptors(args.descriptor.clone(), args.change_descriptor.clone())
+        .descriptor(KeychainKind::External, Some(args.descriptor.clone()), true)
+        .descriptor(
+            KeychainKind::Internal,
+            Some(args.change_descriptor.clone()),
+            true,
+        )
         .network(args.network)
         .load_wallet(&mut db)?;
     let mut wallet = match wallet_opt {