Refactoring

QED-it · Jul 1, 2024 · 0d2512a · 0d2512a
1 parent 2021f5e
commit 0d2512a
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 194 deletions.
diff --git a/halo2_gadgets/src/ecc.rs b/halo2_gadgets/src/ecc.rs
@@ -60,6 +60,15 @@ pub trait EccInstructions<C: CurveAffine>:
         value: Value<C>,
     ) -> Result<Self::Point, Error>;
 
+    /// Witnesses the given constant point as a private input to the circuit.
+    /// This allows the point to be the identity, mapped to (0, 0) in
+    /// affine coordinates.
+    fn witness_point_from_constant(
+        &self,
+        layouter: &mut impl Layouter<C::Base>,
+        value: C,
+    ) -> Result<Self::Point, Error>;
+
     /// Witnesses the given point as a private input to the circuit.
     /// This returns an error if the point is the identity.
     fn witness_point_non_id(
@@ -111,6 +120,15 @@ pub trait EccInstructions<C: CurveAffine>:
         b: &B,
     ) -> Result<Self::Point, Error>;
 
+    /// Performs variable-base sign-scalar multiplication, returning `[sign] point`
+    /// `sign` must be in {-1, 1}.
+    fn mul_sign(
+        &self,
+        layouter: &mut impl Layouter<C::Base>,
+        sign: &AssignedCell<C::Base, C::Base>,
+        point: &Self::Point,
+    ) -> Result<Self::Point, Error>;
+
     /// Performs variable-base scalar multiplication, returning `[scalar] base`.
     fn mul(
         &self,
@@ -145,24 +163,6 @@ pub trait EccInstructions<C: CurveAffine>:
         base_field_elem: Self::Var,
         base: &<Self::FixedPoints as FixedPoints<C>>::Base,
     ) -> Result<Self::Point, Error>;
-
-    /// Witnesses the given constant point as a private input to the circuit.
-    /// This allows the point to be the identity, mapped to (0, 0) in
-    /// affine coordinates.
-    fn witness_point_from_constant(
-        &self,
-        layouter: &mut impl Layouter<C::Base>,
-        value: C,
-    ) -> Result<Self::Point, Error>;
-
-    /// Performs variable-base sign-scalar multiplication, returning `[sign] point`
-    /// `sign` must be in {-1, 1}.
-    fn mul_sign(
-        &self,
-        layouter: &mut impl Layouter<C::Base>,
-        sign: &AssignedCell<C::Base, C::Base>,
-        point: &Self::Point,
-    ) -> Result<Self::Point, Error>;
 }
 
 /// Instructions that can be implemented for a curve whose base field fits into
@@ -408,6 +408,17 @@ impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> Point<C,
         point.map(|inner| Point { chip, inner })
     }
 
+    /// Witnesses the given constant point as a private input to the circuit.
+    /// This allows the point to be the identity, mapped to (0, 0) in affine coordinates.
+    pub fn new_from_constant(
+        chip: EccChip,
+        mut layouter: impl Layouter<C::Base>,
+        value: C,
+    ) -> Result<Self, Error> {
+        let point = chip.witness_point_from_constant(&mut layouter, value);
+        point.map(|inner| Point { chip, inner })
+    }
+
     /// Constrains this point to be equal in value to another point.
     pub fn constrain_equal<Other: Into<Point<C, EccChip>> + Clone>(
         &self,
@@ -451,16 +462,6 @@ impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> Point<C,
             })
     }
 
-    /// Constructs a new point with the given fixed value.
-    pub fn new_from_constant(
-        chip: EccChip,
-        mut layouter: impl Layouter<C::Base>,
-        value: C,
-    ) -> Result<Self, Error> {
-        let point = chip.witness_point_from_constant(&mut layouter, value);
-        point.map(|inner| Point { chip, inner })
-    }
-
     /// Returns `[sign] self`.
     /// `sign` must be in {-1, 1}.
     pub fn mul_sign(

diff --git a/halo2_gadgets/src/ecc/chip.rs b/halo2_gadgets/src/ecc/chip.rs
@@ -465,6 +465,21 @@ where
         )
     }
 
+    /// Witnesses the given constant point as a private input to the circuit.
+    /// This allows the point to be the identity, mapped to (0, 0) in
+    /// affine coordinates.
+    fn witness_point_from_constant(
+        &self,
+        layouter: &mut impl Layouter<pallas::Base>,
+        value: pallas::Affine,
+    ) -> Result<Self::Point, Error> {
+        let config = self.config().witness_point;
+        layouter.assign_region(
+            || "witness point (constant)",
+            |mut region| config.constant_point(value, 0, &mut region),
+        )
+    }
+
     fn witness_point_non_id(
         &self,
         layouter: &mut impl Layouter<pallas::Base>,
@@ -544,6 +559,24 @@ where
         )
     }
 
+    /// Performs variable-base sign-scalar multiplication, returning `[sign] point`
+    /// `sign` must be in {-1, 1}.
+    fn mul_sign(
+        &self,
+        layouter: &mut impl Layouter<pallas::Base>,
+        sign: &AssignedCell<pallas::Base, pallas::Base>,
+        point: &Self::Point,
+    ) -> Result<Self::Point, Error> {
+        // Multiply point by sign, using the same gate as mul_fixed::short.
+        // This also constrains sign to be in {-1, 1}.
+        let config_short = self.config().mul_fixed_short.clone();
+        config_short.assign_scalar_sign(
+            layouter.namespace(|| "variable-base sign-scalar mul"),
+            sign,
+            point,
+        )
+    }
+
     fn mul(
         &self,
         layouter: &mut impl Layouter<pallas::Base>,
@@ -604,39 +637,6 @@ where
             base,
         )
     }
-
-    /// Witnesses the given constant point as a private input to the circuit.
-    /// This allows the point to be the identity, mapped to (0, 0) in
-    /// affine coordinates.
-    fn witness_point_from_constant(
-        &self,
-        layouter: &mut impl Layouter<pallas::Base>,
-        value: pallas::Affine,
-    ) -> Result<Self::Point, Error> {
-        let config = self.config().witness_point;
-        layouter.assign_region(
-            || "witness point (constant)",
-            |mut region| config.constant_point(value, 0, &mut region),
-        )
-    }
-
-    /// Performs variable-base sign-scalar multiplication, returning `[sign] point`
-    /// `sign` must be in {-1, 1}.
-    fn mul_sign(
-        &self,
-        layouter: &mut impl Layouter<pallas::Base>,
-        sign: &AssignedCell<pallas::Base, pallas::Base>,
-        point: &Self::Point,
-    ) -> Result<Self::Point, Error> {
-        // Multiply point by sign, using the same gate as mul_fixed::short.
-        // This also constrains sign to be in {-1, 1}.
-        let config_short = self.config().mul_fixed_short.clone();
-        config_short.assign_scalar_sign(
-            layouter.namespace(|| "variable-base sign-scalar mul"),
-            sign,
-            point,
-        )
-    }
 }
 
 impl<Fixed: FixedPoints<pallas::Affine>, Lookup: PallasLookupRangeCheck>

diff --git a/halo2_gadgets/src/ecc/chip/mul_fixed/full_width.rs b/halo2_gadgets/src/ecc/chip/mul_fixed/full_width.rs
@@ -187,12 +187,14 @@ pub mod tests {
     use pasta_curves::pallas;
     use rand::rngs::OsRng;
 
-    use crate::ecc::{
-        chip::{EccChip, FixedPoint as _, H},
-        tests::{FullWidth, TestFixedBases},
-        FixedPoint, NonIdentityPoint, Point, ScalarFixed,
+    use crate::{
+        ecc::{
+            chip::{EccChip, FixedPoint as _, H},
+            tests::{FullWidth, TestFixedBases},
+            FixedPoint, NonIdentityPoint, Point, ScalarFixed,
+        },
+        utilities::lookup_range_check::PallasLookupRangeCheck,
     };
-    use crate::utilities::lookup_range_check::PallasLookupRangeCheck;
 
     pub(crate) fn test_mul_fixed<Lookup: PallasLookupRangeCheck>(
         chip: EccChip<TestFixedBases, Lookup>,

diff --git a/halo2_gadgets/src/sinsemilla.rs b/halo2_gadgets/src/sinsemilla.rs
@@ -441,44 +441,6 @@ where
         }
     }
 
-    #[allow(clippy::type_complexity)]
-    /// $\mathsf{SinsemillaCommit}$ from [§ 5.4.8.4][concretesinsemillacommit].
-    ///
-    /// [concretesinsemillacommit]: https://zips.z.cash/protocol/nu5.pdf#concretesinsemillacommit
-    pub fn commit(
-        &self,
-        mut layouter: impl Layouter<C::Base>,
-        message: Message<C, SinsemillaChip, K, MAX_WORDS>,
-        r: ecc::ScalarFixed<C, EccChip>,
-    ) -> Result<
-        (
-            ecc::Point<C, EccChip>,
-            Vec<SinsemillaChip::RunningSum>,
-        ),
-        Error,
-    > {
-        assert_eq!(self.M.sinsemilla_chip, message.chip);
-        let (blind, _) = self.R.mul(layouter.namespace(|| "[r] R"), r)?;
-        let (p, zs) = self.M.hash_to_point(layouter.namespace(|| "M"), message)?;
-        let commitment = p.add(layouter.namespace(|| "M + [r] R"), &blind)?;
-        Ok((commitment, zs))
-    }
-
-    #[allow(clippy::type_complexity)]
-    /// $\mathsf{SinsemillaShortCommit}$ from [§ 5.4.8.4][concretesinsemillacommit].
-    ///
-    /// [concretesinsemillacommit]: https://zips.z.cash/protocol/nu5.pdf#concretesinsemillacommit
-    pub fn short_commit(
-        &self,
-        mut layouter: impl Layouter<C::Base>,
-        message: Message<C, SinsemillaChip, K, MAX_WORDS>,
-        r: ecc::ScalarFixed<C, EccChip>,
-    ) -> Result<(ecc::X<C, EccChip>, Vec<SinsemillaChip::RunningSum>), Error> {
-        assert_eq!(self.M.sinsemilla_chip, message.chip);
-        let (p, zs) = self.commit(layouter.namespace(|| "commit"), message, r)?;
-        Ok((p.extract_p(), zs))
-    }
-
     #[allow(non_snake_case)]
     #[allow(clippy::type_complexity)]
     /// Evaluates the Sinsemilla hash of `message` from the private initial point `Q`.
@@ -517,6 +479,44 @@ where
         let (blind, _) = self.R.mul(layouter.namespace(|| "[r] R"), r)?;
         Ok(blind)
     }
+
+    #[allow(clippy::type_complexity)]
+    /// $\mathsf{SinsemillaCommit}$ from [§ 5.4.8.4][concretesinsemillacommit].
+    ///
+    /// [concretesinsemillacommit]: https://zips.z.cash/protocol/nu5.pdf#concretesinsemillacommit
+    pub fn commit(
+        &self,
+        mut layouter: impl Layouter<C::Base>,
+        message: Message<C, SinsemillaChip, K, MAX_WORDS>,
+        r: ecc::ScalarFixed<C, EccChip>,
+    ) -> Result<
+        (
+            ecc::Point<C, EccChip>,
+            Vec<SinsemillaChip::RunningSum>,
+        ),
+        Error,
+    > {
+        assert_eq!(self.M.sinsemilla_chip, message.chip);
+        let (blind, _) = self.R.mul(layouter.namespace(|| "[r] R"), r)?;
+        let (p, zs) = self.M.hash_to_point(layouter.namespace(|| "M"), message)?;
+        let commitment = p.add(layouter.namespace(|| "M + [r] R"), &blind)?;
+        Ok((commitment, zs))
+    }
+
+    #[allow(clippy::type_complexity)]
+    /// $\mathsf{SinsemillaShortCommit}$ from [§ 5.4.8.4][concretesinsemillacommit].
+    ///
+    /// [concretesinsemillacommit]: https://zips.z.cash/protocol/nu5.pdf#concretesinsemillacommit
+    pub fn short_commit(
+        &self,
+        mut layouter: impl Layouter<C::Base>,
+        message: Message<C, SinsemillaChip, K, MAX_WORDS>,
+        r: ecc::ScalarFixed<C, EccChip>,
+    ) -> Result<(ecc::X<C, EccChip>, Vec<SinsemillaChip::RunningSum>), Error> {
+        assert_eq!(self.M.sinsemilla_chip, message.chip);
+        let (p, zs) = self.commit(layouter.namespace(|| "commit"), message, r)?;
+        Ok((p.extract_p(), zs))
+    }
 }
 
 #[cfg(test)]