From b213c6d7f8d304621d272e469e9376de7d4f253a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Est=C3=A9fano=20Bargas?= Date: Tue, 5 Aug 2025 18:04:40 -0300 Subject: [PATCH 1/7] add operate_with_unchecked and operate_with_eip196 --- .../elliptic_curve/short_weierstrass/point.rs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/crates/math/src/elliptic_curve/short_weierstrass/point.rs b/crates/math/src/elliptic_curve/short_weierstrass/point.rs index 45a738231..5529f7d70 100644 --- a/crates/math/src/elliptic_curve/short_weierstrass/point.rs +++ b/crates/math/src/elliptic_curve/short_weierstrass/point.rs @@ -7,6 +7,7 @@ use crate::{ errors::DeserializationError, field::element::FieldElement, traits::{ByteConversion, Deserializable}, + unsigned_integer::element::U256, }; use super::traits::IsShortWeierstrass; @@ -114,6 +115,7 @@ impl ShortWeierstrassProjectivePoint { let point = Self::new([xp, yp, zp]); point.unwrap() } + // https://hyperelliptic.org/EFD/g1p/data/shortw/projective/addition/madd-1998-cmo /// More efficient than operate_with, but must ensure that other is in affine form pub fn operate_with_affine(&self, other: &Self) -> Self { @@ -164,6 +166,66 @@ impl ShortWeierstrassProjectivePoint { let point = Self::new([x, y, z]); point.unwrap() } + + /// Computes the addition of `self` and `other`. + /// Taken from "Moonmath" (Algorithm 7, page 89) + fn operate_with_unchecked(&self, other: &Self) -> Self { + let [px, py, pz] = self.coordinates(); + let [qx, qy, qz] = other.coordinates(); + let u1 = qy * pz; + let u2 = py * qz; + let v1 = qx * pz; + let v2 = px * qz; + if v1 == v2 { + if u1 != u2 || *py == FieldElement::zero() { + Self::neutral_element() + } else { + self.double() + } + } else { + let u = u1 - &u2; + let v = v1 - &v2; + let w = pz * qz; + + let u_square = &u * &u; + let v_square = &v * &v; + let v_cube = &v * &v_square; + let v_square_v2 = &v_square * &v2; + + let a = &u_square * &w - &v_cube - (&v_square_v2 + &v_square_v2); + + let xp = &v * &a; + let yp = u * (&v_square_v2 - a) - &v_cube * u2; + let zp = &v_cube * w; + + debug_assert_eq!( + E::defining_equation_projective(&xp, &yp, &zp), + FieldElement::::zero() + ); + // SAFETY: The values `x_p, y_p, z_p` are computed correctly to be on the curve. + // The assertion above verifies that the resulting point is valid. + Self::new([xp, yp, zp]).unwrap() + } + } + + pub fn operate_with_self_eip196(&self, mut exponent: U256) -> Self { + let mut result = Self::neutral_element(); + let mut base = self.clone(); + + while exponent.limbs[3] != 0 + && exponent.limbs[2] != 0 + && exponent.limbs[1] != 0 + && exponent.limbs[0] != 0 + { + // check lsb + if exponent.limbs[3] & 1 == 1 { + result = Self::operate_with_unchecked(&result, &base); + } + exponent >>= 1; + base = self.double(); + } + result + } } impl PartialEq for ShortWeierstrassProjectivePoint { From c82a2c89a1e09f0342c12b3d0cd275358e0a9bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Est=C3=A9fano=20Bargas?= Date: Tue, 5 Aug 2025 18:50:07 -0300 Subject: [PATCH 2/7] fix while cond --- .../math/src/elliptic_curve/short_weierstrass/point.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/math/src/elliptic_curve/short_weierstrass/point.rs b/crates/math/src/elliptic_curve/short_weierstrass/point.rs index 5529f7d70..49a80e469 100644 --- a/crates/math/src/elliptic_curve/short_weierstrass/point.rs +++ b/crates/math/src/elliptic_curve/short_weierstrass/point.rs @@ -212,17 +212,17 @@ impl ShortWeierstrassProjectivePoint { let mut result = Self::neutral_element(); let mut base = self.clone(); - while exponent.limbs[3] != 0 - && exponent.limbs[2] != 0 - && exponent.limbs[1] != 0 - && exponent.limbs[0] != 0 + while !(exponent.limbs[3] == 0 + && exponent.limbs[2] == 0 + && exponent.limbs[1] == 0 + && exponent.limbs[0] == 0) { // check lsb if exponent.limbs[3] & 1 == 1 { result = Self::operate_with_unchecked(&result, &base); } exponent >>= 1; - base = self.double(); + base = base.double(); } result } From 7c692269cc5e806f04b5844cd3da99ed5823fdb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Est=C3=A9fano=20Bargas?= Date: Wed, 6 Aug 2025 11:44:50 -0300 Subject: [PATCH 3/7] remove unchecked --- crates/math/src/elliptic_curve/short_weierstrass/point.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/math/src/elliptic_curve/short_weierstrass/point.rs b/crates/math/src/elliptic_curve/short_weierstrass/point.rs index 49a80e469..4c432de48 100644 --- a/crates/math/src/elliptic_curve/short_weierstrass/point.rs +++ b/crates/math/src/elliptic_curve/short_weierstrass/point.rs @@ -219,7 +219,7 @@ impl ShortWeierstrassProjectivePoint { { // check lsb if exponent.limbs[3] & 1 == 1 { - result = Self::operate_with_unchecked(&result, &base); + result = Self::operate_with(&result, &base); } exponent >>= 1; base = base.double(); From 6c82fb99ff8ce986bf55e6066900586050055eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Est=C3=A9fano=20Bargas?= Date: Wed, 6 Aug 2025 11:48:15 -0300 Subject: [PATCH 4/7] change algo to use operate_with_unchecked --- .../elliptic_curve/short_weierstrass/point.rs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/math/src/elliptic_curve/short_weierstrass/point.rs b/crates/math/src/elliptic_curve/short_weierstrass/point.rs index 4c432de48..04496bf13 100644 --- a/crates/math/src/elliptic_curve/short_weierstrass/point.rs +++ b/crates/math/src/elliptic_curve/short_weierstrass/point.rs @@ -209,20 +209,30 @@ impl ShortWeierstrassProjectivePoint { } pub fn operate_with_self_eip196(&self, mut exponent: U256) -> Self { - let mut result = Self::neutral_element(); - let mut base = self.clone(); + let mut result = self.clone(); + + // TODO: return early if exponent == 0 + + // find first high bit + while exponent.limbs[3] & 1 != 1 + { + exponent >>= 1; + result = result.double(); + } + + let mut base = result.clone(); while !(exponent.limbs[3] == 0 && exponent.limbs[2] == 0 && exponent.limbs[1] == 0 && exponent.limbs[0] == 0) { + exponent >>= 1; + base = base.double(); // check lsb if exponent.limbs[3] & 1 == 1 { - result = Self::operate_with(&result, &base); + result = Self::operate_with_unchecked(&result, &base); } - exponent >>= 1; - base = base.double(); } result } From bd009efe431c951cf26c82449f2693b05ba7301a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Est=C3=A9fano=20Bargas?= Date: Wed, 6 Aug 2025 11:49:11 -0300 Subject: [PATCH 5/7] fmt & docs --- .../elliptic_curve/short_weierstrass/point.rs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/math/src/elliptic_curve/short_weierstrass/point.rs b/crates/math/src/elliptic_curve/short_weierstrass/point.rs index 04496bf13..fd939f778 100644 --- a/crates/math/src/elliptic_curve/short_weierstrass/point.rs +++ b/crates/math/src/elliptic_curve/short_weierstrass/point.rs @@ -167,8 +167,11 @@ impl ShortWeierstrassProjectivePoint { point.unwrap() } - /// Computes the addition of `self` and `other`. - /// Taken from "Moonmath" (Algorithm 7, page 89) + /// Sames as operate_with, but does not return early if any inputs is the point + /// at infinity, removing that check. + /// + /// This brings a performance boost in cases where `self` and `other` is known to + /// not be the point at infinity, like in the context of the EIP-196 ecmul precompile. fn operate_with_unchecked(&self, other: &Self) -> Self { let [px, py, pz] = self.coordinates(); let [qx, qy, qz] = other.coordinates(); @@ -208,14 +211,21 @@ impl ShortWeierstrassProjectivePoint { } } + /// Same as operate_with_self, but includes some assumptions given by the ecmul precompile + /// (as defined in EIP-196). + /// + /// These assumptions are: + /// 1. `exponent` is a 256 bit scalar + /// 2. `self` is never the point at infinity + /// 3. `self` is short weierstrass projective point + /// We can then use `operate_with_unchecked` and pub fn operate_with_self_eip196(&self, mut exponent: U256) -> Self { - let mut result = self.clone(); + let mut result = self.clone(); // TODO: return early if exponent == 0 // find first high bit - while exponent.limbs[3] & 1 != 1 - { + while exponent.limbs[3] & 1 != 1 { exponent >>= 1; result = result.double(); } From 3d92b73a0fc57d7953e058554aacdc79e8c5b3f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Est=C3=A9fano=20Bargas?= Date: Wed, 6 Aug 2025 15:31:46 -0300 Subject: [PATCH 6/7] fix doc --- crates/math/src/elliptic_curve/short_weierstrass/point.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/math/src/elliptic_curve/short_weierstrass/point.rs b/crates/math/src/elliptic_curve/short_weierstrass/point.rs index fd939f778..6dd47e717 100644 --- a/crates/math/src/elliptic_curve/short_weierstrass/point.rs +++ b/crates/math/src/elliptic_curve/short_weierstrass/point.rs @@ -218,7 +218,6 @@ impl ShortWeierstrassProjectivePoint { /// 1. `exponent` is a 256 bit scalar /// 2. `self` is never the point at infinity /// 3. `self` is short weierstrass projective point - /// We can then use `operate_with_unchecked` and pub fn operate_with_self_eip196(&self, mut exponent: U256) -> Self { let mut result = self.clone(); From f6a17eb509fc8c112945915d7257ad197fa65591 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Est=C3=A9fano=20Bargas?= Date: Wed, 6 Aug 2025 15:31:54 -0300 Subject: [PATCH 7/7] idea: each limb at once --- .../elliptic_curve/short_weierstrass/point.rs | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/crates/math/src/elliptic_curve/short_weierstrass/point.rs b/crates/math/src/elliptic_curve/short_weierstrass/point.rs index 6dd47e717..0866405e4 100644 --- a/crates/math/src/elliptic_curve/short_weierstrass/point.rs +++ b/crates/math/src/elliptic_curve/short_weierstrass/point.rs @@ -1,3 +1,5 @@ +use core::array; + use crate::{ cyclic_group::IsGroup, elliptic_curve::{ @@ -219,31 +221,43 @@ impl ShortWeierstrassProjectivePoint { /// 2. `self` is never the point at infinity /// 3. `self` is short weierstrass projective point pub fn operate_with_self_eip196(&self, mut exponent: U256) -> Self { - let mut result = self.clone(); - // TODO: return early if exponent == 0 - // find first high bit - while exponent.limbs[3] & 1 != 1 { - exponent >>= 1; - result = result.double(); - } + // the idea is that we would calculate the sum of the bits corresponding to each limb + // at the same time, and in the end do the appropiate exponentiation by a power of two + // of each component and add them all up to get the resulting point. - let mut base = result.clone(); + // by doing this we should avoid the >> operator logic of an U256, that moves bits from + // one limb to another when it's actually not necessary. + + let mut base = self.clone(); + let mut result: [Self; 4] = array::from_fn(|_| Self::neutral_element()); while !(exponent.limbs[3] == 0 && exponent.limbs[2] == 0 && exponent.limbs[1] == 0 && exponent.limbs[0] == 0) { - exponent >>= 1; - base = base.double(); - // check lsb - if exponent.limbs[3] & 1 == 1 { - result = Self::operate_with_unchecked(&result, &base); + for (i, limb) in exponent.limbs.iter_mut().enumerate() { + if *limb != 0 { + if *limb & 1 == 1 { + result[i] = Self::operate_with(&result[i], &base); + } + *limb >>= 1; + } } + base = base.double(); } - result + + let exp_64 = |mut point: Self| { + for _ in 0..64 { + point = point.double() + } + point + }; + + let [r0, r1, r2, r3] = result; + exp_64(r2.operate_with(&exp_64(r1.operate_with(&exp_64(r0))))).operate_with(&r3) } }