Merge #448: DerivedDescriptor unfinished business

2ad6555 Rename "derive" things that are not doing derivation (LLFourn)
6dc6aca Make DerivedDescriptorKey first-class citizen (LLFourn)

Pull request description:

  This PR further develops the idea introduced in #345. It has two commits both with relatively detailed commit messages that can be reviewed separately. In summary:

  1. Develop the `DerivedDescriptorKey` (a key that is guaranteed not to have wildcard in it) idea more by adding missing functionality and refining the API. In addition, I removed the error cases from `ConversionError` which seems to have been omitted from #345.
  2. Since the introduction of `DerivedDescriptorKey`, the overlapping usage of the term "derive" has become quite confusing. In addition to the usual definition of "derive" we have also used it to mean "replace a wildcard with a particular derivation index". I deprecated and renamed everything that uses the latter definition.

ACKs for top commit:
  apoelstra:
    ACK 2ad6555
  sanket1729:
    ACK 2ad6555. Thanks for the clean changes.

Tree-SHA512: 0198404a1bfecab2324a8785117248fc566cfbb53decbd234928e378f102bdc5c5de6d31b437b8f1b0ba90ef524a362a46150028f80a4b029589406233abd2fe

Author: sanket1729

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::{DefiniteDescriptorKey, 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::<DefiniteDescriptorKey>::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 DefiniteDescriptorKey(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,
         }
     }
 }
@@ -441,40 +428,50 @@ impl DescriptorPublicKey {
         }
     }
 
-    /// Whether or not the key has a wildcards
+    /// Whether or not the key has a wildcard
+    #[deprecated(note = "use has_wildcard instead")]
     pub fn is_deriveable(&self) -> bool {
+        self.has_wildcard()
+    }
+
+    /// Whether or not the key has a wildcard
+    pub fn has_wildcard(&self) -> bool {
         match *self {
             DescriptorPublicKey::Single(..) => false,
             DescriptorPublicKey::XPub(ref xpub) => xpub.wildcard != Wildcard::None,
         }
     }
 
-    /// Derives the [`DescriptorPublicKey`] at `index` if this key is an xpub and has a wildcard.
+    #[deprecated(note = "use at_derivation_index instead")]
+    /// Deprecated name of [`at_derivation_index`].
+    pub fn derive(self, index: u32) -> DefiniteDescriptorKey {
+        self.at_derivation_index(index)
+    }
+
+    /// Replaces any wildcard (i.e. `/*`) in the key with a particular derivation index, turning it into a
+    /// *definite* key (i.e. one where all the derivation paths are set).
     ///
     /// # Returns
     ///
     /// - 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
     ///
     /// If `index` ≥ 2^31
-    pub fn derive(self, index: u32) -> DerivedDescriptorKey {
-        let derived = match self {
+    pub fn at_derivation_index(self, index: u32) -> DefiniteDescriptorKey {
+        let definite = match self {
             DescriptorPublicKey::Single(_) => self,
             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 +482,7 @@ impl DescriptorPublicKey {
             }
         };
 
-        DerivedDescriptorKey::new(derived, index)
+        DefiniteDescriptorKey::new(definite)
             .expect("The key should not contain any wildcards at this point")
     }
 
@@ -494,13 +491,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 +505,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) => {
@@ -767,7 +762,7 @@ impl MiniscriptKey for DescriptorPublicKey {
     }
 }
 
-impl DerivedDescriptorKey {
+impl DefiniteDescriptorKey {
     /// Computes the raw [`bitcoin::PublicKey`] for this descriptor key.
     ///
     /// Will return an error if the key has any hardened derivation steps
@@ -778,32 +773,51 @@ 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.has_wildcard() {
+            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 DefiniteDescriptorKey {
+    type Err = DescriptorKeyParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let inner = DescriptorPublicKey::from_str(s)?;
+        Ok(
+            DefiniteDescriptorKey::new(inner).ok_or(DescriptorKeyParseError(
+                "cannot parse key with a wilcard as a DerivedDescriptorKey",
+            ))?,
+        )
+    }
 }
 
-impl fmt::Display for DerivedDescriptorKey {
+impl fmt::Display for DefiniteDescriptorKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.key.fmt(f)
+        self.0.fmt(f)
     }
 }
 
-impl MiniscriptKey for DerivedDescriptorKey {
+impl MiniscriptKey for DefiniteDescriptorKey {
     // This allows us to be able to derive public keys even for PkH s
     type RawPkHash = Self;
     type Sha256 = sha256::Hash;
@@ -812,22 +826,22 @@ 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 {
         self.clone()
     }
 }
 
-impl ToPublicKey for DerivedDescriptorKey {
+impl ToPublicKey for DefiniteDescriptorKey {
     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 +865,12 @@ impl ToPublicKey for DerivedDescriptorKey {
     }
 }
 
+impl From<DefiniteDescriptorKey> for DescriptorPublicKey {
+    fn from(d: DefiniteDescriptorKey) -> Self {
+        d.0
+    }
+}
+
 #[cfg(test)]
 mod test {
     use core::str::FromStr;
@@ -957,17 +977,17 @@ mod test {
         let public_key = DescriptorPublicKey::from_str("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2").unwrap();
         assert_eq!(public_key.master_fingerprint().to_string(), "abcdef00");
         assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'/2");
-        assert_eq!(public_key.is_deriveable(), false);
+        assert_eq!(public_key.has_wildcard(), false);
 
         let public_key = DescriptorPublicKey::from_str("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/*").unwrap();
         assert_eq!(public_key.master_fingerprint().to_string(), "abcdef00");
         assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'");
-        assert_eq!(public_key.is_deriveable(), true);
+        assert_eq!(public_key.has_wildcard(), true);
 
         let public_key = DescriptorPublicKey::from_str("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/*h").unwrap();
         assert_eq!(public_key.master_fingerprint().to_string(), "abcdef00");
         assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'");
-        assert_eq!(public_key.is_deriveable(), true);
+        assert_eq!(public_key.has_wildcard(), true);
     }
 
     #[test]
@@ -979,7 +999,7 @@ mod test {
         assert_eq!(public_key.to_string(), "[2cbe2a6d/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2");
         assert_eq!(public_key.master_fingerprint().to_string(), "2cbe2a6d");
         assert_eq!(public_key.full_derivation_path().to_string(), "m/0'/1'/2");
-        assert_eq!(public_key.is_deriveable(), false);
+        assert_eq!(public_key.has_wildcard(), false);
 
         let secret_key = DescriptorSecretKey::from_str("tprv8ZgxMBicQKsPcwcD4gSnMti126ZiETsuX7qwrtMypr6FBwAP65puFn4v6c3jrN9VwtMRMph6nyT63NrfUL4C3nBzPcduzVSuHD7zbX2JKVc/0'/1'/2'").unwrap();
         let public_key = secret_key.to_public(&secp).unwrap();