From 33730a05cb648a07461714503b2be4d63d50a2ec Mon Sep 17 00:00:00 2001 From: Deirdre Connolly Date: Tue, 14 Jul 2020 04:56:53 -0400 Subject: [PATCH] Do not confuse a NoteCommitment for U(NoteCommitment) --- zebra-chain/src/notes.rs | 3 + zebra-chain/src/notes/sapling.rs | 102 ++++++++++++++++-- zebra-chain/src/notes/sprout.rs | 8 +- zebra-chain/src/notes/tests.rs | 1 + zebra-chain/src/notes/tests/arbitrary.rs | 33 ++++++ zebra-chain/src/serde_helpers.rs | 13 +++ zebra-chain/src/transaction/joinsplit.rs | 2 - zebra-chain/src/transaction/serialize.rs | 38 +++---- zebra-chain/src/transaction/shielded_data.rs | 31 +++--- .../src/transaction/tests/arbitrary.rs | 20 ++-- 10 files changed, 189 insertions(+), 62 deletions(-) create mode 100644 zebra-chain/src/notes/tests/arbitrary.rs diff --git a/zebra-chain/src/notes.rs b/zebra-chain/src/notes.rs index cdff1664..0ea10ac9 100644 --- a/zebra-chain/src/notes.rs +++ b/zebra-chain/src/notes.rs @@ -3,6 +3,9 @@ mod memo; pub mod sapling; pub mod sprout; +#[cfg(test)] +mod tests; + /// The randomness used in the Pedersen Hash for note commitment. #[derive(Copy, Clone, Debug, PartialEq)] pub struct NoteCommitmentRandomness(pub [u8; 32]); diff --git a/zebra-chain/src/notes/sapling.rs b/zebra-chain/src/notes/sapling.rs index b0a4ead9..e78bab71 100644 --- a/zebra-chain/src/notes/sapling.rs +++ b/zebra-chain/src/notes/sapling.rs @@ -15,7 +15,7 @@ use crate::{ }; /// A Nullifier for Sapling transactions -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] #[cfg_attr(test, derive(proptest_derive::Arbitrary))] pub struct Nullifier([u8; 32]); @@ -41,7 +41,7 @@ impl ZcashSerialize for Nullifier { /// The randomness used in the Pedersen Hash for note commitment. #[derive(Copy, Clone, Debug, PartialEq)] -pub struct NoteCommitmentRandomness(redjubjub::Randomizer); +pub struct CommitmentRandomness(redjubjub::Randomizer); /// A Note represents that a value is spendable by the recipient who /// holds the spending key corresponding to a given shielded payment @@ -50,7 +50,7 @@ pub struct Note { diversifier: Diversifier, transmission_key: TransmissionKey, value: Amount, - rcm: NoteCommitmentRandomness, + rcm: CommitmentRandomness, } impl Note { @@ -60,23 +60,60 @@ impl Note { /// /// https://zips.z.cash/protocol/protocol.pdf#concretewindowedcommit pub fn commit(&self) -> NoteCommitment { - // Windowed Pedersen Commitment - - // NoteCommitment() unimplemented!() } } /// -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Deserialize, PartialEq, Serialize)] //#[cfg_attr(test, derive(proptest_derive::Arbitrary))] -pub struct NoteCommitment(jubjub::ExtendedPoint); +pub struct NoteCommitment(#[serde(with = "serde_helpers::AffinePoint")] pub jubjub::AffinePoint); + +impl fmt::Debug for NoteCommitment { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("NoteCommitment") + .field("u", &hex::encode(self.0.get_u().to_bytes())) + .field("v", &hex::encode(self.0.get_v().to_bytes())) + .finish() + } +} + +impl From<[u8; 32]> for NoteCommitment { + fn from(bytes: [u8; 32]) -> Self { + Self(jubjub::AffinePoint::from_bytes(bytes).unwrap()) + } +} + +impl Eq for NoteCommitment {} + +impl From for [u8; 32] { + fn from(cm: NoteCommitment) -> [u8; 32] { + cm.0.to_bytes() + } +} + +impl ZcashSerialize for NoteCommitment { + // The u-coordinate of the note commitment, for the output note + // LEBS2OSP256(cm_u) where cm_u = Extract_J(r)(cm). ??? + fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { + writer.write_all(&self.0.to_bytes())?; + Ok(()) + } +} + +impl ZcashDeserialize for NoteCommitment { + fn zcash_deserialize(mut reader: R) -> Result { + Ok(Self( + jubjub::AffinePoint::from_bytes(reader.read_32_bytes()?).unwrap(), + )) + } +} /// The decrypted form of encrypted Sapling notes on the blockchain. pub struct NotePlaintext { diversifier: Diversifier, value: Amount, - rcm: NoteCommitmentRandomness, + rcm: CommitmentRandomness, memo: memo::Memo, } @@ -212,8 +249,51 @@ impl Arbitrary for OutCiphertext { /// Spend and Output Descriptions. /// /// https://zips.z.cash/protocol/protocol.pdf#concretehomomorphiccommit -#[derive(Clone, Copy, Debug)] -pub struct ValueCommitment(pub jubjub::ExtendedPoint); +#[derive(Clone, Deserialize, PartialEq, Serialize)] +//#[cfg_attr(test, derive(proptest_derive::Arbitrary))] +pub struct ValueCommitment(#[serde(with = "serde_helpers::AffinePoint")] pub jubjub::AffinePoint); + +impl fmt::Debug for ValueCommitment { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("ValueCommitment") + .field("u", &hex::encode(self.0.get_u().to_bytes())) + .field("v", &hex::encode(self.0.get_v().to_bytes())) + .finish() + } +} + +impl From<[u8; 32]> for ValueCommitment { + fn from(bytes: [u8; 32]) -> Self { + Self(jubjub::AffinePoint::from_bytes(bytes).unwrap()) + } +} + +impl Eq for ValueCommitment {} + +impl From for [u8; 32] { + fn from(cm: ValueCommitment) -> [u8; 32] { + cm.0.to_bytes() + } +} + +/// LEBS2OSP256(repr_J(cv)) +/// +/// https://zips.z.cash/protocol/protocol.pdf#spendencoding +/// https://zips.z.cash/protocol/protocol.pdf#jubjub +impl ZcashSerialize for ValueCommitment { + fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { + writer.write_all(&self.0.to_bytes())?; + Ok(()) + } +} + +impl ZcashDeserialize for ValueCommitment { + fn zcash_deserialize(mut reader: R) -> Result { + Ok(Self( + jubjub::AffinePoint::from_bytes(reader.read_32_bytes()?).unwrap(), + )) + } +} #[cfg(test)] proptest! { diff --git a/zebra-chain/src/notes/sprout.rs b/zebra-chain/src/notes/sprout.rs index c65bb180..71bc46bb 100644 --- a/zebra-chain/src/notes/sprout.rs +++ b/zebra-chain/src/notes/sprout.rs @@ -96,9 +96,9 @@ impl ZcashSerialize for Nullifier { /// The randomness used in the Pedersen Hash for note commitment. #[derive(Copy, Clone, Debug, PartialEq)] #[cfg_attr(test, derive(proptest_derive::Arbitrary))] -pub struct NoteCommitmentRandomness(pub [u8; 32]); +pub struct CommitmentRandomness(pub [u8; 32]); -impl AsRef<[u8]> for NoteCommitmentRandomness { +impl AsRef<[u8]> for CommitmentRandomness { fn as_ref(&self) -> &[u8] { &self.0 } @@ -125,7 +125,7 @@ pub struct Note { /// Input to PRF^nf to derive the nullifier of the note rho: NullifierSeed, /// A random commitment trapdoor - rcm: NoteCommitmentRandomness, + rcm: CommitmentRandomness, } impl Note { @@ -148,7 +148,7 @@ impl Note { pub struct NotePlaintext { value: Amount, rho: NullifierSeed, - rcm: NoteCommitmentRandomness, + rcm: CommitmentRandomness, memo: Memo, } diff --git a/zebra-chain/src/notes/tests.rs b/zebra-chain/src/notes/tests.rs index e69de29b..be9770e9 100644 --- a/zebra-chain/src/notes/tests.rs +++ b/zebra-chain/src/notes/tests.rs @@ -0,0 +1 @@ +mod arbitrary; diff --git a/zebra-chain/src/notes/tests/arbitrary.rs b/zebra-chain/src/notes/tests/arbitrary.rs new file mode 100644 index 00000000..f3b04daf --- /dev/null +++ b/zebra-chain/src/notes/tests/arbitrary.rs @@ -0,0 +1,33 @@ +use crate::notes::sapling; + +use proptest::{array, prelude::*}; + +impl Arbitrary for sapling::NoteCommitment { + type Parameters = (); + + fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy { + array::uniform32(any::()) + .prop_filter("Valid jubjub::AffinePoint", |b| { + jubjub::AffinePoint::from_bytes(*b).is_some().unwrap_u8() == 1 + }) + .prop_map(Self::from) + .boxed() + } + + type Strategy = BoxedStrategy; +} + +impl Arbitrary for sapling::ValueCommitment { + type Parameters = (); + + fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy { + array::uniform32(any::()) + .prop_filter("Valid jubjub::AffinePoint", |b| { + jubjub::AffinePoint::from_bytes(*b).is_some().unwrap_u8() == 1 + }) + .prop_map(Self::from) + .boxed() + } + + type Strategy = BoxedStrategy; +} diff --git a/zebra-chain/src/serde_helpers.rs b/zebra-chain/src/serde_helpers.rs index e29e475e..e49da915 100644 --- a/zebra-chain/src/serde_helpers.rs +++ b/zebra-chain/src/serde_helpers.rs @@ -23,6 +23,19 @@ impl From for jubjub::AffinePoint { } } +#[derive(Deserialize, Serialize)] +#[serde(remote = "jubjub::Fq")] +pub(crate) struct Fq { + #[serde(getter = "jubjub::Fq::to_bytes")] + bytes: [u8; 32], +} + +impl From for jubjub::Fq { + fn from(local: Fq) -> Self { + jubjub::Fq::from_bytes(&local.bytes).unwrap() + } +} + #[derive(Deserialize, Serialize)] #[serde(remote = "futures::future::Either")] pub(crate) enum Either { diff --git a/zebra-chain/src/transaction/joinsplit.rs b/zebra-chain/src/transaction/joinsplit.rs index d11f447c..385cf3df 100644 --- a/zebra-chain/src/transaction/joinsplit.rs +++ b/zebra-chain/src/transaction/joinsplit.rs @@ -21,8 +21,6 @@ pub struct JoinSplit { /// XXX refine type pub anchor: [u8; 32], /// A nullifier for the input notes. - /// - /// XXX refine type to [T; 2] -- there are two nullifiers pub nullifiers: [crate::notes::sprout::Nullifier; 2], /// A note commitment for this output note. /// diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index 65657a0d..f9c6ffa2 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -8,12 +8,14 @@ use std::{ sync::Arc, }; -use crate::notes; -use crate::proofs::ZkSnarkProof; -use crate::serialization::{ - ReadZcashExt, SerializationError, WriteZcashExt, ZcashDeserialize, ZcashSerialize, +use crate::{ + notes, + proofs::ZkSnarkProof, + serialization::{ + ReadZcashExt, SerializationError, WriteZcashExt, ZcashDeserialize, ZcashSerialize, + }, + types, }; -use crate::types::Script; use super::*; @@ -211,7 +213,7 @@ impl ZcashDeserialize for TransparentInput { hash: TransactionHash(bytes), index: reader.read_u32::()?, }, - unlock_script: Script::zcash_deserialize(&mut reader)?, + unlock_script: types::Script::zcash_deserialize(&mut reader)?, sequence: reader.read_u32::()?, }) } @@ -230,7 +232,7 @@ impl ZcashDeserialize for TransparentOutput { fn zcash_deserialize(mut reader: R) -> Result { Ok(TransparentOutput { value: reader.read_u64::()?.try_into()?, - lock_script: Script::zcash_deserialize(&mut reader)?, + lock_script: tyoes::Script::zcash_deserialize(&mut reader)?, }) } } @@ -262,15 +264,15 @@ impl ZcashDeserialize for JoinSplit

{ vpub_new: reader.read_u64::()?.try_into()?, anchor: reader.read_32_bytes()?, nullifiers: [ - crate::notes::sprout::Nullifier::zcash_deserialize(&mut reader)?, - crate::notes::sprout::Nullifier::zcash_deserialize(&mut reader)?, + notes::sprout::Nullifier::zcash_deserialize(&mut reader)?, + notes::sprout::Nullifier::zcash_deserialize(&mut reader)?, ], commitments: [reader.read_32_bytes()?, reader.read_32_bytes()?], ephemeral_key: x25519_dalek::PublicKey::from(reader.read_32_bytes()?), random_seed: reader.read_32_bytes()?, vmacs: [ - crate::types::MAC::zcash_deserialize(&mut reader)?, - crate::types::MAC::zcash_deserialize(&mut reader)?, + types::MAC::zcash_deserialize(&mut reader)?, + types::MAC::zcash_deserialize(&mut reader)?, ], zkproof: P::zcash_deserialize(&mut reader)?, enc_ciphertexts: [ @@ -319,7 +321,7 @@ impl ZcashDeserialize for Option> { impl ZcashSerialize for Spend { fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { - writer.write_all(&self.cv[..])?; + self.cv.zcash_serialize(&mut writer)?; writer.write_all(&self.anchor.0[..])?; self.nullifier.zcash_serialize(&mut writer)?; writer.write_all(&<[u8; 32]>::from(self.rk)[..])?; @@ -333,9 +335,9 @@ impl ZcashDeserialize for Spend { fn zcash_deserialize(mut reader: R) -> Result { use crate::treestate::note_commitment_tree::SaplingNoteTreeRootHash; Ok(Spend { - cv: reader.read_32_bytes()?, + cv: notes::sapling::ValueCommitment::zcash_deserialize(&mut reader)?, anchor: SaplingNoteTreeRootHash(reader.read_32_bytes()?), - nullifier: crate::notes::sapling::Nullifier::zcash_deserialize(&mut reader)?, + nullifier: notes::sapling::Nullifier::zcash_deserialize(&mut reader)?, rk: reader.read_32_bytes()?.into(), zkproof: Groth16Proof::zcash_deserialize(&mut reader)?, spend_auth_sig: reader.read_64_bytes()?.into(), @@ -345,8 +347,8 @@ impl ZcashDeserialize for Spend { impl ZcashSerialize for Output { fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { - writer.write_all(&self.cv[..])?; - writer.write_all(&self.cmu[..])?; + self.cv.zcash_serialize(&mut writer)?; + writer.write_all(&self.cm_u.to_bytes())?; writer.write_all(&self.ephemeral_key.to_bytes())?; self.enc_ciphertext.zcash_serialize(&mut writer)?; self.out_ciphertext.zcash_serialize(&mut writer)?; @@ -358,8 +360,8 @@ impl ZcashSerialize for Output { impl ZcashDeserialize for Output { fn zcash_deserialize(mut reader: R) -> Result { Ok(Output { - cv: reader.read_32_bytes()?, - cmu: reader.read_32_bytes()?, + cv: notes::sapling::ValueCommitment::zcash_deserialize(&mut reader)?, + cm_u: jubjub::Fq::from_bytes(&reader.read_32_bytes()?).unwrap(), ephemeral_key: jubjub::AffinePoint::from_bytes(reader.read_32_bytes()?).unwrap(), enc_ciphertext: notes::sapling::EncryptedCiphertext::zcash_deserialize(&mut reader)?, out_ciphertext: notes::sapling::OutCiphertext::zcash_deserialize(&mut reader)?, diff --git a/zebra-chain/src/transaction/shielded_data.rs b/zebra-chain/src/transaction/shielded_data.rs index 38c5a97c..b4c3fbc2 100644 --- a/zebra-chain/src/transaction/shielded_data.rs +++ b/zebra-chain/src/transaction/shielded_data.rs @@ -1,8 +1,10 @@ -use crate::notes::sapling; -use crate::proofs::Groth16Proof; -use crate::redjubjub::{self, Binding, SpendAuth}; -use crate::serde_helpers; -use crate::treestate::note_commitment_tree::SaplingNoteTreeRootHash; +use crate::{ + notes, + proofs::Groth16Proof, + redjubjub::{self, Binding, SpendAuth}, + serde_helpers, + treestate::note_commitment_tree::SaplingNoteTreeRootHash, +}; use futures::future::Either; /// A _Spend Description_, as described in [protocol specification ยง7.3][ps]. @@ -11,13 +13,11 @@ use futures::future::Either; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Spend { /// A value commitment to the value of the input note. - /// - /// XXX refine to a specific type. - pub cv: [u8; 32], + pub cv: notes::sapling::ValueCommitment, /// A root of the Sapling note commitment tree at some block height in the past. pub anchor: SaplingNoteTreeRootHash, /// The nullifier of the input note. - pub nullifier: crate::notes::sapling::Nullifier, + pub nullifier: notes::sapling::Nullifier, /// The randomized public key for `spend_auth_sig`. pub rk: redjubjub::VerificationKeyBytes, /// The ZK spend proof. @@ -32,20 +32,17 @@ pub struct Spend { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Output { /// A value commitment to the value of the input note. - /// - /// XXX refine to a specific type. - pub cv: [u8; 32], + pub cv: notes::sapling::ValueCommitment, /// The u-coordinate of the note commitment for the output note. - /// - /// XXX refine to a specific type. - pub cmu: [u8; 32], + #[serde(with = "serde_helpers::Fq")] + pub cm_u: jubjub::Fq, /// An encoding of an ephemeral Jubjub public key. #[serde(with = "serde_helpers::AffinePoint")] pub ephemeral_key: jubjub::AffinePoint, /// A ciphertext component for the encrypted output note. - pub enc_ciphertext: sapling::EncryptedCiphertext, + pub enc_ciphertext: notes::sapling::EncryptedCiphertext, /// A ciphertext component for the encrypted output note. - pub out_ciphertext: sapling::OutCiphertext, + pub out_ciphertext: notes::sapling::OutCiphertext, /// The ZK output proof. pub zkproof: Groth16Proof, } diff --git a/zebra-chain/src/transaction/tests/arbitrary.rs b/zebra-chain/src/transaction/tests/arbitrary.rs index 858c01d6..e4aeef75 100644 --- a/zebra-chain/src/transaction/tests/arbitrary.rs +++ b/zebra-chain/src/transaction/tests/arbitrary.rs @@ -22,7 +22,7 @@ impl Arbitrary for JoinSplit

{ any::>(), any::>(), array::uniform32(any::()), - array::uniform2(any::()), + array::uniform2(any::()), array::uniform2(array::uniform32(any::())), array::uniform32(any::()), array::uniform32(any::()), @@ -94,8 +94,8 @@ impl Arbitrary for Output { fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy { ( - array::uniform32(any::()), - array::uniform32(any::()), + any::(), + any::(), array::uniform32(any::()).prop_filter("Valid jubjub::AffinePoint", |b| { jubjub::AffinePoint::from_bytes(*b).is_some().unwrap_u8() == 1 }), @@ -104,9 +104,9 @@ impl Arbitrary for Output { any::(), ) .prop_map( - |(cv, cmu, ephemeral_key_bytes, enc_ciphertext, out_ciphertext, zkproof)| Self { + |(cv, cm, ephemeral_key_bytes, enc_ciphertext, out_ciphertext, zkproof)| Self { cv, - cmu, + cm_u: cm.0.get_u(), ephemeral_key: jubjub::AffinePoint::from_bytes(ephemeral_key_bytes).unwrap(), enc_ciphertext, out_ciphertext, @@ -153,18 +153,18 @@ impl Arbitrary for Spend { fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy { ( - array::uniform32(any::()), any::(), - any::(), + any::(), + any::(), array::uniform32(any::()), any::(), vec(any::(), 64), ) .prop_map( - |(cv_bytes, anchor, nullifier_bytes, rpk_bytes, proof, sig_bytes)| Self { + |(anchor, cv, nullifier, rpk_bytes, proof, sig_bytes)| Self { anchor, - cv: cv_bytes, - nullifier: nullifier_bytes, + cv, + nullifier, rk: redjubjub::VerificationKeyBytes::from(rpk_bytes), zkproof: proof, spend_auth_sig: redjubjub::Signature::from({