Merge bitcoindevkit#1503: feat(wallet): simplify public_descriptor …

…fn and remove `get_descriptor_for_keych…

e7ec5a8 refactor(wallet)!: remove redundant get_descriptor_for_keychain (Giovanni Napoli)

Pull request description:

  Fixes bitcoindevkit#1501

  ### Description

  Simplifies `public_descriptor` function by using `get_descriptor` and removes `get_descriptor_for_keychain`.

  ### Notes to the reviewers

  Tested with `cargo test --all-features`.

  ### Changelog notice

  - Simplifies `public_descriptor` function and removes `get_descriptor_for_keychain`

  ### Checklists

  #### All Submissions:

  * [X] I've signed all my commits
  * [X] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [X] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  notmandatory:
    ACK e7ec5a8
  storopoli:
    ACK bitcoindevkit@e7ec5a8

Tree-SHA512: 5981190ac882e08e42b1be53c55afb70c4ba7e7a99a8ae2a4e05f0618d0eb23849ce544024bb406e6a6918d9e9757d9ff6ad5a701cd9814b686e36f1ea16b44a

crates/wallet/src/wallet/export.rs

@@ -116,7 +116,7 @@ impl FullyNodedExport {
         include_blockheight: bool,
     ) -> Result<Self, &'static str> {
         let descriptor = wallet
-            .get_descriptor_for_keychain(KeychainKind::External)
+            .public_descriptor(KeychainKind::External)
             .to_string_with_secret(
                 &wallet
                     .get_signers(KeychainKind::External)
@@ -144,7 +144,7 @@ impl FullyNodedExport {
 
         let change_descriptor = {
             let descriptor = wallet
-                .get_descriptor_for_keychain(KeychainKind::Internal)
+                .public_descriptor(KeychainKind::Internal)
                 .to_string_with_secret(
                     &wallet
                         .get_signers(KeychainKind::Internal)

crates/wallet/src/wallet/mod.rs

@@ -1595,7 +1595,7 @@ impl Wallet {
                 let weighted_utxo = match txout_index.index_of_spk(&txout.script_pubkey) {
                     Some(&(keychain, derivation_index)) => {
                         let satisfaction_weight = self
-                            .get_descriptor_for_keychain(keychain)
+                            .public_descriptor(keychain)
                             .max_weight_to_satisfy()
                             .unwrap();
                         WeightedUtxo {
@@ -1763,16 +1763,15 @@ impl Wallet {
         )
     }
 
-    /// Return the "public" version of the wallet's descriptor, meaning a new descriptor that has
-    /// the same structure but with every secret key removed
+    /// 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
+    /// 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
-            .keychains()
-            .find(|(k, _)| *k == &keychain)
-            .map(|(_, d)| d)
+            .get_descriptor(&keychain)
             .expect("keychain must exist")
     }
 
@@ -1878,11 +1877,6 @@ impl Wallet {
         &self.secp
     }
 
-    /// Returns the descriptor used to create addresses for a particular `keychain`.
-    pub fn get_descriptor_for_keychain(&self, keychain: KeychainKind) -> &ExtendedDescriptor {
-        self.public_descriptor(keychain)
-    }
-
     /// The derivation index of this wallet. It will return `None` if it has not derived any addresses.
     /// Otherwise, it will return the index of the highest address it has derived.
     pub fn derivation_index(&self, keychain: KeychainKind) -> Option<u32> {
@@ -1918,7 +1912,7 @@ impl Wallet {
             .indexed_graph
             .index
             .index_of_spk(&txout.script_pubkey)?;
-        let descriptor = self.get_descriptor_for_keychain(keychain);
+        let descriptor = self.public_descriptor(keychain);
         descriptor.at_derivation_index(child).ok()
     }
 
@@ -1927,7 +1921,7 @@ impl Wallet {
             .map(|utxo| {
                 let keychain = utxo.keychain;
                 (utxo, {
-                    self.get_descriptor_for_keychain(keychain)
+                    self.public_descriptor(keychain)
                         .max_weight_to_satisfy()
                         .unwrap()
                 })
@@ -2140,7 +2134,7 @@ impl Wallet {
             ..psbt::Input::default()
         };
 
-        let desc = self.get_descriptor_for_keychain(keychain);
+        let desc = self.public_descriptor(keychain);
         let derived_descriptor = desc
             .at_derivation_index(child)
             .expect("child can't be hardened");
@@ -2180,7 +2174,7 @@ impl Wallet {
             if let Some(&(keychain, child)) =
                 self.indexed_graph.index.index_of_spk(&out.script_pubkey)
             {
-                let desc = self.get_descriptor_for_keychain(keychain);
+                let desc = self.public_descriptor(keychain);
                 let desc = desc
                     .at_derivation_index(child)
                     .expect("child can't be hardened");
@@ -2200,9 +2194,9 @@ impl Wallet {
 
     /// Return the checksum of the public descriptor associated to `keychain`
     ///
-    /// Internally calls [`Self::get_descriptor_for_keychain`] to fetch the right descriptor
+    /// Internally calls [`Self::public_descriptor`] to fetch the right descriptor
     pub fn descriptor_checksum(&self, keychain: KeychainKind) -> String {
-        self.get_descriptor_for_keychain(keychain)
+        self.public_descriptor(keychain)
             .to_string()
             .split_once('#')
             .unwrap()

crates/wallet/src/wallet/tx_builder.rs

@@ -299,7 +299,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
                 .collect::<Result<Vec<_>, _>>()?;
 
             for utxo in utxos {
-                let descriptor = wallet.get_descriptor_for_keychain(utxo.keychain);
+                let descriptor = wallet.public_descriptor(utxo.keychain);
                 let satisfaction_weight = descriptor.max_weight_to_satisfy().unwrap();
                 self.params.utxos.push(WeightedUtxo {
                     satisfaction_weight,

crates/wallet/tests/wallet.rs

@@ -153,7 +153,7 @@ fn load_recovers_wallet() -> anyhow::Result<()> {
             );
             let secp = Secp256k1::new();
             assert_eq!(
-                *wallet.get_descriptor_for_keychain(KeychainKind::External),
+                *wallet.public_descriptor(KeychainKind::External),
                 desc.into_wallet_descriptor(&secp, wallet.network())
                     .unwrap()
                     .0
@@ -1423,7 +1423,7 @@ fn test_add_foreign_utxo() {
         .assume_checked();
     let utxo = wallet2.list_unspent().next().expect("must take!");
     let foreign_utxo_satisfaction = wallet2
-        .get_descriptor_for_keychain(KeychainKind::External)
+        .public_descriptor(KeychainKind::External)
         .max_weight_to_satisfy()
         .unwrap();
 
@@ -1499,7 +1499,7 @@ fn test_calculate_fee_with_missing_foreign_utxo() {
         .assume_checked();
     let utxo = wallet2.list_unspent().next().expect("must take!");
     let foreign_utxo_satisfaction = wallet2
-        .get_descriptor_for_keychain(KeychainKind::External)
+        .public_descriptor(KeychainKind::External)
         .max_weight_to_satisfy()
         .unwrap();
 
@@ -1524,7 +1524,7 @@ fn test_add_foreign_utxo_invalid_psbt_input() {
     let (mut wallet, _) = get_funded_wallet_wpkh();
     let outpoint = wallet.list_unspent().next().expect("must exist").outpoint;
     let foreign_utxo_satisfaction = wallet
-        .get_descriptor_for_keychain(KeychainKind::External)
+        .public_descriptor(KeychainKind::External)
         .max_weight_to_satisfy()
         .unwrap();
 
@@ -1545,7 +1545,7 @@ fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() {
     let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx.clone();
 
     let satisfaction_weight = wallet2
-        .get_descriptor_for_keychain(KeychainKind::External)
+        .public_descriptor(KeychainKind::External)
         .max_weight_to_satisfy()
         .unwrap();
 
@@ -1589,7 +1589,7 @@ fn test_add_foreign_utxo_only_witness_utxo() {
     let utxo2 = wallet2.list_unspent().next().unwrap();
 
     let satisfaction_weight = wallet2
-        .get_descriptor_for_keychain(KeychainKind::External)
+        .public_descriptor(KeychainKind::External)
         .max_weight_to_satisfy()
         .unwrap();
 
@@ -3421,7 +3421,7 @@ fn test_taproot_foreign_utxo() {
     let utxo = wallet2.list_unspent().next().unwrap();
     let psbt_input = wallet2.get_psbt_input(utxo.clone(), None, false).unwrap();
     let foreign_utxo_satisfaction = wallet2
-        .get_descriptor_for_keychain(KeychainKind::External)
+        .public_descriptor(KeychainKind::External)
         .max_weight_to_satisfy()
         .unwrap();