From d6bcf3b5c12568aee591d3dbb86b358ff3ff309b Mon Sep 17 00:00:00 2001
From: Constance Beguier <constance@qed-it.com>
Date: Tue, 25 Jun 2024 14:59:39 +0200
Subject: [PATCH] Fix LookupangeCheck loading in Sinsemilla

Previously, the SinsemillaChip always loaded the lookup range check
without tag column and Sinsemilla45BChip always loaded the lookup
range check with tag column.
Now, we load lookup range check with or without tag according to the
lookup type (LookupRangeCheckConfig or LookupRangeCheck45BConfig).
---
 halo2_gadgets/src/sinsemilla/chip.rs              |  9 ++++-----
 .../src/sinsemilla/chip/generator_table.rs        | 15 ++++++++++++++-
 halo2_gadgets/src/utilities/lookup_range_check.rs | 15 +++++++++++----
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/halo2_gadgets/src/sinsemilla/chip.rs b/halo2_gadgets/src/sinsemilla/chip.rs
index 765ddb742..494639ff1 100644
--- a/halo2_gadgets/src/sinsemilla/chip.rs
+++ b/halo2_gadgets/src/sinsemilla/chip.rs
@@ -148,8 +148,9 @@ where
         layouter: &mut impl Layouter<pallas::Base>,
     ) -> Result<<Self as Chip<pallas::Base>>::Loaded, Error> {
         // Load the lookup table.
-
-        config.generator_table.load(layouter)
+        config
+            .generator_table
+            .load(config.lookup_config.table_range_check_tag(), layouter)
     }
 
     /// # Side-effects
@@ -434,9 +435,7 @@ where
         layouter: &mut impl Layouter<pallas::Base>,
     ) -> Result<<Self as Chip<pallas::Base>>::Loaded, Error> {
         // Load the lookup table.
-        config
-            .generator_table
-            .load_with_tag(config.lookup_config.table_range_check_tag(), layouter)
+        SinsemillaChip::<Hash, Commit, F, PallasLookupRangeCheck45BConfig>::load(config, layouter)
     }
 
     /// Assign y_q to an advice column
diff --git a/halo2_gadgets/src/sinsemilla/chip/generator_table.rs b/halo2_gadgets/src/sinsemilla/chip/generator_table.rs
index d5fe0f055..57ff7e723 100644
--- a/halo2_gadgets/src/sinsemilla/chip/generator_table.rs
+++ b/halo2_gadgets/src/sinsemilla/chip/generator_table.rs
@@ -81,7 +81,20 @@ impl GeneratorTableConfig {
         });
     }
 
-    pub fn load(&self, layouter: &mut impl Layouter<pallas::Base>) -> Result<(), Error> {
+    pub fn load(
+        &self,
+        table_range_check_tag: Option<TableColumn>,
+        layouter: &mut impl Layouter<pallas::Base>,
+    ) -> Result<(), Error> {
+        match table_range_check_tag {
+            Some(tag) => self.load_with_tag(tag, layouter),
+            None => self.load_without_tag(layouter),
+        }
+    }
+    pub fn load_without_tag(
+        &self,
+        layouter: &mut impl Layouter<pallas::Base>,
+    ) -> Result<(), Error> {
         layouter.assign_table(
             || "generator_table",
             |mut table| {
diff --git a/halo2_gadgets/src/utilities/lookup_range_check.rs b/halo2_gadgets/src/utilities/lookup_range_check.rs
index cfb4505c0..890d96dec 100644
--- a/halo2_gadgets/src/utilities/lookup_range_check.rs
+++ b/halo2_gadgets/src/utilities/lookup_range_check.rs
@@ -189,10 +189,6 @@ impl<F: PrimeFieldBits, const K: usize> LookupRangeCheck45BConfig<F, K> {
 
         config
     }
-
-    pub(crate) fn table_range_check_tag(&self) -> TableColumn {
-        self.table_range_check_tag
-    }
 }
 
 impl<F: PrimeFieldBits, const K: usize> LookupRangeCheck<F, K> for LookupRangeCheck45BConfig<F, K> {
@@ -209,6 +205,10 @@ impl<F: PrimeFieldBits, const K: usize> LookupRangeCheck<F, K> for LookupRangeCh
         Self::configure_with_tag(meta, running_sum, table_idx, table_range_check_tag)
     }
 
+    fn table_range_check_tag(&self) -> Option<TableColumn> {
+        Some(self.table_range_check_tag)
+    }
+
     #[cfg(test)]
     // Fill `table_idx` and `table_range_check_tag`.
     // This is only used in testing for now, since the Sinsemilla chip provides a pre-loaded table
@@ -324,6 +324,9 @@ pub trait LookupRangeCheck<F: PrimeFieldBits, const K: usize> {
     where
         Self: Sized;
 
+    /// Returns the table column that contains the range check tag.
+    fn table_range_check_tag(&self) -> Option<TableColumn>;
+
     #[cfg(test)]
     // Fill `table_idx` and `table_range_check_tag`.
     // This is only used in testing for now, since the Sinsemilla chip provides a pre-loaded table
@@ -610,6 +613,10 @@ impl<F: PrimeFieldBits, const K: usize> LookupRangeCheck<F, K> for LookupRangeCh
         config
     }
 
+    fn table_range_check_tag(&self) -> Option<TableColumn> {
+        None
+    }
+
     #[cfg(test)]
     // Fill `table_idx` and `table_range_check_tag`.
     // This is only used in testing for now, since the Sinsemilla chip provides a pre-loaded table