Merge pull request #823 from QED-it/improve-backward-compatability-without-zsa

Add a LookupRangeCheck trait and minor modifications in preparation for ZSA

Author: str4d

.github/workflows/ci.yml

@@ -97,6 +97,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v3
+      - run: sudo apt-get -y install libfontconfig1-dev
       # Check bitrot with stable (as we don't need benchmarks or the test-dev-graph
       # feature flag to work with MSRV).
       - uses: ./.github/actions/prepare
@@ -147,9 +148,10 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: tarpaulin
+          # Extend the timeout to 3600 to ensure the code coverage test pass
           args: >
             ${{ steps.prepare.outputs.feature-flags }}
-            --timeout 600
+            --timeout 3600
             --out Xml
       - name: Upload coverage to Codecov
         uses: codecov/[email protected]

halo2_gadgets/CHANGELOG.md

@@ -6,6 +6,16 @@ and this project adheres to Rust's notion of
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Added
+- `halo2_gadgets::utilities::lookup_range_check::LookupRangeCheck`
+- `halo2_gadgets::utilities::lookup_range_check::PallasLookupRangeCheck` which is
+  a shorthand for `LookupRangeCheck` specialized with `pallas::Base` and `sinsemilla::K`
+- `halo2_gadgets::utilities::lookup_range_check::PallasLookupRangeCheckConfig` which is
+  a shorthand for `LookupRangeCheckConfig` specialized with `pallas::Base` and `sinsemilla::K`
+
+### Changed
+- `halo2_gadgets::utilities::lookup_range_check::witness_short` now takes a generic `Lookup`
+  instead of directly taking a `LookupRangeCheckConfig<F, K>` reference
 
 ## [0.3.1] - 2024-12-16
 - `halo2_gadgets::poseidon::primitives` is now a re-export of the new `halo2_poseidon`

halo2_gadgets/src/ecc.rs

@@ -579,6 +579,7 @@ impl<C: CurveAffine, EccChip: EccInstructions<C>> FixedPointShort<C, EccChip> {
 pub(crate) mod tests {
     use ff::PrimeField;
     use group::{prime::PrimeCurveAffine, Curve, Group};
+    use std::marker::PhantomData;
 
     use halo2_proofs::{
         circuit::{Layouter, SimpleFloorPlanner, Value},
@@ -595,7 +596,10 @@ pub(crate) mod tests {
         },
         FixedPoints,
     };
-    use crate::utilities::lookup_range_check::LookupRangeCheckConfig;
+    use crate::{
+        test_circuits::test_utils::test_against_stored_circuit,
+        utilities::lookup_range_check::{PallasLookupRangeCheck, PallasLookupRangeCheckConfig},
+    };
 
     #[derive(Debug, Eq, PartialEq, Clone)]
     pub(crate) struct TestFixedBases;
@@ -723,17 +727,27 @@ pub(crate) mod tests {
         type Base = BaseField;
     }
 
-    struct MyCircuit {
+    struct MyCircuit<Lookup: PallasLookupRangeCheck> {
         test_errors: bool,
+        _lookup_marker: PhantomData<Lookup>,
+    }
+
+    impl<Lookup: PallasLookupRangeCheck> MyCircuit<Lookup> {
+        fn new(test_errors: bool) -> Self {
+            Self {
+                test_errors,
+                _lookup_marker: PhantomData,
+            }
+        }
     }
 
     #[allow(non_snake_case)]
-    impl Circuit<pallas::Base> for MyCircuit {
-        type Config = EccConfig<TestFixedBases>;
+    impl<Lookup: PallasLookupRangeCheck> Circuit<pallas::Base> for MyCircuit<Lookup> {
+        type Config = EccConfig<TestFixedBases, Lookup>;
         type FloorPlanner = SimpleFloorPlanner;
 
         fn without_witnesses(&self) -> Self {
-            MyCircuit { test_errors: false }
+            MyCircuit::new(false)
         }
 
         fn configure(meta: &mut ConstraintSystem<pallas::Base>) -> Self::Config {
@@ -764,8 +778,13 @@ pub(crate) mod tests {
             let constants = meta.fixed_column();
             meta.enable_constant(constants);
 
-            let range_check = LookupRangeCheckConfig::configure(meta, advices[9], lookup_table);
-            EccChip::<TestFixedBases>::configure(meta, advices, lagrange_coeffs, range_check)
+            let range_check = Lookup::configure(meta, advices[9], lookup_table);
+            EccChip::<TestFixedBases, Lookup>::configure(
+                meta,
+                advices,
+                lagrange_coeffs,
+                range_check,
+            )
         }
 
         fn synthesize(
@@ -896,11 +915,17 @@ pub(crate) mod tests {
     #[test]
     fn ecc_chip() {
         let k = 13;
-        let circuit = MyCircuit { test_errors: true };
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new(true);
         let prover = MockProver::run(k, &circuit, vec![]).unwrap();
         assert_eq!(prover.verify(), Ok(()))
     }
 
+    #[test]
+    fn test_ecc_chip_against_stored_circuit() {
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new(false);
+        test_against_stored_circuit(circuit, "ecc_chip", 3872);
+    }
+
     #[cfg(feature = "test-dev-graph")]
     #[test]
     fn print_ecc_chip() {
@@ -910,7 +935,7 @@ pub(crate) mod tests {
         root.fill(&WHITE).unwrap();
         let root = root.titled("Ecc Chip Layout", ("sans-serif", 60)).unwrap();
 
-        let circuit = MyCircuit { test_errors: false };
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new(false);
         halo2_proofs::dev::CircuitLayout::default()
             .render(13, &circuit, &root)
             .unwrap();

halo2_gadgets/src/ecc/chip.rs

@@ -1,9 +1,9 @@
 //! Chip implementations for the ECC gadgets.
 
 use super::{BaseFitsInScalarInstructions, EccInstructions, FixedPoints};
-use crate::{
-    sinsemilla::primitives as sinsemilla,
-    utilities::{lookup_range_check::LookupRangeCheckConfig, UtilitiesInstructions},
+use crate::utilities::{
+    lookup_range_check::{PallasLookupRangeCheck, PallasLookupRangeCheckConfig},
+    UtilitiesInstructions,
 };
 use arrayvec::ArrayVec;
 
@@ -137,7 +137,10 @@ impl From<NonIdentityEccPoint> for EccPoint {
 /// Configuration for [`EccChip`].
 #[derive(Clone, Debug, Eq, PartialEq)]
 #[allow(non_snake_case)]
-pub struct EccConfig<FixedPoints: super::FixedPoints<pallas::Affine>> {
+pub struct EccConfig<
+    FixedPoints: super::FixedPoints<pallas::Affine>,
+    Lookup: PallasLookupRangeCheck = PallasLookupRangeCheckConfig,
+> {
     /// Advice columns needed by instructions in the ECC chip.
     pub advices: [Column<Advice>; 10],
 
@@ -148,20 +151,20 @@ pub struct EccConfig<FixedPoints: super::FixedPoints<pallas::Affine>> {
     add: add::Config,
 
     /// Variable-base scalar multiplication
-    mul: mul::Config,
+    mul: mul::Config<Lookup>,
 
     /// Fixed-base full-width scalar multiplication
     mul_fixed_full: mul_fixed::full_width::Config<FixedPoints>,
     /// Fixed-base signed short scalar multiplication
     mul_fixed_short: mul_fixed::short::Config<FixedPoints>,
     /// Fixed-base mul using a base field element as a scalar
-    mul_fixed_base_field: mul_fixed::base_field_elem::Config<FixedPoints>,
+    mul_fixed_base_field: mul_fixed::base_field_elem::Config<FixedPoints, Lookup>,
 
     /// Witness point
     witness_point: witness_point::Config,
 
     /// Lookup range check using 10-bit lookup table
-    pub lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
+    pub lookup_config: Lookup,
 }
 
 /// A trait representing the kind of scalar used with a particular `FixedPoint`.
@@ -227,12 +230,17 @@ pub trait FixedPoint<C: CurveAffine>: std::fmt::Debug + Eq + Clone {
 
 /// An [`EccInstructions`] chip that uses 10 advice columns.
 #[derive(Clone, Debug, Eq, PartialEq)]
-pub struct EccChip<FixedPoints: super::FixedPoints<pallas::Affine>> {
-    config: EccConfig<FixedPoints>,
+pub struct EccChip<
+    FixedPoints: super::FixedPoints<pallas::Affine>,
+    Lookup: PallasLookupRangeCheck = PallasLookupRangeCheckConfig,
+> {
+    config: EccConfig<FixedPoints, Lookup>,
 }
 
-impl<FixedPoints: super::FixedPoints<pallas::Affine>> Chip<pallas::Base> for EccChip<FixedPoints> {
-    type Config = EccConfig<FixedPoints>;
+impl<FixedPoints: super::FixedPoints<pallas::Affine>, Lookup: PallasLookupRangeCheck>
+    Chip<pallas::Base> for EccChip<FixedPoints, Lookup>
+{
+    type Config = EccConfig<FixedPoints, Lookup>;
     type Loaded = ();
 
     fn config(&self) -> &Self::Config {
@@ -244,13 +252,15 @@ impl<FixedPoints: super::FixedPoints<pallas::Affine>> Chip<pallas::Base> for Ecc
     }
 }
 
-impl<Fixed: super::FixedPoints<pallas::Affine>> UtilitiesInstructions<pallas::Base>
-    for EccChip<Fixed>
+impl<Fixed: super::FixedPoints<pallas::Affine>, Lookup: PallasLookupRangeCheck>
+    UtilitiesInstructions<pallas::Base> for EccChip<Fixed, Lookup>
 {
     type Var = AssignedCell<pallas::Base, pallas::Base>;
 }
 
-impl<FixedPoints: super::FixedPoints<pallas::Affine>> EccChip<FixedPoints> {
+impl<FixedPoints: super::FixedPoints<pallas::Affine>, Lookup: PallasLookupRangeCheck>
+    EccChip<FixedPoints, Lookup>
+{
     /// Reconstructs this chip from the given config.
     pub fn construct(config: <Self as Chip<pallas::Base>>::Config) -> Self {
         Self { config }
@@ -264,7 +274,7 @@ impl<FixedPoints: super::FixedPoints<pallas::Affine>> EccChip<FixedPoints> {
         meta: &mut ConstraintSystem<pallas::Base>,
         advices: [Column<Advice>; 10],
         lagrange_coeffs: [Column<Fixed>; 8],
-        range_check: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
+        range_check: Lookup,
     ) -> <Self as Chip<pallas::Base>>::Config {
         // Create witness point gate
         let witness_point = witness_point::Config::configure(meta, advices[0], advices[1]);
@@ -301,12 +311,13 @@ impl<FixedPoints: super::FixedPoints<pallas::Affine>> EccChip<FixedPoints> {
             mul_fixed::short::Config::<FixedPoints>::configure(meta, mul_fixed.clone());
 
         // Create gate that is only used in fixed-base mul using a base field element.
-        let mul_fixed_base_field = mul_fixed::base_field_elem::Config::<FixedPoints>::configure(
-            meta,
-            advices[6..9].try_into().unwrap(),
-            range_check,
-            mul_fixed,
-        );
+        let mul_fixed_base_field =
+            mul_fixed::base_field_elem::Config::<FixedPoints, Lookup>::configure(
+                meta,
+                advices[6..9].try_into().unwrap(),
+                range_check,
+                mul_fixed,
+            );
 
         EccConfig {
             advices,
@@ -407,7 +418,8 @@ pub enum ScalarVar {
     FullWidth,
 }
 
-impl<Fixed: FixedPoints<pallas::Affine>> EccInstructions<pallas::Affine> for EccChip<Fixed>
+impl<Fixed: FixedPoints<pallas::Affine>, Lookup: PallasLookupRangeCheck>
+    EccInstructions<pallas::Affine> for EccChip<Fixed, Lookup>
 where
     <Fixed as FixedPoints<pallas::Affine>>::Base:
         FixedPoint<pallas::Affine, FixedScalarKind = BaseFieldElem>,
@@ -594,8 +606,8 @@ where
     }
 }
 
-impl<Fixed: FixedPoints<pallas::Affine>> BaseFitsInScalarInstructions<pallas::Affine>
-    for EccChip<Fixed>
+impl<Fixed: FixedPoints<pallas::Affine>, Lookup: PallasLookupRangeCheck>
+    BaseFitsInScalarInstructions<pallas::Affine> for EccChip<Fixed, Lookup>
 where
     <Fixed as FixedPoints<pallas::Affine>>::Base:
         FixedPoint<pallas::Affine, FixedScalarKind = BaseFieldElem>,

halo2_gadgets/src/ecc/chip/mul.rs

@@ -1,7 +1,7 @@
 use super::{add, EccPoint, NonIdentityEccPoint, ScalarVar, T_Q};
-use crate::{
-    sinsemilla::primitives as sinsemilla,
-    utilities::{bool_check, lookup_range_check::LookupRangeCheckConfig, ternary},
+use crate::utilities::{
+    lookup_range_check::{PallasLookupRangeCheck, PallasLookupRangeCheckConfig},
+    {bool_check, ternary},
 };
 use std::{
     convert::TryInto,
@@ -46,7 +46,7 @@ const INCOMPLETE_LO_LEN: usize = INCOMPLETE_LEN - INCOMPLETE_HI_LEN;
 const COMPLETE_RANGE: Range<usize> = INCOMPLETE_LEN..(INCOMPLETE_LEN + NUM_COMPLETE_BITS);
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub struct Config {
+pub struct Config<Lookup: PallasLookupRangeCheck = PallasLookupRangeCheckConfig> {
     // Selector used to check switching logic on LSB
     q_mul_lsb: Selector,
     // Configuration used in complete addition
@@ -58,14 +58,14 @@ pub struct Config {
     // Configuration used for complete addition part of double-and-add algorithm
     complete_config: complete::Config,
     // Configuration used to check for overflow
-    overflow_config: overflow::Config,
+    overflow_config: overflow::Config<Lookup>,
 }
 
-impl Config {
+impl<Lookup: PallasLookupRangeCheck> Config<Lookup> {
     pub(super) fn configure(
         meta: &mut ConstraintSystem<pallas::Base>,
         add_config: add::Config,
-        lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
+        lookup_config: Lookup,
         advices: [Column<Advice>; 10],
     ) -> Self {
         let hi_config = incomplete::Config::configure(
@@ -473,13 +473,13 @@ pub mod tests {
             tests::TestFixedBases,
             EccInstructions, NonIdentityPoint, Point, ScalarVar,
         },
-        utilities::UtilitiesInstructions,
+        utilities::{lookup_range_check::PallasLookupRangeCheck, UtilitiesInstructions},
     };
 
-    pub(crate) fn test_mul(
-        chip: EccChip<TestFixedBases>,
+    pub(crate) fn test_mul<Lookup: PallasLookupRangeCheck>(
+        chip: EccChip<TestFixedBases, Lookup>,
         mut layouter: impl Layouter<pallas::Base>,
-        p: &NonIdentityPoint<pallas::Affine, EccChip<TestFixedBases>>,
+        p: &NonIdentityPoint<pallas::Affine, EccChip<TestFixedBases, Lookup>>,
         p_val: pallas::Affine,
     ) -> Result<(), Error> {
         let column = chip.config().advices[0];

halo2_gadgets/src/ecc/chip/mul/overflow.rs

@@ -1,6 +1,7 @@
 use super::{T_Q, Z};
 use crate::{
-    sinsemilla::primitives as sinsemilla, utilities::lookup_range_check::LookupRangeCheckConfig,
+    sinsemilla::primitives as sinsemilla,
+    utilities::lookup_range_check::{PallasLookupRangeCheck, PallasLookupRangeCheckConfig},
 };
 
 use group::ff::PrimeField;
@@ -15,19 +16,19 @@ use pasta_curves::pallas;
 use std::iter;
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub struct Config {
+pub struct Config<Lookup: PallasLookupRangeCheck = PallasLookupRangeCheckConfig> {
     // Selector to check z_0 = alpha + t_q (mod p)
     q_mul_overflow: Selector,
     // 10-bit lookup table
-    lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
+    lookup_config: Lookup,
     // Advice columns
     advices: [Column<Advice>; 3],
 }
 
-impl Config {
+impl<Lookup: PallasLookupRangeCheck> Config<Lookup> {
     pub(super) fn configure(
         meta: &mut ConstraintSystem<pallas::Base>,
-        lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
+        lookup_config: Lookup,
         advices: [Column<Advice>; 3],
     ) -> Self {
         for advice in advices.iter() {