Optimize ec add affine

benches/benches/point.rs

@@ -1,70 +1,77 @@
+use std::ops::{Add, AddAssign};
+
 use criterion::{black_box, criterion_group, criterion_main, Criterion};
 use lambdaworks_math::{
     cyclic_group::IsGroup,
     elliptic_curve::{short_weierstrass::curves::stark_curve::StarkCurve, traits::IsEllipticCurve},
 };
 use starknet_curve::{curve_params::GENERATOR, AffinePoint, ProjectivePoint};
-use std::ops::AddAssign;
 
 const BENCHMARK_NAME: &str = "point";
 
 pub fn criterion_benchmark(c: &mut Criterion) {
-    let initial_projective_point = ProjectivePoint::from(&GENERATOR);
-    let second_project_point = initial_projective_point;
+    let starknet_rs_affine_generator = GENERATOR;
 
+    let starknet_rs_initial_projective =
+        ProjectivePoint::from_affine_point(&GENERATOR.add(&GENERATOR));
     // This is the code we are going to bench
     // We test it once outside the bench to check the result matches with Lambdaworks
-    let mut projective_point = initial_projective_point;
+    let mut projective_point_rs = starknet_rs_initial_projective;
     for _i in 0..10000 {
-        projective_point.add_assign(&second_project_point);
+        projective_point_rs.add_assign(&starknet_rs_affine_generator);
     }
 
-    let starknet_rs_x = AffinePoint::from(&projective_point).x.to_string();
-    println!("Starknet RS result X: {} ", starknet_rs_x);
-    let starknet_rs_y = AffinePoint::from(&projective_point).y.to_string();
-    print!("Starknet RS result Y: {} ", starknet_rs_y);
+    let starknet_rs_x = AffinePoint::from(&projective_point_rs).x;
+    println!("Starknet RS result - X: {:#x} ", starknet_rs_x);
+    let starknet_rs_y = AffinePoint::from(&projective_point_rs).y;
+    println!("Starknet RS result - Y: {:#x} ", starknet_rs_y);
 
     {
         c.bench_function(
-            &format!("{} 10k Operations | Starknet RS ", BENCHMARK_NAME),
+            &format!(
+                "{} 10k Operations with Affine | Starknet RS ",
+                BENCHMARK_NAME
+            ),
             |b| {
                 b.iter(|| {
-                    let mut projective_point = initial_projective_point;
-                    // We loop to have a higher variance of numbers, and make the time of the clones not relevant
+                    let mut projective_point_rs = starknet_rs_initial_projective;
                     for _i in 0..10000 {
-                        projective_point.add_assign(&second_project_point);
+                        projective_point_rs.add_assign(black_box(&starknet_rs_affine_generator));
                     }
-                    projective_point
+                    projective_point_rs
                 });
             },
         );
     }
 
-    let initial_projective_point = StarkCurve::generator();
-    let second_projective_point = initial_projective_point.clone();
+    let lambdaworks_affine_generator = StarkCurve::generator();
 
     // This is the code we are going to bench
     // We test it once outside the bench to check the result matches with Starknet RS
-    let mut projective_point = initial_projective_point.clone();
+    let lambdaworks_rs_initial_projective =
+        StarkCurve::generator().operate_with(&StarkCurve::generator());
+
+    let mut projective_point = lambdaworks_rs_initial_projective.clone();
     for _i in 0..10000 {
         projective_point =
-            black_box(projective_point.operate_with(black_box(&second_projective_point)));
+            black_box(projective_point.operate_with(black_box(&lambdaworks_affine_generator)));
     }
+
     let lambdaworks_x = projective_point.to_affine().x().to_string();
     let lambdaworks_y = projective_point.to_affine().y().to_string();
-    println!("Lambdaworks result, X: {}", lambdaworks_x);
-    println!("Lambdaworks result, Y: {}", lambdaworks_y);
+    println!("Lambdaworks result - X: {}", lambdaworks_x);
+    println!("Lambdaworks result - Y: {}", lambdaworks_y);
 
     {
         c.bench_function(
             &format!("{} 10k Operations | Lambdaworks", BENCHMARK_NAME),
             |b| {
                 b.iter(|| {
-                    let mut projective_point = initial_projective_point.clone();
+                    let mut projective_point = lambdaworks_rs_initial_projective.clone();
                     for _i in 0..10000 {
-                        projective_point = black_box(
-                            projective_point.operate_with(black_box(&second_projective_point)),
-                        );
+                        projective_point = projective_point
+                            .operate_with_affine(black_box(&lambdaworks_affine_generator));
+                        // .unwrap();
                     }
                     projective_point
                 });

crypto/src/commitments/kzg.rs

@@ -411,19 +411,4 @@ mod tests {
 
         assert_eq!(srs, deserialized);
     }
-
-    #[test]
-    fn load_srs_from_file() {
-        type TestSrsType = StructuredReferenceString<
-            ShortWeierstrassProjectivePoint<BLS12381Curve>,
-            ShortWeierstrassProjectivePoint<BLS12381TwistCurve>,
-        >;
-
-        let base_dir = env!("CARGO_MANIFEST_DIR");
-        let srs_file = base_dir.to_owned() + "/src/commitments/test_srs/srs_3_g1_elements.bin";
-
-        let srs = TestSrsType::from_file(&srs_file).unwrap();
-
-        assert_eq!(srs.powers_main_group.len(), 3);
-    }
 }

math/Cargo.toml

@@ -86,3 +86,7 @@ harness = false
 name = "criterion_metal"
 harness = false
 required-features = ["metal"]
+
+[[bench]]
+name = "iai_point_add"
+harness = false

math/benches/iai_point_add.rs

@@ -0,0 +1,37 @@
+use std::ops::AddAssign;
+
+use criterion::{black_box, criterion_group, criterion_main, Criterion};
+use lambdaworks_math::{
+    cyclic_group::IsGroup,
+    elliptic_curve::{
+        short_weierstrass::{
+            curves::stark_curve::StarkCurve, point::ShortWeierstrassProjectivePoint,
+        },
+        traits::{FromAffine, IsEllipticCurve},
+    },
+    field::{
+        element::FieldElement, fields::fft_friendly::stark_252_prime_field::Stark252PrimeField,
+    },
+};
+
+const BENCHMARK_NAME: &str = "point";
+
+#[inline(never)]
+pub fn add_for_bench() -> ShortWeierstrassProjectivePoint<StarkCurve> {
+    let initial_projective_point = StarkCurve::generator();
+    let second_projective_point = initial_projective_point.clone();
+
+    // This is the code we are going to bench
+    // We test it once outside the bench to check the result matches with Starknet RS
+    let mut projective_point = initial_projective_point.clone();
+    for _i in 0..10000 {
+        projective_point =
+            black_box(projective_point.operate_with_affine(black_box(&second_projective_point)));
+    }
+    projective_point
+}
+
+iai_callgrind::main!(
+    callgrind_args = "toggle-collect=util::*";
+    functions = add_for_bench,
+);

math/src/elliptic_curve/point.rs

@@ -1,7 +1,8 @@
 use crate::elliptic_curve::traits::IsEllipticCurve;
 use crate::field::element::FieldElement;
 use core::fmt::Debug;
-/// Represents an elliptic curve point using the projective short Weierstrass form:
+
+/// Represents an elliptic curve point using the projective jacobian
 /// y^2 * z = x^3 + a * x * z^2 + b * z^3,
 /// where `x`, `y` and `z` variables are field elements.
 #[derive(Debug, Clone)]
@@ -36,13 +37,21 @@ impl<E: IsEllipticCurve> ProjectivePoint<E> {
     }
 
     /// Creates the same point in affine coordinates. That is,
-    /// returns [x / z: y / z: 1] where `self` is [x: y: z].
+    /// returns [x / z^2: y / z^3: 1] where `self` is [x: y: z].
     /// Panics if `self` is the point at infinity.
     pub fn to_affine(&self) -> Self {
         let [x, y, z] = self.coordinates();
-        assert_ne!(z, &FieldElement::zero());
+        if z == &FieldElement::zero() {
+            return ProjectivePoint::new([
+                FieldElement::zero(),
+                FieldElement::zero(),
+                FieldElement::zero(),
+            ]);
+        }
         let inv_z = z.inv().unwrap();
-        ProjectivePoint::new([x * &inv_z, y * inv_z, FieldElement::one()])
+        let inv_z_square = inv_z.square();
+        let inv_z_cube = &inv_z * &inv_z_square;
+        ProjectivePoint::new([x * &inv_z_square, y * inv_z_cube, FieldElement::one()])
     }
 }
 
@@ -99,8 +108,8 @@ mod tests {
     fn operate_with_self_works_1() {
         let g = TestCurve1::generator();
         assert_eq!(
-            g.operate_with(&g).operate_with(&g),
-            g.operate_with_self(3_u16)
+            g.operate_with(&g).operate_with(&g).to_affine(),
+            g.operate_with_self(3_u16).to_affine()
         );
     }
 
@@ -144,7 +153,7 @@ mod tests {
         )
         .unwrap();
 
-        assert_eq!(point_1, expected_result);
+        assert_eq!(point_1.to_affine(), expected_result.to_affine());
     }
 
     #[test]

math/src/elliptic_curve/short_weierstrass/curves/bls12_377/curve.rs

@@ -61,7 +61,10 @@ mod tests {
     fn adding_five_times_point_1_works() {
         let point_1 = point_1();
         let point_1_times_5 = point_1_times_5();
-        assert_eq!(point_1.operate_with_self(5_u16), point_1_times_5);
+        assert_eq!(
+            point_1.operate_with_self(5_u16).to_affine(),
+            point_1_times_5
+        );
     }
 
     #[test]
@@ -92,8 +95,8 @@ mod tests {
     fn operate_with_self_works_1() {
         let g = BLS12377Curve::generator();
         assert_eq!(
-            g.operate_with(&g).operate_with(&g),
-            g.operate_with_self(3_u16)
+            g.operate_with(&g).operate_with(&g).to_affine(),
+            g.operate_with_self(3_u16).to_affine()
         );
     }
 }

math/src/elliptic_curve/short_weierstrass/curves/bls12_381/compression.rs

@@ -169,7 +169,9 @@ mod tests {
     fn test_compress_decompress_2g() {
         let g = BLS12381Curve::generator();
         // calculate g point operate with itself
-        let g_2 = g.operate_with_self(UnsignedInteger::<4>::from("2"));
+        let g_2 = g
+            .operate_with_self(UnsignedInteger::<4>::from("2"))
+            .to_affine();
 
         let compressed_g2 = compress_g1_point(&g_2);
         let mut compressed_g2_slice: [u8; 48] = compressed_g2.try_into().unwrap();

math/src/elliptic_curve/short_weierstrass/curves/bls12_381/curve.rs

@@ -64,7 +64,10 @@ mod tests {
     fn adding_five_times_point_1_works() {
         let point_1 = point_1();
         let point_1_times_5 = point_1_times_5();
-        assert_eq!(point_1.operate_with_self(5_u16), point_1_times_5);
+        assert_eq!(
+            point_1.operate_with_self(5_u16).to_affine(),
+            point_1_times_5
+        );
     }
 
     #[test]
@@ -113,8 +116,8 @@ mod tests {
     fn operate_with_self_works_1() {
         let g = BLS12381Curve::generator();
         assert_eq!(
-            g.operate_with(&g).operate_with(&g),
-            g.operate_with_self(3_u16)
+            g.operate_with(&g).operate_with(&g).to_affine(),
+            g.operate_with_self(3_u16).to_affine()
         );
     }
 }

math/src/elliptic_curve/short_weierstrass/curves/bls12_381/pairing.rs

@@ -221,7 +221,7 @@ mod tests {
         let mut r = g2.clone();
         let mut f = FieldElement::one();
         double_accumulate_line(&mut r, &g1, &mut f);
-        assert_eq!(r, g2.operate_with(&g2));
+        assert_eq!(r, g2.operate_with(&g2).to_affine());
     }
 
     #[test]
@@ -236,7 +236,7 @@ mod tests {
         let mut r = g2;
         let mut f = FieldElement::one();
         add_accumulate_line(&mut r, &g3, &g1, &mut f);
-        assert_eq!(r, expected);
+        assert_eq!(r, expected.to_affine());
     }
 
     #[test]

math/src/elliptic_curve/short_weierstrass/curves/bls12_381/twist.rs

@@ -157,6 +157,6 @@ mod tests {
         let p = BLS12381TwistCurve::create_point_from_affine(px, py).unwrap();
         let q = BLS12381TwistCurve::create_point_from_affine(qx, qy).unwrap();
         let expected = BLS12381TwistCurve::create_point_from_affine(expectedx, expectedy).unwrap();
-        assert_eq!(p.operate_with(&q), expected);
+        assert_eq!(p.operate_with(&q).to_affine(), expected);
     }
 }