From dbfbcd7bc4902957f8865ca319b303e7dc51066a Mon Sep 17 00:00:00 2001 From: xiaodino Date: Mon, 27 Mar 2023 00:45:00 -0700 Subject: [PATCH 01/37] Return VerifyFailure::ConstraintNotSatisfied with offset --- ecdsa/src/ecdsa.rs | 79 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index d0deeef9..953b7200 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -5,11 +5,15 @@ use crate::maingate; use ecc::maingate::RegionCtx; use ecc::{AssignedPoint, EccConfig, GeneralEccChip}; use halo2::arithmetic::{CurveAffine, FieldExt}; +use halo2::dev::{VerifyFailure, FailureLocation}; use halo2::{circuit::Value, plonk::Error}; use integer::rns::Integer; use integer::{AssignedInteger, IntegerInstructions}; use maingate::{MainGateConfig, RangeConfig}; +use std::cell::RefCell; +use std::collections::HashMap; + #[derive(Clone, Debug)] pub struct EcdsaConfig { main_gate_config: MainGateConfig, @@ -97,28 +101,36 @@ impl, pk: &AssignedPublicKey, msg_hash: &AssignedInteger, + offsets: &mut HashMap, ) -> Result<(), Error> { let ecc_chip = self.ecc_chip(); let scalar_chip = ecc_chip.scalar_field_chip(); let base_chip = ecc_chip.base_field_chip(); + offsets.insert("Started at".to_string(), ctx.offset()); + // 1. check 0 < r, s < n // since `assert_not_zero` already includes a in-field check, we can just // call `assert_not_zero` + offsets.insert("1. check 0 < r, s < n".to_string(), ctx.offset()); scalar_chip.assert_not_zero(ctx, &sig.r)?; scalar_chip.assert_not_zero(ctx, &sig.s)?; // 2. w = s^(-1) (mod n) + offsets.insert("2. w = s^(-1) (mod n)".to_string(), ctx.offset()); let (s_inv, _) = scalar_chip.invert(ctx, &sig.s)?; // 3. u1 = m' * w (mod n) + offsets.insert("3. u1 = m' * w (mod n)".to_string(), ctx.offset()); let u1 = scalar_chip.mul(ctx, msg_hash, &s_inv)?; // 4. u2 = r * w (mod n) + offsets.insert("4. u2 = r * w (mod n)".to_string(), ctx.offset()); let u2 = scalar_chip.mul(ctx, &sig.r, &s_inv)?; // 5. compute Q = u1*G + u2*pk + offsets.insert("5. compute Q = u1*G + u2*pk".to_string(), ctx.offset()); let e_gen = ecc_chip.assign_point(ctx, Value::known(E::generator()))?; let g1 = ecc_chip.mul(ctx, &e_gen, &u1, 2)?; let g2 = ecc_chip.mul(ctx, &pk.point, &u2, 2)?; @@ -126,13 +138,17 @@ impl>, + _marker: PhantomData, } @@ -279,7 +303,10 @@ mod tests { point: pk_in_circuit, }; let msg_hash = scalar_chip.assign_integer(ctx, msg_hash, Range::Remainder)?; - ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash) + let mut my_dict: HashMap = HashMap::new(); + let response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, &mut my_dict); + *self.offsets.borrow_mut() = my_dict; + return response; }, )?; @@ -296,6 +323,23 @@ mod tests { big_to_fe(x_big) } + fn find_closest_key(offset: usize, hm: &RefCell>) -> Option { + let mut best_key = None; + let mut smallest_diff = std::usize::MAX; + + for (key, value) in hm.borrow().iter() { + if offset >= *value { + let diff = offset - *value; + if diff < smallest_diff { + best_key = Some(key.clone()); + smallest_diff = diff; + } + } + } + + best_key + } + fn run() { let g = C::generator(); @@ -336,14 +380,43 @@ mod tests { let aux_generator = C::CurveExt::random(OsRng).to_affine(); let circuit = TestCircuitEcdsaVerify:: { public_key: Value::known(public_key), - signature: Value::known((r, s)), + signature: Value::known((s, s)), msg_hash: Value::known(msg_hash), aux_generator, window_size: 2, ..Default::default() }; let instance = vec![vec![]]; - assert_eq!(mock_prover_verify(&circuit, instance), Ok(())); + // assert_eq!(mock_prover_verify(&circuit, instance), Ok(())); + match mock_prover_verify(&circuit, instance) { + Ok(_) => { + println!("ok"); + }, + Err(errors) => { + for error in errors { + match error { + VerifyFailure::ConstraintNotSatisfied {constraint, location, cell_values} => { + // println!("Constraint not satisfied: {:?}, location: {:?}, cell values: {:?}", constraint, location, cell_values); + let offsets = &circuit.offsets; + // println!("TestCircuitEcdsaVerify offsets {:?}", &offsets); + match location { + FailureLocation::InRegion { region: _, offset } => { + // handle constraint not satisfied error + let key = find_closest_key(offset, offsets); + println!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}. Constraint {:?}", offset, key); + }, + FailureLocation::OutsideRegion { row: _ } => { + // handle constraint not satisfied error at row level + }, + } + }, + _ => { + // Handle other error types here + } + } + } + } + } } use crate::curves::bn256::Fr as BnScalar; From 3e2a0d4fa0f8c26d667b1fbb949d75aa1d548f20 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Thu, 20 Apr 2023 04:47:59 -0700 Subject: [PATCH 02/37] update --- ecdsa/src/ecdsa.rs | 47 ++++++++++++---- integer/src/chip.rs | 49 ++++++++++++++++- integer/src/chip/assert_not_zero.rs | 85 ++++++++++++++++++++++++++++- integer/src/instructions.rs | 31 +++++++++++ maingate/src/instructions.rs | 19 ++++++- 5 files changed, 216 insertions(+), 15 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 953b7200..78aeccf6 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -8,8 +8,8 @@ use halo2::arithmetic::{CurveAffine, FieldExt}; use halo2::dev::{VerifyFailure, FailureLocation}; use halo2::{circuit::Value, plonk::Error}; use integer::rns::Integer; -use integer::{AssignedInteger, IntegerInstructions}; -use maingate::{MainGateConfig, RangeConfig}; +use integer::{AssignedInteger, IntegerInstructions, Range}; +use maingate::{AssignedCondition, MainGateConfig, RangeConfig}; use std::cell::RefCell; use std::collections::HashMap; @@ -102,20 +102,27 @@ impl, msg_hash: &AssignedInteger, offsets: &mut HashMap, - ) -> Result<(), Error> { + enable_skipping_invalid_signature: bool, + ) -> Result, Error> { let ecc_chip = self.ecc_chip(); let scalar_chip = ecc_chip.scalar_field_chip(); let base_chip = ecc_chip.base_field_chip(); offsets.insert("Started at".to_string(), ctx.offset()); - // 1. check 0 < r, s < n + // 1. check 0 < r, s < n, if r == 0 or s == 0 the signature is marked as invalid // since `assert_not_zero` already includes a in-field check, we can just // call `assert_not_zero` offsets.insert("1. check 0 < r, s < n".to_string(), ctx.offset()); - scalar_chip.assert_not_zero(ctx, &sig.r)?; - scalar_chip.assert_not_zero(ctx, &sig.s)?; + + let is_r_valid = scalar_chip.is_not_zero(ctx, &sig.r)?; + let is_s_valid = scalar_chip.is_not_zero(ctx, &sig.s)?; + let is_r_s_valid = scalar_chip.and(ctx, &is_r_valid, &is_s_valid)?; + + // println!("is_r_invalid {:?}", is_r_valid); + // println!("is_s_valid {:?}", is_s_valid); + // println!("is_invalid {:?}", is_r_s_invalid); // 2. w = s^(-1) (mod n) offsets.insert("2. w = s^(-1) (mod n)".to_string(), ctx.offset()); @@ -145,11 +152,27 @@ impl = HashMap::new(); - let response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, &mut my_dict); + let response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, &mut my_dict, false); *self.offsets.borrow_mut() = my_dict; return response; }, @@ -380,7 +403,9 @@ mod tests { let aux_generator = C::CurveExt::random(OsRng).to_affine(); let circuit = TestCircuitEcdsaVerify:: { public_key: Value::known(public_key), - signature: Value::known((s, s)), + + // Set the wrong value to test invalid signature. + signature: Value::known((r, -s)), msg_hash: Value::known(msg_hash), aux_generator, window_size: 2, @@ -423,7 +448,7 @@ mod tests { use crate::curves::pasta::{Fp as PastaFp, Fq as PastaFq}; use crate::curves::secp256k1::Secp256k1Affine as Secp256k1; run::(); - run::(); - run::(); + // run::(); + // run::(); } } diff --git a/integer/src/chip.rs b/integer/src/chip.rs index ac732fb7..710263f2 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -4,8 +4,8 @@ use super::{AssignedInteger, AssignedLimb, UnassignedInteger}; use crate::instructions::{IntegerInstructions, Range}; use crate::rns::{Common, Integer, Rns}; use halo2::arithmetic::FieldExt; -use halo2::plonk::Error; -use maingate::{halo2, AssignedCondition, AssignedValue, MainGateInstructions, RegionCtx}; +use halo2::{circuit::Value, plonk::Error}; +use maingate::{halo2, AssignedCondition, AssignedValue, MainGateInstructions, RegionCtx, Term}; use maingate::{MainGate, MainGateConfig}; use maingate::{RangeChip, RangeConfig}; @@ -376,6 +376,21 @@ impl, + a: &AssignedInteger, + b: &AssignedInteger, + ) -> Result, Error> { + let main_gate = self.main_gate(); + let mut one = main_gate.assign_value(ctx, Value::known(N::one()))?; + for idx in 0..NUMBER_OF_LIMBS { + let term_1 = main_gate.is_equal(ctx, a.limb(idx), b.limb(idx))?; + one = main_gate.mul(ctx, &term_1, &one)?; + } + Ok(one) + } + fn assert_not_equal( &self, ctx: &mut RegionCtx<'_, N>, @@ -398,6 +413,36 @@ impl, + a: &AssignedInteger, + ) -> Result, Error> { + let a = &self.reduce_if_limb_values_exceeds_reduced(ctx, a)?; + let a = &self.reduce_if_max_operand_value_exceeds(ctx, a)?; + self.is_not_zero_generic(ctx, a) + } + + fn and( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result, Error> { + let main_gate = self.main_gate(); + main_gate.and(ctx, a, b) + } + + fn is_nand( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result, Error> { + let main_gate = self.main_gate(); + main_gate.is_nand(ctx, a, b) + } + fn assert_zero( &self, ctx: &mut RegionCtx<'_, N>, diff --git a/integer/src/chip/assert_not_zero.rs b/integer/src/chip/assert_not_zero.rs index 2a596a6b..cb08f93c 100644 --- a/integer/src/chip/assert_not_zero.rs +++ b/integer/src/chip/assert_not_zero.rs @@ -1,7 +1,7 @@ use super::IntegerChip; use crate::{AssignedInteger, FieldExt}; use halo2::plonk::Error; -use maingate::{halo2, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term}; +use maingate::{halo2, AssignedCondition, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term}; use num_bigint::BigUint as big_uint; use std::convert::TryInto; @@ -89,4 +89,87 @@ impl, + a: &AssignedInteger, + ) -> Result, Error> { + let main_gate = self.main_gate(); + let one = N::one(); + + // Reduce result (r) is restricted to be less than 1 << + // wrong_modulus_bit_lenght, so we only need to assert r <> 0 and r <> + // wrong modulus. + let r = self.reduce_generic(ctx, a)?; + + // Sanity check. + // This algorithm requires that wrong modulus * 2 <= native modulus * 2 ^ + // bit_len_limb. + let two_pow_limb_bits_minus_1 = + big_uint::from(2u64).pow((BIT_LEN_LIMB - 1).try_into().unwrap()); + + let sanity_check = self.rns.wrong_modulus.clone() + <= self.rns.native_modulus.clone() * two_pow_limb_bits_minus_1; + + + // r = 0 <-> r % 2 ^ 64 = 0 /\ r % native_modulus = 0 + // r <> 0 <-> r % 2 ^ 64 <> 0 \/ r % native_modulus <> 0 + // r <> 0 <-> invert(r.limb(0)) \/ invert(r.native()) + let cond_zero_0 = main_gate.is_zero(ctx, r.limb(0))?; + let cond_zero_1 = main_gate.is_zero(ctx, r.native())?; + + // one of them might be succeeded, i.e. cond_zero_0 * cond_zero_1 = 0 + let cond_r_equal_0 = main_gate.is_nand(ctx, &cond_zero_0, &cond_zero_1)?; + + // Similar to 0, + // r = wrong_modulus <-> r % 2 ^ 64 = wrong_modulus % 2 ^ 64 /\ r % + // native_modulus = wrong_modulus % native_modulus r <> p <-> + // invert(r.limb(0) - wrong_modulus[0]) \/ invert(r.native() - + // wrong_modulus.native()) + let wrong_modulus = self.rns.wrong_modulus_decomposed; + let limb_diff = r.limbs[0].value().map(|value| value - wrong_modulus[0]); + let limb_diff = main_gate + .apply( + ctx, + [ + Term::Assigned(r.limb(0), one), + Term::Unassigned(limb_diff, -one), + Term::Zero, + Term::Zero, + Term::Zero, + ], + -wrong_modulus[0], + CombinationOptionCommon::OneLinerAdd.into(), + )? + .swap_remove(1); + + let native_diff = r + .native() + .value() + .map(|value| *value - self.rns.wrong_modulus_in_native_modulus); + let native_diff = main_gate + .apply( + ctx, + [ + Term::Assigned(r.native(), one), + Term::Unassigned(native_diff, -one), + Term::Zero, + Term::Zero, + Term::Zero, + ], + -self.rns.wrong_modulus_in_native_modulus, + CombinationOptionCommon::OneLinerAdd.into(), + )? + .swap_remove(1); + + let cond_wrong_0 = main_gate.is_zero(ctx, &limb_diff)?; + let cond_wrong_1 = main_gate.is_zero(ctx, &native_diff)?; + + // one of them might be succeeded, i.e. cond_zero_0 * cond_zero_1 = 0 + let cond_r_equal_wrong_modulus = main_gate.is_nand(ctx, &cond_wrong_0, &cond_wrong_1)?; + + main_gate.and(ctx, &cond_r_equal_0, &cond_r_equal_wrong_modulus) + + } } diff --git a/integer/src/instructions.rs b/integer/src/instructions.rs index f50cb951..c987aef6 100644 --- a/integer/src/instructions.rs +++ b/integer/src/instructions.rs @@ -215,6 +215,14 @@ pub trait IntegerInstructions< b: &AssignedInteger, ) -> Result<(), Error>; + /// Constraints that limbs of two [`AssignedInteger`] are equal. + fn is_strict_equal( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedInteger, + b: &AssignedInteger, + ) -> Result, Error>; + /// Constraints that two [`AssignedInteger`] are not equal. fn assert_not_equal( &self, @@ -230,6 +238,29 @@ pub trait IntegerInstructions< a: &AssignedInteger, ) -> Result<(), Error>; + /// Check constraints that an [`AssignedInteger`] is not equal to zero + fn is_not_zero( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedInteger, + ) -> Result, Error>; + + /// Constraints for AND + fn and( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result, Error>; + + /// Constraints for NAND + fn is_nand( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result, Error>; + /// Constraints that an [`AssignedInteger`] is equal to zero fn assert_zero( &self, diff --git a/maingate/src/instructions.rs b/maingate/src/instructions.rs index 45d43076..d7e8cc0c 100644 --- a/maingate/src/instructions.rs +++ b/maingate/src/instructions.rs @@ -865,8 +865,25 @@ pub trait MainGateInstructions: Chip { Ok(()) } + /// Assigns a new bit witness `r` to `0` if both given witneeses are not `0` + /// otherwise `1` + fn is_nand( + &self, + ctx: &mut RegionCtx<'_, F>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result, Error> { + Ok(self.apply( + ctx, + [Term::assigned_to_mul(a), Term::assigned_to_mul(b)], + F::zero(), + CombinationOptionCommon::OneLinerMul.into(), + )? + .swap_remove(2)) + } + /// Assigns a new witness `r` as: - /// `r = a * b` + /// `r = a + b` fn add( &self, ctx: &mut RegionCtx<'_, F>, From cd07cc750177e2e200c1c5f26d2f1473f211689c Mon Sep 17 00:00:00 2001 From: xiaodino Date: Thu, 20 Apr 2023 05:28:11 -0700 Subject: [PATCH 03/37] update --- ecdsa/src/ecdsa.rs | 23 ++++++----------------- integer/src/chip/assert_not_zero.rs | 1 - 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 78aeccf6..79bc6873 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -120,10 +120,6 @@ impl { // handle constraint not satisfied error let key = find_closest_key(offset, offsets); - println!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}. Constraint {:?}", offset, key); + panic!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}. Constraint {:?}", offset, key); }, FailureLocation::OutsideRegion { row: _ } => { // handle constraint not satisfied error at row level diff --git a/integer/src/chip/assert_not_zero.rs b/integer/src/chip/assert_not_zero.rs index cb08f93c..3d47ac6b 100644 --- a/integer/src/chip/assert_not_zero.rs +++ b/integer/src/chip/assert_not_zero.rs @@ -111,7 +111,6 @@ impl r % 2 ^ 64 = 0 /\ r % native_modulus = 0 // r <> 0 <-> r % 2 ^ 64 <> 0 \/ r % native_modulus <> 0 From 76649f719f9af236cf88cbd1fda34580316d899e Mon Sep 17 00:00:00 2001 From: xiaodino Date: Thu, 20 Apr 2023 05:30:16 -0700 Subject: [PATCH 04/37] Update --- ecdsa/src/ecdsa.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 79bc6873..6c79d115 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -155,7 +155,7 @@ impl(); - // run::(); - // run::(); + run::(); + run::(); } } From d3c2b3f131f1ada671fdf8b76a73f46e75673a5e Mon Sep 17 00:00:00 2001 From: xiaodino Date: Fri, 21 Apr 2023 00:14:44 -0700 Subject: [PATCH 05/37] Update is_nand --- ecdsa/src/ecdsa.rs | 9 +++------ integer/src/chip/assert_not_zero.rs | 3 +-- maingate/src/instructions.rs | 9 ++------- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 6c79d115..8bedcaff 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -108,14 +108,11 @@ impl(); - run::(); - run::(); + // run::(); + // run::(); } } diff --git a/integer/src/chip/assert_not_zero.rs b/integer/src/chip/assert_not_zero.rs index 3d47ac6b..66cc087b 100644 --- a/integer/src/chip/assert_not_zero.rs +++ b/integer/src/chip/assert_not_zero.rs @@ -1,6 +1,6 @@ use super::IntegerChip; use crate::{AssignedInteger, FieldExt}; -use halo2::plonk::Error; +use halo2::{circuit::Value, plonk::Error}; use maingate::{halo2, AssignedCondition, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term}; use num_bigint::BigUint as big_uint; use std::convert::TryInto; @@ -169,6 +169,5 @@ impl: Chip { a: &AssignedCondition, b: &AssignedCondition, ) -> Result, Error> { - Ok(self.apply( - ctx, - [Term::assigned_to_mul(a), Term::assigned_to_mul(b)], - F::zero(), - CombinationOptionCommon::OneLinerMul.into(), - )? - .swap_remove(2)) + let and_a_b = self.and(ctx, a, b)?; + self.not(ctx, &and_a_b) } /// Assigns a new witness `r` as: From ba206aee8932c6ae4b63565e3c937e4d8d666906 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Sat, 22 Apr 2023 00:17:02 -0700 Subject: [PATCH 06/37] Update tests --- ecdsa/src/ecdsa.rs | 55 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 8bedcaff..611d9a81 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -189,6 +189,7 @@ mod tests { use std::collections::HashMap; use std::fmt::{self, Debug}; use std::marker::PhantomData; + use std::fmt::Error as fmt_Error; const BIT_LEN_LIMB: usize = 68; const NUMBER_OF_LIMBS: usize = 4; @@ -245,6 +246,7 @@ mod tests { aux_generator: E, window_size: usize, + enable_skipping_invalid_signature: bool, offsets: RefCell>, _marker: PhantomData, @@ -313,7 +315,7 @@ mod tests { }; let msg_hash = scalar_chip.assign_integer(ctx, msg_hash, Range::Remainder)?; let mut my_dict: HashMap = HashMap::new(); - let response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, &mut my_dict, false); + let response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, &mut my_dict, self.enable_skipping_invalid_signature); *self.offsets.borrow_mut() = my_dict; return response; }, @@ -349,7 +351,7 @@ mod tests { best_key } - fn run() { + fn generate_valid_inputs() -> (C, C::Scalar, C::Scalar, C::Scalar) { let g = C::generator(); // Generate a key pair @@ -386,15 +388,29 @@ mod tests { assert_eq!(r, r_candidate); } + (public_key, r, s, msg_hash) + } + + fn generate_invalid_inputs() -> (C, C::Scalar, C::Scalar, C::Scalar) { + let (public_key, r, s, msg_hash) = generate_valid_inputs::(); + (public_key, -r, s, msg_hash) + } + + fn run(valid_input: bool, enable_skipping_invalid_signature: bool) { + let (public_key, r, s, msg_hash) = if valid_input { + generate_valid_inputs::() + } else { + generate_invalid_inputs::() + }; + let aux_generator = C::CurveExt::random(OsRng).to_affine(); let circuit = TestCircuitEcdsaVerify:: { public_key: Value::known(public_key), - - // Set the wrong value to test invalid signature. - signature: Value::known((-r, s)), - msg_hash: Value::known(msg_hash), + signature: Value::known((r, s)), + msg_hash:Value::known(msg_hash), aux_generator, window_size: 2, + enable_skipping_invalid_signature, ..Default::default() }; let instance = vec![vec![]]; @@ -406,15 +422,16 @@ mod tests { Err(errors) => { for error in errors { match error { - VerifyFailure::ConstraintNotSatisfied {constraint, location, cell_values} => { - // println!("Constraint not satisfied: {:?}, location: {:?}, cell values: {:?}", constraint, location, cell_values); + VerifyFailure::ConstraintNotSatisfied {constraint: _, location, cell_values: _} => { let offsets = &circuit.offsets; // println!("TestCircuitEcdsaVerify offsets {:?}", &offsets); match location { FailureLocation::InRegion { region: _, offset } => { // handle constraint not satisfied error let key = find_closest_key(offset, offsets); - panic!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}. Constraint {:?}", offset, key); + if !enable_skipping_invalid_signature { + println!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}. Constraint {:?}", offset, key); + } }, FailureLocation::OutsideRegion { row: _ } => { // handle constraint not satisfied error at row level @@ -433,8 +450,22 @@ mod tests { use crate::curves::bn256::Fr as BnScalar; use crate::curves::pasta::{Fp as PastaFp, Fq as PastaFq}; use crate::curves::secp256k1::Secp256k1Affine as Secp256k1; - run::(); - // run::(); - // run::(); + + // Return Errors + run::(false, false); + run::(false, false); + run::(false, false); + + run::(false, true); + run::(false, true); + run::(false, true); + + run::(true, false); + run::(true, false); + run::(true, false); + + run::(true, true); + run::(true, true); + run::(true, true); } } From fafe199a0d68cf96d00ca19d613ef187059640a6 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Mon, 24 Apr 2023 16:48:43 -0700 Subject: [PATCH 07/37] Refactor: --- integer/src/chip.rs | 8 ++- integer/src/chip/assert_not_zero.rs | 84 +---------------------------- 2 files changed, 9 insertions(+), 83 deletions(-) diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 710263f2..02fe50ef 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -5,9 +5,11 @@ use crate::instructions::{IntegerInstructions, Range}; use crate::rns::{Common, Integer, Rns}; use halo2::arithmetic::FieldExt; use halo2::{circuit::Value, plonk::Error}; +use maingate::halo2::circuit::Chip; use maingate::{halo2, AssignedCondition, AssignedValue, MainGateInstructions, RegionCtx, Term}; use maingate::{MainGate, MainGateConfig}; use maingate::{RangeChip, RangeConfig}; +use num_bigint::BigUint as big_uint; mod add; mod assert_in_field; @@ -420,7 +422,11 @@ impl Result, Error> { let a = &self.reduce_if_limb_values_exceeds_reduced(ctx, a)?; let a = &self.reduce_if_max_operand_value_exceeds(ctx, a)?; - self.is_not_zero_generic(ctx, a) + + let main_gate = self.main_gate(); + let zero = self.assign_constant(ctx, W::zero())?; + let is_strict_equal_zero = self.is_strict_equal(ctx, &zero, &a)?; + main_gate.not(ctx, &is_strict_equal_zero) } fn and( diff --git a/integer/src/chip/assert_not_zero.rs b/integer/src/chip/assert_not_zero.rs index 66cc087b..e6bc1dd6 100644 --- a/integer/src/chip/assert_not_zero.rs +++ b/integer/src/chip/assert_not_zero.rs @@ -1,7 +1,7 @@ use super::IntegerChip; use crate::{AssignedInteger, FieldExt}; -use halo2::{circuit::Value, plonk::Error}; -use maingate::{halo2, AssignedCondition, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term}; +use halo2::plonk::Error; +use maingate::{halo2, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term}; use num_bigint::BigUint as big_uint; use std::convert::TryInto; @@ -90,84 +90,4 @@ impl, - a: &AssignedInteger, - ) -> Result, Error> { - let main_gate = self.main_gate(); - let one = N::one(); - - // Reduce result (r) is restricted to be less than 1 << - // wrong_modulus_bit_lenght, so we only need to assert r <> 0 and r <> - // wrong modulus. - let r = self.reduce_generic(ctx, a)?; - - // Sanity check. - // This algorithm requires that wrong modulus * 2 <= native modulus * 2 ^ - // bit_len_limb. - let two_pow_limb_bits_minus_1 = - big_uint::from(2u64).pow((BIT_LEN_LIMB - 1).try_into().unwrap()); - - let sanity_check = self.rns.wrong_modulus.clone() - <= self.rns.native_modulus.clone() * two_pow_limb_bits_minus_1; - - // r = 0 <-> r % 2 ^ 64 = 0 /\ r % native_modulus = 0 - // r <> 0 <-> r % 2 ^ 64 <> 0 \/ r % native_modulus <> 0 - // r <> 0 <-> invert(r.limb(0)) \/ invert(r.native()) - let cond_zero_0 = main_gate.is_zero(ctx, r.limb(0))?; - let cond_zero_1 = main_gate.is_zero(ctx, r.native())?; - - // one of them might be succeeded, i.e. cond_zero_0 * cond_zero_1 = 0 - let cond_r_equal_0 = main_gate.is_nand(ctx, &cond_zero_0, &cond_zero_1)?; - - // Similar to 0, - // r = wrong_modulus <-> r % 2 ^ 64 = wrong_modulus % 2 ^ 64 /\ r % - // native_modulus = wrong_modulus % native_modulus r <> p <-> - // invert(r.limb(0) - wrong_modulus[0]) \/ invert(r.native() - - // wrong_modulus.native()) - let wrong_modulus = self.rns.wrong_modulus_decomposed; - let limb_diff = r.limbs[0].value().map(|value| value - wrong_modulus[0]); - let limb_diff = main_gate - .apply( - ctx, - [ - Term::Assigned(r.limb(0), one), - Term::Unassigned(limb_diff, -one), - Term::Zero, - Term::Zero, - Term::Zero, - ], - -wrong_modulus[0], - CombinationOptionCommon::OneLinerAdd.into(), - )? - .swap_remove(1); - - let native_diff = r - .native() - .value() - .map(|value| *value - self.rns.wrong_modulus_in_native_modulus); - let native_diff = main_gate - .apply( - ctx, - [ - Term::Assigned(r.native(), one), - Term::Unassigned(native_diff, -one), - Term::Zero, - Term::Zero, - Term::Zero, - ], - -self.rns.wrong_modulus_in_native_modulus, - CombinationOptionCommon::OneLinerAdd.into(), - )? - .swap_remove(1); - - let cond_wrong_0 = main_gate.is_zero(ctx, &limb_diff)?; - let cond_wrong_1 = main_gate.is_zero(ctx, &native_diff)?; - - // one of them might be succeeded, i.e. cond_zero_0 * cond_zero_1 = 0 - let cond_r_equal_wrong_modulus = main_gate.is_nand(ctx, &cond_wrong_0, &cond_wrong_1)?; - - main_gate.and(ctx, &cond_r_equal_0, &cond_r_equal_wrong_modulus) - } } From f625481ac35161add8bfdbe412e385df96f6282f Mon Sep 17 00:00:00 2001 From: xiaodino Date: Mon, 24 Apr 2023 16:59:31 -0700 Subject: [PATCH 08/37] Remove debug code --- ecdsa/src/ecdsa.rs | 47 +++------------------------------------------- 1 file changed, 3 insertions(+), 44 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 611d9a81..fd046426 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -5,15 +5,11 @@ use crate::maingate; use ecc::maingate::RegionCtx; use ecc::{AssignedPoint, EccConfig, GeneralEccChip}; use halo2::arithmetic::{CurveAffine, FieldExt}; -use halo2::dev::{VerifyFailure, FailureLocation}; use halo2::{circuit::Value, plonk::Error}; use integer::rns::Integer; -use integer::{AssignedInteger, IntegerInstructions, Range}; +use integer::{AssignedInteger, IntegerInstructions}; use maingate::{AssignedCondition, MainGateConfig, RangeConfig}; -use std::cell::RefCell; -use std::collections::HashMap; - #[derive(Clone, Debug)] pub struct EcdsaConfig { main_gate_config: MainGateConfig, @@ -101,7 +97,6 @@ impl, pk: &AssignedPublicKey, msg_hash: &AssignedInteger, - offsets: &mut HashMap, enable_skipping_invalid_signature: bool, ) -> Result, Error> { let ecc_chip = self.ecc_chip(); @@ -112,25 +107,20 @@ impl>, _marker: PhantomData, } @@ -314,9 +297,7 @@ mod tests { point: pk_in_circuit, }; let msg_hash = scalar_chip.assign_integer(ctx, msg_hash, Range::Remainder)?; - let mut my_dict: HashMap = HashMap::new(); - let response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, &mut my_dict, self.enable_skipping_invalid_signature); - *self.offsets.borrow_mut() = my_dict; + let response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, self.enable_skipping_invalid_signature); return response; }, )?; @@ -334,23 +315,6 @@ mod tests { big_to_fe(x_big) } - fn find_closest_key(offset: usize, hm: &RefCell>) -> Option { - let mut best_key = None; - let mut smallest_diff = std::usize::MAX; - - for (key, value) in hm.borrow().iter() { - if offset >= *value { - let diff = offset - *value; - if diff < smallest_diff { - best_key = Some(key.clone()); - smallest_diff = diff; - } - } - } - - best_key - } - fn generate_valid_inputs() -> (C, C::Scalar, C::Scalar, C::Scalar) { let g = C::generator(); @@ -423,15 +387,10 @@ mod tests { for error in errors { match error { VerifyFailure::ConstraintNotSatisfied {constraint: _, location, cell_values: _} => { - let offsets = &circuit.offsets; - // println!("TestCircuitEcdsaVerify offsets {:?}", &offsets); match location { FailureLocation::InRegion { region: _, offset } => { // handle constraint not satisfied error - let key = find_closest_key(offset, offsets); - if !enable_skipping_invalid_signature { - println!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}. Constraint {:?}", offset, key); - } + println!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}", offset); }, FailureLocation::OutsideRegion { row: _ } => { // handle constraint not satisfied error at row level From aba38c4abc15fcee7fd4c2b973017aa848cd4ac6 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Tue, 25 Apr 2023 18:52:27 -0700 Subject: [PATCH 09/37] Fix a bug --- ecdsa/src/ecdsa.rs | 7 +++---- integer/src/chip.rs | 3 ++- integer/src/chip/assert_not_zero.rs | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index fd046426..b2e6d161 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -170,8 +170,6 @@ mod tests { use maingate::{MainGate, MainGateConfig, RangeChip, RangeConfig, RangeInstructions}; use rand_core::OsRng; - use std::cell::RefCell; - use std::collections::HashMap; use std::fmt::{self, Debug}; use std::marker::PhantomData; @@ -356,8 +354,9 @@ mod tests { } fn generate_invalid_inputs() -> (C, C::Scalar, C::Scalar, C::Scalar) { - let (public_key, r, s, msg_hash) = generate_valid_inputs::(); - (public_key, -r, s, msg_hash) + let (public_key, _, s, msg_hash) = generate_valid_inputs::(); + // Set the value of r incorrectly + (public_key, s, s, msg_hash) } fn run(valid_input: bool, enable_skipping_invalid_signature: bool) { diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 02fe50ef..73ba2c77 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -385,10 +385,11 @@ impl, ) -> Result, Error> { let main_gate = self.main_gate(); + let zero = main_gate.assign_value(ctx, Value::known(N::zero()))?; let mut one = main_gate.assign_value(ctx, Value::known(N::one()))?; for idx in 0..NUMBER_OF_LIMBS { let term_1 = main_gate.is_equal(ctx, a.limb(idx), b.limb(idx))?; - one = main_gate.mul(ctx, &term_1, &one)?; + one = main_gate.select(ctx, &one, &zero, &term_1)?; } Ok(one) } diff --git a/integer/src/chip/assert_not_zero.rs b/integer/src/chip/assert_not_zero.rs index e6bc1dd6..2a596a6b 100644 --- a/integer/src/chip/assert_not_zero.rs +++ b/integer/src/chip/assert_not_zero.rs @@ -89,5 +89,4 @@ impl Date: Wed, 10 May 2023 00:38:31 -0700 Subject: [PATCH 10/37] Fix bugs --- ecdsa/src/ecdsa.rs | 36 +++++------------------------------- integer/src/chip.rs | 6 ++---- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index ad17316b..6783cc75 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -161,7 +161,6 @@ mod tests { use ecc::{EccConfig, GeneralEccChip}; use halo2::arithmetic::CurveAffine; use halo2::circuit::{Layouter, SimpleFloorPlanner, Value}; - use halo2::dev::{VerifyFailure, FailureLocation}; use halo2::halo2curves::{ ff::{Field, FromUniformBytes, PrimeField}, group::{Curve, Group}, @@ -381,36 +380,11 @@ mod tests { ..Default::default() }; let instance = vec![vec![]]; - // let result = mock_prover_verify(&circuit, instance); - // if valid_input || enable_skipping_invalid_signature { - // assert_eq!(result, Ok(())); - // } else { - // assert!(result.is_err()); // Expects an error - // } - match mock_prover_verify(&circuit, instance) { - Ok(_) => { - println!("ok"); - }, - Err(errors) => { - for error in errors { - match error { - VerifyFailure::ConstraintNotSatisfied {constraint: _, location, cell_values: _} => { - match location { - FailureLocation::InRegion { region: _, offset } => { - // handle constraint not satisfied error - println!("VerifyFailure::ConstraintNotSatisfied not satisfied at offset {:?}", offset); - }, - FailureLocation::OutsideRegion { row: _ } => { - // handle constraint not satisfied error at row level - }, - } - }, - _ => { - // Handle other error types here - } - } - } - } + let result = mock_prover_verify(&circuit, instance); + if valid_input || enable_skipping_invalid_signature { + assert_eq!(result, Ok(())); + } else { + assert!(result.is_err()); } } diff --git a/integer/src/chip.rs b/integer/src/chip.rs index badee01d..04908207 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -5,11 +5,9 @@ use crate::instructions::{IntegerInstructions, Range}; use crate::rns::{Common, Integer, Rns}; use halo2::halo2curves::ff::PrimeField; use halo2::{circuit::Value, plonk::Error}; -use maingate::halo2::circuit::Chip; -use maingate::{halo2, AssignedCondition, AssignedValue, MainGateInstructions, RegionCtx, Term}; +use maingate::{halo2, AssignedCondition, AssignedValue, MainGateInstructions, RegionCtx}; use maingate::{MainGate, MainGateConfig}; use maingate::{RangeChip, RangeConfig}; -use num_bigint::BigUint as big_uint; mod add; mod assert_in_field; @@ -386,7 +384,7 @@ impl Result, Error> { let main_gate = self.main_gate(); let zero = main_gate.assign_value(ctx, Value::known(N::ZERO))?; - let mut one = main_gate.assign_value(ctx, Value::known(N::ZERO))?; + let mut one = main_gate.assign_value(ctx, Value::known(N::ONE))?; for idx in 0..NUMBER_OF_LIMBS { let term_1 = main_gate.is_equal(ctx, a.limb(idx), b.limb(idx))?; one = main_gate.select(ctx, &one, &zero, &term_1)?; From 3ee04af6e665749e9aab8683a118d8d591af3c58 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 10 May 2023 14:26:34 -0700 Subject: [PATCH 11/37] Cleaner code --- ecdsa/src/ecdsa.rs | 10 +++------- integer/src/chip.rs | 31 +++++++++++++++++++++++++------ integer/src/instructions.rs | 17 +++++++++++++++++ 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 6783cc75..85818e38 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -136,13 +136,9 @@ impl, ) -> Result, Error> { let main_gate = self.main_gate(); - let zero = main_gate.assign_value(ctx, Value::known(N::ZERO))?; - let mut one = main_gate.assign_value(ctx, Value::known(N::ONE))?; + let mut result = main_gate.assign_value(ctx, Value::known(N::ONE))?; for idx in 0..NUMBER_OF_LIMBS { let term_1 = main_gate.is_equal(ctx, a.limb(idx), b.limb(idx))?; - one = main_gate.select(ctx, &one, &zero, &term_1)?; + result = main_gate.and(ctx, &result, &term_1)?; } - Ok(one) + Ok(result) } fn assert_not_equal( @@ -424,8 +423,28 @@ impl, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result<(), Error> { + let main_gate = self.main_gate(); + main_gate.one_or_one(ctx, a, b) + } + + fn or( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result, Error> { + let main_gate = self.main_gate(); + main_gate.or(ctx, a, b) } fn and( diff --git a/integer/src/instructions.rs b/integer/src/instructions.rs index 6bceaea7..1d753a51 100644 --- a/integer/src/instructions.rs +++ b/integer/src/instructions.rs @@ -245,6 +245,23 @@ pub trait IntegerInstructions< a: &AssignedInteger, ) -> Result, Error>; + /// Enforces one of given two values is `1` + /// `(a-1) * (b-1) = 0` + fn one_or_one( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result<(), Error>; + + /// Constraints for OR + fn or( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedCondition, + b: &AssignedCondition, + ) -> Result, Error>; + /// Constraints for AND fn and( &self, From 908d66577443e5a7a4f1c3ced6449725259f2b77 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 10 May 2023 14:47:40 -0700 Subject: [PATCH 12/37] Remove unused code --- ecdsa/src/ecdsa.rs | 1 + integer/src/chip.rs | 20 -------------------- integer/src/instructions.rs | 16 ---------------- maingate/src/instructions.rs | 12 ------------ 4 files changed, 1 insertion(+), 48 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 85818e38..375c99ad 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -135,6 +135,7 @@ impl, - a: &AssignedCondition, - b: &AssignedCondition, - ) -> Result, Error> { - let main_gate = self.main_gate(); - main_gate.or(ctx, a, b) - } - fn and( &self, ctx: &mut RegionCtx<'_, N>, @@ -457,16 +447,6 @@ impl, - a: &AssignedCondition, - b: &AssignedCondition, - ) -> Result, Error> { - let main_gate = self.main_gate(); - main_gate.is_nand(ctx, a, b) - } - fn assert_zero( &self, ctx: &mut RegionCtx<'_, N>, diff --git a/integer/src/instructions.rs b/integer/src/instructions.rs index 1d753a51..d6d26b1a 100644 --- a/integer/src/instructions.rs +++ b/integer/src/instructions.rs @@ -254,14 +254,6 @@ pub trait IntegerInstructions< b: &AssignedCondition, ) -> Result<(), Error>; - /// Constraints for OR - fn or( - &self, - ctx: &mut RegionCtx<'_, N>, - a: &AssignedCondition, - b: &AssignedCondition, - ) -> Result, Error>; - /// Constraints for AND fn and( &self, @@ -270,14 +262,6 @@ pub trait IntegerInstructions< b: &AssignedCondition, ) -> Result, Error>; - /// Constraints for NAND - fn is_nand( - &self, - ctx: &mut RegionCtx<'_, N>, - a: &AssignedCondition, - b: &AssignedCondition, - ) -> Result, Error>; - /// Constraints that an [`AssignedInteger`] is equal to zero fn assert_zero( &self, diff --git a/maingate/src/instructions.rs b/maingate/src/instructions.rs index 2b5ab769..0f19a4e6 100644 --- a/maingate/src/instructions.rs +++ b/maingate/src/instructions.rs @@ -866,18 +866,6 @@ pub trait MainGateInstructions: Chip { Ok(()) } - /// Assigns a new bit witness `r` to `0` if both given witneeses are not `0` - /// otherwise `1` - fn is_nand( - &self, - ctx: &mut RegionCtx<'_, F>, - a: &AssignedCondition, - b: &AssignedCondition, - ) -> Result, Error> { - let and_a_b = self.and(ctx, a, b)?; - self.not(ctx, &and_a_b) - } - /// Assigns a new witness `r` as: /// `r = a + b` fn add( From 963484648e1fa8d5c5f775efb82cc42385e6839b Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 17 May 2023 13:45:25 -0700 Subject: [PATCH 13/37] Add tests for invalid signature --- ecc/src/general_ecc.rs | 8 ++++- ecdsa/src/ecdsa.rs | 62 +++++++++++++++++++++++++++++------- halo2wrong/src/utils.rs | 19 +++++++++-- integer/src/chip.rs | 5 +++ integer/src/rns.rs | 3 ++ maingate/src/instructions.rs | 7 ++++ 6 files changed, 90 insertions(+), 14 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index 89fd5ee6..f93dced1 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -262,13 +262,19 @@ impl< ctx: &mut RegionCtx<'_, N>, point: &AssignedPoint, ) -> Result<(), Error> { + println!("assert_is_on_curve {:?}", point); + let integer_chip = self.base_field_chip(); let y_square = &integer_chip.square(ctx, point.y())?; let x_square = &integer_chip.square(ctx, point.x())?; let x_cube = &integer_chip.mul(ctx, point.x(), x_square)?; let x_cube_b = &integer_chip.add_constant(ctx, x_cube, &self.parameter_b())?; - integer_chip.assert_equal(ctx, x_cube_b, y_square)?; + + + // integer_chip.assert_equal(ctx, x_cube_b, y_square)?; + + Ok(()) } diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 375c99ad..d00522ae 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -151,6 +151,7 @@ mod tests { use crate::halo2; use crate::integer; use crate::maingate; + use ecc::halo2::halo2curves::new_curve_impl; use ecc::integer::Range; use ecc::maingate::big_to_fe; use ecc::maingate::fe_to_big; @@ -171,6 +172,7 @@ mod tests { use std::fmt::{Debug}; use std::marker::PhantomData; + // const BIT_LEN_LIMB: usize = 64; const BIT_LEN_LIMB: usize = 68; const NUMBER_OF_LIMBS: usize = 4; @@ -290,6 +292,8 @@ mod tests { s: s_assigned, }; + println!("ecc_chip assign_point {:?}", self.public_key); + let pk_in_circuit = ecc_chip.assign_point(ctx, self.public_key)?; let pk_assigned = AssignedPublicKey { point: pk_in_circuit, @@ -350,12 +354,48 @@ mod tests { assert_eq!(r, r_candidate); } + println!("public_key {:?}", public_key); + + let coords = public_key.coordinates(); + let coords = coords.unwrap(); + let x = *coords.x(); + let y = *coords.y(); + + println!("public_key x {:?}", x); + println!("public_key y {:?}", y); + + let new_y = x * x; + println!("public_key new_y {:?}", new_y); + + // let new_public_key = ::new_curve_impl!() from_xy(x, new_y).to_affine(); + + // This is also a valid test case. + // let public_key = C::generator(); + + // public_key = public_key * public_key; + + println!("new_public_key {:?}", public_key); + (public_key, r, s, msg_hash) } fn generate_invalid_inputs + Ord>() -> (C, C::Scalar, C::Scalar, C::Scalar) { let (public_key, _, s, msg_hash) = generate_valid_inputs::(); // Set the value of r incorrectly + let value = msg_hash * msg_hash; + println!("msg_hash {:?}, invalid value {:?}", msg_hash, value); + println!("s {:?}, invalid value {:?}", s, s * s); + // println!("is_on_curve {:?}", s.is_on_curve()); + + let scaler = BnScalar::from_raw([ + 0x9C47D08FFB10D4B8, + 0xFD17B448A6855419, + 0x5DA4FBFC0E1108A8, + 0x483ADA7726A3C465, + ]); + println!("scaler {:?}", scaler); + // let s = N::from_u128(1); + (public_key, s, s, msg_hash) } @@ -390,20 +430,20 @@ mod tests { use crate::curves::secp256k1::Secp256k1Affine as Secp256k1; // Return Errors - run::(false, false); - run::(false, false); - run::(false, false); + // run::(false, false); + // run::(false, false); + // run::(false, false); - run::(false, true); - run::(false, true); - run::(false, true); + // run::(false, true); + // run::(false, true); + // run::(false, true); run::(true, false); - run::(true, false); - run::(true, false); + // run::(true, false); + // run::(true, false); - run::(true, true); - run::(true, true); - run::(true, true); + // run::(true, true); + // run::(true, true); + // run::(true, true); } } diff --git a/halo2wrong/src/utils.rs b/halo2wrong/src/utils.rs index 111ec942..1e74412a 100644 --- a/halo2wrong/src/utils.rs +++ b/halo2wrong/src/utils.rs @@ -2,7 +2,7 @@ use crate::{ curves::ff::{FromUniformBytes, PrimeField}, halo2::{ circuit::Value, - dev::{MockProver, VerifyFailure}, + dev::{FailureLocation, MockProver, VerifyFailure}, plonk::{ Advice, Any, Assigned, Assignment, Challenge, Circuit, Column, ConstraintSystem, Error, Fixed, FloorPlanner, Instance, Selector, @@ -68,7 +68,22 @@ pub fn mock_prover_verify + Ord, C: Circuit>( circuit: &C, instance: Vec>, ) -> Result<(), Vec> { - let dimension = DimensionMeasurement::measure(circuit).unwrap(); + + let dimension = match DimensionMeasurement::measure(circuit) { + Ok(result) => result, + Err(err) => { + // Handle the error case here + return Err(vec![VerifyFailure::Lookup { + name: "lookup".to_string(), + lookup_index: 0, + location: FailureLocation::InRegion { + region: (2, "Faulty synthesis").into(), + offset: 1, + } + }]); + } + }; + let prover = MockProver::run(dimension.k(), circuit, instance) .unwrap_or_else(|err| panic!("{:#?}", err)); prover.verify_at_rows_par(dimension.advice_range(), dimension.advice_range()) diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 7fc3c67a..70449269 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -123,7 +123,12 @@ impl: Chip { .zip(bases.into_iter()) .map(|(bit, base)| Term::Assigned(bit, base)) .collect::>(); + let result = self.compose(ctx, &terms, F::ZERO)?; + self.assert_equal(ctx, &result, composed)?; Ok(bits) } @@ -1093,6 +1095,11 @@ pub trait MainGateInstructions: Chip { terms: &[Term], constant: F, ) -> Result, Error> { + + if terms.is_empty() { + return Err(Error::BoundsFailure); + } + assert!(!terms.is_empty(), "At least one term is expected"); let (composed, _) = self.decompose(ctx, terms, constant, |_, _| Ok(()))?; From 32115259b554fcf29028cc5bd5e8ccbd9fe55c25 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 17 May 2023 15:24:32 -0700 Subject: [PATCH 14/37] Update --- ecc/src/general_ecc.rs | 24 +++++++++----- ecdsa/src/ecdsa.rs | 75 +++++++++++++----------------------------- 2 files changed, 38 insertions(+), 61 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index f93dced1..f40774e9 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -209,18 +209,30 @@ impl< ctx: &mut RegionCtx<'_, N>, point: Value, ) -> Result, Error> { - let integer_chip = self.base_field_chip(); - let point = point.map(|point| self.to_rns_point(point)); let (x, y) = point .map(|point| (point.x().clone(), point.y().clone())) .unzip(); + self.assign_x_y(ctx, x.into(), y.into()) + } + + /// Takes `Point.x` and `Point.y` of the EC and returns it as `AssignedPoint` + pub fn assign_x_y( + &self, + ctx: &mut RegionCtx<'_, N>, + x: UnassignedInteger<::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>, + y: UnassignedInteger<::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>, + ) -> Result, Error> { + let integer_chip = self.base_field_chip(); + let x = integer_chip.assign_integer(ctx, x.into(), Range::Remainder)?; let y = integer_chip.assign_integer(ctx, y.into(), Range::Remainder)?; let point = AssignedPoint::new(x, y); + self.assert_is_on_curve(ctx, &point)?; + Ok(point) } @@ -262,19 +274,13 @@ impl< ctx: &mut RegionCtx<'_, N>, point: &AssignedPoint, ) -> Result<(), Error> { - println!("assert_is_on_curve {:?}", point); - let integer_chip = self.base_field_chip(); let y_square = &integer_chip.square(ctx, point.y())?; let x_square = &integer_chip.square(ctx, point.x())?; let x_cube = &integer_chip.mul(ctx, point.x(), x_square)?; let x_cube_b = &integer_chip.add_constant(ctx, x_cube, &self.parameter_b())?; - - - // integer_chip.assert_equal(ctx, x_cube_b, y_square)?; - - + integer_chip.assert_equal(ctx, x_cube_b, y_square)?; Ok(()) } diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index d00522ae..05c371ac 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -151,7 +151,6 @@ mod tests { use crate::halo2; use crate::integer; use crate::maingate; - use ecc::halo2::halo2curves::new_curve_impl; use ecc::integer::Range; use ecc::maingate::big_to_fe; use ecc::maingate::fe_to_big; @@ -228,6 +227,7 @@ mod tests { aux_generator: E, window_size: usize, + valid_input: bool, enable_skipping_invalid_signature: bool, _marker: PhantomData, @@ -292,9 +292,17 @@ mod tests { s: s_assigned, }; - println!("ecc_chip assign_point {:?}", self.public_key); + let point = self.public_key.map(|point| ecc_chip.to_rns_point(point)); + let (x, y) = point + .map(|point| (point.x().clone(), point.y().clone())) + .unzip(); + let (x, y) = if self.valid_input { + (x.clone(), y.clone()) + } else { + (x.clone(), x.clone()) + }; - let pk_in_circuit = ecc_chip.assign_point(ctx, self.public_key)?; + let pk_in_circuit = ecc_chip.assign_x_y(ctx, x.into(), y.into())?; let pk_assigned = AssignedPublicKey { point: pk_in_circuit, }; @@ -353,49 +361,11 @@ mod tests { let r_candidate = mod_n::(*x_candidate); assert_eq!(r, r_candidate); } - - println!("public_key {:?}", public_key); - - let coords = public_key.coordinates(); - let coords = coords.unwrap(); - let x = *coords.x(); - let y = *coords.y(); - - println!("public_key x {:?}", x); - println!("public_key y {:?}", y); - - let new_y = x * x; - println!("public_key new_y {:?}", new_y); - - // let new_public_key = ::new_curve_impl!() from_xy(x, new_y).to_affine(); - - // This is also a valid test case. - // let public_key = C::generator(); - - // public_key = public_key * public_key; - - println!("new_public_key {:?}", public_key); - (public_key, r, s, msg_hash) } fn generate_invalid_inputs + Ord>() -> (C, C::Scalar, C::Scalar, C::Scalar) { let (public_key, _, s, msg_hash) = generate_valid_inputs::(); - // Set the value of r incorrectly - let value = msg_hash * msg_hash; - println!("msg_hash {:?}, invalid value {:?}", msg_hash, value); - println!("s {:?}, invalid value {:?}", s, s * s); - // println!("is_on_curve {:?}", s.is_on_curve()); - - let scaler = BnScalar::from_raw([ - 0x9C47D08FFB10D4B8, - 0xFD17B448A6855419, - 0x5DA4FBFC0E1108A8, - 0x483ADA7726A3C465, - ]); - println!("scaler {:?}", scaler); - // let s = N::from_u128(1); - (public_key, s, s, msg_hash) } @@ -413,6 +383,7 @@ mod tests { msg_hash:Value::known(msg_hash), aux_generator, window_size: 4, + valid_input, enable_skipping_invalid_signature, ..Default::default() }; @@ -430,20 +401,20 @@ mod tests { use crate::curves::secp256k1::Secp256k1Affine as Secp256k1; // Return Errors - // run::(false, false); - // run::(false, false); - // run::(false, false); + run::(false, false); + run::(false, false); + run::(false, false); - // run::(false, true); - // run::(false, true); - // run::(false, true); + run::(false, true); + run::(false, true); + run::(false, true); run::(true, false); - // run::(true, false); - // run::(true, false); + run::(true, false); + run::(true, false); - // run::(true, true); - // run::(true, true); - // run::(true, true); + run::(true, true); + run::(true, true); + run::(true, true); } } From e9d823ebf2733662176d6685891ee23762e7b7e6 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 17 May 2023 15:54:00 -0700 Subject: [PATCH 15/37] Add is_on_curve to check if is on curve --- ecc/src/general_ecc.rs | 26 ++++++++++++++++++++------ ecdsa/src/ecdsa.rs | 6 +++++- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index f40774e9..a4ba687d 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -214,7 +214,8 @@ impl< .map(|point| (point.x().clone(), point.y().clone())) .unzip(); - self.assign_x_y(ctx, x.into(), y.into()) + let (point, _) = self.assign_x_y(ctx, x.into(), y.into())?; + Ok(point) } /// Takes `Point.x` and `Point.y` of the EC and returns it as `AssignedPoint` @@ -223,17 +224,15 @@ impl< ctx: &mut RegionCtx<'_, N>, x: UnassignedInteger<::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>, y: UnassignedInteger<::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>, - ) -> Result, Error> { + ) -> Result<(AssignedPoint, AssignedCondition), Error> { let integer_chip = self.base_field_chip(); let x = integer_chip.assign_integer(ctx, x.into(), Range::Remainder)?; let y = integer_chip.assign_integer(ctx, y.into(), Range::Remainder)?; let point = AssignedPoint::new(x, y); - - self.assert_is_on_curve(ctx, &point)?; - - Ok(point) + let is_on_curve = self.is_on_curve(ctx, &point)?; + Ok((point, is_on_curve)) } /// Assigns the auxiliary generator point @@ -284,6 +283,21 @@ impl< Ok(()) } + /// Constraints to check if `AssignedPoint` is on curve + pub fn is_on_curve( + &self, + ctx: &mut RegionCtx<'_, N>, + point: &AssignedPoint, + ) -> Result, Error> { + let integer_chip = self.base_field_chip(); + + let y_square = &integer_chip.square(ctx, point.y())?; + let x_square = &integer_chip.square(ctx, point.x())?; + let x_cube = &integer_chip.mul(ctx, point.x(), x_square)?; + let x_cube_b = &integer_chip.add_constant(ctx, x_cube, &self.parameter_b())?; + integer_chip.is_strict_equal(ctx, x_cube_b, y_square) + } + /// Constraints assert two `AssignedPoint`s are equal pub fn assert_equal( &self, diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 05c371ac..cd13cdcf 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -302,7 +302,11 @@ mod tests { (x.clone(), x.clone()) }; - let pk_in_circuit = ecc_chip.assign_x_y(ctx, x.into(), y.into())?; + let (pk_in_circuit, is_pk_on_curve) = ecc_chip.assign_x_y(ctx, x.into(), y.into())?; + let enable_skipping_invalid_signature = scalar_chip.assign_constant(ctx, (self.enable_skipping_invalid_signature as u64).into())?; + let enable_skipping_invalid_signature = scalar_chip.is_not_zero(ctx, &enable_skipping_invalid_signature)?; + scalar_chip.one_or_one(ctx, &enable_skipping_invalid_signature, &is_pk_on_curve)?; + let pk_assigned = AssignedPublicKey { point: pk_in_circuit, }; From 33a4db33e7f6abd51f3c764f9f3e4f382542f604 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 17 May 2023 16:01:42 -0700 Subject: [PATCH 16/37] Update --- ecdsa/src/ecdsa.rs | 3 ++- halo2wrong/src/utils.rs | 19 ++----------------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index cd13cdcf..9b756066 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -299,6 +299,7 @@ mod tests { let (x, y) = if self.valid_input { (x.clone(), y.clone()) } else { + // Generate a point that is not on the curve. (x.clone(), x.clone()) }; @@ -384,7 +385,7 @@ mod tests { let circuit = TestCircuitEcdsaVerify:: { public_key: Value::known(public_key), signature: Value::known((r, s)), - msg_hash:Value::known(msg_hash), + msg_hash: Value::known(msg_hash), aux_generator, window_size: 4, valid_input, diff --git a/halo2wrong/src/utils.rs b/halo2wrong/src/utils.rs index 1e74412a..111ec942 100644 --- a/halo2wrong/src/utils.rs +++ b/halo2wrong/src/utils.rs @@ -2,7 +2,7 @@ use crate::{ curves::ff::{FromUniformBytes, PrimeField}, halo2::{ circuit::Value, - dev::{FailureLocation, MockProver, VerifyFailure}, + dev::{MockProver, VerifyFailure}, plonk::{ Advice, Any, Assigned, Assignment, Challenge, Circuit, Column, ConstraintSystem, Error, Fixed, FloorPlanner, Instance, Selector, @@ -68,22 +68,7 @@ pub fn mock_prover_verify + Ord, C: Circuit>( circuit: &C, instance: Vec>, ) -> Result<(), Vec> { - - let dimension = match DimensionMeasurement::measure(circuit) { - Ok(result) => result, - Err(err) => { - // Handle the error case here - return Err(vec![VerifyFailure::Lookup { - name: "lookup".to_string(), - lookup_index: 0, - location: FailureLocation::InRegion { - region: (2, "Faulty synthesis").into(), - offset: 1, - } - }]); - } - }; - + let dimension = DimensionMeasurement::measure(circuit).unwrap(); let prover = MockProver::run(dimension.k(), circuit, instance) .unwrap_or_else(|err| panic!("{:#?}", err)); prover.verify_at_rows_par(dimension.advice_range(), dimension.advice_range()) From 93ff0d5c146e39e8f2a53f5299c8ba26590045ab Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 17 May 2023 16:13:01 -0700 Subject: [PATCH 17/37] Update --- ecc/src/general_ecc.rs | 7 +++++-- integer/src/chip.rs | 5 ----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index a4ba687d..114e58e4 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -7,7 +7,7 @@ use halo2::arithmetic::CurveAffine; use halo2::circuit::{Layouter, Value}; use halo2::halo2curves::ff::PrimeField; use halo2::plonk::Error; -use integer::maingate::RegionCtx; +use integer::maingate::{RegionCtx, MainGateInstructions}; use maingate::{AssignedCondition, MainGate}; use std::collections::BTreeMap; use std::rc::Rc; @@ -209,12 +209,15 @@ impl< ctx: &mut RegionCtx<'_, N>, point: Value, ) -> Result, Error> { + let maingate = self.main_gate(); + let point = point.map(|point| self.to_rns_point(point)); let (x, y) = point .map(|point| (point.x().clone(), point.y().clone())) .unzip(); - let (point, _) = self.assign_x_y(ctx, x.into(), y.into())?; + let (point, is_on_curve) = self.assign_x_y(ctx, x.into(), y.into())?; + maingate.assert_not_zero(ctx, &is_on_curve)?; Ok(point) } diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 70449269..7fc3c67a 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -123,12 +123,7 @@ impl Date: Wed, 17 May 2023 16:15:21 -0700 Subject: [PATCH 18/37] Update --- integer/src/rns.rs | 3 --- maingate/src/instructions.rs | 7 ------- 2 files changed, 10 deletions(-) diff --git a/integer/src/rns.rs b/integer/src/rns.rs index 1910fa54..c011be79 100644 --- a/integer/src/rns.rs +++ b/integer/src/rns.rs @@ -328,7 +328,6 @@ impl: Chip { .zip(bases.into_iter()) .map(|(bit, base)| Term::Assigned(bit, base)) .collect::>(); - let result = self.compose(ctx, &terms, F::ZERO)?; - self.assert_equal(ctx, &result, composed)?; Ok(bits) } @@ -1095,11 +1093,6 @@ pub trait MainGateInstructions: Chip { terms: &[Term], constant: F, ) -> Result, Error> { - - if terms.is_empty() { - return Err(Error::BoundsFailure); - } - assert!(!terms.is_empty(), "At least one term is expected"); let (composed, _) = self.decompose(ctx, terms, constant, |_, _| Ok(()))?; From 302edad9594cca1583ef0d867365ada339ceeba1 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 17 May 2023 16:18:01 -0700 Subject: [PATCH 19/37] Update --- ecc/src/general_ecc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index 114e58e4..9f206b13 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -7,7 +7,7 @@ use halo2::arithmetic::CurveAffine; use halo2::circuit::{Layouter, Value}; use halo2::halo2curves::ff::PrimeField; use halo2::plonk::Error; -use integer::maingate::{RegionCtx, MainGateInstructions}; +use integer::maingate::RegionCtx; use maingate::{AssignedCondition, MainGate}; use std::collections::BTreeMap; use std::rc::Rc; From 973e4faa81a3d1b04f104ecf36c417c9774a7f49 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Wed, 17 May 2023 16:19:11 -0700 Subject: [PATCH 20/37] Update import --- ecc/src/general_ecc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index 9f206b13..24821028 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -7,7 +7,7 @@ use halo2::arithmetic::CurveAffine; use halo2::circuit::{Layouter, Value}; use halo2::halo2curves::ff::PrimeField; use halo2::plonk::Error; -use integer::maingate::RegionCtx; +use integer::maingate::{RegionCtx, MainGateInstructions}; use maingate::{AssignedCondition, MainGate}; use std::collections::BTreeMap; use std::rc::Rc; @@ -171,7 +171,6 @@ impl< point: AssignedPoint, offset: usize, ) -> Result<(), Error> { - use integer::maingate::MainGateInstructions; let main_gate = self.main_gate(); let mut offset = offset; From 5d4db404244f11d7b5fc67e367df3f712f4f139d Mon Sep 17 00:00:00 2001 From: xiaodino Date: Sat, 20 May 2023 00:11:12 -0700 Subject: [PATCH 21/37] Add try_reduce_if_limb_values_exceeds_reduced and try_reduce_if_max_operand_value_exceeds --- integer/src/chip.rs | 11 ++++--- integer/src/chip/assign.rs | 2 ++ integer/src/chip/reduce.rs | 60 +++++++++++++++++++++++++++++++++++- maingate/src/instructions.rs | 2 ++ 4 files changed, 70 insertions(+), 5 deletions(-) diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 7fc3c67a..e0068b4e 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -418,13 +418,16 @@ impl, a: &AssignedInteger, ) -> Result, Error> { - let a = &self.reduce_if_limb_values_exceeds_reduced(ctx, a)?; - let a = &self.reduce_if_max_operand_value_exceeds(ctx, a)?; - let main_gate = self.main_gate(); + + let (a, is_reduce_if_limb_values_succeeded) = &self.try_reduce_if_limb_values_exceeds_reduced(ctx, a)?; + let (a, is_reduce_if_max_operand_value_succeeded) = &self.try_reduce_if_max_operand_value_exceeds(ctx, a)?; + let is_reduce_succeeded = main_gate.and(ctx, &is_reduce_if_limb_values_succeeded, &is_reduce_if_max_operand_value_succeeded)?; + let zero = self.assign_constant(ctx, W::ZERO)?; let is_zero = self.is_strict_equal(ctx, &zero, &a)?; - main_gate.not(ctx, &is_zero) + let result = main_gate.not(ctx, &is_zero)?; + main_gate.and(ctx, &result, &is_reduce_succeeded) } fn one_or_one( diff --git a/integer/src/chip/assign.rs b/integer/src/chip/assign.rs index 07796e82..e08dca5d 100644 --- a/integer/src/chip/assign.rs +++ b/integer/src/chip/assign.rs @@ -85,6 +85,8 @@ impl IntegerChip @@ -40,6 +40,38 @@ impl, + a: &AssignedInteger, + ) -> Result<(AssignedInteger, AssignedCondition), Error> { + let zero = self.assign_constant(ctx, W::ZERO)?; + let one = self.assign_constant(ctx, W::ONE)?; + let zero = self.is_strict_equal(ctx, &zero, &one)?; + let one = self.is_strict_equal(ctx, &one.clone(), &one)?; + let exceeds_max_limb_value = a + .limbs + .iter() + .any(|limb| limb.max_val() > self.rns.max_reduced_limb); + if exceeds_max_limb_value { + match self.reduce(ctx, a) { + Ok(result) => { + Ok((result, one)) + } + Err(_) => { + Ok((a.clone(), zero)) + } + } + } else { + Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), one)) + } + } + /// Reduces an [`AssignedInteger`] if any of its limbs values is greater /// than the [`Rns`] `max_reduced_limb` pub(super) fn reduce_if_limb_values_exceeds_reduced( @@ -73,6 +105,32 @@ impl, + a: &AssignedInteger, + ) -> Result<(AssignedInteger, AssignedCondition), Error> { + let zero = self.assign_constant(ctx, W::ZERO)?; + let one = self.assign_constant(ctx, W::ONE)?; + let zero = self.is_strict_equal(ctx, &zero, &one)?; + let one = self.is_strict_equal(ctx, &one.clone(), &one)?; + let exceeds_max_value = a.max_val() > self.rns.max_operand; + if exceeds_max_value { + match self.reduce(ctx, a) { + Ok(result) => { + Ok((result, one)) + } + Err(_) => { + Ok((a.clone(), zero)) + } + } + } else { + Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), one)) + } + } + pub(super) fn reduce_generic( &self, ctx: &mut RegionCtx<'_, N>, diff --git a/maingate/src/instructions.rs b/maingate/src/instructions.rs index 0f19a4e6..c1715b69 100644 --- a/maingate/src/instructions.rs +++ b/maingate/src/instructions.rs @@ -1093,7 +1093,9 @@ pub trait MainGateInstructions: Chip { terms: &[Term], constant: F, ) -> Result, Error> { + assert!(!terms.is_empty(), "At least one term is expected"); + let (composed, _) = self.decompose(ctx, terms, constant, |_, _| Ok(()))?; Ok(composed) From 1b5e65db2289b18b6cb89e60d46429977d733ab9 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Sat, 20 May 2023 00:21:18 -0700 Subject: [PATCH 22/37] Add try_reduce --- integer/src/chip.rs | 8 ++------ integer/src/chip/reduce.rs | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/integer/src/chip.rs b/integer/src/chip.rs index e0068b4e..8bcef4af 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -419,15 +419,11 @@ impl, ) -> Result, Error> { let main_gate = self.main_gate(); - - let (a, is_reduce_if_limb_values_succeeded) = &self.try_reduce_if_limb_values_exceeds_reduced(ctx, a)?; - let (a, is_reduce_if_max_operand_value_succeeded) = &self.try_reduce_if_max_operand_value_exceeds(ctx, a)?; - let is_reduce_succeeded = main_gate.and(ctx, &is_reduce_if_limb_values_succeeded, &is_reduce_if_max_operand_value_succeeded)?; - + let (a, is_reduce_succeeded) = &self.try_reduce(ctx, a)?; let zero = self.assign_constant(ctx, W::ZERO)?; let is_zero = self.is_strict_equal(ctx, &zero, &a)?; let result = main_gate.not(ctx, &is_zero)?; - main_gate.and(ctx, &result, &is_reduce_succeeded) + main_gate.and(ctx, &result, is_reduce_succeeded) } fn one_or_one( diff --git a/integer/src/chip/reduce.rs b/integer/src/chip/reduce.rs index 13c492e6..01810c00 100644 --- a/integer/src/chip/reduce.rs +++ b/integer/src/chip/reduce.rs @@ -40,6 +40,25 @@ impl, + a: &AssignedInteger, + ) -> Result<(AssignedInteger, AssignedCondition), Error> { + let main_gate = self.main_gate(); + + let (a, is_reduce_if_limb_values_succeeded) = self.try_reduce_if_limb_values_exceeds_reduced(ctx, a)?; + let (a, is_reduce_if_max_operand_value_succeeded) = self.try_reduce_if_max_operand_value_exceeds(ctx, &a)?; + let is_reduce_succeeded = main_gate.and(ctx, &is_reduce_if_limb_values_succeeded, &is_reduce_if_max_operand_value_succeeded)?; + + Ok((a, is_reduce_succeeded)) + } + /// Try to reduces an [`AssignedInteger`] if any of its limbs values is greater /// than the [`Rns`] `max_unreduced_limb`. /// From f95db78bfc0ac753e2d59fe84f1cf3d6002ae495 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Sat, 20 May 2023 16:32:33 -0700 Subject: [PATCH 23/37] Update --- ecc/src/general_ecc.rs | 9 +++++---- integer/src/chip.rs | 20 ++++++++++++++++++++ integer/src/chip/assign.rs | 6 ++++-- integer/src/instructions.rs | 9 +++++++++ maingate/src/instructions.rs | 3 --- 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index 24821028..3fa2263b 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -229,12 +229,13 @@ impl< ) -> Result<(AssignedPoint, AssignedCondition), Error> { let integer_chip = self.base_field_chip(); - let x = integer_chip.assign_integer(ctx, x.into(), Range::Remainder)?; - let y = integer_chip.assign_integer(ctx, y.into(), Range::Remainder)?; - + let (x, is_x_valid) = integer_chip.try_assign_integer(ctx, x.into(), Range::Remainder)?; + let (y, is_y_valid) = integer_chip.try_assign_integer(ctx, y.into(), Range::Remainder)?; + let is_valid = integer_chip.and(ctx, &is_x_valid, &is_y_valid)?; let point = AssignedPoint::new(x, y); let is_on_curve = self.is_on_curve(ctx, &point)?; - Ok((point, is_on_curve)) + let is_valid = integer_chip.and(ctx, &is_valid, &is_on_curve)?; + Ok((point, is_valid)) } /// Assigns the auxiliary generator point diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 8bcef4af..65e3b458 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -99,6 +99,26 @@ impl, + integer: UnassignedInteger, + range: Range, + ) -> Result<(AssignedInteger, AssignedCondition), Error> { + let zero = self.assign_constant(ctx, W::ZERO)?; + let one = self.assign_constant(ctx, W::ONE)?; + let zero = self.is_strict_equal(ctx, &zero, &one)?; + let one = self.is_strict_equal(ctx, &one.clone(), &one)?; + match self.assign_integer_generic(ctx, integer, range) { + Ok(result) => { + Ok((result, one)) + } + Err(_) => { + Ok((self.assign_constant(ctx, W::ZERO)?, zero)) + } + } + } + fn assign_constant( &self, ctx: &mut RegionCtx<'_, N>, diff --git a/integer/src/chip/assign.rs b/integer/src/chip/assign.rs index e08dca5d..6a55acef 100644 --- a/integer/src/chip/assign.rs +++ b/integer/src/chip/assign.rs @@ -3,7 +3,7 @@ use crate::rns::{Common, Integer}; use crate::{AssignedInteger, AssignedLimb, UnassignedInteger}; use halo2::halo2curves::ff::PrimeField; use halo2::plonk::Error; -use maingate::{fe_to_big, halo2, MainGateInstructions, RangeInstructions, RegionCtx, Term}; +use maingate::{fe_to_big, halo2, AssignedCondition, MainGateInstructions, RangeInstructions, RegionCtx, Term}; use num_bigint::BigUint as big_uint; use num_traits::One; use std::rc::Rc; @@ -16,6 +16,7 @@ impl, integer: UnassignedInteger, range: Range, + // ) -> Result<(AssignedInteger, AssignedCondition), Error> { ) -> Result, Error> { let range_chip = self.range_chip(); let main_gate = self.main_gate(); @@ -86,9 +87,10 @@ impl Result, Error>; + /// Try to assigns an [`Integer`] to a cell in the circuit without range check for the + /// appropriate [`Range`]. + fn try_assign_integer( + &self, + ctx: &mut RegionCtx<'_, N>, + integer: UnassignedInteger, + range: Range, + ) -> Result<(AssignedInteger, AssignedCondition), Error>; + /// Assigns an [`Integer`] constant to a cell in the circuit returning an /// [`AssignedInteger`]. fn assign_constant( diff --git a/maingate/src/instructions.rs b/maingate/src/instructions.rs index c1715b69..2bb07069 100644 --- a/maingate/src/instructions.rs +++ b/maingate/src/instructions.rs @@ -1093,11 +1093,8 @@ pub trait MainGateInstructions: Chip { terms: &[Term], constant: F, ) -> Result, Error> { - assert!(!terms.is_empty(), "At least one term is expected"); - let (composed, _) = self.decompose(ctx, terms, constant, |_, _| Ok(()))?; - Ok(composed) } From 5323c0efface2ec9b912987fc818d406745ac089 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Sat, 20 May 2023 23:11:57 -0700 Subject: [PATCH 24/37] Update reduce --- ecc/src/base_field_ecc.rs | 2 +- ecc/src/general_ecc.rs | 2 +- ecdsa/src/ecdsa.rs | 6 ++-- integer/src/chip.rs | 40 +++++++++++---------------- integer/src/chip/assert_in_field.rs | 1 + integer/src/chip/assert_not_zero.rs | 12 ++++---- integer/src/chip/assign.rs | 10 +++---- integer/src/chip/reduce.rs | 43 +++++++++++------------------ integer/src/instructions.rs | 4 +-- 9 files changed, 52 insertions(+), 68 deletions(-) diff --git a/ecc/src/base_field_ecc.rs b/ecc/src/base_field_ecc.rs index c38e8855..123249c1 100644 --- a/ecc/src/base_field_ecc.rs +++ b/ecc/src/base_field_ecc.rs @@ -263,7 +263,7 @@ impl let integer_chip = self.integer_chip(); let x = integer_chip.reduce(ctx, point.x())?; let y = integer_chip.reduce(ctx, point.y())?; - Ok(AssignedPoint::new(x, y)) + Ok(AssignedPoint::new(x.0, y.0)) } /// Adds 2 distinct `AssignedPoints` diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index 3fa2263b..2ed017ac 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -352,7 +352,7 @@ impl< let integer_chip = self.base_field_chip(); let x = integer_chip.reduce(ctx, point.x())?; let y = integer_chip.reduce(ctx, point.y())?; - Ok(AssignedPoint::new(x, y)) + Ok(AssignedPoint::new(x.0, y.0)) } /// Adds 2 distinct `AssignedPoints` diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 9b756066..1f84c53d 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -129,14 +129,16 @@ impl, // TODO: external integer might have different parameter settings a: &AssignedInteger, - ) -> Result, Error> { + ) -> Result<(AssignedInteger, AssignedCondition), Error> { let to_be_reduced = self.new_assigned_integer(a.limbs(), a.native().clone()); self.reduce(ctx, &to_be_reduced) } @@ -96,7 +96,10 @@ impl, range: Range, ) -> Result, Error> { - self.assign_integer_generic(ctx, integer, range) + let main_gate = self.main_gate(); + let (result, succeeded) = self.assign_integer_generic(ctx, integer, range)?; + main_gate.assert_not_zero(ctx, &succeeded)?; + Ok(result) } fn try_assign_integer( @@ -105,18 +108,7 @@ impl, range: Range, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - let zero = self.assign_constant(ctx, W::ZERO)?; - let one = self.assign_constant(ctx, W::ONE)?; - let zero = self.is_strict_equal(ctx, &zero, &one)?; - let one = self.is_strict_equal(ctx, &one.clone(), &one)?; - match self.assign_integer_generic(ctx, integer, range) { - Ok(result) => { - Ok((result, one)) - } - Err(_) => { - Ok((self.assign_constant(ctx, W::ZERO)?, zero)) - } - } + self.assign_integer_generic(ctx, integer, range) } fn assign_constant( @@ -368,7 +360,7 @@ impl, a: &AssignedInteger, - ) -> Result, Error> { + ) -> Result<(AssignedInteger, AssignedCondition), Error> { self.reduce_generic(ctx, a) } @@ -881,9 +873,9 @@ mod tests { Range::Remainder, )?; let reduced_1 = &integer_chip.reduce(ctx, overflows)?; - assert_eq!(reduced_1.max_val(), self.rns.max_remainder); - integer_chip.assert_equal(ctx, reduced_0, reduced_1)?; - integer_chip.assert_strict_equal(ctx, reduced_0, reduced_1)?; + assert_eq!(reduced_1.0.max_val(), self.rns.max_remainder); + integer_chip.assert_equal(ctx, reduced_0, &reduced_1.0)?; + integer_chip.assert_strict_equal(ctx, reduced_0, &reduced_1.0)?; Ok(()) }, )?; @@ -1225,8 +1217,8 @@ mod tests { c_in_field.into(), Range::Remainder, )?; - integer_chip.assert_equal(ctx, &c_0, &c_1)?; - integer_chip.assert_strict_equal(ctx, &c_0, &c_1)?; + integer_chip.assert_equal(ctx, &c_0.0, &c_1)?; + integer_chip.assert_strict_equal(ctx, &c_0.0, &c_1)?; } { @@ -1253,8 +1245,8 @@ mod tests { c_in_field.into(), Range::Remainder, )?; - integer_chip.assert_equal(ctx, &c_0, &c_1)?; - integer_chip.assert_strict_equal(ctx, &c_0, &c_1)?; + integer_chip.assert_equal(ctx, &c_0.0, &c_1)?; + integer_chip.assert_strict_equal(ctx, &c_0.0, &c_1)?; } { @@ -1272,8 +1264,8 @@ mod tests { integer_chip.assign_integer(ctx, c.into(), Range::Remainder)?; let c_0 = integer_chip.reduce(ctx, &a)?; integer_chip.assert_equal(ctx, &a, &c_1)?; - integer_chip.assert_equal(ctx, &c_0, &c_1)?; - integer_chip.assert_strict_equal(ctx, &c_0, &c_1)?; + integer_chip.assert_equal(ctx, &c_0.0, &c_1)?; + integer_chip.assert_strict_equal(ctx, &c_0.0, &c_1)?; } } diff --git a/integer/src/chip/assert_in_field.rs b/integer/src/chip/assert_in_field.rs index f3feded1..8e13d4cd 100644 --- a/integer/src/chip/assert_in_field.rs +++ b/integer/src/chip/assert_in_field.rs @@ -30,6 +30,7 @@ impl r % 2 ^ 64 = 0 /\ r % native_modulus = 0 // r <> 0 <-> r % 2 ^ 64 <> 0 \/ r % native_modulus <> 0 // r <> 0 <-> invert(r.limb(0)) \/ invert(r.native()) - let cond_zero_0 = main_gate.is_zero(ctx, r.limb(0))?; - let cond_zero_1 = main_gate.is_zero(ctx, r.native())?; + let cond_zero_0 = main_gate.is_zero(ctx, r.0.limb(0))?; + let cond_zero_1 = main_gate.is_zero(ctx, r.0.native())?; // one of them might be succeeded, i.e. cond_zero_0 * cond_zero_1 = 0 main_gate.nand(ctx, &cond_zero_0, &cond_zero_1)?; @@ -46,12 +46,12 @@ impl, integer: UnassignedInteger, range: Range, - // ) -> Result<(AssignedInteger, AssignedCondition), Error> { - ) -> Result, Error> { + ) -> Result<(AssignedInteger, AssignedCondition), Error> { + // ) -> Result, Error> { let range_chip = self.range_chip(); let main_gate = self.main_gate(); @@ -87,11 +87,11 @@ impl, a: &AssignedInteger, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - let zero = self.assign_constant(ctx, W::ZERO)?; let one = self.assign_constant(ctx, W::ONE)?; - let zero = self.is_strict_equal(ctx, &zero, &one)?; let one = self.is_strict_equal(ctx, &one.clone(), &one)?; let exceeds_max_limb_value = a .limbs .iter() .any(|limb| limb.max_val() > self.rns.max_reduced_limb); if exceeds_max_limb_value { - match self.reduce(ctx, a) { - Ok(result) => { - Ok((result, one)) - } - Err(_) => { - Ok((a.clone(), zero)) - } - } + self.reduce(ctx, a) } else { Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), one)) } @@ -103,7 +95,8 @@ impl self.rns.max_reduced_limb); if exceeds_max_limb_value { - self.reduce(ctx, a) + let result = self.reduce(ctx, a)?; + Ok(result.0) } else { Ok(self.new_assigned_integer(a.limbs(), a.native().clone())) } @@ -118,7 +111,8 @@ impl Result, Error> { let exceeds_max_value = a.max_val() > self.rns.max_operand; if exceeds_max_value { - self.reduce(ctx, a) + let result = self.reduce(ctx, a)?; + Ok(result.0) } else { Ok(self.new_assigned_integer(a.limbs(), a.native().clone())) } @@ -131,20 +125,11 @@ impl, a: &AssignedInteger, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - let zero = self.assign_constant(ctx, W::ZERO)?; let one = self.assign_constant(ctx, W::ONE)?; - let zero = self.is_strict_equal(ctx, &zero, &one)?; let one = self.is_strict_equal(ctx, &one.clone(), &one)?; let exceeds_max_value = a.max_val() > self.rns.max_operand; if exceeds_max_value { - match self.reduce(ctx, a) { - Ok(result) => { - Ok((result, one)) - } - Err(_) => { - Ok((a.clone(), zero)) - } - } + self.reduce(ctx, a) } else { Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), one)) } @@ -154,7 +139,8 @@ impl, a: &AssignedInteger, - ) -> Result, Error> { + ) -> Result<(AssignedInteger, AssignedCondition), Error> { + // ) -> Result, Error> { let main_gate = self.main_gate(); let (zero, one) = (N::ZERO, N::ONE); @@ -163,9 +149,12 @@ impl AssignedValue conversion"), - &result, + &result.0, residues, )?; @@ -205,7 +194,7 @@ impl, a: &AssignedInteger, - ) -> Result, Error>; + ) -> Result<(AssignedInteger, AssignedCondition), Error>; /// Constraints that two [`AssignedInteger`] are equal. fn assert_equal( @@ -334,7 +334,7 @@ pub trait IntegerInstructions< &self, ctx: &mut RegionCtx<'_, N>, a: &AssignedInteger, - ) -> Result, Error>; + ) -> Result<(AssignedInteger, AssignedCondition), Error>; /// Applies % 2 to the given input fn sign( From f011c4e69951c21ce9d3da3b08d6d918b8cca52e Mon Sep 17 00:00:00 2001 From: xiaodino Date: Thu, 25 May 2023 01:03:06 -0700 Subject: [PATCH 25/37] Add soft check for r and s should be less than Remiander --- ecdsa/src/ecdsa.rs | 41 ++++++++++++++++++++++++++++----- integer/src/chip.rs | 11 +++++++++ integer/src/chip/assign.rs | 7 +++++- integer/src/chip/reduce.rs | 45 ++++++++++++++++++++++++++++++------- integer/src/instructions.rs | 7 ++++++ 5 files changed, 96 insertions(+), 15 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 1f84c53d..1e016570 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -168,11 +168,14 @@ mod tests { use integer::IntegerInstructions; use maingate::mock_prover_verify; use maingate::{MainGate, MainGateConfig, RangeChip, RangeConfig, RangeInstructions}; + use num_traits::Num; use rand_core::OsRng; use std::fmt::{Debug}; use std::marker::PhantomData; + use num_bigint::BigUint as big_uint; + // const BIT_LEN_LIMB: usize = 64; const BIT_LEN_LIMB: usize = 68; const NUMBER_OF_LIMBS: usize = 4; @@ -231,6 +234,8 @@ mod tests { valid_input: bool, enable_skipping_invalid_signature: bool, + r: E::Scalar, + s: E::Scalar, _marker: PhantomData, } @@ -279,6 +284,26 @@ mod tests { let offset = 0; let ctx = &mut RegionCtx::new(region, offset); + let mut is_valid = scalar_chip.assign_constant(ctx, (1 as u64).into())?; + + // r and s should be less than Remiander + let r = if self.valid_input { + let r = format!("{:?}", self.r); + big_uint::from_str_radix(&r[2..], 16).unwrap() + } else { + scalar_chip.rns().max_remainder.clone() + big_uint::from(20u32) + }; + let s = if self.valid_input { + let s = format!("{:?}", self.s); + big_uint::from_str_radix(&s[2..], 16).unwrap() + } else { + scalar_chip.rns().max_remainder.clone() + big_uint::from(20u32) + }; + if r > scalar_chip.rns().max_remainder || s > scalar_chip.rns().max_remainder { + is_valid = scalar_chip.assign_constant(ctx, (0 as u64).into())?; + } + let is_valid = scalar_chip.is_not_zero(ctx, &is_valid)?; + let r = self.signature.map(|signature| signature.0); let s = self.signature.map(|signature| signature.1); let integer_r = ecc_chip.new_unassigned_scalar(r); @@ -286,12 +311,12 @@ mod tests { let msg_hash = ecc_chip.new_unassigned_scalar(self.msg_hash); let r_assigned = - scalar_chip.assign_integer(ctx, integer_r, Range::Remainder)?; + scalar_chip.try_assign_integer(ctx, integer_r, Range::Remainder)?; let s_assigned = - scalar_chip.assign_integer(ctx, integer_s, Range::Remainder)?; + scalar_chip.try_assign_integer(ctx, integer_s, Range::Remainder)?; let sig = AssignedEcdsaSig { - r: r_assigned, - s: s_assigned, + r: r_assigned.0, + s: s_assigned.0, }; let point = self.public_key.map(|point| ecc_chip.to_rns_point(point)); @@ -306,9 +331,11 @@ mod tests { }; let (pk_in_circuit, is_pk_on_curve) = ecc_chip.assign_x_y(ctx, x.into(), y.into())?; + let is_valid = scalar_chip.and(ctx, &is_valid, &is_pk_on_curve)?; + let enable_skipping_invalid_signature = scalar_chip.assign_constant(ctx, (self.enable_skipping_invalid_signature as u64).into())?; let enable_skipping_invalid_signature = scalar_chip.is_not_zero(ctx, &enable_skipping_invalid_signature)?; - scalar_chip.one_or_one(ctx, &enable_skipping_invalid_signature, &is_pk_on_curve)?; + scalar_chip.one_or_one(ctx, &enable_skipping_invalid_signature, &is_valid)?; let pk_assigned = AssignedPublicKey { point: pk_in_circuit, @@ -386,12 +413,14 @@ mod tests { let aux_generator = C::CurveExt::random(OsRng).to_affine(); let circuit = TestCircuitEcdsaVerify:: { public_key: Value::known(public_key), - signature: Value::known((r, s)), + signature: Value::known((r.clone(), s.clone())), msg_hash: Value::known(msg_hash), aux_generator, window_size: 4, valid_input, enable_skipping_invalid_signature, + r: r.clone(), + s: s.clone(), ..Default::default() }; let instance = vec![vec![]]; diff --git a/integer/src/chip.rs b/integer/src/chip.rs index c4446fc9..6a0c8d13 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -438,6 +438,17 @@ impl, + a: &AssignedInteger, + ) -> Result, Error> { + let main_gate = self.main_gate(); + let zero = self.assign_constant(ctx, W::ZERO)?; + let is_zero = self.is_strict_equal(ctx, &zero, &a)?; + main_gate.not(ctx, &is_zero) + } + fn one_or_one( &self, ctx: &mut RegionCtx<'_, N>, diff --git a/integer/src/chip/assign.rs b/integer/src/chip/assign.rs index be8f3db4..261c7b4d 100644 --- a/integer/src/chip/assign.rs +++ b/integer/src/chip/assign.rs @@ -3,6 +3,7 @@ use crate::rns::{Common, Integer}; use crate::{AssignedInteger, AssignedLimb, UnassignedInteger}; use halo2::halo2curves::ff::PrimeField; use halo2::plonk::Error; +use halo2::circuit::Value; use maingate::{fe_to_big, halo2, AssignedCondition, MainGateInstructions, RangeInstructions, RegionCtx, Term}; use num_bigint::BigUint as big_uint; use num_traits::One; @@ -21,6 +22,8 @@ impl self.rns.max_most_significant_operand_limb.bits(), Range::Remainder => self.rns.max_most_significant_reduced_limb.bits(), @@ -88,9 +91,11 @@ impl, a: &AssignedInteger, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - let one = self.assign_constant(ctx, W::ONE)?; - let one = self.is_strict_equal(ctx, &one.clone(), &one)?; let exceeds_max_limb_value = a .limbs .iter() .any(|limb| limb.max_val() > self.rns.max_reduced_limb); + + // Soft sanity check for completeness + // Reduction quotient is limited upto a dense single limb. It is quite possible + // to make it more than a single limb. However even single limb will + // support quite amount of lazy additions and make reduction process + // much easier. + let max_reduction_quotient = self.rns.max_reduced_limb.clone(); + let max_reducible_value = + max_reduction_quotient * &self.rns.wrong_modulus + &self.rns.max_remainder; + let is_valid = self.assign_constant(ctx, ((a.max_val() < max_reducible_value) as u64).into())?; + let is_valid = self.is_not_zero_without_reduce(ctx, &is_valid)?; + if exceeds_max_limb_value { - self.reduce(ctx, a) + let (result, is_reduce_succeeded) = self.reduce(ctx, a)?; + let is_valid = self.and(ctx, &is_valid, &is_reduce_succeeded)?; + Ok((result, is_valid)) } else { - Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), one)) + let zero = self.assign_constant(ctx, W::ZERO)?; + let zero = self.is_strict_equal(ctx, &zero.clone(), &zero)?; + Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), zero)) } } @@ -125,13 +139,27 @@ impl, a: &AssignedInteger, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - let one = self.assign_constant(ctx, W::ONE)?; - let one = self.is_strict_equal(ctx, &one.clone(), &one)?; let exceeds_max_value = a.max_val() > self.rns.max_operand; + + // Soft sanity check for completeness + // Reduction quotient is limited upto a dense single limb. It is quite possible + // to make it more than a single limb. However even single limb will + // support quite amount of lazy additions and make reduction process + // much easier. + let max_reduction_quotient = self.rns.max_reduced_limb.clone(); + let max_reducible_value = + max_reduction_quotient * &self.rns.wrong_modulus + &self.rns.max_remainder; + let is_valid = self.assign_constant(ctx, ((a.max_val() < max_reducible_value) as u64).into())?; + let is_valid = self.is_not_zero_without_reduce(ctx, &is_valid)?; + if exceeds_max_value { - self.reduce(ctx, a) + let (result, is_reduce_succeeded) = self.reduce(ctx, a)?; + let is_valid = self.and(ctx, &is_valid, &is_reduce_succeeded)?; + Ok((result, is_valid)) } else { - Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), one)) + let zero = self.assign_constant(ctx, W::ZERO)?; + let zero = self.is_strict_equal(ctx, &zero.clone(), &zero)?; + Ok((self.new_assigned_integer(a.limbs(), a.native().clone()), zero)) } } @@ -155,6 +183,7 @@ impl, ) -> Result, Error>; + /// Check constraints that an [`AssignedInteger`] is not equal to zero + fn is_not_zero_without_reduce( + &self, + ctx: &mut RegionCtx<'_, N>, + a: &AssignedInteger, + ) -> Result, Error>; + /// Enforces one of given two values is `1` /// `(a-1) * (b-1) = 0` fn one_or_one( From fcdc4b73b4a8e66e531c0514ba74566255898de9 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Thu, 25 May 2023 01:15:03 -0700 Subject: [PATCH 26/37] Update --- ecdsa/src/ecdsa.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 1e016570..e92fe43c 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -310,26 +310,21 @@ mod tests { let integer_s = ecc_chip.new_unassigned_scalar(s); let msg_hash = ecc_chip.new_unassigned_scalar(self.msg_hash); - let r_assigned = + let (r_assigned, is_assigned_integer_succeeded ) = scalar_chip.try_assign_integer(ctx, integer_r, Range::Remainder)?; - let s_assigned = + let is_valid = scalar_chip.and(ctx, &is_valid, &is_assigned_integer_succeeded)?; + let (s_assigned, is_assigned_integer_succeeded) = scalar_chip.try_assign_integer(ctx, integer_s, Range::Remainder)?; + let is_valid = scalar_chip.and(ctx, &is_valid, &is_assigned_integer_succeeded)?; let sig = AssignedEcdsaSig { - r: r_assigned.0, - s: s_assigned.0, + r: r_assigned, + s: s_assigned, }; let point = self.public_key.map(|point| ecc_chip.to_rns_point(point)); let (x, y) = point .map(|point| (point.x().clone(), point.y().clone())) .unzip(); - let (x, y) = if self.valid_input { - (x.clone(), y.clone()) - } else { - // Generate a point that is not on the curve. - (x.clone(), x.clone()) - }; - let (pk_in_circuit, is_pk_on_curve) = ecc_chip.assign_x_y(ctx, x.into(), y.into())?; let is_valid = scalar_chip.and(ctx, &is_valid, &is_pk_on_curve)?; From 03745af81f92c713b98181f5fd590ce09e37cfba Mon Sep 17 00:00:00 2001 From: xiaodino Date: Thu, 25 May 2023 07:07:21 -0700 Subject: [PATCH 27/37] Update --- ecdsa/src/ecdsa.rs | 9 ++------- integer/src/chip/assign.rs | 2 -- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index e92fe43c..68b7ea6a 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -176,7 +176,6 @@ mod tests { use num_bigint::BigUint as big_uint; - // const BIT_LEN_LIMB: usize = 64; const BIT_LEN_LIMB: usize = 68; const NUMBER_OF_LIMBS: usize = 4; @@ -284,8 +283,6 @@ mod tests { let offset = 0; let ctx = &mut RegionCtx::new(region, offset); - let mut is_valid = scalar_chip.assign_constant(ctx, (1 as u64).into())?; - // r and s should be less than Remiander let r = if self.valid_input { let r = format!("{:?}", self.r); @@ -299,10 +296,8 @@ mod tests { } else { scalar_chip.rns().max_remainder.clone() + big_uint::from(20u32) }; - if r > scalar_chip.rns().max_remainder || s > scalar_chip.rns().max_remainder { - is_valid = scalar_chip.assign_constant(ctx, (0 as u64).into())?; - } - let is_valid = scalar_chip.is_not_zero(ctx, &is_valid)?; + let is_r_s_within_ranage = scalar_chip.assign_constant(ctx, ((r <= scalar_chip.rns().max_remainder && s <= scalar_chip.rns().max_remainder) as u64).into())?; + let is_valid = scalar_chip.is_not_zero_without_reduce(ctx, &is_r_s_within_ranage)?; let r = self.signature.map(|signature| signature.0); let s = self.signature.map(|signature| signature.1); diff --git a/integer/src/chip/assign.rs b/integer/src/chip/assign.rs index 261c7b4d..7ddc8334 100644 --- a/integer/src/chip/assign.rs +++ b/integer/src/chip/assign.rs @@ -18,7 +18,6 @@ impl, range: Range, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - // ) -> Result, Error> { let range_chip = self.range_chip(); let main_gate = self.main_gate(); @@ -96,7 +95,6 @@ impl Date: Sat, 27 May 2023 00:04:09 -0700 Subject: [PATCH 28/37] Refactor --- ecdsa/src/ecdsa.rs | 54 +++++++++++++++++++---------- integer/src/chip.rs | 7 ++-- integer/src/chip/assert_in_field.rs | 5 ++- integer/src/chip/assign.rs | 22 ++++++++++++ integer/src/rns.rs | 3 +- maingate/src/range.rs | 7 +++- 6 files changed, 74 insertions(+), 24 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 68b7ea6a..5ed5b81a 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -283,21 +283,36 @@ mod tests { let offset = 0; let ctx = &mut RegionCtx::new(region, offset); - // r and s should be less than Remiander - let r = if self.valid_input { + // r and s should be less than Remainder + let r_test = if self.valid_input { let r = format!("{:?}", self.r); big_uint::from_str_radix(&r[2..], 16).unwrap() } else { + // Test data where r is larger than Remainder scalar_chip.rns().max_remainder.clone() + big_uint::from(20u32) }; - let s = if self.valid_input { + let s_test = if self.valid_input { let s = format!("{:?}", self.s); big_uint::from_str_radix(&s[2..], 16).unwrap() } else { + // Test data where s is larger than Remainder scalar_chip.rns().max_remainder.clone() + big_uint::from(20u32) }; - let is_r_s_within_ranage = scalar_chip.assign_constant(ctx, ((r <= scalar_chip.rns().max_remainder && s <= scalar_chip.rns().max_remainder) as u64).into())?; - let is_valid = scalar_chip.is_not_zero_without_reduce(ctx, &is_r_s_within_ranage)?; + println!("scalar_chip.rns().max_remainder {:?}", scalar_chip.rns().max_remainder.clone()); + + let invalid_signature = Value::known((self.r.clone(), self.s.clone())); + let r = invalid_signature.map(|signature| signature.0 + signature.0); + println!("r {:?}", r); + let s = invalid_signature.map(|signature| signature.1); + let integer_r = ecc_chip.new_unassigned_scalar(r); + let (r_assigne, is_valid ) = + scalar_chip.try_assign_integer(ctx, integer_r, Range::Remainder)?; + + // let is_r_s_within_ranage = scalar_chip.assign_constant(ctx, ((r <= scalar_chip.rns().max_remainder && s <= scalar_chip.rns().max_remainder) as u64).into())?; + // let test = true; + // let is_r_s_within_ranage = scalar_chip.assign_constant(ctx, (test as u64).into())?; + + // let is_valid = scalar_chip.is_not_zero_without_reduce(ctx, &is_r_s_within_ranage)?; let r = self.signature.map(|signature| signature.0); let s = self.signature.map(|signature| signature.1); @@ -389,8 +404,8 @@ mod tests { } fn generate_invalid_inputs + Ord>() -> (C, C::Scalar, C::Scalar, C::Scalar) { - let (public_key, _, s, msg_hash) = generate_valid_inputs::(); - (public_key, s, s, msg_hash) + let (public_key, r, s, msg_hash) = generate_valid_inputs::(); + (public_key, r, s, msg_hash) } fn run + Ord>(valid_input: bool, enable_skipping_invalid_signature: bool) { @@ -418,7 +433,8 @@ mod tests { if valid_input || enable_skipping_invalid_signature { assert_eq!(result, Ok(())); } else { - assert!(result.is_err()); + // assert!(result.is_err()); + assert_eq!(result, Ok(())); } } @@ -428,19 +444,19 @@ mod tests { // Return Errors run::(false, false); - run::(false, false); - run::(false, false); + // run::(false, false); + // run::(false, false); - run::(false, true); - run::(false, true); - run::(false, true); + // run::(false, true); + // run::(false, true); + // run::(false, true); - run::(true, false); - run::(true, false); - run::(true, false); + // run::(true, false); + // run::(true, false); + // run::(true, false); - run::(true, true); - run::(true, true); - run::(true, true); + // run::(true, true); + // run::(true, true); + // run::(true, true); } } diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 6a0c8d13..8bcd88a1 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -108,7 +108,9 @@ impl, range: Range, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - self.assign_integer_generic(ctx, integer, range) + let (result, succeeded) = self.assign_integer_generic(ctx, integer, range)?; + self.assert_in_field(ctx, &result)?; + Ok((result, succeeded)) } fn assign_constant( @@ -577,7 +579,8 @@ impl Result<(), Error> { let a = &self.reduce_if_limb_values_exceeds_reduced(ctx, a)?; let a = &self.reduce_if_max_operand_value_exceeds(ctx, a)?; - self.assert_in_field_generic(ctx, a) + let result = self.assert_in_field_generic(ctx, a)?; + Ok(()) } fn sign( diff --git a/integer/src/chip/assert_in_field.rs b/integer/src/chip/assert_in_field.rs index 8e13d4cd..e240f48f 100644 --- a/integer/src/chip/assert_in_field.rs +++ b/integer/src/chip/assert_in_field.rs @@ -2,7 +2,7 @@ use super::{IntegerChip, Range}; use crate::{AssignedInteger, PrimeField}; use halo2::plonk::Error; use maingate::{ - halo2, AssignedValue, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term, + halo2, AssignedCondition, AssignedValue, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term, }; impl @@ -12,6 +12,7 @@ impl, input: &AssignedInteger, + // ) -> Result<(AssignedCondition), Error> { ) -> Result<(), Error> { // Constraints for `NUMBER_OF_LIMBS = 4` // 0 = -c_0 + p_0 - a_0 + b_0 * R @@ -51,6 +52,7 @@ impl 0, } as usize; + + match range { + Range::Remainder => { + let _ = integer.clone().0.map(|int| { + let max = self.rns.max_remainder.clone(); + let in_range = int.value() <= max; + if !in_range { + println!("max {:?} int {:?}", max, int.value().clone()); + println!("in_range {:?}", in_range); + + } + // let in_range = main_gate.assign_constant(ctx, (in_range as u64).into()).unwrap(); + // is_valid = main_gate.and(ctx, &is_valid, &in_range).unwrap(); + }); + }, + _ => {} + }; + + + // let comparision_witness = integer.0.as_ref().map(|integer| integer.compare_to_modulus()); + + let max_val_msb = (big_uint::one() << bit_len_limb_msb) - 1usize; let max_val = (big_uint::one() << BIT_LEN_LIMB) - 1usize; diff --git a/integer/src/rns.rs b/integer/src/rns.rs index c011be79..377b565e 100644 --- a/integer/src/rns.rs +++ b/integer/src/rns.rs @@ -366,7 +366,8 @@ impl RangeChip { let (s_overflow, tag_overflow) = if !overflow_bit_lens.is_empty() { let s_overflow = meta.complex_selector(); let tag_overflow = if overflow_bit_lens.len() > 1 { + /* let tag = meta.fixed_column(); Self::configure_lookup_with_column_tag( meta, @@ -308,8 +309,11 @@ impl RangeChip { t_tag, t_value, ); - Some(tag) + */ + // Some(tag) + None } else { + /* Self::configure_lookup_with_constant_tag( meta, "overflow_a", @@ -319,6 +323,7 @@ impl RangeChip { t_tag, t_value, ); + */ None }; From 73df0e6583b45338ad8c43107af6fa66332cb4c6 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Mon, 29 May 2023 00:35:20 -0700 Subject: [PATCH 29/37] Refactor --- ecc/src/general_ecc.rs | 2 ++ ecdsa/src/ecdsa.rs | 52 +++++++++----------------------------- halo2wrong/src/utils.rs | 4 +++ integer/src/chip/assign.rs | 22 ---------------- integer/src/rns.rs | 4 +-- 5 files changed, 20 insertions(+), 64 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index 2ed017ac..40fc9227 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -12,6 +12,8 @@ use maingate::{AssignedCondition, MainGate}; use std::collections::BTreeMap; use std::rc::Rc; +use num_bigint::BigUint as big_uint; + mod add; mod mul; diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 5ed5b81a..30eba200 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -155,6 +155,7 @@ mod tests { use crate::maingate; use ecc::integer::Range; use ecc::maingate::big_to_fe; + use ecc::maingate::big_to_fe_without_modulus; use ecc::maingate::fe_to_big; use ecc::maingate::RegionCtx; use ecc::{EccConfig, GeneralEccChip}; @@ -168,14 +169,11 @@ mod tests { use integer::IntegerInstructions; use maingate::mock_prover_verify; use maingate::{MainGate, MainGateConfig, RangeChip, RangeConfig, RangeInstructions}; - use num_traits::Num; use rand_core::OsRng; use std::fmt::{Debug}; use std::marker::PhantomData; - use num_bigint::BigUint as big_uint; - const BIT_LEN_LIMB: usize = 68; const NUMBER_OF_LIMBS: usize = 4; @@ -233,8 +231,6 @@ mod tests { valid_input: bool, enable_skipping_invalid_signature: bool, - r: E::Scalar, - s: E::Scalar, _marker: PhantomData, } @@ -283,38 +279,17 @@ mod tests { let offset = 0; let ctx = &mut RegionCtx::new(region, offset); - // r and s should be less than Remainder - let r_test = if self.valid_input { - let r = format!("{:?}", self.r); - big_uint::from_str_radix(&r[2..], 16).unwrap() - } else { - // Test data where r is larger than Remainder - scalar_chip.rns().max_remainder.clone() + big_uint::from(20u32) - }; - let s_test = if self.valid_input { - let s = format!("{:?}", self.s); - big_uint::from_str_radix(&s[2..], 16).unwrap() + let is_valid = scalar_chip.assign_constant(ctx, (true as u64).into())?; + let is_valid = scalar_chip.is_not_zero(ctx, &is_valid)?; + + let r = if self.valid_input { + self.signature.map(|signature| signature.0) } else { - // Test data where s is larger than Remainder - scalar_chip.rns().max_remainder.clone() + big_uint::from(20u32) + let max_remainder = scalar_chip.rns().max_remainder.clone() + 10usize; + let r: E::Scalar = big_to_fe_without_modulus(max_remainder); + Value::known(r) }; - println!("scalar_chip.rns().max_remainder {:?}", scalar_chip.rns().max_remainder.clone()); - let invalid_signature = Value::known((self.r.clone(), self.s.clone())); - let r = invalid_signature.map(|signature| signature.0 + signature.0); - println!("r {:?}", r); - let s = invalid_signature.map(|signature| signature.1); - let integer_r = ecc_chip.new_unassigned_scalar(r); - let (r_assigne, is_valid ) = - scalar_chip.try_assign_integer(ctx, integer_r, Range::Remainder)?; - - // let is_r_s_within_ranage = scalar_chip.assign_constant(ctx, ((r <= scalar_chip.rns().max_remainder && s <= scalar_chip.rns().max_remainder) as u64).into())?; - // let test = true; - // let is_r_s_within_ranage = scalar_chip.assign_constant(ctx, (test as u64).into())?; - - // let is_valid = scalar_chip.is_not_zero_without_reduce(ctx, &is_r_s_within_ranage)?; - - let r = self.signature.map(|signature| signature.0); let s = self.signature.map(|signature| signature.1); let integer_r = ecc_chip.new_unassigned_scalar(r); let integer_s = ecc_chip.new_unassigned_scalar(s); @@ -404,8 +379,8 @@ mod tests { } fn generate_invalid_inputs + Ord>() -> (C, C::Scalar, C::Scalar, C::Scalar) { - let (public_key, r, s, msg_hash) = generate_valid_inputs::(); - (public_key, r, s, msg_hash) + let (public_key, r, _, msg_hash) = generate_valid_inputs::(); + (public_key, r, r, msg_hash) } fn run + Ord>(valid_input: bool, enable_skipping_invalid_signature: bool) { @@ -424,8 +399,6 @@ mod tests { window_size: 4, valid_input, enable_skipping_invalid_signature, - r: r.clone(), - s: s.clone(), ..Default::default() }; let instance = vec![vec![]]; @@ -433,8 +406,7 @@ mod tests { if valid_input || enable_skipping_invalid_signature { assert_eq!(result, Ok(())); } else { - // assert!(result.is_err()); - assert_eq!(result, Ok(())); + assert!(result.is_err()); } } diff --git a/halo2wrong/src/utils.rs b/halo2wrong/src/utils.rs index 111ec942..a6a9b72f 100644 --- a/halo2wrong/src/utils.rs +++ b/halo2wrong/src/utils.rs @@ -30,6 +30,10 @@ pub fn big_to_fe(e: big_uint) -> F { F::from_str_vartime(&e.to_str_radix(10)[..]).unwrap() } +pub fn big_to_fe_without_modulus(e: big_uint) -> F { + F::from_str_vartime(&e.to_str_radix(10)[..]).unwrap() +} + pub fn fe_to_big(fe: F) -> big_uint { big_uint::from_bytes_le(fe.to_repr().as_ref()) } diff --git a/integer/src/chip/assign.rs b/integer/src/chip/assign.rs index c4553ca6..7ddc8334 100644 --- a/integer/src/chip/assign.rs +++ b/integer/src/chip/assign.rs @@ -30,28 +30,6 @@ impl 0, } as usize; - - match range { - Range::Remainder => { - let _ = integer.clone().0.map(|int| { - let max = self.rns.max_remainder.clone(); - let in_range = int.value() <= max; - if !in_range { - println!("max {:?} int {:?}", max, int.value().clone()); - println!("in_range {:?}", in_range); - - } - // let in_range = main_gate.assign_constant(ctx, (in_range as u64).into()).unwrap(); - // is_valid = main_gate.and(ctx, &is_valid, &in_range).unwrap(); - }); - }, - _ => {} - }; - - - // let comparision_witness = integer.0.as_ref().map(|integer| integer.compare_to_modulus()); - - let max_val_msb = (big_uint::one() << bit_len_limb_msb) - 1usize; let max_val = (big_uint::one() << BIT_LEN_LIMB) - 1usize; diff --git a/integer/src/rns.rs b/integer/src/rns.rs index 377b565e..23f9412a 100644 --- a/integer/src/rns.rs +++ b/integer/src/rns.rs @@ -366,8 +366,8 @@ impl Date: Tue, 30 May 2023 00:05:00 -0700 Subject: [PATCH 30/37] Refactor --- ecc/src/general_ecc.rs | 9 +++++++++ ecdsa/src/ecdsa.rs | 15 +++++++-------- integer/src/chip.rs | 4 +--- integer/src/chip/assert_in_field.rs | 5 +---- integer/src/rns.rs | 1 - maingate/src/range.rs | 7 +------ 6 files changed, 19 insertions(+), 22 deletions(-) diff --git a/ecc/src/general_ecc.rs b/ecc/src/general_ecc.rs index 40fc9227..6099d875 100644 --- a/ecc/src/general_ecc.rs +++ b/ecc/src/general_ecc.rs @@ -99,6 +99,15 @@ impl< e.map(|e| Integer::from_fe(e, self.rns_scalar())).into() } + /// Assign integer for chip + pub fn new_unassigned_big( + &self, + e: big_uint, + ) -> UnassignedInteger { + let big = Integer::from_big(e, self.rns_scalar()); + Value::known(big).into() + } + /// Return `IntegerChip` for the base field of the EC pub fn base_field_chip( &self, diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 30eba200..d794897e 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -282,16 +282,15 @@ mod tests { let is_valid = scalar_chip.assign_constant(ctx, (true as u64).into())?; let is_valid = scalar_chip.is_not_zero(ctx, &is_valid)?; - let r = if self.valid_input { - self.signature.map(|signature| signature.0) + let r = self.signature.map(|signature| signature.0); + let s = self.signature.map(|signature| signature.1); + + let integer_r = if self.valid_input { + ecc_chip.new_unassigned_scalar(r.clone()) } else { - let max_remainder = scalar_chip.rns().max_remainder.clone() + 10usize; - let r: E::Scalar = big_to_fe_without_modulus(max_remainder); - Value::known(r) + let max_reminder = scalar_chip.rns().max_remainder.clone() + 10usize; + ecc_chip.new_unassigned_big(max_reminder) }; - - let s = self.signature.map(|signature| signature.1); - let integer_r = ecc_chip.new_unassigned_scalar(r); let integer_s = ecc_chip.new_unassigned_scalar(s); let msg_hash = ecc_chip.new_unassigned_scalar(self.msg_hash); diff --git a/integer/src/chip.rs b/integer/src/chip.rs index 8bcd88a1..0753638b 100644 --- a/integer/src/chip.rs +++ b/integer/src/chip.rs @@ -109,7 +109,6 @@ impl Result<(AssignedInteger, AssignedCondition), Error> { let (result, succeeded) = self.assign_integer_generic(ctx, integer, range)?; - self.assert_in_field(ctx, &result)?; Ok((result, succeeded)) } @@ -579,8 +578,7 @@ impl Result<(), Error> { let a = &self.reduce_if_limb_values_exceeds_reduced(ctx, a)?; let a = &self.reduce_if_max_operand_value_exceeds(ctx, a)?; - let result = self.assert_in_field_generic(ctx, a)?; - Ok(()) + self.assert_in_field_generic(ctx, a) } fn sign( diff --git a/integer/src/chip/assert_in_field.rs b/integer/src/chip/assert_in_field.rs index e240f48f..8e13d4cd 100644 --- a/integer/src/chip/assert_in_field.rs +++ b/integer/src/chip/assert_in_field.rs @@ -2,7 +2,7 @@ use super::{IntegerChip, Range}; use crate::{AssignedInteger, PrimeField}; use halo2::plonk::Error; use maingate::{ - halo2, AssignedCondition, AssignedValue, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term, + halo2, AssignedValue, CombinationOptionCommon, MainGateInstructions, RegionCtx, Term, }; impl @@ -12,7 +12,6 @@ impl, input: &AssignedInteger, - // ) -> Result<(AssignedCondition), Error> { ) -> Result<(), Error> { // Constraints for `NUMBER_OF_LIMBS = 4` // 0 = -c_0 + p_0 - a_0 + b_0 * R @@ -52,7 +51,6 @@ impl RangeChip { let (s_overflow, tag_overflow) = if !overflow_bit_lens.is_empty() { let s_overflow = meta.complex_selector(); let tag_overflow = if overflow_bit_lens.len() > 1 { - /* let tag = meta.fixed_column(); Self::configure_lookup_with_column_tag( meta, @@ -309,11 +308,8 @@ impl RangeChip { t_tag, t_value, ); - */ - // Some(tag) - None + Some(tag) } else { - /* Self::configure_lookup_with_constant_tag( meta, "overflow_a", @@ -323,7 +319,6 @@ impl RangeChip { t_tag, t_value, ); - */ None }; From 581500f4c116f7e2a81679b3ff236fcf0987e117 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Tue, 30 May 2023 00:06:59 -0700 Subject: [PATCH 31/37] Refactor --- ecdsa/src/ecdsa.rs | 22 +++++++++++----------- maingate/src/instructions.rs | 1 + 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index d794897e..bb554f21 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -415,19 +415,19 @@ mod tests { // Return Errors run::(false, false); - // run::(false, false); - // run::(false, false); + run::(false, false); + run::(false, false); - // run::(false, true); - // run::(false, true); - // run::(false, true); + run::(false, true); + run::(false, true); + run::(false, true); - // run::(true, false); - // run::(true, false); - // run::(true, false); + run::(true, false); + run::(true, false); + run::(true, false); - // run::(true, true); - // run::(true, true); - // run::(true, true); + run::(true, true); + run::(true, true); + run::(true, true); } } diff --git a/maingate/src/instructions.rs b/maingate/src/instructions.rs index 2bb07069..0f19a4e6 100644 --- a/maingate/src/instructions.rs +++ b/maingate/src/instructions.rs @@ -1095,6 +1095,7 @@ pub trait MainGateInstructions: Chip { ) -> Result, Error> { assert!(!terms.is_empty(), "At least one term is expected"); let (composed, _) = self.decompose(ctx, terms, constant, |_, _| Ok(()))?; + Ok(composed) } From 10fed9bcad54570d25c39801387a19613d689651 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Tue, 30 May 2023 13:37:51 -0700 Subject: [PATCH 32/37] Update --- ecdsa/src/ecdsa.rs | 24 ++++++++++++------------ maingate/src/range.rs | 1 + 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index bb554f21..ce309673 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -288,7 +288,7 @@ mod tests { let integer_r = if self.valid_input { ecc_chip.new_unassigned_scalar(r.clone()) } else { - let max_reminder = scalar_chip.rns().max_remainder.clone() + 10usize; + let max_reminder = scalar_chip.rns().max_remainder.clone() + 1usize; ecc_chip.new_unassigned_big(max_reminder) }; let integer_s = ecc_chip.new_unassigned_scalar(s); @@ -414,20 +414,20 @@ mod tests { use crate::curves::secp256k1::Secp256k1Affine as Secp256k1; // Return Errors - run::(false, false); - run::(false, false); - run::(false, false); + // run::(false, false); + // run::(false, false); + // run::(false, false); run::(false, true); - run::(false, true); - run::(false, true); + // run::(false, true); + // run::(false, true); - run::(true, false); - run::(true, false); - run::(true, false); + // run::(true, false); + // run::(true, false); + // run::(true, false); - run::(true, true); - run::(true, true); - run::(true, true); + // run::(true, true); + // run::(true, true); + // run::(true, true); } } diff --git a/maingate/src/range.rs b/maingate/src/range.rs index 0e61d048..73f613fc 100644 --- a/maingate/src/range.rs +++ b/maingate/src/range.rs @@ -297,6 +297,7 @@ impl RangeChip { let (s_overflow, tag_overflow) = if !overflow_bit_lens.is_empty() { let s_overflow = meta.complex_selector(); + println!("overflow_bit_lens {:?}", overflow_bit_lens); let tag_overflow = if overflow_bit_lens.len() > 1 { let tag = meta.fixed_column(); Self::configure_lookup_with_column_tag( From 1438e678dace67dde66392a893f6496adba67067 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Tue, 30 May 2023 14:11:19 -0700 Subject: [PATCH 33/37] update --- ecdsa/src/ecdsa.rs | 2 +- maingate/src/range.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 525b0c52..caa0804d 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -288,7 +288,7 @@ mod tests { let integer_r = if self.valid_input { ecc_chip.new_unassigned_scalar(r.clone()) } else { - let max_reminder = scalar_chip.rns().max_remainder.clone() + 1usize; + let max_reminder = scalar_chip.rns().max_remainder.clone(); ecc_chip.new_unassigned_big(max_reminder) }; let integer_s = ecc_chip.new_unassigned_scalar(s); diff --git a/maingate/src/range.rs b/maingate/src/range.rs index 34275e39..3647209b 100644 --- a/maingate/src/range.rs +++ b/maingate/src/range.rs @@ -297,7 +297,6 @@ impl RangeChip { let (s_overflow, tag_overflow) = if !overflow_bit_lens.is_empty() { let s_overflow = meta.complex_selector(); - println!("overflow_bit_lens {:?}", overflow_bit_lens); let tag_overflow = if overflow_bit_lens.len() > 1 { let tag = meta.fixed_column(); Self::configure_lookup_with_column_tag( From abf5926db9fcc97688c0ebbec2156a32f71a0013 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Sat, 3 Jun 2023 22:25:32 -0700 Subject: [PATCH 34/37] Test --- ecdsa/src/ecdsa.rs | 25 ++++++++++++------------- halo2wrong/src/utils.rs | 4 ---- integer/src/chip/reduce.rs | 1 - 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index caa0804d..081dd5ba 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -141,7 +141,7 @@ impl(false, false); - // run::(false, false); - // run::(false, false); + run::(false, false); + run::(false, false); + run::(false, false); run::(false, true); - // run::(false, true); - // run::(false, true); + run::(false, true); + run::(false, true); - // run::(true, false); - // run::(true, false); - // run::(true, false); + run::(true, false); + run::(true, false); + run::(true, false); - // run::(true, true); - // run::(true, true); - // run::(true, true); + run::(true, true); + run::(true, true); + run::(true, true); } } diff --git a/halo2wrong/src/utils.rs b/halo2wrong/src/utils.rs index a6a9b72f..111ec942 100644 --- a/halo2wrong/src/utils.rs +++ b/halo2wrong/src/utils.rs @@ -30,10 +30,6 @@ pub fn big_to_fe(e: big_uint) -> F { F::from_str_vartime(&e.to_str_radix(10)[..]).unwrap() } -pub fn big_to_fe_without_modulus(e: big_uint) -> F { - F::from_str_vartime(&e.to_str_radix(10)[..]).unwrap() -} - pub fn fe_to_big(fe: F) -> big_uint { big_uint::from_bytes_le(fe.to_repr().as_ref()) } diff --git a/integer/src/chip/reduce.rs b/integer/src/chip/reduce.rs index 94dd99ed..0093f228 100644 --- a/integer/src/chip/reduce.rs +++ b/integer/src/chip/reduce.rs @@ -168,7 +168,6 @@ impl, a: &AssignedInteger, ) -> Result<(AssignedInteger, AssignedCondition), Error> { - // ) -> Result, Error> { let main_gate = self.main_gate(); let (zero, one) = (N::ZERO, N::ONE); From 0abd5b9c059eefd502e3f723cbf7b29af4c5cad4 Mon Sep 17 00:00:00 2001 From: xiaodino Date: Sat, 3 Jun 2023 22:46:54 -0700 Subject: [PATCH 35/37] Test --- ecdsa/src/ecdsa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index 081dd5ba..5887d2f9 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -141,7 +141,7 @@ impl Date: Thu, 15 Jun 2023 00:29:04 -0700 Subject: [PATCH 36/37] Update --- ecdsa/src/ecdsa.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index f2db0c1d..f1668a32 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -137,12 +137,15 @@ impl(false, false); - run::(false, false); - run::(false, false); + // run::(false, false); + // run::(false, false); - run::(false, true); - run::(false, true); - run::(false, true); + // run::(false, true); + // run::(false, true); + // run::(false, true); - run::(true, false); - run::(true, false); - run::(true, false); + // run::(true, false); + // run::(true, false); + // run::(true, false); - run::(true, true); - run::(true, true); - run::(true, true); + // run::(true, true); + // run::(true, true); + // run::(true, true); } } From 082a983a91fa0a527f78c38b51feb1575398065c Mon Sep 17 00:00:00 2001 From: xiaodino Date: Thu, 15 Jun 2023 00:49:07 -0700 Subject: [PATCH 37/37] Update --- ecdsa/src/ecdsa.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/ecdsa/src/ecdsa.rs b/ecdsa/src/ecdsa.rs index f1668a32..9e29a634 100644 --- a/ecdsa/src/ecdsa.rs +++ b/ecdsa/src/ecdsa.rs @@ -143,9 +143,6 @@ impl(false, false); - // run::(false, false); - // run::(false, false); + run::(false, false); + run::(false, false); - // run::(false, true); - // run::(false, true); - // run::(false, true); + run::(false, true); + run::(false, true); + run::(false, true); - // run::(true, false); - // run::(true, false); - // run::(true, false); + run::(true, false); + run::(true, false); + run::(true, false); - // run::(true, true); - // run::(true, true); - // run::(true, true); + run::(true, true); + run::(true, true); + run::(true, true); } }