Add Jacobian coordinates for Short Weierstrass #917
Conversation
…lambdaworks into add_jacobian_coordinates_sw
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #917 +/- ##
==========================================
+ Coverage 73.34% 73.36% +0.01%
==========================================
Files 155 155
Lines 35328 35611 +283
==========================================
+ Hits 25913 26127 +214
- Misses 9415 9484 +69
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -52,6 +52,27 @@ impl<E: IsShortWeierstrass> ShortWeierstrassProjectivePoint<E> { | |||
| Self(self.0.to_affine()) | |||
| } | |||
|
|
|||
| // Need to check if this is correct | |||
| // It think it is not | |||
| pub fn to_jacobian(&self) -> Self { | |||
There was a problem hiding this comment.
This is transforming to affine
| @@ -272,6 +294,9 @@ where | |||
| bytes.extend(&x_bytes); | |||
| bytes.extend(&y_bytes); | |||
| } | |||
| PointFormat::Jacobian => { | |||
| panic!("Jacobian format is not supporter for ShortWeierstrassProjectivePoint"); | |||
There was a problem hiding this comment.
Should handle this without panicking
| [self.x(), self.y(), self.z()] | ||
| } | ||
|
|
||
| pub fn to_affine(&self) -> Self { |
There was a problem hiding this comment.
This is already a function defined previously
|
|
||
| pub fn to_affine(&self) -> (FieldElement<E::BaseField>, FieldElement<E::BaseField>) { | ||
| if self.is_neutral_element() { | ||
| panic!("Cannot convert the point at infinity to affine coordinates"); |
There was a problem hiding this comment.
You have a representation for the point in affine
| let e = &a * FieldElement::<E::BaseField>::from(3u64); // E = 3 * A | ||
| let f = e.square(); // F = E^2 | ||
| let x3 = &f - &d.double(); // X3 = F - 2 * D | ||
| let y3 = &e * (&d - &x3) - &c * FieldElement::<E::BaseField>::from(8u64); // Y3 = E * (D - X3) - 8 * C |
There was a problem hiding this comment.
Perhaps this is more efficient as double().double().double()
math/src/elliptic_curve/point.rs
Outdated
| let [x, y, z] = self.coordinates(); | ||
| // If it's the point at infinite | ||
| if z == &FieldElement::zero() { | ||
| // We make sure all the points in the infinite have the same values |
|
|
||
| let a_coeff = E::a(); | ||
| //http://www.hyperelliptic.org/EFD/g1p/data/shortw/jacobian-0/doubling/dbl-2009-l | ||
| if a_coeff == FieldElement::zero() { |
There was a problem hiding this comment.
no need to create a_coeff
| let [x2, y2, z2] = other.coordinates(); | ||
|
|
||
| if *z2 != FieldElement::one() { | ||
| panic!("The second point must be in affine coordinates (Z = 1)"); |
There was a problem hiding this comment.
we could leave as unchecked or handle the error
| return self.clone(); | ||
| } | ||
|
|
||
| let z1z1 = z1.square(); |
There was a problem hiding this comment.
The points could be the same, so we have to handle that fact
| impl<E: IsShortWeierstrass> ShortWeierstrassJacobianPoint<E> { | ||
| /// Creates an elliptic curve point giving the projective [x: y: z] coordinates. | ||
| pub const fn new(value: [FieldElement<E::BaseField>; 3]) -> Self { | ||
| Self(JacobianPoint::new(value)) |
There was a problem hiding this comment.
This does not check that the point is valid
|
|
||
| #[cfg(feature = "alloc")] | ||
| use crate::{ | ||
| elliptic_curve::short_weierstrass::curves::bls12_381::field_extension::BLS12381PrimeField, | ||
| field::element::FieldElement, | ||
| }; | ||
| pub const SUBGROUP_ORDER: U256 = U256::from_hex_unchecked( | ||
| "73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001", |
There was a problem hiding this comment.
Don't define new constants, either import them or create an integer
Add Jacobian coordinates
Description
This PR adds Jacobian coordinates for Short Weierstrass elliptic curves.
Type of change