Make DerivedDescriptorKey first-class citizen

Earlier we introduced DerivedDescriptorKey (an descriptor whose
wildcards are known to have been replaced with derivation indexes) to
eliminate wildcard derivation errors. This commit attempts to realise the
full benefits of this idea. I tried to keep backwards compatibility.

The naming of things no longer follows the logic of code so I'm going to
fix that in a follow up.

Author: LLFourn

examples/xpub_descriptors.rs

@@ -18,7 +18,7 @@ use std::str::FromStr;
 
 use miniscript::bitcoin::secp256k1::{Secp256k1, Verification};
 use miniscript::bitcoin::{Address, Network};
-use miniscript::{Descriptor, DescriptorPublicKey};
+use miniscript::{DerivedDescriptorKey, Descriptor, DescriptorPublicKey};
 
 const XPUB_1: &str = "xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB";
 const XPUB_2: &str = "xpub69H7F5d8KSRgmmdJg2KhpAK8SR3DjMwAdkxj3ZuxV27CprR9LgpeyGmXUbC6wb7ERfvrnKZjXoUmmDznezpbZb7ap6r1D3tgFxHmwMkQTPH";
@@ -40,9 +40,9 @@ fn p2wsh<C: Verification>(secp: &Secp256k1<C>) -> Address {
     let s = format!("wsh(sortedmulti(1,{},{}))", XPUB_1, XPUB_2);
     // let s = format!("wsh(sortedmulti(1,{},{}))", XPUB_2, XPUB_1);
 
-    let address = Descriptor::<DescriptorPublicKey>::from_str(&s)
+    let address = Descriptor::<DerivedDescriptorKey>::from_str(&s)
         .unwrap()
-        .derived_descriptor(&secp, 0) // dummy index value if it not a wildcard
+        .derived_descriptor(&secp)
         .unwrap()
         .address(Network::Bitcoin)
         .unwrap();

src/descriptor/key.rs

@@ -70,14 +70,9 @@ pub enum SinglePubKey {
     XOnly(XOnlyPublicKey),
 }
 
-/// A derived [`DescriptorPublicKey`]
-///
-/// Derived keys are guaranteed to never contain wildcards
+/// A [`DescriptorPublicKey`] without any wildcards.
 #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
-pub struct DerivedDescriptorKey {
-    key: DescriptorPublicKey,
-    index: u32,
-}
+pub struct DerivedDescriptorKey(DescriptorPublicKey);
 
 impl fmt::Display for DescriptorSecretKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -357,22 +352,14 @@ impl FromStr for DescriptorPublicKey {
 /// Descriptor key conversion error
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)]
 pub enum ConversionError {
-    /// Attempted to convert a key with a wildcard to a bitcoin public key
-    Wildcard,
     /// Attempted to convert a key with hardened derivations to a bitcoin public key
     HardenedChild,
-    /// Attempted to convert a key with a hardened wildcard to a bitcoin public key
-    HardenedWildcard,
 }
 
 impl fmt::Display for ConversionError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.write_str(match *self {
-            ConversionError::Wildcard => "uninstantiated wildcard in bip32 path",
             ConversionError::HardenedChild => "hardened child step in bip32 path",
-            ConversionError::HardenedWildcard => {
-                "hardened and uninstantiated wildcard in bip32 path"
-            }
         })
     }
 }
@@ -383,7 +370,7 @@ impl error::Error for ConversionError {
         use self::ConversionError::*;
 
         match self {
-            Wildcard | HardenedChild | HardenedWildcard => None,
+            HardenedChild => None,
         }
     }
 }
@@ -455,10 +442,7 @@ impl DescriptorPublicKey {
     ///
     /// - If this key is not an xpub, returns `self`.
     /// - 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`].
+    /// - Otherwise, returns the xpub at derivation `index` (removing the wildcard).
     ///
     /// # Panics
     ///
@@ -469,12 +453,12 @@ impl DescriptorPublicKey {
             DescriptorPublicKey::XPub(xpub) => {
                 let derivation_path = match xpub.wildcard {
                     Wildcard::None => xpub.derivation_path,
-                    Wildcard::Unhardened => xpub
-                        .derivation_path
-                        .into_child(bip32::ChildNumber::from_normal_idx(index).unwrap()),
-                    Wildcard::Hardened => xpub
-                        .derivation_path
-                        .into_child(bip32::ChildNumber::from_hardened_idx(index).unwrap()),
+                    Wildcard::Unhardened => xpub.derivation_path.into_child(
+                        bip32::ChildNumber::from_normal_idx(index).expect("index must < 2^31"),
+                    ),
+                    Wildcard::Hardened => xpub.derivation_path.into_child(
+                        bip32::ChildNumber::from_hardened_idx(index).expect("index must < 2^31"),
+                    ),
                 };
                 DescriptorPublicKey::XPub(DescriptorXKey {
                     origin: xpub.origin,
@@ -485,7 +469,7 @@ impl DescriptorPublicKey {
             }
         };
 
-        DerivedDescriptorKey::new(derived, index)
+        DerivedDescriptorKey::new(derived)
             .expect("The key should not contain any wildcards at this point")
     }
 
@@ -494,13 +478,10 @@ impl DescriptorPublicKey {
     /// and returns the obtained full [`bitcoin::PublicKey`]. All BIP32 derivations
     /// always return a compressed key
     ///
-    /// Will return an error if the descriptor key has any hardened
-    /// derivation steps in its path, or if the key has any wildcards.
+    /// Will return an error if the descriptor key has any hardened derivation steps in its path. To
+    /// avoid this error you should replace any such public keys first with [`translate_pk`].
     ///
-    /// To ensure there are no wildcards, call `.derive(0)` or similar;
-    /// 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.
+    /// [`translate_pk`]: crate::TranslatePk::translate_pk
     pub fn derive_public_key<C: Verification>(
         &self,
         secp: &Secp256k1<C>,
@@ -511,8 +492,9 @@ impl DescriptorPublicKey {
                 SinglePubKey::XOnly(xpk) => Ok(xpk.to_public_key()),
             },
             DescriptorPublicKey::XPub(ref xpk) => match xpk.wildcard {
-                Wildcard::Unhardened => Err(ConversionError::Wildcard),
-                Wildcard::Hardened => Err(ConversionError::HardenedWildcard),
+                Wildcard::Unhardened | Wildcard::Hardened => {
+                    unreachable!("we've excluded this error case")
+                }
                 Wildcard::None => match xpk.xkey.derive_pub(secp, &xpk.derivation_path.as_ref()) {
                     Ok(xpub) => Ok(bitcoin::PublicKey::new(xpub.public_key)),
                     Err(bip32::Error::CannotDeriveFromHardenedKey) => {
@@ -778,28 +760,47 @@ impl DerivedDescriptorKey {
         &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
+        self.0.derive_public_key(secp)
     }
 
     /// 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 }),
+    fn new(key: DescriptorPublicKey) -> Option<Self> {
+        if key.is_deriveable() {
+            None
+        } else {
+            Some(Self(key))
         }
     }
+
+    /// The fingerprint of the master key associated with this key, `0x00000000` if none.
+    pub fn master_fingerprint(&self) -> bip32::Fingerprint {
+        self.0.master_fingerprint()
+    }
+
+    /// Full path, from the master key
+    pub fn full_derivation_path(&self) -> bip32::DerivationPath {
+        self.0.full_derivation_path()
+    }
+}
+
+impl FromStr for DerivedDescriptorKey {
+    type Err = DescriptorKeyParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let inner = DescriptorPublicKey::from_str(s)?;
+        Ok(
+            DerivedDescriptorKey::new(inner).ok_or(DescriptorKeyParseError(
+                "cannot parse key with a wilcard as a DerivedDescriptorKey",
+            ))?,
+        )
+    }
 }
 
 impl fmt::Display for DerivedDescriptorKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.key.fmt(f)
+        self.0.fmt(f)
     }
 }
 
@@ -812,11 +813,11 @@ impl MiniscriptKey for DerivedDescriptorKey {
     type Hash160 = hash160::Hash;
 
     fn is_uncompressed(&self) -> bool {
-        self.key.is_uncompressed()
+        self.0.is_uncompressed()
     }
 
     fn is_x_only_key(&self) -> bool {
-        self.key.is_x_only_key()
+        self.0.is_x_only_key()
     }
 
     fn to_pubkeyhash(&self) -> Self {
@@ -827,7 +828,7 @@ impl MiniscriptKey for DerivedDescriptorKey {
 impl ToPublicKey for DerivedDescriptorKey {
     fn to_public_key(&self) -> bitcoin::PublicKey {
         let secp = Secp256k1::verification_only();
-        self.key.derive_public_key(&secp).unwrap()
+        self.0.derive_public_key(&secp).unwrap()
     }
 
     fn hash_to_hash160(hash: &Self) -> hash160::Hash {
@@ -851,6 +852,12 @@ impl ToPublicKey for DerivedDescriptorKey {
     }
 }
 
+impl From<DerivedDescriptorKey> for DescriptorPublicKey {
+    fn from(d: DerivedDescriptorKey) -> Self {
+        d.0
+    }
+}
+
 #[cfg(test)]
 mod test {
     use core::str::FromStr;

src/descriptor/mod.rs

@@ -539,27 +539,29 @@ impl Descriptor<DescriptorPublicKey> {
             .expect("BIP 32 key index substitution cannot fail")
     }
 
-    /// Derive a [`Descriptor`] with a concrete [`bitcoin::PublicKey`] at a given index
-    /// Removes all extended pubkeys and wildcards from the descriptor and only leaves
-    /// concrete [`bitcoin::PublicKey`]. All [`bitcoin::XOnlyPublicKey`]s are converted
-    /// to [`bitcoin::PublicKey`]s by adding a default(0x02) y-coordinate. For [`Tr`]
-    /// descriptor, spend info is also cached.
+    /// 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
+    /// default(0x02) y-coordinate.
     ///
-    /// # Examples
+    /// This is a shorthand for:
     ///
     /// ```
-    /// use miniscript::descriptor::{Descriptor, DescriptorPublicKey};
-    /// use miniscript::bitcoin::secp256k1;
-    /// use std::str::FromStr;
-    ///
-    /// // test from bip 86
-    /// let secp = secp256k1::Secp256k1::verification_only();
-    /// let descriptor = Descriptor::<DescriptorPublicKey>::from_str("tr(xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)")
+    /// # use miniscript::{Descriptor, DescriptorPublicKey, bitcoin::secp256k1::Secp256k1};
+    /// # use core::str::FromStr;
+    /// # let descriptor = Descriptor::<DescriptorPublicKey>::from_str("tr(xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)")
     ///     .expect("Valid ranged descriptor");
-    /// let result = descriptor.derived_descriptor(&secp, 0).expect("Non-hardened derivation");
-    /// assert_eq!(result.to_string(), "tr(03cc8a4bc64d897bddc5fbc2f670f7a8ba0b386779106cf1223c6fc5d7cd6fc115)#6qm9h8ym");
+    /// # let index = 42;
+    /// # let secp = Secp256k1::verification_only();
+    /// let derived_descriptor = descriptor.derive(index).derived_descriptor(&secp);
+    /// # assert_eq!(descriptor.derived_descriptor(&secp, index), derived_descriptor);
     /// ```
     ///
+    /// and is only here really here for backwards compatbility.
+    /// See [`derive`] and `[derived_descriptor`] for more documentation.
+    ///
+    /// [`derive`]: Self::derive
+    /// [`derived_descriptor`]: crate::DerivedDescriptor::derived_descriptor
+    ///
     /// # Errors
     ///
     /// This function will return an error if hardened derivation is attempted.
@@ -568,29 +570,7 @@ impl Descriptor<DescriptorPublicKey> {
         secp: &secp256k1::Secp256k1<C>,
         index: u32,
     ) -> Result<Descriptor<bitcoin::PublicKey>, ConversionError> {
-        struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1<C>);
-
-        impl<'a, C: secp256k1::Verification>
-            PkTranslator<DerivedDescriptorKey, bitcoin::PublicKey, ConversionError>
-            for Derivator<'a, C>
-        {
-            fn pk(
-                &mut self,
-                pk: &DerivedDescriptorKey,
-            ) -> Result<bitcoin::PublicKey, ConversionError> {
-                pk.derive_public_key(&self.0)
-            }
-
-            fn pkh(
-                &mut self,
-                pkh: &DerivedDescriptorKey,
-            ) -> Result<bitcoin::hashes::hash160::Hash, ConversionError> {
-                Ok(pkh.derive_public_key(&self.0)?.to_pubkeyhash())
-            }
-        }
-
-        let derived = self.derive(index).translate_pk(&mut Derivator(secp))?;
-        Ok(derived)
+        self.derive(index).derived_descriptor(&secp)
     }
 
     /// Parse a descriptor that may contain secret keys
@@ -744,6 +724,59 @@ impl Descriptor<DescriptorPublicKey> {
     }
 }
 
+impl Descriptor<DerivedDescriptorKey> {
+    /// 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
+    /// default(0x02) y-coordinate.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use miniscript::descriptor::{Descriptor, DescriptorPublicKey};
+    /// use miniscript::bitcoin::secp256k1;
+    /// use std::str::FromStr;
+    ///
+    /// // test from bip 86
+    /// let secp = secp256k1::Secp256k1::verification_only();
+    /// let descriptor = Descriptor::<DescriptorPublicKey>::from_str("tr(xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)")
+    ///     .expect("Valid ranged descriptor");
+    /// let result = descriptor.derive(0).derived_descriptor(&secp).expect("Non-hardened derivation");
+    /// assert_eq!(result.to_string(), "tr(03cc8a4bc64d897bddc5fbc2f670f7a8ba0b386779106cf1223c6fc5d7cd6fc115)#6qm9h8ym");
+    /// ```
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if hardened derivation is attempted.
+    pub fn derived_descriptor<C: secp256k1::Verification>(
+        &self,
+        secp: &secp256k1::Secp256k1<C>,
+    ) -> Result<Descriptor<bitcoin::PublicKey>, ConversionError> {
+        struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1<C>);
+
+        impl<'a, C: secp256k1::Verification>
+            PkTranslator<DerivedDescriptorKey, bitcoin::PublicKey, ConversionError>
+            for Derivator<'a, C>
+        {
+            fn pk(
+                &mut self,
+                pk: &DerivedDescriptorKey,
+            ) -> Result<bitcoin::PublicKey, ConversionError> {
+                pk.derive_public_key(&self.0)
+            }
+
+            fn pkh(
+                &mut self,
+                pkh: &DerivedDescriptorKey,
+            ) -> Result<bitcoin::hashes::hash160::Hash, ConversionError> {
+                Ok(pkh.derive_public_key(&self.0)?.to_pubkeyhash())
+            }
+        }
+
+        let derived = self.translate_pk(&mut Derivator(secp))?;
+        Ok(derived)
+    }
+}
+
 impl_from_tree!(
     Descriptor<Pk>,
     /// Parse an expression tree into a descriptor.
@@ -1564,12 +1597,14 @@ mod tests {
 
             // Same address
             let addr_one = desc_one
-                .derived_descriptor(&secp_ctx, index)
+                .derive(index)
+                .derived_descriptor(&secp_ctx)
                 .unwrap()
                 .address(bitcoin::Network::Bitcoin)
                 .unwrap();
             let addr_two = desc_two
-                .derived_descriptor(&secp_ctx, index)
+                .derive(index)
+                .derived_descriptor(&secp_ctx)
                 .unwrap()
                 .address(bitcoin::Network::Bitcoin)
                 .unwrap();

src/lib.rs

@@ -135,7 +135,7 @@ use std::error;
 use bitcoin::blockdata::{opcodes, script};
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
 
-pub use crate::descriptor::{Descriptor, DescriptorPublicKey};
+pub use crate::descriptor::{DerivedDescriptorKey, Descriptor, DescriptorPublicKey};
 pub use crate::interpreter::Interpreter;
 pub use crate::miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0, Tap};
 pub use crate::miniscript::decode::Terminal;