BIP32 extended keys are using Scep256k1 keys instead of bitcoin ECDSA

According to #588, BIP32 does not support uncompressed keys and using type with compression flag is a mistake

Author: dr-orlovsky

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;
@@ -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

@@ -28,8 +28,9 @@ use hashes::{sha512, Hash, HashEngine, Hmac, HmacEngine};
 use secp256k1::{self, Secp256k1};
 
 use network::constants::Network;
-use util::{base58, endian, key};
-use util::ecdsa::{PublicKey, PrivateKey};
+use util::{base58, endian};
+use util::key;
+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,7 +522,7 @@ 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..]),
         })
     }
@@ -532,7 +548,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 +559,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 +594,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])?,
         })
     }
 
@@ -617,7 +633,7 @@ impl ExtendedPubKey {
             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
         }
     }
@@ -638,19 +654,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 +681,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 +713,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 +728,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)
     }
 

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;
+    use secp256k1::{Secp256k1, self};
 
     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();
 
@@ -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(),
                     vec![8, 5, 4],
                 )].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(),