From 25fc3eebeede3af8974e6cb1a29922aa64eb503a Mon Sep 17 00:00:00 2001 From: guorong009 Date: Mon, 24 Jun 2024 17:23:39 +0800 Subject: [PATCH 1/4] patch: use cell name in "Mockprover" assign functions --- halo2_frontend/src/dev.rs | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/halo2_frontend/src/dev.rs b/halo2_frontend/src/dev.rs index 438648a18..8bd5a1571 100644 --- a/halo2_frontend/src/dev.rs +++ b/halo2_frontend/src/dev.rs @@ -399,7 +399,12 @@ impl Assignment for MockProver { } } - fn enable_selector(&mut self, _: A, selector: &Selector, row: usize) -> Result<(), Error> + fn enable_selector( + &mut self, + name: A, + selector: &Selector, + row: usize, + ) -> Result<(), Error> where A: FnOnce() -> AR, AR: Into, @@ -410,7 +415,9 @@ impl Assignment for MockProver { assert!( self.usable_rows.contains(&row), - "row={} not in usable_rows={:?}, k={}", + "cell `{}` enable error: column=Selector({}), row={} not in usable_rows={:?}, k={}", + name().into(), + selector.index(), row, self.usable_rows, self.k, @@ -438,7 +445,8 @@ impl Assignment for MockProver { ) -> Result, Error> { assert!( self.usable_rows.contains(&row), - "row={}, usable_rows={:?}, k={}", + "query_instance error: column=Instance({}), row={}, usable_rows={:?}, k={}", + column.index(), row, self.usable_rows, self.k, @@ -454,7 +462,7 @@ impl Assignment for MockProver { fn assign_advice( &mut self, - _: A, + name: A, column: Column, row: usize, to: V, @@ -468,7 +476,9 @@ impl Assignment for MockProver { if self.in_phase(FirstPhase) { assert!( self.usable_rows.contains(&row), - "row={}, usable_rows={:?}, k={}", + "cell `{}` assign error: column=Advice({}), row={}, usable_rows={:?}, k={}", + name().into(), + column.index(), row, self.usable_rows, self.k, @@ -507,7 +517,7 @@ impl Assignment for MockProver { fn assign_fixed( &mut self, - _: A, + name: A, column: Column, row: usize, to: V, @@ -524,7 +534,9 @@ impl Assignment for MockProver { assert!( self.usable_rows.contains(&row), - "row={}, usable_rows={:?}, k={}", + "cell `{}` assign error: column=Fixed({}), row={}, usable_rows={:?}, k={}", + name().into(), + column.index(), row, self.usable_rows, self.k, @@ -561,8 +573,12 @@ impl Assignment for MockProver { assert!( self.usable_rows.contains(&left_row) && self.usable_rows.contains(&right_row), - "left_row={}, right_row={}, usable_rows={:?}, k={}", + "copy error: left_column={:?}({}), left_row={}, right_column={:?}({}), right_row={}, usable_rows={:?}, k={}", + left_column.column_type(), + left_column.index(), left_row, + right_column.column_type(), + right_column.index(), right_row, self.usable_rows, self.k, @@ -584,7 +600,9 @@ impl Assignment for MockProver { assert!( self.usable_rows.contains(&from_row), - "row={}, usable_rows={:?}, k={}", + "fill_from_row error: column={:?}({}), from_row={}, usable_rows={:?}, k={}", + col.column_type(), + col.index(), from_row, self.usable_rows, self.k, From 752996f401d6efc17f12a38d0ae8e8a072eff720 Mon Sep 17 00:00:00 2001 From: guorong009 Date: Tue, 25 Jun 2024 16:51:01 +0800 Subject: [PATCH 2/4] feat: introduce new "AssignmentError" enum --- halo2_frontend/src/plonk/error.rs | 118 +++++++++++++++++++++++++++++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git a/halo2_frontend/src/plonk/error.rs b/halo2_frontend/src/plonk/error.rs index 18133f268..03670371f 100644 --- a/halo2_frontend/src/plonk/error.rs +++ b/halo2_frontend/src/plonk/error.rs @@ -1,7 +1,7 @@ use std::fmt; use super::TableColumn; -use crate::plonk::Column; +use crate::plonk::{Column, Selector}; use halo2_middleware::circuit::Any; /// This is an error that could occur during circuit synthesis. @@ -27,6 +27,8 @@ pub enum Error { ColumnNotInPermutation(Column), /// An error relating to a lookup table. TableError(TableError), + /// An error relating to an `Assignment`. + AssignmentError(AssignmentError), /// Generic error not covered by previous cases Other(String), } @@ -58,6 +60,7 @@ impl fmt::Display for Error { "Column {column:?} must be included in the permutation. Help: try applying `meta.enable_equalty` on the column", ), Error::TableError(error) => write!(f, "{error}"), + Error::AssignmentError(error) => write!(f, "{error}"), Error::Other(error) => write!(f, "Other: {error}"), } } @@ -101,3 +104,116 @@ impl fmt::Display for TableError { } } } + +/// This is an error that could occur during assign_*, copy, etc. +#[derive(Debug)] +pub enum AssignmentError { + AssignAdvice { + desc: String, + col: Column, + row: usize, + usable_rows: (usize, usize), + k: u32, + }, + AssignFixed { + desc: String, + col: Column, + row: usize, + usable_rows: (usize, usize), + k: u32, + }, + EnableSelector { + desc: String, + selector: Selector, + row: usize, + usable_rows: (usize, usize), + k: u32, + }, + QueryInstance { + col: Column, + row: usize, + usable_rows: (usize, usize), + k: u32, + }, + Copy { + left_col: Column, + left_row: usize, + right_col: Column, + right_row: usize, + usable_rows: (usize, usize), + k: u32, + }, + FillFromRow { + col: Column, + from_row: usize, + usable_rows: (usize, usize), + k: u32, + }, +} + +impl fmt::Display for AssignmentError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AssignmentError::AssignAdvice { desc, col, row, usable_rows:(start, end), k } => write!( + f, + "assign_advice `{}` error: column={:?}({}), row={}, usable_rows={}..{}, k={}", + desc, + col.column_type(), + col.index(), + row, + start, end, + k, + ), + AssignmentError::AssignFixed {desc, col, row, usable_rows: (start, end), k } => write!( + f, + "assign_fixed `{}` error: column={:?}({}), row={}, usable_rows={}..{}, k={}", + desc, + col.column_type(), + col.index(), + row, + start, end, + k, + ), + AssignmentError::EnableSelector { desc, selector, row, usable_rows: (start, end), k } => write!( + f, + "enable_selector `{}` error: column=Selector({:?}), row={}, usable_rows={}..{}, k={}", + desc, + selector.index(), + row, + start, end, + k, + ), + AssignmentError::QueryInstance { col, row, usable_rows:(start, end), k } => write!( + f, + "query_instance error: column={:?}({}), row={}, usable_rows={}..{}, k={}", + col.column_type, + col.index(), + row, + start, + end, + k, + ), + AssignmentError::Copy { left_col, left_row, right_col, right_row, usable_rows:(start, end), k } => write!( + f, + "copy error: left_column={:?}({}), left_row={}, right_column={:?}({}), right_row={}, usable_rows={}..{}, k={}", + left_col.column_type(), + left_col.index(), + left_row, + right_col.column_type(), + right_col.index(), + right_row, + start, end, + k, + ), + AssignmentError::FillFromRow { col, from_row, usable_rows:(start, end), k } => write!( + f, + "fill_from_row error: column={:?}({}), from_row={}, usable_rows={}..{}, k={}", + col.column_type(), + col.index(), + from_row, + start, end, + k, + ), + } + } +} From 2a21cbc3fc110eb770cc28a0809c6d91ade9c8da Mon Sep 17 00:00:00 2001 From: guorong009 Date: Tue, 25 Jun 2024 16:51:39 +0800 Subject: [PATCH 3/4] feat: use "AssignmentError" in "Mockprover" & "WitnessCollection" --- halo2_frontend/src/circuit.rs | 19 ++++-- halo2_frontend/src/dev.rs | 116 +++++++++++++++++----------------- 2 files changed, 72 insertions(+), 63 deletions(-) diff --git a/halo2_frontend/src/circuit.rs b/halo2_frontend/src/circuit.rs index 4f6737854..48cbc4c09 100644 --- a/halo2_frontend/src/circuit.rs +++ b/halo2_frontend/src/circuit.rs @@ -1,6 +1,6 @@ //! Traits and structs for implementing circuit components. -use crate::plonk; +use crate::plonk::{self, AssignmentError}; use crate::plonk::{ permutation, sealed::{self, SealedPhase}, @@ -154,7 +154,12 @@ impl<'a, F: Field> Assignment for WitnessCollection<'a, F> { fn query_instance(&self, column: Column, row: usize) -> Result, Error> { if !self.usable_rows.contains(&row) { - return Err(Error::not_enough_rows_available(self.k)); + return Err(Error::AssignmentError(AssignmentError::QueryInstance { + col: column.into(), + row, + usable_rows: (0, self.usable_rows.end), + k: self.k, + })); } self.instances @@ -166,7 +171,7 @@ impl<'a, F: Field> Assignment for WitnessCollection<'a, F> { fn assign_advice( &mut self, - _: A, + desc: A, column: Column, row: usize, to: V, @@ -184,7 +189,13 @@ impl<'a, F: Field> Assignment for WitnessCollection<'a, F> { } if !self.usable_rows.contains(&row) { - return Err(Error::not_enough_rows_available(self.k)); + return Err(Error::AssignmentError(AssignmentError::AssignAdvice { + desc: desc().into(), + col: column.into(), + row, + usable_rows: (0, self.usable_rows.end), + k: self.k, + })); } *self diff --git a/halo2_frontend/src/dev.rs b/halo2_frontend/src/dev.rs index 8bd5a1571..fe9b53779 100644 --- a/halo2_frontend/src/dev.rs +++ b/halo2_frontend/src/dev.rs @@ -7,6 +7,7 @@ use std::ops::{Add, Mul, Neg, Range}; use blake2b_simd::blake2b; +use crate::plonk::AssignmentError; use crate::{ circuit, plonk::{ @@ -401,7 +402,7 @@ impl Assignment for MockProver { fn enable_selector( &mut self, - name: A, + desc: A, selector: &Selector, row: usize, ) -> Result<(), Error> @@ -413,15 +414,15 @@ impl Assignment for MockProver { return Ok(()); } - assert!( - self.usable_rows.contains(&row), - "cell `{}` enable error: column=Selector({}), row={} not in usable_rows={:?}, k={}", - name().into(), - selector.index(), - row, - self.usable_rows, - self.k, - ); + if !self.usable_rows.contains(&row) { + return Err(Error::AssignmentError(AssignmentError::EnableSelector { + desc: desc().into(), + selector: *selector, + row, + usable_rows: (self.usable_rows.start, self.usable_rows.end), + k: self.k, + })); + } // Track that this selector was enabled. We require that all selectors are enabled // inside some region (i.e. no floating selectors). @@ -443,14 +444,14 @@ impl Assignment for MockProver { column: Column, row: usize, ) -> Result, Error> { - assert!( - self.usable_rows.contains(&row), - "query_instance error: column=Instance({}), row={}, usable_rows={:?}, k={}", - column.index(), - row, - self.usable_rows, - self.k, - ); + if !self.usable_rows.contains(&row) { + return Err(Error::AssignmentError(AssignmentError::QueryInstance { + col: column.into(), + row, + usable_rows: (self.usable_rows.start, self.usable_rows.end), + k: self.k, + })); + } Ok(self .instance @@ -462,7 +463,7 @@ impl Assignment for MockProver { fn assign_advice( &mut self, - name: A, + desc: A, column: Column, row: usize, to: V, @@ -474,15 +475,15 @@ impl Assignment for MockProver { AR: Into, { if self.in_phase(FirstPhase) { - assert!( - self.usable_rows.contains(&row), - "cell `{}` assign error: column=Advice({}), row={}, usable_rows={:?}, k={}", - name().into(), - column.index(), - row, - self.usable_rows, - self.k, - ); + if !self.usable_rows.contains(&row) { + return Err(Error::AssignmentError(AssignmentError::AssignAdvice { + desc: desc().into(), + col: column.into(), + row, + usable_rows: (self.usable_rows.start, self.usable_rows.end), + k: self.k, + })); + } if let Some(region) = self.current_region.as_mut() { region.update_extent(column.into(), row); @@ -517,7 +518,7 @@ impl Assignment for MockProver { fn assign_fixed( &mut self, - name: A, + desc: A, column: Column, row: usize, to: V, @@ -532,15 +533,15 @@ impl Assignment for MockProver { return Ok(()); } - assert!( - self.usable_rows.contains(&row), - "cell `{}` assign error: column=Fixed({}), row={}, usable_rows={:?}, k={}", - name().into(), - column.index(), - row, - self.usable_rows, - self.k, - ); + if !self.usable_rows.contains(&row) { + return Err(Error::AssignmentError(AssignmentError::AssignFixed { + desc: desc().into(), + col: column.into(), + row, + usable_rows: (self.usable_rows.start, self.usable_rows.end), + k: self.k, + })); + } if let Some(region) = self.current_region.as_mut() { region.update_extent(column.into(), row); @@ -571,18 +572,16 @@ impl Assignment for MockProver { return Ok(()); } - assert!( - self.usable_rows.contains(&left_row) && self.usable_rows.contains(&right_row), - "copy error: left_column={:?}({}), left_row={}, right_column={:?}({}), right_row={}, usable_rows={:?}, k={}", - left_column.column_type(), - left_column.index(), - left_row, - right_column.column_type(), - right_column.index(), - right_row, - self.usable_rows, - self.k, - ); + if !self.usable_rows.contains(&left_row) || !self.usable_rows.contains(&right_row) { + return Err(Error::AssignmentError(AssignmentError::Copy { + left_col: left_column, + left_row, + right_col: right_column, + right_row, + usable_rows: (self.usable_rows.start, self.usable_rows.end), + k: self.k, + })); + } self.permutation .copy(left_column, left_row, right_column, right_row) @@ -598,15 +597,14 @@ impl Assignment for MockProver { return Ok(()); } - assert!( - self.usable_rows.contains(&from_row), - "fill_from_row error: column={:?}({}), from_row={}, usable_rows={:?}, k={}", - col.column_type(), - col.index(), - from_row, - self.usable_rows, - self.k, - ); + if !self.usable_rows.contains(&from_row) { + return Err(Error::AssignmentError(AssignmentError::FillFromRow { + col: col.into(), + from_row, + usable_rows: (self.usable_rows.start, self.usable_rows.end), + k: self.k, + })); + } for row in self.usable_rows.clone().skip(from_row) { self.assign_fixed(|| "", col, row, || to)?; From d42c3bffbe62debfb0e05b991e8ec786e83617a3 Mon Sep 17 00:00:00 2001 From: guorong009 Date: Tue, 25 Jun 2024 17:05:28 +0800 Subject: [PATCH 4/4] doc: update comments for "AssignmentError" --- halo2_frontend/src/plonk/error.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/halo2_frontend/src/plonk/error.rs b/halo2_frontend/src/plonk/error.rs index 03670371f..89f2e9709 100644 --- a/halo2_frontend/src/plonk/error.rs +++ b/halo2_frontend/src/plonk/error.rs @@ -4,7 +4,12 @@ use super::TableColumn; use crate::plonk::{Column, Selector}; use halo2_middleware::circuit::Any; -/// This is an error that could occur during circuit synthesis. +/// This is an error that could occur during circuit synthesis. +/// +/// **NOTE**: [`AssignmentError`] is introduced to provide more debugging info +/// to developers when assigning witnesses to circuit cells. +/// Hence, they are used for [`MockProver`] and [`WitnessCollection`]. +/// The [`keygen`] process use the [`NotEnoughRowsAvailable`], since it is just enough. #[derive(Debug)] pub enum Error { /// This is an error that can occur during synthesis of the circuit, for @@ -105,7 +110,7 @@ impl fmt::Display for TableError { } } -/// This is an error that could occur during assign_*, copy, etc. +/// This is an error that could occur during `assign_advice`, `assign_fixed`, `copy`, etc. #[derive(Debug)] pub enum AssignmentError { AssignAdvice {