Change elliptic curve new point function

math/benches/fields/baby_bear.rs

@@ -15,7 +15,7 @@ use lambdaworks_math::field::{
 
 use p3_baby_bear::BabyBear;
 use p3_field::extension::BinomialExtensionField;
-use p3_field::{Field, FieldAlgebra};
+use p3_field::{Field, PrimeCharacteristicRing};
 
 use rand::random;
 use rand::Rng;
@@ -83,18 +83,25 @@ pub fn rand_babybear_u32_fp4_elements(num: usize) -> Vec<(Fp4Eu32, Fp4Eu32)> {
     }
     result
 }
-
+fn random_baby_bear<R: Rng>(rng: &mut R) -> BabyBear {
+    BabyBear::new(rng.gen::<u32>())
+}
 fn rand_babybear_elements_p3(num: usize) -> Vec<(BabyBear, BabyBear)> {
     let mut rng = rand::thread_rng();
     (0..num)
-        .map(|_| (rng.gen::<BabyBear>(), rng.gen::<BabyBear>()))
+        .map(|_| (random_baby_bear(&mut rng), random_baby_bear(&mut rng)))
         .collect()
 }
 
 fn rand_babybear_fp4_elements_p3(num: usize) -> Vec<(EF4, EF4)> {
     let mut rng = rand::thread_rng();
     (0..num)
-        .map(|_| (rng.gen::<EF4>(), rng.gen::<EF4>()))
+        .map(|_| {
+            (
+                EF4::from(random_baby_bear(&mut rng)),
+                EF4::from(random_baby_bear(&mut rng)),
+            )
+        })
         .collect()
 }
 

math/src/elliptic_curve/edwards/curves/bandersnatch/curve.rs

@@ -12,18 +12,39 @@ impl IsEllipticCurve for BandersnatchCurve {
     type BaseField = BaseBandersnatchFieldElement;
     type PointRepresentation = EdwardsProjectivePoint<Self>;
 
-    // Values are from https://github.com/arkworks-rs/curves/blob/5a41d7f27a703a7ea9c48512a4148443ec6c747e/ed_on_bls12_381_bandersnatch/src/curves/mod.rs#L120
-    // Converted to Hex
+    /// Returns the generator point of the Bandersnatch curve.
+    ///
+    /// The generator point is defined with coordinates `(x, y, 1)`, where `x` and `y`
+    /// are precomputed constants that belong to the curve.
+    ///
+    /// # Safety
+    ///
+    /// - The generator values are taken from the [Arkworks implementation](https://github.com/arkworks-rs/curves/blob/5a41d7f27a703a7ea9c48512a4148443ec6c747e/ed_on_bls12_381_bandersnatch/src/curves/mod.rs#L120)
+    ///   and have been converted to hexadecimal.
+    /// - `unwrap_unchecked()` is safe because:
+    ///   - The generator point is **known to be valid** on the curve.
+    ///   - The function only uses **hardcoded** and **verified** constants.
+    /// - This function should **never** be modified unless the new generator is fully verified.
     fn generator() -> Self::PointRepresentation {
-        Self::PointRepresentation::new([
-            FieldElement::<Self::BaseField>::new_base(
-                "29C132CC2C0B34C5743711777BBE42F32B79C022AD998465E1E71866A252AE18",
-            ),
-            FieldElement::<Self::BaseField>::new_base(
-                "2A6C669EDA123E0F157D8B50BADCD586358CAD81EEE464605E3167B6CC974166",
-            ),
-            FieldElement::one(),
-        ])
+        // SAFETY:
+        // - The generator point coordinates (x, y) are taken from a well-tested,
+        //   verified implementation.
+        // - The constructor will only fail if the values are invalid, which is
+        //   impossible given that they are constants taken from a trusted source.
+        // - `unwrap_unchecked()` avoids unnecessary checks, as we guarantee
+        //   correctness based on external verification.
+        unsafe {
+            Self::PointRepresentation::new([
+                FieldElement::<Self::BaseField>::new_base(
+                    "29C132CC2C0B34C5743711777BBE42F32B79C022AD998465E1E71866A252AE18",
+                ),
+                FieldElement::<Self::BaseField>::new_base(
+                    "2A6C669EDA123E0F157D8B50BADCD586358CAD81EEE464605E3167B6CC974166",
+                ),
+                FieldElement::one(),
+            ])
+            .unwrap_unchecked()
+        }
     }
 }
 

math/src/elliptic_curve/edwards/curves/ed448_goldilocks.rs

@@ -13,13 +13,28 @@ impl IsEllipticCurve for Ed448Goldilocks {
     type BaseField = P448GoldilocksPrimeField;
     type PointRepresentation = EdwardsProjectivePoint<Self>;
 
-    /// Taken from https://www.rfc-editor.org/rfc/rfc7748#page-6
+    /// Returns the generator point of the Ed448-Goldilocks curve.
+    ///
+    /// This generator is taken from [RFC 7748](https://www.rfc-editor.org/rfc/rfc7748#page-6).
+    ///
+    /// # Safety
+    ///
+    /// - The generator coordinates `(x, y, 1)` are well-known, predefined constants.
+    /// - `unwrap_unchecked()` is used because the values are **known to be valid** points
+    ///   on the Ed448-Goldilocks curve.
+    /// - This function must **not** be modified unless new constants are mathematically verified.
     fn generator() -> Self::PointRepresentation {
-        Self::PointRepresentation::new([
+        // SAFETY:
+        // - These values are taken from RFC 7748 and are known to be valid.
+        // - `unwrap_unchecked()` is safe because `new()` will only fail if the point is
+        //   invalid, which is **not possible** with hardcoded, verified values.
+        unsafe {
+            Self::PointRepresentation::new([
             FieldElement::<Self::BaseField>::from_hex("4f1970c66bed0ded221d15a622bf36da9e146570470f1767ea6de324a3d3a46412ae1af72ab66511433b80e18b00938e2626a82bc70cc05e").unwrap(),
             FieldElement::<Self::BaseField>::from_hex("693f46716eb6bc248876203756c9c7624bea73736ca3984087789c1e05a0c2d73ad3ff1ce67c39c4fdbd132c4ed7c8ad9808795bf230fa14").unwrap(),
             FieldElement::one(),
-        ])
+        ]).unwrap_unchecked()
+        }
     }
 }
 

math/src/elliptic_curve/edwards/curves/tiny_jub_jub.rs

@@ -14,12 +14,28 @@ impl IsEllipticCurve for TinyJubJubEdwards {
     type BaseField = U64PrimeField<13>;
     type PointRepresentation = EdwardsProjectivePoint<Self>;
 
+    /// Returns the generator point of the TinyJubJub Edwards curve.
+    ///
+    /// This generator is taken from **Moonmath Manual (page 97)**.
+    ///
+    /// # Safety
+    ///
+    /// - The generator coordinates `(8, 5, 1)` are **predefined** and belong to the TinyJubJub curve.
+    /// - `unwrap_unchecked()` is used because the generator is a **verified valid point**,
+    ///   meaning there is **no risk** of runtime failure.
+    /// - This function must **not** be modified unless the new generator is mathematically verified.
     fn generator() -> Self::PointRepresentation {
-        Self::PointRepresentation::new([
-            FieldElement::from(8),
-            FieldElement::from(5),
-            FieldElement::one(),
-        ])
+        // SAFETY:
+        // - The generator point `(8, 5, 1)` is **mathematically valid** on the curve.
+        // - `unwrap_unchecked()` is safe because we **know** the point satisfies the curve equation.
+        unsafe {
+            Self::PointRepresentation::new([
+                FieldElement::from(8),
+                FieldElement::from(5),
+                FieldElement::one(),
+            ])
+            .unwrap_unchecked()
+        }
     }
 }
 

math/src/elliptic_curve/edwards/point.rs

@@ -12,10 +12,27 @@ use super::traits::IsEdwards;
 #[derive(Clone, Debug)]
 pub struct EdwardsProjectivePoint<E: IsEllipticCurve>(ProjectivePoint<E>);
 
-impl<E: IsEllipticCurve> EdwardsProjectivePoint<E> {
+impl<E: IsEllipticCurve + IsEdwards> EdwardsProjectivePoint<E> {
     /// Creates an elliptic curve point giving the projective [x: y: z] coordinates.
-    pub fn new(value: [FieldElement<E::BaseField>; 3]) -> Self {
-        Self(ProjectivePoint::new(value))
+    pub fn new(value: [FieldElement<E::BaseField>; 3]) -> Result<Self, EllipticCurveError> {
+        let (x, y, z) = (&value[0], &value[1], &value[2]);
+
+        // The point at infinity is (0, 1, 1).
+        // We convert every (0, y, y) into the infinity.
+        if x == &FieldElement::<E::BaseField>::zero() && z == y {
+            return Ok(Self(ProjectivePoint::new([
+                FieldElement::<E::BaseField>::zero(),
+                FieldElement::<E::BaseField>::one(),
+                FieldElement::<E::BaseField>::one(),
+            ])));
+        }
+        if z != &FieldElement::<E::BaseField>::zero()
+            && E::defining_equation_projective(x, y, z) == FieldElement::<E::BaseField>::zero()
+        {
+            Ok(Self(ProjectivePoint::new(value)))
+        } else {
+            Err(EllipticCurveError::InvalidPoint)
+        }
     }
 
     /// Returns the `x` coordinate of the point.
@@ -56,35 +73,52 @@ impl<E: IsEdwards> FromAffine<E::BaseField> for EdwardsProjectivePoint<E> {
     fn from_affine(
         x: FieldElement<E::BaseField>,
         y: FieldElement<E::BaseField>,
-    ) -> Result<Self, crate::elliptic_curve::traits::EllipticCurveError> {
-        if E::defining_equation(&x, &y) != FieldElement::zero() {
-            Err(EllipticCurveError::InvalidPoint)
-        } else {
-            let coordinates = [x, y, FieldElement::one()];
-            Ok(EdwardsProjectivePoint::new(coordinates))
-        }
+    ) -> Result<Self, EllipticCurveError> {
+        let coordinates = [x, y, FieldElement::one()];
+        EdwardsProjectivePoint::new(coordinates)
     }
 }
 
 impl<E: IsEllipticCurve> Eq for EdwardsProjectivePoint<E> {}
 
 impl<E: IsEdwards> IsGroup for EdwardsProjectivePoint<E> {
-    /// The point at infinity.
+    /// Returns the point at infinity (neutral element) in projective coordinates.
+    ///
+    /// # Safety
+    ///
+    /// - The values `[0, 1, 1]` are the **canonical representation** of the neutral element
+    ///   in the Edwards curve, meaning they are guaranteed to be a valid point.
+    /// - `unwrap_unchecked()` is used because this point is **known** to be valid, so
+    ///   there is no need for additional runtime checks.
     fn neutral_element() -> Self {
-        Self::new([
-            FieldElement::zero(),
-            FieldElement::one(),
-            FieldElement::one(),
-        ])
+        // SAFETY:
+        // - `[0, 1, 1]` is a mathematically verified neutral element in Edwards curves.
+        // - `unwrap_unchecked()` is safe because this point is **always valid**.
+        unsafe {
+            Self::new([
+                FieldElement::zero(),
+                FieldElement::one(),
+                FieldElement::one(),
+            ])
+            .unwrap_unchecked()
+        }
     }
 
     fn is_neutral_element(&self) -> bool {
         let [px, py, pz] = self.coordinates();
         px == &FieldElement::zero() && py == pz
     }
 
-    /// Computes the addition of `self` and `other`.
-    /// Taken from "Moonmath" (Eq 5.38, page 97)
+    /// Computes the addition of `self` and `other` using the Edwards curve addition formula.
+    ///
+    /// This implementation follows Equation (5.38) from "Moonmath" (page 97).
+    ///
+    /// # Safety
+    ///
+    /// - The function assumes both `self` and `other` are valid points on the curve.
+    /// - The resulting coordinates are computed using a well-defined formula that
+    ///   maintains the elliptic curve invariants.
+    /// - `unwrap_unchecked()` is safe because the formula guarantees the result is valid.
     fn operate_with(&self, other: &Self) -> Self {
         // This avoids dropping, which in turn saves us from having to clone the coordinates.
         let (s_affine, o_affine) = (self.to_affine(), other.to_affine());
@@ -103,13 +137,22 @@ impl<E: IsEdwards> IsGroup for EdwardsProjectivePoint<E> {
         let num_s2 = &y1y2 - E::a() * &x1x2;
         let den_s2 = &one - &dx1x2y1y2;
 
-        Self::new([&num_s1 / &den_s1, &num_s2 / &den_s2, one])
+        // SAFETY: The creation of the result point is safe because the inputs are always points that belong to the curve.
+        unsafe { Self::new([&num_s1 / &den_s1, &num_s2 / &den_s2, one]).unwrap_unchecked() }
     }
 
     /// Returns the additive inverse of the projective point `p`
+    ///  
+    /// # Safety
+    ///
+    /// - Negating the x-coordinate of a valid Edwards point results in another valid point.
+    /// - `unwrap_unchecked()` is safe because negation does not break the curve equation.
     fn neg(&self) -> Self {
         let [px, py, pz] = self.coordinates();
-        Self::new([-px, py.clone(), pz.clone()])
+        // SAFETY:
+        // - The negation formula for Edwards curves is well-defined.
+        // - The result remains a valid curve point.
+        unsafe { Self::new([-px, py.clone(), pz.clone()]).unwrap_unchecked() }
     }
 }
 
@@ -156,17 +199,20 @@ mod tests {
             FieldElement::from(5),
             FieldElement::from(5),
             FieldElement::from(1),
-        ]);
+        ])
+        .unwrap();
         let q = EdwardsProjectivePoint::<TinyJubJubEdwards>::new([
             FieldElement::from(8),
             FieldElement::from(5),
             FieldElement::from(1),
-        ]);
+        ])
+        .unwrap();
         let expected = EdwardsProjectivePoint::<TinyJubJubEdwards>::new([
             FieldElement::from(0),
             FieldElement::from(1),
             FieldElement::from(1),
-        ]);
+        ])
+        .unwrap();
         assert_eq!(p.operate_with(&q), expected);
     }
 

math/src/elliptic_curve/edwards/traits.rs

@@ -7,6 +7,8 @@ pub trait IsEdwards: IsEllipticCurve + Clone + Debug {
 
     fn d() -> FieldElement<Self::BaseField>;
 
+    // Edwards equation in affine coordinates:
+    // ax^2 + y^2 - 1 = d * x^2 * y^2
     fn defining_equation(
         x: &FieldElement<Self::BaseField>,
         y: &FieldElement<Self::BaseField>,
@@ -15,4 +17,16 @@ pub trait IsEdwards: IsEllipticCurve + Clone + Debug {
             - FieldElement::<Self::BaseField>::one()
             - Self::d() * x.pow(2_u16) * y.pow(2_u16)
     }
+
+    // Edwards equation in projective coordinates.
+    // a * x^2 * z^2 + y^2 * z^2 - z^4 = d * x^2 * y^2
+    fn defining_equation_projective(
+        x: &FieldElement<Self::BaseField>,
+        y: &FieldElement<Self::BaseField>,
+        z: &FieldElement<Self::BaseField>,
+    ) -> FieldElement<Self::BaseField> {
+        Self::a() * x.square() * z.square() + y.square() * z.square()
+            - z.square().square()
+            - Self::d() * x.square() * y.square()
+    }
 }

math/src/elliptic_curve/montgomery/curves/tiny_jub_jub.rs

@@ -14,12 +14,29 @@ impl IsEllipticCurve for TinyJubJubMontgomery {
     type BaseField = U64PrimeField<13>;
     type PointRepresentation = MontgomeryProjectivePoint<Self>;
 
+    /// Returns the generator point of the TinyJubJub Montgomery curve.
+    ///
+    /// This generator is taken from **Moonmath Manual (page 91)**.
+    ///
+    /// # Safety
+    ///
+    /// - The generator coordinates `(3, 5, 1)` are **predefined** and are **valid** points
+    ///   on the TinyJubJub Montgomery curve.
+    /// - `unwrap_unchecked()` is used because the generator is **guaranteed** to satisfy
+    ///   the Montgomery curve equation.
+    /// - This function must **not** be modified unless the new generator is mathematically verified.
     fn generator() -> Self::PointRepresentation {
-        Self::PointRepresentation::new([
-            FieldElement::from(3),
-            FieldElement::from(5),
-            FieldElement::one(),
-        ])
+        // SAFETY:
+        // - The generator point `(3, 5, 1)` is **mathematically verified**.
+        // - `unwrap_unchecked()` is safe because the input values **guarantee** validity.
+        unsafe {
+            Self::PointRepresentation::new([
+                FieldElement::from(3),
+                FieldElement::from(5),
+                FieldElement::one(),
+            ])
+            .unwrap_unchecked()
+        }
     }
 }