Based on v_2022_10_22 - Return VerifyFailure::ConstraintNotSatisfied with offset

ecc/src/base_field_ecc.rs

@@ -263,7 +263,7 @@ impl<C: CurveAffine, const NUMBER_OF_LIMBS: usize, const BIT_LEN_LIMB: usize>
         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`

ecc/src/general_ecc.rs

@@ -7,11 +7,13 @@ 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;
 
+use num_bigint::BigUint as big_uint;
+
 mod add;
 mod mul;
 
@@ -97,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<Emulated::Scalar, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB> {
+        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,
@@ -171,7 +182,6 @@ impl<
         point: AssignedPoint<Emulated::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
         offset: usize,
     ) -> Result<(), Error> {
-        use integer::maingate::MainGateInstructions;
         let main_gate = self.main_gate();
 
         let mut offset = offset;
@@ -209,19 +219,34 @@ impl<
         ctx: &mut RegionCtx<'_, N>,
         point: Value<Emulated>,
     ) -> Result<AssignedPoint<Emulated::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>, Error> {
-        let integer_chip = self.base_field_chip();
+        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 x = integer_chip.assign_integer(ctx, x.into(), Range::Remainder)?;
-        let y = integer_chip.assign_integer(ctx, y.into(), Range::Remainder)?;
+        let (point, is_on_curve) = self.assign_x_y(ctx, x.into(), y.into())?;
+        maingate.assert_not_zero(ctx, &is_on_curve)?;
+        Ok(point)
+    }
+
+    /// 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<<Emulated as CurveAffine>::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
+        y: UnassignedInteger<<Emulated as CurveAffine>::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
+    ) -> Result<(AssignedPoint<Emulated::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>, AssignedCondition<N>), Error> {
+        let integer_chip = self.base_field_chip();
 
+        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);
-        self.assert_is_on_curve(ctx, &point)?;
-        Ok(point)
+        let is_on_curve = self.is_on_curve(ctx, &point)?;
+        let is_valid = integer_chip.and(ctx, &is_valid, &is_on_curve)?;
+        Ok((point, is_valid))
     }
 
     /// Assigns the auxiliary generator point
@@ -272,6 +297,21 @@ impl<
         Ok(())
     }
 
+    /// Constraints to check if `AssignedPoint` is on curve
+    pub fn is_on_curve(
+        &self,
+        ctx: &mut RegionCtx<'_, N>,
+        point: &AssignedPoint<Emulated::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
+    ) -> Result<AssignedCondition<N>, 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,
@@ -323,7 +363,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`

ecdsa/src/ecdsa.rs

@@ -9,7 +9,7 @@ use halo2::halo2curves::ff::PrimeField;
 use halo2::{circuit::Value, plonk::Error};
 use integer::rns::Integer;
 use integer::{AssignedInteger, IntegerInstructions};
-use maingate::{MainGateConfig, RangeConfig};
+use maingate::{AssignedCondition, MainGateConfig, RangeConfig};
 
 #[derive(Clone, Debug)]
 pub struct EcdsaConfig {
@@ -98,17 +98,19 @@ impl<E: CurveAffine, N: PrimeField, const NUMBER_OF_LIMBS: usize, const BIT_LEN_
         sig: &AssignedEcdsaSig<E::Scalar, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
         pk: &AssignedPublicKey<E::Base, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
         msg_hash: &AssignedInteger<E::Scalar, N, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
-    ) -> Result<(), Error> {
+        enable_skipping_invalid_signature: bool,
+    ) -> Result<AssignedCondition<N>, Error> {
         let ecc_chip = self.ecc_chip();
         let scalar_chip = ecc_chip.scalar_field_chip();
         let base_chip = ecc_chip.base_field_chip();
 
-        // 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`
-        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)?;
 
         // 2. w = s^(-1) (mod n)
         let (s_inv, _) = scalar_chip.invert(ctx, &sig.s)?;
@@ -127,13 +129,21 @@ impl<E: CurveAffine, N: PrimeField, const NUMBER_OF_LIMBS: usize, const BIT_LEN_
         // 6. reduce q_x in E::ScalarExt
         // assuming E::Base/E::ScalarExt have the same number of limbs
         let q_x = q.x();
-        let q_x_reduced_in_q = base_chip.reduce(ctx, q_x)?;
-        let q_x_reduced_in_r = scalar_chip.reduce_external(ctx, &q_x_reduced_in_q)?;
+        let (q_x_reduced_in_q, is_q_x_reduced_in_q_valid) = base_chip.reduce(ctx, q_x)?;
+        let (q_x_reduced_in_r, is_q_x_reduced_in_r_valid) = scalar_chip.reduce_external(ctx, &q_x_reduced_in_q)?;
+        let is_q_x_reduced_valid = scalar_chip.and(ctx, &is_q_x_reduced_in_q_valid, &is_q_x_reduced_in_r_valid)?;
 
         // 7. check if Q.x == r (mod n)
-        scalar_chip.assert_strict_equal(ctx, &q_x_reduced_in_r, &sig.r)?;
-
-        Ok(())
+        let is_q_x_reduced_in_r_equal_to_r = scalar_chip.is_strict_equal(ctx, &q_x_reduced_in_r, &sig.r)?;
+
+        // 8. check if both is_r_s_valid and is_q_x_reduced_in_r_equal_to_r are true to determine overall validity
+        let is_valid = scalar_chip.and(ctx, &is_q_x_reduced_in_r_equal_to_r, &is_r_s_valid)?;
+        let is_valid = scalar_chip.and(ctx, &is_valid, &is_q_x_reduced_valid)?;
+        let enable_skipping_invalid_signature = scalar_chip.assign_constant(ctx, (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_valid)?;
+
+        Ok(is_valid)
     }
 }
 
@@ -159,6 +169,8 @@ mod tests {
     use maingate::mock_prover_verify;
     use maingate::{MainGate, MainGateConfig, RangeChip, RangeConfig, RangeInstructions};
     use rand_core::OsRng;
+
+    use std::fmt::{Debug};
     use std::marker::PhantomData;
 
     const BIT_LEN_LIMB: usize = 68;
@@ -215,6 +227,10 @@ mod tests {
 
         aux_generator: E,
         window_size: usize,
+
+        valid_input: bool,
+        enable_skipping_invalid_signature: bool,
+
         _marker: PhantomData<N>,
     }
 
@@ -262,27 +278,49 @@ mod tests {
                     let offset = 0;
                     let ctx = &mut RegionCtx::new(region, offset);
 
+                    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 = 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_r = if self.valid_input {
+                        ecc_chip.new_unassigned_scalar(r.clone())
+                    } else {
+                        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);
                     let msg_hash = ecc_chip.new_unassigned_scalar(self.msg_hash);
 
-                    let r_assigned =
-                        scalar_chip.assign_integer(ctx, integer_r, Range::Remainder)?;
-                    let s_assigned =
-                        scalar_chip.assign_integer(ctx, integer_s, Range::Remainder)?;
+                    let (r_assigned, is_assigned_integer_succeeded ) =
+                        scalar_chip.try_assign_integer(ctx, integer_r, Range::Remainder)?;
+                    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,
                         s: s_assigned,
                     };
 
-                    let pk_in_circuit = ecc_chip.assign_point(ctx, 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 (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_valid)?;
+
                     let pk_assigned = AssignedPublicKey {
                         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 response = ecdsa_chip.verify(ctx, &sig, &pk_assigned, &msg_hash, self.enable_skipping_invalid_signature);
+                    return response;
                 },
             )?;
 
@@ -299,7 +337,7 @@ mod tests {
             big_to_fe(x_big)
         }
 
-        fn run<C: CurveAffine, N: FromUniformBytes<64> + Ord>() {
+        fn generate_valid_inputs<C: CurveAffine, N: FromUniformBytes<64> + Ord>() -> (C, C::Scalar, C::Scalar, C::Scalar) {
             let g = C::generator();
 
             // Generate a key pair
@@ -335,25 +373,60 @@ mod tests {
                 let r_candidate = mod_n::<C>(*x_candidate);
                 assert_eq!(r, r_candidate);
             }
+            (public_key, r, s, msg_hash)
+        }
+
+        fn generate_invalid_inputs<C: CurveAffine, N: FromUniformBytes<64> + Ord>() -> (C, C::Scalar, C::Scalar, C::Scalar) {
+            let (public_key, r, _, msg_hash) = generate_valid_inputs::<C, N>();
+            (public_key, r, r, msg_hash)
+        }
+
+        fn run<C: CurveAffine, N: FromUniformBytes<64> + Ord>(valid_input: bool, enable_skipping_invalid_signature: bool) {
+            let (public_key, r, s, msg_hash) = if valid_input {
+                generate_valid_inputs::<C, N>()
+            } else {
+                generate_invalid_inputs::<C, N>()
+            };
 
             let aux_generator = C::CurveExt::random(OsRng).to_affine();
             let circuit = TestCircuitEcdsaVerify::<C, N> {
                 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,
                 ..Default::default()
             };
             let instance = vec![vec![]];
-            mock_prover_verify(&circuit, instance);
+            let result = mock_prover_verify(&circuit, instance);
+            if valid_input || enable_skipping_invalid_signature {
+                assert_eq!(result, Ok(()));
+            } else {
+                assert!(result.is_err());
+            }
         }
 
         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::<Secp256k1, BnScalar>();
-        run::<Secp256k1, PastaFp>();
-        run::<Secp256k1, PastaFq>();
+
+        // Return Errors
+        run::<Secp256k1, BnScalar>(false, false);
+        run::<Secp256k1, PastaFp>(false, false);
+        run::<Secp256k1, PastaFq>(false, false);
+
+        run::<Secp256k1, BnScalar>(false, true);
+        run::<Secp256k1, PastaFp>(false, true);
+        run::<Secp256k1, PastaFq>(false, true);
+
+        run::<Secp256k1, BnScalar>(true, false);
+        run::<Secp256k1, PastaFp>(true, false);
+        run::<Secp256k1, PastaFq>(true, false);
+
+        run::<Secp256k1, BnScalar>(true, true);
+        run::<Secp256k1, PastaFp>(true, true);
+        run::<Secp256k1, PastaFq>(true, true);
     }
 }

halo2wrong/src/utils.rs

@@ -2,7 +2,7 @@ use crate::{
     curves::ff::{FromUniformBytes, PrimeField},
     halo2::{
         circuit::Value,
-        dev::MockProver,
+        dev::{MockProver, VerifyFailure},
         plonk::{
             Advice, Any, Assigned, Assignment, Challenge, Circuit, Column, ConstraintSystem, Error,
             Fixed, FloorPlanner, Instance, Selector,
@@ -67,14 +67,11 @@ pub fn compose(input: Vec<big_uint>, bit_len: usize) -> big_uint {
 pub fn mock_prover_verify<F: FromUniformBytes<64> + Ord, C: Circuit<F>>(
     circuit: &C,
     instance: Vec<Vec<F>>,
-) {
+) -> Result<(), Vec<VerifyFailure>> {
     let dimension = DimensionMeasurement::measure(circuit).unwrap();
     let prover = MockProver::run(dimension.k(), circuit, instance)
         .unwrap_or_else(|err| panic!("{:#?}", err));
-    assert_eq!(
-        prover.verify_at_rows_par(dimension.advice_range(), dimension.advice_range()),
-        Ok(())
-    )
+    prover.verify_at_rows_par(dimension.advice_range(), dimension.advice_range())
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]