diff --git a/src/secret_key.rs b/src/secret_key.rs index b60bc1f..b293b3a 100644 --- a/src/secret_key.rs +++ b/src/secret_key.rs @@ -1,13 +1,16 @@ -use std::marker::PhantomData; +use std::{ + convert::{TryFrom, TryInto}, + marker::PhantomData, +}; -use crate::{PublicKey, Randomizer, Scalar, SigType, Signature, SpendAuth}; +use crate::{Error, PublicKey, Randomizer, Scalar, SigType, Signature, SpendAuth}; use rand_core::{CryptoRng, RngCore}; /// A RedJubJub secret key. #[derive(Copy, Clone, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serde", serde(from = "SerdeHelper"))] +#[cfg_attr(feature = "serde", serde(try_from = "SerdeHelper"))] #[cfg_attr(feature = "serde", serde(into = "SerdeHelper"))] #[cfg_attr(feature = "serde", serde(bound = "T: SigType"))] pub struct SecretKey { @@ -27,26 +30,30 @@ impl From> for [u8; 32] { } } -impl From<[u8; 32]> for SecretKey { - fn from(bytes: [u8; 32]) -> Self { - let sk = { - // XXX-jubjub: would be nice to unconditionally deser - // This incantation ensures deserialization is infallible. - let mut wide = [0; 64]; - wide[0..32].copy_from_slice(&bytes); - Scalar::from_bytes_wide(&wide) - }; - let pk = PublicKey::from_secret(&sk); - SecretKey { sk, pk } +impl TryFrom<[u8; 32]> for SecretKey { + type Error = Error; + + fn try_from(bytes: [u8; 32]) -> Result { + // XXX-jubjub: this should not use CtOption + let maybe_sk = Scalar::from_bytes(&bytes); + if maybe_sk.is_some().into() { + let sk = maybe_sk.unwrap(); + let pk = PublicKey::from_secret(&sk); + Ok(SecretKey { sk, pk }) + } else { + Err(Error::MalformedSecretKey) + } } } #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] struct SerdeHelper([u8; 32]); -impl From for SecretKey { - fn from(helper: SerdeHelper) -> Self { - helper.0.into() +impl TryFrom for SecretKey { + type Error = Error; + + fn try_from(helper: SerdeHelper) -> Result { + helper.0.try_into() } } diff --git a/tests/bincode.rs b/tests/bincode.rs index dedac21..834a4a6 100644 --- a/tests/bincode.rs +++ b/tests/bincode.rs @@ -9,26 +9,30 @@ proptest! { fn secretkey_serialization( bytes in prop::array::uniform32(any::()), ) { - let sk_from = SecretKey::::from(bytes); - let sk_bincode: SecretKey:: - = bincode::deserialize(&bytes[..]).unwrap(); + let sk_result_from = SecretKey::::try_from(bytes); + let sk_result_bincode: Result, _> + = bincode::deserialize(&bytes[..]); - // Check 1: both decoding methods should have the same public key - let pk_bytes_from = PublicKeyBytes::from(PublicKey::from(&sk_from)); - let pk_bytes_bincode = PublicKeyBytes::from(PublicKey::from(&sk_bincode)); - assert_eq!(pk_bytes_from, pk_bytes_bincode); + // Check 1: both decoding methods should agree + match (sk_result_from, sk_result_bincode) { + // Both agree on success + (Ok(sk_from), Ok(sk_bincode)) => { + let pk_bytes_from = PublicKeyBytes::from(PublicKey::from(&sk_from)); + let pk_bytes_bincode = PublicKeyBytes::from(PublicKey::from(&sk_bincode)); + assert_eq!(pk_bytes_from, pk_bytes_bincode); - // The below tests fail because we do not require canonically-encoded secret keys. - /* + // Check 2: bincode encoding should match original bytes. + let bytes_bincode = bincode::serialize(&sk_from).unwrap(); + assert_eq!(&bytes[..], &bytes_bincode[..]); - // Check 2: bincode encoding should match original bytes. - let bytes_bincode = bincode::serialize(&sk_from).unwrap(); - assert_eq!(&bytes[..], &bytes_bincode[..]); - - // Check 3: From encoding should match original bytes. - let bytes_from: [u8; 32] = sk_bincode.into(); - assert_eq!(&bytes[..], &bytes_from[..]); - */ + // Check 3: From encoding should match original bytes. + let bytes_from: [u8; 32] = sk_bincode.into(); + assert_eq!(&bytes[..], &bytes_from[..]); + } + // Both agree on failure + (Err(_), Err(_)) => {}, + _ => panic!("bincode and try_from do not agree"), + } } #[test]