Add new unchecked (#1024)

* add new_uncheckes for short weiestrass

* remove unused files

* use unchecked for stark curve generator

* revert changes for vesta curve

* add missing comment

* make the point field private

* fix clippy

* add safety comments and rename function

---------

Co-authored-by: Nicole <nicole.graus@lambdaclass.com>

Author: jotabulacios

crates/crypto/src/hash/pedersen/constants.rs

@@ -89,9 +89,10 @@ impl ShortWeierstrassProjectivePoint<BLS12377Curve> {
     /// Returns 𝜙(P) = (𝑥, 𝑦) ⇒ (𝛽𝑥, 𝑦), where 𝛽 is the Cube Root of Unity in the base prime field
     /// https://eprint.iacr.org/2022/352.pdf 2 Preliminaries
     fn phi(&self) -> Self {
-        let mut a = self.clone();
-        a.0.value[0] = a.x() * CUBE_ROOT_OF_UNITY_G1;
-        a
+        let [x, y, z] = self.coordinates();
+        let new_x = x * CUBE_ROOT_OF_UNITY_G1;
+        // SAFETY: The value `x` is computed correctly, so the point is in the curve.
+        Self::new_unchecked([new_x, y.clone(), z.clone()])
     }
 
     /// 𝜙(P) = −𝑢²P

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

@@ -82,10 +82,10 @@ impl ShortWeierstrassProjectivePoint<BLS12381Curve> {
     /// Returns 𝜙(P) = (𝑥, 𝑦) ⇒ (𝛽𝑥, 𝑦), where 𝛽 is the Cube Root of Unity in the base prime field
     /// https://eprint.iacr.org/2022/352.pdf 2 Preliminaries
     fn phi(&self) -> Self {
-        // This clone is unsightly
-        let mut a = self.clone();
-        a.0.value[0] = a.x() * CUBE_ROOT_OF_UNITY_G1;
-        a
+        let [x, y, z] = self.coordinates();
+        let new_x = x * CUBE_ROOT_OF_UNITY_G1;
+        // SAFETY: The value `x` is computed correctly, so the point is in the curve.
+        Self::new_unchecked([new_x, y.clone(), z.clone()])
     }
 
     /// 𝜙(P) = −𝑢²P

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

@@ -159,7 +159,8 @@ fn double_accumulate_line(
     let x1_sq_3 = three * x1.square();
     let [x1_sq_30, x1_sq_31] = x1_sq_3.value();
 
-    t.0.value = [x3, y3, z3];
+    // SAFETY: The values `x_3, y_3, z_3` are computed correctly to be on the curve.
+    t.set_unchecked([x3, y3, z3]);
 
     // (a0 + a2w2 + a4w4 + a1w + a3w3 + a5w5) * (b0 + b2 w2 + b3 w3) =
     // (a0b0 + r (a3b3 + a4b2)) w0 + (a1b0 + r (a4b3 + a5b2)) w
@@ -213,7 +214,8 @@ fn add_accumulate_line(
     let y3 = &theta * (g - h) - i;
     let z3 = z1 * e;
 
-    t.0.value = [x3, y3, z3];
+    // SAFETY: The values `x_3, y_3, z_3` are computed correctly to be on the curve.
+    t.set_unchecked([x3, y3, z3]);
 
     let [lambda0, lambda1] = lambda.value();
     let [theta0, theta1] = theta.value();

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

@@ -1,6 +1,5 @@
 use crate::{
     elliptic_curve::{
-        point::ProjectivePoint,
         short_weierstrass::{point::ShortWeierstrassProjectivePoint, traits::IsShortWeierstrass},
         traits::IsEllipticCurve,
     },
@@ -13,15 +12,18 @@ use crate::{
 pub struct StarkCurve;
 
 impl StarkCurve {
-    pub const fn from_affine_hex_string(
+    pub const fn from_affine_hex_string_unchecked(
         x_hex: &str,
         y_hex: &str,
     ) -> ShortWeierstrassProjectivePoint<Self> {
-        ShortWeierstrassProjectivePoint(ProjectivePoint::new([
+        // SAFETY: The values `x_hex, y_hex` must represent valid coordinates that satisfy
+        // the curve equation. Invalid coordinates violate the invariant and produce
+        // silently incorrect results in subsequent operations.
+        ShortWeierstrassProjectivePoint::new_unchecked([
             FieldElement::<Stark252PrimeField>::from_hex_unchecked(x_hex),
             FieldElement::<Stark252PrimeField>::from_hex_unchecked(y_hex),
             FieldElement::<Stark252PrimeField>::from_hex_unchecked("1"),
-        ]))
+        ])
     }
 }
 
@@ -32,17 +34,15 @@ impl IsEllipticCurve for StarkCurve {
     fn generator() -> Self::PointRepresentation {
         // SAFETY:
         // - The generator point is mathematically verified to be a valid point on the curve.
-        // - `unwrap()` is safe because the provided coordinates satisfy the curve equation.
-        let point = Self::PointRepresentation::new([
+        Self::PointRepresentation::new_unchecked([
             FieldElement::<Self::BaseField>::from_hex_unchecked(
                 "1EF15C18599971B7BECED415A40F0C7DEACFD9B0D1819E03D723D8BC943CFCA",
             ),
             FieldElement::<Self::BaseField>::from_hex_unchecked(
                 "5668060AA49730B7BE4801DF46EC62DE53ECD11ABE43A32873000C36E8DC1F",
             ),
             FieldElement::one(),
-        ]);
-        point.unwrap()
+        ])
     }
 }
 

crates/math/src/elliptic_curve/short_weierstrass/curves/stark_curve.rs

@@ -17,13 +17,12 @@ use crate::traits::AsBytes;
 use alloc::vec::Vec;
 
 #[derive(Clone, Debug)]
-pub struct ShortWeierstrassProjectivePoint<E: IsEllipticCurve>(pub ProjectivePoint<E>);
+pub struct ShortWeierstrassProjectivePoint<E: IsEllipticCurve>(ProjectivePoint<E>);
 
 impl<E: IsShortWeierstrass> ShortWeierstrassProjectivePoint<E> {
     /// Creates an elliptic curve point giving the projective [x: y: z] coordinates.
     pub fn new(value: [FieldElement<E::BaseField>; 3]) -> Result<Self, EllipticCurveError> {
         let (x, y, z) = (&value[0], &value[1], &value[2]);
-
         if z != &FieldElement::<E::BaseField>::zero()
             && E::defining_equation_projective(x, y, z) == FieldElement::<E::BaseField>::zero()
         {
@@ -43,6 +42,22 @@ impl<E: IsShortWeierstrass> ShortWeierstrassProjectivePoint<E> {
         }
     }
 
+    /// Creates an elliptic curve point giving the projective [x: y: z] coordinates without
+    /// checking that the point satisfies the curve equation.
+    pub const fn new_unchecked(value: [FieldElement<E::BaseField>; 3]) -> Self {
+        // SAFETY: The caller MUST ensure that [x:y:z] represents valid point on the
+        // curve. Passing arbitrary coordinates here can violate the invariant
+        // and produce silently incorrect results in subsequent operations.
+        Self(ProjectivePoint::new(value))
+    }
+
+    /// Changes the point coordinates without checking that it satisfies the curve equation.
+    pub fn set_unchecked(&mut self, value: [FieldElement<E::BaseField>; 3]) {
+        // SAFETY: The caller MUST ensure that the provided coordinates represent a valid curve
+        // point. Setting invalid coordinates may lead to silently incorrect computations later on.
+        self.0.value = value
+    }
+
     /// Returns the `x` coordinate of the point.
     pub fn x(&self) -> &FieldElement<E::BaseField> {
         self.0.x()
@@ -111,8 +126,7 @@ impl<E: IsShortWeierstrass> ShortWeierstrassProjectivePoint<E> {
         );
         // 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.
-        let point = Self::new([xp, yp, zp]);
-        point.unwrap()
+        Self::new_unchecked([xp, yp, zp])
     }
     // 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
@@ -132,12 +146,11 @@ impl<E: IsShortWeierstrass> ShortWeierstrassProjectivePoint<E> {
         if u == *py {
             if v != *px || *py == FieldElement::zero() {
                 // SAFETY: The point (0, 1, 0) is defined as the point at infinity.
-                return Self::new([
+                return Self::new_unchecked([
                     FieldElement::zero(),
                     FieldElement::one(),
                     FieldElement::zero(),
-                ])
-                .unwrap();
+                ]);
             } else {
                 return self.double();
             }
@@ -161,8 +174,7 @@ impl<E: IsShortWeierstrass> ShortWeierstrassProjectivePoint<E> {
         );
         // SAFETY: The values `x, y, z` are computed correctly to be on the curve.
         // The assertion above verifies that the resulting point is valid.
-        let point = Self::new([x, y, z]);
-        point.unwrap()
+        Self::new_unchecked([x, y, z])
     }
 }
 
@@ -189,13 +201,11 @@ impl<E: IsShortWeierstrass> IsGroup for ShortWeierstrassProjectivePoint<E> {
     fn neutral_element() -> Self {
         // SAFETY:
         // - `(0, 1, 0)` is **mathematically valid** as the neutral element.
-        // - `unwrap()` is safe because this is **a known valid point**.
-        Self::new([
+        Self::new_unchecked([
             FieldElement::zero(),
             FieldElement::one(),
             FieldElement::zero(),
         ])
-        .unwrap()
     }
 
     fn is_neutral_element(&self) -> bool {
@@ -245,7 +255,7 @@ impl<E: IsShortWeierstrass> IsGroup for ShortWeierstrassProjectivePoint<E> {
                 );
                 // 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()
+                Self::new_unchecked([xp, yp, zp])
             }
         }
     }
@@ -255,8 +265,7 @@ impl<E: IsShortWeierstrass> IsGroup for ShortWeierstrassProjectivePoint<E> {
         let [px, py, pz] = self.coordinates();
         // SAFETY:
         // - Negating `y` maintains the curve structure.
-        // - `unwraps()` is safe because negation **is always valid**.
-        Self::new([px.clone(), -py, pz.clone()]).unwrap()
+        Self::new_unchecked([px.clone(), -py, pz.clone()])
     }
 }
 
@@ -454,6 +463,15 @@ impl<E: IsShortWeierstrass> ShortWeierstrassJacobianPoint<E> {
         }
     }
 
+    /// Creates an elliptic curve point giving the projective [x: y: z] coordinates without
+    /// checking that the point satisfies the curve equation.
+    pub const fn new_unchecked(value: [FieldElement<E::BaseField>; 3]) -> Self {
+        // SAFETY: The caller MUST ensure that [x:y:z] represents either a valid point on the
+        // curve. Passing arbitrary coordinates here can violate the invariant
+        // and produce silently incorrect results in subsequent operations.
+        Self(JacobianPoint::new(value))
+    }
+
     /// Returns the `x` coordinate of the point.
     pub fn x(&self) -> &FieldElement<E::BaseField> {
         self.0.x()
@@ -508,7 +526,7 @@ impl<E: IsShortWeierstrass> ShortWeierstrassJacobianPoint<E> {
             );
             // SAFETY: The values `x_3, y_3, z_3` are computed correctly to be on the curve.
             // The assertion above verifies that the resulting point is valid.
-            Self::new([x3, y3, z3]).unwrap()
+            Self::new_unchecked([x3, y3, z3])
         } else {
             // http://www.hyperelliptic.org/EFD/g1p/data/shortw/jacobian-0/doubling/dbl-2009-alnr
             // http://www.hyperelliptic.org/EFD/g1p/auto-shortw-jacobian-0.html#doubling-dbl-2009-l
@@ -528,7 +546,7 @@ impl<E: IsShortWeierstrass> ShortWeierstrassJacobianPoint<E> {
             );
             // SAFETY: The values `x_3, y_3, z_3` are computed correctly to be on the curve.
             // The assertion above verifies that the resulting point is valid.
-            Self::new([x3, y3, z3]).unwrap()
+            Self::new_unchecked([x3, y3, z3])
         }
     }
 
@@ -572,7 +590,7 @@ impl<E: IsShortWeierstrass> ShortWeierstrassJacobianPoint<E> {
             );
             // SAFETY: The values `x_3, y_3, z_3` are computed correctly to be on the curve.
             // The assertion above verifies that the resulting point is valid.
-            Self::new([x3, y3, z3]).unwrap()
+            Self::new_unchecked([x3, y3, z3])
         }
     }
 }
@@ -600,14 +618,11 @@ impl<E: IsShortWeierstrass> IsGroup for ShortWeierstrassJacobianPoint<E> {
     fn neutral_element() -> Self {
         // SAFETY:
         // - `(1, 1, 0)` is **mathematically valid** as the neutral element.
-        // - `unwrap()` is safe because this is **a known valid point**.
-
-        Self::new([
+        Self::new_unchecked([
             FieldElement::one(),
             FieldElement::one(),
             FieldElement::zero(),
         ])
-        .unwrap()
     }
 
     fn is_neutral_element(&self) -> bool {
@@ -677,7 +692,7 @@ impl<E: IsShortWeierstrass> IsGroup for ShortWeierstrassJacobianPoint<E> {
         );
         // SAFETY: The values `x_3, y_3, z_3` are computed correctly to be on the curve.
         // The assertion above verifies that the resulting point is valid.
-        Self::new([x3, y3, z3]).unwrap()
+        Self::new_unchecked([x3, y3, z3])
     }
 
     /// Returns the additive inverse of the jacobian point `p`
@@ -686,7 +701,7 @@ impl<E: IsShortWeierstrass> IsGroup for ShortWeierstrassJacobianPoint<E> {
         // SAFETY:
         // - The negation formula for Short Weierstrass curves is well-defined.
         // - The result remains a valid curve point.
-        Self::new([x.clone(), -y, z.clone()]).unwrap()
+        Self::new_unchecked([x.clone(), -y, z.clone()])
     }
 }