-
Notifications
You must be signed in to change notification settings - Fork 13
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
bellpepper-bls12381 implementation #15
Conversation
Includes the field-specific functions for e2, e6 and e12. Uses a patched bls12_381 library to expose the fp2, fp6, fp12 datatypes.
Not all torus functions have corresponding tests (yet) since they all would also require a native counterpart function to check against. Reorganized the modules a bit, created empty files for g1/g2/pairing files.
All the places where constant strings are used just call .unwrap() directly
* When a large function creates a new NS, denote it with :: and the explicit arguments * Removed unused bigint_to_e2elem function * Additional E2/E6/E12 -> Fp2/Fp6/Fp12 renamings
I didn't change the `alloc_num_equals_constant` docstring since that is using `Num`s directly and not `EmulatedFieldElement`s
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 looks overall great, for a first draft! I intend to fuzz this quite a bit later.
Re: the fork of the bls12-381 crate, I'm ok with it for now, but we definitely want to remove this dependency in the near future.
- to override the cargo-deny checks, you can edit the last few lines of the deny.toml, (I'll approve when this is done)
- we should open an issue to fix this unsightly git dependency (irrespective of how)
- instead of forking the repo, have you looked at whether the sought-after types are accessible in https://github.com/filecoin-project/blstrs ?
use pasta_curves::Fp; | ||
|
||
use expect_test::{expect, Expect}; | ||
fn expect_eq(computed: usize, expected: Expect) { |
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.
General testing nits (you can safely ignore this for this milestone):
fn expect_eq(computed: usize, expected: &Expect)
- we could probably share this function in a
proptest
would probably save you from a lot of manual fiddling with the RNG, and would make your methods for randomly generating a composite input .. well, composable :)
Probably issues for a next iteration
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.
Agreed on all counts, we can improve this later, I just wanted to get expect-test
in this initial PR because every test has the constraint count and there's quite a few of those. I think using proptest
would be a neat followup
The arguments in `conditionally_select` were inverted due to the fact the order is inverted compared to gnark's code. I've added a test that performs 2 pairings. Due to the large circuit size, it takes a long time to complete for >2 pairings, but it seems to be working with 3 pairings too. The multiple pairings test is also commented out for perf reasons. Also added a `reduce` call before a `conditionally_select` to ensure that the limb counts between the `dummy` variable and `e.c1` are the same.
Tracking issue: #21
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.
Fantastic, thank you so much @wwared !
Done in a03fdc7
Opened #21
I looked at it briefly before but not too deeply, IIRC it doesn't export things like EDIT: To be clear, |
This PR adds a$E_{12}$ .
bellpepper-bls12381
crate that includes gadgets for a circuit performing pairings and other basic BLS12-381 operations. The implementation is heavily inspired (i.e. copied) fromgnark
's std/algebra/emulated/ package. This makes use of torus transformations for more efficient exponentiation inCurrently, there are still (non-trivial) low-hanging optimizations in the base operations used by the pairing (implemented by
bellpepper-emulated
). Currently, one pairing takes roughly 28M constraints, but the majority of the constraints are for bit reductions and right shifts, so it should (hopefully) be possible to bring this number down to <10M constraints (or lower) if we improve the reduction strategy used bybellpepper-emulated
.1This crate uses a minimal fork of
bls12_381
that exposes some private elements of the crate. CI is rejecting the PR because of the git dependency to a non-lurk-lab repository, so the fork could be transferred to the lurk-lab org.Because of the high constraint count, I've commented out two tests that take a long time to run.
Future improvements that would come in future PRs:
reduce
flow onbellpepper-emulated
operations.is_on_curve
for G1/G2).pub
elements (right now basically everything ispub
for convenience).Footnotes
Currently,
bellpepper-emulated
's implementation ofreduce
ends up callingassert_is_equal
which does a bit decomposition of the values in theassert_limb_equality_slow
path, and it ends up exploding the number of constraints.gnark
instead commits to some polynomials and does a more optimized multiplication hint (which is not feasible in pure R1CS).circom-pairing
has some different strategies for performing multiplications and prime reductions, which may be better than theassert_is_equal
slow path as well (here and here). The strategy used bybp-ed25519
should be similar tocircom-pairing
's and is also a possible inspiration to draw from. ↩