From f250f4a37b12aff6d18fd4a1ac02310a44cb381a Mon Sep 17 00:00:00 2001 From: Andrew Gu <67293785+MonkeyKing-1@users.noreply.github.com> Date: Thu, 4 Jan 2024 12:42:58 -0500 Subject: [PATCH 1/6] feat: safe ecc_sum and tests --- halo2-ecc/src/bn254/tests/ec_add.rs | 55 +++++++++++++++++++++++++++++ halo2-ecc/src/ecc/mod.rs | 52 +++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/halo2-ecc/src/bn254/tests/ec_add.rs b/halo2-ecc/src/bn254/tests/ec_add.rs index 1df235f1..7ebe731b 100644 --- a/halo2-ecc/src/bn254/tests/ec_add.rs +++ b/halo2-ecc/src/bn254/tests/ec_add.rs @@ -48,6 +48,27 @@ fn g2_add_test( assert_eq!(answer.y, y); } +fn g1_sum_safe_test( + ctx: &mut Context, + range: &RangeChip, + params: CircuitParams, + _points: Vec, +) { + let fp_chip = FpChip::::new(range, params.limb_bits, params.num_limbs); + let g1_chip = EccChip::new(&fp_chip); + + let points = + _points.iter().map(|pt| g1_chip.assign_point_unchecked(ctx, *pt)).collect::>(); + + let acc = g1_chip.sum_safe::(ctx, points); + + let answer = _points.iter().fold(G1Affine::identity(), |a, b| (a + b).to_affine()); + let x = fp_chip.get_assigned_value(&acc.x.into()); + let y = fp_chip.get_assigned_value(&acc.y.into()); + assert_eq!(answer.x, x); + assert_eq!(answer.y, y); +} + #[test] fn test_ec_add() { let path = "configs/bn254/ec_add_circuit.config"; @@ -65,6 +86,40 @@ fn test_ec_add() { .run(|ctx, range| g2_add_test(ctx, range, params, points)); } +#[test] +fn test_ec_sum_safe() { + let path = "configs/bn254/ec_add_circuit.config"; + let params: CircuitParams = serde_json::from_reader( + File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")), + ) + .unwrap(); + + let k = params.degree; + let points = (0..params.batch_size).map(|_| G1Affine::random(OsRng)).collect_vec(); + + base_test() + .k(k) + .lookup_bits(params.lookup_bits) + .run(|ctx, range| g1_sum_safe_test(ctx, range, params, points)); +} + +#[test] +fn test_ec_zero_sum_safe() { + let path = "configs/bn254/ec_add_circuit.config"; + let params: CircuitParams = serde_json::from_reader( + File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")), + ) + .unwrap(); + + let k = params.degree; + let points = (0..params.batch_size).map(|_| G1Affine::identity()).collect_vec(); + + base_test() + .k(k) + .lookup_bits(params.lookup_bits) + .run(|ctx, range| g1_sum_safe_test(ctx, range, params, points)); +} + #[test] fn bench_ec_add() -> Result<(), Box> { let config_path = "configs/bn254/bench_ec_add.config"; diff --git a/halo2-ecc/src/ecc/mod.rs b/halo2-ecc/src/ecc/mod.rs index 14bd0911..c05dfb3b 100644 --- a/halo2-ecc/src/ecc/mod.rs +++ b/halo2-ecc/src/ecc/mod.rs @@ -936,8 +936,18 @@ impl<'chip, F: BigPrimeField, FC: FieldChip> EccChip<'chip, F, FC> { EcPoint::new(P.x, self.field_chip.negate(ctx, P.y)) } + pub fn negate_strict( + &self, + ctx: &mut Context, + P: impl Into>, + ) -> StrictEcPoint { + let P = P.into(); + StrictEcPoint::new(P.x, self.field_chip.negate(ctx, P.y)) + } + /// Assumes that P.x != Q.x /// If `is_strict == true`, then actually constrains that `P.x != Q.x` + /// Neither are points at infinity (otherwise, undefined behavior) pub fn add_unequal( &self, ctx: &mut Context, @@ -980,6 +990,18 @@ impl<'chip, F: BigPrimeField, FC: FieldChip> EccChip<'chip, F, FC> { self.field_chip.range().gate().and(ctx, x_is_equal, y_is_equal) } + /// Checks if a point is the point at infinity (represented by (0, 0)) + pub fn is_infinity( + &self, + ctx: &mut Context, + P: EcPoint, + ) -> AssignedValue { + // TODO: optimize + let x_is_zero = self.field_chip.is_zero(ctx, P.x); + let y_is_zero = self.field_chip.is_zero(ctx, P.y); + self.field_chip.range().gate().and(ctx, x_is_zero, y_is_zero) + } + pub fn assert_equal( &self, ctx: &mut Context, @@ -1014,6 +1036,36 @@ impl<'chip, F: BigPrimeField, FC: FieldChip> EccChip<'chip, F, FC> where FC: Selectable, { + pub fn sum_safe( + &self, + ctx: &mut Context, + points: impl IntoIterator>, + ) -> EcPoint + where + C: CurveAffineExt, + { + let rand_point = self.load_random_point::(ctx); + let rand_point2 = self.load_random_point::(ctx); + let zero = ctx.load_constant(F::ZERO); + let rand_point = into_strict_point(self.field_chip, ctx, rand_point); + let neg_rand_point = self.negate_strict(ctx, rand_point.clone()); + let mut acc = StrictEcPoint::from(neg_rand_point.clone()); + for point in points { + let point_is_inf = self.is_infinity(ctx, point.clone()); + let addend = self.select(ctx, rand_point2.clone(), point.clone(), point_is_inf); + let _acc = self.add_unequal(ctx, acc.clone(), addend.clone(), true); + let _acc = self.select(ctx, acc.clone().into(), _acc, point_is_inf); + acc = into_strict_point(self.field_chip, ctx, _acc); + let acc_is_inf = self.is_infinity(ctx, acc.clone().into()); + ctx.constrain_equal(&acc_is_inf, &zero); + } + let acc_is_neg_rand = self.is_equal(ctx, acc.clone().into(), neg_rand_point.into()); + let addend = self.select(ctx, rand_point2.clone(), acc.clone().into(), acc_is_neg_rand); + let sum = self.add_unequal(ctx, addend, rand_point, true); + let inf = self.load_private_unchecked(ctx, (FC::FieldType::ZERO, FC::FieldType::ZERO)); + self.select(ctx, inf, sum, acc_is_neg_rand) + } + pub fn select( &self, ctx: &mut Context, From 7e2c0fd9f13caaab96db06892adf7ea23fd890a5 Mon Sep 17 00:00:00 2001 From: Andrew Gu <67293785+MonkeyKing-1@users.noreply.github.com> Date: Mon, 8 Jan 2024 11:52:37 -0500 Subject: [PATCH 2/6] fix clippy + rename functions --- halo2-ecc/src/bn254/tests/ec_add.rs | 4 ++-- halo2-ecc/src/ecc/ecdsa.rs | 2 +- halo2-ecc/src/ecc/mod.rs | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/halo2-ecc/src/bn254/tests/ec_add.rs b/halo2-ecc/src/bn254/tests/ec_add.rs index 7ebe731b..67edd936 100644 --- a/halo2-ecc/src/bn254/tests/ec_add.rs +++ b/halo2-ecc/src/bn254/tests/ec_add.rs @@ -39,7 +39,7 @@ fn g2_add_test( let points = _points.iter().map(|pt| g2_chip.assign_point_unchecked(ctx, *pt)).collect::>(); - let acc = g2_chip.sum::(ctx, points); + let acc = g2_chip.sum_unsafe::(ctx, points); let answer = _points.iter().fold(G2Affine::identity(), |a, b| (a + b).to_affine()); let x = fp2_chip.get_assigned_value(&acc.x.into()); @@ -60,7 +60,7 @@ fn g1_sum_safe_test( let points = _points.iter().map(|pt| g1_chip.assign_point_unchecked(ctx, *pt)).collect::>(); - let acc = g1_chip.sum_safe::(ctx, points); + let acc = g1_chip.sum::(ctx, points); let answer = _points.iter().fold(G1Affine::identity(), |a, b| (a + b).to_affine()); let x = fp_chip.get_assigned_value(&acc.x.into()); diff --git a/halo2-ecc/src/ecc/ecdsa.rs b/halo2-ecc/src/ecc/ecdsa.rs index c72b3974..798c6893 100644 --- a/halo2-ecc/src/ecc/ecdsa.rs +++ b/halo2-ecc/src/ecc/ecdsa.rs @@ -71,7 +71,7 @@ where // compute (x1, y1) = u1 * G + u2 * pubkey and check (r mod n) == x1 as integers // because it is possible for u1 * G == u2 * pubkey, we must use `EccChip::sum` - let sum = chip.sum::(ctx, [u1_mul, u2_mul]); + let sum = chip.sum_unsafe::(ctx, [u1_mul, u2_mul]); // WARNING: For optimization reasons, does not reduce x1 mod n, which is // invalid unless p is very close to n in size. // enforce x1 < n diff --git a/halo2-ecc/src/ecc/mod.rs b/halo2-ecc/src/ecc/mod.rs index c05dfb3b..90c92546 100644 --- a/halo2-ecc/src/ecc/mod.rs +++ b/halo2-ecc/src/ecc/mod.rs @@ -1012,8 +1012,8 @@ impl<'chip, F: BigPrimeField, FC: FieldChip> EccChip<'chip, F, FC> { self.field_chip.assert_equal(ctx, P.y, Q.y); } - /// None of elements in `points` can be point at infinity. - pub fn sum( + /// None of elements in `points` can be point at infinity. Sum cannot be point at infinity either. + pub fn sum_unsafe( &self, ctx: &mut Context, points: impl IntoIterator>, @@ -1036,7 +1036,8 @@ impl<'chip, F: BigPrimeField, FC: FieldChip> EccChip<'chip, F, FC> where FC: Selectable, { - pub fn sum_safe( + /// Expensive version of `sum_unsafe`, but works generally + pub fn sum( &self, ctx: &mut Context, points: impl IntoIterator>, @@ -1047,15 +1048,14 @@ where let rand_point = self.load_random_point::(ctx); let rand_point2 = self.load_random_point::(ctx); let zero = ctx.load_constant(F::ZERO); - let rand_point = into_strict_point(self.field_chip, ctx, rand_point); - let neg_rand_point = self.negate_strict(ctx, rand_point.clone()); - let mut acc = StrictEcPoint::from(neg_rand_point.clone()); + let neg_rand_point = self.negate(ctx, rand_point.clone()); + let mut acc = ComparableEcPoint::from(neg_rand_point.clone()); for point in points { let point_is_inf = self.is_infinity(ctx, point.clone()); let addend = self.select(ctx, rand_point2.clone(), point.clone(), point_is_inf); let _acc = self.add_unequal(ctx, acc.clone(), addend.clone(), true); let _acc = self.select(ctx, acc.clone().into(), _acc, point_is_inf); - acc = into_strict_point(self.field_chip, ctx, _acc); + acc = _acc.into(); let acc_is_inf = self.is_infinity(ctx, acc.clone().into()); ctx.constrain_equal(&acc_is_inf, &zero); } From 187b3b8ae95e3d026bb89e38b3b458cc32c1c5f2 Mon Sep 17 00:00:00 2001 From: Andrew Gu <67293785+MonkeyKing-1@users.noreply.github.com> Date: Mon, 8 Jan 2024 11:59:18 -0500 Subject: [PATCH 3/6] remove unnecessary .into() --- halo2-ecc/src/ecc/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/halo2-ecc/src/ecc/mod.rs b/halo2-ecc/src/ecc/mod.rs index 90c92546..d9bc9cae 100644 --- a/halo2-ecc/src/ecc/mod.rs +++ b/halo2-ecc/src/ecc/mod.rs @@ -1059,7 +1059,7 @@ where let acc_is_inf = self.is_infinity(ctx, acc.clone().into()); ctx.constrain_equal(&acc_is_inf, &zero); } - let acc_is_neg_rand = self.is_equal(ctx, acc.clone().into(), neg_rand_point.into()); + let acc_is_neg_rand = self.is_equal(ctx, acc.clone().into(), neg_rand_point); let addend = self.select(ctx, rand_point2.clone(), acc.clone().into(), acc_is_neg_rand); let sum = self.add_unequal(ctx, addend, rand_point, true); let inf = self.load_private_unchecked(ctx, (FC::FieldType::ZERO, FC::FieldType::ZERO)); From a4e5c86e5367972a3c1e78f35f4d7925f1b6f6b3 Mon Sep 17 00:00:00 2001 From: Andrew Gu <67293785+MonkeyKing-1@users.noreply.github.com> Date: Sat, 6 Apr 2024 23:41:18 -0400 Subject: [PATCH 4/6] feat: add comments, remove extraneous, optimize is_inf --- halo2-ecc/src/ecc/mod.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/halo2-ecc/src/ecc/mod.rs b/halo2-ecc/src/ecc/mod.rs index d9bc9cae..125b2891 100644 --- a/halo2-ecc/src/ecc/mod.rs +++ b/halo2-ecc/src/ecc/mod.rs @@ -991,14 +991,15 @@ impl<'chip, F: BigPrimeField, FC: FieldChip> EccChip<'chip, F, FC> { } /// Checks if a point is the point at infinity (represented by (0, 0)) + /// Assumes points at infinity are always serialized as (0, 0) as bigints pub fn is_infinity( &self, ctx: &mut Context, P: EcPoint, ) -> AssignedValue { // TODO: optimize - let x_is_zero = self.field_chip.is_zero(ctx, P.x); - let y_is_zero = self.field_chip.is_zero(ctx, P.y); + let x_is_zero = self.field_chip.is_soft_zero(ctx, P.x); + let y_is_zero = self.field_chip.is_soft_zero(ctx, P.y); self.field_chip.range().gate().and(ctx, x_is_zero, y_is_zero) } @@ -1036,7 +1037,9 @@ impl<'chip, F: BigPrimeField, FC: FieldChip> EccChip<'chip, F, FC> where FC: Selectable, { - /// Expensive version of `sum_unsafe`, but works generally + /// Expensive version of `sum_unsafe`, but works generally meaning that + /// * sum can be the point at infinity + /// * addends can be points at infinity pub fn sum( &self, ctx: &mut Context, @@ -1056,8 +1059,6 @@ where let _acc = self.add_unequal(ctx, acc.clone(), addend.clone(), true); let _acc = self.select(ctx, acc.clone().into(), _acc, point_is_inf); acc = _acc.into(); - let acc_is_inf = self.is_infinity(ctx, acc.clone().into()); - ctx.constrain_equal(&acc_is_inf, &zero); } let acc_is_neg_rand = self.is_equal(ctx, acc.clone().into(), neg_rand_point); let addend = self.select(ctx, rand_point2.clone(), acc.clone().into(), acc_is_neg_rand); From 2df3ccbee9e93f5bc996da9392c1ec2eefbe45ee Mon Sep 17 00:00:00 2001 From: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Tue, 9 Apr 2024 15:52:15 +0530 Subject: [PATCH 5/6] chore: fix `clap` version in dev-dependency to prevent rustc version issues --- halo2-base/Cargo.toml | 3 +++ halo2-ecc/Cargo.toml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/halo2-base/Cargo.toml b/halo2-base/Cargo.toml index d957cb39..9da3cab9 100644 --- a/halo2-base/Cargo.toml +++ b/halo2-base/Cargo.toml @@ -43,6 +43,9 @@ env_logger = "0.10.0" proptest = "1.1.0" # native poseidon for testing pse-poseidon = { git = "https://github.com/axiom-crypto/pse-poseidon.git" } +clap = "=4.4" # fix clap version to prevent requiring rustc 1.74 +clap_builder = "=4.4" +clap_lex = "=0.6.0" # memory allocation [target.'cfg(not(target_env = "msvc"))'.dependencies] diff --git a/halo2-ecc/Cargo.toml b/halo2-ecc/Cargo.toml index 5a041b5c..7cb11d5d 100644 --- a/halo2-ecc/Cargo.toml +++ b/halo2-ecc/Cargo.toml @@ -29,6 +29,9 @@ criterion-macro = "0.4" halo2-base = { path = "../halo2-base", default-features = false, features = ["test-utils"] } test-log = "0.2.12" env_logger = "0.10.0" +clap = "=4.4" # fix clap version to prevent requiring rustc 1.74 +clap_builder = "=4.4" +clap_lex = "=0.6.0" [features] default = ["jemallocator", "halo2-axiom", "display"] From 2ede0a7d7e3a29f46701ae62a09a895008e5936a Mon Sep 17 00:00:00 2001 From: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Tue, 9 Apr 2024 15:52:37 +0530 Subject: [PATCH 6/6] chore: remove unused variable --- halo2-ecc/src/ecc/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/halo2-ecc/src/ecc/mod.rs b/halo2-ecc/src/ecc/mod.rs index 125b2891..bef0ac15 100644 --- a/halo2-ecc/src/ecc/mod.rs +++ b/halo2-ecc/src/ecc/mod.rs @@ -1050,7 +1050,6 @@ where { let rand_point = self.load_random_point::(ctx); let rand_point2 = self.load_random_point::(ctx); - let zero = ctx.load_constant(F::ZERO); let neg_rand_point = self.negate(ctx, rand_point.clone()); let mut acc = ComparableEcPoint::from(neg_rand_point.clone()); for point in points {