Merge #590: Taproot: BIP32 extended keys using Scep256k1 keys instead…

… of bitcoin ECDSA

cf0c48c Improve Debug for PrivateKey (Dr Maxim Orlovsky)
b65a6ae Test for extended private key keypair generation  f5875a (Dr Maxim Orlovsky)
e6a3d60 BIP32 extended key `to_ecdsa()` and `to_schnorr()` methods (Dr Maxim Orlovsky)
b72f56c BIP32 extended keys are using Scep256k1 keys instead of bitcoin ECDSA (Dr Maxim Orlovsky)

Pull request description:

  This is third step required to introduce Schnorr key support according to #588. This PR starts API-breaking changes and is follow-up to non-API breaking #589, which is already merged.

  PR rationale: BIP32 does not support uncompressed keys and using type with compression flag was a mistake

ACKs for top commit:
  apoelstra:
    ACK cf0c48c
  sanket1729:
    ACK cf0c48c. #757 might need rework after this

Tree-SHA512: 6356a65004e7517256bacbf9aaeb69a22fd8536b341e567c5c4e819288e1105d083fe12ac0641404c407c97acf039bdc525f8e02b1b594a6cdda90106f3b1bdc

examples/bip32.rs

@@ -4,7 +4,7 @@ use std::{env, process};
 use std::str::FromStr;
 
 use bitcoin::secp256k1::Secp256k1;
-use bitcoin::util::ecdsa::PrivateKey;
+use bitcoin::{PrivateKey, PublicKey};
 use bitcoin::util::bip32::ExtendedPrivKey;
 use bitcoin::util::bip32::ExtendedPubKey;
 use bitcoin::util::bip32::DerivationPath;
@@ -49,7 +49,7 @@ fn main() {
     let path = DerivationPath::from_str("m/84h/0h/0h").unwrap();
     let child = root.derive_priv(&secp, &path).unwrap();
     println!("Child at {}: {}", path, child);
-    let xpub = ExtendedPubKey::from_private(&secp, &child);
+    let xpub = ExtendedPubKey::from_priv(&secp, &child);
     println!("Public key at {}: {}", path, xpub);
 
     // generate first receiving address at m/0/0
@@ -58,7 +58,7 @@ fn main() {
     let public_key = xpub.derive_pub(&secp, &vec![zero, zero])
                          .unwrap()
                          .public_key;
-    let address = Address::p2wpkh(&public_key, network).unwrap();
+    let address = Address::p2wpkh(&PublicKey::new(public_key), network).unwrap();
     println!("First receiving address: {}", address);
 
 }

src/util/bip32.rs

@@ -25,11 +25,12 @@ use core::{fmt, str::FromStr, default::Default};
 
 use hash_types::XpubIdentifier;
 use hashes::{sha512, Hash, HashEngine, Hmac, HmacEngine};
-use secp256k1::{self, Secp256k1};
+use secp256k1::{self, Secp256k1, XOnlyPublicKey};
 
 use network::constants::Network;
-use util::{base58, endian, key};
-use util::ecdsa::{PublicKey, PrivateKey};
+use util::{base58, endian};
+use util::{key, ecdsa, schnorr};
+use io::Write;
 
 /// A chain code
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -44,7 +45,8 @@ impl_array_newtype!(Fingerprint, u8, 4);
 impl_bytes_newtype!(Fingerprint, 4);
 
 /// Extended private key
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "std", derive(Debug))]
 pub struct ExtendedPrivKey {
     /// The network this key is to be used on
     pub network: Network,
@@ -55,12 +57,26 @@ pub struct ExtendedPrivKey {
     /// Child number of the key used to derive from parent (0 for master)
     pub child_number: ChildNumber,
     /// Private key
-    pub private_key: PrivateKey,
+    pub private_key: secp256k1::SecretKey,
     /// Chain code
     pub chain_code: ChainCode
 }
 serde_string_impl!(ExtendedPrivKey, "a BIP-32 extended private key");
 
+#[cfg(not(feature = "std"))]
+#[cfg_attr(docsrs, doc(cfg(not(feature = "std"))))]
+impl fmt::Debug for ExtendedPrivKey {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("ExtendedPrivKey")
+            .field("network", &self.network)
+            .field("depth", &self.depth)
+            .field("parent_fingerprint", &self.parent_fingerprint)
+            .field("child_number", &self.child_number)
+            .field("chain_code", &self.chain_code)
+            .finish_non_exhaustive()
+    }
+}
+
 /// Extended public key
 #[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, Hash)]
 pub struct ExtendedPubKey {
@@ -73,7 +89,7 @@ pub struct ExtendedPubKey {
     /// Child number of the key used to derive from parent (0 for master)
     pub child_number: ChildNumber,
     /// Public key
-    pub public_key: PublicKey,
+    pub public_key: secp256k1::PublicKey,
     /// Chain code
     pub chain_code: ChainCode
 }
@@ -506,11 +522,26 @@ impl ExtendedPrivKey {
             depth: 0,
             parent_fingerprint: Default::default(),
             child_number: ChildNumber::from_normal_idx(0)?,
-            private_key: PrivateKey::from_slice(&hmac_result[..32], network)?,
+            private_key: secp256k1::SecretKey::from_slice(&hmac_result[..32])?,
             chain_code: ChainCode::from(&hmac_result[32..]),
         })
     }
 
+    /// Constructs ECDSA compressed private key matching internal secret key representation.
+    pub fn to_priv(&self) -> ecdsa::PrivateKey {
+        ecdsa::PrivateKey {
+            compressed: true,
+            network: self.network,
+            key: self.private_key
+        }
+    }
+
+    /// Constructs BIP340 keypair for Schnorr signatures and Taproot use matching the internal
+    /// secret key representation.
+    pub fn to_keypair<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>) -> schnorr::KeyPair {
+        schnorr::KeyPair::from_seckey_slice(secp, &self.private_key[..]).expect("BIP32 internal private key representation is broken")
+    }
+
     /// Attempts to derive an extended private key from a path.
     ///
     /// The `path` argument can be both of type `DerivationPath` or `Vec<ChildNumber>`.
@@ -532,7 +563,7 @@ impl ExtendedPrivKey {
         match i {
             ChildNumber::Normal { .. } => {
                 // Non-hardened key: compute public data and use that
-                hmac_engine.input(&PublicKey::from_private_key(secp, &self.private_key).key.serialize()[..]);
+                hmac_engine.input(&secp256k1::PublicKey::from_secret_key(secp, &self.private_key).serialize()[..]);
             }
             ChildNumber::Hardened { .. } => {
                 // Hardened key: use only secret data to prevent public derivation
@@ -543,8 +574,8 @@ impl ExtendedPrivKey {
 
         hmac_engine.input(&endian::u32_to_array_be(u32::from(i)));
         let hmac_result: Hmac<sha512::Hash> = Hmac::from_engine(hmac_engine);
-        let mut sk = PrivateKey::from_slice(&hmac_result[..32], self.network)?;
-        sk.key.add_assign(&self.private_key[..])?;
+        let mut sk = secp256k1::SecretKey::from_slice(&hmac_result[..32])?;
+        sk.add_assign(&self.private_key[..])?;
 
         Ok(ExtendedPrivKey {
             network: self.network,
@@ -578,7 +609,7 @@ impl ExtendedPrivKey {
             parent_fingerprint: Fingerprint::from(&data[5..9]),
             child_number: endian::slice_to_u32_be(&data[9..13]).into(),
             chain_code: ChainCode::from(&data[13..45]),
-            private_key: PrivateKey::from_slice(&data[46..78], network)?,
+            private_key: secp256k1::SecretKey::from_slice(&data[46..78])?,
         })
     }
 
@@ -600,7 +631,7 @@ impl ExtendedPrivKey {
 
     /// Returns the HASH160 of the public key belonging to the xpriv
     pub fn identifier<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>) -> XpubIdentifier {
-        ExtendedPubKey::from_private(secp, self).identifier()
+        ExtendedPubKey::from_priv(secp, self).identifier()
     }
 
     /// Returns the first four bytes of the identifier
@@ -611,17 +642,37 @@ impl ExtendedPrivKey {
 
 impl ExtendedPubKey {
     /// Derives a public key from a private key
+    #[deprecated(since = "0.28.0", note = "use ExtendedPubKey::from_priv")]
     pub fn from_private<C: secp256k1::Signing>(secp: &Secp256k1<C>, sk: &ExtendedPrivKey) -> ExtendedPubKey {
+        ExtendedPubKey::from_priv(secp, sk)
+    }
+
+    /// Derives a public key from a private key
+    pub fn from_priv<C: secp256k1::Signing>(secp: &Secp256k1<C>, sk: &ExtendedPrivKey) -> ExtendedPubKey {
         ExtendedPubKey {
             network: sk.network,
             depth: sk.depth,
             parent_fingerprint: sk.parent_fingerprint,
             child_number: sk.child_number,
-            public_key: PublicKey::from_private_key(secp, &sk.private_key),
+            public_key: secp256k1::PublicKey::from_secret_key(secp, &sk.private_key),
             chain_code: sk.chain_code
         }
     }
 
+    /// Constructs ECDSA compressed public key matching internal public key representation.
+    pub fn to_pub(&self) -> ecdsa::PublicKey {
+        ecdsa::PublicKey {
+            compressed: true,
+            key: self.public_key
+        }
+    }
+
+    /// Constructs BIP340 x-only public key for BIP-340 signatures and Taproot use matching
+    /// the internal public key representation.
+    pub fn to_x_only_pub(&self) -> XOnlyPublicKey {
+        XOnlyPublicKey::from(self.public_key)
+    }
+
     /// Attempts to derive an extended public key from a path.
     ///
     /// The `path` argument can be both of type `DerivationPath` or `Vec<ChildNumber>`.
@@ -638,19 +689,19 @@ impl ExtendedPubKey {
     }
 
     /// Compute the scalar tweak added to this key to get a child key
-    pub fn ckd_pub_tweak(&self, i: ChildNumber) -> Result<(PrivateKey, ChainCode), Error> {
+    pub fn ckd_pub_tweak(&self, i: ChildNumber) -> Result<(secp256k1::SecretKey, ChainCode), Error> {
         match i {
             ChildNumber::Hardened { .. } => {
                 Err(Error::CannotDeriveFromHardenedKey)
             }
             ChildNumber::Normal { index: n } => {
                 let mut hmac_engine: HmacEngine<sha512::Hash> = HmacEngine::new(&self.chain_code[..]);
-                hmac_engine.input(&self.public_key.key.serialize()[..]);
+                hmac_engine.input(&self.public_key.serialize()[..]);
                 hmac_engine.input(&endian::u32_to_array_be(n));
 
                 let hmac_result: Hmac<sha512::Hash> = Hmac::from_engine(hmac_engine);
 
-                let private_key = PrivateKey::from_slice(&hmac_result[..32], self.network)?;
+                let private_key = secp256k1::SecretKey::from_slice(&hmac_result[..32])?;
                 let chain_code = ChainCode::from(&hmac_result[32..]);
                 Ok((private_key, chain_code))
             }
@@ -665,7 +716,7 @@ impl ExtendedPubKey {
     ) -> Result<ExtendedPubKey, Error> {
         let (sk, chain_code) = self.ckd_pub_tweak(i)?;
         let mut pk = self.public_key;
-        pk.key.add_exp_assign(secp, &sk[..])?;
+        pk.add_exp_assign(secp, &sk[..])?;
 
         Ok(ExtendedPubKey {
             network: self.network,
@@ -697,7 +748,7 @@ impl ExtendedPubKey {
             parent_fingerprint: Fingerprint::from(&data[5..9]),
             child_number: endian::slice_to_u32_be(&data[9..13]).into(),
             chain_code: ChainCode::from(&data[13..45]),
-            public_key: PublicKey::from_slice(&data[45..78])?,
+            public_key: secp256k1::PublicKey::from_slice(&data[45..78])?,
         })
     }
 
@@ -712,14 +763,14 @@ impl ExtendedPubKey {
         ret[5..9].copy_from_slice(&self.parent_fingerprint[..]);
         ret[9..13].copy_from_slice(&endian::u32_to_array_be(u32::from(self.child_number)));
         ret[13..45].copy_from_slice(&self.chain_code[..]);
-        ret[45..78].copy_from_slice(&self.public_key.key.serialize()[..]);
+        ret[45..78].copy_from_slice(&self.public_key.serialize()[..]);
         ret
     }
 
     /// Returns the HASH160 of the chaincode
     pub fn identifier(&self) -> XpubIdentifier {
         let mut engine = XpubIdentifier::engine();
-        self.public_key.write_into(&mut engine).expect("engines don't error");
+        engine.write(&self.public_key.serialize()).expect("engines don't error");
         XpubIdentifier::from_engine(engine)
     }
 
@@ -853,7 +904,7 @@ mod tests {
                  expected_pk: &str) {
 
         let mut sk = ExtendedPrivKey::new_master(network, seed).unwrap();
-        let mut pk = ExtendedPubKey::from_private(secp, &sk);
+        let mut pk = ExtendedPubKey::from_priv(secp, &sk);
 
         // Check derivation convenience method for ExtendedPrivKey
         assert_eq!(
@@ -881,15 +932,15 @@ mod tests {
             match num {
                 Normal {..} => {
                     let pk2 = pk.ckd_pub(secp, num).unwrap();
-                    pk = ExtendedPubKey::from_private(secp, &sk);
+                    pk = ExtendedPubKey::from_priv(secp, &sk);
                     assert_eq!(pk, pk2);
                 }
                 Hardened {..} => {
                     assert_eq!(
                         pk.ckd_pub(secp, num),
                         Err(Error::CannotDeriveFromHardenedKey)
                     );
-                    pk = ExtendedPubKey::from_private(secp, &sk);
+                    pk = ExtendedPubKey::from_priv(secp, &sk);
                 }
             }
         }
@@ -1080,5 +1131,42 @@ mod tests {
         assert_eq!("42", &format!("{}", ChildNumber::from_normal_idx(42).unwrap()));
         assert_eq!("000042", &format!("{:06}", ChildNumber::from_normal_idx(42).unwrap()));
     }
+
+    #[test]
+    #[should_panic(expected = "Secp256k1(InvalidSecretKey)")]
+    fn schnorr_broken_privkey_zeros() {
+        /* this is how we generate key:
+        let mut sk = secp256k1::key::ONE_KEY;
+
+        let zeros = [0u8; 32];
+        unsafe {
+            sk.as_mut_ptr().copy_from(zeros.as_ptr(), 32);
+        }
+
+        let xpriv = ExtendedPrivKey {
+            network: Network::Bitcoin,
+            depth: 0,
+            parent_fingerprint: Default::default(),
+            child_number: ChildNumber::Normal { index: 0 },
+            private_key: sk,
+            chain_code: ChainCode::from(&[0u8; 32][..])
+        };
+
+        println!("{}", xpriv);
+         */
+
+        // Xpriv having secret key set to all zeros
+        let xpriv_str = "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzF93Y5wvzdUayhgkkFoicQZcP3y52uPPxFnfoLZB21Teqt1VvEHx";
+        ExtendedPrivKey::from_str(xpriv_str).unwrap();
+    }
+
+
+    #[test]
+    #[should_panic(expected = "Secp256k1(InvalidSecretKey)")]
+    fn schnorr_broken_privkey_ffs() {
+        // Xpriv having secret key set to all 0xFF's
+        let xpriv_str = "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD9y5gkZ6Eq3Rjuahrv17fENZ3QzxW";
+        ExtendedPrivKey::from_str(xpriv_str).unwrap();
+    }
 }
 

src/util/ecdsa.rs

@@ -174,6 +174,7 @@ impl FromStr for PublicKey {
 
 /// A Bitcoin ECDSA private key
 #[derive(Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "std", derive(Debug))]
 pub struct PrivateKey {
     /// Whether this private key should be serialized as compressed
     pub compressed: bool,
@@ -280,6 +281,7 @@ impl fmt::Display for PrivateKey {
     }
 }
 
+#[cfg(not(feature = "std"))]
 impl fmt::Debug for PrivateKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "[private key data]")