Merge pull request #183 from dongcarl/2018-9-pubkey-wrapper

Ready for Review: Introduce util::key and deprecate util::privkey

src/consensus/encode.rs

@@ -42,6 +42,7 @@ use hex::encode as hex_encode;
 
 use bitcoin_bech32;
 use bitcoin_hashes::{sha256d, Hash as HashTrait};
+use secp256k1;
 
 use util::base58;
 
@@ -56,6 +57,8 @@ pub enum Error {
     Bech32(bitcoin_bech32::Error),
     /// Error from the `byteorder` crate
     ByteOrder(io::Error),
+    /// secp-related error
+    Secp256k1(secp256k1::Error),
     /// Network magic was not expected
     UnexpectedNetworkMagic {
         /// The expected network magic
@@ -98,6 +101,7 @@ impl fmt::Display for Error {
             Error::Base58(ref e) => fmt::Display::fmt(e, f),
             Error::Bech32(ref e) => fmt::Display::fmt(e, f),
             Error::ByteOrder(ref e) => fmt::Display::fmt(e, f),
+            Error::Secp256k1(ref e) => fmt::Display::fmt(e, f),
             Error::UnexpectedNetworkMagic { expected: ref e, actual: ref a } => write!(f, "{}: expected {}, actual {}", error::Error::description(self), e, a),
             Error::OversizedVectorAllocation { requested: ref r, max: ref m } => write!(f, "{}: requested {}, maximum {}", error::Error::description(self), r, m),
             Error::InvalidChecksum { expected: ref e, actual: ref a } => write!(f, "{}: expected {}, actual {}", error::Error::description(self), hex_encode(e), hex_encode(a)),
@@ -118,6 +122,7 @@ impl error::Error for Error {
             Error::Base58(ref e) => Some(e),
             Error::Bech32(ref e) => Some(e),
             Error::ByteOrder(ref e) => Some(e),
+            Error::Secp256k1(ref e) => Some(e),
             Error::UnexpectedNetworkMagic { .. }
             | Error::OversizedVectorAllocation { .. }
             | Error::InvalidChecksum { .. }
@@ -136,6 +141,7 @@ impl error::Error for Error {
             Error::Base58(ref e) => e.description(),
             Error::Bech32(ref e) => e.description(),
             Error::ByteOrder(ref e) => e.description(),
+            Error::Secp256k1(ref e) => e.description(),
             Error::UnexpectedNetworkMagic { .. } => "unexpected network magic",
             Error::OversizedVectorAllocation { .. } => "allocation of oversized vector requested",
             Error::InvalidChecksum { .. } => "invalid checksum",
@@ -163,6 +169,12 @@ impl From<bitcoin_bech32::Error> for Error {
     }
 }
 
+#[doc(hidden)]
+impl From<secp256k1::Error> for Error {
+    fn from(e: secp256k1::Error) -> Error {
+        Error::Secp256k1(e)
+    }
+}
 
 #[doc(hidden)]
 impl From<io::Error> for Error {

src/lib.rs

@@ -76,6 +76,7 @@ pub use network::constants::Network;
 pub use util::Error;
 pub use util::address::Address;
 pub use util::hash::BitcoinHash;
-pub use util::privkey::Privkey;
+pub use util::key::PrivateKey;
+pub use util::key::PublicKey;
 pub use util::decimal::Decimal;
 pub use util::decimal::UDecimal;

src/util/address.rs

@@ -24,13 +24,17 @@
 //! 
 //! use bitcoin::network::constants::Network;
 //! use bitcoin::util::address::Address;
+//! use bitcoin::util::key;
 //! use secp256k1::Secp256k1;
 //! use rand::thread_rng;
 //! 
 //! fn main() {
 //!     // Generate random key pair
 //!     let s = Secp256k1::new();
-//!     let (_, public_key) = s.generate_keypair(&mut thread_rng());
+//!     let public_key = key::PublicKey {
+//!         compressed: true,
+//!         key: s.generate_keypair(&mut thread_rng()).1,
+//!     };
 //! 
 //!     // Generate pay-to-pubkey-hash address
 //!     let address = Address::p2pkh(&public_key, Network::Bitcoin);
@@ -42,7 +46,6 @@ use std::str::FromStr;
 
 use bitcoin_bech32::{self, WitnessProgram, u5};
 use bitcoin_hashes::{hash160, Hash};
-use secp256k1::key::PublicKey;
 
 #[cfg(feature = "serde")]
 use serde;
@@ -52,6 +55,7 @@ use blockdata::script;
 use network::constants::Network;
 use consensus::encode;
 use util::base58;
+use util::key;
 
 /// The method used to produce an address
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -70,28 +74,20 @@ pub struct Address {
     /// The type of the address
     pub payload: Payload,
     /// The network on which this address is usable
-    pub network: Network
+    pub network: Network,
 }
 
 impl Address {
     /// Creates a pay to (compressed) public key hash address from a public key
     /// This is the preferred non-witness type address
     #[inline]
-    pub fn p2pkh(pk: &PublicKey, network: Network) -> Address {
+    pub fn p2pkh(pk: &key::PublicKey, network: Network) -> Address {
-        Address {
+        let mut hash_engine = hash160::Hash::engine();
-            network: network,
+        pk.write_into(&mut hash_engine);
-            payload: Payload::PubkeyHash(hash160::Hash::hash(&pk.serialize()[..]))
-        }
-    }
 
-    /// Creates a pay to uncompressed public key hash address from a public key
-    /// This address type is discouraged as it uses more space but otherwise equivalent to p2pkh
-    /// therefore only adds ambiguity
-    #[inline]
-    pub fn p2upkh(pk: &PublicKey, network: Network) -> Address {
         Address {
             network: network,
-            payload: Payload::PubkeyHash(hash160::Hash::hash(&pk.serialize_uncompressed()[..]))
+            payload: Payload::PubkeyHash(hash160::Hash::from_engine(hash_engine))
         }
     }
 
@@ -107,23 +103,30 @@ impl Address {
 
     /// Create a witness pay to public key address from a public key
     /// This is the native segwit address type for an output redeemable with a single signature
-    pub fn p2wpkh (pk: &PublicKey, network: Network) -> Address {
+    pub fn p2wpkh (pk: &key::PublicKey, network: Network) -> Address {
+        let mut hash_engine = hash160::Hash::engine();
+        pk.write_into(&mut hash_engine);
+
         Address {
             network: network,
             payload: Payload::WitnessProgram(
                 // unwrap is safe as witness program is known to be correct as above
                 WitnessProgram::new(u5::try_from_u8(0).expect("0<32"),
-                                    hash160::Hash::hash(&pk.serialize()[..])[..].to_vec(),
+                                    hash160::Hash::from_engine(hash_engine)[..].to_vec(),
                                     Address::bech_network(network)).unwrap())
         }
     }
 
     /// Create a pay to script address that embeds a witness pay to public key
     /// This is a segwit address type that looks familiar (as p2sh) to legacy clients
-    pub fn p2shwpkh (pk: &PublicKey, network: Network) -> Address {
+    pub fn p2shwpkh (pk: &key::PublicKey, network: Network) -> Address {
+        let mut hash_engine = hash160::Hash::engine();
+        pk.write_into(&mut hash_engine);
+
         let builder = script::Builder::new()
             .push_int(0)
-            .push_slice(&hash160::Hash::hash(&pk.serialize()[..])[..]);
+            .push_slice(&hash160::Hash::from_engine(hash_engine)[..]);
+
         Address {
             network: network,
             payload: Payload::ScriptHash(
@@ -146,7 +149,7 @@ impl Address {
                     sha256::Hash::hash(&script[..])[..].to_vec(),
                     Address::bech_network(network)
                 ).unwrap()
-            )
+            ),
         }
     }
 
@@ -252,7 +255,7 @@ impl FromStr for Address {
             }
             return Ok(Address {
                 network: network,
-                payload: Payload::WitnessProgram(witprog)
+                payload: Payload::WitnessProgram(witprog),
             });
         }
 
@@ -359,11 +362,12 @@ mod tests {
     use std::string::ToString;
 
     use bitcoin_hashes::{hash160, Hash};
-    use secp256k1::key::PublicKey;
     use hex::decode as hex_decode;
 
     use blockdata::script::Script;
     use network::constants::Network::{Bitcoin, Testnet, Regtest};
+    use util::key::PublicKey;
+
     use super::*;
 
     macro_rules! hex (($hex:expr) => (hex_decode($hex).unwrap()));
@@ -388,7 +392,7 @@ mod tests {
     #[test]
     fn test_p2pkh_from_key() {
         let key = hex_key!("048d5141948c1702e8c95f438815794b87f706a8d4cd2bffad1dc1570971032c9b6042a0431ded2478b5c9cf2d81c124a5e57347a3c63ef0e7716cf54d613ba183");
-        let addr = Address::p2upkh(&key, Bitcoin);
+        let addr = Address::p2pkh(&key, Bitcoin);
         assert_eq!(&addr.to_string(), "1QJVDzdqb1VpbDK7uDeyVXy9mR27CJiyhY");
 
         let key = hex_key!(&"03df154ebfcf29d29cc10d5c2565018bce2d9edbab267c31d2caf44a63056cf99f");

src/util/bip143.rs

@@ -107,8 +107,8 @@ mod tests {
     use network::constants::Network;
     use util::misc::hex_bytes;
     use util::address::Address;
+    use util::key::PublicKey;
     use hex;
-    use secp256k1::PublicKey;
 
     use super::*;
 

src/util/key.rs

@@ -11,86 +11,77 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-//! Private key
+//! Bitcoin Keys
 //!
-//! A private key represents the secret data associated with its proposed use
+//! Keys used in Bitcoin that can be roundtrip (de)serialized.
 //!
 
 use std::fmt::{self, Write};
+use std::io;
 use std::str::FromStr;
 use secp256k1::{self, Secp256k1};
-use secp256k1::key::{PublicKey, SecretKey};
-use util::address::Address;
 use consensus::encode;
 use network::constants::Network;
 use util::base58;
 
+/// A Bitcoin ECDSA public key
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct PublicKey {
+    /// Whether this public key should be serialized as compressed
+    pub compressed: bool,
+    /// The actual ECDSA key
+    pub key: secp256k1::PublicKey,
+}
+
+impl PublicKey {
+    /// Write the public key into a writer
+    pub fn write_into<W: io::Write>(&self, writer: &mut W) {
+        let write_res: io::Result<()> = if self.compressed {
+            writer.write_all(&self.key.serialize())
+        } else {
+            writer.write_all(&self.key.serialize_uncompressed())
+        };
+        debug_assert!(write_res.is_ok());
+    }
+
+    /// Deserialize a public key from a slice
+    pub fn from_slice(data: &[u8]) -> Result<PublicKey, encode::Error> {
+        let compressed: bool = match data.len() {
+            33 => true,
+            65 => false,
+            len =>  { return Err(base58::Error::InvalidLength(len).into()); },
+        };
+
+        Ok(PublicKey {
+            compressed: compressed,
+            key: secp256k1::PublicKey::from_slice(data)?,
+        })
+    }
+
+    /// Computes the public key as supposed to be used with this secret
+    pub fn from_private_key<C: secp256k1::Signing>(secp: &Secp256k1<C>, sk: &PrivateKey) -> PublicKey {
+        sk.public_key(secp)
+    }
+}
+
 #[derive(Clone, PartialEq, Eq)]
 /// A Bitcoin ECDSA private key
-pub struct Privkey {
+pub struct PrivateKey {
-    /// Whether this private key represents a compressed address
+    /// Whether this private key should be serialized as compressed
     pub compressed: bool,
     /// The network on which this key should be used
     pub network: Network,
     /// The actual ECDSA key
-    pub key: SecretKey
+    pub key: secp256k1::SecretKey,
 }
 
-impl Privkey {
+impl PrivateKey {
-    /// Creates a `Privkey` from a raw secp256k1 secret key
+    /// Creates a public key from this private key
-    #[inline]
-    pub fn from_secret_key(key: SecretKey, compressed: bool, network: Network) -> Privkey {
-        Privkey {
-            compressed: compressed,
-            network: network,
-            key: key,
-        }
-    }
-
-    /// Computes the public key as supposed to be used with this secret
     pub fn public_key<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>) -> PublicKey {
-        PublicKey::from_secret_key(secp, &self.key)
+        PublicKey {
-    }
+            compressed: self.compressed,
-
+            key: secp256k1::PublicKey::from_secret_key(secp, &self.key)
-    /// Converts a private key to a segwit address
-    #[inline]
-    pub fn to_address<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>) -> Address {
-        Address::p2wpkh(&self.public_key(secp), self.network)
-    }
-
-    /// Converts a private key to a legacy (non-segwit) address
-    #[inline]
-    pub fn to_legacy_address<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>) -> Address {
-        if self.compressed {
-            Address::p2pkh(&self.public_key(secp), self.network)
         }
-        else {
-            Address::p2upkh(&self.public_key(secp), self.network)
-        }
-    }
-
-    /// Accessor for the underlying secp key
-    #[inline]
-    pub fn secret_key(&self) -> &SecretKey {
-        &self.key
-    }
-
-    /// Accessor for the underlying secp key that consumes the privkey
-    #[inline]
-    pub fn into_secret_key(self) -> SecretKey {
-        self.key
-    }
-
-    /// Accessor for the network type
-    #[inline]
-    pub fn network(&self) -> Network {
-        self.network
-    }
-
-    /// Accessor for the compressed flag
-    #[inline]
-    pub fn is_compressed(&self) -> bool {
-        self.compressed
     }
 
     /// Format the private key to WIF format.
@@ -111,7 +102,6 @@ impl Privkey {
     }
 
     /// Get WIF encoding of this private key.
-    #[inline]
     pub fn to_wif(&self) -> String {
         let mut buf = String::new();
         buf.write_fmt(format_args!("{}", self)).unwrap();
@@ -120,7 +110,7 @@ impl Privkey {
     }
 
     /// Parse WIF encoded private key.
-    pub fn from_wif(wif: &str) -> Result<Privkey, encode::Error> {
+    pub fn from_wif(wif: &str) -> Result<PrivateKey, encode::Error> {
         let data = base58::from_check(wif)?;
 
         let compressed = match data.len() {
@@ -135,70 +125,68 @@ impl Privkey {
             x   => { return Err(encode::Error::Base58(base58::Error::InvalidVersion(vec![x]))); }
         };
 
-        let key = SecretKey::from_slice(&data[1..33])
+        Ok(PrivateKey {
-            .map_err(|_| base58::Error::Other("Secret key out of range".to_owned()))?;
-
-        Ok(Privkey {
             compressed: compressed,
             network: network,
-            key: key
+            key: secp256k1::SecretKey::from_slice(&data[1..33])?,
         })
     }
 }
 
-impl fmt::Display for Privkey {
+impl fmt::Display for PrivateKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         self.fmt_wif(f)
     }
 }
 
-impl fmt::Debug for Privkey {
+impl fmt::Debug for PrivateKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "[private key data]")
     }
 }
 
-impl FromStr for Privkey {
+impl FromStr for PrivateKey {
     type Err = encode::Error;
-    fn from_str(s: &str) -> Result<Privkey, encode::Error> {
+    fn from_str(s: &str) -> Result<PrivateKey, encode::Error> {
-        Privkey::from_wif(s)
+        PrivateKey::from_wif(s)
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::Privkey;
+    use super::PrivateKey;
     use secp256k1::Secp256k1;
     use std::str::FromStr;
     use network::constants::Network::Testnet;
     use network::constants::Network::Bitcoin;
+    use util::address::Address;
 
     #[test]
     fn test_key_derivation() {
         // testnet compressed
-        let sk = Privkey::from_wif("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy").unwrap();
+        let sk = PrivateKey::from_wif("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy").unwrap();
-        assert_eq!(sk.network(), Testnet);
+        assert_eq!(sk.network, Testnet);
-        assert_eq!(sk.is_compressed(), true);
+        assert_eq!(sk.compressed, true);
         assert_eq!(&sk.to_wif(), "cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy");
 
         let secp = Secp256k1::new();
-        let pk = sk.to_legacy_address(&secp);
+        let pk = Address::p2pkh(&sk.public_key(&secp), sk.network);
         assert_eq!(&pk.to_string(), "mqwpxxvfv3QbM8PU8uBx2jaNt9btQqvQNx");
 
         // test string conversion
         assert_eq!(&sk.to_string(), "cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy");
         let sk_str =
-            Privkey::from_str("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy").unwrap();
+            PrivateKey::from_str("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy").unwrap();
         assert_eq!(&sk.to_wif(), &sk_str.to_wif());
 
         // mainnet uncompressed
-        let sk = Privkey::from_wif("5JYkZjmN7PVMjJUfJWfRFwtuXTGB439XV6faajeHPAM9Z2PT2R3").unwrap();
+        let sk = PrivateKey::from_wif("5JYkZjmN7PVMjJUfJWfRFwtuXTGB439XV6faajeHPAM9Z2PT2R3").unwrap();
-        assert_eq!(sk.network(), Bitcoin);
+        assert_eq!(sk.network, Bitcoin);
-        assert_eq!(sk.is_compressed(), false);
+        assert_eq!(sk.compressed, false);
         assert_eq!(&sk.to_wif(), "5JYkZjmN7PVMjJUfJWfRFwtuXTGB439XV6faajeHPAM9Z2PT2R3");
 
         let secp = Secp256k1::new();
-        let pk = sk.to_legacy_address(&secp);
+        let pk = Address::p2pkh(&sk.public_key(&secp), sk.network);
         assert_eq!(&pk.to_string(), "1GhQvF6dL8xa6wBxLnWmHcQsurx9RxiMc8");
     }
 }

src/util/mod.rs

@@ -16,7 +16,7 @@
 //!
 //! Functions needed by all parts of the Bitcoin library
 
-pub mod privkey;
+pub mod key;
 pub mod address;
 pub mod base58;
 pub mod bip32;
@@ -29,8 +29,6 @@ pub mod uint;
 
 use std::{error, fmt};
 
-use secp256k1;
-
 use network;
 use consensus::encode;
 
@@ -59,8 +57,6 @@ pub trait BitArray {
 /// if appropriate.
 #[derive(Debug)]
 pub enum Error {
-    /// secp-related error
-    Secp256k1(secp256k1::Error),
     /// Encoding error
     Encode(encode::Error),
     /// Network error
@@ -74,7 +70,6 @@ pub enum Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
-            Error::Secp256k1(ref e) => fmt::Display::fmt(e, f),
             Error::Encode(ref e) => fmt::Display::fmt(e, f),
             Error::Network(ref e) => fmt::Display::fmt(e, f),
             Error::SpvBadProofOfWork | Error::SpvBadTarget => f.write_str(error::Error::description(self)),
@@ -85,7 +80,6 @@ impl fmt::Display for Error {
 impl error::Error for Error {
     fn cause(&self) -> Option<&error::Error> {
         match *self {
-            Error::Secp256k1(ref e) => Some(e),
             Error::Encode(ref e) => Some(e),
             Error::Network(ref e) => Some(e),
             Error::SpvBadProofOfWork | Error::SpvBadTarget => None
@@ -94,7 +88,6 @@ impl error::Error for Error {
 
     fn description(&self) -> &str {
         match *self {
-            Error::Secp256k1(ref e) => e.description(),
             Error::Encode(ref e) => e.description(),
             Error::Network(ref e) => e.description(),
             Error::SpvBadProofOfWork => "target correct but not attained",
@@ -103,13 +96,6 @@ impl error::Error for Error {
     }
 }
 
-#[doc(hidden)]
-impl From<secp256k1::Error> for Error {
-    fn from(e: secp256k1::Error) -> Error {
-        Error::Secp256k1(e)
-    }
-}
-
 #[doc(hidden)]
 impl From<encode::Error> for Error {
     fn from(e: encode::Error) -> Error {