-
Notifications
You must be signed in to change notification settings - Fork 251
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
pairing: Refactor traits to reflect bls12_381 crate API #238
Conversation
These associated types were completly unused. The only place we need information about the base field of an elliptic curve is inside Jubjub when operating over its coordinates to implement EC math inside the circuit, and we can handle that either concretely, or with a future trait specifically for that use-case.
These are unused now that the Base associated types have been removed from the group traits.
This enables MultiMillerLoop to be conditionally implemented, for example in libraries where Engine::pairing supports no-std, but MultiMillerLoop requires an allocator.
pairing/src/lib.rs
Outdated
@@ -23,14 +23,13 @@ pub mod bls12_381; | |||
use core::ops::Mul; | |||
use ff::{Field, PrimeField, ScalarEngine}; |
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.
The lint here is expected; it will be resolved when both this PR and #237 have been merged.
Codecov Report
@@ Coverage Diff @@
## master #238 +/- ##
===========================================
- Coverage 65.43% 35.31% -30.13%
===========================================
Files 105 93 -12
Lines 14936 11246 -3690
===========================================
- Hits 9774 3971 -5803
- Misses 5162 7275 +2113
Continue to review full report at Codecov.
|
/// function. | ||
/// | ||
/// `MillerLoopResult`s cannot be compared with each other until | ||
/// [`MillerLoopResult::final_exponentiation`] is called, which is also expensive. |
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.
This comment suggests that calling final_exponentiation
on each of two MillerLoopResult
s is the preferred way to compare them. Even if you can't rewrite to a single multi_miller_loop
, it's still more efficient to conjugate one of the loop results, multiply by the other, apply a single final exponentiation, and compare to the identity in Gt
. So it might be worth exposing a std::cmp::Eq
implemention that does 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.
That would make equality-checking very expensive, which would be very non-obvious in a == b
usage.
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.
I think that's fine as long as it's documented. But it doesn't block this PR, anyway.
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.
utACK
Prepared elements are only used by MultiMillerLoop, and we don't need the ability to "prepare" G1 elements there.
No description provided.