From abbd43ff57ab16b83cb32535856866d55bb51447 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 23 Aug 2019 23:08:09 +0100 Subject: [PATCH] Make pk_d validity an invariant of PaymentAddress Introduces a PaymentAddress::from_parts constructor, and getters for the diversifier and pk_d fields (which are now private). --- librustzcash/src/tests/key_agreement.rs | 4 +- librustzcash/src/tests/key_components.rs | 2 +- zcash_client_backend/src/encoding.rs | 36 +++++++++------- zcash_primitives/src/note_encryption.rs | 20 ++++----- zcash_primitives/src/primitives.rs | 46 ++++++++++++++++----- zcash_primitives/src/transaction/builder.rs | 29 ++++++------- zcash_primitives/src/zip32.rs | 2 +- zcash_proofs/src/circuit/sapling.rs | 6 +-- zcash_proofs/src/sapling/prover.rs | 2 +- 9 files changed, 85 insertions(+), 62 deletions(-) diff --git a/librustzcash/src/tests/key_agreement.rs b/librustzcash/src/tests/key_agreement.rs index 8f2c273..1250cc7 100644 --- a/librustzcash/src/tests/key_agreement.rs +++ b/librustzcash/src/tests/key_agreement.rs @@ -47,7 +47,7 @@ fn test_key_agreement() { // Serialize pk_d for the call to librustzcash_sapling_ka_agree let mut addr_pk_d = [0u8; 32]; - addr.pk_d.write(&mut addr_pk_d[..]).unwrap(); + addr.pk_d().write(&mut addr_pk_d[..]).unwrap(); assert!(librustzcash_sapling_ka_agree( &addr_pk_d, @@ -59,7 +59,7 @@ fn test_key_agreement() { // using the diversifier and esk. let mut epk = [0u8; 32]; assert!(librustzcash_sapling_ka_derivepublic( - &addr.diversifier.0, + &addr.diversifier().0, &esk, &mut epk )); diff --git a/librustzcash/src/tests/key_components.rs b/librustzcash/src/tests/key_components.rs index a15a40a..5975e5e 100644 --- a/librustzcash/src/tests/key_components.rs +++ b/librustzcash/src/tests/key_components.rs @@ -707,7 +707,7 @@ fn key_components() { let addr = fvk.to_payment_address(diversifier, &JUBJUB).unwrap(); { let mut vec = Vec::new(); - addr.pk_d.write(&mut vec).unwrap(); + addr.pk_d().write(&mut vec).unwrap(); assert_eq!(&vec, &tv.default_pk_d); } { diff --git a/zcash_client_backend/src/encoding.rs b/zcash_client_backend/src/encoding.rs index 4d87587..4e38995 100644 --- a/zcash_client_backend/src/encoding.rs +++ b/zcash_client_backend/src/encoding.rs @@ -110,10 +110,11 @@ pub fn decode_extended_full_viewing_key( /// 0xbc, 0xe5, /// ]); /// -/// let pa = PaymentAddress { -/// diversifier: Diversifier([0u8; 11]), -/// pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), -/// }; +/// let pa = PaymentAddress::from_parts( +/// Diversifier([0u8; 11]), +/// edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), +/// ) +/// .unwrap(); /// /// assert_eq!( /// encode_payment_address(HRP_SAPLING_PAYMENT_ADDRESS, &pa), @@ -147,10 +148,11 @@ pub fn encode_payment_address(hrp: &str, addr: &PaymentAddress) -> String /// 0xbc, 0xe5, /// ]); /// -/// let pa = PaymentAddress { -/// diversifier: Diversifier([0u8; 11]), -/// pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), -/// }; +/// let pa = PaymentAddress::from_parts( +/// Diversifier([0u8; 11]), +/// edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), +/// ) +/// .unwrap(); /// /// assert_eq!( /// decode_payment_address( @@ -193,10 +195,11 @@ mod tests { 0xbc, 0xe5, ]); - let addr = PaymentAddress { - diversifier: Diversifier([0u8; 11]), - pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), - }; + let addr = PaymentAddress::from_parts( + Diversifier([0u8; 11]), + edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), + ) + .unwrap(); let encoded_main = "zs1qqqqqqqqqqqqqqqqqrjq05nyfku05msvu49mawhg6kr0wwljahypwyk2h88z6975u563j8nfaxd"; @@ -237,10 +240,11 @@ mod tests { 0xbc, 0xe5, ]); - let addr = PaymentAddress { - diversifier: Diversifier([1u8; 11]), - pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), - }; + let addr = PaymentAddress::from_parts( + Diversifier([1u8; 11]), + edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), + ) + .unwrap(); let encoded_main = encode_payment_address(constants::mainnet::HRP_SAPLING_PAYMENT_ADDRESS, &addr); diff --git a/zcash_primitives/src/note_encryption.rs b/zcash_primitives/src/note_encryption.rs index 2644016..3b6e83b 100644 --- a/zcash_primitives/src/note_encryption.rs +++ b/zcash_primitives/src/note_encryption.rs @@ -232,10 +232,7 @@ fn prf_ock( /// /// let diversifier = Diversifier([0; 11]); /// let pk_d = diversifier.g_d::(&JUBJUB).unwrap(); -/// let to = PaymentAddress { -/// pk_d, -/// diversifier, -/// }; +/// let to = PaymentAddress::from_parts(diversifier, pk_d).unwrap(); /// let ovk = OutgoingViewingKey([0; 32]); /// /// let value = 1000; @@ -294,14 +291,14 @@ impl SaplingNoteEncryption { /// Generates `encCiphertext` for this note. pub fn encrypt_note_plaintext(&self) -> [u8; ENC_CIPHERTEXT_SIZE] { - let shared_secret = sapling_ka_agree(&self.esk, &self.to.pk_d); + let shared_secret = sapling_ka_agree(&self.esk, self.to.pk_d()); let key = kdf_sapling(shared_secret, &self.epk); // Note plaintext encoding is defined in section 5.5 of the Zcash Protocol // Specification. let mut input = [0; NOTE_PLAINTEXT_SIZE]; input[0] = 1; - input[1..12].copy_from_slice(&self.to.diversifier.0); + input[1..12].copy_from_slice(&self.to.diversifier().0); (&mut input[12..20]) .write_u64::(self.note.value) .unwrap(); @@ -375,7 +372,7 @@ fn parse_note_plaintext_without_memo( .g_d::(&JUBJUB)? .mul(ivk.into_repr(), &JUBJUB); - let to = PaymentAddress { pk_d, diversifier }; + let to = PaymentAddress::from_parts(diversifier, pk_d)?; let note = to.create_note(v, rcm, &JUBJUB).unwrap(); if note.cm(&JUBJUB) != *cmu { @@ -535,7 +532,7 @@ pub fn try_sapling_output_recovery( return None; } - let to = PaymentAddress { pk_d, diversifier }; + let to = PaymentAddress::from_parts(diversifier, pk_d)?; let note = to.create_note(v, rcm, &JUBJUB).unwrap(); if note.cm(&JUBJUB) != *cmu { @@ -701,7 +698,7 @@ mod tests { let diversifier = Diversifier([0; 11]); let ivk = Fs::random(&mut rng); let pk_d = diversifier.g_d::(&JUBJUB).unwrap().mul(ivk, &JUBJUB); - let pa = PaymentAddress { diversifier, pk_d }; + let pa = PaymentAddress::from_parts(diversifier, pk_d).unwrap(); // Construct the value commitment for the proof instance let value = 100; @@ -1317,10 +1314,7 @@ mod tests { let ock = prf_ock(&ovk, &cv, &cmu, &epk); assert_eq!(ock.as_bytes(), tv.ock); - let to = PaymentAddress { - pk_d, - diversifier: Diversifier(tv.default_d), - }; + let to = PaymentAddress::from_parts(Diversifier(tv.default_d), pk_d).unwrap(); let note = to.create_note(tv.v, rcm, &JUBJUB).unwrap(); assert_eq!(note.cm(&JUBJUB), cmu); diff --git a/zcash_primitives/src/primitives.rs b/zcash_primitives/src/primitives.rs index 00ff2bf..4f60c64 100644 --- a/zcash_primitives/src/primitives.rs +++ b/zcash_primitives/src/primitives.rs @@ -94,10 +94,10 @@ impl ViewingKey { diversifier: Diversifier, params: &E::Params, ) -> Option> { - diversifier.g_d(params).map(|g_d| { + diversifier.g_d(params).and_then(|g_d| { let pk_d = g_d.mul(self.ivk(), params); - PaymentAddress { pk_d, diversifier } + PaymentAddress::from_parts(diversifier, pk_d) }) } } @@ -118,10 +118,16 @@ impl Diversifier { } } +/// A Sapling payment address. +/// +/// # Invariants +/// +/// `pk_d` is guaranteed to be prime-order (i.e. in the prime-order subgroup of Jubjub, +/// and not the identity). #[derive(Clone, Debug)] pub struct PaymentAddress { - pub pk_d: edwards::Point, - pub diversifier: Diversifier, + pk_d: edwards::Point, + diversifier: Diversifier, } impl PartialEq for PaymentAddress { @@ -131,6 +137,20 @@ impl PartialEq for PaymentAddress { } impl PaymentAddress { + /// Constructs a PaymentAddress from a diversifier and a Jubjub point. + /// + /// Returns None if `pk_d` is the identity. + pub fn from_parts( + diversifier: Diversifier, + pk_d: edwards::Point, + ) -> Option { + if pk_d == edwards::Point::zero() { + None + } else { + Some(PaymentAddress { pk_d, diversifier }) + } + } + /// Parses a PaymentAddress from bytes. pub fn from_bytes(bytes: &[u8; 43], params: &E::Params) -> Option { let diversifier = { @@ -146,13 +166,7 @@ impl PaymentAddress { edwards::Point::::read(&bytes[11..43], params) .ok()? .as_prime_order(params) - .and_then(|pk_d| { - if pk_d == edwards::Point::zero() { - None - } else { - Some(PaymentAddress { pk_d, diversifier }) - } - }) + .and_then(|pk_d| PaymentAddress::from_parts(diversifier, pk_d)) } /// Returns the byte encoding of this `PaymentAddress`. @@ -163,6 +177,16 @@ impl PaymentAddress { bytes } + /// Returns the [`Diversifier`] for this `PaymentAddress`. + pub fn diversifier(&self) -> &Diversifier { + &self.diversifier + } + + /// Returns `pk_d` for this `PaymentAddress`. + pub fn pk_d(&self) -> &edwards::Point { + &self.pk_d + } + pub fn g_d(&self, params: &E::Params) -> Option> { self.diversifier.g_d(params) } diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 281ccbf..b100d9c 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -77,7 +77,7 @@ impl SaplingOutput { let note = Note { g_d, - pk_d: to.pk_d.clone(), + pk_d: to.pk_d().clone(), value: value.into(), r: rcm, }; @@ -344,10 +344,11 @@ impl Builder { } else if !self.spends.is_empty() { ( self.spends[0].extsk.expsk.ovk, - PaymentAddress { - diversifier: self.spends[0].diversifier, - pk_d: self.spends[0].note.pk_d.clone(), - }, + PaymentAddress::from_parts( + self.spends[0].diversifier, + self.spends[0].note.pk_d.clone(), + ) + .ok_or(Error::InvalidAddress)?, ) } else { return Err(Error::NoChangeAddress); @@ -450,16 +451,16 @@ impl Builder { (diversifier, g_d) }; - let pk_d = { + let (pk_d, payment_address) = loop { let dummy_ivk = Fs::random(&mut self.rng); - g_d.mul(dummy_ivk, &JUBJUB) + let pk_d = g_d.mul(dummy_ivk, &JUBJUB); + if let Some(addr) = PaymentAddress::from_parts(diversifier, pk_d.clone()) { + break (pk_d, addr); + } }; ( - PaymentAddress { - diversifier, - pk_d: pk_d.clone(), - }, + payment_address, Note { g_d, pk_d, @@ -644,7 +645,7 @@ mod tests { builder .add_sapling_spend( extsk.clone(), - to.diversifier, + *to.diversifier(), note1.clone(), witness1.clone(), ) @@ -683,10 +684,10 @@ mod tests { { let mut builder = Builder::new(0); builder - .add_sapling_spend(extsk.clone(), to.diversifier, note1, witness1) + .add_sapling_spend(extsk.clone(), *to.diversifier(), note1, witness1) .unwrap(); builder - .add_sapling_spend(extsk, to.diversifier, note2, witness2) + .add_sapling_spend(extsk, *to.diversifier(), note2, witness2) .unwrap(); builder .add_sapling_output(ovk, to, Amount::from_u64(30000).unwrap(), None) diff --git a/zcash_primitives/src/zip32.rs b/zcash_primitives/src/zip32.rs index e287602..6cbc462 100644 --- a/zcash_primitives/src/zip32.rs +++ b/zcash_primitives/src/zip32.rs @@ -548,7 +548,7 @@ mod tests { let (j_m, addr_m) = xsk_m.default_address().unwrap(); assert_eq!(j_m.0, [0; 11]); assert_eq!( - addr_m.diversifier.0, + addr_m.diversifier().0, // Computed using this Rust implementation [59, 246, 250, 31, 131, 191, 69, 99, 200, 167, 19] ); diff --git a/zcash_proofs/src/circuit/sapling.rs b/zcash_proofs/src/circuit/sapling.rs index de6887b..08e55e6 100644 --- a/zcash_proofs/src/circuit/sapling.rs +++ b/zcash_proofs/src/circuit/sapling.rs @@ -464,7 +464,7 @@ impl<'a, E: JubjubEngine> Circuit for Output<'a, E> { // they would like. { // Just grab pk_d from the witness - let pk_d = self.payment_address.as_ref().map(|e| e.pk_d.to_xy()); + let pk_d = self.payment_address.as_ref().map(|e| e.pk_d().to_xy()); // Witness the y-coordinate, encoded as little // endian bits (to match the representation) @@ -584,7 +584,7 @@ fn test_input_circuit_with_bls12_381() { } } - let g_d = payment_address.diversifier.g_d(params).unwrap(); + let g_d = payment_address.diversifier().g_d(params).unwrap(); let commitment_randomness = fs::Fs::random(rng); let auth_path = vec![Some((Fr::random(rng), rng.next_u32() % 2 != 0)); tree_depth]; let ar = fs::Fs::random(rng); @@ -595,7 +595,7 @@ fn test_input_circuit_with_bls12_381() { let note = Note { value: value_commitment.value, g_d: g_d.clone(), - pk_d: payment_address.pk_d.clone(), + pk_d: payment_address.pk_d().clone(), r: commitment_randomness.clone(), }; diff --git a/zcash_proofs/src/sapling/prover.rs b/zcash_proofs/src/sapling/prover.rs index 0603424..9b0f871 100644 --- a/zcash_proofs/src/sapling/prover.rs +++ b/zcash_proofs/src/sapling/prover.rs @@ -100,7 +100,7 @@ impl SaplingProvingContext { g_d: diversifier .g_d::(params) .expect("was a valid diversifier before"), - pk_d: payment_address.pk_d.clone(), + pk_d: payment_address.pk_d().clone(), r: rcm, };