-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimized subgroup checks #580
Changes from all commits
934b433
e98ad66
d7505a9
76659ba
17edd37
c3374b0
7904e42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,26 +1,17 @@ | ||||||
use super::field_extension::BLS12381PrimeField; | ||||||
use super::curve::{BLS12381Curve, BLS12381FieldElement}; | ||||||
use crate::cyclic_group::IsGroup; | ||||||
use crate::elliptic_curve::short_weierstrass::point::ShortWeierstrassProjectivePoint; | ||||||
use crate::elliptic_curve::traits::FromAffine; | ||||||
use crate::field::element::FieldElement; | ||||||
use crate::unsigned_integer::element::U256; | ||||||
use crate::{ | ||||||
elliptic_curve::short_weierstrass::curves::bls12_381::curve::BLS12381Curve, | ||||||
errors::ByteConversionError, traits::ByteConversion, | ||||||
}; | ||||||
use crate::{errors::ByteConversionError, traits::ByteConversion}; | ||||||
use std::cmp::Ordering; | ||||||
use std::ops::Neg; | ||||||
|
||||||
pub type G1Point = ShortWeierstrassProjectivePoint<BLS12381Curve>; | ||||||
pub type BLS12381FieldElement = FieldElement<BLS12381PrimeField>; | ||||||
#[allow(dead_code)] // This is done to not let clippy warn. Must be removed when MODULUS is used. | ||||||
const MODULUS: U256 = | ||||||
U256::from_hex_unchecked("73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001"); | ||||||
|
||||||
pub fn check_point_is_in_subgroup(point: &G1Point) -> bool { | ||||||
let inf = G1Point::neutral_element(); | ||||||
let aux_point = point.operate_with_self(MODULUS); | ||||||
inf == aux_point | ||||||
} | ||||||
pub type G1Point = ShortWeierstrassProjectivePoint<BLS12381Curve>; | ||||||
|
||||||
pub fn decompress_g1_point(input_bytes: &mut [u8; 48]) -> Result<G1Point, ByteConversionError> { | ||||||
let first_byte = input_bytes.first().unwrap(); | ||||||
|
@@ -66,7 +57,8 @@ pub fn decompress_g1_point(input_bytes: &mut [u8; 48]) -> Result<G1Point, ByteCo | |||||
let point = | ||||||
G1Point::from_affine(x, y.clone()).map_err(|_| ByteConversionError::InvalidValue)?; | ||||||
|
||||||
check_point_is_in_subgroup(&point) | ||||||
point | ||||||
.is_in_subgroup() | ||||||
.then_some(point) | ||||||
.ok_or(ByteConversionError::PointNotInSubgroup) | ||||||
} | ||||||
|
@@ -112,13 +104,13 @@ mod tests { | |||||
fn test_zero_point() { | ||||||
let g1 = BLS12381Curve::generator(); | ||||||
|
||||||
assert!(super::check_point_is_in_subgroup(&g1)); | ||||||
assert!(g1.is_in_subgroup()); | ||||||
let new_x = BLS12381FieldElement::zero(); | ||||||
let new_y = BLS12381FieldElement::one() + BLS12381FieldElement::one(); | ||||||
|
||||||
let false_point2 = G1Point::from_affine(new_x, new_y).unwrap(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please rename the variable. There's no such thing as a false point. Add please a test that asserts that the point is not in the subgroup by other means. For example asserting that it's order does not divide the order of the subgroup being checked.
Suggested change
|
||||||
|
||||||
assert!(!super::check_point_is_in_subgroup(&false_point2)); | ||||||
assert!(!false_point2.is_in_subgroup()); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,13 +1,14 @@ | ||||||
use super::curve::SEED; | ||||||
use super::field_extension::{Degree12ExtensionField, Degree2ExtensionField}; | ||||||
use crate::cyclic_group::IsGroup; | ||||||
use crate::elliptic_curve::short_weierstrass::point::ShortWeierstrassProjectivePoint; | ||||||
use crate::elliptic_curve::traits::IsEllipticCurve; | ||||||
use crate::field::traits::IsField; | ||||||
use crate::unsigned_integer::element::U384; | ||||||
use crate::{ | ||||||
elliptic_curve::short_weierstrass::traits::IsShortWeierstrass, field::element::FieldElement, | ||||||
}; | ||||||
|
||||||
use super::field_extension::{Degree12ExtensionField, Degree2ExtensionField}; | ||||||
|
||||||
const GENERATOR_X_0: U384 = U384::from_hex_unchecked("024aa2b2f08f0a91260805272dc51051c6e47ad4fa403b02b4510b647ae3d1770bac0326a805bbefd48056c8c121bdb8"); | ||||||
const GENERATOR_X_1: U384 = U384::from_hex_unchecked("13e02b6052719f607dacd3a088274f65596bd0d09920b61ab5da61bbdc7f5049334cf11213945d57e5ac7d055d042b7e"); | ||||||
const GENERATOR_Y_0: U384 = U384::from_hex_unchecked("0ce5d527727d6e118cc9cdc6da2e351aadfd9baa8cbdd3a76d429a695160d12c923ac9cc3baca289e193548608b82801"); | ||||||
|
@@ -46,7 +47,57 @@ impl IsShortWeierstrass for BLS12381TwistCurve { | |||||
} | ||||||
} | ||||||
|
||||||
impl BLS12381TwistCurve { | ||||||
pub fn is_on_curve(point: &<Self as IsEllipticCurve>::PointRepresentation) -> bool { | ||||||
if point.z() == &FieldElement::zero() { | ||||||
true | ||||||
} else { | ||||||
let point = point.to_affine(); | ||||||
Self::defining_equation(&point.x(), &point.y()) == FieldElement::zero() | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
impl ShortWeierstrassProjectivePoint<BLS12381TwistCurve> { | ||||||
/// Returns the "Untwist-Frobenius-Twist" endomorphism | ||||||
pub fn psi(&self) -> Self { | ||||||
Comment on lines
61
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a test that checks that this endomorphism has minimal polynomial |
||||||
// Coefficients borrowed from https://github.com/Consensys/gnark-crypto/blob/55489ac07ca1a88bf0b830a29625fcb0d8879a48/ecc/bls12-381/bls12-381.go#L132C1-L135C138 | ||||||
let psi_coeff_x = FieldElement::<<BLS12381TwistCurve as IsEllipticCurve>::BaseField>::new([ | ||||||
FieldElement::zero(), | ||||||
FieldElement::new(U384::from_hex_unchecked("1A0111EA397FE699EC02408663D4DE85AA0D857D89759AD4897D29650FB85F9B409427EB4F49FFFD8BFD00000000AAAD")) | ||||||
]); | ||||||
|
||||||
let psi_coeff_y = FieldElement::<<BLS12381TwistCurve as IsEllipticCurve>::BaseField>::new([ | ||||||
FieldElement::new(U384::from_hex_unchecked("135203E60180A68EE2E9C448D77A2CD91C3DEDD930B1CF60EF396489F61EB45E304466CF3E67FA0AF1EE7B04121BDEA2")), | ||||||
FieldElement::new(U384::from_hex_unchecked("6AF0E0437FF400B6831E36D6BD17FFE48395DABC2D3435E77F76E17009241C5EE67992F72EC05F4C81084FBEDE3CC09")) | ||||||
]); | ||||||
|
||||||
let frob_map_x = Degree2ExtensionField::frobenius_map(self.x().value()); | ||||||
let frob_map_x_with_psi_coeff_x = | ||||||
Degree2ExtensionField::mul(&frob_map_x, psi_coeff_x.value()); | ||||||
|
||||||
let frob_map_y = Degree2ExtensionField::frobenius_map(self.y().value()); | ||||||
let frob_map_y_with_psi_coeff_y = | ||||||
Degree2ExtensionField::mul(&frob_map_y, psi_coeff_y.value()); | ||||||
|
||||||
let frob_map_z = Degree2ExtensionField::frobenius_map(self.z().value()); | ||||||
|
||||||
<BLS12381TwistCurve as IsEllipticCurve>::PointRepresentation::new([ | ||||||
FieldElement::new(frob_map_x_with_psi_coeff_x), | ||||||
FieldElement::new(frob_map_y_with_psi_coeff_y), | ||||||
FieldElement::new(frob_map_z), | ||||||
]) | ||||||
} | ||||||
|
||||||
/// Returns true if the point is in the prime subgroup `G_2` of order `r`. | ||||||
/// Makes use of endomorphism for efficient point multiplication. | ||||||
pub fn is_in_subgroup(&self) -> bool { | ||||||
let seed_times_p = self.operate_with_self(SEED); | ||||||
let psi_plus_seed_times_p = self.psi().operate_with(&seed_times_p); | ||||||
|
||||||
BLS12381TwistCurve::is_on_curve(&psi_plus_seed_times_p) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not checking that
Suggested change
|
||||||
} | ||||||
|
||||||
/// This function is related to the map ψ: E_twist(𝔽p²) -> E(𝔽p¹²). | ||||||
/// Given an affine point G in E_twist(𝔽p²) returns x, y such that | ||||||
/// ψ(G) = (x', y', 1) with x' = x * x'' and y' = y * y'' | ||||||
|
@@ -80,6 +131,7 @@ impl ShortWeierstrassProjectivePoint<BLS12381TwistCurve> { | |||||
|
||||||
#[cfg(test)] | ||||||
mod tests { | ||||||
use super::BLS12381TwistCurve; | ||||||
use crate::{ | ||||||
cyclic_group::IsGroup, | ||||||
elliptic_curve::{ | ||||||
|
@@ -94,7 +146,6 @@ mod tests { | |||||
unsigned_integer::element::U384, | ||||||
}; | ||||||
|
||||||
use super::BLS12381TwistCurve; | ||||||
type Level0FE = FieldElement<BLS12381PrimeField>; | ||||||
type Level1FE = FieldElement<Degree2ExtensionField>; | ||||||
|
||||||
|
@@ -154,4 +205,16 @@ mod tests { | |||||
let expected = BLS12381TwistCurve::create_point_from_affine(expectedx, expectedy).unwrap(); | ||||||
assert_eq!(p.operate_with(&q), expected); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn check_generator_g2_in_subgroup() { | ||||||
let gen = BLS12381TwistCurve::generator(); | ||||||
assert!(gen.is_in_subgroup()); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn check_arbitrary_g2_point_in_subgroup() { | ||||||
let arb_point = BLS12381TwistCurve::generator().operate_with_self(420_u32); | ||||||
assert!(arb_point.is_in_subgroup()); | ||||||
} | ||||||
Comment on lines
+209
to
+219
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a test that checks that |
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename this test