Merge #293: Follow up to v21 TODO

tcharding · tcharding · commit f00ea0f64f55 · 2025-07-25T08:44:38.000+10:00
e396ad0 Improve import_descriptors test (Jamil Lambert, PhD) 746c5dc Add a constructor for ImportDescriptorsRequest (Jamil Lambert, PhD) 4f534f3 Rewrite create descriptor wallet client macro (Jamil Lambert, PhD) 62e01e5 Improve getindexinfo test (Jamil Lambert, PhD) Pull request description: There were a couple of points in the review of #289 that needed to be addressed. Improve the tests as suggested, and add a comment to the rustdocs. Redefine the `create_wallet` client macro so that for v21 and v22 where the default is a legacy wallet there is a separate function to create a descriptor wallet. Similarly for v23 and above where the default is a descriptor wallet there is a separate function to create a legacy wallet. Closes #292 ACKs for top commit: tcharding: ACK e396ad0 Tree-SHA512: e035315a926e9a789068a72b0e21f838f30d5c7feb2eba7245dbeea73f500b0c433a11b24620e5141550199a3173cabf3bb478b174d34194806f89b020b8eb05
diff --git a/client/src/client_sync/v21/mod.rs b/client/src/client_sync/v21/mod.rs
@@ -128,8 +128,7 @@ crate::impl_client_v17__abort_rescan!();
 crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
-crate::impl_client_v17__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
+crate::impl_client_v21__create_wallet!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
@@ -193,3 +192,10 @@ pub struct ImportDescriptorsRequest {
     /// Time from which to start rescanning the blockchain for this descriptor, in UNIX epoch time or "now".
     pub timestamp: serde_json::Value,
 }
+
+impl ImportDescriptorsRequest {
+    /// Constructs a new ImportDescriptorsRequest.
+    pub fn new(descriptor: impl Into<String>, timestamp: impl Into<serde_json::Value>) -> Self {
+        ImportDescriptorsRequest { descriptor: descriptor.into(), timestamp: timestamp.into() }
+    }
+}
diff --git a/client/src/client_sync/v21/wallet.rs b/client/src/client_sync/v21/wallet.rs
@@ -9,22 +9,51 @@
 //!
 //! See or use the `define_jsonrpc_minreq_client!` macro to define a `Client`.
 
-/// Implements Bitcoin Core JSON-RPC API method `createwallet` with descriptors=true (descriptor wallet).
+/// Implements Bitcoin Core JSON-RPC API method `createwallet`.
 #[macro_export]
-macro_rules! impl_client_v21__create_wallet_with_descriptors {
+macro_rules! impl_client_v21__create_wallet {
     () => {
         impl Client {
-            pub fn create_wallet_with_descriptors(&self, wallet: &str) -> Result<CreateWallet> {
-                let args = [
-                    wallet.into(),
-                    false.into(),            // disable_private_keys
-                    false.into(),            // blank
-                    serde_json::Value::Null, // passphrase
-                    false.into(),            // avoid_reuse
-                    true.into(),             // descriptors=true
-                    serde_json::Value::Null, // load_on_startup
-                ];
-                self.call("createwallet", &args)
+            /// Calls `createwallet` with `wallet` as the only argument.
+            ///
+            /// In v21 and v22 this creates a legacy wallet. Use `create_descriptor_wallet` to create
+            /// a descriptor wallet.
+            pub fn create_wallet(&self, wallet: &str) -> Result<CreateWallet> {
+                self.call("createwallet", &[wallet.into()])
+            }
+
+            /// Creates a wallet with descriptors=true (descriptor wallet).
+            ///
+            /// > createwallet "wallet_name" ( disable_private_keys blank "passphrase" avoid_reuse descriptors load_on_startup )
+            /// >
+            /// > Creates and loads a new wallet.
+            /// >
+            /// > Arguments:
+            /// > 1. wallet_name             (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.
+            /// > 2. disable_private_keys    (boolean, optional, default=false) Disable the possibility of private keys (only watchonlys are possible in this mode).
+            /// > 3. blank                   (boolean, optional, default=false) Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed.
+            /// > 4. passphrase              (string, optional) Encrypt the wallet with this passphrase.
+            /// > 5. avoid_reuse             (boolean, optional, default=false) Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind.
+            /// > 6. descriptors             (boolean, optional, default=true) Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation
+            /// > 7. load_on_startup         (boolean, optional) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
+            pub fn create_descriptor_wallet(&self, wallet: &str) -> Result<CreateWallet> {
+                let disable_private_keys = false;
+                let blank = false;
+                let passphrase = String::new();
+                let avoid_reuse = false;
+                let descriptors = true;
+
+                self.call(
+                    "createwallet",
+                    &[
+                        wallet.into(),
+                        disable_private_keys.into(),
+                        blank.into(),
+                        passphrase.into(),
+                        avoid_reuse.into(),
+                        descriptors.into(),
+                    ],
+                )
             }
         }
     };
diff --git a/client/src/client_sync/v22/mod.rs b/client/src/client_sync/v22/mod.rs
@@ -130,8 +130,7 @@ crate::impl_client_v17__abort_rescan!();
 crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
-crate::impl_client_v17__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
+crate::impl_client_v21__create_wallet!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/client/src/client_sync/v23/mod.rs b/client/src/client_sync/v23/mod.rs
@@ -134,7 +134,6 @@ crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
 crate::impl_client_v23__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/client/src/client_sync/v23/wallet.rs b/client/src/client_sync/v23/wallet.rs
@@ -15,6 +15,9 @@ macro_rules! impl_client_v23__create_wallet {
     () => {
         impl Client {
             /// Calls `createwallet` with `wallet` as the only argument.
+            ///
+            /// In v23 and later this creates a descriptor wallet. Use `create_legacy_wallet` to create
+            /// a legacy wallet.
             pub fn create_wallet(&self, wallet: &str) -> Result<CreateWallet> {
                 self.call("createwallet", &[wallet.into()])
             }
diff --git a/client/src/client_sync/v24/mod.rs b/client/src/client_sync/v24/mod.rs
@@ -131,7 +131,6 @@ crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
 crate::impl_client_v23__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/client/src/client_sync/v25/mod.rs b/client/src/client_sync/v25/mod.rs
@@ -133,7 +133,6 @@ crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
 crate::impl_client_v23__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/client/src/client_sync/v26/mod.rs b/client/src/client_sync/v26/mod.rs
@@ -137,7 +137,6 @@ crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
 crate::impl_client_v23__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/client/src/client_sync/v27/mod.rs b/client/src/client_sync/v27/mod.rs
@@ -133,7 +133,6 @@ crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
 crate::impl_client_v23__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/client/src/client_sync/v28/mod.rs b/client/src/client_sync/v28/mod.rs
@@ -135,7 +135,6 @@ crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
 crate::impl_client_v23__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/client/src/client_sync/v29/mod.rs b/client/src/client_sync/v29/mod.rs
@@ -135,7 +135,6 @@ crate::impl_client_v17__add_multisig_address!();
 crate::impl_client_v17__backup_wallet!();
 crate::impl_client_v17__bump_fee!();
 crate::impl_client_v23__create_wallet!();
-crate::impl_client_v21__create_wallet_with_descriptors!();
 crate::impl_client_v17__dump_priv_key!();
 crate::impl_client_v17__dump_wallet!();
 crate::impl_client_v17__encrypt_wallet!();
diff --git a/integration_test/tests/util.rs b/integration_test/tests/util.rs
@@ -66,8 +66,11 @@ fn util__get_descriptor_info() {
 #[cfg(not(feature = "v20_and_below"))]
 #[test]
 fn util__get_index_info() {
-    let node = Node::with_wallet(Wallet::Default, &[]);
-    let _: GetIndexInfo = node.client.get_index_info().expect("getindexinfo");
+    let node = Node::with_wallet(Wallet::Default, &["-txindex"]);
+    let index_info: GetIndexInfo = node.client.get_index_info().expect("getindexinfo");
+
+    let txindex_info = index_info.0.get("txindex").unwrap();
+    assert!(txindex_info.best_block_height < u32::MAX, "best_block_height should be a valid block height");
 }
 
 #[test]
diff --git a/integration_test/tests/wallet.rs b/integration_test/tests/wallet.rs
@@ -3,14 +3,19 @@
 //! Tests for methods found under the `== Wallet ==` section of the API docs.
 
 #![allow(non_snake_case)] // Test names intentionally use double underscore.
+#![allow(unused_imports)] // Some imports are only used in specific versions.
 
-#[cfg(feature = "TODO")]
-use bitcoin::address::{Address, NetworkChecked};
-use bitcoin::{Amount, FeeRate, PrivateKey, PublicKey};
+use bitcoin::address::{Address, KnownHrp, NetworkChecked};
+use bitcoin::{secp256k1, Amount, CompressedPublicKey, FeeRate, PrivateKey, PublicKey};
 use integration_test::{Node, NodeExt as _, Wallet};
 use node::{mtype, AddressType, ImportMultiRequest, ImportMultiScriptPubKey, ImportMultiTimestamp};
+
+#[cfg(not(feature = "v20_and_below"))]
+use node::ImportDescriptorsRequest;
+
 use node::vtype::*;             // All the version specific types.
 use std::fs;
+use std::time::{SystemTime, UNIX_EPOCH};
 
 #[test]
 fn wallet__abandon_transaction() {
@@ -326,21 +331,47 @@ fn wallet__import_address() {
 #[test]
 #[cfg(not(feature = "v20_and_below"))]
 fn wallet__import_descriptors() {
-    use node::{serde_json, ImportDescriptorsRequest};
-
     let node = Node::with_wallet(Wallet::None, &[]);
     let wallet_name = "desc_wallet";
-    node.client.create_wallet_with_descriptors(wallet_name).expect("create descriptor wallet");
 
-    let address = node.client.new_address().expect("failed to get new address");
-    let descriptor = format!("addr({})", address);
+    #[cfg(feature = "v22_and_below")]
+    node.client.create_descriptor_wallet(wallet_name).expect("create descriptor wallet");
 
-    let request = ImportDescriptorsRequest {
-        descriptor,
-        timestamp: serde_json::Value::String("now".to_string()),
-    };
+    // v23 onwards uses descriptor wallets by default.
+    #[cfg(not(feature = "v22_and_below"))]
+    node.client.create_wallet(wallet_name).expect("create wallet");
+
+    node.fund_wallet();
+
+    // 1. Get the current time
+    let start_time = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .expect("failed to get current time")
+        .as_secs();
+
+    // 2. Use a known private key, derive the address from it and send some coins to it.
+    let privkey =
+        PrivateKey::from_wif("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy").unwrap();
+    let secp = secp256k1::Secp256k1::new();
+    let pubkey = privkey.public_key(&secp);
+    let address = Address::p2wpkh(&CompressedPublicKey(pubkey.inner), KnownHrp::Regtest);
+    let amount = Amount::from_sat(10_000);
+    let _txid = node.client.send_to_address(&address, amount).expect("sendtoaddress");
+
+    // 3. Get the descriptor from the private key.
+    let raw_descriptor = format!("wpkh({})", privkey.to_wif());
+    let info = node.client.get_descriptor_info(&raw_descriptor).expect("get_descriptor_info");
+    let descriptor = format!("{}#{}", raw_descriptor, info.checksum);
+
+    // 4. Mine 100 blocks
+    let mining_address = node.client.new_address().expect("failed to get mining address");
+    let _blocks = node.client.generate_to_address(100, &mining_address).expect("generatetoaddress");
 
-    let _: ImportDescriptors = node.client.import_descriptors(&[request]).expect("importdescriptors");
+    // 5. Scan for the descriptor using the time from (1)
+    let request = ImportDescriptorsRequest::new(descriptor, start_time);
+    let result: ImportDescriptors = node.client.import_descriptors(&[request]).expect("importdescriptors");
+    assert_eq!(result.0.len(), 1, "should have exactly one import result");
+    assert!(result.0[0].success);
 }
 
 #[test]
@@ -515,7 +546,7 @@ fn wallet__list_descriptors() {
     let wallet_name = "desc_wallet";
 
     #[cfg(feature = "v22_and_below")]
-    node.client.create_wallet_with_descriptors(wallet_name).expect("create descriptor wallet");
+    node.client.create_descriptor_wallet(wallet_name).expect("create descriptor wallet");
 
     // v23 onwards uses descriptor wallets by default.
     #[cfg(not(feature = "v22_and_below"))]
