From 75155b7dc0bb7dfee9046d323fb87b0f8e0321c1 Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 6 Aug 2024 11:52:31 -0400 Subject: [PATCH 1/6] feat(wallet): Add method `Wallet::create_single` that allows creating a wallet with a single descriptor. --- crates/wallet/src/wallet/mod.rs | 238 +++++++++++++++++-------- crates/wallet/src/wallet/params.rs | 33 +++- crates/wallet/src/wallet/tx_builder.rs | 9 +- 3 files changed, 197 insertions(+), 83 deletions(-) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 7217d32fb..37dc57ee4 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -298,6 +298,53 @@ impl fmt::Display for ApplyBlockError { impl std::error::Error for ApplyBlockError {} impl Wallet { + /// Build a new single descriptor [`Wallet`]. + /// + /// If you have previously created a wallet, use [`load`](Self::load) instead. + /// + /// # Note + /// + /// Only use this method when creating a wallet designed to be used with a single + /// descriptor and keychain. Otherwise the recommended way to construct a new wallet is + /// by using [`Wallet::create`]. It's worth noting that not all features are available + /// with single descriptor wallets, for example setting a [`change_policy`] on [`TxBuilder`] + /// and related methods such as [`do_not_spend_change`]. This is because all payments are + /// received on the external keychain (including change), and without a change keychain + /// BDK lacks enough information to distinguish between change and outside payments. + /// + /// Additionally because this wallet has no internal (change) keychain, all methods that + /// require a [`KeychainKind`] as input, e.g. [`reveal_next_address`] should only be called + /// using the [`External`] variant. In most cases passing [`Internal`] is treated as the + /// equivalent of [`External`] but can lead to confusing results. + /// + /// # Example + /// + /// ```rust + /// # use bdk_wallet::Wallet; + /// # use bitcoin::Network; + /// # const EXTERNAL_DESC: &str = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; + /// # let temp_dir = tempfile::tempdir().expect("must create tempdir"); + /// # let file_path = temp_dir.path().join("store.db"); + /// // Create a wallet that is persisted to SQLite database. + /// use bdk_wallet::rusqlite::Connection; + /// let mut conn = Connection::open(file_path)?; + /// let wallet = Wallet::create_single(EXTERNAL_DESC) + /// .network(Network::Testnet) + /// .create_wallet(&mut conn)?; + /// # Ok::<_, anyhow::Error>(()) + /// ``` + /// [`change_policy`]: TxBuilder::change_policy + /// [`do_not_spend_change`]: TxBuilder::do_not_spend_change + /// [`External`]: KeychainKind::External + /// [`Internal`]: KeychainKind::Internal + /// [`reveal_next_address`]: Self::reveal_next_address + pub fn create_single(descriptor: D) -> CreateParams + where + D: IntoWalletDescriptor + Clone + 'static, + { + CreateParams::new_single(descriptor) + } + /// Build a new [`Wallet`]. /// /// If you have previously created a wallet, use [`load`](Self::load) instead. @@ -347,20 +394,22 @@ impl Wallet { let (descriptor, mut descriptor_keymap) = (params.descriptor)(&secp, network)?; descriptor_keymap.extend(params.descriptor_keymap); - let (change_descriptor, mut change_descriptor_keymap) = - (params.change_descriptor)(&secp, network)?; - change_descriptor_keymap.extend(params.change_descriptor_keymap); - let signers = Arc::new(SignersContainer::build( descriptor_keymap, &descriptor, &secp, )); - let change_signers = Arc::new(SignersContainer::build( - change_descriptor_keymap, - &change_descriptor, - &secp, - )); + + 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) + } + None => (None, Arc::new(SignersContainer::new())), + }; + let index = create_indexer(descriptor, change_descriptor, params.lookahead)?; let descriptor = index.get_descriptor(KeychainKind::External).cloned(); @@ -456,11 +505,34 @@ impl Wallet { .ok_or(LoadError::MissingDescriptor(KeychainKind::External))?; check_wallet_descriptor(&descriptor).map_err(LoadError::Descriptor)?; - let mut change_descriptor_keymap = params.change_descriptor_keymap; - let change_descriptor = changeset - .change_descriptor - .ok_or(LoadError::MissingDescriptor(KeychainKind::Internal))?; - check_wallet_descriptor(&change_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 { @@ -492,30 +564,12 @@ impl Wallet { })); } } - 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 signers = Arc::new(SignersContainer::build( descriptor_keymap, &descriptor, &secp, )); - let change_signers = Arc::new(SignersContainer::build( - change_descriptor_keymap, - &change_descriptor, - &secp, - )); let index = create_indexer(descriptor, change_descriptor, params.lookahead) .map_err(LoadError::Descriptor)?; @@ -555,6 +609,7 @@ impl Wallet { /// This panics when the caller requests for an address of derivation index greater than the /// [BIP32](https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki) max index. pub fn peek_address(&self, keychain: KeychainKind, mut index: u32) -> AddressInfo { + let keychain = self.map_keychain(keychain); let mut spk_iter = self .indexed_graph .index @@ -600,6 +655,7 @@ impl Wallet { /// # Ok::<(), anyhow::Error>(()) /// ``` pub fn reveal_next_address(&mut self, keychain: KeychainKind) -> AddressInfo { + let keychain = self.map_keychain(keychain); let index = &mut self.indexed_graph.index; let stage = &mut self.stage; @@ -631,6 +687,7 @@ impl Wallet { keychain: KeychainKind, index: u32, ) -> impl Iterator + '_ { + let keychain = self.map_keychain(keychain); let (spks, index_changeset) = self .indexed_graph .index @@ -655,6 +712,7 @@ impl Wallet { /// **WARNING**: To avoid address reuse you must persist the changes resulting from one or more /// calls to this method before closing the wallet. See [`Wallet::reveal_next_address`]. pub fn next_unused_address(&mut self, keychain: KeychainKind) -> AddressInfo { + let keychain = self.map_keychain(keychain); let index = &mut self.indexed_graph.index; let ((index, spk), index_changeset) = index @@ -702,7 +760,7 @@ impl Wallet { ) -> impl DoubleEndedIterator + '_ { self.indexed_graph .index - .unused_keychain_spks(keychain) + .unused_keychain_spks(self.map_keychain(keychain)) .map(move |(index, spk)| AddressInfo { index, address: Address::from_script(spk.as_script(), self.network) @@ -784,7 +842,7 @@ impl Wallet { ) -> impl Iterator> + Clone { self.indexed_graph .index - .unbounded_spk_iter(keychain) + .unbounded_spk_iter(self.map_keychain(keychain)) .expect("keychain must exist") } @@ -1053,17 +1111,17 @@ impl Wallet { } /// Set the keymap for a given keychain. + /// + /// Note this does nothing if the given keychain has no descriptor because we won't + /// know the context (segwit, taproot, etc) in which to create signatures. pub fn set_keymap(&mut self, keychain: KeychainKind, keymap: KeyMap) { let wallet_signers = match keychain { KeychainKind::External => Arc::make_mut(&mut self.signers), KeychainKind::Internal => Arc::make_mut(&mut self.change_signers), }; - let descriptor = self - .indexed_graph - .index - .get_descriptor(keychain) - .expect("keychain must exist"); - *wallet_signers = SignersContainer::build(keymap, descriptor, &self.secp); + if let Some(descriptor) = self.indexed_graph.index.get_descriptor(keychain) { + *wallet_signers = SignersContainer::build(keymap, descriptor, &self.secp) + } } /// Set the keymap for each keychain. @@ -1143,14 +1201,19 @@ impl Wallet { ) -> Result { let keychains: BTreeMap<_, _> = self.indexed_graph.index.keychains().collect(); let external_descriptor = keychains.get(&KeychainKind::External).expect("must exist"); - let internal_descriptor = keychains.get(&KeychainKind::Internal).expect("must exist"); + let internal_descriptor = keychains.get(&KeychainKind::Internal); let external_policy = external_descriptor .extract_policy(&self.signers, BuildSatisfaction::None, &self.secp)? .unwrap(); let internal_policy = internal_descriptor - .extract_policy(&self.change_signers, BuildSatisfaction::None, &self.secp)? - .unwrap(); + .map(|desc| { + Ok::<_, CreateTxError>( + desc.extract_policy(&self.change_signers, BuildSatisfaction::None, &self.secp)? + .unwrap(), + ) + }) + .transpose()?; // The policy allows spending external outputs, but it requires a policy path that hasn't been // provided @@ -1163,14 +1226,16 @@ impl Wallet { )); }; // Same for the internal_policy path - if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeForbidden - && internal_policy.requires_path() - && params.internal_policy_path.is_none() - { - return Err(CreateTxError::SpendingPolicyRequired( - KeychainKind::Internal, - )); - }; + if let Some(internal_policy) = &internal_policy { + if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeForbidden + && internal_policy.requires_path() + && params.internal_policy_path.is_none() + { + return Err(CreateTxError::SpendingPolicyRequired( + KeychainKind::Internal, + )); + }; + } let external_requirements = external_policy.get_condition( params @@ -1178,14 +1243,21 @@ impl Wallet { .as_ref() .unwrap_or(&BTreeMap::new()), )?; - let internal_requirements = internal_policy.get_condition( - params - .internal_policy_path - .as_ref() - .unwrap_or(&BTreeMap::new()), - )?; + let internal_requirements = internal_policy + .map(|policy| { + Ok::<_, CreateTxError>( + policy.get_condition( + params + .internal_policy_path + .as_ref() + .unwrap_or(&BTreeMap::new()), + )?, + ) + }) + .transpose()?; - let requirements = external_requirements.merge(&internal_requirements)?; + let requirements = + external_requirements.merge(&internal_requirements.unwrap_or_default())?; let version = match params.version { Some(tx_builder::Version(0)) => return Err(CreateTxError::Version0), @@ -1351,7 +1423,7 @@ impl Wallet { let drain_script = match params.drain_to { Some(ref drain_recipient) => drain_recipient.clone(), None => { - let change_keychain = KeychainKind::Internal; + let change_keychain = self.map_keychain(KeychainKind::Internal); let ((index, spk), index_changeset) = self .indexed_graph .index @@ -1728,14 +1800,14 @@ impl Wallet { } /// Returns the descriptor used to create addresses for a particular `keychain`. + /// /// It's the "public" version of the wallet's descriptor, meaning a new descriptor that has /// the same structure but with the all secret keys replaced by their corresponding public key. - /// /// This can be used to build a watch-only version of a wallet. pub fn public_descriptor(&self, keychain: KeychainKind) -> &ExtendedDescriptor { self.indexed_graph .index - .get_descriptor(keychain) + .get_descriptor(self.map_keychain(keychain)) .expect("keychain must exist") } @@ -1851,7 +1923,7 @@ impl Wallet { pub fn next_derivation_index(&self, keychain: KeychainKind) -> u32 { self.indexed_graph .index - .next_index(keychain) + .next_index(self.map_keychain(keychain)) .expect("keychain must exist") .0 } @@ -1977,7 +2049,7 @@ impl Wallet { let mut i = 0; may_spend.retain(|u| { - let retain = change_policy.is_satisfied_by(&u.0) + let retain = (self.keychains().count() == 1 || change_policy.is_satisfied_by(&u.0)) && !unspendable.contains(&u.0.outpoint) && satisfies_confirmed[i]; i += 1; @@ -2310,6 +2382,18 @@ impl Wallet { .batch_insert_relevant_unconfirmed(unconfirmed_txs); self.stage.merge(indexed_graph_changeset.into()); } + + /// Used internally to ensure that all methods requiring a [`KeychainKind`] will use a + /// keychain with an associated descriptor. For example in case the wallet was created + /// with only one keychain, passing [`KeychainKind::Internal`] here will instead return + /// [`KeychainKind::External`]. + fn map_keychain(&self, keychain: KeychainKind) -> KeychainKind { + if self.keychains().count() == 1 { + KeychainKind::External + } else { + keychain + } + } } /// Methods to construct sync/full-scan requests for spk-based chain sources. @@ -2389,7 +2473,7 @@ fn new_local_utxo( fn create_indexer( descriptor: ExtendedDescriptor, - change_descriptor: ExtendedDescriptor, + change_descriptor: Option, lookahead: u32, ) -> Result, DescriptorError> { let mut indexer = KeychainTxOutIndex::::new(lookahead); @@ -2402,19 +2486,21 @@ fn create_indexer( // let (descriptor, keymap) = change_descriptor; // let change_signers = Arc::new(SignersContainer::build(keymap, &descriptor, secp)); - assert!(indexer - .insert_descriptor(KeychainKind::Internal, change_descriptor) - .map_err(|e| { - use bdk_chain::indexer::keychain_txout::InsertDescriptorError; - match e { - InsertDescriptorError::DescriptorAlreadyAssigned { .. } => { - crate::descriptor::error::Error::ExternalAndInternalAreTheSame - } - InsertDescriptorError::KeychainAlreadyAssigned { .. } => { - unreachable!("this is the first time we're assigning internal") + if let Some(change_descriptor) = change_descriptor { + assert!(indexer + .insert_descriptor(KeychainKind::Internal, change_descriptor) + .map_err(|e| { + use bdk_chain::indexer::keychain_txout::InsertDescriptorError; + match e { + InsertDescriptorError::DescriptorAlreadyAssigned { .. } => { + crate::descriptor::error::Error::ExternalAndInternalAreTheSame + } + InsertDescriptorError::KeychainAlreadyAssigned { .. } => { + unreachable!("this is the first time we're assigning internal") + } } - } - })?); + })?); + } Ok(indexer) } diff --git a/crates/wallet/src/wallet/params.rs b/crates/wallet/src/wallet/params.rs index 44d0d1db1..62d314ba9 100644 --- a/crates/wallet/src/wallet/params.rs +++ b/crates/wallet/src/wallet/params.rs @@ -32,7 +32,7 @@ where pub struct CreateParams { pub(crate) descriptor: DescriptorToExtract, pub(crate) descriptor_keymap: KeyMap, - pub(crate) change_descriptor: DescriptorToExtract, + pub(crate) change_descriptor: Option, pub(crate) change_descriptor_keymap: KeyMap, pub(crate) network: Network, pub(crate) genesis_hash: Option, @@ -40,14 +40,39 @@ pub struct CreateParams { } impl CreateParams { - /// Construct parameters with provided `descriptor`, `change_descriptor` and `network`. + /// Construct parameters with provided `descriptor`. /// - /// Default values: `genesis_hash` = `None`, `lookahead` = [`DEFAULT_LOOKAHEAD`] + /// Default values: + /// * `change_descriptor` = `None` + /// * `network` = [`Network::Bitcoin`] + /// * `genesis_hash` = `None` + /// * `lookahead` = [`DEFAULT_LOOKAHEAD`] + /// + /// Use this method only when building a wallet with a single descriptor. See + /// also [`Wallet::create_single`]. + pub fn new_single(descriptor: D) -> Self { + Self { + descriptor: make_descriptor_to_extract(descriptor), + descriptor_keymap: KeyMap::default(), + change_descriptor: None, + change_descriptor_keymap: KeyMap::default(), + network: Network::Bitcoin, + genesis_hash: None, + lookahead: DEFAULT_LOOKAHEAD, + } + } + + /// Construct parameters with provided `descriptor` and `change_descriptor`. + /// + /// Default values: + /// * `network` = [`Network::Bitcoin`] + /// * `genesis_hash` = `None` + /// * `lookahead` = [`DEFAULT_LOOKAHEAD`] pub fn new(descriptor: D, change_descriptor: D) -> Self { Self { descriptor: make_descriptor_to_extract(descriptor), descriptor_keymap: KeyMap::default(), - change_descriptor: make_descriptor_to_extract(change_descriptor), + change_descriptor: Some(make_descriptor_to_extract(change_descriptor)), change_descriptor_keymap: KeyMap::default(), network: Network::Bitcoin, genesis_hash: None, diff --git a/crates/wallet/src/wallet/tx_builder.rs b/crates/wallet/src/wallet/tx_builder.rs index 962edd56f..9d0adc729 100644 --- a/crates/wallet/src/wallet/tx_builder.rs +++ b/crates/wallet/src/wallet/tx_builder.rs @@ -481,7 +481,8 @@ impl<'a, Cs> TxBuilder<'a, Cs> { /// Do not spend change outputs /// /// This effectively adds all the change outputs to the "unspendable" list. See - /// [`TxBuilder::unspendable`]. + /// [`TxBuilder::unspendable`]. This method assumes the presence of an internal + /// keychain, otherwise it has no effect. pub fn do_not_spend_change(&mut self) -> &mut Self { self.params.change_policy = ChangeSpendPolicy::ChangeForbidden; self @@ -490,14 +491,16 @@ impl<'a, Cs> TxBuilder<'a, Cs> { /// Only spend change outputs /// /// This effectively adds all the non-change outputs to the "unspendable" list. See - /// [`TxBuilder::unspendable`]. + /// [`TxBuilder::unspendable`]. This method assumes the presence of an internal + /// keychain, otherwise it has no effect. pub fn only_spend_change(&mut self) -> &mut Self { self.params.change_policy = ChangeSpendPolicy::OnlyChange; self } /// Set a specific [`ChangeSpendPolicy`]. See [`TxBuilder::do_not_spend_change`] and - /// [`TxBuilder::only_spend_change`] for some shortcuts. + /// [`TxBuilder::only_spend_change`] for some shortcuts. This method assumes the presence + /// of an internal keychain, otherwise it has no effect. pub fn change_policy(&mut self, change_policy: ChangeSpendPolicy) -> &mut Self { self.params.change_policy = change_policy; self From 31f1c2d665363137e6a2d9fda3ae4ed2532d680b Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 6 Aug 2024 14:52:14 -0400 Subject: [PATCH 2/6] fix(wallet): Change FromSql type to `Option<_>` when selecting a wallet row from sqlite. This is consistent with the structure of the wallet `ChangeSet` where the fields `descriptor`, `change_descriptor`, and `network` are all optional. --- crates/chain/src/lib.rs | 7 +++++++ crates/wallet/src/wallet/changeset.rs | 16 +++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/chain/src/lib.rs b/crates/chain/src/lib.rs index 67e2aea15..a756ab11c 100644 --- a/crates/chain/src/lib.rs +++ b/crates/chain/src/lib.rs @@ -113,6 +113,13 @@ pub type KeychainIndexed = ((K, u32), T); /// A wrapper that we use to impl remote traits for types in our crate or dependency crates. pub struct Impl(pub T); +impl Impl { + /// Returns the inner `T`. + pub fn into_inner(self) -> T { + self.0 + } +} + impl From for Impl { fn from(value: T) -> Self { Self(value) diff --git a/crates/wallet/src/wallet/changeset.rs b/crates/wallet/src/wallet/changeset.rs index 46b2f4321..5f3b9b3dc 100644 --- a/crates/wallet/src/wallet/changeset.rs +++ b/crates/wallet/src/wallet/changeset.rs @@ -103,16 +103,18 @@ impl ChangeSet { let row = wallet_statement .query_row([], |row| { Ok(( - row.get::<_, Impl>>("descriptor")?, - row.get::<_, Impl>>("change_descriptor")?, - row.get::<_, Impl>("network")?, + row.get::<_, Option>>>("descriptor")?, + row.get::<_, Option>>>( + "change_descriptor", + )?, + row.get::<_, Option>>("network")?, )) }) .optional()?; - if let Some((Impl(desc), Impl(change_desc), Impl(network))) = row { - changeset.descriptor = Some(desc); - changeset.change_descriptor = Some(change_desc); - changeset.network = Some(network); + if let Some((desc, change_desc, network)) = row { + changeset.descriptor = desc.map(Impl::into_inner); + changeset.change_descriptor = change_desc.map(Impl::into_inner); + changeset.network = network.map(Impl::into_inner); } changeset.local_chain = local_chain::ChangeSet::from_sqlite(db_tx)?; From 2ca8b6ff73308c3852f454579206f3a234790ce9 Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 6 Aug 2024 13:24:42 -0400 Subject: [PATCH 3/6] test(wallet): Add tests for single descriptor wallet --- crates/wallet/tests/wallet.rs | 72 +++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index a4433bc19..6ef8330da 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -265,6 +265,41 @@ fn wallet_load_checks() -> anyhow::Result<()> { Ok(()) } +#[test] +fn single_descriptor_wallet_persist_and_recover() { + use bdk_chain::miniscript::Descriptor; + use bdk_chain::miniscript::DescriptorPublicKey; + use bdk_chain::rusqlite; + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("wallet.db"); + let mut db = rusqlite::Connection::open(db_path).unwrap(); + + let desc = get_test_tr_single_sig_xprv(); + let mut wallet = CreateParams::new_single(desc) + .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) = >::parse_descriptor(secp, desc).unwrap(); + let wallet = LoadParams::new() + .keymap(KeychainKind::External, keymap.clone()) + .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, + ); +} + #[test] fn test_error_external_and_internal_are_the_same() { // identical descriptors should fail to create wallet @@ -4116,3 +4151,40 @@ fn test_insert_tx_balance_and_utxos() { assert!(wallet.list_unspent().next().is_none()); assert_eq!(wallet.balance().total().to_sat(), 0); } + +#[test] +fn single_descriptor_wallet_can_create_tx_and_receive_change() { + // create single descriptor wallet and fund it + let mut wallet = CreateParams::new_single(get_test_tr_single_sig_xprv()) + .network(Network::Testnet) + .create_wallet_no_persist() + .unwrap(); + assert_eq!(wallet.keychains().count(), 1); + let amt = Amount::from_sat(5_000); + receive_output( + &mut wallet, + 2 * amt.to_sat(), + ConfirmationTime::Unconfirmed { last_seen: 2 }, + ); + // create spend tx that produces a change output + let addr = Address::from_str("bcrt1qc6fweuf4xjvz4x3gx3t9e0fh4hvqyu2qw4wvxm") + .unwrap() + .assume_checked(); + let mut builder = wallet.build_tx(); + builder.add_recipient(addr.script_pubkey(), amt); + let mut psbt = builder.finish().unwrap(); + assert!(wallet.sign(&mut psbt, SignOptions::default()).unwrap()); + let tx = psbt.extract_tx().unwrap(); + let txid = tx.compute_txid(); + wallet.insert_tx(tx); + insert_seen_at(&mut wallet, txid, 4); + let unspent: Vec<_> = wallet.list_unspent().collect(); + assert_eq!(unspent.len(), 1); + let utxo = unspent.first().unwrap(); + assert!(utxo.txout.value < amt); + assert_eq!( + utxo.keychain, + KeychainKind::External, + "tx change should go to external keychain" + ); +} From b8027147056bf78beab5e1c606d7fd93c2a57872 Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 6 Aug 2024 13:25:25 -0400 Subject: [PATCH 4/6] example(wallet): simplify miniscript compiler example --- crates/wallet/examples/compiler.rs | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/crates/wallet/examples/compiler.rs b/crates/wallet/examples/compiler.rs index d3f1391eb..72bed012e 100644 --- a/crates/wallet/examples/compiler.rs +++ b/crates/wallet/examples/compiler.rs @@ -56,28 +56,8 @@ fn main() -> Result<(), Box> { println!("Compiled into Descriptor: \n{}", descriptor); - // Do the same for another (internal) keychain - let policy_str = "or( - 10@thresh(2, - pk(029ffbe722b147f3035c87cb1c60b9a5947dd49c774cc31e94773478711a929ac0),pk(025f05815e3a1a8a83bfbb03ce016c9a2ee31066b98f567f6227df1d76ec4bd143),pk(025625f41e4a065efc06d5019cbbd56fe8c07595af1231e7cbc03fafb87ebb71ec) - ),1@and( - pk(03deae92101c790b12653231439f27b8897264125ecb2f46f48278603102573165), - older(12960) - ) - )" - .replace(&[' ', '\n', '\t'][..], ""); - - println!("Compiling internal policy: \n{}", policy_str); - - let policy = Concrete::::from_str(&policy_str)?; - let internal_descriptor = Descriptor::new_wsh(policy.compile()?)?.to_string(); - println!( - "Compiled into internal Descriptor: \n{}", - internal_descriptor - ); - // Create a new wallet from descriptors - let mut wallet = Wallet::create(descriptor, internal_descriptor) + let mut wallet = Wallet::create_single(descriptor) .network(Network::Regtest) .create_wallet_no_persist()?; From 3951110bb596d56a30ddc4cf09903cee4eebedea Mon Sep 17 00:00:00 2001 From: valued mammal Date: Fri, 9 Aug 2024 09:09:22 -0400 Subject: [PATCH 5/6] fix(wallet)!: Change method `LoadParams::descriptors` to just `descriptor` that takes a `KeychainKind` and optional `D: IntoWalletDescriptor` representing the expected value of the descriptor in the changeset. Add method `LoadParams::extract_keys` that will use any private keys in the provided descriptors to add a signer to the wallet. --- crates/wallet/README.md | 4 +- crates/wallet/src/wallet/mod.rs | 147 +++++++++++------- crates/wallet/src/wallet/params.rs | 30 +++- crates/wallet/tests/wallet.rs | 43 ++++- example-crates/wallet_electrum/src/main.rs | 4 +- .../wallet_esplora_async/src/main.rs | 4 +- .../wallet_esplora_blocking/src/main.rs | 4 +- example-crates/wallet_rpc/src/main.rs | 6 +- 8 files changed, 166 insertions(+), 76 deletions(-) diff --git a/crates/wallet/README.md b/crates/wallet/README.md index 9dee46aaa..4bcd7e8e6 100644 --- a/crates/wallet/README.md +++ b/crates/wallet/README.md @@ -79,8 +79,10 @@ 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) .network(network) + .descriptor(KeychainKind::External, Some(descriptor)) + .descriptor(KeychainKind::Internal, Some(change_descriptor)) + .extract_keys() .load_wallet(&mut db) .expect("wallet"); let mut wallet = match wallet_opt { diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 37dc57ee4..11e547b29 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/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, /// The expected descriptor. - expected: ExtendedDescriptor, + expected: Option, }, } @@ -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,9 @@ 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)) + /// .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC)) + /// .extract_keys() /// // you can also manually add private keys /// .keymap(KeychainKind::External, external_keymap) /// .keymap(KeychainKind::Internal, internal_keymap) @@ -499,42 +505,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 +521,88 @@ 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); - if descriptor.descriptor_id() != exp_descriptor.descriptor_id() { + 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(expected) = params.check_descriptor { + if let Some(make_desc) = expected { + let (exp_desc, keymap) = + make_desc(&secp, network).map_err(LoadError::Descriptor)?; + if descriptor.descriptor_id() != exp_desc.descriptor_id() { + return Err(LoadError::Mismatch(LoadMismatch::Descriptor { + keychain: KeychainKind::External, + loaded: Some(descriptor), + expected: Some(exp_desc), + })); + } + if params.extract_keys { + external_keymap.extend(keymap); + } + } else { return Err(LoadError::Mismatch(LoadMismatch::Descriptor { keychain: KeychainKind::External, - loaded: descriptor, - expected: exp_descriptor, + loaded: Some(descriptor), + expected: None, })); } } + let signers = Arc::new(SignersContainer::build(external_keymap, &descriptor, &secp)); + + let (mut change_descriptor, mut change_signers) = (None, Arc::new(SignersContainer::new())); + match (changeset.change_descriptor, params.check_change_descriptor) { + // empty signer + (None, None) => {} + (None, Some(expect)) => { + // expected desc but none loaded + if let Some(make_desc) = expect { + let (exp_desc, _) = make_desc(&secp, network).map_err(LoadError::Descriptor)?; + return Err(LoadError::Mismatch(LoadMismatch::Descriptor { + keychain: KeychainKind::Internal, + loaded: None, + expected: Some(exp_desc), + })); + } + } + // nothing expected + (Some(desc), None) => { + check_wallet_descriptor(&desc).map_err(LoadError::Descriptor)?; + change_descriptor = Some(desc); + } + (Some(desc), Some(expect)) => match expect { + // expected none for existing + None => { + return Err(LoadError::Mismatch(LoadMismatch::Descriptor { + keychain: KeychainKind::Internal, + loaded: Some(desc), + expected: None, + })) + } + // parameters must match + Some(make_desc) => { + let (exp_desc, keymap) = + make_desc(&secp, network).map_err(LoadError::Descriptor)?; + if desc.descriptor_id() != exp_desc.descriptor_id() { + return Err(LoadError::Mismatch(LoadMismatch::Descriptor { + keychain: KeychainKind::Internal, + loaded: Some(desc), + 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 signers = Arc::new(SignersContainer::build( - descriptor_keymap, - &descriptor, - &secp, - )); let index = create_indexer(descriptor, change_descriptor, params.lookahead) .map_err(LoadError::Descriptor)?; diff --git a/crates/wallet/src/wallet/params.rs b/crates/wallet/src/wallet/params.rs index 62d314ba9..134218589 100644 --- a/crates/wallet/src/wallet/params.rs +++ b/crates/wallet/src/wallet/params.rs @@ -144,8 +144,9 @@ pub struct LoadParams { pub(crate) lookahead: u32, pub(crate) check_network: Option, pub(crate) check_genesis_hash: Option, - pub(crate) check_descriptor: Option, - pub(crate) check_change_descriptor: Option, + pub(crate) check_descriptor: Option>, + pub(crate) check_change_descriptor: Option>, + pub(crate) extract_keys: bool, } impl LoadParams { @@ -161,6 +162,7 @@ impl LoadParams { check_genesis_hash: None, check_descriptor: None, check_change_descriptor: None, + extract_keys: false, } } @@ -174,14 +176,21 @@ impl LoadParams { self } - /// Checks that `descriptor` of `keychain` matches this, and extracts private keys (if - /// available). - pub fn descriptors(mut self, descriptor: D, change_descriptor: D) -> Self + /// Checks the `expected_descriptor` matches exactly what is loaded for `keychain`. + /// + /// # Note + /// + /// You must also specify [`extract_keys`](Self::extract_keys) if you wish to add a signer + /// for an expected descriptor containing secrets. + pub fn descriptor(mut self, keychain: KeychainKind, expected_descriptor: Option) -> 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)); + let expected = expected_descriptor.map(|d| make_descriptor_to_extract(d)); + match keychain { + KeychainKind::External => self.check_descriptor = Some(expected), + KeychainKind::Internal => self.check_change_descriptor = Some(expected), + } self } @@ -203,6 +212,13 @@ impl LoadParams { self } + /// Whether to try extracting private keys from the *provided descriptors* upon loading. + /// See also [`LoadParams::descriptor`]. + pub fn extract_keys(mut self) -> Self { + self.extract_keys = true; + self + } + /// Load [`PersistedWallet`] with the given `Db`. pub fn load_wallet( self, diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 6ef8330da..ee60ab972 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -134,10 +134,18 @@ fn wallet_is_persisted() -> anyhow::Result<()> { }; // recover wallet + { + let mut db = open_db(&file_path).context("failed to recover db")?; + let _ = Wallet::load() + .network(Network::Testnet) + .load_wallet(&mut db)? + .expect("wallet must exist"); + } { 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)) + .descriptor(KeychainKind::Internal, Some(internal_desc)) .network(Network::Testnet) .load_wallet(&mut db)? .expect("wallet must exist"); @@ -240,7 +248,16 @@ fn wallet_load_checks() -> anyhow::Result<()> { ); assert_matches!( Wallet::load() - .descriptors(internal_desc, external_desc) + .descriptor(KeychainKind::External, Some(internal_desc)) + .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) .load_wallet(&mut open_db(&file_path)?), Err(LoadWithPersistError::InvalidChangeSet(LoadError::Mismatch( LoadMismatch::Descriptor { .. } @@ -279,25 +296,39 @@ 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) = >::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)) + .extract_keys() .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, _) = >::parse_descriptor(secp, desc).unwrap(); + let err = Wallet::load() + .descriptor(KeychainKind::Internal, Some(desc)) + .extract_keys() + .load_wallet(&mut db); + assert_matches!( + err, + Err(LoadWithPersistError::InvalidChangeSet(LoadError::Mismatch(LoadMismatch::Descriptor { keychain, loaded, expected }))) + if keychain == KeychainKind::Internal && loaded.is_none() && expected == Some(exp_desc), + "single descriptor wallet should refuse change descriptor param" + ); } #[test] diff --git a/example-crates/wallet_electrum/src/main.rs b/example-crates/wallet_electrum/src/main.rs index 35413a962..b1e7655de 100644 --- a/example-crates/wallet_electrum/src/main.rs +++ b/example-crates/wallet_electrum/src/main.rs @@ -26,8 +26,10 @@ fn main() -> Result<(), anyhow::Error> { let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; let wallet_opt = Wallet::load() - .descriptors(EXTERNAL_DESC, INTERNAL_DESC) .network(NETWORK) + .descriptor(KeychainKind::External, Some(EXTERNAL_DESC)) + .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC)) + .extract_keys() .load_wallet(&mut db)?; let mut wallet = match wallet_opt { Some(wallet) => wallet, diff --git a/example-crates/wallet_esplora_async/src/main.rs b/example-crates/wallet_esplora_async/src/main.rs index cccd83395..535abc6af 100644 --- a/example-crates/wallet_esplora_async/src/main.rs +++ b/example-crates/wallet_esplora_async/src/main.rs @@ -23,8 +23,10 @@ async fn main() -> Result<(), anyhow::Error> { let mut conn = Connection::open(DB_PATH)?; let wallet_opt = Wallet::load() - .descriptors(EXTERNAL_DESC, INTERNAL_DESC) .network(NETWORK) + .descriptor(KeychainKind::External, Some(EXTERNAL_DESC)) + .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC)) + .extract_keys() .load_wallet(&mut conn)?; let mut wallet = match wallet_opt { Some(wallet) => wallet, diff --git a/example-crates/wallet_esplora_blocking/src/main.rs b/example-crates/wallet_esplora_blocking/src/main.rs index 4c4fe99ed..7e825150d 100644 --- a/example-crates/wallet_esplora_blocking/src/main.rs +++ b/example-crates/wallet_esplora_blocking/src/main.rs @@ -22,8 +22,10 @@ fn main() -> Result<(), anyhow::Error> { let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), DB_PATH)?; let wallet_opt = Wallet::load() - .descriptors(EXTERNAL_DESC, INTERNAL_DESC) .network(NETWORK) + .descriptor(KeychainKind::External, Some(EXTERNAL_DESC)) + .descriptor(KeychainKind::Internal, Some(INTERNAL_DESC)) + .extract_keys() .load_wallet(&mut db)?; let mut wallet = match wallet_opt { Some(wallet) => wallet, diff --git a/example-crates/wallet_rpc/src/main.rs b/example-crates/wallet_rpc/src/main.rs index 2533de644..324cf0445 100644 --- a/example-crates/wallet_rpc/src/main.rs +++ b/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,8 +89,10 @@ fn main() -> anyhow::Result<()> { let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), args.db_path)?; let wallet_opt = Wallet::load() - .descriptors(args.descriptor.clone(), args.change_descriptor.clone()) .network(args.network) + .descriptor(KeychainKind::External, Some(args.descriptor.clone())) + .descriptor(KeychainKind::Internal, Some(args.change_descriptor.clone())) + .extract_keys() .load_wallet(&mut db)?; let mut wallet = match wallet_opt { Some(wallet) => wallet, From 13e7008f00b77802481733eb74daf38f48a2faef Mon Sep 17 00:00:00 2001 From: valued mammal Date: Tue, 13 Aug 2024 22:14:28 -0400 Subject: [PATCH 6/6] doc(wallet): clarify docs for `Wallet::load` --- crates/wallet/src/wallet/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 11e547b29..4303ad298 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -443,10 +443,11 @@ impl Wallet { /// Build [`Wallet`] by loading from persistence or [`ChangeSet`]. /// - /// 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::descriptor`]. + /// Note that the descriptor secret keys are not persisted to the db. You can add + /// signers after-the-fact with [`Wallet::add_signer`] or [`Wallet::set_keymap`]. You + /// can also add keys when building the wallet by using [`LoadParams::keymap`]. Finally + /// you can check the wallet's descriptors are what you expect with [`LoadParams::descriptor`] + /// which will try to populate signers if [`LoadParams::extract_keys`] is enabled. /// /// # Synopsis ///