From 15b8183ea8bf9d9e19988ff28ea49de567985620 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 28 Aug 2014 11:11:25 -0700 Subject: [PATCH] Remove error return from `PublicKey::from_secret_key()` Make sure that you cannot create an invalid `SecretKey` in the first place. Unbreaks the API. [unbreaking-change] --- src/key.rs | 26 ++++++++++++++++---------- src/secp256k1.rs | 9 ++------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/key.rs b/src/key.rs index c4770b9..be55ec7 100644 --- a/src/key.rs +++ b/src/key.rs @@ -76,6 +76,12 @@ impl SecretKey { /// Creates a new random secret key #[inline] pub fn new(rng: &mut R) -> SecretKey { + let mut data = random_32_bytes(rng); + unsafe { + while ffi::secp256k1_ecdsa_seckey_verify(data.as_ptr()) == 0 { + data = random_32_bytes(rng); + } + } SecretKey(random_32_bytes(rng)) } @@ -129,21 +135,21 @@ impl PublicKey { /// Creates a new public key from a secret key. #[inline] - pub fn from_secret_key(sk: &SecretKey, compressed: bool) -> Result { + pub fn from_secret_key(sk: &SecretKey, compressed: bool) -> PublicKey { let mut pk = PublicKey::new(compressed); let compressed = if compressed {1} else {0}; let mut len = 0; init(); unsafe { - if ffi::secp256k1_ecdsa_pubkey_create( + // We can assume the return value because it's not possible to construct + // an invalid `SecretKey` without transmute trickery or something + assert_eq!(ffi::secp256k1_ecdsa_pubkey_create( pk.as_mut_ptr(), &mut len, - sk.as_ptr(), compressed) != 1 { - return Err(InvalidSecretKey); - } + sk.as_ptr(), compressed), 1); } assert_eq!(len as uint, pk.len()); - Ok(pk) + pk } /// Creates a public key directly from a slice @@ -363,15 +369,15 @@ mod test { let (mut sk1, mut pk1) = s.generate_keypair(true).unwrap(); let (mut sk2, mut pk2) = s.generate_keypair(true).unwrap(); - assert_eq!(PublicKey::from_secret_key(&sk1, true), Ok(pk1)); + assert_eq!(PublicKey::from_secret_key(&sk1, true), pk1); assert!(sk1.add_assign(&sk2).is_ok()); assert!(pk1.add_exp_assign(&sk2).is_ok()); - assert_eq!(PublicKey::from_secret_key(&sk1, true), Ok(pk1)); + assert_eq!(PublicKey::from_secret_key(&sk1, true), pk1); - assert_eq!(PublicKey::from_secret_key(&sk2, true), Ok(pk2)); + assert_eq!(PublicKey::from_secret_key(&sk2, true), pk2); assert!(sk2.add_assign(&sk1).is_ok()); assert!(pk2.add_exp_assign(&sk1).is_ok()); - assert_eq!(PublicKey::from_secret_key(&sk2, true), Ok(pk2)); + assert_eq!(PublicKey::from_secret_key(&sk2, true), pk2); } } diff --git a/src/secp256k1.rs b/src/secp256k1.rs index 6d4f88c..c759442 100644 --- a/src/secp256k1.rs +++ b/src/secp256k1.rs @@ -136,13 +136,8 @@ impl Secp256k1 { -> Result<(key::SecretKey, key::PublicKey)> { match self.rng { Ok(ref mut rng) => { - let mut sk = key::SecretKey::new(rng); - let mut pk = key::PublicKey::from_secret_key(&sk, compressed); - while pk.is_err() { - sk = key::SecretKey::new(rng); - pk = key::PublicKey::from_secret_key(&sk, compressed); - } - Ok((sk, pk.unwrap())) + let sk = key::SecretKey::new(rng); + Ok((sk, key::PublicKey::from_secret_key(&sk, compressed))) } Err(ref e) => Err(RngError(e.clone())) }