diff --git a/halo2-base/src/gates/flex_gate.rs b/halo2-base/src/gates/flex_gate.rs index cc24c76a..211b6cf5 100644 --- a/halo2-base/src/gates/flex_gate.rs +++ b/halo2-base/src/gates/flex_gate.rs @@ -1100,7 +1100,7 @@ impl GateInstructions for GateChip { /// /// Assumes `range_bits >= number of bits in a`. /// * `a`: [QuantumCell] of the value to convert - /// * `range_bits`: range of bits needed to represent `a` + /// * `range_bits`: range of bits needed to represent `a`. Assumes `range_bits > 0`. fn num_to_bits( &self, ctx: &mut Context, diff --git a/halo2-base/src/gates/range.rs b/halo2-base/src/gates/range.rs index eb553896..7a6b6173 100644 --- a/halo2-base/src/gates/range.rs +++ b/halo2-base/src/gates/range.rs @@ -441,8 +441,8 @@ pub struct RangeChip { pub gate: GateChip, /// Defines the number of bits represented in the lookup table [0,2lookup_bits). pub lookup_bits: usize, - /// [Vec] of 'limbs' represented as [QuantumCell] that divide the underlying scalar field element into sections smaller than lookup_bits. - /// * This allows range checks on field elements that are larger than the maximum value of the lookup table. + /// [Vec] of powers of `2 ** lookup_bits` represented as [QuantumCell::Constant]. + /// These are precomputed and cached as a performance optimization for later limb decompositions. We precompute up to the higher power that fits in `F`, which is `2 ** ((F::CAPACITY / lookup_bits) * lookup_bits)`. pub limb_bases: Vec>, } @@ -453,7 +453,7 @@ impl RangeChip { pub fn new(strategy: RangeStrategy, lookup_bits: usize) -> Self { let limb_base = F::from(1u64 << lookup_bits); let mut running_base = limb_base; - let num_bases = F::NUM_BITS as usize / lookup_bits; + let num_bases = F::CAPACITY as usize / lookup_bits; let mut limb_bases = Vec::with_capacity(num_bases + 1); limb_bases.extend([Constant(F::one()), Constant(running_base)]); for _ in 2..=num_bases { @@ -494,13 +494,16 @@ impl RangeInstructions for RangeChip { /// Checks and constrains that `a` lies in the range [0, 2range_bits). /// - /// This is done by decomposing `a` into `k` limbs, where `k = (range_bits + lookup_bits - 1) / lookup_bits`. + /// This is done by decomposing `a` into `k` limbs, where `k = ceil(range_bits / lookup_bits)`. /// Each limb is constrained to be within the range [0, 2lookup_bits). /// The limbs are then combined to form `a` again with the last limb having `rem_bits` number of bits. /// /// * `a`: [AssignedValue] value to be range checked /// * `range_bits`: number of bits in the range /// * `lookup_bits`: number of bits in the lookup table + /// + /// # Assumptions + /// * `ceil(range_bits / lookup_bits) * lookup_bits <= F::CAPACITY` fn range_check(&self, ctx: &mut Context, a: AssignedValue, range_bits: usize) { // the number of limbs let k = (range_bits + self.lookup_bits - 1) / self.lookup_bits; @@ -585,10 +588,13 @@ impl RangeInstructions for RangeChip { /// Constrains whether `a` is in `[0, b)`, and returns 1 if `a` < `b`, otherwise 0. /// - /// Assumes that`a` and `b` are known to have <= num_bits bits. /// * a: first [QuantumCell] to compare /// * b: second [QuantumCell] to compare /// * num_bits: number of bits to represent the values + /// + /// # Assumptions + /// * `a` and `b` are known to have `<= num_bits` bits. + /// * (`ceil(num_bits / lookup_bits) + 1) * lookup_bits <= F::CAPACITY` fn is_less_than( &self, ctx: &mut Context, diff --git a/halo2-base/src/utils.rs b/halo2-base/src/utils.rs index 979fc099..ea190e32 100644 --- a/halo2-base/src/utils.rs +++ b/halo2-base/src/utils.rs @@ -184,7 +184,8 @@ pub fn fe_to_biguint(fe: &F) -> BigUint { /// Converts a [BigPrimeField] element into a [BigInt] element by sending `fe` in `[0, F::modulus())` to /// ``` -/// fe < F::modulus() / 2 ? fe : fe - F::modulus() +/// fe, if fe < F::modulus() / 2 +/// fe - F::modulus(), otherwise /// ``` pub fn fe_to_bigint(fe: &F) -> BigInt { // TODO: `F` should just have modulus as lazy_static or something diff --git a/halo2-ecc/src/bigint/add_no_carry.rs b/halo2-ecc/src/bigint/add_no_carry.rs index e7d920a8..8ba736ea 100644 --- a/halo2-ecc/src/bigint/add_no_carry.rs +++ b/halo2-ecc/src/bigint/add_no_carry.rs @@ -1,25 +1,28 @@ use super::{CRTInteger, OverflowInteger}; use halo2_base::{gates::GateInstructions, utils::ScalarField, Context}; +use itertools::Itertools; use std::cmp::max; +/// # Assumptions +/// * `a, b` have same number of limbs pub fn assign( gate: &impl GateInstructions, ctx: &mut Context, a: &OverflowInteger, b: &OverflowInteger, ) -> OverflowInteger { - debug_assert_eq!(a.limbs.len(), b.limbs.len()); - let out_limbs = a .limbs .iter() - .zip(b.limbs.iter()) + .zip_eq(b.limbs.iter()) .map(|(&a_limb, &b_limb)| gate.add(ctx, a_limb, b_limb)) .collect(); OverflowInteger::construct(out_limbs, max(a.max_limb_bits, b.max_limb_bits) + 1) } +/// # Assumptions +/// * `a, b` have same number of limbs // pass by reference to avoid cloning the BigInt in CRTInteger, unclear if this is optimal pub fn crt( gate: &impl GateInstructions, @@ -27,7 +30,6 @@ pub fn crt( a: &CRTInteger, b: &CRTInteger, ) -> CRTInteger { - debug_assert_eq!(a.truncation.limbs.len(), b.truncation.limbs.len()); let out_trunc = assign::(gate, ctx, &a.truncation, &b.truncation); let out_native = gate.add(ctx, a.native, b.native); let out_val = &a.value + &b.value; diff --git a/halo2-ecc/src/bigint/big_is_equal.rs b/halo2-ecc/src/bigint/big_is_equal.rs index f64a3fae..281b0975 100644 --- a/halo2-ecc/src/bigint/big_is_equal.rs +++ b/halo2-ecc/src/bigint/big_is_equal.rs @@ -1,22 +1,25 @@ use super::{CRTInteger, OverflowInteger}; use halo2_base::{gates::GateInstructions, utils::ScalarField, AssignedValue, Context}; +use itertools::Itertools; /// Given OverflowInteger's `a` and `b` of the same shape, /// returns whether `a == b`. +/// +/// # Assumptions: +/// * `a, b` have the same number of limbs. +/// * The number of limbs is nonzero. pub fn assign( gate: &impl GateInstructions, ctx: &mut Context, a: &OverflowInteger, b: &OverflowInteger, ) -> AssignedValue { - let k = a.limbs.len(); - debug_assert_eq!(k, b.limbs.len()); - debug_assert_ne!(k, 0); + debug_assert!(!a.limbs.is_empty()); let mut a_limbs = a.limbs.iter(); let mut b_limbs = b.limbs.iter(); let mut partial = gate.is_equal(ctx, *a_limbs.next().unwrap(), *b_limbs.next().unwrap()); - for (&a_limb, &b_limb) in a_limbs.zip(b_limbs) { + for (&a_limb, &b_limb) in a_limbs.zip_eq(b_limbs) { let eq_limb = gate.is_equal(ctx, a_limb, b_limb); partial = gate.and(ctx, eq_limb, partial); } diff --git a/halo2-ecc/src/bigint/big_is_zero.rs b/halo2-ecc/src/bigint/big_is_zero.rs index d6b03cd5..f52fcc13 100644 --- a/halo2-ecc/src/bigint/big_is_zero.rs +++ b/halo2-ecc/src/bigint/big_is_zero.rs @@ -1,7 +1,10 @@ use super::{CRTInteger, OverflowInteger}; use halo2_base::{gates::GateInstructions, utils::ScalarField, AssignedValue, Context}; -/// assume you know that the limbs of `a` are all in [0, 2^{a.max_limb_bits}) +/// # Assumptions +/// * `a` has nonzero number of limbs +/// * The limbs of `a` are all in [0, 2a.max_limb_bits) +/// * a.limbs.len() * 2a.max_limb_bits ` is less than modulus of `F` pub fn positive( gate: &impl GateInstructions, ctx: &mut Context, @@ -15,14 +18,16 @@ pub fn positive( gate.is_zero(ctx, sum) } -// given OverflowInteger `a`, returns whether `a == 0` +/// Given OverflowInteger `a`, returns whether `a == 0` +/// +/// # Assumptions +/// * `a` has nonzero number of limbs pub fn assign( gate: &impl GateInstructions, ctx: &mut Context, a: &OverflowInteger, ) -> AssignedValue { - let k = a.limbs.len(); - debug_assert_ne!(k, 0); + debug_assert!(!a.limbs.is_empty()); let mut a_limbs = a.limbs.iter(); let mut partial = gate.is_zero(ctx, *a_limbs.next().unwrap()); diff --git a/halo2-ecc/src/bigint/carry_mod.rs b/halo2-ecc/src/bigint/carry_mod.rs index 4b266cf3..8ad48899 100644 --- a/halo2-ecc/src/bigint/carry_mod.rs +++ b/halo2-ecc/src/bigint/carry_mod.rs @@ -18,7 +18,10 @@ use std::{cmp::max, iter}; // `out.native = (a (mod modulus)) % (native_modulus::)` // We constrain `a = out + modulus * quotient` and range check `out` and `quotient` // -// Assumption: the leading two bits (in big endian) are 1, and `abs(a) <= 2^{n * k - 1 + F::NUM_BITS - 2}` (A weaker assumption is also enough, but this is good enough for forseeable use cases) +// Assumption: the leading two bits (in big endian) are 1, +/// # Assumptions +/// * abs(a) <= 2n * k - 1 + F::NUM_BITS - 2 (A weaker assumption is also enough, but this is good enough for forseeable use cases) +/// * `native_modulus::` requires *exactly* `k = a.limbs.len()` limbs to represent // This is currently optimized for limbs greater than 64 bits, so we need `F` to be a `BigPrimeField` // In the future we'll need a slightly different implementation for limbs that fit in 32 or 64 bits (e.g., `F` is Goldilocks) diff --git a/halo2-ecc/src/bigint/check_carry_mod_to_zero.rs b/halo2-ecc/src/bigint/check_carry_mod_to_zero.rs index db6f9084..b1f8c184 100644 --- a/halo2-ecc/src/bigint/check_carry_mod_to_zero.rs +++ b/halo2-ecc/src/bigint/check_carry_mod_to_zero.rs @@ -43,7 +43,7 @@ pub fn crt( // we need to constrain that `sum_i quot_vec[i] * 2^{n*i} = quot_native` in `F` let (quot_val, _out_val) = a.value.div_mod_floor(modulus); - // only perform safety checks in display mode so we can turn them off in production + // only perform safety checks in debug mode debug_assert_eq!(_out_val, BigInt::zero()); debug_assert!(quot_val.abs() < (BigInt::one() << quot_max_bits)); diff --git a/halo2-ecc/src/bigint/mul_no_carry.rs b/halo2-ecc/src/bigint/mul_no_carry.rs index b6d5e745..4b7e31ae 100644 --- a/halo2-ecc/src/bigint/mul_no_carry.rs +++ b/halo2-ecc/src/bigint/mul_no_carry.rs @@ -1,6 +1,11 @@ use super::{CRTInteger, OverflowInteger}; use halo2_base::{gates::GateInstructions, utils::ScalarField, Context, QuantumCell::Existing}; +/// # Assumptions +/// * `a` and `b` have the same number of limbs `k` +/// * `k` is nonzero +/// * `num_limbs_log2_ceil = log2_ceil(k)` +/// * `log2_ceil(k) + a.max_limb_bits + b.max_limb_bits <= F::NUM_BITS as usize - 2` pub fn truncate( gate: &impl GateInstructions, ctx: &mut Context, diff --git a/halo2-ecc/src/bigint/scalar_mul_and_add_no_carry.rs b/halo2-ecc/src/bigint/scalar_mul_and_add_no_carry.rs index 579aff01..56c3f59d 100644 --- a/halo2-ecc/src/bigint/scalar_mul_and_add_no_carry.rs +++ b/halo2-ecc/src/bigint/scalar_mul_and_add_no_carry.rs @@ -5,9 +5,15 @@ use halo2_base::{ Context, QuantumCell::Constant, }; +use itertools::Itertools; use std::cmp::max; /// compute a * c + b = b + a * c +/// +/// # Assumptions +/// * `a, b` have same number of limbs +/// * Number of limbs is nonzero +/// * `c_log2_ceil = log2_ceil(c)` where `c` is the BigUint value of `c_f` // this is uniquely suited for our simple gate pub fn assign( gate: &impl GateInstructions, @@ -17,12 +23,10 @@ pub fn assign( c_f: F, c_log2_ceil: usize, ) -> OverflowInteger { - debug_assert_eq!(a.limbs.len(), b.limbs.len()); - let out_limbs = a .limbs .iter() - .zip(b.limbs.iter()) + .zip_eq(b.limbs.iter()) .map(|(&a_limb, &b_limb)| gate.mul_add(ctx, a_limb, Constant(c_f), b_limb)) .collect(); diff --git a/halo2-ecc/src/bigint/select.rs b/halo2-ecc/src/bigint/select.rs index 1146eeb5..70ae3b67 100644 --- a/halo2-ecc/src/bigint/select.rs +++ b/halo2-ecc/src/bigint/select.rs @@ -1,7 +1,11 @@ use super::{CRTInteger, OverflowInteger}; use halo2_base::{gates::GateInstructions, utils::ScalarField, AssignedValue, Context}; +use itertools::Itertools; use std::cmp::max; +/// # Assumptions +/// * `a, b` have same number of limbs +/// * Number of limbs is nonzero pub fn assign( gate: &impl GateInstructions, ctx: &mut Context, @@ -9,11 +13,10 @@ pub fn assign( b: OverflowInteger, sel: AssignedValue, ) -> OverflowInteger { - debug_assert_eq!(a.limbs.len(), b.limbs.len()); let out_limbs = a .limbs .into_iter() - .zip(b.limbs.into_iter()) + .zip_eq(b.limbs) .map(|(a_limb, b_limb)| gate.select(ctx, a_limb, b_limb, sel)) .collect(); diff --git a/halo2-ecc/src/bigint/sub.rs b/halo2-ecc/src/bigint/sub.rs index 2d4d83ff..f72bfb85 100644 --- a/halo2-ecc/src/bigint/sub.rs +++ b/halo2-ecc/src/bigint/sub.rs @@ -5,8 +5,11 @@ use halo2_base::{ AssignedValue, Context, QuantumCell::{Constant, Existing, Witness}, }; +use itertools::Itertools; -/// Should only be called on integers a, b in proper representation with all limbs having at most `limb_bits` number of bits +/// # Assumptions +/// * Should only be called on integers a, b in proper representation with all limbs having at most `limb_bits` number of bits +/// * `a, b` have same nonzero number of limbs pub fn assign( range: &impl RangeInstructions, ctx: &mut Context, @@ -17,12 +20,11 @@ pub fn assign( ) -> (OverflowInteger, AssignedValue) { debug_assert!(a.max_limb_bits <= limb_bits); debug_assert!(b.max_limb_bits <= limb_bits); - debug_assert_eq!(a.limbs.len(), b.limbs.len()); let k = a.limbs.len(); let mut out_limbs = Vec::with_capacity(k); let mut borrow: Option> = None; - for (&a_limb, &b_limb) in a.limbs.iter().zip(b.limbs.iter()) { + for (&a_limb, &b_limb) in a.limbs.iter().zip_eq(b.limbs.iter()) { let (bottom, lt) = match borrow { None => { let lt = range.is_less_than(ctx, a_limb, b_limb, limb_bits); diff --git a/halo2-ecc/src/bigint/sub_no_carry.rs b/halo2-ecc/src/bigint/sub_no_carry.rs index ae4bb8a3..3f7c8f92 100644 --- a/halo2-ecc/src/bigint/sub_no_carry.rs +++ b/halo2-ecc/src/bigint/sub_no_carry.rs @@ -1,18 +1,20 @@ use super::{CRTInteger, OverflowInteger}; use halo2_base::{gates::GateInstructions, utils::ScalarField, Context}; +use itertools::Itertools; use std::cmp::max; +/// # Assumptions +/// * `a, b` have same number of limbs pub fn assign( gate: &impl GateInstructions, ctx: &mut Context, a: &OverflowInteger, b: &OverflowInteger, ) -> OverflowInteger { - debug_assert_eq!(a.limbs.len(), b.limbs.len()); let out_limbs = a .limbs .iter() - .zip(b.limbs.iter()) + .zip_eq(b.limbs.iter()) .map(|(&a_limb, &b_limb)| gate.sub(ctx, a_limb, b_limb)) .collect(); diff --git a/halo2-ecc/src/ecc/fixed_base.rs b/halo2-ecc/src/ecc/fixed_base.rs index 52e6634f..3b3bb58e 100644 --- a/halo2-ecc/src/ecc/fixed_base.rs +++ b/halo2-ecc/src/ecc/fixed_base.rs @@ -50,7 +50,7 @@ where } // computes `[scalar] * P` on y^2 = x^3 + b where `P` is fixed (constant) -// - `scalar` is represented as a reference array of `AssignedCell`s +// - `scalar` is represented as a non-empty reference array of `AssignedValue`s // - `scalar = sum_i scalar_i * 2^{max_bits * i}` // - an array of length > 1 is needed when `scalar` exceeds the modulus of scalar field `F` // assumes: @@ -261,6 +261,9 @@ where chip.sum::(ctx, sm.iter()) } +/// # Assumptions +/// * `points.len() = scalars.len()` +/// * `scalars[i].len() = scalars[j].len()` for all `i,j` pub fn msm_par( chip: &EccChip, builder: &mut GateThreadBuilder, diff --git a/halo2-ecc/src/ecc/pippenger.rs b/halo2-ecc/src/ecc/pippenger.rs index 58082c37..6da511af 100644 --- a/halo2-ecc/src/ecc/pippenger.rs +++ b/halo2-ecc/src/ecc/pippenger.rs @@ -143,7 +143,11 @@ where (acc, rand_point) } -/// Currently does not support if the final answer is actually the point at infinity +/// Currently does not support if the final answer is actually the point at infinity (meaning constraints will fail in that case) +/// +/// # Assumptions +/// * `points.len() == scalars.len()` +/// * `scalars[i].len() == scalars[j].len()` for all `i, j` pub fn multi_exp( chip: &FC, ctx: &mut Context, @@ -202,7 +206,11 @@ where /// Multi-thread witness generation for multi-scalar multiplication. /// Should give exact same circuit as `multi_exp`. /// -/// Currently does not support if the final answer is actually the point at infinity +/// Currently does not support if the final answer is actually the point at infinity (meaning constraints will fail in that case) +/// +/// # Assumptions +/// * `points.len() == scalars.len()` +/// * `scalars[i].len() == scalars[j].len()` for all `i, j` pub fn multi_exp_par( chip: &FC, // these are the "threads" within a single Phase diff --git a/halo2-ecc/src/fields/fp.rs b/halo2-ecc/src/fields/fp.rs index 1ae6e002..a1dc5ca1 100644 --- a/halo2-ecc/src/fields/fp.rs +++ b/halo2-ecc/src/fields/fp.rs @@ -290,6 +290,8 @@ impl<'range, F: PrimeField, Fp: PrimeField> FieldChip for FpChip<'range, F, F ) } + /// # Assumptions + /// * `max_bits` in `(n * (k - 1), n * k]` fn range_check( &self, ctx: &mut Context, @@ -301,7 +303,6 @@ impl<'range, F: PrimeField, Fp: PrimeField> FieldChip for FpChip<'range, F, F debug_assert!(max_bits > n * (k - 1) && max_bits <= n * k); let last_limb_bits = max_bits - n * (k - 1); - #[cfg(debug_assertions)] debug_assert!(a.value.bits() as usize <= max_bits); // range check limbs of `a` are in [0, 2^n) except last limb should be in [0, 2^last_limb_bits) diff --git a/hashes/zkevm-keccak/src/util/constraint_builder.rs b/hashes/zkevm-keccak/src/util/constraint_builder.rs index 94f47c8c..bae9f4a4 100644 --- a/hashes/zkevm-keccak/src/util/constraint_builder.rs +++ b/hashes/zkevm-keccak/src/util/constraint_builder.rs @@ -53,7 +53,7 @@ impl BaseConstraintBuilder { pub(crate) fn validate_degree(&self, degree: usize, name: &'static str) { if self.max_degree > 0 { - debug_assert!( + assert!( degree <= self.max_degree, "Expression {} degree too high: {} > {}", name,