Merge bitcoindevkit/bdk#1662: fix(wallet): fix building change signers in load_with_params

b6b767f test(wallet): check keymaps can be set when loading wallet (valued mammal)
517fb53 fix(wallet): fix building change signers in `load_with_params` (valued mammal)

Pull request description:

  This fixes an issue that prevented change signers from being built when using the `keymap` method on `LoadParams`. Now we always build a `SignersContainer` with the internal keymap whenever a change descriptor is loaded. Note, the signer may still be "empty" if neither `keymap` or `extract_keys` is used.

  ### Notes to the reviewers

  Building a `SignersContainer` now happens outside of the [match statement](https://github.com/bitcoindevkit/bdk/blob/b6b767f3fc12062d6787eec8b3e1021458283985/crates/wallet/src/wallet/mod.rs#L557) in `load_with_params` which simplifies the logic such that we can interpret this area of code as "initialize a value for `change_descriptor` and optionally extend the internal keymap, returning a `LoadMismatch` error if the changeset doesn't match the expected params."

  ### Changelog notice

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [x] I've added tests to reproduce the issue which are now passing

ACKs for top commit:
  evanlinjin:
    ACK b6b767f

Tree-SHA512: fef2de691191e84f3e7cbe50c080cafb3b297d41e9bd493b97e45dfc1b3d77f4985c09b8fc3b398d54e22a43cf90501f6df70f8738e9b31a663efc0886f795e3

Author: evanlinjin

crates/wallet/src/wallet/mod.rs

@@ -551,7 +551,9 @@ impl Wallet {
         }
         let signers = Arc::new(SignersContainer::build(external_keymap, &descriptor, &secp));
 
-        let (mut change_descriptor, mut change_signers) = (None, Arc::new(SignersContainer::new()));
+        let mut change_descriptor = None;
+        let mut internal_keymap = params.change_descriptor_keymap;
+
         match (changeset.change_descriptor, params.check_change_descriptor) {
             // empty signer
             (None, None) => {}
@@ -592,17 +594,23 @@ impl Wallet {
                             expected: Some(exp_desc),
                         }));
                     }
-                    let mut internal_keymap = params.change_descriptor_keymap;
                     if params.extract_keys {
                         internal_keymap.extend(keymap);
                     }
-                    change_signers =
-                        Arc::new(SignersContainer::build(internal_keymap, &desc, &secp));
                     change_descriptor = Some(desc);
                 }
             },
         }
 
+        let change_signers = match change_descriptor {
+            Some(ref change_descriptor) => Arc::new(SignersContainer::build(
+                internal_keymap,
+                change_descriptor,
+                &secp,
+            )),
+            None => Arc::new(SignersContainer::new()),
+        };
+
         let index = create_indexer(descriptor, change_descriptor, params.lookahead)
             .map_err(LoadError::Descriptor)?;
 

crates/wallet/tests/wallet.rs

@@ -26,6 +26,7 @@ use bitcoin::{
     absolute, transaction, Address, Amount, BlockHash, FeeRate, Network, OutPoint, ScriptBuf,
     Sequence, Transaction, TxIn, TxOut, Txid, Weight,
 };
+use miniscript::{descriptor::KeyMap, Descriptor, DescriptorPublicKey};
 use rand::rngs::StdRng;
 use rand::SeedableRng;
 
@@ -99,6 +100,11 @@ fn insert_seen_at(wallet: &mut Wallet, txid: Txid, seen_at: u64) {
         .unwrap();
 }
 
+fn parse_descriptor(s: &str) -> (Descriptor<DescriptorPublicKey>, KeyMap) {
+    <Descriptor<DescriptorPublicKey>>::parse_descriptor(&Secp256k1::new(), s)
+        .expect("failed to parse descriptor")
+}
+
 // The satisfaction size of a P2WPKH is 112 WU =
 // 1 (elements in witness) + 1 (OP_PUSH) + 33 (pk) + 1 (OP_PUSH) + 72 (signature + sighash) + 1*4 (script len)
 // On the witness itself, we have to push once for the pk (33WU) and once for signature + sighash (72WU), for
@@ -251,7 +257,22 @@ fn wallet_load_checks() -> anyhow::Result<()> {
             ))),
             "unexpected descriptors check result",
         );
-
+        // check setting keymaps
+        let (_, external_keymap) = parse_descriptor(external_desc);
+        let (_, internal_keymap) = parse_descriptor(internal_desc);
+        let wallet = Wallet::load()
+            .keymap(KeychainKind::External, external_keymap)
+            .keymap(KeychainKind::Internal, internal_keymap)
+            .load_wallet(&mut open_db(&file_path)?)
+            .expect("db should not fail")
+            .expect("wallet was persisted");
+        for keychain in [KeychainKind::External, KeychainKind::Internal] {
+            let keymap = wallet.get_signers(keychain).as_key_map(wallet.secp_ctx());
+            assert!(
+                !keymap.is_empty(),
+                "load should populate keymap for keychain {keychain:?}"
+            );
+        }
         Ok(())
     }