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

notmandatory · Jul 13, 2024 · dd03db4 · dd03db4
1 parent b978e16
commit dd03db4
Show file tree

Hide file tree

Showing 14 changed files with 152 additions and 104 deletions.
diff --git a/crates/hwi/src/lib.rs b/crates/hwi/src/lib.rs
@@ -20,7 +20,7 @@
 //!
 //! # let mut wallet = Wallet::new(
 //! #     "",
-//! #     "",
+//! #     Some(""),
 //! #     Network::Testnet,
 //! # )?;
 //! #

diff --git a/crates/wallet/README.md b/crates/wallet/README.md
@@ -82,7 +82,7 @@ let changeset = db.aggregate_changesets().expect("changeset loaded");
 let mut wallet = if let Some(changeset) = changeset {
     Wallet::load(changeset).expect("loaded wallet")
 } else {
-    Wallet::new(descriptor, change_descriptor, Network::Testnet).expect("created new wallet")
+    Wallet::new(descriptor, Some(change_descriptor), Network::Testnet).expect("created new wallet")
 };
 
 // Get a new address to receive bitcoin.

diff --git a/crates/wallet/examples/compiler.rs b/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{}",

diff --git a/crates/wallet/src/descriptor/template.rs b/crates/wallet/src/descriptor/template.rs
@@ -81,7 +81,11 @@ impl<T: DescriptorTemplate> IntoWalletDescriptor for T {
 ///     bitcoin::PrivateKey::from_wif("cTc4vURSzdx6QE6KVynWGomDbLaA75dNALMNyfjh3p8DRRar84Um")?;
 /// let key_internal =
 ///     bitcoin::PrivateKey::from_wif("cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW")?;
-/// let mut wallet = Wallet::new(P2Pkh(key_external), P2Pkh(key_internal), Network::Testnet)?;
+/// let mut wallet = Wallet::new(
+///     P2Pkh(key_external),
+///     Some(P2Pkh(key_internal)),
+///     Network::Testnet,
+/// )?;
 ///
 /// assert_eq!(
 ///     wallet
@@ -115,7 +119,7 @@ impl<K: IntoDescriptorKey<Legacy>> DescriptorTemplate for P2Pkh<K> {
 ///     bitcoin::PrivateKey::from_wif("cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW")?;
 /// let mut wallet = Wallet::new(
 ///     P2Wpkh_P2Sh(key_external),
-///     P2Wpkh_P2Sh(key_internal),
+///     Some(P2Wpkh_P2Sh(key_internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -150,7 +154,11 @@ impl<K: IntoDescriptorKey<Segwitv0>> DescriptorTemplate for P2Wpkh_P2Sh<K> {
 ///     bitcoin::PrivateKey::from_wif("cTc4vURSzdx6QE6KVynWGomDbLaA75dNALMNyfjh3p8DRRar84Um")?;
 /// let key_internal =
 ///     bitcoin::PrivateKey::from_wif("cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW")?;
-/// let mut wallet = Wallet::new(P2Wpkh(key_external), P2Wpkh(key_internal), Network::Testnet)?;
+/// let mut wallet = Wallet::new(
+///     P2Wpkh(key_external),
+///     Some(P2Wpkh(key_internal)),
+///     Network::Testnet,
+/// )?;
 ///
 /// assert_eq!(
 ///     wallet
@@ -182,7 +190,11 @@ impl<K: IntoDescriptorKey<Segwitv0>> DescriptorTemplate for P2Wpkh<K> {
 ///     bitcoin::PrivateKey::from_wif("cTc4vURSzdx6QE6KVynWGomDbLaA75dNALMNyfjh3p8DRRar84Um")?;
 /// let key_internal =
 ///     bitcoin::PrivateKey::from_wif("cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW")?;
-/// let mut wallet = Wallet::new(P2TR(key_external), P2TR(key_internal), Network::Testnet)?;
+/// let mut wallet = Wallet::new(
+///     P2TR(key_external),
+///     Some(P2TR(key_internal)),
+///     Network::Testnet,
+/// )?;
 ///
 /// assert_eq!(
 ///     wallet
@@ -217,7 +229,7 @@ impl<K: IntoDescriptorKey<Tap>> DescriptorTemplate for P2TR<K> {
 /// let key = bitcoin::bip32::Xpriv::from_str("tprv8ZgxMBicQKsPeZRHk4rTG6orPS2CRNFX3njhUXx5vj9qGog5ZMH4uGReDWN5kCkY3jmWEtWause41CDvBRXD1shKknAMKxT99o9qUTRVC6m")?;
 /// let mut wallet = Wallet::new(
 ///     Bip44(key.clone(), KeychainKind::External),
-///     Bip44(key, KeychainKind::Internal),
+///     Some(Bip44(key, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -254,7 +266,7 @@ impl<K: DerivableKey<Legacy>> DescriptorTemplate for Bip44<K> {
 /// let fingerprint = bitcoin::bip32::Fingerprint::from_str("c55b303f")?;
 /// let mut wallet = Wallet::new(
 ///     Bip44Public(key.clone(), fingerprint, KeychainKind::External),
-///     Bip44Public(key, fingerprint, KeychainKind::Internal),
+///     Some(Bip44Public(key, fingerprint, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -290,7 +302,7 @@ impl<K: DerivableKey<Legacy>> DescriptorTemplate for Bip44Public<K> {
 /// let key = bitcoin::bip32::Xpriv::from_str("tprv8ZgxMBicQKsPeZRHk4rTG6orPS2CRNFX3njhUXx5vj9qGog5ZMH4uGReDWN5kCkY3jmWEtWause41CDvBRXD1shKknAMKxT99o9qUTRVC6m")?;
 /// let mut wallet = Wallet::new(
 ///     Bip49(key.clone(), KeychainKind::External),
-///     Bip49(key, KeychainKind::Internal),
+///     Some(Bip49(key, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -327,7 +339,7 @@ impl<K: DerivableKey<Segwitv0>> DescriptorTemplate for Bip49<K> {
 /// let fingerprint = bitcoin::bip32::Fingerprint::from_str("c55b303f")?;
 /// let mut wallet = Wallet::new(
 ///     Bip49Public(key.clone(), fingerprint, KeychainKind::External),
-///     Bip49Public(key, fingerprint, KeychainKind::Internal),
+///     Some(Bip49Public(key, fingerprint, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -363,7 +375,7 @@ impl<K: DerivableKey<Segwitv0>> DescriptorTemplate for Bip49Public<K> {
 /// let key = bitcoin::bip32::Xpriv::from_str("tprv8ZgxMBicQKsPeZRHk4rTG6orPS2CRNFX3njhUXx5vj9qGog5ZMH4uGReDWN5kCkY3jmWEtWause41CDvBRXD1shKknAMKxT99o9qUTRVC6m")?;
 /// let mut wallet = Wallet::new(
 ///     Bip84(key.clone(), KeychainKind::External),
-///     Bip84(key, KeychainKind::Internal),
+///     Some(Bip84(key, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -400,7 +412,7 @@ impl<K: DerivableKey<Segwitv0>> DescriptorTemplate for Bip84<K> {
 /// let fingerprint = bitcoin::bip32::Fingerprint::from_str("c55b303f")?;
 /// let mut wallet = Wallet::new(
 ///     Bip84Public(key.clone(), fingerprint, KeychainKind::External),
-///     Bip84Public(key, fingerprint, KeychainKind::Internal),
+///     Some(Bip84Public(key, fingerprint, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -436,7 +448,7 @@ impl<K: DerivableKey<Segwitv0>> DescriptorTemplate for Bip84Public<K> {
 /// let key = bitcoin::bip32::Xpriv::from_str("tprv8ZgxMBicQKsPeZRHk4rTG6orPS2CRNFX3njhUXx5vj9qGog5ZMH4uGReDWN5kCkY3jmWEtWause41CDvBRXD1shKknAMKxT99o9qUTRVC6m")?;
 /// let mut wallet = Wallet::new(
 ///     Bip86(key.clone(), KeychainKind::External),
-///     Bip86(key, KeychainKind::Internal),
+///     Some(Bip86(key, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///
@@ -473,7 +485,7 @@ impl<K: DerivableKey<Tap>> DescriptorTemplate for Bip86<K> {
 /// let fingerprint = bitcoin::bip32::Fingerprint::from_str("c55b303f")?;
 /// let mut wallet = Wallet::new(
 ///     Bip86Public(key.clone(), fingerprint, KeychainKind::External),
-///     Bip86Public(key, fingerprint, KeychainKind::Internal),
+///     Some(Bip86Public(key, fingerprint, KeychainKind::Internal)),
 ///     Network::Testnet,
 /// )?;
 ///

diff --git a/crates/wallet/src/wallet/export.rs b/crates/wallet/src/wallet/export.rs
@@ -31,7 +31,7 @@
 //! let import = FullyNodedExport::from_str(import)?;
 //! let wallet = Wallet::new(
 //!     &import.descriptor(),
-//!     &import.change_descriptor().expect("change descriptor"),
+//!     Some(&import.change_descriptor().expect("change descriptor")),
 //!     Network::Testnet,
 //! )?;
 //! # Ok::<_, Box<dyn std::error::Error>>(())
@@ -44,7 +44,7 @@
 //! # use bdk_wallet::*;
 //! let wallet = Wallet::new(
 //!     "wpkh([c258d2e4/84h/1h/0h]tpubDD3ynpHgJQW8VvWRzQ5WFDCrs4jqVFGHB3vLC3r49XHJSqP8bHKdK4AriuUKLccK68zfzowx7YhmDN8SiSkgCDENUFx9qVw65YyqM78vyVe/0/*)",
-//!     "wpkh([c258d2e4/84h/1h/0h]tpubDD3ynpHgJQW8VvWRzQ5WFDCrs4jqVFGHB3vLC3r49XHJSqP8bHKdK4AriuUKLccK68zfzowx7YhmDN8SiSkgCDENUFx9qVw65YyqM78vyVe/1/*)",
+//!     Some("wpkh([c258d2e4/84h/1h/0h]tpubDD3ynpHgJQW8VvWRzQ5WFDCrs4jqVFGHB3vLC3r49XHJSqP8bHKdK4AriuUKLccK68zfzowx7YhmDN8SiSkgCDENUFx9qVw65YyqM78vyVe/1/*)"),
 //!     Network::Testnet,
 //! )?;
 //! let export = FullyNodedExport::export_wallet(&wallet, "exported wallet", true).unwrap();
@@ -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![],

diff --git a/crates/wallet/src/wallet/mod.rs b/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> {
@@ -1013,7 +1013,7 @@ impl Wallet {
     /// # use bdk_wallet::bitcoin::Network;
     /// let descriptor = "wpkh(tprv8ZgxMBicQKsPe73PBRSmNbTfbcsZnwWhz5eVmhHpi31HW29Z7mc9B4cWGRQzopNUzZUT391DeDJxL2PefNunWyLgqCKRMDkU1s2s8bAfoSk/84'/1'/0'/0/*)";
     /// let change_descriptor = "wpkh(tprv8ZgxMBicQKsPe73PBRSmNbTfbcsZnwWhz5eVmhHpi31HW29Z7mc9B4cWGRQzopNUzZUT391DeDJxL2PefNunWyLgqCKRMDkU1s2s8bAfoSk/84'/1'/0'/1/*)";
-    /// let wallet = Wallet::new(descriptor, change_descriptor, Network::Testnet)?;
+    /// let wallet = Wallet::new(descriptor, Some(change_descriptor), Network::Testnet)?;
     /// for secret_key in wallet.get_signers(KeychainKind::External).signers().iter().filter_map(|s| s.descriptor_secret_key()) {
     ///     // secret_key: tprv8ZgxMBicQKsPe73PBRSmNbTfbcsZnwWhz5eVmhHpi31HW29Z7mc9B4cWGRQzopNUzZUT391DeDJxL2PefNunWyLgqCKRMDkU1s2s8bAfoSk/84'/0'/0'/0/*
     ///     println!("secret_key: {}", secret_key);
@@ -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))
 }
@@ -2377,7 +2390,7 @@ macro_rules! doctest_wallet {
 
         let mut wallet = Wallet::new(
             descriptor,
-            change_descriptor,
+            Some(change_descriptor),
             Network::Regtest,
         )
         .unwrap();

diff --git a/crates/wallet/src/wallet/signer.rs b/crates/wallet/src/wallet/signer.rs
@@ -69,7 +69,7 @@
 //!
 //! let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/0/*)";
 //! let change_descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/1/*)";
-//! let mut wallet = Wallet::new(descriptor, change_descriptor, Network::Testnet)?;
+//! let mut wallet = Wallet::new(descriptor, Some(change_descriptor), Network::Testnet)?;
 //! wallet.add_signer(
 //!     KeychainKind::External,
 //!     SignerOrdering(200),