add comments

QED-it · Apr 15, 2024 · 08c554b · 08c554b
1 parent fc37508
commit 08c554b
Show file tree

Hide file tree

Showing 12 changed files with 317 additions and 223 deletions.
diff --git a/halo2_gadgets/src/ecc_opt.rs b/halo2_gadgets/src/ecc_opt.rs
@@ -0,0 +1 @@
+pub mod chip;
diff --git a/halo2_gadgets/src/ecc_opt/chip.rs b/halo2_gadgets/src/ecc_opt/chip.rs
@@ -0,0 +1 @@
+// TODO: EccConfig has LookupRangeCheckConfig
diff --git a/halo2_gadgets/src/lib.rs b/halo2_gadgets/src/lib.rs
@@ -22,6 +22,7 @@
 #![deny(unsafe_code)]
 
 pub mod ecc;
+mod ecc_opt;
 pub mod poseidon;
 #[cfg(feature = "unstable-sha256-gadget")]
 #[cfg_attr(docsrs, doc(cfg(feature = "unstable-sha256-gadget")))]

diff --git a/halo2_gadgets/src/sinsemilla/chip.rs b/halo2_gadgets/src/sinsemilla/chip.rs
@@ -11,8 +11,6 @@ use crate::{
     },
     utilities::lookup_range_check::LookupRangeCheckConfig,
 };
-use ff::PrimeField;
-use pasta_curves::arithmetic::CurveAffine;
 use std::marker::PhantomData;
 
 use halo2_proofs::{
@@ -25,17 +23,12 @@ use halo2_proofs::{
 };
 use pasta_curves::pallas;
 use pasta_curves::pallas::Base;
-use proptest::test_runner::Config;
 
 mod generator_table;
-use crate::sinsemilla::primitives::{lebs2ip_k, INV_TWO_POW_K, SINSEMILLA_S};
 use generator_table::GeneratorTableConfig;
-use halo2_proofs::circuit::Region;
-use halo2_proofs::plonk::Assigned;
-
 mod hash_to_point;
 
-/// Configuration for the Sinsemilla hash chip
+/// Configuration for the Sinsemilla hash chip common parts
 #[derive(Eq, PartialEq, Clone, Debug)]
 pub struct SinsemillaConfigCommon<Hash, Commit, F>
 where
@@ -78,14 +71,17 @@ where
     /// An advice column configured to perform lookup range checks.
     lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
 }
-// TODO: add doc, rename it to SinsemillaConfigProps
+
+/// Trait that provides common methods for SinsemillaConfig and SinsemillaConfigOptimized
 pub trait SinsemillaConfigProps<Hash, Commit, F>
 where
     Hash: HashDomains<pallas::Affine>,
     F: FixedPoints<pallas::Affine>,
     Commit: CommitDomains<pallas::Affine, F, Hash>,
 {
     type LookupConfigType;
+
+    /// Returns a reference to the `SinsemillaConfigCommon` instance.
     fn base(&self) -> &SinsemillaConfigCommon<Hash, Commit, F>;
 
     /// Returns an array of all advice columns in this config, in arbitrary order.
@@ -109,8 +105,12 @@ where
         q_s2.clone() * (q_s2 - one)
     }
 
+    /// querying a value 'y_q' from certain column
     fn get_y_q(&self, meta: &mut VirtualCells<pallas::Base>) -> Expression<pallas::Base>;
 
+    /// Configures constraints in the constraint system `meta` using the value 'y_q'
+    /// This function sets up various gates within the circuit to enforce the correct relationships
+    /// between variables according to elliptic curve arithmetic and the Sinsemilla hash function.
     fn configure_from_y_q(&self, meta: &mut ConstraintSystem<pallas::Base>) {
         let two = pallas::Base::from(2);
 
@@ -188,7 +188,7 @@ where
         });
     }
 }
-// TODO: add doc
+
 impl<Hash, Commit, F> SinsemillaConfigProps<Hash, Commit, F> for SinsemillaConfig<Hash, Commit, F>
 where
     Hash: HashDomains<pallas::Affine>,
@@ -204,7 +204,7 @@ where
         self.lookup_config
     }
 
-    // todo: add doc
+    /// Query a fixed value from the circuit's fixed column using the configuration `fixed_y_q`.
     fn get_y_q(&self, meta: &mut VirtualCells<Base>) -> Expression<Base> {
         meta.query_fixed(self.base.fixed_y_q)
     }
@@ -223,7 +223,7 @@ where
     config: SinsemillaConfig<Hash, Commit, Fixed>,
 }
 
-// TODO: add doc,rename it to SinsemillaChipProps
+/// Trait that provides common methods for SinsemillaChip and SinsemillaChipOptimized
 pub trait SinsemillaChipProps<Hash, Commit, F>
 where
     Hash: HashDomains<pallas::Affine>,
@@ -246,6 +246,10 @@ where
         config: Self::SinsemillaConfigType,
         layouter: &mut impl Layouter<pallas::Base>,
     ) -> Result<Self::Loaded, Error>;
+
+    /// # Side-effects
+    ///
+    /// All columns in `advices` and will be equality-enabled.
     fn configure(
         meta: &mut ConstraintSystem<pallas::Base>,
         advices: [Column<Advice>; 5],
@@ -274,12 +278,10 @@ where
         &self.config.base
     }
 
-    /// Reconstructs this chip from the given config.
     fn construct(config: Self::SinsemillaConfigType) -> Self {
         Self { config }
     }
 
-    /// Loads the lookup table required by this chip into the circuit.
     fn load(
         config: Self::SinsemillaConfigType,
         layouter: &mut impl Layouter<pallas::Base>,
@@ -288,9 +290,6 @@ where
         config.generator_table.load(layouter)
     }
 
-    /// # Side-effects
-    ///
-    /// All columns in `advices` and will be equality-enabled.
     #[allow(clippy::too_many_arguments)]
     #[allow(non_snake_case)]
     fn configure(
@@ -325,7 +324,7 @@ where
     }
 }
 
-// TODO: add doc
+/// A function to generate the common part of SinsemillaConfig 'SinsemillaConfigCommon'
 pub fn create_common_config<Hash, Commit, F>(
     meta: &mut ConstraintSystem<pallas::Base>,
     advices: [Column<Advice>; 5],
@@ -354,7 +353,7 @@ where
     }
 }
 
-// TODO: remove duplicate?
+// TODO: remove duplicated code?
 impl<Hash, Commit, Fixed> Chip<pallas::Base> for SinsemillaChip<Hash, Commit, Fixed>
 where
     Hash: HashDomains<pallas::Affine>,
@@ -373,7 +372,7 @@ where
     }
 }
 
-// TODO: remove duplicate?
+// TODO: remove duplicated code?
 
 // Implement `SinsemillaInstructions` for `SinsemillaChip`
 impl<Hash, Commit, F> SinsemillaInstructions<pallas::Affine, { sinsemilla::K }, { sinsemilla::C }>
@@ -419,7 +418,6 @@ where
         Ok(MessagePiece::new(cell, num_words))
     }
 
-    // TODO: in the opt version: hash_message_vanilla -> hash_message
     #[allow(non_snake_case)]
     #[allow(clippy::type_complexity)]
     fn hash_to_point(
@@ -430,6 +428,7 @@ where
     ) -> Result<(Self::NonIdentityPoint, Vec<Self::RunningSum>), Error> {
         layouter.assign_region(
             || "hash_to_point",
+            // TODO: in the opt version: hash_message_vanilla -> hash_message
             |mut region| self.hash_message_vanilla(&mut region, Q, &message),
         )
     }

diff --git a/halo2_gadgets/src/sinsemilla/chip/hash_to_point.rs b/halo2_gadgets/src/sinsemilla/chip/hash_to_point.rs
@@ -58,40 +58,6 @@ where
         ))
     }
 
-    /// [Specification](https://p.z.cash/halo2-0.1:sinsemilla-constraints?partial).
-    #[allow(non_snake_case)]
-    #[allow(clippy::type_complexity)]
-    fn hash_message(
-        &self,
-        region: &mut Region<'_, pallas::Base>,
-        Q: pallas::Affine,
-        message: &<Self as SinsemillaInstructions<
-            pallas::Affine,
-            { sinsemilla::K },
-            { sinsemilla::C },
-        >>::Message,
-    ) -> Result<
-        (
-            NonIdentityEccPoint,
-            Vec<Vec<AssignedCell<pallas::Base, pallas::Base>>>,
-        ),
-        Error,
-    > {
-        let (offset, x_a, y_a) = self.public_initialization(region, Q)?;
-
-        let (x_a, y_a, zs_sum) = self.hash_all_pieces(region, offset, message, x_a, y_a)?;
-
-        // todo: add test
-
-        x_a.value()
-            .zip(y_a.value())
-            .error_if_known_and(|(x_a, y_a)| x_a.is_zero_vartime() || y_a.is_zero_vartime())?;
-        Ok((
-            NonIdentityEccPoint::from_coordinates_unchecked(x_a.0, y_a),
-            zs_sum,
-        ))
-    }
-
     /// [Specification](https://p.z.cash/halo2-0.1:sinsemilla-constraints?partial).
     #[allow(non_snake_case)]
     #[allow(clippy::type_complexity)]
@@ -171,15 +137,15 @@ where
         Q: pallas::Affine,
     ) -> Result<(usize, X<pallas::Base>, Y<pallas::Base>), Error> {
         let config = self.config().clone();
-        let mut offset = 0;
+        let offset = 0;
 
         // Get the `x`- and `y`-coordinates of the starting `Q` base.
         let x_q = *Q.coordinates().unwrap().x();
         let y_q = *Q.coordinates().unwrap().y();
 
         // Constrain the initial x_a, lambda_1, lambda_2, x_p using the q_sinsemilla4
         // selector.
-        let mut y_a: Y<pallas::Base> = {
+        let y_a: Y<pallas::Base> = {
             // Enable `q_sinsemilla4` on the first row.
             config.base.q_sinsemilla4.enable(region, offset)?;
             region.assign_fixed(
@@ -192,55 +158,6 @@ where
             Value::known(y_q.into()).into()
         };
 
-        // Constrain the initial x_q to equal the x-coordinate of the domain's `Q`.
-        let mut x_a: X<pallas::Base> = {
-            let x_a = region.assign_advice_from_constant(
-                || "fixed x_q",
-                config.base.double_and_add.x_a,
-                offset,
-                x_q.into(),
-            )?;
-
-            x_a.into()
-        };
-        Ok((offset, x_a, y_a))
-    }
-    #[allow(non_snake_case)]
-    /// Assign the coordinates of the initial public point `Q`
-    ///
-    /// | offset | x_A | x_P | q_sinsemilla4 |
-    /// --------------------------------------
-    /// |   0    |     | y_Q |               |
-    /// |   1    | x_Q |     |       1       |
-    fn public_initialization(
-        &self,
-        region: &mut Region<'_, pallas::Base>,
-        Q: pallas::Affine,
-    ) -> Result<(usize, X<pallas::Base>, Y<pallas::Base>), Error> {
-        let config = self.config().clone();
-        let mut offset = 0;
-
-        // Get the `x`- and `y`-coordinates of the starting `Q` base.
-        let x_q = *Q.coordinates().unwrap().x();
-        let y_q = *Q.coordinates().unwrap().y();
-
-        // Constrain the initial x_a, lambda_1, lambda_2, x_p using the q_sinsemilla4
-        // selector.
-        let y_a: Y<pallas::Base> = {
-            // Enable `q_sinsemilla4` on the second row.
-            config.base.q_sinsemilla4.enable(region, offset + 1)?;
-            let y_a: AssignedCell<Assigned<pallas::Base>, pallas::Base> = region
-                .assign_advice_from_constant(
-                    || "fixed y_q",
-                    config.base.double_and_add.x_p,
-                    offset,
-                    y_q.into(),
-                )?;
-
-            y_a.value_field().into()
-        };
-        offset += 1;
-
         // Constrain the initial x_q to equal the x-coordinate of the domain's `Q`.
         let x_a: X<pallas::Base> = {
             let x_a = region.assign_advice_from_constant(
@@ -252,7 +169,6 @@ where
 
             x_a.into()
         };
-
         Ok((offset, x_a, y_a))
     }
 

diff --git a/halo2_gadgets/src/sinsemilla_opt/chip.rs b/halo2_gadgets/src/sinsemilla_opt/chip.rs
@@ -1,30 +1,21 @@
 mod generator_table;
 mod hash_to_point;
 
+use crate::ecc::{chip::NonIdentityEccPoint, FixedPoints};
 use crate::sinsemilla::message::{Message, MessagePiece};
 use crate::sinsemilla::primitives as sinsemilla;
-use crate::{
-    ecc::{
-        chip::{DoubleAndAdd, NonIdentityEccPoint},
-        FixedPoints,
-    },
-    utilities::lookup_range_check::LookupRangeCheckConfig,
-};
 use halo2_proofs::{
     circuit::{AssignedCell, Chip, Layouter, Value},
     plonk::{
-        Advice, Column, ConstraintSystem, Constraints, Error, Expression, Fixed, Selector,
-        TableColumn, VirtualCells,
+        Advice, Column, ConstraintSystem, Error, Expression, Fixed, TableColumn, VirtualCells,
     },
     poly::Rotation,
 };
 use pasta_curves::pallas;
 use pasta_curves::pallas::Base;
-use std::marker::PhantomData;
 
 use crate::sinsemilla::chip::{
-    create_common_config, SinsemillaChip, SinsemillaChipProps, SinsemillaConfigCommon,
-    SinsemillaConfigProps,
+    create_common_config, SinsemillaChipProps, SinsemillaConfigCommon, SinsemillaConfigProps,
 };
 use crate::sinsemilla::{CommitDomains, HashDomains, SinsemillaInstructions};
 use crate::utilities_opt::lookup_range_check::LookupRangeCheckConfigOptimized;
@@ -62,7 +53,7 @@ where
         self.lookup_config
     }
 
-    // todo: add doc
+    /// Query an advice value 'y_q' from a specific advice column `x_p` at the previous rotation.
     fn get_y_q(&self, meta: &mut VirtualCells<Base>) -> Expression<Base> {
         meta.query_advice(self.base.double_and_add.x_p, Rotation::prev())
     }
@@ -114,9 +105,6 @@ where
         config.generator_table.load(layouter)
     }
 
-    /// # Side-effects
-    ///
-    /// All columns in `advices` and will be equality-enabled.
     #[allow(clippy::too_many_arguments)]
     #[allow(non_snake_case)]
     fn configure(
@@ -152,7 +140,7 @@ where
     }
 }
 
-// TODO: remove duplicate?
+// TODO: remove duplicated code?
 impl<Hash, Commit, Fixed> Chip<pallas::Base> for SinsemillaChipOptimized<Hash, Commit, Fixed>
 where
     Hash: HashDomains<pallas::Affine>,
@@ -171,7 +159,7 @@ where
     }
 }
 
-// TODO: remove duplicate?
+// TODO: remove duplicated code?
 
 // Implement `SinsemillaInstructions` for `SinsemillaChip`
 impl<Hash, Commit, F> SinsemillaInstructions<pallas::Affine, { sinsemilla::K }, { sinsemilla::C }>