Use Scott's subgroup membership tests for `G1` and `G2` of BLS12-381. #74

simonmasson · 2021-09-08T13:50:14Z

Description

Implementation of the fast subgroup check for G1 and G2 using the Scott algorithm from eprint 2021/1130.

The Scott's implementation (eprint 2021/1130) is slightly faster than Bowe's implementation:

Author	G1 subgroup check	G2 subgroup check
Bowe	50.435ns	68.635ns
Scott	42.917ns	60.793ns

closes: #XXXX

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (master)
Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
Wrote unit tests
Updated relevant documentation in the code
Added a relevant changelog entry to the Pending section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

Pratyush · 2021-09-08T17:50:42Z

bls12_381/src/curves/mod.rs


-pub struct Parameters;
+pub struct Parameters0;


Hmm why this rename?

Because Parameters is also used for the SWModelParameters struct... See below in this file

type G1Parameters = self::g1::Parameters

maybe instead of importing crate::*, we can use crate::Parameters wherever necessary? That way we don't have to rename anything.

Pratyush · 2021-09-08T18:00:39Z

bls12_381/src/curves/g1.rs

@@ -51,3 +80,11 @@ pub const G1_GENERATOR_X: Fq = field_new!(Fq, "368541675371338701678108831518307
 /// 1339506544944476473020471379941921221584933875938349620426543736416511423956333506472724655353366534992391756441569
 #[rustfmt::skip]
 pub const G1_GENERATOR_Y: Fq = field_new!(Fq, "1339506544944476473020471379941921221584933875938349620426543736416511423956333506472724655353366534992391756441569");
+
+pub const BETA: Fq = field_new!(Fq,"793479390729215512621379701633421447060886740281060493010456487427281649075476305620758731620350");


Do you have a script to calculate this value? Or maybe a comment that can let us check it?

Pratyush · 2021-09-08T18:00:51Z

bls12_381/src/curves/g1.rs

+
+pub const BETA: Fq = field_new!(Fq,"793479390729215512621379701633421447060886740281060493010456487427281649075476305620758731620350");
+
+pub fn sigma(p: &GroupAffine<Parameters>) -> GroupAffine<Parameters> {


Some documentation for this would be great =)

Pratyush · 2021-09-08T18:01:54Z

bls12_381/src/curves/g1.rs

+            if Parameters0::X_IS_NEGATIVE {
+                x_times_p = -x_times_p;
+            }
+            if (-*p + x_times_p).is_zero() {


Suggested change

if (-*p + x_times_p).is_zero() {

if (x_times_p - p).is_zero() {

this is not working, and neither x_times_p - *p. The only way I found is to write x_times_p + (-*p)... Any suggestion?

It was because x_times_p was GroupAffine. The following code should work:

Suggested change

if (-*p + x_times_p).is_zero() {

if x_times_p.add_mixed(-*p).is_zero() {

Pratyush · 2021-09-08T18:04:30Z

bls12_381/src/curves/g2.rs

+    fn is_in_correct_subgroup_assuming_on_curve(point: &GroupAffine<Parameters>) -> bool {
+        // check that [p]P = [X]P
+        let mut x_times_point: GroupAffine<_> =
+            point.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();


Hmm if you'd like, we can add a mul_limbs (or some other name) method similar to this one on GroupProjective: https://docs.rs/ark-ec/0.3.0/ark_ec/trait.ProjectiveCurve.html#method.mul. That way you can directly use point.mul_limbs(Parameters::X).

Alternatively, you can also just use the existing point.mul_bits(BitIteratorBE::without_leading_zeros(Parameters::X))

the mul_bits function is private and so I was not able to use it...

Pratyush · 2021-09-08T18:19:41Z

bls12_381/src/curves/g1.rs

+        } else {
+            let sigma_p = sigma(p);
+            let mut x_times_p: GroupAffine<_> =
+                p.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();


Suggested change

p.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();

p.mul_bits(BitIteratorBE::without_leading_zeros(Parameters::X).into();

the mul_bits function is private and so I was not able to use it...

It would be much cleaner like that, I agree :)

Hmm I'll make a PR to expose that API, but for the time being we can use the existing code. If you can leave a TODO then i'll refactor to use that later.

Pratyush · 2021-09-09T23:25:47Z

bls12_381/src/curves/g1.rs

+            let mut x_times_p: GroupAffine<_> =
+                p.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();


Let's avoid the inversion here:

Suggested change

let mut x_times_p: GroupAffine<_> =

p.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();

let mut x_times_p = p.mul(BigInteger([Parameters0::X[0], 0, 0, 0]));

Pratyush · 2021-09-09T23:27:40Z

bls12_381/src/curves/g1.rs

+                let mut x2_times_p: GroupAffine<_> = x_times_p
+                    .mul(BigInteger([Parameters0::X[0], 0, 0, 0]))
+                    .into();


Suggested change

let mut x2_times_p: GroupAffine<_> = x_times_p

.mul(BigInteger([Parameters0::X[0], 0, 0, 0]))

.into();

let mut x2_times_p = x_times_p.mul(BigInteger([Parameters0::X[0], 0, 0, 0]));

Pratyush · 2021-09-09T23:29:47Z

bls12_381/src/curves/g1.rs

+                if Parameters0::X_IS_NEGATIVE {
+                    x2_times_p = -x2_times_p;
+                }
+                (x2_times_p + sigma_p).is_zero()


Suggested change

(x2_times_p + sigma_p).is_zero()

x2_times_p.add_mixed(sigma_p).is_zero()

Pratyush · 2021-09-09T23:30:18Z

bls12_381/src/curves/g2.rs

+        let mut x_times_point: GroupAffine<_> =
+            point.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();


Suggested change

let mut x_times_point: GroupAffine<_> =

point.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();

let mut x_times_point =

point.mul(BigInteger([Parameters0::X[0], 0, 0, 0]));

Pratyush · 2021-09-09T23:31:21Z

bls12_381/src/curves/g2.rs

+// psi(x,y) = (x^p * PSI_X, y^p * PSI_Y) is the Frobenius composed
+// with the quadratic twist and its inverse
+pub const PSI_X:Fq2 = field_new!(
+    Fq2,
+    FQ_ZERO,
+    field_new!(
+       Fq,
+       "4002409555221667392624310435006688643935503118305586438271171395842971157480381377015405980053539358417135540939437"
+    )
+);
+pub const PSI_Y: Fq2 = field_new!(
+    Fq2,
+    field_new!(
+       Fq,
+       "2973677408986561043442465346520108879172042883009249989176415018091420807192182638567116318576472649347015917690530"),
+    field_new!(
+       Fq,
+       "1028732146235106349975324479215795277384839936929757896155643118032610843298655225875571310552543014690878354869257")
+);


Some documentation on how these constants are derived would be great.

Pratyush · 2021-09-09T23:36:13Z

bls12_381/src/curves/g2.rs

+       Fq,
+       "1028732146235106349975324479215795277384839936929757896155643118032610843298655225875571310552543014690878354869257")
+);
+pub fn psi(p: &GroupAffine<Parameters>) -> GroupAffine<Parameters> {


Ditto here.

Pratyush · 2021-09-09T23:40:37Z

bls12_381/src/curves/g2.rs

+        if Parameters0::X_IS_NEGATIVE {
+            x_times_point = -x_times_point;
+        }
+        let p_times_point = psi(point);
+        (-x_times_point + p_times_point).is_zero()


Suggested change

if Parameters0::X_IS_NEGATIVE {

x_times_point = -x_times_point;

}

let p_times_point = psi(point);

(-x_times_point + p_times_point).is_zero()

// The check is `p_times_point - x_times_point == 0`?

// If `x` is negative, then the LHS becomes `p_times_point + x_times_point`.

// If `x` is positive, then it remains `p_times_point - x_times_point`.

// So, we negate if `x` is positive, and add the result.

if !Parameters0::X_IS_NEGATIVE {

x_times_point = -x_times_point;

}

let p_times_point = psi(point);

x_times_point.add_mixed(p_times_point).is_zero()

Pratyush · 2021-09-09T23:42:30Z

Left another round of reviews, focusing on adding some more documentation (eg: links to descriptions of the algorithms in papers or blogposts etc) and on not doing unnecessary affine operations. Thanks for your patience @simonmasson!

simonmasson · 2021-09-10T10:13:23Z

Thank you for all the comments/suggestions! I add some changes, let me know if something else is needed.

daira · 2021-09-11T09:13:01Z

bls12_381/src/curves/g2.rs

+pub fn psi(p: &GroupAffine<Parameters>) -> GroupAffine<Parameters> {
+    // Psi is an endomorphism of the curve defined with the sextic
+    // twists and the frobenius endomorphism on the G1 curve
+    // (x,y) -> (x^p / (u+1)**((p-1)//3), y^p / (u+1)**((p-1)//2))


Is there an intended distinction between ^ and ** here?

No. I will change ^ into ** here :-)

weikengchen · 2021-09-18T08:15:45Z

I will need to double check the paper.

Based on the paper, it seems that the following is not expected:

BETA^2 + BETA + 1 = 0

Rather, it is expected:

lambda^2 + lambda + 1 = 0

where for BLS-12, lambda = -u^2.

And, if beta is the cubic root of unity, then:

BETA^3 = 1

weikengchen · 2021-09-18T08:37:24Z

Let me correct my previous post.

For the beta we are finding,

BETA^3  = 1 (mod Fq)
BETA^2 + BETA + 1 = 0 (mod Fq)

Both are satisfied.

Because x^3 - 1 = (x - 1)(x^2 + x + 1).

weikengchen · 2021-09-19T21:40:00Z

I added some documents and renamed some method names. Let me ping @Pratyush to do a final pass.

weikengchen · 2021-09-24T20:49:42Z

There is some offline discussion with Simon. I will update this PR soon to reflect the discussion.

yelhousni · 2022-03-23T10:40:41Z

So it seems there was an error in Scott preprint for G2 membership test proof. However, the result is still correct (more on that here: https://eprint.iacr.org/2022/352.pdf).

weikengchen · 2022-03-23T18:19:22Z

I think one todo is to add additional comment to reference to the new paper for the proof.

It is a relief that the original method still works. (not common)

simonmasson added 6 commits September 7, 2021 09:58

implementation of the fast subgroup check for bls12_381

9cb9eb1

add a bench

0ac5781

subgroup check for g1

dc1893a

subgroup check modifications

42ac45d

remove useless test

fff08cf

fmt

9c5d979

simonmasson mentioned this pull request Sep 8, 2021

Fast subgroup check for bls12_381 #73

Closed

6 tasks

need the last version of arkworks/algebra

0f0ece9

Pratyush reviewed Sep 8, 2021

View reviewed changes

Pratyush reviewed Sep 9, 2021

View reviewed changes

simonmasson added 7 commits September 10, 2021 10:58

remove Parameters0

8c8de24

using projective points is more efficient

f378718

use of projective coordinates in G2

8f25a9d

fmt

01163e0

documentation on the constants and the psi function

d077d6e

references for algorithms of eprint 2021/1130

d557005

fmt

65046fd

daira reviewed Sep 11, 2021

View reviewed changes

simonmasson and others added 5 commits September 13, 2021 10:25

sed ^ **

402b153

minor improvement

03e0bbe

fmt

39ab93f

fix Cargo toml

4847737

nits

79e7617

weikengchen added 7 commits September 18, 2021 03:31

some cleanup for g1

d41955e

add the beta test back

e88dd5d

fmt

51832bc

g2

17488f9

changelog

7b11a07

add a note on the Cargo.toml

03633f9

nits

345af9d

weikengchen requested a review from Pratyush September 19, 2021 21:39

avoid variable name conflicts

7edc467

add the early-out optimization

6c43cc6

weikengchen changed the title ~~Scott subgroup checks~~ Add the Scott subgroup checks for BLS12-381 Sep 25, 2021

weikengchen changed the title ~~Add the Scott subgroup checks for BLS12-381~~ Use Scott's subgroup membership tests for G1 and G2 of BLS12-381. Sep 25, 2021

weikengchen merged commit 2118e14 into arkworks-rs:master Sep 25, 2021

This was referenced Nov 4, 2021

[enhancement] Implement Scott's subgroup checks supranational/blst#88

Closed

Possibly speed up G1,G2 and GT group membership checks supranational/blst#79

Closed

ghost mentioned this pull request Apr 7, 2022

add fast subgroup check for is_torsion_free zkcrypto/bls12_381#83

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Scott's subgroup membership tests for `G1` and `G2` of BLS12-381. #74

Use Scott's subgroup membership tests for `G1` and `G2` of BLS12-381. #74

simonmasson commented Sep 8, 2021 •

edited by weikengchen

Loading

Pratyush Sep 8, 2021

simonmasson Sep 9, 2021

Pratyush Sep 9, 2021

Pratyush Sep 8, 2021

Pratyush Sep 8, 2021

Pratyush Sep 8, 2021 •

edited

Loading

simonmasson Sep 9, 2021

Pratyush Sep 9, 2021

Pratyush Sep 8, 2021

Pratyush Sep 8, 2021

simonmasson Sep 9, 2021

Pratyush Sep 8, 2021

simonmasson Sep 9, 2021

simonmasson Sep 9, 2021

Pratyush Sep 9, 2021

Pratyush Sep 9, 2021 •

edited

Loading

Pratyush Sep 9, 2021 •

edited

Loading

Pratyush Sep 9, 2021

Pratyush Sep 9, 2021

Pratyush Sep 9, 2021

Pratyush Sep 9, 2021

Pratyush Sep 9, 2021

Pratyush commented Sep 9, 2021

simonmasson commented Sep 10, 2021

daira Sep 11, 2021

simonmasson Sep 13, 2021

weikengchen commented Sep 18, 2021

weikengchen commented Sep 18, 2021 •

edited

Loading

weikengchen commented Sep 19, 2021

weikengchen commented Sep 24, 2021

yelhousni commented Mar 23, 2022

weikengchen commented Mar 23, 2022


		pub const BETA: Fq = field_new!(Fq,"793479390729215512621379701633421447060886740281060493010456487427281649075476305620758731620350");

		pub fn sigma(p: &GroupAffine<Parameters>) -> GroupAffine<Parameters> {

	if (-*p + x_times_p).is_zero() {
	if (x_times_p - p).is_zero() {

	if (-*p + x_times_p).is_zero() {
	if x_times_p.add_mixed(-*p).is_zero() {

	p.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();
	p.mul_bits(BitIteratorBE::without_leading_zeros(Parameters::X).into();

		let mut x_times_p: GroupAffine<_> =
		p.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();

	let mut x_times_p: GroupAffine<_> =
	p.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();
	let mut x_times_p = p.mul(BigInteger([Parameters0::X[0], 0, 0, 0]));

	(x2_times_p + sigma_p).is_zero()
	x2_times_p.add_mixed(sigma_p).is_zero()

		let mut x_times_point: GroupAffine<_> =
		point.mul(BigInteger([Parameters0::X[0], 0, 0, 0])).into();

Use Scott's subgroup membership tests for G1 and G2 of BLS12-381. #74

Use Scott's subgroup membership tests for G1 and G2 of BLS12-381. #74

Conversation

simonmasson commented Sep 8, 2021 • edited by weikengchen Loading

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pratyush Sep 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pratyush Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

Pratyush Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pratyush commented Sep 9, 2021

simonmasson commented Sep 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weikengchen commented Sep 18, 2021

weikengchen commented Sep 18, 2021 • edited Loading

weikengchen commented Sep 19, 2021

weikengchen commented Sep 24, 2021

yelhousni commented Mar 23, 2022

weikengchen commented Mar 23, 2022

Use Scott's subgroup membership tests for `G1` and `G2` of BLS12-381. #74

Use Scott's subgroup membership tests for `G1` and `G2` of BLS12-381. #74

simonmasson commented Sep 8, 2021 •

edited by weikengchen

Loading

Pratyush Sep 8, 2021 •

edited

Loading

Pratyush Sep 9, 2021 •

edited

Loading

Pratyush Sep 9, 2021 •

edited

Loading

weikengchen commented Sep 18, 2021 •

edited

Loading