From dcb4f70c616bdf33a31f0805779743f9c76b45d8 Mon Sep 17 00:00:00 2001 From: valued mammal Date: Wed, 31 Jul 2024 23:09:16 -0500 Subject: [PATCH] feat(params): Add method `new_single_descriptor` The change descriptor is made optional, making this an effective reversion of #1390 and enables creating wallets with a single descriptor. --- crates/wallet/src/wallet/mod.rs | 188 +++++++++++++++++------------ crates/wallet/src/wallet/params.rs | 24 +++- crates/wallet/tests/wallet.rs | 52 ++++++++ 3 files changed, 186 insertions(+), 78 deletions(-) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index df22c8a641..a366c9c71c 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -347,20 +347,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(); @@ -457,10 +459,33 @@ impl Wallet { 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)?; + // 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 +517,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)?; @@ -550,7 +557,7 @@ impl Wallet { /// /// For non-wildcard descriptors this returns the same address at every provided index. /// - /// # Panics + /// # Panics (TODO: document panics for address methods) /// /// 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. @@ -785,7 +792,13 @@ impl Wallet { self.indexed_graph .index .unbounded_spk_iter(keychain) - .expect("keychain must exist") + .unwrap_or_else(|| { + // Given an empty keychain just create a dummy `SpkIterator` with + // a range that will always return None. + let (desc, _) = ::parse_descriptor(self.secp_ctx(), "wsh(0)") + .expect("valid descriptor"); + bdk_chain::SpkIterator::new_with_range(desc, 1..) + }) } /// Returns the utxo owned by this wallet corresponding to `outpoint` if it exists in the @@ -1053,17 +1066,18 @@ impl Wallet { } /// Set the keymap for a given keychain. + /// + /// Note this is a no-op if the given keychain is not assigned to a 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 +1157,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 +1182,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 +1199,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 +1379,11 @@ 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 = if self.is_single_descriptor_wallet() { + KeychainKind::External + } else { + KeychainKind::Internal + }; let ((index, spk), index_changeset) = self .indexed_graph .index @@ -1852,8 +1884,8 @@ impl Wallet { self.indexed_graph .index .next_index(keychain) - .expect("keychain must exist") - .0 + .map(|(i, _)| i) + .unwrap_or_default() } /// Informs the wallet that you no longer intend to broadcast a tx that was built from it. @@ -1977,7 +2009,8 @@ impl Wallet { let mut i = 0; may_spend.retain(|u| { - let retain = change_policy.is_satisfied_by(&u.0) + let retain = (self.is_single_descriptor_wallet() + || change_policy.is_satisfied_by(&u.0)) && !unspendable.contains(&u.0.outpoint) && satisfies_confirmed[i]; i += 1; @@ -2310,6 +2343,11 @@ impl Wallet { .batch_insert_relevant_unconfirmed(unconfirmed_txs); self.stage.merge(indexed_graph_changeset.into()); } + + /// Returns `true` if this wallet only has a single descriptor. + fn is_single_descriptor_wallet(&self) -> bool { + self.keychains().count() == 1 + } } /// Methods to construct sync/full-scan requests for spk-based chain sources. @@ -2391,7 +2429,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); @@ -2404,19 +2442,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 44d0d1db13..ae2661fb34 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,30 @@ pub struct CreateParams { } impl CreateParams { - /// Construct parameters with provided `descriptor`, `change_descriptor` and `network`. + /// Construct parameters with provided `descriptor`. + pub fn new_single_descriptor(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`] /// - /// Default values: `genesis_hash` = `None`, `lookahead` = [`DEFAULT_LOOKAHEAD`] + /// To build a single descriptor wallet use [`CreateParams::new_single_descriptor`]. 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/tests/wallet.rs b/crates/wallet/tests/wallet.rs index a4433bc19a..9a6823fd9a 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -101,6 +101,58 @@ const P2WPKH_FAKE_WITNESS_SIZE: usize = 106; const DB_MAGIC: &[u8] = &[0x21, 0x24, 0x48]; +#[test] +fn single_descriptor_wallet_can_create_tx_and_receive_change() { + // create single descriptor wallet and fund it + let mut wallet = CreateParams::new_single_descriptor(get_test_tr_single_sig_xprv()) + .network(Network::Testnet) + .create_wallet_no_persist() + .unwrap(); + assert_eq!(wallet.keychains().count(), 1); + receive_output( + &mut wallet, + 10_000, + 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(), Amount::from_sat(5_000)); + let mut psbt = builder + .finish() + .expect("single descriptor wallet should create tx"); + let _ = wallet.sign(&mut psbt, SignOptions::default()); + 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.to_sat() < 5_000); + assert_eq!( + utxo.keychain, + KeychainKind::External, + "tx change should go to external keychain" + ); +} + +#[test] +fn single_desc_internal_spk_iter_should_be_empty() { + let wallet = CreateParams::new_single_descriptor(get_test_tr_single_sig_xprv()) + .network(Network::Testnet) + .create_wallet_no_persist() + .unwrap(); + + let mut spk_iter = wallet.unbounded_spk_iter(KeychainKind::External); + assert!(spk_iter.next().is_some()); + + let mut empty_iter = wallet.unbounded_spk_iter(KeychainKind::Internal); + assert!(empty_iter.next().is_none()); +} + #[test] fn wallet_is_persisted() -> anyhow::Result<()> { fn run(