From 8cf7a6872ca4f4e4d9a5df1333cab4f62ab8be1f Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Thu, 22 Jul 2021 22:14:34 +0800 Subject: [PATCH] Minor refactors, text fixes, and docfixes. Co-authored-by: Jack Grigg Co-authored-by: Daira Hopwood --- Cargo.toml | 1 - src/builder.rs | 8 +++-- src/circuit.rs | 34 +++++++++---------- src/circuit/gadget/ecc/chip.rs | 2 +- .../gadget/ecc/chip/mul_fixed/short.rs | 2 +- src/circuit/gadget/poseidon.rs | 19 ++++++++++- src/circuit/gadget/sinsemilla.rs | 15 ++++---- src/circuit/gadget/sinsemilla/chip.rs | 7 +++- .../gadget/sinsemilla/chip/hash_to_point.rs | 2 +- src/circuit/gadget/sinsemilla/commit_ivk.rs | 8 ----- .../gadget/utilities/lookup_range_check.rs | 26 ++++++-------- src/keys.rs | 24 +++++-------- src/note/commitment.rs | 24 +++++-------- src/note/nullifier.rs | 8 ----- src/tree.rs | 23 +++++++------ src/value.rs | 12 +++---- 16 files changed, 101 insertions(+), 114 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2662fca4..b6f66efc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,4 +1,3 @@ - [package] name = "orchard" version = "0.0.0" diff --git a/src/builder.rs b/src/builder.rs index 7ffb8ffc..31bade20 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -135,6 +135,8 @@ impl ActionInfo { let nf_old = self.spend.note.nullifier(&self.spend.fvk); let sender_address = self.spend.fvk.default_address(); let rho_old = self.spend.note.rho(); + let psi_old = self.spend.note.rseed().psi(&rho_old); + let rcm_old = self.spend.note.rseed().rcm(&rho_old); let ak: SpendValidatingKey = self.spend.fvk.clone().into(); let alpha = pallas::Scalar::random(&mut rng); let rk = ak.randomize(&alpha); @@ -181,9 +183,9 @@ impl ActionInfo { g_d_old: Some(sender_address.g_d()), pk_d_old: Some(*sender_address.pk_d()), v_old: Some(self.spend.note.value()), - rho_old: Some(self.spend.note.rho()), - psi_old: Some(self.spend.note.rseed().psi(&rho_old)), - rcm_old: Some(self.spend.note.rseed().rcm(&rho_old)), + rho_old: Some(rho_old), + psi_old: Some(psi_old), + rcm_old: Some(rcm_old), cm_old: Some(self.spend.note.commitment()), alpha: Some(alpha), ak: Some(ak), diff --git a/src/circuit.rs b/src/circuit.rs index c84ac111..8ad2b02f 100644 --- a/src/circuit.rs +++ b/src/circuit.rs @@ -71,9 +71,9 @@ const K: u32 = 11; // Absolute offsets for public inputs. const ANCHOR: usize = 0; -const NF_OLD: usize = 1; -const CV_NET_X: usize = 2; -const CV_NET_Y: usize = 3; +const CV_NET_X: usize = 1; +const CV_NET_Y: usize = 2; +const NF_OLD: usize = 3; const RK_X: usize = 4; const RK_Y: usize = 5; const CMX: usize = 6; @@ -339,7 +339,7 @@ impl plonk::Circuit for Circuit { let cm_old = Point::new( config.ecc_chip(), layouter.namespace(|| "cm_old"), - self.cm_old.as_ref().map(|cm| cm.to_affine()), + self.cm_old.as_ref().map(|cm| cm.inner().to_affine()), )?; // Witness g_d_old @@ -361,7 +361,7 @@ impl plonk::Circuit for Circuit { let nk = self.load_private( layouter.namespace(|| "witness nk"), config.advices[0], - self.nk.map(|nk| *nk), + self.nk.map(|nk| nk.inner()), )?; // Witness v_old. @@ -446,7 +446,7 @@ impl plonk::Circuit for Circuit { // blind = [rcv] ValueCommitR let (blind, _rcv) = { - let rcv = self.rcv.as_ref().map(|rcv| **rcv); + let rcv = self.rcv.as_ref().map(|rcv| rcv.inner()); let value_commit_r = OrchardFixedBasesFull::ValueCommitR; let value_commit_r = FixedPoint::from_inner(ecc_chip.clone(), value_commit_r); @@ -482,9 +482,9 @@ impl plonk::Circuit for Circuit { || value.ok_or(plonk::Error::SynthesisError), )?; region.constrain_equal(var, message[i].cell())?; - Ok(Word::<_, _, poseidon::OrchardNullifier, 3, 2> { - inner: StateWord::new(var, value), - }) + Ok(Word::<_, _, poseidon::OrchardNullifier, 3, 2>::from_inner( + StateWord::new(var, value), + )) }; Ok([message_word(0)?, message_word(1)?]) @@ -500,7 +500,7 @@ impl plonk::Circuit for Circuit { layouter.namespace(|| "Poseidon hash (nk, rho_old)"), poseidon_message, )?; - let poseidon_output: CellValue = poseidon_output.inner.into(); + let poseidon_output: CellValue = poseidon_output.inner().into(); poseidon_output }; @@ -576,7 +576,7 @@ impl plonk::Circuit for Circuit { let commit_ivk_config = config.commit_ivk_config.clone(); let ivk = { - let rivk = self.rivk.map(|rivk| *rivk); + let rivk = self.rivk.map(|rivk| rivk.inner()); commit_ivk_config.assign_region( config.sinsemilla_chip_1(), @@ -599,7 +599,7 @@ impl plonk::Circuit for Circuit { let pk_d_old = Point::new( ecc_chip.clone(), layouter.namespace(|| "witness pk_d_old"), - self.pk_d_old.map(|pk_d_old| (*pk_d_old).to_affine()), + self.pk_d_old.map(|pk_d_old| pk_d_old.inner().to_affine()), )?; derived_pk_d_old .constrain_equal(layouter.namespace(|| "pk_d_old equality"), &pk_d_old)?; @@ -611,7 +611,7 @@ impl plonk::Circuit for Circuit { { let old_note_commit_config = config.old_note_commit_config.clone(); - let rcm_old = self.rcm_old.as_ref().map(|rcm_old| **rcm_old); + let rcm_old = self.rcm_old.as_ref().map(|rcm_old| rcm_old.inner()); // g★_d || pk★_d || i2lebsp_{64}(v) || i2lebsp_{255}(rho) || i2lebsp_{255}(psi) let derived_cm_old = old_note_commit_config.assign_region( @@ -667,7 +667,7 @@ impl plonk::Circuit for Circuit { self.psi_new, )?; - let rcm_new = self.rcm_new.as_ref().map(|rcm_new| **rcm_new); + let rcm_new = self.rcm_new.as_ref().map(|rcm_new| rcm_new.inner()); // g★_d || pk★_d || i2lebsp_{64}(v) || i2lebsp_{255}(rho) || i2lebsp_{255}(psi) let cm_new = new_note_commit_config.assign_region( @@ -795,10 +795,10 @@ impl Instance { fn to_halo2_instance(&self) -> [[vesta::Scalar; 9]; 1] { let mut instance = [vesta::Scalar::zero(); 9]; - instance[ANCHOR] = *self.anchor; + instance[ANCHOR] = self.anchor.inner(); instance[CV_NET_X] = self.cv_net.x(); instance[CV_NET_Y] = self.cv_net.y(); - instance[NF_OLD] = *self.nf_old; + instance[NF_OLD] = self.nf_old.0; let rk = pallas::Point::from_bytes(&self.rk.clone().into()) .unwrap() @@ -808,7 +808,7 @@ impl Instance { instance[RK_X] = *rk.x(); instance[RK_Y] = *rk.y(); - instance[CMX] = *self.cmx; + instance[CMX] = self.cmx.inner(); instance[ENABLE_SPEND] = vesta::Scalar::from_u64(self.enable_spend.into()); instance[ENABLE_OUTPUT] = vesta::Scalar::from_u64(self.enable_output.into()); diff --git a/src/circuit/gadget/ecc/chip.rs b/src/circuit/gadget/ecc/chip.rs index c96dc16e..c59dda32 100644 --- a/src/circuit/gadget/ecc/chip.rs +++ b/src/circuit/gadget/ecc/chip.rs @@ -147,7 +147,7 @@ impl EccChip { /// # Side effects /// - /// All columns in `advices` and `constants` will be equality-enabled. + /// All columns in `advices` will be equality-enabled. #[allow(non_snake_case)] pub fn configure( meta: &mut ConstraintSystem, diff --git a/src/circuit/gadget/ecc/chip/mul_fixed/short.rs b/src/circuit/gadget/ecc/chip/mul_fixed/short.rs index 4c663ac7..c4a3c529 100644 --- a/src/circuit/gadget/ecc/chip/mul_fixed/short.rs +++ b/src/circuit/gadget/ecc/chip/mul_fixed/short.rs @@ -499,7 +499,7 @@ pub mod tests { row: 26 }, VerifyFailure::Permutation { - column: (Any::Fixed, 1).into(), + column: (Any::Fixed, 9).into(), row: 2 }, VerifyFailure::Permutation { diff --git a/src/circuit/gadget/poseidon.rs b/src/circuit/gadget/poseidon.rs index c9a01444..0c77f2b1 100644 --- a/src/circuit/gadget/poseidon.rs +++ b/src/circuit/gadget/poseidon.rs @@ -67,7 +67,24 @@ pub struct Word< const T: usize, const RATE: usize, > { - pub inner: PoseidonChip::Word, + inner: PoseidonChip::Word, +} + +impl< + F: FieldExt, + PoseidonChip: PoseidonInstructions, + S: Spec, + const T: usize, + const RATE: usize, + > Word +{ + pub(crate) fn inner(&self) -> PoseidonChip::Word { + self.inner + } + + pub(crate) fn from_inner(inner: PoseidonChip::Word) -> Self { + Self { inner } + } } fn poseidon_duplex< diff --git a/src/circuit/gadget/sinsemilla.rs b/src/circuit/gadget/sinsemilla.rs index 743163df..ddff43fe 100644 --- a/src/circuit/gadget/sinsemilla.rs +++ b/src/circuit/gadget/sinsemilla.rs @@ -285,24 +285,23 @@ where message: Message, ) -> Result<(ecc::Point, Vec), Error> { assert_eq!(self.sinsemilla_chip, message.chip); - let (p, zs) = self - .sinsemilla_chip - .hash_to_point(layouter, self.Q, message.inner)?; - let p = ecc::Point::from_inner(self.ecc_chip.clone(), p); - Ok((p, zs)) + self.sinsemilla_chip + .hash_to_point(layouter, self.Q, message.inner) + .map(|(point, zs)| (ecc::Point::from_inner(self.ecc_chip.clone(), point), zs)) } /// $\mathsf{SinsemillaHash}$ from [§ 5.4.1.9][concretesinsemillahash]. /// /// [concretesinsemillahash]: https://zips.z.cash/protocol/protocol.pdf#concretesinsemillahash + #[allow(clippy::type_complexity)] pub fn hash( &self, layouter: impl Layouter, message: Message, - ) -> Result, Error> { + ) -> Result<(ecc::X, Vec), Error> { assert_eq!(self.sinsemilla_chip, message.chip); - let (p, _) = self.hash_to_point(layouter, message)?; - Ok(p.extract_p()) + let (p, zs) = self.hash_to_point(layouter, message)?; + Ok((p.extract_p(), zs)) } } diff --git a/src/circuit/gadget/sinsemilla/chip.rs b/src/circuit/gadget/sinsemilla/chip.rs index 0521fa47..fed40eb5 100644 --- a/src/circuit/gadget/sinsemilla/chip.rs +++ b/src/circuit/gadget/sinsemilla/chip.rs @@ -103,7 +103,7 @@ impl SinsemillaChip { /// # Side-effects /// - /// All columns in `advices` and `constants` will be equality-enabled. + /// All columns in `advices` and will be equality-enabled. #[allow(clippy::too_many_arguments)] #[allow(non_snake_case)] pub fn configure( @@ -113,6 +113,11 @@ impl SinsemillaChip { lookup: (Column, Column, Column), range_check: LookupRangeCheckConfig, ) -> >::Config { + // Enable equality on all advice columns + for advice in advices.iter() { + meta.enable_equality((*advice).into()) + } + let config = SinsemillaConfig { q_sinsemilla1: meta.selector(), q_sinsemilla2: meta.fixed_column(), diff --git a/src/circuit/gadget/sinsemilla/chip/hash_to_point.rs b/src/circuit/gadget/sinsemilla/chip/hash_to_point.rs index 72670ecf..1e6c7f79 100644 --- a/src/circuit/gadget/sinsemilla/chip/hash_to_point.rs +++ b/src/circuit/gadget/sinsemilla/chip/hash_to_point.rs @@ -40,7 +40,7 @@ impl SinsemillaChip { // Constrain the initial x_q to equal the x-coordinate of the domain's `Q`. let x_a = { let cell = - region.assign_advice_from_constant(|| "public x_q", config.x_a, offset, x_q)?; + region.assign_advice_from_constant(|| "fixed x_q", config.x_a, offset, x_q)?; CellValue::new(cell, Some(x_q)) }; diff --git a/src/circuit/gadget/sinsemilla/commit_ivk.rs b/src/circuit/gadget/sinsemilla/commit_ivk.rs index 82b9ea71..d460a152 100644 --- a/src/circuit/gadget/sinsemilla/commit_ivk.rs +++ b/src/circuit/gadget/sinsemilla/commit_ivk.rs @@ -18,14 +18,6 @@ use super::{ CommitDomain, Message, MessagePiece, }; -// -// We need to hash `ak || nk` where each of `ak`, `nk` is a field element (255 bits). -// -// a = bits 0..=249 of `ak` -// b = b_0||b_1||b_2` = (bits 250..=253 of `ak`) || (bit 254 of `ak`) || (bits 0..=4 of `nk`) -// c = bits 5..=244 of `nk` -// d = d_0||d_1` = (bits 245..=253 of `nk`) || (bit 254 of `nk`) - #[derive(Clone, Debug)] pub struct CommitIvkConfig { q_canon: Selector, diff --git a/src/circuit/gadget/utilities/lookup_range_check.rs b/src/circuit/gadget/utilities/lookup_range_check.rs index 3546ee78..9915bca6 100644 --- a/src/circuit/gadget/utilities/lookup_range_check.rs +++ b/src/circuit/gadget/utilities/lookup_range_check.rs @@ -131,12 +131,7 @@ impl LookupRangeCheckConfig // Copy `element` and initialize running sum `z_0 = element` to decompose it. let z_0 = copy(&mut region, || "z_0", self.running_sum, 0, &element)?; - let zs = self.range_check(&mut region, z_0, num_words)?; - - if strict { - // Constrain the final `z` to be zero. - region.constrain_constant(zs.last().unwrap().cell(), F::zero())?; - } + let zs = self.range_check(&mut region, z_0, num_words, strict)?; Ok(zs) }, @@ -151,7 +146,7 @@ impl LookupRangeCheckConfig num_words: usize, strict: bool, ) -> Result<(CellValue, Vec>), Error> { - let (z_0, zs) = layouter.assign_region( + layouter.assign_region( || "Witness element", |mut region| { let z_0 = { @@ -164,18 +159,11 @@ impl LookupRangeCheckConfig CellValue::new(cell, value) }; - let zs = self.range_check(&mut region, z_0, num_words)?; - - if strict { - // Constrain the final `z` to be zero. - region.constrain_constant(zs.last().unwrap().cell(), F::zero())?; - } + let zs = self.range_check(&mut region, z_0, num_words, strict)?; Ok((z_0, zs)) }, - )?; - - Ok((z_0, zs)) + ) } /// If `strict` is set to "true", the field element must fit into @@ -190,6 +178,7 @@ impl LookupRangeCheckConfig region: &mut Region<'_, F>, element: CellValue, num_words: usize, + strict: bool, ) -> Result>, Error> { // `num_words` must fit into a single field element. assert!(num_words * K <= F::CAPACITY as usize); @@ -252,6 +241,11 @@ impl LookupRangeCheckConfig zs.push(z); } + if strict { + // Constrain the final `z` to be zero. + region.constrain_constant(zs.last().unwrap().cell(), F::zero())?; + } + Ok(zs) } diff --git a/src/keys.rs b/src/keys.rs index abcf0091..5a229a80 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -150,11 +150,9 @@ impl SpendValidatingKey { #[derive(Copy, Debug, Clone)] pub(crate) struct NullifierDerivingKey(pallas::Base); -impl std::ops::Deref for NullifierDerivingKey { - type Target = pallas::Base; - - fn deref(&self) -> &pallas::Base { - &self.0 +impl NullifierDerivingKey { + pub(crate) fn inner(&self) -> pallas::Base { + self.0 } } @@ -184,11 +182,9 @@ impl From<&SpendingKey> for CommitIvkRandomness { } } -impl std::ops::Deref for CommitIvkRandomness { - type Target = pallas::Scalar; - - fn deref(&self) -> &pallas::Scalar { - &self.0 +impl CommitIvkRandomness { + pub(crate) fn inner(&self) -> pallas::Scalar { + self.0 } } @@ -466,11 +462,9 @@ impl AsRef<[u8; 32]> for OutgoingViewingKey { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct DiversifiedTransmissionKey(NonIdentityPallasPoint); -impl std::ops::Deref for DiversifiedTransmissionKey { - type Target = pallas::Point; - - fn deref(&self) -> &pallas::Point { - &(*self.0) +impl DiversifiedTransmissionKey { + pub(crate) fn inner(&self) -> NonIdentityPallasPoint { + self.0 } } diff --git a/src/note/commitment.rs b/src/note/commitment.rs index 0cd671b8..96532434 100644 --- a/src/note/commitment.rs +++ b/src/note/commitment.rs @@ -10,11 +10,9 @@ use crate::{constants::L_ORCHARD_BASE, primitives::sinsemilla, spec::extract_p, #[derive(Debug)] pub(crate) struct NoteCommitTrapdoor(pub(super) pallas::Scalar); -impl std::ops::Deref for NoteCommitTrapdoor { - type Target = pallas::Scalar; - - fn deref(&self) -> &pallas::Scalar { - &self.0 +impl NoteCommitTrapdoor { + pub(crate) fn inner(&self) -> pallas::Scalar { + self.0 } } @@ -22,11 +20,9 @@ impl std::ops::Deref for NoteCommitTrapdoor { #[derive(Clone, Debug)] pub struct NoteCommitment(pub(super) pallas::Point); -impl std::ops::Deref for NoteCommitment { - type Target = pallas::Point; - - fn deref(&self) -> &pallas::Point { - &self.0 +impl NoteCommitment { + pub(crate) fn inner(&self) -> pallas::Point { + self.0 } } @@ -81,11 +77,9 @@ impl From for ExtractedNoteCommitment { } } -impl std::ops::Deref for ExtractedNoteCommitment { - type Target = pallas::Base; - - fn deref(&self) -> &pallas::Base { - &self.0 +impl ExtractedNoteCommitment { + pub(crate) fn inner(&self) -> pallas::Base { + self.0 } } diff --git a/src/note/nullifier.rs b/src/note/nullifier.rs index 0f98b850..75d2c40b 100644 --- a/src/note/nullifier.rs +++ b/src/note/nullifier.rs @@ -14,14 +14,6 @@ use crate::{ #[derive(Clone, Copy, Debug)] pub struct Nullifier(pub(crate) pallas::Base); -impl std::ops::Deref for Nullifier { - type Target = pallas::Base; - - fn deref(&self) -> &pallas::Base { - &self.0 - } -} - impl Nullifier { /// Generates a dummy nullifier for use as $\rho$ in dummy spent notes. /// diff --git a/src/tree.rs b/src/tree.rs index 4a38e0f2..7e4676bd 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -53,11 +53,9 @@ impl From for Anchor { } } -impl std::ops::Deref for Anchor { - type Target = pallas::Base; - - fn deref(&self) -> &pallas::Base { - &self.0 +impl Anchor { + pub(crate) fn inner(&self) -> pallas::Base { + self.0 } } @@ -102,10 +100,13 @@ impl MerklePath { self.auth_path .iter() .enumerate() - .fold(CtOption::new(*cmx, 1.into()), |node, (l, sibling)| { - let swap = self.position & (1 << l) != 0; - node.and_then(|n| hash_with_l(l, cond_swap(swap, n, *sibling))) - }) + .fold( + CtOption::new(cmx.inner(), 1.into()), + |node, (l, sibling)| { + let swap = self.position & (1 << l) != 0; + node.and_then(|n| hash_with_l(l, cond_swap(swap, n, *sibling))) + }, + ) .map(Anchor) } @@ -187,7 +188,7 @@ impl MerkleCrhOrchardOutput { /// Creates an incremental tree leaf digest from the specified /// Orchard extracted note commitment. pub fn from_cmx(value: &ExtractedNoteCommitment) -> Self { - MerkleCrhOrchardOutput(**value) + MerkleCrhOrchardOutput(value.inner()) } /// Convert this digest to its canonical byte representation. @@ -339,7 +340,7 @@ pub mod testing { let perfect_subtree = { let mut perfect_subtree: Vec>> = vec![ - padded_leaves.iter().map(|cmx| cmx.map(|cmx| *cmx)).collect() + padded_leaves.iter().map(|cmx| cmx.map(|cmx| cmx.inner())).collect() ]; // diff --git a/src/value.rs b/src/value.rs index 4bfc4817..d7ec5289 100644 --- a/src/value.rs +++ b/src/value.rs @@ -164,11 +164,9 @@ impl TryFrom for i64 { #[derive(Clone, Debug)] pub struct ValueCommitTrapdoor(pallas::Scalar); -impl std::ops::Deref for ValueCommitTrapdoor { - type Target = pallas::Scalar; - - fn deref(&self) -> &pallas::Scalar { - &self.0 +impl ValueCommitTrapdoor { + pub(crate) fn inner(&self) -> pallas::Scalar { + self.0 } } @@ -277,7 +275,7 @@ impl ValueCommitment { } /// x-coordinate of this value commitment. - pub fn x(&self) -> pallas::Base { + pub(crate) fn x(&self) -> pallas::Base { if self.0 == pallas::Point::identity() { pallas::Base::zero() } else { @@ -286,7 +284,7 @@ impl ValueCommitment { } /// y-coordinate of this value commitment. - pub fn y(&self) -> pallas::Base { + pub(crate) fn y(&self) -> pallas::Base { if self.0 == pallas::Point::identity() { pallas::Base::zero() } else {