Skip to content

Commit

Permalink
Add support for Column annotations for MockProver debugging (#109)
Browse files Browse the repository at this point in the history
* feat: Add `name_column` to `Layouter` & `RegionLayouter`

This adds the trait-associated function `name_column` in order to enable
the possibility of the Layouter to store annotations aobut the colums.

This function does nothing for all the trait implementors (V1,
SimpleFloor, Assembly....) except for the `MockProver`. Which is
responsible of storing a map that links within a `Region` index, the
`column::Metadata` to the annotation `String`.

* feta: Update metadata/dbg structs to hold Col->Ann mapping

* feat: Update emitter module to print Column annotations

* feat: Add lookup column annotations

This adds the fn `annotate_lookup_column` for `ConstraintSystem` which
allows to carry annotations for the lookup columns declared for a
circuit within a CS.

* feat: Add Lookup TableColumn annotations

This allows to annotate lookup `TableColumn`s and print it's annotation
within the `assert_satisfied` fn.

This has required to change the `ConstraintSystem::lookup_annotations`
to have keys as `metadata::Column` rather than `usize` as otherwise it's
impossible within the `emitter` scope to distinguish between regular
advice columns (local to the Region) and fixed columns which come from
`TableColumn`s.

* fix: Customly derive PartialEq for metadata::Region

This allows to ignore the annotation map of the metadata::Region so that
is easier to match against `VerifyFailure` errors in tests.

* fix: Update ConstraintNotSatisfied testcase

* fix: Update Debug & Display for VerifyFailure

It was necessary to improve the `prover.verify` output also. To do so,
this required auxiliary types which are obfuscated to any other part of the lib
but that are necessary in order to be able to inject the Column names inside of
the `Column` section itself.
This also required to re-implement manually `Debug` and `Display` for this enum.

This closes zcash#705

* fix: Address clippy & warnings

* fix: Add final comments & polish

* fix: Resolve cherry-pick merge conflics & errors

* chore: Change DebugColumn visibility

* chore: Allow to fetch annotations from metadata

* chore: Fix clippy lints

* chore: Remove comments from code for testing

* feat: Add support for Advice and Instance anns in lookups

* feat: Allow `V1` layouter to annotate columns too

* fix: Support `Constant` & `Selector` for lookup exprs

* chore: Address review comments

* chore: Propagete write! result in `VerifyFailure::Display`

* chore: Address clippy lints
  • Loading branch information
CPerezz authored Jan 16, 2023
1 parent 4ad07d4 commit 54e4c57
Show file tree
Hide file tree
Showing 14 changed files with 841 additions and 76 deletions.
14 changes: 14 additions & 0 deletions halo2_proofs/src/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,20 @@ impl<'r, F: Field> Region<'r, F> {
.enable_selector(&|| annotation().into(), selector, offset)
}

/// Allows the circuit implementor to name/annotate a Column within a Region context.
///
/// This is useful in order to improve the amount of information that `prover.verify()`
/// and `prover.assert_satisfied()` can provide.
pub fn name_column<A, AR, T>(&mut self, annotation: A, column: T)
where
A: Fn() -> AR,
AR: Into<String>,
T: Into<Column<Any>>,
{
self.region
.name_column(&|| annotation().into(), column.into());
}

/// Assign an advice column value (witness).
///
/// Even though `to` has `FnMut` bounds, it is guaranteed to be called at most once.
Expand Down
8 changes: 8 additions & 0 deletions halo2_proofs/src/circuit/floor_planner/single_pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,14 @@ impl<'r, 'a, F: Field, CS: Assignment<F> + 'a> RegionLayouter<F>
)
}

fn name_column<'v>(
&'v mut self,
annotation: &'v (dyn Fn() -> String + 'v),
column: Column<Any>,
) {
self.layouter.cs.annotate_column(annotation, column);
}

fn assign_advice<'v>(
&'v mut self,
annotation: &'v (dyn Fn() -> String + 'v),
Expand Down
8 changes: 8 additions & 0 deletions halo2_proofs/src/circuit/floor_planner/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,14 @@ impl<'r, 'a, F: Field, CS: Assignment<F> + 'a> RegionLayouter<F> for V1Region<'r
Ok(())
}

fn name_column<'v>(
&'v mut self,
annotation: &'v (dyn Fn() -> String + 'v),
column: Column<Any>,
) {
self.plan.cs.annotate_column(annotation, column)
}

fn constrain_equal(&mut self, left: Cell, right: Cell) -> Result<(), Error> {
self.plan.cs.copy(
left.column,
Expand Down
18 changes: 18 additions & 0 deletions halo2_proofs/src/circuit/layouter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ pub trait RegionLayouter<F: Field>: fmt::Debug {
offset: usize,
) -> Result<(), Error>;

/// Allows the circuit implementor to name/annotate a Column within a Region context.
///
/// This is useful in order to improve the amount of information that `prover.verify()`
/// and `prover.assert_satisfied()` can provide.
fn name_column<'v>(
&'v mut self,
annotation: &'v (dyn Fn() -> String + 'v),
column: Column<Any>,
);

/// Assign an advice column value (witness)
fn assign_advice<'v>(
&'v mut self,
Expand Down Expand Up @@ -275,6 +285,14 @@ impl<F: Field> RegionLayouter<F> for RegionShape {
})
}

fn name_column<'v>(
&'v mut self,
_annotation: &'v (dyn Fn() -> String + 'v),
_column: Column<Any>,
) {
// Do nothing
}

fn constrain_constant(&mut self, _cell: Cell, _constant: Assigned<F>) -> Result<(), Error> {
// Global constants don't affect the region shape.
Ok(())
Expand Down
Loading

0 comments on commit 54e4c57

Please sign in to comment.