From 93ee7143fead3ee6dbe98b6db0fe8d954bfaf55c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 8 Dec 2021 02:27:21 +0000 Subject: [PATCH 1/8] `impl From<&Assigned> for Assigned` In zcash/halo2#383 we altered the bounds on region assignment methods like `Region::assign_advice` to constrain the value closure's result on `for<'vr> Assigned: From<&'vr VR>` instead of `VR: Into>`. This had the unintended side-effect that `Assigned` could no longer be returned from the closure, because we were previously relying on the implicit `impl From for T` provided by Rust, which no longer fits the bound. This commit adds the missing from-reference impl to restore functionality, re-enabling inversion deferrment. --- halo2_proofs/src/plonk/assigned.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/halo2_proofs/src/plonk/assigned.rs b/halo2_proofs/src/plonk/assigned.rs index 8bb002e5..d958fce8 100644 --- a/halo2_proofs/src/plonk/assigned.rs +++ b/halo2_proofs/src/plonk/assigned.rs @@ -17,6 +17,12 @@ pub enum Assigned { Rational(F, F), } +impl From<&Assigned> for Assigned { + fn from(val: &Assigned) -> Self { + *val + } +} + impl From<&F> for Assigned { fn from(numerator: &F) -> Self { Assigned::Trivial(*numerator) From 9d0e0b7be9279ef6cf8ccd0f23818a8eec0c09cc Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 8 Dec 2021 03:27:40 +0000 Subject: [PATCH 2/8] Add `AssignedCell, F>::evaluate()` method We don't want to provide a generic `map` function, since that would enable users to arbitrarily alter the value connected to a given cell. If a new value is being produced, that should either happen outside of the context of a cell (e.g. intermediate values from witness generation) or in the context of a newly-assigned cell. However, in the case of the `Assigned` type, we do need the ability to evaluate the deferred inversion in some cases (e.g. to then operate on the bits of the value). So for this `AssignedCell` specialization, we provide a pass-through `evaluate()` method that otherwise preserves the cell-value connection. --- halo2_proofs/src/circuit.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/halo2_proofs/src/circuit.rs b/halo2_proofs/src/circuit.rs index f2b1b130..eb2f7841 100644 --- a/halo2_proofs/src/circuit.rs +++ b/halo2_proofs/src/circuit.rs @@ -122,6 +122,20 @@ where } } +impl AssignedCell, F> { + /// Evaluates this assigned cell's value directly, performing an unbatched inversion + /// if necessary. + /// + /// If the denominator is zero, the returned cell's value is zero. + pub fn evaluate(self) -> AssignedCell { + AssignedCell { + value: self.value.map(|v| v.evaluate()), + cell: self.cell, + _marker: Default::default(), + } + } +} + impl AssignedCell where for<'v> Assigned: From<&'v V>, From 927463f76a0c019c08590ab1dd25948ebe3449e3 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 8 Dec 2021 03:34:57 +0000 Subject: [PATCH 3/8] Add `Assigned::is_zero_vartime` method --- halo2_proofs/src/plonk/assigned.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/halo2_proofs/src/plonk/assigned.rs b/halo2_proofs/src/plonk/assigned.rs index d958fce8..3c1795c1 100644 --- a/halo2_proofs/src/plonk/assigned.rs +++ b/halo2_proofs/src/plonk/assigned.rs @@ -152,6 +152,18 @@ impl Assigned { } } + /// Returns true iff this element is zero. + pub fn is_zero_vartime(&self) -> bool { + match self { + Self::Zero => true, + Self::Trivial(x) => x.is_zero_vartime(), + // Assigned maps x/0 -> 0. + Self::Rational(numerator, denominator) => { + numerator.is_zero_vartime() || denominator.is_zero_vartime() + } + } + } + /// Inverts this assigned value (taking the inverse of zero to be zero). pub fn invert(&self) -> Self { match self { From 8d00acace58a454c6356b20238f2c723791e94ee Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 8 Dec 2021 03:50:07 +0000 Subject: [PATCH 4/8] `impl Eq for Assigned` --- halo2_proofs/src/plonk/assigned.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/halo2_proofs/src/plonk/assigned.rs b/halo2_proofs/src/plonk/assigned.rs index 3c1795c1..67e7a560 100644 --- a/halo2_proofs/src/plonk/assigned.rs +++ b/halo2_proofs/src/plonk/assigned.rs @@ -41,6 +41,36 @@ impl From<(F, F)> for Assigned { } } +impl PartialEq for Assigned { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + // At least one side is directly zero. + (Self::Zero, Self::Zero) => true, + (Self::Zero, x) | (x, Self::Zero) => x.is_zero_vartime(), + + // One side is x/0 which maps to zero. + (Self::Rational(_, denominator), x) | (x, Self::Rational(_, denominator)) + if denominator.is_zero_vartime() => + { + x.is_zero_vartime() + } + + // Okay, we need to do some actual math... + (Self::Trivial(lhs), Self::Trivial(rhs)) => lhs == rhs, + (Self::Trivial(x), Self::Rational(numerator, denominator)) + | (Self::Rational(numerator, denominator), Self::Trivial(x)) => { + &(*x * denominator) == numerator + } + ( + Self::Rational(lhs_numerator, lhs_denominator), + Self::Rational(rhs_numerator, rhs_denominator), + ) => *lhs_numerator * rhs_denominator == *lhs_denominator * rhs_numerator, + } + } +} + +impl Eq for Assigned {} + impl Neg for Assigned { type Output = Assigned; fn neg(self) -> Self::Output { From 50b8e05913151e8a8629a941fc702b87624ae937 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 11 Dec 2021 00:40:29 +0000 Subject: [PATCH 5/8] Add other `Add*, Sub*, Mul*` variant impls to `Assigned` --- halo2_proofs/src/plonk/assigned.rs | 59 +++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/halo2_proofs/src/plonk/assigned.rs b/halo2_proofs/src/plonk/assigned.rs index 67e7a560..d6d8b852 100644 --- a/halo2_proofs/src/plonk/assigned.rs +++ b/halo2_proofs/src/plonk/assigned.rs @@ -1,4 +1,4 @@ -use std::ops::{Add, Mul, Neg, Sub}; +use std::ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign}; use group::ff::Field; @@ -121,6 +121,25 @@ impl Add for Assigned { } } +impl Add<&Assigned> for Assigned { + type Output = Assigned; + fn add(self, rhs: &Self) -> Assigned { + self + *rhs + } +} + +impl AddAssign for Assigned { + fn add_assign(&mut self, rhs: Self) { + *self = *self + rhs; + } +} + +impl AddAssign<&Assigned> for Assigned { + fn add_assign(&mut self, rhs: &Self) { + *self = *self + rhs; + } +} + impl Sub for Assigned { type Output = Assigned; fn sub(self, rhs: Assigned) -> Assigned { @@ -135,6 +154,25 @@ impl Sub for Assigned { } } +impl Sub<&Assigned> for Assigned { + type Output = Assigned; + fn sub(self, rhs: &Self) -> Assigned { + self - *rhs + } +} + +impl SubAssign for Assigned { + fn sub_assign(&mut self, rhs: Self) { + *self = *self - rhs; + } +} + +impl SubAssign<&Assigned> for Assigned { + fn sub_assign(&mut self, rhs: &Self) { + *self = *self - rhs; + } +} + impl Mul for Assigned { type Output = Assigned; fn mul(self, rhs: Assigned) -> Assigned { @@ -163,6 +201,25 @@ impl Mul for Assigned { } } +impl Mul<&Assigned> for Assigned { + type Output = Assigned; + fn mul(self, rhs: &Assigned) -> Assigned { + self * *rhs + } +} + +impl MulAssign for Assigned { + fn mul_assign(&mut self, rhs: Self) { + *self = *self * rhs; + } +} + +impl MulAssign<&Assigned> for Assigned { + fn mul_assign(&mut self, rhs: &Self) { + *self = *self * rhs; + } +} + impl Assigned { /// Returns the numerator. pub fn numerator(&self) -> F { From a7e45495cf33208892e71a367bd7489aee9f5d6e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 11 Dec 2021 00:48:20 +0000 Subject: [PATCH 6/8] Add `Assigned::{double, square, cube}` methods --- halo2_proofs/src/plonk/assigned.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/halo2_proofs/src/plonk/assigned.rs b/halo2_proofs/src/plonk/assigned.rs index d6d8b852..637d2a58 100644 --- a/halo2_proofs/src/plonk/assigned.rs +++ b/halo2_proofs/src/plonk/assigned.rs @@ -251,6 +251,36 @@ impl Assigned { } } + /// Doubles this element. + #[must_use] + pub fn double(&self) -> Self { + match self { + Self::Zero => Self::Zero, + Self::Trivial(x) => Self::Trivial(x.double()), + Self::Rational(numerator, denominator) => { + Self::Rational(numerator.double(), *denominator) + } + } + } + + /// Squares this element. + #[must_use] + pub fn square(&self) -> Self { + match self { + Self::Zero => Self::Zero, + Self::Trivial(x) => Self::Trivial(x.square()), + Self::Rational(numerator, denominator) => { + Self::Rational(numerator.square(), denominator.square()) + } + } + } + + /// Cubes this element. + #[must_use] + pub fn cube(&self) -> Self { + self.square() * self + } + /// Inverts this assigned value (taking the inverse of zero to be zero). pub fn invert(&self) -> Self { match self { From 05a4d26bea15b1c7850c20e929535dcd8449e1b7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 11 Dec 2021 01:24:59 +0000 Subject: [PATCH 7/8] Add unary operators to `Assigned` proptest --- halo2_proofs/src/plonk/assigned.rs | 160 ++++++++++++++++++++++++++--- 1 file changed, 143 insertions(+), 17 deletions(-) diff --git a/halo2_proofs/src/plonk/assigned.rs b/halo2_proofs/src/plonk/assigned.rs index 637d2a58..fcf6b2a3 100644 --- a/halo2_proofs/src/plonk/assigned.rs +++ b/halo2_proofs/src/plonk/assigned.rs @@ -389,26 +389,108 @@ mod tests { #[cfg(test)] mod proptests { use std::{ + cmp, convert::TryFrom, - ops::{Add, Mul, Sub}, + ops::{Add, Mul, Neg, Sub}, }; + use group::ff::Field; use pasta_curves::{arithmetic::FieldExt, Fp}; use proptest::{collection::vec, prelude::*, sample::select}; use super::Assigned; + trait UnaryOperand: Neg { + fn double(&self) -> Self; + fn square(&self) -> Self; + fn cube(&self) -> Self; + fn inv0(&self) -> Self; + } + + impl UnaryOperand for F { + fn double(&self) -> Self { + self.double() + } + + fn square(&self) -> Self { + self.square() + } + + fn cube(&self) -> Self { + self.cube() + } + + fn inv0(&self) -> Self { + self.invert().unwrap_or(F::zero()) + } + } + + impl UnaryOperand for Assigned { + fn double(&self) -> Self { + self.double() + } + + fn square(&self) -> Self { + self.square() + } + + fn cube(&self) -> Self { + self.cube() + } + + fn inv0(&self) -> Self { + self.invert() + } + } + #[derive(Clone, Debug)] - enum Operation { + enum UnaryOperator { + Neg, + Double, + Square, + Cube, + Inv0, + } + + const UNARY_OPERATORS: &[UnaryOperator] = &[ + UnaryOperator::Neg, + UnaryOperator::Double, + UnaryOperator::Square, + UnaryOperator::Cube, + UnaryOperator::Inv0, + ]; + + impl UnaryOperator { + fn apply(&self, a: F) -> F { + match self { + Self::Neg => -a, + Self::Double => a.double(), + Self::Square => a.square(), + Self::Cube => a.cube(), + Self::Inv0 => a.inv0(), + } + } + } + + trait BinaryOperand: Sized + Add + Sub + Mul {} + impl BinaryOperand for F {} + impl BinaryOperand for Assigned {} + + #[derive(Clone, Debug)] + enum BinaryOperator { Add, Sub, Mul, } - const OPERATIONS: &[Operation] = &[Operation::Add, Operation::Sub, Operation::Mul]; + const BINARY_OPERATORS: &[BinaryOperator] = &[ + BinaryOperator::Add, + BinaryOperator::Sub, + BinaryOperator::Mul, + ]; - impl Operation { - fn apply + Sub + Mul>(&self, a: F, b: F) -> F { + impl BinaryOperator { + fn apply(&self, a: F, b: F) -> F { match self { Self::Add => a + b, Self::Sub => a - b, @@ -417,6 +499,12 @@ mod proptests { } } + #[derive(Clone, Debug)] + enum Operator { + Unary(UnaryOperator), + Binary(BinaryOperator), + } + prop_compose! { /// Use narrow that can be easily reduced. fn arb_element()(val in any::()) -> Fp { @@ -440,15 +528,31 @@ mod proptests { } } + prop_compose! { + fn arb_operators(num_unary: usize, num_binary: usize)( + unary in vec(select(UNARY_OPERATORS), num_unary), + binary in vec(select(BINARY_OPERATORS), num_binary), + ) -> Vec { + unary.into_iter() + .map(Operator::Unary) + .chain(binary.into_iter().map(Operator::Binary)) + .collect() + } + } + prop_compose! { fn arb_testcase()( - num_operations in 1usize..5, + num_unary in 0usize..5, + num_binary in 0usize..5, )( values in vec( prop_oneof![Just(Assigned::Zero), arb_trivial(), arb_rational()], - num_operations + 1), - operations in vec(select(OPERATIONS), num_operations), - ) -> (Vec>, Vec) { + // Ensure that: + // - we have at least one value to apply unary operators to. + // - we can apply every binary operator pairwise sequentially. + cmp::max(if num_unary > 0 { 1 } else { 0 }, num_binary + 1)), + operations in arb_operators(num_unary, num_binary).prop_shuffle(), + ) -> (Vec>, Vec) { (values, operations) } } @@ -460,14 +564,36 @@ mod proptests { let elements: Vec<_> = values.iter().cloned().map(|v| v.evaluate()).collect(); // Apply the operations to both the deferred and evaluated values. - let deferred_result = { - let mut ops = operations.iter(); - values.into_iter().reduce(|a, b| ops.next().unwrap().apply(a, b)).unwrap() - }; - let evaluated_result = { - let mut ops = operations.iter(); - elements.into_iter().reduce(|a, b| ops.next().unwrap().apply(a, b)).unwrap() - }; + fn evaluate( + items: Vec, + operators: &[Operator], + ) -> F { + let mut ops = operators.iter(); + + // Process all binary operators. We are guaranteed to have exactly as many + // binary operators as we need calls to the reduction closure. + let mut res = items.into_iter().reduce(|mut a, b| loop { + match ops.next() { + Some(Operator::Unary(op)) => a = op.apply(a), + Some(Operator::Binary(op)) => break op.apply(a, b), + None => unreachable!(), + } + }).unwrap(); + + // Process any unary operators that weren't handled in the reduce() call + // above (either if we only had one item, or there were unary operators + // after the last binary operator). We are guaranteed to have no binary + // operators remaining at this point. + loop { + match ops.next() { + Some(Operator::Unary(op)) => res = op.apply(res), + Some(Operator::Binary(_)) => unreachable!(), + None => break res, + } + } + } + let deferred_result = evaluate(values, &operations); + let evaluated_result = evaluate(elements, &operations); // The two should be equal, i.e. deferred inversion should commute with the // list of operations. From b7944e5c40d282bd6999c10b95a282daff388549 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 11 Dec 2021 01:40:09 +0000 Subject: [PATCH 8/8] Make `Assigned::Zero` slightly less likely in `Assigned` proptest --- halo2_proofs/src/plonk/assigned.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/halo2_proofs/src/plonk/assigned.rs b/halo2_proofs/src/plonk/assigned.rs index fcf6b2a3..2216083e 100644 --- a/halo2_proofs/src/plonk/assigned.rs +++ b/halo2_proofs/src/plonk/assigned.rs @@ -522,7 +522,10 @@ mod proptests { /// Generates half of the denominators as zero to represent a deferred inversion. fn arb_rational()( numerator in arb_element(), - denominator in prop_oneof![Just(Fp::zero()), arb_element()], + denominator in prop_oneof![ + 1 => Just(Fp::zero()), + 2 => arb_element(), + ], ) -> Assigned { Assigned::Rational(numerator, denominator) } @@ -546,7 +549,11 @@ mod proptests { num_binary in 0usize..5, )( values in vec( - prop_oneof![Just(Assigned::Zero), arb_trivial(), arb_rational()], + prop_oneof![ + 1 => Just(Assigned::Zero), + 2 => arb_trivial(), + 2 => arb_rational(), + ], // Ensure that: // - we have at least one value to apply unary operators to. // - we can apply every binary operator pairwise sequentially.