Skip to content

Commit

Permalink
remove unchecked from unwrap. Save work
Browse files Browse the repository at this point in the history
  • Loading branch information
jotabulacios committed Feb 21, 2025
1 parent 4c7a1ec commit 0475dea
Show file tree
Hide file tree
Showing 26 changed files with 286 additions and 314 deletions.
2 changes: 1 addition & 1 deletion math/benches/criterion_elliptic_curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ use elliptic_curves::{
criterion_group!(
name = elliptic_curve_benches;
config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
targets = bn_254_elliptic_curve_benchmarks,bls12_381_elliptic_curve_benchmarks,bls12_377_elliptic_curve_benchmarks
targets = bn_254_elliptic_curve_benchmarks,
);
criterion_main!(elliptic_curve_benches);
17 changes: 13 additions & 4 deletions math/benches/elliptic_curves/bn_254.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,22 @@ type G2 = ShortWeierstrassProjectivePoint<BN254TwistCurve>;
#[allow(dead_code)]
pub fn bn_254_elliptic_curve_benchmarks(c: &mut Criterion) {
let mut rng = StdRng::seed_from_u64(42);
let a_val: u128 = rng.gen();
let b_val: u128 = rng.gen();
let a_g1 = BN254Curve::generator().operate_with_self(a_val);

// let a_val: u128 = rng.gen();
// let b_val: u128 = rng.gen();
let a_val: u128 = 2;
let b_val: u128 = 93;
let p = BN254Curve::generator();
let q = BN254TwistCurve::generator();
let a_g1 = BN254Curve::generator().operate_with(&p);
let b_g1 = BN254Curve::generator().operate_with_self(b_val);

let a_g2 = BN254TwistCurve::generator().operate_with_self(a_val);
//let a_g2 = BN254TwistCurve::generator().operate_with_self(2u32);
let a_g2 = BN254TwistCurve::generator().operate_with_self(2u32);
assert!(a_g2.is_in_subgroup(), "a_g2 is not in the subgroup!");

let b_g2 = BN254TwistCurve::generator().operate_with_self(b_val);

let f_12 = Fp12E::from_coefficients(&[
"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12",
]);
Expand Down
23 changes: 11 additions & 12 deletions math/src/elliptic_curve/edwards/curves/bandersnatch/curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,17 @@ impl IsEllipticCurve for BandersnatchCurve {
// 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()
}

Self::PointRepresentation::new([
FieldElement::<Self::BaseField>::new_base(
"29C132CC2C0B34C5743711777BBE42F32B79C022AD998465E1E71866A252AE18",
),
FieldElement::<Self::BaseField>::new_base(
"2A6C669EDA123E0F157D8B50BADCD586358CAD81EEE464605E3167B6CC974166",
),
FieldElement::one(),
])
.unwrap()
}
}

Expand Down
7 changes: 3 additions & 4 deletions math/src/elliptic_curve/edwards/curves/ed448_goldilocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,12 @@ impl IsEllipticCurve for Ed448Goldilocks {
// - 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([

Self::PointRepresentation::new([
FieldElement::<Self::BaseField>::from_hex("4f1970c66bed0ded221d15a622bf36da9e146570470f1767ea6de324a3d3a46412ae1af72ab66511433b80e18b00938e2626a82bc70cc05e").unwrap(),
FieldElement::<Self::BaseField>::from_hex("693f46716eb6bc248876203756c9c7624bea73736ca3984087789c1e05a0c2d73ad3ff1ce67c39c4fdbd132c4ed7c8ad9808795bf230fa14").unwrap(),
FieldElement::one(),
]).unwrap_unchecked()
}
]).unwrap()
}
}

Expand Down
15 changes: 7 additions & 8 deletions math/src/elliptic_curve/edwards/curves/tiny_jub_jub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ impl IsEllipticCurve for TinyJubJubEdwards {
// 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()
}

Self::PointRepresentation::new([
FieldElement::from(8),
FieldElement::from(5),
FieldElement::one(),
])
.unwrap()
}
}

Expand Down
19 changes: 9 additions & 10 deletions math/src/elliptic_curve/edwards/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,13 @@ impl<E: IsEdwards> IsGroup for EdwardsProjectivePoint<E> {
// 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()
}

Self::new([
FieldElement::zero(),
FieldElement::one(),
FieldElement::one(),
])
.unwrap()
}

fn is_neutral_element(&self) -> bool {
Expand Down Expand Up @@ -138,7 +137,7 @@ impl<E: IsEdwards> IsGroup for EdwardsProjectivePoint<E> {
let den_s2 = &one - &dx1x2y1y2;

// 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() }
Self::new([&num_s1 / &den_s1, &num_s2 / &den_s2, one]).unwrap()
}

/// Returns the additive inverse of the projective point `p`
Expand All @@ -152,7 +151,7 @@ impl<E: IsEdwards> IsGroup for EdwardsProjectivePoint<E> {
// 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() }
Self::new([-px, py.clone(), pz.clone()]).unwrap()
}
}

Expand Down
15 changes: 7 additions & 8 deletions math/src/elliptic_curve/montgomery/curves/tiny_jub_jub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ impl IsEllipticCurve for TinyJubJubMontgomery {
// 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()
}

Self::PointRepresentation::new([
FieldElement::from(3),
FieldElement::from(5),
FieldElement::one(),
])
.unwrap()
}
}

Expand Down
21 changes: 10 additions & 11 deletions math/src/elliptic_curve/montgomery/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,13 @@ impl<E: IsMontgomery> IsGroup for MontgomeryProjectivePoint<E> {
// SAFETY:
// - `(0, 1, 0)` is **mathematically valid** as the neutral element.
// - `unwrap_unchecked()` is safe because this is **a known valid point**.
unsafe {
Self::new([
FieldElement::zero(),
FieldElement::one(),
FieldElement::zero(),
])
.unwrap_unchecked()
}

Self::new([
FieldElement::zero(),
FieldElement::one(),
FieldElement::zero(),
])
.unwrap()
}

fn is_neutral_element(&self) -> bool {
Expand Down Expand Up @@ -151,7 +150,7 @@ impl<E: IsMontgomery> IsGroup for MontgomeryProjectivePoint<E> {
// SAFETY:
// - The Montgomery addition formula guarantees a **valid** curve point.
// - `unwrap_unchecked()` is safe because the input points are **valid**.
unsafe { Self::new([new_x, new_y, one]).unwrap_unchecked() }
Self::new([new_x, new_y, one]).unwrap()
// In the rest of the cases we have x1 != x2
} else {
let num = &y2 - &y1;
Expand All @@ -164,7 +163,7 @@ impl<E: IsMontgomery> IsGroup for MontgomeryProjectivePoint<E> {
// SAFETY:
// - The result of the Montgomery addition formula is **guaranteed** to be a valid point.
// - `unwrap_unchecked()` is safe because we **control** the inputs.
unsafe { Self::new([new_x, new_y, FieldElement::one()]).unwrap_unchecked() }
Self::new([new_x, new_y, FieldElement::one()]).unwrap()
}
}
}
Expand All @@ -180,7 +179,7 @@ impl<E: IsMontgomery> IsGroup for MontgomeryProjectivePoint<E> {
// SAFETY:
// - Negating `y` maintains the curve structure.
// - `unwrap_unchecked()` is safe because negation **is always valid**.
unsafe { Self::new([px.clone(), -py, pz.clone()]).unwrap_unchecked() }
Self::new([px.clone(), -py, pz.clone()]).unwrap()
}
}

Expand Down
22 changes: 10 additions & 12 deletions math/src/elliptic_curve/short_weierstrass/curves/bls12_377/curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,12 @@ impl IsEllipticCurve for BLS12377Curve {
// SAFETY:
// - These values are mathematically verified and known to be valid points on BLS12-377.
// - `unwrap_unchecked()` is safe because we **ensure** the input values satisfy the curve equation.
unsafe {
Self::PointRepresentation::new([

Self::PointRepresentation::new([
FieldElement::<Self::BaseField>::new_base("8848defe740a67c8fc6225bf87ff5485951e2caa9d41bb188282c8bd37cb5cd5481512ffcd394eeab9b16eb21be9ef"),
FieldElement::<Self::BaseField>::new_base("1914a69c5102eff1f674f5d30afeec4bd7fb348ca3e52d96d182ad44fb82305c2fe3d3634a9591afd82de55559c8ea6"),
FieldElement::one()
]).unwrap_unchecked()
}
]).unwrap()
}
}

Expand Down Expand Up @@ -124,14 +123,13 @@ impl ShortWeierstrassProjectivePoint<BLS12377TwistCurve> {
// resulting point satisfies the curve equation.
// - `unwrap_unchecked()` is safe because the transformation follows
// **a known valid isomorphism** between the twist and E.
unsafe {
Self::new([
x.conjugate() * GAMMA_12,
y.conjugate() * GAMMA_13,
z.conjugate(),
])
.unwrap_unchecked()
}

Self::new([
x.conjugate() * GAMMA_12,
y.conjugate() * GAMMA_13,
z.conjugate(),
])
.unwrap()
}

/// 𝜓(P) = 𝑢P, where 𝑢 = SEED of the curve
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ fn line(p: &G1Point, t: &G2Point, q: &G2Point) -> (G2Point, Fp12E) {
);
// SAFETY: `unwrap_unchecked()` is used here because we ensure that `x_r, y_r, z_r`
// satisfy the curve equation. The previous assertion checks that this is indeed the case.
let r = unsafe { G2Point::new([x_r, y_r, z_r]).unwrap_unchecked() };
let r = G2Point::new([x_r, y_r, z_r]).unwrap();

let l = Fp12E::new([
Fp6E::new([y_p * (-h), Fp2E::zero(), Fp2E::zero()]),
Expand Down Expand Up @@ -200,7 +200,7 @@ fn line(p: &G1Point, t: &G2Point, q: &G2Point) -> (G2Point, Fp12E) {
);
// SAFETY: The values `x_r, y_r, z_r` are computed correctly to be on the curve.
// The assertion above verifies that the resulting point is valid.
let r = unsafe { G2Point::new([x_r, y_r, z_r]).unwrap_unchecked() };
let r = G2Point::new([x_r, y_r, z_r]).unwrap();

let l = Fp12E::new([
Fp6E::new([y_p * lambda, Fp2E::zero(), Fp2E::zero()]),
Expand Down
27 changes: 13 additions & 14 deletions math/src/elliptic_curve/short_weierstrass/curves/bls12_377/twist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,19 @@ impl IsEllipticCurve for BLS12377TwistCurve {
// SAFETY:
// - The generator point is mathematically verified to be a valid point on the curve.
// - `unwrap_unchecked()` is safe because the provided coordinates satisfy the curve equation.
unsafe {
Self::PointRepresentation::new([
FieldElement::new([
FieldElement::new(GENERATOR_X_0),
FieldElement::new(GENERATOR_X_1),
]),
FieldElement::new([
FieldElement::new(GENERATOR_Y_0),
FieldElement::new(GENERATOR_Y_1),
]),
FieldElement::one(),
])
.unwrap_unchecked()
}

Self::PointRepresentation::new([
FieldElement::new([
FieldElement::new(GENERATOR_X_0),
FieldElement::new(GENERATOR_X_1),
]),
FieldElement::new([
FieldElement::new(GENERATOR_Y_0),
FieldElement::new(GENERATOR_Y_1),
]),
FieldElement::one(),
])
.unwrap()
}
}

Expand Down
46 changes: 21 additions & 25 deletions math/src/elliptic_curve/short_weierstrass/curves/bls12_381/curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,12 @@ impl IsEllipticCurve for BLS12381Curve {
// SAFETY:
// - These values are mathematically verified and known to be valid points on BLS12-381.
// - `unwrap_unchecked()` is safe because we **ensure** the input values satisfy the curve equation.
unsafe {
Self::PointRepresentation::new([

Self::PointRepresentation::new([
FieldElement::<Self::BaseField>::new_base("17f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb"),
FieldElement::<Self::BaseField>::new_base("8b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e1"),
FieldElement::one()
]).unwrap_unchecked()
}
]).unwrap()
}
}

Expand Down Expand Up @@ -111,20 +110,19 @@ impl ShortWeierstrassProjectivePoint<BLS12381TwistCurve> {
/// - `unwrap_unchecked()` is used because `psi()` is defined to **always return a valid point**.
fn psi(&self) -> Self {
let [x, y, z] = self.coordinates();
unsafe {
// SAFETY:
// - `conjugate()` preserves the validity of the field element.
// - `ENDO_U` and `ENDO_V` are precomputed constants that ensure the
// resulting point satisfies the curve equation.
// - `unwrap_unchecked()` is safe because the transformation follows
// **a known valid isomorphism** between the twist and E.
Self::new([
x.conjugate() * ENDO_U,
y.conjugate() * ENDO_V,
z.conjugate(),
])
.unwrap_unchecked()
}

// SAFETY:
// - `conjugate()` preserves the validity of the field element.
// - `ENDO_U` and `ENDO_V` are precomputed constants that ensure the
// resulting point satisfies the curve equation.
// - `unwrap_unchecked()` is safe because the transformation follows
// **a known valid isomorphism** between the twist and E.
Self::new([
x.conjugate() * ENDO_U,
y.conjugate() * ENDO_V,
z.conjugate(),
])
.unwrap()
}

/// 𝜓(P) = 𝑢P, where 𝑢 = SEED of the curve
Expand Down Expand Up @@ -175,13 +173,11 @@ mod tests {
) -> ShortWeierstrassProjectivePoint<BLS12381TwistCurve> {
let [x, y, z] = p.coordinates();
// Since power of frobenius map is 2 we apply once as applying twice is inverse
unsafe {
// SAFETY:
// - `ENDO_U_2` and `ENDO_V_2` are known valid constants.
// - `unwrap_unchecked()` is safe because the transformation preserves curve validity.
ShortWeierstrassProjectivePoint::new([x * ENDO_U_2, y * ENDO_V_2, z.clone()])
.unwrap_unchecked()
}

// SAFETY:
// - `ENDO_U_2` and `ENDO_V_2` are known valid constants.
// - `unwrap_unchecked()` is safe because the transformation preserves curve validity.
ShortWeierstrassProjectivePoint::new([x * ENDO_U_2, y * ENDO_V_2, z.clone()]).unwrap()
}

#[allow(clippy::upper_case_acronyms)]
Expand Down
Loading

0 comments on commit 0475dea

Please sign in to comment.