Remove unsafe markers and just call `init` everywhere

It turns out I need to run `init` before pretty-much every FFI function,
which means that most everything would have to be marked unsafe if I'm
expecting the Rust user to do this. This is unacceptable -- users who
need to sacrifice safety for speed can just use the `ffi::` functions
instead.

Also, I noticed that I was locking up in `PublicKey::from_secret_key`.
Fix to return an error value -- unfortunately a breaking change since
it changes the function signature.

[breaking-change]
This commit is contained in:
Andrew Poelstra 2014-08-28 10:59:44 -07:00
parent a5951eff47
commit a67260eb3a
2 changed files with 35 additions and 25 deletions

View File

@ -21,6 +21,7 @@ use std::rand::Rng;
use constants; use constants;
use ffi; use ffi;
use super::init;
use super::{Result, InvalidNonce, InvalidPublicKey, InvalidSecretKey, Unknown}; use super::{Result, InvalidNonce, InvalidPublicKey, InvalidSecretKey, Unknown};
/// Secret 256-bit nonce used as `k` in an ECDSA signature /// Secret 256-bit nonce used as `k` in an ECDSA signature
@ -81,16 +82,17 @@ impl SecretKey {
/// Converts a `SECRET_KEY_SIZE`-byte slice to a secret key /// Converts a `SECRET_KEY_SIZE`-byte slice to a secret key
#[inline] #[inline]
pub fn from_slice(data: &[u8]) -> Result<SecretKey> { pub fn from_slice(data: &[u8]) -> Result<SecretKey> {
init();
match data.len() { match data.len() {
constants::SECRET_KEY_SIZE => { constants::SECRET_KEY_SIZE => {
let mut ret = [0, ..constants::SECRET_KEY_SIZE]; let mut ret = [0, ..constants::SECRET_KEY_SIZE];
unsafe { unsafe {
copy_nonoverlapping_memory(ret.as_mut_ptr(),
data.as_ptr(),
data.len());
if ffi::secp256k1_ecdsa_seckey_verify(data.as_ptr()) == 0 { if ffi::secp256k1_ecdsa_seckey_verify(data.as_ptr()) == 0 {
return Err(InvalidSecretKey); return Err(InvalidSecretKey);
} }
copy_nonoverlapping_memory(ret.as_mut_ptr(),
data.as_ptr(),
data.len());
} }
Ok(SecretKey(ret)) Ok(SecretKey(ret))
} }
@ -100,7 +102,11 @@ impl SecretKey {
#[inline] #[inline]
/// Adds one secret key to another, modulo the curve order /// Adds one secret key to another, modulo the curve order
/// Marked `unsafe` since you must
/// call `init()` (or construct a `Secp256k1`, which does this for you) before
/// using this function
pub fn add_assign(&mut self, other: &SecretKey) -> Result<()> { pub fn add_assign(&mut self, other: &SecretKey) -> Result<()> {
init();
unsafe { unsafe {
if ffi::secp256k1_ecdsa_privkey_tweak_add(self.as_mut_ptr(), other.as_ptr()) != 1 { if ffi::secp256k1_ecdsa_privkey_tweak_add(self.as_mut_ptr(), other.as_ptr()) != 1 {
Err(Unknown) Err(Unknown)
@ -121,23 +127,23 @@ impl PublicKey {
) )
} }
/// Creates a new public key from a secret key. Marked `unsafe` since you must /// Creates a new public key from a secret key.
/// call `init()` (or construct a `Secp256k1`, which does this for you) before
/// using this function
#[inline] #[inline]
pub unsafe fn from_secret_key(sk: &SecretKey, compressed: bool) -> PublicKey { pub fn from_secret_key(sk: &SecretKey, compressed: bool) -> Result<PublicKey> {
let mut pk = PublicKey::new(compressed); let mut pk = PublicKey::new(compressed);
let compressed = if compressed {1} else {0}; let compressed = if compressed {1} else {0};
let mut len = 0; let mut len = 0;
while ffi::secp256k1_ecdsa_pubkey_create( init();
pk.as_mut_ptr(), &mut len, unsafe {
sk.as_ptr(), compressed) != 1 { if ffi::secp256k1_ecdsa_pubkey_create(
// loop pk.as_mut_ptr(), &mut len,
sk.as_ptr(), compressed) != 1 {
return Err(InvalidSecretKey);
}
} }
assert_eq!(len as uint, pk.len()); assert_eq!(len as uint, pk.len());
Ok(pk)
pk
} }
/// Creates a public key directly from a slice /// Creates a public key directly from a slice
@ -218,6 +224,7 @@ impl PublicKey {
#[inline] #[inline]
/// Adds the pk corresponding to `other` to the pk `self` in place /// Adds the pk corresponding to `other` to the pk `self` in place
pub fn add_exp_assign(&mut self, other: &SecretKey) -> Result<()> { pub fn add_exp_assign(&mut self, other: &SecretKey) -> Result<()> {
init();
unsafe { unsafe {
if ffi::secp256k1_ecdsa_pubkey_tweak_add(self.as_mut_ptr(), if ffi::secp256k1_ecdsa_pubkey_tweak_add(self.as_mut_ptr(),
self.len() as ::libc::c_int, self.len() as ::libc::c_int,
@ -356,17 +363,15 @@ mod test {
let (mut sk1, mut pk1) = s.generate_keypair(true).unwrap(); let (mut sk1, mut pk1) = s.generate_keypair(true).unwrap();
let (mut sk2, mut pk2) = s.generate_keypair(true).unwrap(); let (mut sk2, mut pk2) = s.generate_keypair(true).unwrap();
unsafe { 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!(sk1.add_assign(&sk2).is_ok()); assert!(pk1.add_exp_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), pk2); assert_eq!(PublicKey::from_secret_key(&sk2, true), Ok(pk2));
assert!(sk2.add_assign(&sk1).is_ok()); assert!(sk2.add_assign(&sk1).is_ok());
assert!(pk2.add_exp_assign(&sk1).is_ok()); assert!(pk2.add_exp_assign(&sk1).is_ok());
assert_eq!(PublicKey::from_secret_key(&sk2, true), pk2); assert_eq!(PublicKey::from_secret_key(&sk2, true), Ok(pk2));
}
} }
} }

View File

@ -136,8 +136,13 @@ impl Secp256k1 {
-> Result<(key::SecretKey, key::PublicKey)> { -> Result<(key::SecretKey, key::PublicKey)> {
match self.rng { match self.rng {
Ok(ref mut rng) => { Ok(ref mut rng) => {
let sk = key::SecretKey::new(rng); let mut sk = key::SecretKey::new(rng);
Ok(unsafe { (sk, key::PublicKey::from_secret_key(&sk, compressed)) }) 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()))
} }
Err(ref e) => Err(RngError(e.clone())) Err(ref e) => Err(RngError(e.clone()))
} }