Implement group and pairing traits for bls12_381 and jubjub crates #245

str4d · 2020-06-25T03:52:42Z

Closes #163. Closes #167. Closes #169.

jubjub/src/lib.rs

bls12_381/src/g1.rs

bls12_381/src/g2.rs

jubjub/src/lib.rs

codecov · 2020-06-25T04:10:09Z

Codecov Report

Merging #245 into master will decrease coverage by 1.69%.
The diff coverage is 1.62%.

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
- Coverage   66.73%   65.03%   -1.70%     
==========================================
  Files         117      117              
  Lines       16192    16623     +431     
==========================================
+ Hits        10805    10811       +6     
- Misses       5387     5812     +425

Impacted Files	Coverage Δ
bls12_381/src/fp.rs	`44.19% <0.00%> (-2.66%)`	⬇️
bls12_381/src/fp12.rs	`44.64% <0.00%> (-1.23%)`	⬇️
bls12_381/src/fp2.rs	`55.55% <0.00%> (-1.12%)`	⬇️
bls12_381/src/fp6.rs	`52.73% <0.00%> (-1.49%)`	⬇️
bls12_381/src/g1.rs	`65.15% <0.00%> (-9.94%)`	⬇️
bls12_381/src/g2.rs	`58.04% <0.00%> (-8.09%)`	⬇️
group/src/lib.rs	`50.00% <ø> (ø)`
jubjub/src/lib.rs	`41.95% <2.45%> (-14.26%)`	⬇️
bls12_381/src/pairings.rs	`55.01% <3.30%> (-21.73%)`	⬇️
zcash_primitives/src/serialize.rs	`64.28% <0.00%> (-1.03%)`	⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 964532e...3200ffc. Read the comment docs.

nuttycom

Non-blocking style comments/questions; I don't know how to verify the correctness of the actual group operations, but they look like what I'd expect.

bls12_381/src/fp.rs

nuttycom · 2020-07-21T17:58:11Z

bls12_381/src/g2.rs

+    }
+}
+
+impl Sum for G2Subgroup {


Having seen this implementation duplicated a few times now, could it just be done via a blanket trait with constraints on Add and Identity? Or, is there a Monoid trait that combines the two? In Haskell we have this in the standard library:

Prelude> :t Data.Foldable.fold Data.Foldable.fold :: (Foldable t, Monoid m) => t m -> m

bls12_381/src/g1.rs

bls12_381/src/g2.rs

nuttycom · 2020-07-22T18:35:27Z

jubjub/src/lib.rs

+    }
+}
+
+impl<'r> Sum<&'r ExtendedPoint> for ExtendedPoint {


If we can't use a blanket trait for these, we should at least make a macro for them.

There's already a set of common macros that are planned to be extracted out; we can do this at the same time.

jubjub/src/lib.rs

bls12_381/src/g1.rs

bls12_381/src/g2.rs

bls12_381/src/g1.rs

bls12_381/src/pairings.rs

jubjub/src/lib.rs

str4d · 2020-08-12T23:16:27Z

Okay, I think that (aside from specifying the full-group generators for G1 and G2) this is ready for review. Best reviewed by-commit.

bls12_381/src/pairings.rs

daira

Need to check a few things, otherwise utACK.

bls12_381/src/fp.rs

bls12_381/src/g1.rs

jubjub/src/lib.rs

daira · 2020-08-17T17:12:34Z

jubjub/src/lib.rs

+                0x0000_0000_0000_0000,
+                0x0000_0000_0000_0000,
+                0x0000_0000_0000_0000,
+            ]),


@daira should check that this is the canonical point as defined that generates the whole group.

I reviewed find_curve_generator() and it looks correct to me. This doesn't check that the point arithmetic or Montgomery conversion is done correctly, but it's enough to satisfy me.

bls12_381/src/pairings.rs

bls12_381/src/fp.rs

str4d · 2020-08-17T22:44:50Z

Force-pushed to address some of @daira's comments. Per my responses above, I think they are all addressed now.

…ment

daira

utACK

str4d requested a review from ebfull June 25, 2020 03:52

str4d added this to the Core Sprint 2020-25 milestone Jun 25, 2020

str4d force-pushed the impl-group-pairing-traits branch from dc2acd8 to 2e062a7 Compare June 25, 2020 03:53

str4d commented Jun 25, 2020

View reviewed changes

jubjub/src/lib.rs Outdated Show resolved Hide resolved

str4d commented Jun 25, 2020

View reviewed changes

bls12_381/src/g1.rs Outdated Show resolved Hide resolved

str4d commented Jun 25, 2020

View reviewed changes

bls12_381/src/g2.rs Outdated Show resolved Hide resolved

str4d commented Jun 25, 2020

View reviewed changes

jubjub/src/lib.rs Outdated Show resolved Hide resolved

ebfull mentioned this pull request Jun 26, 2020

Sapling Implementation Refactor #41

Closed

18 tasks

str4d force-pushed the impl-group-pairing-traits branch 2 times, most recently from 79d95bf to 1c4faf9 Compare July 2, 2020 02:32

r3ld3v modified the milestones: Core Sprint 2020-25, Core Sprint 2020-29 Jul 4, 2020

nuttycom self-requested a review July 20, 2020 21:56

nuttycom reviewed Jul 22, 2020

View reviewed changes

r3ld3v modified the milestones: Core Sprint 2020-29, Core Sprint 2020-33 Jul 30, 2020

ebfull requested changes Aug 11, 2020

View reviewed changes

str4d mentioned this pull request Aug 12, 2020

Updates to group traits #266

Merged

str4d force-pushed the impl-group-pairing-traits branch from 1c4faf9 to f39f8d1 Compare August 12, 2020 23:11

str4d commented Aug 13, 2020

View reviewed changes

bls12_381/src/pairings.rs Show resolved Hide resolved

str4d force-pushed the impl-group-pairing-traits branch from f39f8d1 to b1e6f8a Compare August 13, 2020 18:42

daira reviewed Aug 17, 2020

View reviewed changes

bls12_381/src/fp.rs Show resolved Hide resolved

daira reviewed Aug 17, 2020

View reviewed changes

bls12_381/src/fp.rs Show resolved Hide resolved

str4d added 4 commits August 17, 2020 23:40

bls12_381: Implement group traits

d11b600

jubjub: Implement group traits

eae5df0

bls12_381: Implement pairing traits

4b4a4ee

group: Exclude identity element from output of Group::random

3a72f08

str4d force-pushed the impl-group-pairing-traits branch from b1e6f8a to 3a72f08 Compare August 17, 2020 22:41

group: GroupEncoding::Repr: Default is not required to be a valid ele…

3200ffc

…ment

This was referenced Aug 18, 2020

Review or recalculate w-NAF recommendations zkcrypto/bls12_381#42

Open

Recalculate w-NAF recommendations zkcrypto/jubjub#36

Open

ebfull approved these changes Aug 18, 2020

View reviewed changes

daira approved these changes Aug 19, 2020

View reviewed changes

str4d merged commit 544d593 into zcash:master Aug 19, 2020

str4d deleted the impl-group-pairing-traits branch August 19, 2020 22:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement group and pairing traits for bls12_381 and jubjub crates #245

Implement group and pairing traits for bls12_381 and jubjub crates #245

str4d commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading

nuttycom left a comment

nuttycom Jul 21, 2020

nuttycom Jul 22, 2020

str4d Jul 23, 2020

str4d commented Aug 12, 2020

daira left a comment

daira Aug 17, 2020

daira Aug 19, 2020

str4d commented Aug 17, 2020

daira left a comment

Implement group and pairing traits for bls12_381 and jubjub crates #245

Implement group and pairing traits for bls12_381 and jubjub crates #245

Conversation

str4d commented Jun 25, 2020 • edited Loading

codecov bot commented Jun 25, 2020 • edited Loading

Codecov Report

nuttycom left a comment

Choose a reason for hiding this comment

nuttycom Jul 21, 2020

Choose a reason for hiding this comment

nuttycom Jul 22, 2020

Choose a reason for hiding this comment

str4d Jul 23, 2020

Choose a reason for hiding this comment

str4d commented Aug 12, 2020

daira left a comment

Choose a reason for hiding this comment

daira Aug 17, 2020

Choose a reason for hiding this comment

daira Aug 19, 2020

Choose a reason for hiding this comment

str4d commented Aug 17, 2020

daira left a comment

Choose a reason for hiding this comment

str4d commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading