diff --git a/zebra-chain/src/orchard/action.rs b/zebra-chain/src/orchard/action.rs index 33dfcb56c..a4136ba1a 100644 --- a/zebra-chain/src/orchard/action.rs +++ b/zebra-chain/src/orchard/action.rs @@ -1,4 +1,4 @@ -use std::io; +use std::{convert::TryFrom, io}; use halo2::pasta::pallas; @@ -61,7 +61,7 @@ impl ZcashDeserialize for Action { fn zcash_deserialize(mut reader: R) -> Result { Ok(Action { cv: ValueCommitment::zcash_deserialize(&mut reader)?, - nullifier: Nullifier::from(reader.read_32_bytes()?), + nullifier: Nullifier::try_from(reader.read_32_bytes()?)?, rk: reader.read_32_bytes()?.into(), cm_x: pallas::Base::zcash_deserialize(&mut reader)?, ephemeral_key: keys::EphemeralPublicKey::zcash_deserialize(&mut reader)?, diff --git a/zebra-chain/src/orchard/arbitrary.rs b/zebra-chain/src/orchard/arbitrary.rs index ded9d3d0d..0147b01a1 100644 --- a/zebra-chain/src/orchard/arbitrary.rs +++ b/zebra-chain/src/orchard/arbitrary.rs @@ -1,12 +1,15 @@ use group::prime::PrimeCurveAffine; use halo2::pasta::pallas; -use proptest::{arbitrary::any, array, prelude::*}; +use proptest::{arbitrary::any, array, collection::vec, prelude::*}; use crate::primitives::redpallas::{Signature, SpendAuth, VerificationKeyBytes}; use super::{keys, note, Action, AuthorizedAction, Flags, NoteCommitment, ValueCommitment}; -use std::marker::PhantomData; +use std::{ + convert::{TryFrom, TryInto}, + marker::PhantomData, +}; impl Arbitrary for Action { type Parameters = (); @@ -41,8 +44,12 @@ impl Arbitrary for note::Nullifier { fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy { use halo2::arithmetic::FieldExt; - (any::()) - .prop_map(|number| Self::from(pallas::Scalar::from_u64(number).to_bytes())) + (vec(any::(), 64)) + .prop_map(|bytes| { + let bytes = bytes.try_into().expect("vec is the correct length"); + Self::try_from(pallas::Scalar::from_bytes_wide(&bytes).to_bytes()) + .expect("a valid generated nullifier") + }) .boxed() } diff --git a/zebra-chain/src/orchard/note/nullifiers.rs b/zebra-chain/src/orchard/note/nullifiers.rs index 897ce6482..831ea161b 100644 --- a/zebra-chain/src/orchard/note/nullifiers.rs +++ b/zebra-chain/src/orchard/note/nullifiers.rs @@ -3,13 +3,16 @@ use halo2::{arithmetic::FieldExt, pasta::pallas}; -use crate::serialization::serde_helpers; +use crate::serialization::{serde_helpers, SerializationError}; use super::super::{ commitment::NoteCommitment, keys::NullifierDerivingKey, note::Note, sinsemilla::*, }; -use std::hash::{Hash, Hasher}; +use std::{ + convert::TryFrom, + hash::{Hash, Hasher}, +}; /// A cryptographic permutation, defined in [poseidonhash]. /// @@ -46,9 +49,19 @@ impl Hash for Nullifier { } } -impl From<[u8; 32]> for Nullifier { - fn from(bytes: [u8; 32]) -> Self { - Self(pallas::Base::from_bytes(&bytes).unwrap()) +impl TryFrom<[u8; 32]> for Nullifier { + type Error = SerializationError; + + fn try_from(bytes: [u8; 32]) -> Result { + let possible_point = pallas::Base::from_bytes(&bytes); + + if possible_point.is_some().into() { + Ok(Self(possible_point.unwrap())) + } else { + Err(SerializationError::Parse( + "Invalid pallas::Base value for orchard Nullifier", + )) + } } } diff --git a/zebra-chain/src/transaction/tests/vectors.rs b/zebra-chain/src/transaction/tests/vectors.rs index c9e0d088e..b7bbe4d24 100644 --- a/zebra-chain/src/transaction/tests/vectors.rs +++ b/zebra-chain/src/transaction/tests/vectors.rs @@ -315,3 +315,29 @@ fn fake_v5_round_trip_for_network(network: Network) { ); } } + +#[test] +fn invalid_orchard_nullifier() { + zebra_test::init(); + + use std::convert::TryFrom; + + // generated by proptest using something as: + // ```rust + // ... + // array::uniform32(any::()).prop_map(|x| Self::try_from(x).unwrap()).boxed() + // ... + // ``` + let invalid_nullifier_bytes = [ + 62, 157, 27, 63, 100, 228, 1, 82, 140, 16, 238, 78, 68, 19, 221, 184, 189, 207, 230, 95, + 194, 216, 165, 24, 110, 221, 139, 195, 106, 98, 192, 71, + ]; + + assert_eq!( + orchard::Nullifier::try_from(invalid_nullifier_bytes) + .err() + .unwrap() + .to_string(), + SerializationError::Parse("Invalid pallas::Base value for orchard Nullifier").to_string() + ); +}