desc: check for mismatch in the number of derivation paths between multipath keys

This is invalid by BIP389 (as combinations would blowup when getting
single descriptors otherwise).

We add a num_der_paths method to MiniscriptKey in order to be able to
check for this at parsing time without specializing from_str for
Descriptor<DescriptorPublicKey>.

Author: darosior

src/descriptor/key.rs

@@ -977,6 +977,14 @@ impl MiniscriptKey for DescriptorPublicKey {
             _ => false,
         }
     }
+
+    fn num_der_paths(&self) -> usize {
+        match self {
+            DescriptorPublicKey::Single(_) => 0,
+            DescriptorPublicKey::XPub(_) => 1,
+            DescriptorPublicKey::MultiXPub(xpub) => xpub.derivation_paths.len(),
+        }
+    }
 }
 
 impl DefiniteDescriptorKey {
@@ -1070,6 +1078,10 @@ impl MiniscriptKey for DefiniteDescriptorKey {
     fn is_x_only_key(&self) -> bool {
         self.0.is_x_only_key()
     }
+
+    fn num_der_paths(&self) -> usize {
+        self.0.num_der_paths()
+    }
 }
 
 impl ToPublicKey for DefiniteDescriptorKey {
@@ -1110,7 +1122,7 @@ mod test {
 
     use super::{
         DescriptorKeyParseError, DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey,
-        Wildcard,
+        MiniscriptKey, Wildcard,
     };
     use crate::prelude::*;
 
@@ -1297,8 +1309,12 @@ mod test {
         );
     }
 
-    fn get_multipath_xpub(key_str: &str) -> DescriptorMultiXKey<bip32::ExtendedPubKey> {
+    fn get_multipath_xpub(
+        key_str: &str,
+        num_paths: usize,
+    ) -> DescriptorMultiXKey<bip32::ExtendedPubKey> {
         let desc_key = DescriptorPublicKey::from_str(key_str).unwrap();
+        assert_eq!(desc_key.num_der_paths(), num_paths);
         match desc_key {
             DescriptorPublicKey::MultiXPub(xpub) => xpub,
             _ => unreachable!(),
@@ -1318,7 +1334,7 @@ mod test {
         let secp = secp256k1::Secp256k1::signing_only();
 
         // We can have a key in a descriptor that has multiple paths
-        let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;42;9854>");
+        let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;42;9854>", 4);
         assert_eq!(
             xpub.derivation_paths,
             vec![
@@ -1333,7 +1349,7 @@ mod test {
             get_multipath_xpub(&DescriptorPublicKey::MultiXPub(xpub.clone()).to_string(), 4)
         );
         // Even if it's in the middle of the derivation path.
-        let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/0/5/10");
+        let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/0/5/10", 3);
         assert_eq!(
             xpub.derivation_paths,
             vec![
@@ -1347,7 +1363,7 @@ mod test {
             get_multipath_xpub(&DescriptorPublicKey::MultiXPub(xpub.clone()).to_string(), 3)
         );
         // Even if it is a wildcard extended key.
-        let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/3456/9876/*");
+        let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/3456/9876/*", 3);
         assert_eq!(xpub.wildcard, Wildcard::Unhardened);
         assert_eq!(
             xpub.derivation_paths,
@@ -1362,7 +1378,7 @@ mod test {
             get_multipath_xpub(&DescriptorPublicKey::MultiXPub(xpub.clone()).to_string(), 3)
         );
         // Also even if it has an origin.
-        let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/<0;1>/*");
+        let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/<0;1>/*", 2);
         assert_eq!(xpub.wildcard, Wildcard::Unhardened);
         assert_eq!(
             xpub.derivation_paths,
@@ -1377,7 +1393,7 @@ mod test {
         );
         // Also if it has hardened steps in the derivation path. In fact, it can also have hardened
         // indexes even at the step with multiple indexes!
-        let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/9478'/<0';1h>/8h/*'");
+        let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/9478'/<0';1h>/8h/*'", 2);
         assert_eq!(xpub.wildcard, Wildcard::Hardened);
         assert_eq!(
             xpub.derivation_paths,

src/descriptor/mod.rs

@@ -791,6 +791,42 @@ impl Descriptor<DescriptorPublicKey> {
     }
 }
 
+impl<Pk: MiniscriptKey> Descriptor<Pk> {
+    /// Whether this descriptor is a multipath descriptor that contains any 2 multipath keys
+    /// with a different number of derivation paths.
+    /// Such a descriptor is invalid according to BIP389.
+    pub fn multipath_length_mismatch(&self) -> bool {
+        // (Ab)use `for_each_key` to record the number of derivation paths a multipath key has.
+        #[derive(PartialEq)]
+        enum MultipathLenChecker {
+            SinglePath,
+            MultipathLen(usize),
+            LenMismatch,
+        }
+
+        let mut checker = MultipathLenChecker::SinglePath;
+        self.for_each_key(|key| {
+            match key.num_der_paths() {
+                0 | 1 => {}
+                n => match checker {
+                    MultipathLenChecker::SinglePath => {
+                        checker = MultipathLenChecker::MultipathLen(n);
+                    }
+                    MultipathLenChecker::MultipathLen(len) => {
+                        if len != n {
+                            checker = MultipathLenChecker::LenMismatch;
+                        }
+                    }
+                    MultipathLenChecker::LenMismatch => {}
+                },
+            }
+            true
+        });
+
+        checker == MultipathLenChecker::LenMismatch
+    }
+}
+
 impl Descriptor<DefiniteDescriptorKey> {
     /// Convert all the public keys in the descriptor to [`bitcoin::PublicKey`] by deriving them or
     /// otherwise converting them. All [`bitcoin::XOnlyPublicKey`]s are converted to by adding a
@@ -861,13 +897,19 @@ impl_from_str!(
         // tr tree parsing has special code
         // Tr::from_str will check the checksum
         // match "tr(" to handle more extensibly
-        if s.starts_with("tr(") {
+        let desc = if s.starts_with("tr(") {
             Ok(Descriptor::Tr(Tr::from_str(s)?))
         } else {
             let desc_str = verify_checksum(s)?;
             let top = expression::Tree::from_str(desc_str)?;
             expression::FromTree::from_tree(&top)
+        }?;
+
+        if desc.multipath_length_mismatch() {
+            return Err(Error::MultipathDescLenMismatch);
         }
+
+        Ok(desc)
     }
 );
 
@@ -1911,6 +1953,7 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
         // We can parse a multipath descriptors, and make it into separate single-path descriptors.
         let desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<7';8h;20>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/<0;1;987>/*)))").unwrap();
         assert!(desc.is_multipath());
+        assert!(!desc.multipath_length_mismatch());
         assert_eq!(desc.into_single_descriptors().unwrap(), vec![
             Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/7'/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/0/*)))").unwrap(),
             Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/8h/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/1/*)))").unwrap(),
@@ -1920,6 +1963,7 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
         // Even if only one of the keys is multipath.
         let desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap();
         assert!(desc.is_multipath());
+        assert!(!desc.multipath_length_mismatch());
         assert_eq!(desc.into_single_descriptors().unwrap(), vec![
             Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/0/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap(),
             Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/1/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap(),
@@ -1928,9 +1972,14 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
         // We can detect regular single-path descriptors.
         let notmulti_desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap();
         assert!(!notmulti_desc.is_multipath());
+        assert!(!notmulti_desc.multipath_length_mismatch());
         assert_eq!(
             notmulti_desc.clone().into_single_descriptors().unwrap(),
             vec![notmulti_desc]
         );
+
+        // We refuse to parse multipath descriptors with a mismatch in the number of derivation paths between keys.
+        Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2;3;4>/*)))").unwrap_err();
+        Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1;2;3>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2>/*)))").unwrap_err();
     }
 }

src/interpreter/mod.rs

@@ -138,6 +138,10 @@ impl MiniscriptKey for BitcoinKey {
     type Hash256 = hash256::Hash;
     type Ripemd160 = ripemd160::Hash;
     type Hash160 = hash160::Hash;
+
+    fn num_der_paths(&self) -> usize {
+        0
+    }
 }
 
 impl<'txin> Interpreter<'txin> {

src/lib.rs

@@ -162,6 +162,10 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha
         false
     }
 
+    /// Returns the number of different derivation paths in this key. Only >1 for keys
+    /// in BIP389 multipath descriptors.
+    fn num_der_paths(&self) -> usize;
+
     /// The associated [`sha256::Hash`] for this [`MiniscriptKey`],
     /// used in the hash256 fragment.
     type Sha256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash;
@@ -183,6 +187,10 @@ impl MiniscriptKey for bitcoin::secp256k1::PublicKey {
     type Hash256 = hash256::Hash;
     type Ripemd160 = ripemd160::Hash;
     type Hash160 = hash160::Hash;
+
+    fn num_der_paths(&self) -> usize {
+        0
+    }
 }
 
 impl MiniscriptKey for bitcoin::PublicKey {
@@ -191,6 +199,10 @@ impl MiniscriptKey for bitcoin::PublicKey {
         !self.compressed
     }
 
+    fn num_der_paths(&self) -> usize {
+        0
+    }
+
     type Sha256 = sha256::Hash;
     type Hash256 = hash256::Hash;
     type Ripemd160 = ripemd160::Hash;
@@ -206,13 +218,21 @@ impl MiniscriptKey for bitcoin::secp256k1::XOnlyPublicKey {
     fn is_x_only_key(&self) -> bool {
         true
     }
+
+    fn num_der_paths(&self) -> usize {
+        0
+    }
 }
 
 impl MiniscriptKey for String {
     type Sha256 = String; // specify hashes as string
     type Hash256 = String;
     type Ripemd160 = String;
     type Hash160 = String;
+
+    fn num_der_paths(&self) -> usize {
+        0
+    }
 }
 
 /// Trait describing public key types which can be converted to bitcoin pubkeys
@@ -345,6 +365,10 @@ impl MiniscriptKey for DummyKey {
     type Hash256 = DummyHash256Hash;
     type Ripemd160 = DummyRipemd160Hash;
     type Hash160 = DummyHash160Hash;
+
+    fn num_der_paths(&self) -> usize {
+        0
+    }
 }
 
 impl hash::Hash for DummyKey {