From 517fb53760626ada00551716fd35dbd84ed6d48e Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 22 Oct 2024 09:12:32 -0400 Subject: [PATCH 1/2] fix(wallet): fix building change signers in `load_with_params` 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, although the signer may still be empty if neither `keymap` nor `extract_keys` is used. --- crates/wallet/src/wallet/mod.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 562e684c1..58b504dc9 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/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)?; From b6b767f3fc12062d6787eec8b3e1021458283985 Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 22 Oct 2024 10:29:07 -0400 Subject: [PATCH 2/2] test(wallet): check keymaps can be set when loading wallet --- crates/wallet/tests/wallet.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 44b194a83..d51af3352 100644 --- a/crates/wallet/tests/wallet.rs +++ b/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, KeyMap) { + >::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(()) }