Merge rust-bitcoin/rust-bitcoin#590: Taproot: BIP32 extended keys using Scep256k1 keys instead of bitcoin ECDSA

cf0c48cc86 Improve Debug for PrivateKey (Dr Maxim Orlovsky)
b65a6ae49b Test for extended private key keypair generation  f5875a (Dr Maxim Orlovsky)
e6a3d603c9 BIP32 extended key `to_ecdsa()` and `to_schnorr()` methods (Dr Maxim Orlovsky)
b72f56c4ae 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 cf0c48cc86
  sanket1729:
    ACK cf0c48cc86. #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::{base58, endian};
-use util::ecdsa::{PublicKey, PrivateKey};
+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)?;
+        let mut sk = secp256k1::SecretKey::from_slice(&hmac_result[..32])?;
-        sk.key.add_assign(&self.private_key[..])?;
+        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,7 +932,7 @@ 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 {..} => {
@@ -889,7 +940,7 @@ mod tests {
                         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]")

src/util/psbt/mod.rs

@@ -243,24 +243,23 @@ impl Decodable for PartiallySignedTransaction {
 
 #[cfg(test)]
 mod tests {
+    use super::PartiallySignedTransaction;
+
     use hashes::hex::FromHex;
     use hashes::{sha256, hash160, Hash, ripemd160};
     use hash_types::Txid;
 
-
+    use secp256k1::{Secp256k1, self};
-    use secp256k1::Secp256k1;
 
     use blockdata::script::Script;
     use blockdata::transaction::{Transaction, TxIn, TxOut, OutPoint};
     use network::constants::Network::Bitcoin;
     use consensus::encode::{deserialize, serialize, serialize_hex};
     use util::bip32::{ChildNumber, ExtendedPrivKey, ExtendedPubKey, Fingerprint, KeySource};
-    use util::ecdsa::PublicKey;
+    use util::ecdsa;
     use util::psbt::map::{Output, Input};
     use util::psbt::raw;
 
-    use super::PartiallySignedTransaction;
-    use util::psbt::raw::ProprietaryKey;
     use std::collections::BTreeMap;
     use blockdata::witness::Witness;
 
@@ -292,7 +291,7 @@ mod tests {
         let secp = &Secp256k1::new();
         let seed = Vec::from_hex("000102030405060708090a0b0c0d0e0f").unwrap();
 
-        let mut hd_keypaths: BTreeMap<PublicKey, KeySource> = Default::default();
+        let mut hd_keypaths: BTreeMap<secp256k1::PublicKey, KeySource> = Default::default();
 
         let mut sk: ExtendedPrivKey = ExtendedPrivKey::new_master(Bitcoin, &seed).unwrap();
 
@@ -311,7 +310,7 @@ mod tests {
 
         sk = sk.derive_priv(secp, &dpath).unwrap();
 
-        let pk: ExtendedPubKey = ExtendedPubKey::from_private(&secp, &sk);
+        let pk: ExtendedPubKey = ExtendedPubKey::from_priv(&secp, &sk);
 
         hd_keypaths.insert(pk.public_key, (fprint, dpath.into()));
 
@@ -322,7 +321,10 @@ mod tests {
             witness_script: Some(hex_script!(
                 "a9143545e6e33b832c47050f24d3eeb93c9c03948bc787"
             )),
-            bip32_derivation: hd_keypaths,
+            bip32_derivation: hd_keypaths.into_iter().map(|(key, src)| (ecdsa::PublicKey {
+                compressed: true,
+                key,
+            }, src)).collect(),
             ..Default::default()
         };
 
@@ -435,7 +437,7 @@ mod tests {
             vec![3, 4 ,5],
         )].into_iter().collect();
         let key_source = ("deadbeef".parse().unwrap(), "m/0'/1".parse().unwrap());
-        let keypaths: BTreeMap<PublicKey, KeySource> = vec![(
+        let keypaths: BTreeMap<secp256k1::PublicKey, KeySource> = vec![(
             "0339880dc92394b7355e3d0439fa283c31de7590812ea011c4245c0674a685e883".parse().unwrap(),
             key_source.clone(),
         )].into_iter().collect();
@@ -479,7 +481,10 @@ mod tests {
                     "0339880dc92394b7355e3d0439fa283c31de7590812ea011c4245c0674a685e883".parse().unwrap(),
                     "304402204f67e2afb76142d44fae58a2495d33a3419daa26cd0db8d04f3452b63289ac0f022010762a9fb67e94cc5cad9026f6dc99ff7f070f4278d30fbc7d0c869dd38c7fe701".parse().unwrap(),
                 )].into_iter().collect(),
-                bip32_derivation: keypaths.clone(),
+                bip32_derivation: keypaths.clone().into_iter().map(|(key, src)| (ecdsa::PublicKey {
+                    compressed: true,
+                    key,
+                }, src)).collect(),
                 final_script_witness: Some(vec![vec![1, 3], vec![5]]),
                 ripemd160_preimages: vec![(ripemd160::Hash::hash(&[]), vec![1, 2])].into_iter().collect(),
                 sha256_preimages: vec![(sha256::Hash::hash(&[]), vec![1, 2])].into_iter().collect(),
@@ -490,7 +495,10 @@ mod tests {
                 ..Default::default()
             }],
             outputs: vec![Output {
-                bip32_derivation: keypaths.clone(),
+                bip32_derivation: keypaths.into_iter().map(|(key, src)| (ecdsa::PublicKey {
+                    compressed: true,
+                    key,
+                }, src)).collect(),
                 proprietary: proprietary.clone(),
                 unknown: unknown.clone(),
                 ..Default::default()
@@ -1012,7 +1020,7 @@ mod tests {
     #[test]
     fn serialize_and_deserialize_proprietary() {
         let mut psbt: PartiallySignedTransaction = hex_psbt!("70736274ff0100a00200000002ab0949a08c5af7c49b8212f417e2f15ab3f5c33dcf153821a8139f877a5b7be40000000000feffffffab0949a08c5af7c49b8212f417e2f15ab3f5c33dcf153821a8139f877a5b7be40100000000feffffff02603bea0b000000001976a914768a40bbd740cbe81d988e71de2a4d5c71396b1d88ac8e240000000000001976a9146f4620b553fa095e721b9ee0efe9fa039cca459788ac000000000001076a47304402204759661797c01b036b25928948686218347d89864b719e1f7fcf57d1e511658702205309eabf56aa4d8891ffd111fdf1336f3a29da866d7f8486d75546ceedaf93190121035cdc61fc7ba971c0b501a646a2a83b102cb43881217ca682dc86e2d73fa882920001012000e1f5050000000017a9143545e6e33b832c47050f24d3eeb93c9c03948bc787010416001485d13537f2e265405a34dbafa9e3dda01fb82308000000").unwrap();
-        psbt.proprietary.insert(ProprietaryKey {
+        psbt.proprietary.insert(raw::ProprietaryKey {
             prefix: b"test".to_vec(),
             subtype: 0u8,
             key: b"test".to_vec(),