From 6cf73391e8f4adf3c74be6ec6102039bf41be579 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 4 Jun 2021 03:41:06 +0100 Subject: [PATCH 1/3] Enable annotating individual constraints within gates The closure passed to `ConstraintSystem::create_gate` can now return: - Tuples of `(&'static str, Expression)` - Anything implementing `IntoIterator` (e.g. `Some(Expression)`) --- examples/simple-example.rs | 1 + examples/two-chip.rs | 1 + src/dev.rs | 20 ++++++++++++++--- src/plonk/circuit.rs | 45 +++++++++++++++++++++++++++++++++++--- 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/examples/simple-example.rs b/examples/simple-example.rs index c57ebf41..7f971b4f 100644 --- a/examples/simple-example.rs +++ b/examples/simple-example.rs @@ -364,6 +364,7 @@ fn main() { gate_index: 1, gate_name: "public input", constraint_index: 0, + constraint_name: "", row: 6, }]) ); diff --git a/examples/two-chip.rs b/examples/two-chip.rs index 23dea6fa..979504f0 100644 --- a/examples/two-chip.rs +++ b/examples/two-chip.rs @@ -631,6 +631,7 @@ fn main() { gate_index: 0, gate_name: "public input", constraint_index: 0, + constraint_name: "", row: 7, }]) ); diff --git a/src/dev.rs b/src/dev.rs index b1a904d6..0abf8bfb 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -58,6 +58,9 @@ pub enum VerifyFailure { /// from the closure passed to `ConstraintSystem::create_gate` during /// `Circuit::configure`. constraint_index: usize, + /// The name of the unsatisfied constraint. This is specified by the gate creator + /// (such as a chip implementation), and may not be unique. + constraint_name: &'static str, /// The row on which this constraint is not satisfied. row: usize, }, @@ -102,12 +105,21 @@ impl fmt::Display for VerifyFailure { gate_index, gate_name, constraint_index, + constraint_name, row, } => { write!( f, - "Constraint {} in gate {} ('{}') is not satisfied on row {}", - constraint_index, gate_index, gate_name, row + "Constraint {}{} in gate {} ('{}') is not satisfied on row {}", + constraint_index, + if constraint_name.is_empty() { + String::new() + } else { + format!(" ('{}')", constraint_name) + }, + gate_index, + gate_name, + row ) } Self::Lookup { lookup_index, row } => { @@ -175,7 +187,7 @@ impl fmt::Display for VerifyFailure { /// let c = meta.query_advice(c, Rotation::cur()); /// /// // BUG: Should be a * b - c -/// vec![a * b + c] +/// Some(("buggy R1CS", a * b + c)) /// }); /// /// MyConfig { a, b, c } @@ -212,6 +224,7 @@ impl fmt::Display for VerifyFailure { /// gate_index: 0, /// gate_name: "R1CS constraint", /// constraint_index: 0, +/// constraint_name: "buggy R1CS", /// row: 0 /// }]) /// ); @@ -501,6 +514,7 @@ impl MockProver { gate_index, gate_name: gate.name(), constraint_index: poly_index, + constraint_name: gate.constraint_name(poly_index), row: (row - n) as usize, }) } diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index 808277ec..4a3a56cf 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -486,9 +486,37 @@ impl>> From<(Col, Rotation)> for VirtualCell { } } +/// An individual polynomial constraint. +/// +/// These are returned by the closures passed to `ConstraintSystem::create_gate`. +#[derive(Debug)] +pub struct Constraint { + name: &'static str, + poly: Expression, +} + +impl From> for Constraint { + fn from(poly: Expression) -> Self { + Constraint { name: "", poly } + } +} + +impl From<(&'static str, Expression)> for Constraint { + fn from((name, poly): (&'static str, Expression)) -> Self { + Constraint { name, poly } + } +} + +impl From> for Vec> { + fn from(poly: Expression) -> Self { + vec![Constraint { name: "", poly }] + } +} + #[derive(Clone, Debug)] pub(crate) struct Gate { name: &'static str, + constraint_names: Vec<&'static str>, polys: Vec>, /// We track queried selectors separately from other cells, so that we can use them to /// trigger debug checks on gates. @@ -501,6 +529,10 @@ impl Gate { self.name } + pub(crate) fn constraint_name(&self, constraint_index: usize) -> &'static str { + self.constraint_names[constraint_index] + } + pub(crate) fn polynomials(&self) -> &[Expression] { &self.polys } @@ -727,18 +759,25 @@ impl ConstraintSystem { } /// Create a new gate - pub fn create_gate( + pub fn create_gate>, Iter: IntoIterator>( &mut self, name: &'static str, - f: impl FnOnce(&mut VirtualCells<'_, F>) -> Vec>, + f: impl FnOnce(&mut VirtualCells<'_, F>) -> Iter, ) { let mut cells = VirtualCells::new(self); - let polys = f(&mut cells); + let constraints = f(&mut cells); let queried_selectors = cells.queried_selectors; let queried_cells = cells.queried_cells; + let (constraint_names, polys) = constraints + .into_iter() + .map(|c| c.into()) + .map(|c| (c.name, c.poly)) + .unzip(); + self.gates.push(Gate { name, + constraint_names, polys, queried_selectors, queried_cells, From 876587c818ee5f9a0601d7a9d525e67bd398f647 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 4 Jun 2021 12:59:57 +0100 Subject: [PATCH 2/3] Panic in ConstraintSystem::create_gate if it would contain no constraints We use iterators to allow a gate to contain more than one constraint, but it is a programming error for a gate to not contain any constraints. --- src/plonk/circuit.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index 4a3a56cf..eb0d6e93 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -758,23 +758,33 @@ impl ConstraintSystem { } } - /// Create a new gate + /// Creates a new gate. + /// + /// # Panics + /// + /// A gate is required to contain polynomial constraints. This method will panic if + /// `constraints` returns an empty iterator. pub fn create_gate>, Iter: IntoIterator>( &mut self, name: &'static str, - f: impl FnOnce(&mut VirtualCells<'_, F>) -> Iter, + constraints: impl FnOnce(&mut VirtualCells<'_, F>) -> Iter, ) { let mut cells = VirtualCells::new(self); - let constraints = f(&mut cells); + let constraints = constraints(&mut cells); let queried_selectors = cells.queried_selectors; let queried_cells = cells.queried_cells; - let (constraint_names, polys) = constraints + let (constraint_names, polys): (_, Vec<_>) = constraints .into_iter() .map(|c| c.into()) .map(|c| (c.name, c.poly)) .unzip(); + assert!( + !polys.is_empty(), + "Gates must contain at least one constraint." + ); + self.gates.push(Gate { name, constraint_names, From 1dff93a8ad3d958412eb6f821cb557988f128eef Mon Sep 17 00:00:00 2001 From: str4d Date: Fri, 4 Jun 2021 23:12:55 +0100 Subject: [PATCH 3/3] dev: Update documentation for constraint_name Co-authored-by: Daira Hopwood --- src/dev.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dev.rs b/src/dev.rs index 0abf8bfb..97c0e782 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -59,7 +59,7 @@ pub enum VerifyFailure { /// `Circuit::configure`. constraint_index: usize, /// The name of the unsatisfied constraint. This is specified by the gate creator - /// (such as a chip implementation), and may not be unique. + /// (such as a chip implementation), and is not enforced to be unique. constraint_name: &'static str, /// The row on which this constraint is not satisfied. row: usize,