Skip to content

Commit

Permalink
Fix LookupangeCheck loading in Sinsemilla
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
ConstanceBeguier committed Jun 25, 2024
1 parent 18e3e0d commit d6bcf3b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
9 changes: 4 additions & 5 deletions halo2_gadgets/src/sinsemilla/chip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 14 additions & 1 deletion halo2_gadgets/src/sinsemilla/chip/generator_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
15 changes: 11 additions & 4 deletions halo2_gadgets/src/utilities/lookup_range_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d6bcf3b

Please sign in to comment.