Merge rust-bitcoin/rust-miniscript#345: Define a new type for derived DescriptorPublicKeys

9cf065b7fafc41a0681374df8aa83baf0b9d3425 Define a new type for derived `DescriptorPublicKey`s (Alekos Filini)

Pull request description:

  After a brief chat on IRC and in PR #339, here's a quick draft for a new type specific to derived `DescriptorPublicKey`s.

  This is the same design we use in BDK, where we wrap a normal `DescriptorPublicKey` and a secp context in a new struct. This has the drawback that the struct carries a lifetime and generic for the ctx, but I couldn't think of any other way to get this to work unless we drop the implementation of `ToPublicKey` for `DerivedDescriptorKey` which in my opinion can be very useful.

ACKs for top commit:
  apoelstra:
    ACK 9cf065b7fafc41a0681374df8aa83baf0b9d3425
  sanket1729:
    ACK 9cf065b7fafc41a0681374df8aa83baf0b9d3425

Tree-SHA512: 3dd486d52c589d104da9611c65229988825cc79115a1ff45d1a39aa63e49d2d2a76b40b21a278a58d0b8566b2b37682ffc5efbbb78779ef4e5dadeaf14bbcc6c

Author: heap-coder

src/descriptor/key.rs

@@ -2,10 +2,9 @@ use std::{error, fmt, str::FromStr};
 
 use bitcoin::{
     self,
-    hashes::Hash,
+    hashes::{hash160, Hash},
     hashes::{hex::FromHex, HashEngine},
-    secp256k1,
-    secp256k1::{Secp256k1, Signing},
+    secp256k1::{Secp256k1, Signing, Verification},
     util::bip32,
     XOnlyPublicKey, XpubIdentifier,
 };
@@ -70,6 +69,15 @@ pub enum SinglePubKey {
     XOnly(XOnlyPublicKey),
 }
 
+/// A derived [`DescriptorPublicKey`]
+///
+/// Derived keys are guaranteed to never contain wildcards
+#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
+pub struct DerivedDescriptorKey {
+    key: DescriptorPublicKey,
+    index: u32,
+}
+
 impl fmt::Display for DescriptorSecretKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
@@ -434,11 +442,14 @@ impl DescriptorPublicKey {
     /// - If this key is an xpub but does not have a wildcard, returns `self`.
     /// - Otherwise, returns the derived xpub at `index` (removing the wildcard).
     ///
+    /// Since it's guaranteed that extended keys won't have wildcards, the key is returned as
+    /// [`DerivedDescriptorKey`].
+    ///
     /// # Panics
     ///
     /// If `index` ≥ 2^31
-    pub fn derive(self, index: u32) -> DescriptorPublicKey {
-        match self {
+    pub fn derive(self, index: u32) -> DerivedDescriptorKey {
+        let derived = match self {
             DescriptorPublicKey::Single(_) => self,
             DescriptorPublicKey::XPub(xpub) => {
                 let derivation_path = match xpub.wildcard {
@@ -457,7 +468,10 @@ impl DescriptorPublicKey {
                     wildcard: Wildcard::None,
                 })
             }
-        }
+        };
+
+        DerivedDescriptorKey::new(derived, index)
+            .expect("The key should not contain any wildcards at this point")
     }
 
     /// Computes the public key corresponding to this descriptor key.
@@ -472,7 +486,7 @@ impl DescriptorPublicKey {
     /// to avoid hardened derivation steps, start from a `DescriptorSecretKey`
     /// and call `to_public`, or call `TranslatePk2::translate_pk2` with
     /// some function which has access to secret key data.
-    pub fn derive_public_key<C: secp256k1::Verification>(
+    pub fn derive_public_key<C: Verification>(
         &self,
         secp: &Secp256k1<C>,
     ) -> Result<bitcoin::PublicKey, ConversionError> {
@@ -717,6 +731,70 @@ impl MiniscriptKey for DescriptorPublicKey {
     }
 }
 
+impl DerivedDescriptorKey {
+    /// Computes the raw [`bitcoin::PublicKey`] for this descriptor key.
+    ///
+    /// Will return an error if the key has any hardened derivation steps
+    /// in its path, but unlike [`DescriptorPublicKey::derive_public_key`]
+    /// this won't error in case of wildcards, because derived keys are
+    /// guaranteed to never contain one.
+    pub fn derive_public_key<C: Verification>(
+        &self,
+        secp: &Secp256k1<C>,
+    ) -> Result<bitcoin::PublicKey, ConversionError> {
+        self.key.derive_public_key(secp)
+    }
+
+    /// Return the derivation index of this key
+    pub fn index(&self) -> u32 {
+        self.index
+    }
+
+    /// Construct an instance from a descriptor key and a derivation index
+    ///
+    /// Returns `None` if the key contains a wildcard
+    fn new(key: DescriptorPublicKey, index: u32) -> Option<Self> {
+        match key {
+            DescriptorPublicKey::XPub(ref xpk) if xpk.wildcard != Wildcard::None => None,
+            k => Some(DerivedDescriptorKey { key: k, index }),
+        }
+    }
+}
+
+impl fmt::Display for DerivedDescriptorKey {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.key.fmt(f)
+    }
+}
+
+impl MiniscriptKey for DerivedDescriptorKey {
+    // This allows us to be able to derive public keys even for PkH s
+    type Hash = Self;
+
+    fn is_uncompressed(&self) -> bool {
+        self.key.is_uncompressed()
+    }
+
+    fn is_x_only_key(&self) -> bool {
+        self.key.is_x_only_key()
+    }
+
+    fn to_pubkeyhash(&self) -> Self {
+        self.clone()
+    }
+}
+
+impl ToPublicKey for DerivedDescriptorKey {
+    fn to_public_key(&self) -> bitcoin::PublicKey {
+        let secp = Secp256k1::verification_only();
+        self.key.derive_public_key(&secp).unwrap()
+    }
+
+    fn hash_to_hash160(hash: &Self) -> hash160::Hash {
+        hash.to_public_key().to_pubkeyhash()
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::{DescriptorKeyParseError, DescriptorPublicKey, DescriptorSecretKey};

src/descriptor/mod.rs

@@ -63,8 +63,8 @@ mod checksum;
 mod key;
 
 pub use self::key::{
-    ConversionError, DescriptorKeyParseError, DescriptorPublicKey, DescriptorSecretKey,
-    DescriptorXKey, InnerXKey, SinglePriv, SinglePub, SinglePubKey, Wildcard,
+    ConversionError, DerivedDescriptorKey, DescriptorKeyParseError, DescriptorPublicKey,
+    DescriptorSecretKey, DescriptorXKey, InnerXKey, SinglePriv, SinglePub, SinglePubKey, Wildcard,
 };
 
 /// Alias type for a map of public key to secret key
@@ -661,7 +661,7 @@ impl Descriptor<DescriptorPublicKey> {
     ///
     /// In most cases, you would want to use [`Self::derived_descriptor`] directly to obtain
     /// a [`Descriptor<bitcoin::PublicKey>`]
-    pub fn derive(&self, index: u32) -> Descriptor<DescriptorPublicKey> {
+    pub fn derive(&self, index: u32) -> Descriptor<DerivedDescriptorKey> {
         self.translate_pk2_infallible(|pk| pk.clone().derive(index))
     }
 
@@ -1616,18 +1616,17 @@ mod tests {
             let index = 5;
 
             // Parse descriptor
-            let mut desc_one = Descriptor::<DescriptorPublicKey>::from_str(raw_desc_one).unwrap();
-            let mut desc_two = Descriptor::<DescriptorPublicKey>::from_str(raw_desc_two).unwrap();
+            let desc_one = Descriptor::<DescriptorPublicKey>::from_str(raw_desc_one).unwrap();
+            let desc_two = Descriptor::<DescriptorPublicKey>::from_str(raw_desc_two).unwrap();
 
             // Same string formatting
             assert_eq!(desc_one.to_string(), raw_desc_one);
             assert_eq!(desc_two.to_string(), raw_desc_two);
 
-            // Derive a child if the descriptor is ranged
-            if raw_desc_one.contains("*") && raw_desc_two.contains("*") {
-                desc_one = desc_one.derive(index);
-                desc_two = desc_two.derive(index);
-            }
+            // Derive a child in case the descriptor is ranged. If it's not this won't have any
+            // effect
+            let desc_one = desc_one.derive(index);
+            let desc_two = desc_two.derive(index);
 
             // Same address
             let addr_one = desc_one
@@ -1723,7 +1722,7 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
             res_descriptor_str.parse().unwrap();
         let res_descriptor = Descriptor::new_sh(res_policy.compile().unwrap()).unwrap();
 
-        assert_eq!(res_descriptor, derived_descriptor);
+        assert_eq!(res_descriptor.to_string(), derived_descriptor.to_string());
     }
 
     #[test]