From 8b2fd03d62ae69168a788748029e69334d066a14 Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Fri, 3 Feb 2023 13:20:23 +0100 Subject: [PATCH 1/2] feat: make AffineRepr::xy() more flexible Currently [`AffineRepr::xy()` returns the field elements as references]. This is only possible if the underlying data structure that implements that traits owns those field elements and can directly have a reference to it. Though there are cases, where providing those direct references is not possible. For example if the underlying structure has a different layout, or if the fields elements you're returning need to be wrapped in a newtype struct (that's my case). With this change, `AffineRepr::xy()` now returns owned data. It's more flexible as now the underlying data structure can have any shape as long as it can return those field elements somehow. [`AffineRepr::xy()` returns the field elements as references]: https://github.com/arkworks-rs/algebra/blob/4e4fc17e69178ab7f929602f17a5e769ebaf34c7/ec/src/lib.rs#L208 --- ec/src/hashing/curve_maps/wb/mod.rs | 6 +++--- ec/src/lib.rs | 6 +++--- ec/src/models/bls12/g2.rs | 4 ++-- ec/src/models/bls12/mod.rs | 8 ++++---- ec/src/models/short_weierstrass/affine.rs | 4 ++-- ec/src/models/short_weierstrass/group.rs | 4 ++-- ec/src/models/twisted_edwards/affine.rs | 4 ++-- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ec/src/hashing/curve_maps/wb/mod.rs b/ec/src/hashing/curve_maps/wb/mod.rs index 4e2644009..10ddbb0f7 100644 --- a/ec/src/hashing/curve_maps/wb/mod.rs +++ b/ec/src/hashing/curve_maps/wb/mod.rs @@ -53,10 +53,10 @@ where let y_num = DensePolynomial::from_coefficients_slice(self.y_map_numerator); let y_den = DensePolynomial::from_coefficients_slice(self.y_map_denominator); - let mut v: [BaseField; 2] = [x_den.evaluate(x), y_den.evaluate(x)]; + let mut v: [BaseField; 2] = [x_den.evaluate(&x), y_den.evaluate(&x)]; batch_inversion(&mut v); - let img_x = x_num.evaluate(x) * v[0]; - let img_y = (y_num.evaluate(x) * y) * v[1]; + let img_x = x_num.evaluate(&x) * v[0]; + let img_y = (y_num.evaluate(&x) * y) * v[1]; Ok(Affine::::new_unchecked(img_x, img_y)) }, None => Ok(Affine::identity()), diff --git a/ec/src/lib.rs b/ec/src/lib.rs index daf169566..af5a0eb68 100644 --- a/ec/src/lib.rs +++ b/ec/src/lib.rs @@ -205,15 +205,15 @@ pub trait AffineRepr: + MulAssign; // needed due to https://github.com/rust-lang/rust/issues/69640 /// Returns the x and y coordinates of this affine point. - fn xy(&self) -> Option<(&Self::BaseField, &Self::BaseField)>; + fn xy(&self) -> Option<(Self::BaseField, Self::BaseField)>; /// Returns the x coordinate of this affine point. - fn x(&self) -> Option<&Self::BaseField> { + fn x(&self) -> Option { self.xy().map(|(x, _)| x) } /// Returns the y coordinate of this affine point. - fn y(&self) -> Option<&Self::BaseField> { + fn y(&self) -> Option { self.xy().map(|(_, y)| y) } diff --git a/ec/src/models/bls12/g2.rs b/ec/src/models/bls12/g2.rs index 661486351..579db4629 100644 --- a/ec/src/models/bls12/g2.rs +++ b/ec/src/models/bls12/g2.rs @@ -57,7 +57,7 @@ impl From> for G2Prepared

{ ell_coeffs: vec![], infinity: true, }; - q.xy().map_or(zero, |(&q_x, &q_y)| { + q.xy().map_or(zero, |(q_x, q_y)| { let mut ell_coeffs = vec![]; let mut r = G2HomProjective::

{ x: q_x, @@ -133,7 +133,7 @@ impl G2HomProjective

{ } fn add_in_place(&mut self, q: &G2Affine

) -> EllCoeff

{ - let (&qx, &qy) = q.xy().unwrap(); + let (qx, qy) = q.xy().unwrap(); // Formula for line function when working with // homogeneous projective coordinates. let theta = self.y - &(qy * &self.z); diff --git a/ec/src/models/bls12/mod.rs b/ec/src/models/bls12/mod.rs index ca8a66351..f5275f697 100644 --- a/ec/src/models/bls12/mod.rs +++ b/ec/src/models/bls12/mod.rs @@ -178,13 +178,13 @@ impl Bls12

{ match P::TWIST_TYPE { TwistType::M => { - c2.mul_assign_by_fp(py); - c1.mul_assign_by_fp(px); + c2.mul_assign_by_fp(&py); + c1.mul_assign_by_fp(&px); f.mul_by_014(&c0, &c1, &c2); }, TwistType::D => { - c0.mul_assign_by_fp(py); - c1.mul_assign_by_fp(px); + c0.mul_assign_by_fp(&py); + c1.mul_assign_by_fp(&px); f.mul_by_034(&c0, &c1, &c2); }, } diff --git a/ec/src/models/short_weierstrass/affine.rs b/ec/src/models/short_weierstrass/affine.rs index b1340de3a..7633d321a 100644 --- a/ec/src/models/short_weierstrass/affine.rs +++ b/ec/src/models/short_weierstrass/affine.rs @@ -205,8 +205,8 @@ impl AffineRepr for Affine

{ type ScalarField = P::ScalarField; type Group = Projective

; - fn xy(&self) -> Option<(&Self::BaseField, &Self::BaseField)> { - (!self.infinity).then(|| (&self.x, &self.y)) + fn xy(&self) -> Option<(Self::BaseField, Self::BaseField)> { + (!self.infinity).then(|| (self.x, self.y)) } #[inline] diff --git a/ec/src/models/short_weierstrass/group.rs b/ec/src/models/short_weierstrass/group.rs index 99fa901d2..49511646f 100644 --- a/ec/src/models/short_weierstrass/group.rs +++ b/ec/src/models/short_weierstrass/group.rs @@ -332,7 +332,7 @@ impl Neg for Projective

{ impl>> AddAssign for Projective

{ fn add_assign(&mut self, other: T) { let other = other.borrow(); - if let Some((&other_x, &other_y)) = other.xy() { + if let Some((other_x, other_y)) = other.xy() { if self.is_zero() { self.x = other_x; self.y = other_y; @@ -563,7 +563,7 @@ impl> Mul for Projective

{ impl From> for Projective

{ #[inline] fn from(p: Affine

) -> Projective

{ - p.xy().map_or(Projective::zero(), |(&x, &y)| Self { + p.xy().map_or(Projective::zero(), |(x, y)| Self { x, y, z: P::BaseField::one(), diff --git a/ec/src/models/twisted_edwards/affine.rs b/ec/src/models/twisted_edwards/affine.rs index a6c908e31..4bf186721 100644 --- a/ec/src/models/twisted_edwards/affine.rs +++ b/ec/src/models/twisted_edwards/affine.rs @@ -166,8 +166,8 @@ impl AffineRepr for Affine

{ type ScalarField = P::ScalarField; type Group = Projective

; - fn xy(&self) -> Option<(&Self::BaseField, &Self::BaseField)> { - (!self.is_zero()).then(|| (&self.x, &self.y)) + fn xy(&self) -> Option<(Self::BaseField, Self::BaseField)> { + (!self.is_zero()).then(|| (self.x, self.y)) } fn generator() -> Self { From ba5ed7a2497ab425bc1030848900f73bc6d96263 Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Tue, 7 Feb 2023 15:38:23 +0100 Subject: [PATCH 2/2] chore: add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08d1b511e..609f94157 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Breaking changes +- [\#593](https://github.com/arkworks-rs/algebra/pull/593) (`ark-ec`) Change `AffineRepr::xy()` to return owned values. + ### Features ### Improvements