refactor(wallet)!: make internal descriptor optional in constructors

crates/hwi/src/lib.rs

@@ -20,7 +20,7 @@
 //!
 //! # let mut wallet = Wallet::new(
 //! #     "",
-//! #     "",
+//! #     Some(""),
 //! #     Network::Testnet,
 //! # )?;
 //! #

crates/wallet/examples/compiler.rs

@@ -77,7 +77,7 @@ fn main() -> Result<(), Box<dyn Error>> {
     );
 
     // Create a new wallet from descriptors
-    let mut wallet = Wallet::new(&descriptor, &internal_descriptor, Network::Regtest)?;
+    let mut wallet = Wallet::new(&descriptor, Some(&internal_descriptor), Network::Regtest)?;
 
     println!(
         "First derived address from the descriptor: \n{}",

crates/wallet/src/wallet/export.rs

@@ -224,7 +224,7 @@ mod test {
     fn get_test_wallet(descriptor: &str, change_descriptor: &str, network: Network) -> Wallet {
         use crate::wallet::Update;
         use bdk_chain::TxGraph;
-        let mut wallet = Wallet::new(descriptor, change_descriptor, network).unwrap();
+        let mut wallet = Wallet::new(descriptor, Some(change_descriptor), network).unwrap();
         let transaction = Transaction {
             input: vec![],
             output: vec![],

crates/wallet/src/wallet/mod.rs

@@ -294,7 +294,7 @@ impl Wallet {
     /// automatically to the new [`Wallet`].
     pub fn new<E: IntoWalletDescriptor>(
         descriptor: E,
-        change_descriptor: E,
+        change_descriptor: Option<E>,
         network: Network,
     ) -> Result<Self, NewError> {
         let genesis_hash = genesis_block(network).block_hash();
@@ -307,7 +307,7 @@ impl Wallet {
     /// for syncing from alternative networks.
     pub fn new_with_genesis_hash<E: IntoWalletDescriptor>(
         descriptor: E,
-        change_descriptor: E,
+        change_descriptor: Option<E>,
         network: Network,
         genesis_hash: BlockHash,
     ) -> Result<Self, NewError> {
@@ -1072,14 +1072,10 @@ impl Wallet {
     ) -> Result<Psbt, CreateTxError> {
         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 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();
 
         // The policy allows spending external outputs, but it requires a policy path that hasn't been
         // provided
@@ -1091,30 +1087,39 @@ impl Wallet {
                 KeychainKind::External,
             ));
         };
-        // 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,
-            ));
-        };
 
         let external_requirements = external_policy.get_condition(
             params
                 .external_policy_path
                 .as_ref()
                 .unwrap_or(&BTreeMap::new()),
         )?;
-        let internal_requirements = internal_policy.get_condition(
-            params
-                .internal_policy_path
-                .as_ref()
-                .unwrap_or(&BTreeMap::new()),
-        )?;
+        let mut requirements = external_requirements;
+
+        if let Some(internal_descriptor) = keychains.get(&KeychainKind::Internal) {
+            let internal_policy = internal_descriptor
+                .extract_policy(&self.change_signers, BuildSatisfaction::None, &self.secp)?
+                .unwrap();
 
-        let requirements = external_requirements.merge(&internal_requirements)?;
+            // 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,
+                ));
+            };
+
+            let internal_requirements = internal_policy.get_condition(
+                params
+                    .internal_policy_path
+                    .as_ref()
+                    .unwrap_or(&BTreeMap::new()),
+            )?;
+
+            requirements = requirements.merge(&internal_requirements)?;
+        }
 
         let version = match params.version {
             Some(tx_builder::Version(0)) => return Err(CreateTxError::Version0),
@@ -1280,12 +1285,18 @@ impl Wallet {
         let drain_script = match params.drain_to {
             Some(ref drain_recipient) => drain_recipient.clone(),
             None => {
-                let change_keychain = KeychainKind::Internal;
+                let mut change_keychain = KeychainKind::Internal;
                 let ((index, spk), index_changeset) = self
                     .indexed_graph
                     .index
                     .next_unused_spk(&change_keychain)
-                    .expect("keychain must exist");
+                    .unwrap_or_else(|| {
+                        change_keychain = KeychainKind::External;
+                        self.indexed_graph
+                            .index
+                            .next_unused_spk(&change_keychain)
+                            .expect("")
+                    });
                 self.indexed_graph.index.mark_used(change_keychain, index);
                 self.stage.merge(index_changeset.into());
                 spk
@@ -1529,12 +1540,8 @@ impl Wallet {
         if tx.output.len() > 1 {
             let mut change_index = None;
             for (index, txout) in tx.output.iter().enumerate() {
-                let change_keychain = KeychainKind::Internal;
-                match txout_index.index_of_spk(&txout.script_pubkey) {
-                    Some((keychain, _)) if *keychain == change_keychain => {
-                        change_index = Some(index)
-                    }
-                    _ => {}
+                if self.is_mine(&txout.script_pubkey) {
+                    change_index = Some(index)
                 }
             }
 
@@ -2322,32 +2329,38 @@ fn create_signers<E: IntoWalletDescriptor>(
     index: &mut KeychainTxOutIndex<KeychainKind>,
     secp: &Secp256k1<All>,
     descriptor: E,
-    change_descriptor: E,
+    change_descriptor: Option<E>,
     network: Network,
 ) -> Result<(Arc<SignersContainer>, Arc<SignersContainer>), DescriptorError> {
     let descriptor = into_wallet_descriptor_checked(descriptor, secp, network)?;
-    let change_descriptor = into_wallet_descriptor_checked(change_descriptor, secp, network)?;
+    let change_descriptor = change_descriptor
+        .map(|cd| into_wallet_descriptor_checked(cd, secp, network))
+        .transpose()?;
     let (descriptor, keymap) = descriptor;
     let signers = Arc::new(SignersContainer::build(keymap, &descriptor, secp));
     let _ = index
         .insert_descriptor(KeychainKind::External, descriptor)
         .expect("this is the first descriptor we're inserting");
 
-    let (descriptor, keymap) = change_descriptor;
-    let change_signers = Arc::new(SignersContainer::build(keymap, &descriptor, secp));
-    let _ = index
-        .insert_descriptor(KeychainKind::Internal, 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")
+    let change_signers = if let Some((descriptor, keymap)) = change_descriptor {
+        let change_signer = Arc::new(SignersContainer::build(keymap, &descriptor, secp));
+        let _ = index
+            .insert_descriptor(KeychainKind::Internal, 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")
+                    }
                 }
-            }
-        })?;
+            })?;
+        change_signer
+    } else {
+        Arc::new(SignersContainer::new())
+    };
 
     Ok((signers, change_signers))
 }

crates/wallet/tests/common.rs

@@ -15,7 +15,12 @@ use std::str::FromStr;
 /// The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000
 /// to a foreign address and one returning 50_000 back to the wallet. The remaining 1000
 /// sats are the transaction fee.
-pub fn get_funded_wallet_with_change(descriptor: &str, change: &str) -> (Wallet, bitcoin::Txid) {
+///
+/// Note: no change descriptor is used internal (change) SPKs will be derived from the external descriptor.
+pub fn get_funded_wallet_with_change(
+    descriptor: &str,
+    change: Option<&str>,
+) -> (Wallet, bitcoin::Txid) {
     let mut wallet = Wallet::new(descriptor, change, Network::Regtest).unwrap();
     let receive_address = wallet.peek_address(KeychainKind::External, 0).address;
     let sendto_address = Address::from_str("bcrt1q3qtze4ys45tgdvguj66zrk4fu6hq3a3v9pfly5")
@@ -113,16 +118,13 @@ pub fn get_funded_wallet_with_change(descriptor: &str, change: &str) -> (Wallet,
 /// to a foreign address and one returning 50_000 back to the wallet. The remaining 1000
 /// sats are the transaction fee.
 ///
-/// Note: the change descriptor will have script type `p2wpkh`. If passing some other script type
-/// as argument, make sure you're ok with getting a wallet where the keychains have potentially
-/// different script types. Otherwise, use `get_funded_wallet_with_change`.
+/// Note: no change descriptor is used internal (change) SPKs will be derived from the external descriptor.
 pub fn get_funded_wallet(descriptor: &str) -> (Wallet, bitcoin::Txid) {
-    let change = get_test_wpkh_change();
-    get_funded_wallet_with_change(descriptor, change)
+    get_funded_wallet_with_change(descriptor, None)
 }
 
 pub fn get_funded_wallet_wpkh() -> (Wallet, bitcoin::Txid) {
-    get_funded_wallet_with_change(get_test_wpkh(), get_test_wpkh_change())
+    get_funded_wallet_with_change(get_test_wpkh(), None)
 }
 
 pub fn get_test_wpkh() -> &'static str {

crates/wallet/tests/psbt.rs

@@ -144,7 +144,7 @@ fn test_psbt_fee_rate_with_missing_txout() {
 
     let desc = "pkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/0)";
     let change_desc = "pkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/1)";
-    let (mut pkh_wallet, _) = get_funded_wallet_with_change(desc, change_desc);
+    let (mut pkh_wallet, _) = get_funded_wallet_with_change(desc, Some(change_desc));
     let addr = pkh_wallet.peek_address(KeychainKind::External, 0);
     let mut builder = pkh_wallet.build_tx();
     builder.drain_to(addr.script_pubkey()).drain_wallet();
@@ -173,7 +173,7 @@ fn test_psbt_multiple_internalkey_signers() {
     let keypair = Keypair::from_secret_key(&secp, &prv.inner);
 
     let change_desc = "tr(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW)";
-    let (mut wallet, _) = get_funded_wallet_with_change(&desc, change_desc);
+    let (mut wallet, _) = get_funded_wallet_with_change(&desc, Some(change_desc));
     let to_spend = wallet.balance().total();
     let send_to = wallet.peek_address(KeychainKind::External, 0);
     let mut builder = wallet.build_tx();