bls12381: Add map_to_g2 implementation #35

wwared · 2024-02-20T07:47:55Z

This PR adds a map_to_g2 function following RFC 9380, with implementation heavily based on circom-pairing's bls12_381_hash_to_G2.

The following pieces are included:

assert_subgroup_check() for both G1 and G2
clear_cofactor() for G2
assert_is_on_curve() for both G1 and G2
phi() and psi()/psi2() endomorphisms for G1 and G2 respectively
scalar_mul_by_seed_square() for G1 subgroup check
opt_simple_swu2() and iso3_map() functions
sgn0() function added to bellpepper-emulated
map_to_g2() function for turning a pair of Fp2 elements into a G2 point

Any constants added were either taken directly from the RFC or from circom-pairing's source code.

This PR requires this additional commit to the bls12_381 fork exposing a few extra private crate members, mainly to support the new tests. The feature experimental is necessary to include the hash_to_g2 module. This PR also adds short docstrings for most useful g1 and g2 functions and some minor refactors.

Additional improvements and cleanup included in the PR:

The default limb count/bit size has been set to 7/55 instead. This value seems to give the lowest amount of constraints for the emulated field operations when the base field is Bn256, which is what we actually care about. The performance in pasta's Fp isn't much worse so it seems like better defaults to use. (For the record: gnark uses 6/64, circom-pairing uses 7/55)
The tests now run on Bn256 instead of pasta
Unnecessary allocations and reductions were cleaned up and reorganized
Addition of eager modular reductions in Fp12Element::square() that resulted in the majority of performance gains

The current constraint count for one pairing is now 7.1M constraints (down from >20M), and two pairings take 17M constraints. A map_to_g2() call takes 1.4M constraints. 🎉 (However, the pairing tests remain commented out because they take over 60 seconds to run, and the multi_pairing test requires too much RAM, so CI fails trying to run them)

Future work in upcoming PRs:

Implement the hash_to_curve section of RFC 9380
A better add_or_double implementation based on gnark's AddUnified
Properly document and add workarounds for when a point at infinity is passed to the important top-level functions. Currently, most functions (pairing(), map_to_g2()) will simply fail due to a division by zero, but these edge cases should be properly documented and tested (and fixed if they should not fail)

huitseeker

Thanks a lot @wwared, this looks amazing, notably because of the thorough testing.

huitseeker · 2024-02-22T16:38:41Z

crates/bls12381/src/curves/g1.rs

+        let mut rng = rand::thread_rng();
+        let n = Scalar::random(&mut rng);
+        let a = G1Affine::from(G1Projective::generator() * n);


This is not meant as criticism, but your testing life would I feel be a lot easier with proptest, specifically with its closure-style invocation:
https://docs.rs/proptest/latest/proptest/macro.proptest.html#closure-style-invocation

I'm in favor of adding instances of Arbitrary for core types wherever useful, and for foreign types, you can look at how Arecibo does it here:
https://github.com/lurk-lab/arecibo/blob/74792f9324e317020a11d265d7379aca7ca99f66/src/r1cs/util.rs

Yeah, I think you're very right. I haven't gotten around to integrating proptest yet but I think implementing Arbitrary for the EC types would've made my life easier, I'll probably do it before the hash_to_g2 PR is finished

crates/bls12381/src/fields/fp12.rs

* replace psi with correct version, add psi2 * clear_cofactor and subgroup_check for G2 working * initial G1 subgroup check skeleton

One of the problems seems to be that constant elements (e.g. created with `from_dec`) do not seem to work correctly with some operations. Specifically, the `.neg()` calls for `xi` and `a` were just not doing anything. This needs more investigation.

* Add assert_is_on_curve for both G1 and G2 * Fix G1 phi endomorphism, subgroup_check, add tests * Update Cargo.toml to enable "experimental" feature for exposing hash_to_g2 stuff in bls12_381 * Fix README link * Remove handful of unnecessary constant allocations * Make sgn0 work for constants and add handful more test cases * Remove debug and commented out code chunks * Remove debug panic!s

Also undo the pub(crate) changes

This requires a small change in pairing.rs to prevent overflow Also change the default limb counts to 55/7, which gives a significant reduction in constraints for BN256

The TODOs aren't as relevant because the current choice already uses fewer contraints than the alternative

huitseeker · 2024-02-26T14:27:25Z

@wwared I have pushed a rebased version of this branch to https://github.com/lurk-lab/bellpepper-gadgets/tree/blshashtog2_rebased, which I believe is just a plain vanilla rebase of this PR. I will stamp this PR the minute it's reset to that branch (or something better, in case I have made a mistake).

@wwared please also link with @samuelburnham to make sure you have the ability to merge your own PRs on this repo, you should absolutely not be blocked on somebody merging your (approved) PRs in any circumstance.

Also fix a few poorly named namespaces

wwared · 2024-02-26T14:31:24Z

@wwared I have pushed a rebased version of this branch to https://github.com/lurk-lab/bellpepper-gadgets/tree/blshashtog2_rebased, which I believe is just a plain vanilla rebase of this PR. I will stamp this PR the minute it's reset to that branch (or something better, in case I have made a mistake).

I'm finishing rebasing the branch right now, running the tests to ensure I didn't screw anything up and double checking that I didn't reintroduce anything, will force push once everything is green.

@wwared please also link with @samuelburnham to make sure you have the ability to merge your own PRs on this repo, you should absolutely not be blocked on somebody merging your PRs in any circumstance.

Will do!

huitseeker · 2024-02-26T14:45:37Z

crates/bls12381/Cargo.toml

@@ -19,8 +19,8 @@ num-bigint = { workspace = true, features = ["rand"] }
 num-integer = { workspace = true }
 num-traits = { workspace = true}
 rand = { workspace = true}
-bls12_381 = { git = "https://github.com/wrinqle/bls12_381" }
+bls12_381 = { git = "https://github.com/wrinqle/bls12_381", features = ["experimental"] }


@wwared Please open an issue to sort out hosting this on a Lurk fork.

There's already #21 tracking this fork issue

huitseeker · 2024-02-26T17:04:10Z

@wwared You're good to merge!

wwared mentioned this pull request Feb 20, 2024

WIP: bls12381 hash_to_g2 implementation #25

Closed

wwared force-pushed the blshashtog2 branch from 72cc291 to 9f1a79c Compare February 20, 2024 08:06

wwared requested review from huitseeker and porcuquine February 20, 2024 08:21

wwared force-pushed the blshashtog2 branch from d0113b6 to 495c6de Compare February 20, 2024 18:38

wwared changed the title ~~Add BLS12-381 map_to_g2 implementation~~ bls12381: Add map_to_g2 implementation Feb 20, 2024

wwared force-pushed the blshashtog2 branch 3 times, most recently from 736ad52 to a321a74 Compare February 22, 2024 01:11

huitseeker previously approved these changes Feb 22, 2024

View reviewed changes

wwared dismissed huitseeker’s stale review via feb5485 February 22, 2024 17:09

wwared requested a review from huitseeker February 22, 2024 17:09

wwared added 9 commits February 26, 2024 11:13

WIP: Add initial map_to_g2 skeleton

4350673

* replace psi with correct version, add psi2 * clear_cofactor and subgroup_check for G2 working * initial G1 subgroup check skeleton

Working iso3_map, finished osswu2 but not working

4865231

Fix iso3_map den_0/den_1 exception case

c584909

Move sgn0 to bellpepper-emulated

ddb4abb

Also undo the pub(crate) changes

Change default test field to BN256 and limb params

ae23276

This requires a small change in pairing.rs to prevent overflow Also change the default limb counts to 55/7, which gives a significant reduction in constraints for BN256

Replace FIXME with eprintln!

5b681f2

Lower reductions and remove unnecessary TODOs

d276d44

The TODOs aren't as relevant because the current choice already uses fewer contraints than the alternative

wwared added 8 commits February 26, 2024 11:27

Add docstrings for G1 and G2 functions

cd2b2ab

Cleanup opt_simple_swu2 and fix lints

49d0dcc

Comment out test_random_multi_pairing for CI

03815b4

Comment out the other pairing test as well

3939925

Attempt to run pairing tests in CI again

0e5038e

Attempt to refresh CI

dadac8d

Readd fail-fast: false

ea5ea2d

Add comment to Fp12::square

9a9a45f

Also fix a few poorly named namespaces

Update expected test results after rebase

1badd2d

wwared force-pushed the blshashtog2 branch from feb5485 to 1badd2d Compare February 26, 2024 14:37

huitseeker reviewed Feb 26, 2024

View reviewed changes

huitseeker approved these changes Feb 26, 2024

View reviewed changes

wwared merged commit ae5a150 into main Feb 26, 2024
13 checks passed

wwared deleted the blshashtog2 branch February 26, 2024 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bls12381: Add map_to_g2 implementation #35

bls12381: Add map_to_g2 implementation #35

wwared commented Feb 20, 2024 •

edited

Loading

huitseeker left a comment •

edited

Loading

huitseeker Feb 22, 2024

wwared Feb 22, 2024

huitseeker commented Feb 26, 2024 •

edited

Loading

wwared commented Feb 26, 2024

huitseeker Feb 26, 2024

wwared Feb 26, 2024

huitseeker commented Feb 26, 2024

bls12381: Add map_to_g2 implementation #35

bls12381: Add map_to_g2 implementation #35

Conversation

wwared commented Feb 20, 2024 • edited Loading

huitseeker left a comment • edited Loading

Choose a reason for hiding this comment

huitseeker Feb 22, 2024

Choose a reason for hiding this comment

wwared Feb 22, 2024

Choose a reason for hiding this comment

huitseeker commented Feb 26, 2024 • edited Loading

wwared commented Feb 26, 2024

huitseeker Feb 26, 2024

Choose a reason for hiding this comment

wwared Feb 26, 2024

Choose a reason for hiding this comment

huitseeker commented Feb 26, 2024

wwared commented Feb 20, 2024 •

edited

Loading

huitseeker left a comment •

edited

Loading

huitseeker commented Feb 26, 2024 •

edited

Loading