Yuliakot/scalar multiply tests (#83)

* More ecdsa tests * Update mod.rs * Update tests.rs * Update ecdsa.rs * Update ecdsa.rs * Update ecdsa.rs * msm tests * Update mod.rs * Update msm_sum_infinity.rs * fix: ec_sub_strict was panicing when output is identity * affects the MSM functions: right now if the answer is identity, there will be a panic due to divide by 0 instead of just returning 0 * there could be a more optimal solution, but due to the traits for EccChip, we just generate a random point solely to avoid divide by 0 in the case of identity point * fix: change all `1` to `1u64` to prevent unexpected overflow (#72) * fixed base msm tests for output infinity * fixed base msm tests for output infinity * [Fix] Panic when dealing with identity point (#71) * More ecdsa tests * Update mod.rs * Update tests.rs * Update ecdsa.rs * Update ecdsa.rs * Update ecdsa.rs * msm tests * Update mod.rs * Update msm_sum_infinity.rs * fix: ec_sub_strict was panicing when output is identity * affects the MSM functions: right now if the answer is identity, there will be a panic due to divide by 0 instead of just returning 0 * there could be a more optimal solution, but due to the traits for EccChip, we just generate a random point solely to avoid divide by 0 in the case of identity point * Fix/fb msm zero (#77) * fix: fixed_base scalar multiply for [-1]P * feat: use `multi_scalar_multiply` instead of `scalar_multiply` * to reduce code maintanence / redundancy * fix: add back scalar_multiply using any_point * feat: remove flag from variable base `scalar_multiply` * feat: add scalar multiply tests for secp256k1 * fix: variable scalar_multiply last select * Fix/msm tests output identity (#75) * fixed base msm tests for output infinity * fixed base msm tests for output infinity --------- Co-authored-by: yulliakot <yulliakotel@gmail.com> * feat: add tests and update CI --------- Co-authored-by: yuliakot <93175658+yuliakot@users.noreply.github.com> Co-authored-by: yulliakot <yulliakotel@gmail.com> --------- Co-authored-by: yulliakot <yulliakotel@gmail.com> Co-authored-by: yuliakot <93175658+yuliakot@users.noreply.github.com> * sm * scalar multiplication sep256k1 things * Delete msm_sum_infinity.rs * Delete msm_sum_infinity_fixed_base.rs * Delete mod.rs * Update mod.rs * Update mod.rs * Update mod.rs * Update mod.rs * fix: replace `scalar_multiply` with passthrough to MSM for now * feat(msm): use strict mode always * Previously did not use strict because we make assumptions about the curve `C`. Since this was not documented and is easy to miss, we use strict mode always. * fix: clippy and add tests to CI * docs: add assumptions to ec_sub_strict --------- Co-authored-by: yulliakot <yulliakotel@gmail.com> Co-authored-by: yuliakot <93175658+yuliakot@users.noreply.github.com> Co-authored-by: Jonathan Wang <jonathanpwang@users.noreply.github.com> Co-authored-by: yulliakot <yuliakotel@gmail.com>
axiom-crypto · Jun 9, 2023 · 84df66e · 84df66e
1 parent f7a38f0
commit 84df66e
Show file tree

Hide file tree

Showing 5 changed files with 159 additions and 2 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -32,6 +32,7 @@ jobs:
           cargo test -- test_ec_add
           cargo test -- test_fixed
           cargo test -- test_msm
+          cargo test -- test_sm
           cargo test -- test_fb
           cargo test -- test_pairing
           cd ..

diff --git a/halo2-ecc/configs/secp256k1/scalar_multiplication.config b/halo2-ecc/configs/secp256k1/scalar_multiplication.config
@@ -0,0 +1,9 @@
+{"strategy":"Simple",
+"degree":18,
+"num_advice":2,
+"num_lookup_advice":1,
+"num_fixed":1,
+"lookup_bits":17,
+"limb_bits":88,
+"num_limbs":3,
+"window_bits":4}
diff --git a/halo2-ecc/src/ecc/mod.rs b/halo2-ecc/src/ecc/mod.rs
@@ -246,6 +246,9 @@ pub fn ec_sub_unequal<F: PrimeField, FC: FieldChip<F>>(
 
 /// Constrains `P != -Q` but allows `P == Q`, in which case output is (0,0).
 /// For Weierstrass curves only.
+///
+/// Assumptions
+/// # Neither P or Q is the point at infinity
 pub fn ec_sub_strict<F: PrimeField, FC: FieldChip<F>>(
     chip: &FC,
     ctx: &mut Context<F>,
@@ -496,7 +499,8 @@ where
     assert!(!scalar.is_empty());
     assert!((max_bits as u64) <= modulus::<F>().bits());
     assert!(window_bits != 0);
-
+    multi_scalar_multiply::<F, FC, C>(chip, ctx, &[P], vec![scalar], max_bits, window_bits)
+    /*
     let total_bits = max_bits * scalar.len();
     let num_windows = (total_bits + window_bits - 1) / window_bits;
     let rounded_bitlen = num_windows * window_bits;
@@ -577,6 +581,7 @@ where
     // if at the end, return identity point (0,0) if still not started
     let zero = chip.load_constant(ctx, FC::FieldType::zero());
     ec_select(chip, ctx, curr_point, EcPoint::new(zero.clone(), zero), *is_started.last().unwrap())
+    */
 }
 
 /// Checks that `P` is indeed a point on the elliptic curve `C`.
@@ -729,7 +734,7 @@ where
         ctx,
         &rand_start_vec[k],
         &rand_start_vec[0],
-        k >= F::CAPACITY as usize,
+        true, // k >= F::CAPACITY as usize, // this assumed random points on `C` were of prime order equal to modulus of `F`. Since this is easily missed, we turn on strict mode always
     );
     let mut curr_point = start_point.clone();
 

diff --git a/halo2-ecc/src/secp256k1/tests/mod.rs b/halo2-ecc/src/secp256k1/tests/mod.rs
@@ -160,3 +160,4 @@ fn test_secp_sm_0_1() {
     let circuit = sm_circuit(params, CircuitBuilderStage::Mock, None, base, s);
     MockProver::run(params.degree, &circuit, vec![]).unwrap().assert_satisfied();
 }
+pub mod sm_unsafe_scalars;
diff --git a/halo2-ecc/src/secp256k1/tests/sm_unsafe_scalars.rs b/halo2-ecc/src/secp256k1/tests/sm_unsafe_scalars.rs
@@ -0,0 +1,141 @@
+use crate::{
+    ecc::fixed_base::scalar_multiply,
+    fields::{fp::FpChip, FpStrategy},
+};
+use ff::PrimeField;
+use halo2_base::{
+    gates::{builder::GateThreadBuilder, RangeChip},
+    halo2_proofs::halo2curves::secp256k1::{Fp, Secp256k1, Secp256k1Affine},
+};
+use std::fs::File;
+
+use super::*;
+
+const GROUP_ORDER: Fp =
+    Fp::from_raw([0xbfd25e8cd0364141, 0xbaaedce6af48a03b, 0xfffffffffffffffe, 0xffffffffffffffff]);
+
+#[derive(Clone, Copy, Debug, Serialize, Deserialize)]
+struct SMCircuitParams {
+    strategy: FpStrategy,
+    degree: u32,
+    num_advice: usize,
+    num_lookup_advice: usize,
+    num_fixed: usize,
+    lookup_bits: usize,
+    limb_bits: usize,
+    num_limbs: usize,
+    window_bits: usize,
+}
+
+fn scalar_multiply_test(params: SMCircuitParams, point: Secp256k1Affine, scalar: Fp) {
+    let mut builder = GateThreadBuilder::mock();
+
+    std::env::set_var("LOOKUP_BITS", params.lookup_bits.to_string());
+    let range = RangeChip::<Fp>::default(params.lookup_bits);
+    let fp_chip = FpChip::<Fp, Fp>::new(&range, params.limb_bits, params.num_limbs);
+    let ecc_chip = EccChip::new(&fp_chip);
+
+    let ctx = builder.main(0);
+    let scalar_assigned = vec![ctx.load_witness(scalar)];
+
+    let _sm = scalar_multiply(
+        ecc_chip.field_chip(),
+        ctx,
+        &point,
+        scalar_assigned,
+        Fp::NUM_BITS as usize,
+        params.window_bits,
+    );
+
+    //println!("{:?}", sm.x().value());
+}
+
+#[test]
+fn test_sm1() {
+    let path = "configs/secp256k1/scalar_multiplication.config";
+    let params: SMCircuitParams = serde_json::from_reader(
+        File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")),
+    )
+    .unwrap();
+    let generator_point = Secp256k1::generator().to_affine();
+
+    let scalar = GROUP_ORDER;
+
+    println!("the scalar is {scalar:?}");
+    scalar_multiply_test(params, generator_point, scalar);
+}
+
+#[test]
+fn test_sm2() {
+    let path = "configs/secp256k1/scalar_multiplication.config";
+    let params: SMCircuitParams = serde_json::from_reader(
+        File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")),
+    )
+    .unwrap();
+    let generator_point = Secp256k1::generator().to_affine();
+
+    let scalar = GROUP_ORDER - Fp::one();
+
+    println!("the scalar is {scalar:?}");
+    scalar_multiply_test(params, generator_point, scalar);
+}
+
+#[test]
+fn test_sm3() {
+    let path = "configs/secp256k1/scalar_multiplication.config";
+    let params: SMCircuitParams = serde_json::from_reader(
+        File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")),
+    )
+    .unwrap();
+    let generator_point = Secp256k1::generator().to_affine();
+
+    let scalar = GROUP_ORDER - Fp::one();
+
+    println!("the scalar is {scalar:?}");
+    scalar_multiply_test(params, generator_point, scalar);
+}
+
+#[test]
+fn test_sm4() {
+    let path = "configs/secp256k1/scalar_multiplication.config";
+    let params: SMCircuitParams = serde_json::from_reader(
+        File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")),
+    )
+    .unwrap();
+    let generator_point = Secp256k1::generator().to_affine();
+
+    let scalar = GROUP_ORDER - Fp::from(2);
+
+    println!("the scalar is {scalar:?}");
+    scalar_multiply_test(params, generator_point, scalar);
+}
+
+#[test]
+fn test_sm5() {
+    let path = "configs/secp256k1/scalar_multiplication.config";
+    let params: SMCircuitParams = serde_json::from_reader(
+        File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")),
+    )
+    .unwrap();
+    let generator_point = Secp256k1::generator().to_affine();
+
+    let scalar = GROUP_ORDER + Fp::one();
+
+    println!("the scalar is {scalar:?}");
+    scalar_multiply_test(params, generator_point, scalar);
+}
+
+#[test]
+fn test_sm6() {
+    let path = "configs/secp256k1/scalar_multiplication.config";
+    let params: SMCircuitParams = serde_json::from_reader(
+        File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")),
+    )
+    .unwrap();
+    let generator_point = Secp256k1::generator().to_affine();
+
+    let scalar = GROUP_ORDER + Fp::from(2);
+
+    println!("the scalar is {scalar:?}");
+    scalar_multiply_test(params, generator_point, scalar);
+}