[BREAKING CHANGE] Make PK::from_secret_key() return a Result; change from_ffi functions to From impls

If you try to call PublicKey::from_secret() key with an incapable context it will
now return an error. Before it would pass through to the underlying library which
would terminate the process, something we strive to never expose.

Also change the from_ffi functions on various types to impl's of From to be more
Rustic. We cannot change the from_slice functions because they have error returns.

Also add a Secp256k1::without_caps() function which creates a capability-less
context. I find myself using this in so many places downstream that it seems
appropriate.

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "secp256k1"
-version = "0.2.4"
+version = "0.3.0"
 authors = [ "Dawid Ciężarkiewicz <dpc@ucore.info>",
             "Andrew Poelstra <apoelstra@wpsoftware.net>" ]
 license = "CC0-1.0"

src/ecdh.rs

@@ -38,12 +38,6 @@ impl SharedSecret {
         }
     }
 
-    /// Creates a new shared secret from a FFI shared secret
-    #[inline]
-    pub fn from_ffi(ss: ffi::SharedSecret) -> SharedSecret {
-        SharedSecret(ss)
-    }
-
     /// Obtains a raw pointer suitable for use with FFI functions
     #[inline]
     pub fn as_ptr(&self) -> *const ffi::SharedSecret {
@@ -51,6 +45,15 @@ impl SharedSecret {
     }
 }
 
+/// Creates a new shared secret from a FFI shared secret
+impl From<ffi::SharedSecret> for SharedSecret {
+    #[inline]
+    fn from(ss: ffi::SharedSecret) -> SharedSecret {
+        SharedSecret(ss)
+    }
+}
+
+
 impl ops::Index<usize> for SharedSecret {
     type Output = u8;
 

src/key.rs

@@ -22,8 +22,8 @@ use rand::Rng;
 use serialize::{Decoder, Decodable, Encoder, Encodable};
 use serde::{Serialize, Deserialize, Serializer, Deserializer};
 
-use super::Secp256k1;
+use super::{Secp256k1, ContextFlag};
-use super::Error::{self, InvalidPublicKey, InvalidSecretKey, Unknown};
+use super::Error::{self, IncapableContext, InvalidPublicKey, InvalidSecretKey, Unknown};
 use constants;
 use ffi;
 
@@ -103,12 +103,6 @@ impl PublicKey {
         PublicKey(ffi::PublicKey::new())
     }
 
-    /// Creates a new public key from a FFI public key
-    #[inline]
-    pub fn from_ffi(pk: ffi::PublicKey) -> PublicKey {
-        PublicKey(pk)
-    }
-
     /// Determines whether a pubkey is valid
     #[inline]
     pub fn is_valid(&self) -> bool {
@@ -127,7 +121,10 @@ impl PublicKey {
     #[inline]
     pub fn from_secret_key(secp: &Secp256k1,
                            sk: &SecretKey)
-                           -> PublicKey {
+                           -> Result<PublicKey, Error> {
+        if secp.caps == ContextFlag::VerifyOnly || secp.caps == ContextFlag::None {
+            return Err(IncapableContext);
+        }
         let mut pk = unsafe { ffi::PublicKey::blank() };
         unsafe {
             // We can assume the return value because it's not possible to construct
@@ -135,7 +132,7 @@ impl PublicKey {
             let res = ffi::secp256k1_ec_pubkey_create(secp.ctx, &mut pk, sk.as_ptr());
             debug_assert_eq!(res, 1);
         }
-        PublicKey(pk)
+        Ok(PublicKey(pk))
     }
 
     /// Creates a public key directly from a slice
@@ -215,6 +212,15 @@ impl Decodable for PublicKey {
     }
 }
 
+/// Creates a new public key from a FFI public key
+impl From<ffi::PublicKey> for PublicKey {
+    #[inline]
+    fn from(pk: ffi::PublicKey) -> PublicKey {
+        PublicKey(pk)
+    }
+}
+
+
 impl Encodable for PublicKey {
     fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
         let secp = Secp256k1::with_caps(::ContextFlag::None);
@@ -277,7 +283,7 @@ impl Serialize for PublicKey {
 #[cfg(test)]
 mod test {
     use super::super::Secp256k1;
-    use super::super::Error::{InvalidPublicKey, InvalidSecretKey};
+    use super::super::Error::{InvalidPublicKey, InvalidSecretKey, IncapableContext};
     use super::{PublicKey, SecretKey};
     use super::super::constants;
 
@@ -337,6 +343,13 @@ mod test {
                                         0xBF, 0xD2, 0x5E, 0x8C, 0xD0, 0x36, 0x41, 0x41]).is_err());
     }
 
+    #[test]
+    fn test_pubkey_from_slice_bad_context() {
+        let s = Secp256k1::without_caps();
+        let sk = SecretKey::new(&s, &mut thread_rng());
+        assert_eq!(PublicKey::from_secret_key(&s, &sk), Err(IncapableContext))
+    }
+
     #[test]
     fn test_bad_deserialize() {
         use std::io::Cursor;
@@ -554,15 +567,15 @@ mod test {
         let (mut sk1, mut pk1) = s.generate_keypair(&mut thread_rng()).unwrap();
         let (mut sk2, mut pk2) = s.generate_keypair(&mut thread_rng()).unwrap();
 
-        assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1);
+        assert_eq!(PublicKey::from_secret_key(&s, &sk1).unwrap(), pk1);
         assert!(sk1.add_assign(&s, &sk2).is_ok());
         assert!(pk1.add_exp_assign(&s, &sk2).is_ok());
-        assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1);
+        assert_eq!(PublicKey::from_secret_key(&s, &sk1).unwrap(), pk1);
 
-        assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2);
+        assert_eq!(PublicKey::from_secret_key(&s, &sk2).unwrap(), pk2);
         assert!(sk2.add_assign(&s, &sk1).is_ok());
         assert!(pk2.add_exp_assign(&s, &sk1).is_ok());
-        assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2);
+        assert_eq!(PublicKey::from_secret_key(&s, &sk2).unwrap(), pk2);
     }
 }
 

src/lib.rs

@@ -99,12 +99,6 @@ impl Signature {
         }
     }
 
-    /// Creates a new public key from a FFI public key
-    #[inline]
-    pub fn from_ffi(sig: ffi::Signature) -> Signature {
-        Signature(sig)
-    }
-
     /// Obtains a raw pointer suitable for use with FFI functions
     #[inline]
     pub fn as_ptr(&self) -> *const ffi::Signature {
@@ -126,6 +120,15 @@ impl Signature {
     }
 }
 
+/// Creates a new signature from a FFI signature
+impl From<ffi::Signature> for Signature {
+    #[inline]
+    fn from(sig: ffi::Signature) -> Signature {
+        Signature(sig)
+    }
+}
+
+
 impl RecoverableSignature {
     #[inline]
     /// Converts a compact-encoded byte slice to a signature. This
@@ -146,12 +149,6 @@ impl RecoverableSignature {
         }
     }
 
-    /// Creates a new public key from a FFI public key
-    #[inline]
-    pub fn from_ffi(sig: ffi::RecoverableSignature) -> RecoverableSignature {
-        RecoverableSignature(sig)
-    }
-
     /// Obtains a raw pointer suitable for use with FFI functions
     #[inline]
     pub fn as_ptr(&self) -> *const ffi::RecoverableSignature {
@@ -184,6 +181,14 @@ impl RecoverableSignature {
     }
 }
 
+/// Creates a new recoverable signature from a FFI one
+impl From<ffi::RecoverableSignature> for RecoverableSignature {
+    #[inline]
+    fn from(sig: ffi::RecoverableSignature) -> RecoverableSignature {
+        RecoverableSignature(sig)
+    }
+}
+
 impl ops::Index<usize> for Signature {
     type Output = u8;
 
@@ -345,6 +350,11 @@ impl Secp256k1 {
         Secp256k1 { ctx: unsafe { ffi::secp256k1_context_create(flag) }, caps: caps }
     }
 
+    /// Creates a new Secp256k1 context with no capabilities (just de/serialization)
+    pub fn without_caps() -> Secp256k1 {
+        Secp256k1::with_caps(ContextFlag::None)
+    }
+
     /// (Re)randomizes the Secp256k1 context for cheap sidechannel resistence;
     /// see comment in libsecp256k1 commit d2275795f by Gregory Maxwell
     pub fn randomize<R: Rng>(&mut self, rng: &mut R) {
@@ -370,12 +380,8 @@ impl Secp256k1 {
     #[inline]
     pub fn generate_keypair<R: Rng>(&self, rng: &mut R)
                                    -> Result<(key::SecretKey, key::PublicKey), Error> {
-        if self.caps == ContextFlag::VerifyOnly || self.caps == ContextFlag::None {
-            return Err(Error::IncapableContext);
-        }
-
         let sk = key::SecretKey::new(self, rng);
-        let pk = key::PublicKey::from_secret_key(self, &sk);
+        let pk = try!(key::PublicKey::from_secret_key(self, &sk));
         Ok((sk, pk))
     }
 
@@ -395,7 +401,7 @@ impl Secp256k1 {
                                                  sk.as_ptr(), ffi::secp256k1_nonce_function_rfc6979,
                                                  ptr::null()), 1);
         }
-        Ok(Signature::from_ffi(ret))
+        Ok(Signature::from(ret))
     }
 
     /// Constructs a signature for `msg` using the secret key `sk` and nonce `nonce`.
@@ -414,7 +420,7 @@ impl Secp256k1 {
                                                              sk.as_ptr(), ffi::secp256k1_nonce_function_rfc6979,
                                                              ptr::null()), 1);
         }
-        Ok(RecoverableSignature::from_ffi(ret))
+        Ok(RecoverableSignature::from(ret))
     }
 
     /// Determines the public key for which `sig` is a valid signature for
@@ -434,7 +440,7 @@ impl Secp256k1 {
                 return Err(Error::InvalidSignature);
             }
         };
-        Ok(key::PublicKey::from_ffi(pk))
+        Ok(key::PublicKey::from(pk))
     }
 
     /// Checks that `sig` is a valid ECDSA signature for `msg` using the public
@@ -636,7 +642,7 @@ mod tests {
         for key in wild_keys.iter().map(|k| SecretKey::from_slice(&s, &k[..]).unwrap()) {
             for msg in wild_msgs.iter().map(|m| Message::from_slice(&m[..]).unwrap()) {
                 let sig = s.sign(&msg, &key).unwrap();
-                let pk = PublicKey::from_secret_key(&s, &key);
+                let pk = PublicKey::from_secret_key(&s, &key).unwrap();
                 assert_eq!(s.verify(&msg, &sig, &pk), Ok(()));
             }
         }