diff --git a/zebra-chain/src/orchard/arbitrary.rs b/zebra-chain/src/orchard/arbitrary.rs index 5bb60e3dc..30ac3a530 100644 --- a/zebra-chain/src/orchard/arbitrary.rs +++ b/zebra-chain/src/orchard/arbitrary.rs @@ -29,7 +29,7 @@ impl Arbitrary for Action { nullifier, rk, cm_x: NoteCommitment(pallas::Affine::identity()).extract_x(), - ephemeral_key: keys::EphemeralPublicKey(pallas::Affine::identity()), + ephemeral_key: keys::EphemeralPublicKey(pallas::Affine::generator()), enc_ciphertext, out_ciphertext, }) diff --git a/zebra-chain/src/orchard/keys.rs b/zebra-chain/src/orchard/keys.rs index 73842b6de..b9ba4d349 100644 --- a/zebra-chain/src/orchard/keys.rs +++ b/zebra-chain/src/orchard/keys.rs @@ -16,7 +16,7 @@ use aes::Aes256; use bech32::{self, ToBase32, Variant}; use bitvec::prelude::*; use fpe::ff1::{BinaryNumeralString, FF1}; -use group::{Group, GroupEncoding}; +use group::{prime::PrimeCurveAffine, Group, GroupEncoding}; use halo2::{ arithmetic::{Coordinates, CurveAffine, FieldExt}, pasta::pallas, @@ -1085,11 +1085,30 @@ impl PartialEq<[u8; 32]> for EphemeralPublicKey { impl TryFrom<[u8; 32]> for EphemeralPublicKey { type Error = &'static str; + /// Convert an array into a [`EphemeralPublicKey`]. + /// + /// Returns an error if the encoding is malformed or if [it encodes the + /// identity point][1]. + /// + /// > epk cannot be ๐’ช_P + /// + /// Note that this is [intrinsic to the EphemeralPublicKey][2] type and it is not + /// a separate consensus rule: + /// + /// > Define KA^{Orchard}.Public := P^*. + /// + /// [1]: https://zips.z.cash/protocol/protocol.pdf#actiondesc + /// [2]: https://zips.z.cash/protocol/protocol.pdf#concreteorchardkeyagreement fn try_from(bytes: [u8; 32]) -> Result { let possible_point = pallas::Affine::from_bytes(&bytes); if possible_point.is_some().into() { - Ok(Self(possible_point.unwrap())) + let point = possible_point.unwrap(); + if point.to_curve().is_identity().into() { + Err("pallas::Affine value for Orchard EphemeralPublicKey is the identity") + } else { + Ok(Self(possible_point.unwrap())) + } } else { Err("Invalid pallas::Affine value for Orchard EphemeralPublicKey") } diff --git a/zebra-chain/src/sapling/address.rs b/zebra-chain/src/sapling/address.rs index 693746086..7cb27c77e 100644 --- a/zebra-chain/src/sapling/address.rs +++ b/zebra-chain/src/sapling/address.rs @@ -1,6 +1,7 @@ //! Shielded addresses. use std::{ + convert::TryFrom, fmt, io::{self, Read, Write}, }; @@ -81,7 +82,8 @@ impl std::str::FromStr for Address { _ => Network::Testnet, }, diversifier: keys::Diversifier::from(diversifier_bytes), - transmission_key: keys::TransmissionKey::from(transmission_key_bytes), + transmission_key: keys::TransmissionKey::try_from(transmission_key_bytes) + .unwrap(), }) } _ => Err(SerializationError::Parse("bech32 decoding error")), @@ -147,7 +149,8 @@ mod tests { keys::IncomingViewingKey::from((authorizing_key, nullifier_deriving_key)); let diversifier = keys::Diversifier::new(&mut OsRng); - let transmission_key = keys::TransmissionKey::from((incoming_viewing_key, diversifier)); + let transmission_key = keys::TransmissionKey::try_from((incoming_viewing_key, diversifier)) + .expect("should be a valid transmission key"); let _sapling_shielded_address = Address { network: Network::Mainnet, diff --git a/zebra-chain/src/sapling/arbitrary.rs b/zebra-chain/src/sapling/arbitrary.rs index 8d681ef0e..923f49f4d 100644 --- a/zebra-chain/src/sapling/arbitrary.rs +++ b/zebra-chain/src/sapling/arbitrary.rs @@ -1,6 +1,7 @@ use std::convert::TryInto; -use jubjub::AffinePoint; +use group::Group; +use jubjub::{AffinePoint, ExtendedPoint}; use proptest::{arbitrary::any, collection::vec, prelude::*}; use rand::SeedableRng; use rand_chacha::ChaChaRng; @@ -8,8 +9,9 @@ use rand_chacha::ChaChaRng; use crate::primitives::Groth16Proof; use super::{ - keys, note, tree, FieldNotPresent, NoteCommitment, Output, OutputInTransactionV4, - PerSpendAnchor, SharedAnchor, Spend, ValueCommitment, + keys::{self, ValidatingKey}, + note, tree, FieldNotPresent, NoteCommitment, Output, OutputInTransactionV4, PerSpendAnchor, + SharedAnchor, Spend, }; impl Arbitrary for Spend { @@ -25,7 +27,7 @@ impl Arbitrary for Spend { ) .prop_map(|(per_spend_anchor, nullifier, rk, proof, sig_bytes)| Self { per_spend_anchor, - cv: ValueCommitment(AffinePoint::identity()), + cv: ExtendedPoint::generator().try_into().unwrap(), nullifier, rk, zkproof: proof, @@ -53,7 +55,7 @@ impl Arbitrary for Spend { ) .prop_map(|(nullifier, rk, proof, sig_bytes)| Self { per_spend_anchor: FieldNotPresent, - cv: ValueCommitment(AffinePoint::identity()), + cv: ExtendedPoint::generator().try_into().unwrap(), nullifier, rk, zkproof: proof, @@ -79,9 +81,11 @@ impl Arbitrary for Output { any::(), ) .prop_map(|(enc_ciphertext, out_ciphertext, zkproof)| Self { - cv: ValueCommitment(AffinePoint::identity()), + cv: ExtendedPoint::generator().try_into().unwrap(), cm_u: NoteCommitment(AffinePoint::identity()).extract_u(), - ephemeral_key: keys::EphemeralPublicKey(AffinePoint::identity()), + ephemeral_key: keys::EphemeralPublicKey( + ExtendedPoint::generator().try_into().unwrap(), + ), enc_ciphertext, out_ciphertext, zkproof, @@ -104,12 +108,13 @@ impl Arbitrary for OutputInTransactionV4 { /// Creates Strategy for generation VerificationKeyBytes, since the `redjubjub` /// crate does not provide an Arbitrary implementation for it. -fn spendauth_verification_key_bytes( -) -> impl Strategy> { +fn spendauth_verification_key_bytes() -> impl Strategy { prop::array::uniform32(any::()).prop_map(|bytes| { let mut rng = ChaChaRng::from_seed(bytes); let sk = redjubjub::SigningKey::::new(&mut rng); - redjubjub::VerificationKey::::from(&sk).into() + redjubjub::VerificationKey::::from(&sk) + .try_into() + .unwrap() }) } diff --git a/zebra-chain/src/sapling/commitment.rs b/zebra-chain/src/sapling/commitment.rs index 9c232b3bb..1952a5228 100644 --- a/zebra-chain/src/sapling/commitment.rs +++ b/zebra-chain/src/sapling/commitment.rs @@ -5,9 +5,13 @@ mod test_vectors; pub mod pedersen_hashes; -use std::{convert::TryFrom, fmt, io}; +use std::{ + convert::{TryFrom, TryInto}, + fmt, io, +}; use bitvec::prelude::*; +use jubjub::ExtendedPoint; use rand_core::{CryptoRng, RngCore}; use crate::{ @@ -146,12 +150,15 @@ impl NoteCommitment { } } -/// A Homomorphic Pedersen commitment to the value of a note, used in Spend and -/// Output descriptions. +/// A Homomorphic Pedersen commitment to the value of a note. +/// +/// This can be used as an intermediate value in some computations. For the +/// type actually stored in Spend and Output descriptions, see +/// [`NotSmallOrderValueCommitment`]. /// /// https://zips.z.cash/protocol/protocol.pdf#concretehomomorphiccommit #[derive(Clone, Copy, Deserialize, PartialEq, Serialize)] -pub struct ValueCommitment(#[serde(with = "serde_helpers::AffinePoint")] pub jubjub::AffinePoint); +pub struct ValueCommitment(#[serde(with = "serde_helpers::AffinePoint")] jubjub::AffinePoint); impl<'a> std::ops::Add<&'a ValueCommitment> for ValueCommitment { type Output = Self; @@ -186,6 +193,7 @@ impl fmt::Debug for ValueCommitment { } impl From for ValueCommitment { + /// Convert a Jubjub point into a ValueCommitment. fn from(extended_point: jubjub::ExtendedPoint) -> Self { Self(jubjub::AffinePoint::from(extended_point)) } @@ -248,26 +256,14 @@ impl TryFrom<[u8; 32]> for ValueCommitment { let possible_point = jubjub::AffinePoint::from_bytes(bytes); if possible_point.is_some().into() { - Ok(Self(possible_point.unwrap())) + let point = possible_point.unwrap(); + Ok(ExtendedPoint::from(point).into()) } else { Err("Invalid jubjub::AffinePoint value") } } } -impl ZcashSerialize for ValueCommitment { - fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { - writer.write_all(&<[u8; 32]>::from(*self)[..])?; - Ok(()) - } -} - -impl ZcashDeserialize for ValueCommitment { - fn zcash_deserialize(mut reader: R) -> Result { - Self::try_from(reader.read_32_bytes()?).map_err(SerializationError::Parse) - } -} - impl ValueCommitment { /// Generate a new _ValueCommitment_. /// @@ -297,6 +293,80 @@ impl ValueCommitment { } } +/// A Homomorphic Pedersen commitment to the value of a note, used in Spend and +/// Output descriptions. +/// +/// Elements that are of small order are not allowed. This is a separate +/// consensus rule and not intrinsic of value commitments; which is why this +/// type exists. +/// +/// This is denoted by `cv` in the specification. +/// +/// https://zips.z.cash/protocol/protocol.pdf#spenddesc +/// https://zips.z.cash/protocol/protocol.pdf#outputdesc +#[derive(Debug, Clone, Copy, Deserialize, PartialEq, Eq, Serialize)] +pub struct NotSmallOrderValueCommitment(ValueCommitment); + +impl TryFrom for NotSmallOrderValueCommitment { + type Error = &'static str; + + /// Convert a ValueCommitment into a NotSmallOrderValueCommitment. + /// + /// Returns an error if the point is of small order. + /// + /// > cv and rk [MUST NOT be of small order][1], i.e. [h_J]cv MUST NOT be ๐’ช_J + /// > and [h_J]rk MUST NOT be ๐’ช_J. + /// + /// > cv and epk [MUST NOT be of small order][2], i.e. [h_J]cv MUST NOT be ๐’ช_J + /// > and [โ„Ž_J]epk MUST NOT be ๐’ช_J. + /// + /// [1]: https://zips.z.cash/protocol/protocol.pdf#spenddesc + /// [2]: https://zips.z.cash/protocol/protocol.pdf#outputdesc + fn try_from(value_commitment: ValueCommitment) -> Result { + if value_commitment.0.is_small_order().into() { + Err("jubjub::AffinePoint value for Sapling ValueCommitment is of small order") + } else { + Ok(Self(value_commitment)) + } + } +} + +impl TryFrom for NotSmallOrderValueCommitment { + type Error = &'static str; + + /// Convert a Jubjub point into a NotSmallOrderValueCommitment. + fn try_from(extended_point: jubjub::ExtendedPoint) -> Result { + ValueCommitment::from(extended_point).try_into() + } +} + +impl From for ValueCommitment { + fn from(cv: NotSmallOrderValueCommitment) -> Self { + cv.0 + } +} + +impl From for jubjub::AffinePoint { + fn from(cv: NotSmallOrderValueCommitment) -> Self { + cv.0 .0 + } +} + +impl ZcashSerialize for NotSmallOrderValueCommitment { + fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { + writer.write_all(&<[u8; 32]>::from(self.0)[..])?; + Ok(()) + } +} + +impl ZcashDeserialize for NotSmallOrderValueCommitment { + fn zcash_deserialize(mut reader: R) -> Result { + let vc = ValueCommitment::try_from(reader.read_32_bytes()?) + .map_err(SerializationError::Parse)?; + vc.try_into().map_err(SerializationError::Parse) + } +} + #[cfg(test)] mod tests { diff --git a/zebra-chain/src/sapling/keys.rs b/zebra-chain/src/sapling/keys.rs index cb6806f10..e8808f113 100644 --- a/zebra-chain/src/sapling/keys.rs +++ b/zebra-chain/src/sapling/keys.rs @@ -16,7 +16,7 @@ mod test_vectors; mod tests; use std::{ - convert::{From, Into, TryFrom}, + convert::{From, Into, TryFrom, TryInto}, fmt, io::{self, Write}, str::FromStr, @@ -472,9 +472,30 @@ pub struct AuthorizingKey(pub(crate) redjubjub::VerificationKey); impl Eq for AuthorizingKey {} -impl From<[u8; 32]> for AuthorizingKey { - fn from(bytes: [u8; 32]) -> Self { - Self(redjubjub::VerificationKey::try_from(bytes).unwrap()) +impl TryFrom<[u8; 32]> for AuthorizingKey { + type Error = &'static str; + + /// Convert an array into an AuthorizingKey. + /// + /// Returns an error if the encoding is malformed or if [it does not encode + /// a prime-order point][1]: + /// + /// > When decoding this representation, the key MUST be considered invalid + /// > if abst_J returns โŠฅ for either ak or nk, or if ak not in J^{(r)*} + /// + /// [1]: https://zips.z.cash/protocol/protocol.pdf#saplingfullviewingkeyencoding + fn try_from(bytes: [u8; 32]) -> Result { + let affine_point = jubjub::AffinePoint::from_bytes(bytes); + if affine_point.is_none().into() { + return Err("Invalid jubjub::AffinePoint for Sapling AuthorizingKey"); + } + if affine_point.unwrap().is_prime_order().into() { + Ok(Self(redjubjub::VerificationKey::try_from(bytes).map_err( + |_e| "Invalid jubjub::AffinePoint for Sapling AuthorizingKey", + )?)) + } else { + Err("jubjub::AffinePoint value for Sapling AuthorizingKey is not of prime order") + } } } @@ -822,7 +843,13 @@ impl Diversifier { /// Derived by multiplying a JubJub point [derived][ps] from a /// _Diversifier_ by the _IncomingViewingKey_ scalar. /// -/// [ps]: https://zips.z.cash/protocol/protocol.pdf#concretediversifyhash +/// The diversified TransmissionKey is denoted `pk_d` in the specification. +/// Note that it can be the identity point, since its type is +/// [`KA^{Sapling}.PublicPrimeSubgroup`][ka] which in turn is [`J^{(r)}`][jubjub]. +/// +/// [ps]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents +/// [ka]: https://zips.z.cash/protocol/protocol.pdf#concretesaplingkeyagreement +/// [jubjub]: https://zips.z.cash/protocol/protocol.pdf#jubjub #[derive(Copy, Clone, PartialEq)] pub struct TransmissionKey(pub(crate) jubjub::AffinePoint); @@ -837,14 +864,22 @@ impl fmt::Debug for TransmissionKey { impl Eq for TransmissionKey {} -impl From<[u8; 32]> for TransmissionKey { - /// Attempts to interpret a byte representation of an - /// affine point, failing if the element is not on - /// the curve or non-canonical. +impl TryFrom<[u8; 32]> for TransmissionKey { + type Error = &'static str; + + /// Attempts to interpret a byte representation of an affine Jubjub point, failing if the + /// element is not on the curve, non-canonical, or not in the prime-order subgroup. /// /// https://github.com/zkcrypto/jubjub/blob/master/src/lib.rs#L411 - fn from(bytes: [u8; 32]) -> Self { - Self(jubjub::AffinePoint::from_bytes(bytes).unwrap()) + /// https://zips.z.cash/zip-0216 + fn try_from(bytes: [u8; 32]) -> Result { + let affine_point = jubjub::AffinePoint::from_bytes(bytes).unwrap(); + // Check if it's identity or has prime order (i.e. is in the prime-order subgroup). + if affine_point.is_torsion_free().into() { + Ok(Self(affine_point)) + } else { + Err("Invalid jubjub::AffinePoint value for Sapling TransmissionKey") + } } } @@ -854,16 +889,23 @@ impl From for [u8; 32] { } } -impl From<(IncomingViewingKey, Diversifier)> for TransmissionKey { +impl TryFrom<(IncomingViewingKey, Diversifier)> for TransmissionKey { + type Error = &'static str; + /// This includes _KA^Sapling.DerivePublic(ivk, G_d)_, which is just a /// scalar mult _\[ivk\]G_d_. /// /// https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents /// https://zips.z.cash/protocol/protocol.pdf#concretesaplingkeyagreement - fn from((ivk, d): (IncomingViewingKey, Diversifier)) -> Self { - Self(jubjub::AffinePoint::from( - diversify_hash(d.0).unwrap() * ivk.scalar, - )) + fn try_from((ivk, d): (IncomingViewingKey, Diversifier)) -> Result { + let affine_point = jubjub::AffinePoint::from( + diversify_hash(d.0).ok_or("invalid diversifier")? * ivk.scalar, + ); + // We need to ensure that the result point is in the prime-order subgroup. + // Since diversify_hash() returns a point in the prime-order subgroup, + // then the result point will also be in the prime-order subgroup + // and there is no need to check anything. + Ok(Self(affine_point)) } } @@ -946,7 +988,8 @@ impl FromStr for FullViewingKey { fvk_hrp::MAINNET => Network::Mainnet, _ => Network::Testnet, }, - authorizing_key: AuthorizingKey::from(authorizing_key_bytes), + authorizing_key: AuthorizingKey::try_from(authorizing_key_bytes) + .map_err(SerializationError::Parse)?, nullifier_deriving_key: NullifierDerivingKey::from( nullifier_deriving_key_bytes, ), @@ -958,9 +1001,16 @@ impl FromStr for FullViewingKey { } } -/// An ephemeral public key for Sapling key agreement. +/// An [ephemeral public key][1] for Sapling key agreement. /// -/// https://zips.z.cash/protocol/protocol.pdf#concretesaplingkeyagreement +/// Public keys containing points of small order are not allowed. +/// +/// It is denoted by `epk` in the specification. (This type does _not_ +/// represent [KA^{Sapling}.Public][2], which allows any points, including +/// of small order). +/// +/// [1]: https://zips.z.cash/protocol/protocol.pdf#outputdesc +/// [2]: https://zips.z.cash/protocol/protocol.pdf#concretesaplingkeyagreement #[derive(Copy, Clone, Deserialize, PartialEq, Serialize)] pub struct EphemeralPublicKey( #[serde(with = "serde_helpers::AffinePoint")] pub(crate) jubjub::AffinePoint, @@ -998,13 +1048,24 @@ impl PartialEq<[u8; 32]> for EphemeralPublicKey { impl TryFrom<[u8; 32]> for EphemeralPublicKey { type Error = &'static str; + /// Read an EphemeralPublicKey from a byte array. + /// + /// Returns an error if the key is non-canonical, or [it is of small order][1]. + /// + /// > Check that a Output description's cv and epk are not of small order, + /// > i.e. [h_J]cv MUST NOT be ๐’ช_J and [h_J]epk MUST NOT be ๐’ช_J. + /// + /// [1]: https://zips.z.cash/protocol/protocol.pdf#outputdesc fn try_from(bytes: [u8; 32]) -> Result { let possible_point = jubjub::AffinePoint::from_bytes(bytes); - if possible_point.is_some().into() { - Ok(Self(possible_point.unwrap())) + if possible_point.is_none().into() { + return Err("Invalid jubjub::AffinePoint value for Sapling EphemeralPublicKey"); + } + if possible_point.unwrap().is_small_order().into() { + Err("jubjub::AffinePoint value for Sapling EphemeralPublicKey point is of small order") } else { - Err("Invalid jubjub::AffinePoint value") + Ok(Self(possible_point.unwrap())) } } } @@ -1021,3 +1082,60 @@ impl ZcashDeserialize for EphemeralPublicKey { Self::try_from(reader.read_32_bytes()?).map_err(SerializationError::Parse) } } + +/// A randomized [validating key][1] that should be used to validate `spendAuthSig`. +/// +/// It is denoted by `rk` in the specification. (This type does _not_ +/// represent [SpendAuthSig^{Sapling}.Public][2], which allows any points, including +/// of small order). +/// +/// [1]: https://zips.z.cash/protocol/protocol.pdf#spenddesc +/// [2]: https://zips.z.cash/protocol/protocol.pdf#concretereddsa +#[derive(Debug, PartialEq, Serialize, Deserialize, Clone)] +pub struct ValidatingKey(redjubjub::VerificationKey); + +impl TryFrom> for ValidatingKey { + type Error = &'static str; + + /// Convert an array into a ValidatingKey. + /// + /// Returns an error if the key is malformed or [is of small order][1]. + /// + /// > Check that a Spend description's cv and rk are not of small order, + /// > i.e. [h_J]cv MUST NOT be ๐’ช_J and [h_J]rk MUST NOT be ๐’ช_J. + /// + /// [1]: https://zips.z.cash/protocol/protocol.pdf#spenddesc + fn try_from(key: redjubjub::VerificationKey) -> Result { + if bool::from( + jubjub::AffinePoint::from_bytes(key.into()) + .unwrap() + .is_small_order(), + ) { + Err("jubjub::AffinePoint value for Sapling ValidatingKey is of small order") + } else { + Ok(Self(key)) + } + } +} + +impl TryFrom<[u8; 32]> for ValidatingKey { + type Error = &'static str; + + fn try_from(value: [u8; 32]) -> Result { + let vk = redjubjub::VerificationKey::::try_from(value) + .map_err(|_| "Invalid redjubjub::ValidatingKey for Sapling ValidatingKey")?; + vk.try_into() + } +} + +impl From for [u8; 32] { + fn from(key: ValidatingKey) -> Self { + key.0.into() + } +} + +impl From for redjubjub::VerificationKeyBytes { + fn from(key: ValidatingKey) -> Self { + key.0.into() + } +} diff --git a/zebra-chain/src/sapling/keys/tests.rs b/zebra-chain/src/sapling/keys/tests.rs index 8b8f43228..9b4ba3170 100644 --- a/zebra-chain/src/sapling/keys/tests.rs +++ b/zebra-chain/src/sapling/keys/tests.rs @@ -22,7 +22,7 @@ impl Arbitrary for TransmissionKey { let diversifier = Diversifier::from(spending_key); - Self::from((incoming_viewing_key, diversifier)) + Self::try_from((incoming_viewing_key, diversifier)).unwrap() }) .boxed() } @@ -60,7 +60,8 @@ mod tests { let diversifier = Diversifier::from(spending_key); assert_eq!(diversifier, test_vector.default_d); - let transmission_key = TransmissionKey::from((incoming_viewing_key, diversifier)); + let transmission_key = TransmissionKey::try_from((incoming_viewing_key, diversifier)) + .expect("should be a valid transmission key"); assert_eq!(transmission_key, test_vector.default_pk_d); let _full_viewing_key = FullViewingKey { diff --git a/zebra-chain/src/sapling/output.rs b/zebra-chain/src/sapling/output.rs index 44597d99c..2535a3d3a 100644 --- a/zebra-chain/src/sapling/output.rs +++ b/zebra-chain/src/sapling/output.rs @@ -25,7 +25,7 @@ use super::{commitment, keys, note}; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Output { /// A value commitment to the value of the input note. - pub cv: commitment::ValueCommitment, + pub cv: commitment::NotSmallOrderValueCommitment, /// The u-coordinate of the note commitment for the output note. #[serde(with = "serde_helpers::Fq")] pub cm_u: jubjub::Fq, @@ -56,7 +56,7 @@ pub struct OutputInTransactionV4(pub Output); #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct OutputPrefixInTransactionV5 { /// A value commitment to the value of the input note. - pub cv: commitment::ValueCommitment, + pub cv: commitment::NotSmallOrderValueCommitment, /// The u-coordinate of the note commitment for the output note. #[serde(with = "serde_helpers::Fq")] pub cm_u: jubjub::Fq, @@ -135,7 +135,7 @@ impl ZcashSerialize for OutputInTransactionV4 { impl ZcashDeserialize for OutputInTransactionV4 { fn zcash_deserialize(mut reader: R) -> Result { Ok(OutputInTransactionV4(Output { - cv: commitment::ValueCommitment::zcash_deserialize(&mut reader)?, + cv: commitment::NotSmallOrderValueCommitment::zcash_deserialize(&mut reader)?, cm_u: jubjub::Fq::zcash_deserialize(&mut reader)?, ephemeral_key: keys::EphemeralPublicKey::zcash_deserialize(&mut reader)?, enc_ciphertext: note::EncryptedNote::zcash_deserialize(&mut reader)?, @@ -165,7 +165,7 @@ impl ZcashSerialize for OutputPrefixInTransactionV5 { impl ZcashDeserialize for OutputPrefixInTransactionV5 { fn zcash_deserialize(mut reader: R) -> Result { Ok(OutputPrefixInTransactionV5 { - cv: commitment::ValueCommitment::zcash_deserialize(&mut reader)?, + cv: commitment::NotSmallOrderValueCommitment::zcash_deserialize(&mut reader)?, cm_u: jubjub::Fq::zcash_deserialize(&mut reader)?, ephemeral_key: keys::EphemeralPublicKey::zcash_deserialize(&mut reader)?, enc_ciphertext: note::EncryptedNote::zcash_deserialize(&mut reader)?, diff --git a/zebra-chain/src/sapling/shielded_data.rs b/zebra-chain/src/sapling/shielded_data.rs index 6eee9bce8..b35ad0634 100644 --- a/zebra-chain/src/sapling/shielded_data.rs +++ b/zebra-chain/src/sapling/shielded_data.rs @@ -289,10 +289,10 @@ where /// of the value commitments in the Spend descriptions and Output /// descriptions of the transaction, and the balancing value. /// - /// https://zips.z.cash/protocol/protocol.pdf#saplingbalance + /// pub fn binding_verification_key(&self) -> redjubjub::VerificationKeyBytes { - let cv_old: ValueCommitment = self.spends().map(|spend| spend.cv).sum(); - let cv_new: ValueCommitment = self.outputs().map(|output| output.cv).sum(); + let cv_old: ValueCommitment = self.spends().map(|spend| spend.cv.into()).sum(); + let cv_new: ValueCommitment = self.outputs().map(|output| output.cv.into()).sum(); let cv_balance: ValueCommitment = ValueCommitment::new(jubjub::Fr::zero(), self.value_balance); diff --git a/zebra-chain/src/sapling/spend.rs b/zebra-chain/src/sapling/spend.rs index 830f29a09..d0f1e373e 100644 --- a/zebra-chain/src/sapling/spend.rs +++ b/zebra-chain/src/sapling/spend.rs @@ -3,7 +3,7 @@ //! Zebra uses a generic spend type for `V4` and `V5` transactions. //! The anchor change is handled using the `AnchorVariant` type trait. -use std::{fmt, io}; +use std::{convert::TryInto, fmt, io}; use crate::{ block::MAX_BLOCK_BYTES, @@ -17,7 +17,10 @@ use crate::{ }, }; -use super::{commitment, note, tree, AnchorVariant, FieldNotPresent, PerSpendAnchor, SharedAnchor}; +use super::{ + commitment, keys::ValidatingKey, note, tree, AnchorVariant, FieldNotPresent, PerSpendAnchor, + SharedAnchor, +}; /// A _Spend Description_, as described in [protocol specification ยง7.3][ps]. /// @@ -31,10 +34,10 @@ use super::{commitment, note, tree, AnchorVariant, FieldNotPresent, PerSpendAnch /// `V5` transactions split them into multiple arrays. /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#spendencoding -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct Spend { /// A value commitment to the value of the input note. - pub cv: commitment::ValueCommitment, + pub cv: commitment::NotSmallOrderValueCommitment, /// An anchor for this spend. /// /// The anchor is the root of the Sapling note commitment tree in a previous @@ -48,7 +51,7 @@ pub struct Spend { /// The nullifier of the input note. pub nullifier: note::Nullifier, /// The randomized public key for `spend_auth_sig`. - pub rk: redjubjub::VerificationKeyBytes, + pub rk: ValidatingKey, /// The ZK spend proof. pub zkproof: Groth16Proof, /// A signature authorizing this spend. @@ -63,16 +66,24 @@ pub struct Spend { /// Serialized as `SpendDescriptionV5` in [protocol specification ยง7.3][ps]. /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#spendencoding -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct SpendPrefixInTransactionV5 { /// A value commitment to the value of the input note. - pub cv: commitment::ValueCommitment, + pub cv: commitment::NotSmallOrderValueCommitment, /// The nullifier of the input note. pub nullifier: note::Nullifier, /// The randomized public key for `spend_auth_sig`. - pub rk: redjubjub::VerificationKeyBytes, + pub rk: ValidatingKey, } +// We can't derive Eq because `VerificationKey` does not implement it, +// even if it is valid for it. +impl Eq for Spend {} + +// We can't derive Eq because `VerificationKey` does not implement it, +// even if it is valid for it. +impl Eq for SpendPrefixInTransactionV5 {} + impl fmt::Display for Spend where AnchorV: AnchorVariant + Clone, @@ -152,7 +163,7 @@ impl ZcashSerialize for Spend { self.cv.zcash_serialize(&mut writer)?; writer.write_all(&self.per_spend_anchor.0[..])?; writer.write_32_bytes(&self.nullifier.into())?; - writer.write_all(&<[u8; 32]>::from(self.rk)[..])?; + writer.write_all(&<[u8; 32]>::from(self.rk.clone())[..])?; self.zkproof.zcash_serialize(&mut writer)?; writer.write_all(&<[u8; 64]>::from(self.spend_auth_sig)[..])?; Ok(()) @@ -162,10 +173,13 @@ impl ZcashSerialize for Spend { impl ZcashDeserialize for Spend { fn zcash_deserialize(mut reader: R) -> Result { Ok(Spend { - cv: commitment::ValueCommitment::zcash_deserialize(&mut reader)?, + cv: commitment::NotSmallOrderValueCommitment::zcash_deserialize(&mut reader)?, per_spend_anchor: tree::Root(reader.read_32_bytes()?), nullifier: note::Nullifier::from(reader.read_32_bytes()?), - rk: reader.read_32_bytes()?.into(), + rk: reader + .read_32_bytes()? + .try_into() + .map_err(SerializationError::Parse)?, zkproof: Groth16Proof::zcash_deserialize(&mut reader)?, spend_auth_sig: reader.read_64_bytes()?.into(), }) @@ -182,7 +196,7 @@ impl ZcashSerialize for SpendPrefixInTransactionV5 { fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { self.cv.zcash_serialize(&mut writer)?; writer.write_32_bytes(&self.nullifier.into())?; - writer.write_all(&<[u8; 32]>::from(self.rk)[..])?; + writer.write_all(&<[u8; 32]>::from(self.rk.clone())[..])?; Ok(()) } } @@ -190,9 +204,12 @@ impl ZcashSerialize for SpendPrefixInTransactionV5 { impl ZcashDeserialize for SpendPrefixInTransactionV5 { fn zcash_deserialize(mut reader: R) -> Result { Ok(SpendPrefixInTransactionV5 { - cv: commitment::ValueCommitment::zcash_deserialize(&mut reader)?, + cv: commitment::NotSmallOrderValueCommitment::zcash_deserialize(&mut reader)?, nullifier: note::Nullifier::from(reader.read_32_bytes()?), - rk: reader.read_32_bytes()?.into(), + rk: reader + .read_32_bytes()? + .try_into() + .map_err(SerializationError::Parse)?, }) } } diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index 356329fae..f8fb51443 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -9,7 +9,9 @@ use std::{ }; use chrono::{TimeZone, Utc}; -use proptest::{arbitrary::any, array, collection::vec, option, prelude::*}; +use proptest::{ + arbitrary::any, array, collection::vec, option, prelude::*, test_runner::TestRunner, +}; use crate::{ amount::{self, Amount, NegativeAllowed, NonNegative}, @@ -22,7 +24,7 @@ use crate::{ Bctv14Proof, Groth16Proof, Halo2Proof, ZkSnarkProof, }, sapling::{self, AnchorVariant, PerSpendAnchor, SharedAnchor}, - serialization::{ZcashDeserialize, ZcashDeserializeInto}, + serialization::ZcashDeserializeInto, sprout, transparent, value_balance::{ValueBalance, ValueBalanceError}, LedgerState, @@ -975,11 +977,12 @@ pub fn fake_v5_transactions_for_network<'b>( pub fn insert_fake_orchard_shielded_data( transaction: &mut Transaction, ) -> &mut orchard::ShieldedData { - // Create a dummy action, it doesn't need to be valid - let dummy_action_bytes = [0u8; 820]; - let mut dummy_action_bytes_reader = &dummy_action_bytes[..]; - let dummy_action = orchard::Action::zcash_deserialize(&mut dummy_action_bytes_reader) - .expect("Dummy action should deserialize"); + // Create a dummy action + let mut runner = TestRunner::default(); + let dummy_action = orchard::Action::arbitrary() + .new_tree(&mut runner) + .unwrap() + .current(); // Pair the dummy action with a fake signature let dummy_authorized_action = orchard::AuthorizedAction { diff --git a/zebra-consensus/src/primitives/groth16.rs b/zebra-consensus/src/primitives/groth16.rs index 5e7b4fa49..a1fc3ab91 100644 --- a/zebra-consensus/src/primitives/groth16.rs +++ b/zebra-consensus/src/primitives/groth16.rs @@ -162,11 +162,11 @@ impl Description for Spend { fn primary_inputs(&self) -> Vec { let mut inputs = vec![]; - let rk_affine = jubjub::AffinePoint::from_bytes(self.rk.into()).unwrap(); + let rk_affine = jubjub::AffinePoint::from_bytes(self.rk.clone().into()).unwrap(); inputs.push(rk_affine.get_u()); inputs.push(rk_affine.get_v()); - let cv_affine = jubjub::AffinePoint::from_bytes(self.cv.into()).unwrap(); + let cv_affine = jubjub::AffinePoint::from(self.cv); inputs.push(cv_affine.get_u()); inputs.push(cv_affine.get_v()); @@ -197,7 +197,7 @@ impl Description for Output { fn primary_inputs(&self) -> Vec { let mut inputs = vec![]; - let cv_affine = jubjub::AffinePoint::from_bytes(self.cv.into()).unwrap(); + let cv_affine = jubjub::AffinePoint::from(self.cv); inputs.push(cv_affine.get_u()); inputs.push(cv_affine.get_v()); diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index 734521e68..96614bd43 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -694,13 +694,6 @@ where if let Some(sapling_shielded_data) = sapling_shielded_data { for spend in sapling_shielded_data.spends_per_anchor() { - // Consensus rule: cv and rk MUST NOT be of small - // order, i.e. [h_J]cv MUST NOT be ๐’ช_J and [h_J]rk - // MUST NOT be ๐’ช_J. - // - // https://zips.z.cash/protocol/protocol.pdf#spenddesc - check::spend_cv_rk_not_small_order(&spend)?; - // Consensus rule: The proof ฯ€_ZKSpend MUST be valid // given a primary input formed from the other // fields except spendAuthSig. @@ -728,18 +721,11 @@ where async_checks.push( primitives::redjubjub::VERIFIER .clone() - .oneshot((spend.rk, spend.spend_auth_sig, shielded_sighash).into()), + .oneshot((spend.rk.into(), spend.spend_auth_sig, shielded_sighash).into()), ); } for output in sapling_shielded_data.outputs() { - // Consensus rule: cv and wpk MUST NOT be of small - // order, i.e. [h_J]cv MUST NOT be ๐’ช_J and [h_J]wpk - // MUST NOT be ๐’ช_J. - // - // https://zips.z.cash/protocol/protocol.pdf#outputdesc - check::output_cv_epk_not_small_order(output)?; - // Consensus rule: The proof ฯ€_ZKOutput MUST be // valid given a primary input formed from the other // fields except C^enc and C^out. diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index a20ba6b1b..c7d1e7462 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -12,7 +12,6 @@ use zebra_chain::{ orchard::Flags, parameters::{Network, NetworkUpgrade}, primitives::zcash_note_encryption, - sapling::{Output, PerSpendAnchor, Spend}, transaction::{LockTime, Transaction}, }; @@ -128,42 +127,6 @@ pub fn coinbase_tx_no_prevout_joinsplit_spend(tx: &Transaction) -> Result<(), Tr Ok(()) } -/// Check that a Spend description's cv and rk are not of small order, -/// i.e. [h_J]cv MUST NOT be ๐’ช_J and [h_J]rk MUST NOT be ๐’ช_J. -/// -/// https://zips.z.cash/protocol/protocol.pdf#spenddesc -pub fn spend_cv_rk_not_small_order(spend: &Spend) -> Result<(), TransactionError> { - if bool::from(spend.cv.0.is_small_order()) - || bool::from( - jubjub::AffinePoint::from_bytes(spend.rk.into()) - .unwrap() - .is_small_order(), - ) - { - Err(TransactionError::SmallOrder) - } else { - Ok(()) - } -} - -/// Check that a Output description's cv and epk are not of small order, -/// i.e. [h_J]cv MUST NOT be ๐’ช_J and [h_J]epk MUST NOT be ๐’ช_J. -/// -/// https://zips.z.cash/protocol/protocol.pdf#outputdesc -pub fn output_cv_epk_not_small_order(output: &Output) -> Result<(), TransactionError> { - if bool::from(output.cv.0.is_small_order()) - || bool::from( - jubjub::AffinePoint::from_bytes(output.ephemeral_key.into()) - .unwrap() - .is_small_order(), - ) - { - Err(TransactionError::SmallOrder) - } else { - Ok(()) - } -} - /// Check if JoinSplits in the transaction have one of its v_{pub} values equal /// to zero. /// diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 839223cad..f14ebabd0 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -53,18 +53,6 @@ fn v5_fake_transactions() -> Result<(), Report> { // make sure there are no joinsplits nor spends in coinbase check::coinbase_tx_no_prevout_joinsplit_spend(&transaction)?; - - // validate the sapling shielded data - if transaction.version() == 5 { - for spend in transaction.sapling_spends_per_anchor() { - check::spend_cv_rk_not_small_order(&spend)?; - } - for output in transaction.sapling_outputs() { - check::output_cv_epk_not_small_order(output)?; - } - } else { - panic!("we should have no tx other than 5"); - } } }