feat(multipath): Add checks for desc pair

- add check for int-desc as a multipath desc

diff --git a/src/error.rs b/src/error.rs
index 32fe765d1e34221ba0260ae57c217f651e0aa499..bfad510a34b065649f84a866f00d7e5e98190ad5 100644 (file)
--- a/src/error.rs
+++ b/src/error.rs
@@ -49,6 +49,11 @@ pub enum BDKCliError {
     #[error("LocalChain error: {0}")]
     LocalChainError(#[from] bdk_wallet::chain::local_chain::ApplyHeaderError),
 
+    #[error(
+        "The internal descriptor cannot be a multipath descriptor. Provide it as the external descriptor instead."
+    )]
+    MultipathInternalDescriptor,
+
     #[error("Miniscript error: {0}")]
     MiniscriptError(#[from] bdk_wallet::miniscript::Error),
 

diff --git a/src/handlers/config.rs b/src/handlers/config.rs
index 3409bfa34adf32f63b2c2d568a051301c3237819..13131c19433df54295afbe528306f82868cfedbe 100644 (file)
--- a/src/handlers/config.rs
+++ b/src/handlers/config.rs
@@ -14,6 +14,7 @@ use crate::handlers::Init;
 use crate::handlers::{AppCommand, AppContext};
 #[cfg(any(feature = "sqlite", feature = "redb"))]
 use crate::persister::DatabaseType;
+use crate::utils::descriptors::validate_descriptor_pair;
 use crate::utils::types::{StatusResult, WalletsListResult};
 use bdk_wallet::bitcoin::Network;
 use clap::Args;
@@ -44,6 +45,8 @@ impl AppCommand<AppContext<Init>> for SaveConfigCommand {
         let ext_descriptor = self.wallet_opts.ext_descriptor.clone();
         let int_descriptor = self.wallet_opts.int_descriptor.clone();
 
+        validate_descriptor_pair(&ext_descriptor, int_descriptor.as_deref(), ctx.network)?;
+
         if ext_descriptor.contains("xprv") || ext_descriptor.contains("tprv") {
             eprintln!(
                 "WARNING: Your external descriptor contains PRIVATE KEYS.

diff --git a/src/handlers/payjoin/db.rs b/src/handlers/payjoin/db.rs
index dd64e4d9f3ca4a48edb148110ed29a4ec12dd933..1b75f1242969ef6bdb57d012c956f13a1377a09c 100644 (file)
--- a/src/handlers/payjoin/db.rs
+++ b/src/handlers/payjoin/db.rs
@@ -534,7 +534,6 @@ mod tests {
     use std::time::{SystemTime, UNIX_EPOCH};
 
     use payjoin::HpkeKeyPair;
-    use payjoin::persist::SessionPersister as _;
     use payjoin::receive::v2::SessionOutcome as ReceiverSessionOutcome;
     use payjoin::send::v2::SessionOutcome as SenderSessionOutcome;
 

diff --git a/src/persister.rs b/src/persister.rs
index 4ff61f1a68f0cd3765f922bcdeeb90480b9b858b..2a1d1ee3d9de165403a7e31922fcfcf9873518b1 100644 (file)
--- a/src/persister.rs
+++ b/src/persister.rs
@@ -1,6 +1,6 @@
 use crate::commands::WalletOpts;
 use crate::error::BDKCliError as Error;
-use crate::utils::descriptors::is_multipath_descriptor;
+use crate::utils::descriptors::validate_descriptor_pair;
 use bdk_wallet::Wallet;
 use bdk_wallet::bitcoin::Network;
 #[cfg(any(feature = "sqlite", feature = "redb"))]
@@ -69,10 +69,8 @@ where
     let ext_descriptor = wallet_opts.ext_descriptor.clone();
     let int_descriptor = wallet_opts.int_descriptor.clone();
 
-    let ext_is_multipath = is_multipath_descriptor(&ext_descriptor, network)?;
-    if ext_is_multipath && int_descriptor.is_some() {
-        return Err(Error::AmbiguousDescriptors);
-    }
+    let ext_is_multipath =
+        validate_descriptor_pair(&ext_descriptor, int_descriptor.as_deref(), network)?;
 
     let mut wallet_load_params = Wallet::load();
     wallet_load_params = if ext_is_multipath {
@@ -118,10 +116,8 @@ pub(crate) fn new_wallet(network: Network, wallet_opts: &WalletOpts) -> Result<W
     let ext_descriptor = wallet_opts.ext_descriptor.clone();
     let int_descriptor = wallet_opts.int_descriptor.clone();
 
-    let ext_is_multipath = is_multipath_descriptor(&ext_descriptor, network)?;
-    if ext_is_multipath && int_descriptor.is_some() {
-        return Err(Error::AmbiguousDescriptors);
-    }
+    let ext_is_multipath =
+        validate_descriptor_pair(&ext_descriptor, int_descriptor.as_deref(), network)?;
 
     let builder = if let Some(int_descriptor) = int_descriptor {
         Wallet::create(ext_descriptor, int_descriptor)

diff --git a/src/utils/descriptors.rs b/src/utils/descriptors.rs
index 924041dd65dcef39aac84cfb03f8af05d6746f91..3fa0490f9d40ce21916ae93f8a68202fb67fb811 100644 (file)
--- a/src/utils/descriptors.rs
+++ b/src/utils/descriptors.rs
@@ -199,26 +199,45 @@ pub fn generate_descriptor_from_mnemonic(
     Ok(result)
 }
 
-/// Returns `true` if `descriptor` is a supported two-path BIP-389 multipath descriptor
-/// (external and internal), `false` for a normal single-path descriptor.
+/// Returns the number of derivation paths in `descriptor`.
 ///
-/// Errors if the descriptor is unparseable, or if it's a multipath descriptor with a number
-/// of paths other than two (supports only external/internal two-path multipath).
-pub fn is_multipath_descriptor(descriptor: &str, network: Network) -> Result<bool, Error> {
+/// Errors if the descriptor is unparseable or its keys don't match `network`.
+fn descriptor_path_count(descriptor: &str, network: Network) -> Result<usize, Error> {
     let secp = Secp256k1::new();
     let (descriptor, _) = descriptor.into_wallet_descriptor(&secp, network.into())?;
+    Ok(descriptor.into_single_descriptors()?.len())
+}
 
-    if !descriptor.is_multipath() {
-        return Ok(false);
-    }
-
-    let paths = descriptor.into_single_descriptors()?.len();
-    if paths != 2 {
+/// Validates the external/internal descriptor pair, returning `true` if `ext_descriptor` is a
+/// supported two-path BIP-389 multipath descriptor.
+///
+/// Errors if either descriptor is unparseable or doesn't match `network`, if `ext_descriptor` is
+/// a multipath descriptor with a number of paths other than two (only external/internal two-path
+/// multipath is supported), if a multipath `ext_descriptor` is paired with a separate
+/// `int_descriptor`, or if `int_descriptor` is itself a multipath descriptor.
+pub fn validate_descriptor_pair(
+    ext_descriptor: &str,
+    int_descriptor: Option<&str>,
+    network: Network,
+) -> Result<bool, Error> {
+    let ext_paths = descriptor_path_count(ext_descriptor, network)?;
+    if ext_paths > 2 {
         return Err(Error::Generic(format!(
-            "Unsupported multipath descriptor: expected exactly 2 paths (external/internal), found {paths}."
+            "Unsupported multipath descriptor: expected exactly 2 paths (external/internal), found {ext_paths}."
         )));
     }
-    Ok(true)
+    let ext_is_multipath = ext_paths == 2;
+
+    if let Some(int_descriptor) = int_descriptor {
+        if ext_is_multipath {
+            return Err(Error::AmbiguousDescriptors);
+        }
+        if descriptor_path_count(int_descriptor, network)? > 1 {
+            return Err(Error::MultipathInternalDescriptor);
+        }
+    }
+
+    Ok(ext_is_multipath)
 }
 
 #[cfg(test)]
@@ -226,26 +245,18 @@ mod multipath_tests {
     use super::*;
 
     const MULTIPATH: &str = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/<0;1>/*)";
-    const THREE_PATH: &str = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/<0;1;2>/*)";
     const SINGLE: &str = "wpkh([07234a14/84'/1'/0']tpubDCSgT6PaVLQH9h2TAxKryhvkEurUBcYRJc9dhTcMDyahhWiMWfEWvQQX89yaw7w7XU8bcVujoALfxq59VkFATri3Cxm5mkp9kfHfRFDckEh/0/*)#429nsxmg";
 
     #[test]
-    fn detects_multipath() {
-        assert!(is_multipath_descriptor(MULTIPATH, Network::Testnet).unwrap());
-    }
-
-    #[test]
-    fn detects_single_path() {
-        assert!(!is_multipath_descriptor(SINGLE, Network::Testnet).unwrap());
-    }
-
-    #[test]
-    fn rejects_unparseable() {
-        assert!(is_multipath_descriptor("not a descriptor", Network::Testnet).is_err());
+    fn rejects_multipath_with_internal_descriptor() {
+        assert!(matches!(
+            validate_descriptor_pair(MULTIPATH, Some(SINGLE), Network::Testnet),
+            Err(Error::AmbiguousDescriptors)
+        ));
     }
 
     #[test]
-    fn rejects_more_than_two_paths() {
-        assert!(is_multipath_descriptor(THREE_PATH, Network::Testnet).is_err());
+    fn accepts_single_path_with_internal_descriptor() {
+        assert!(!validate_descriptor_pair(SINGLE, Some(SINGLE), Network::Testnet).unwrap());
     }
 }

diff --git a/tests/integration/offline.rs b/tests/integration/offline.rs
index 2fceefc30e9088e231f68bd3c1f890b2f6937171..27b6bd160c8d11861cf51db3372517f28d8d3dc4 100644 (file)
--- a/tests/integration/offline.rs
+++ b/tests/integration/offline.rs
@@ -417,18 +417,19 @@ mod multipath_tests {
     }
 
     #[test]
-    fn multipath_with_internal_is_ambiguous() {
+    fn multipath_with_internal_is_rejected_at_config_time() {
         let tmp = TempDir::new().unwrap();
         let cli = BdkCli::new("testnet", Some(tmp.path().to_path_buf()));
         save_config(&cli, "multipath_wallet", MULTIPATH_DESC, Some(INT_DESC))
-            .assert()
-            .success();
-
-        cli.wallet_cmd(&["--wallet", "multipath_wallet", "new_address"])
             .assert()
             .failure()
             .stderr(predicate::str::contains(
                 "multipath descriptor and a separate internal descriptor",
             ));
+
+        // Nothing was written, so the wallet does not exist.
+        cli.wallet_cmd(&["--wallet", "multipath_wallet", "new_address"])
+            .assert()
+            .failure();
     }
 }