Skip to content

Commit

Permalink
feat: improve Assignment API errors (#357)
Browse files Browse the repository at this point in the history 
* patch: use cell name in "Mockprover" assign functions

* feat: introduce new "AssignmentError" enum

* feat: use "AssignmentError" in "Mockprover" & "WitnessCollection"

* doc: update comments for "AssignmentError"
  • Loading branch information
@guorong009
guorong009 authored Sep 18, 2024
1 parent c84fbb8 commit 4cf2a70
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 52 deletions.
19 changes: 15 additions & 4 deletions halo2_frontend/src/circuit.rs
View file
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -154,7 +154,12 @@ impl<'a, F: Field> Assignment<F> for WitnessCollection<'a, F> {

fn query_instance(&self, column: Column<Instance>, row: usize) -> Result<Value<F>, 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
Expand All @@ -166,7 +171,7 @@ impl<'a, F: Field> Assignment<F> for WitnessCollection<'a, F> {

fn assign_advice<V, VR, A, AR>(
&mut self,
_: A,
desc: A,
column: Column<Advice>,
row: usize,
to: V,
Expand All @@ -184,7 +189,13 @@ impl<'a, F: Field> Assignment<F> 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
Expand Down
108 changes: 62 additions & 46 deletions halo2_frontend/src/dev.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::ops::{Add, Mul, Neg, Range};

use blake2b_simd::blake2b;

use crate::plonk::AssignmentError;
use crate::{
circuit,
plonk::{
Expand Down Expand Up @@ -399,7 +400,12 @@ impl<F: Field> Assignment<F> for MockProver<F> {
}
}

fn enable_selector<A, AR>(&mut self, _: A, selector: &Selector, row: usize) -> Result<(), Error>
fn enable_selector<A, AR>(
&mut self,
desc: A,
selector: &Selector,
row: usize,
) -> Result<(), Error>
where
A: FnOnce() -> AR,
AR: Into<String>,
Expand All @@ -408,13 +414,15 @@ impl<F: Field> Assignment<F> for MockProver<F> {
return Ok(());
}

assert!(
self.usable_rows.contains(&row),
"row={} not in usable_rows={:?}, k={}",
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).
Expand All @@ -436,13 +444,14 @@ impl<F: Field> Assignment<F> for MockProver<F> {
column: Column<Instance>,
row: usize,
) -> Result<circuit::Value<F>, Error> {
assert!(
self.usable_rows.contains(&row),
"row={}, usable_rows={:?}, k={}",
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
Expand All @@ -454,7 +463,7 @@ impl<F: Field> Assignment<F> for MockProver<F> {

fn assign_advice<V, VR, A, AR>(
&mut self,
_: A,
desc: A,
column: Column<Advice>,
row: usize,
to: V,
Expand All @@ -466,13 +475,15 @@ impl<F: Field> Assignment<F> for MockProver<F> {
AR: Into<String>,
{
if self.in_phase(FirstPhase) {
assert!(
self.usable_rows.contains(&row),
"row={}, usable_rows={:?}, k={}",
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);
Expand Down Expand Up @@ -507,7 +518,7 @@ impl<F: Field> Assignment<F> for MockProver<F> {

fn assign_fixed<V, VR, A, AR>(
&mut self,
_: A,
desc: A,
column: Column<Fixed>,
row: usize,
to: V,
Expand All @@ -522,13 +533,15 @@ impl<F: Field> Assignment<F> for MockProver<F> {
return Ok(());
}

assert!(
self.usable_rows.contains(&row),
"row={}, usable_rows={:?}, k={}",
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);
Expand Down Expand Up @@ -559,14 +572,16 @@ impl<F: Field> Assignment<F> for MockProver<F> {
return Ok(());
}

assert!(
self.usable_rows.contains(&left_row) && self.usable_rows.contains(&right_row),
"left_row={}, right_row={}, usable_rows={:?}, k={}",
left_row,
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)
Expand All @@ -582,13 +597,14 @@ impl<F: Field> Assignment<F> for MockProver<F> {
return Ok(());
}

assert!(
self.usable_rows.contains(&from_row),
"row={}, usable_rows={:?}, k={}",
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)?;
Expand Down
125 changes: 123 additions & 2 deletions halo2_frontend/src/plonk/error.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
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.
/// 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`].

Check warning on line 11 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / deploy 

unresolved link to `MockProver`

Check warning on line 11 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / deploy 

unresolved link to `WitnessCollection`

Check warning on line 11 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / Intra-doc links 

unresolved link to `MockProver`

Check warning on line 11 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / Intra-doc links 

unresolved link to `WitnessCollection`
/// The [`keygen`] process use the [`NotEnoughRowsAvailable`], since it is just enough.

Check warning on line 12 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / deploy 

unresolved link to `keygen`

Check warning on line 12 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / deploy 

unresolved link to `NotEnoughRowsAvailable`

Check warning on line 12 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / Intra-doc links 

unresolved link to `keygen`

Check warning on line 12 in halo2_frontend/src/plonk/error.rs

View workflow job for this annotation

GitHub Actions / Intra-doc links 

unresolved link to `NotEnoughRowsAvailable`
#[derive(Debug)]
pub enum Error {
/// This is an error that can occur during synthesis of the circuit, for
Expand All @@ -27,6 +32,8 @@ pub enum Error {
ColumnNotInPermutation(Column<Any>),
/// 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),
}
Expand Down Expand Up @@ -58,6 +65,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}"),
}
}
Expand Down Expand Up @@ -101,3 +109,116 @@ impl fmt::Display for TableError {
}
}
}

/// This is an error that could occur during `assign_advice`, `assign_fixed`, `copy`, etc.
#[derive(Debug)]
pub enum AssignmentError {
AssignAdvice {
desc: String,
col: Column<Any>,
row: usize,
usable_rows: (usize, usize),
k: u32,
},
AssignFixed {
desc: String,
col: Column<Any>,
row: usize,
usable_rows: (usize, usize),
k: u32,
},
EnableSelector {
desc: String,
selector: Selector,
row: usize,
usable_rows: (usize, usize),
k: u32,
},
QueryInstance {
col: Column<Any>,
row: usize,
usable_rows: (usize, usize),
k: u32,
},
Copy {
left_col: Column<Any>,
left_row: usize,
right_col: Column<Any>,
right_row: usize,
usable_rows: (usize, usize),
k: u32,
},
FillFromRow {
col: Column<Any>,
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,
),
}
}
}

0 comments on commit 4cf2a70

Please sign in to comment.