Skip to content
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

feat: BN254 pairing #411

Merged
merged 5 commits into from
Mar 6, 2023
Merged

feat: BN254 pairing #411

merged 5 commits into from
Mar 6, 2023

Conversation

ivokub
Copy link
Collaborator

@ivokub ivokub commented Dec 12, 2022

This PR adds pairing on BN254. The interface which is made public is small, but in general includes G2 and GT operations.

@gbotrel
Copy link
Collaborator

gbotrel commented Dec 20, 2022

note on perf; compile time for R1CS is slowww, and mostly, due to known issues.

  1. Lots of assertboolean, that adds DebugInfo which is stored in a .. less than ideal way. Will fix.
  2. frontend.Capacity option is not implemented post-refactor PR, will fix, it will help too.
  3. The more unknown is that it seems garbage collection is very active, will have to do a pointer hunt.

@ivokub
Copy link
Collaborator Author

ivokub commented Feb 17, 2023

note on perf; compile time for R1CS is slowww, and mostly, due to known issues.

  1. Lots of assertboolean, that adds DebugInfo which is stored in a .. less than ideal way. Will fix.
  2. frontend.Capacity option is not implemented post-refactor PR, will fix, it will help too.
  3. The more unknown is that it seems garbage collection is very active, will have to do a pointer hunt.

So, rebased on top of develop and compile time is still slow (a few minutes), but doable. I did a profile but there wasn't anything very particular. There is a lot of work in assertIsBoolean, but this is to be expected (this PR is not using range checking yet. When range checking is merged then will start natively use them).

Added documentation and examples.

@ivokub ivokub requested review from yelhousni and gbotrel and removed request for gbotrel and yelhousni February 17, 2023 18:45
@ivokub ivokub marked this pull request as ready for review February 21, 2023 11:42
Copy link
Contributor

@yelhousni yelhousni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It seems a mutatis-mutandis translation of the BN254 pairing in gnark-crypto, which is working fine. There are couple of circuit-specific optimisations that I started on a different PR. Also I'm thinking of how to do the infinity points filtering in-circuit or just proceed them (as the MultiMillerLoop is what is needed in the ECPAIR precompile). For the record, so far:

  • MillerLoop:
    Groth16 9.9M / Plonk 24.9M
  • FinalExp:
    Groth16 11.4M / Plonk 29.4M

@yelhousni
Copy link
Contributor

@ivokub One more thing, I would like to reorganise the packages a bit: sw_bls12377 and sw_bls24315 include the pairing computations while for emulated field there is weierstrass package for G1 arithmetic (including BN254) and then pairing_bn254 for the pairing, g1, g2, gt and tower. I would suggest to rename weierstrass to sw_emulated and rearrange pairing_bn254 into sw_bn254 and fields_bn254. I'll do that in the optimisation PR if you don't have any objection to this.

@ivokub ivokub merged commit db76d9e into develop Mar 6, 2023
@ivokub ivokub deleted the feat/nonnative-pairing branch March 6, 2023 14:17
@ivokub
Copy link
Collaborator Author

ivokub commented Mar 6, 2023

@ivokub One more thing, I would like to reorganise the packages a bit: sw_bls12377 and sw_bls24315 include the pairing computations while for emulated field there is weierstrass package for G1 arithmetic (including BN254) and then pairing_bn254 for the pairing, g1, g2, gt and tower. I would suggest to rename weierstrass to sw_emulated and rearrange pairing_bn254 into sw_bn254 and fields_bn254. I'll do that in the optimisation PR if you don't have any objection to this.

I'm OK with that. At some point I tried to make the towers more general, but got a bit stuck with non-ideal type inference in Go generics. But it should get better with Go 1.21 and maybe then would make more sense. Right now I wouldn't generalize too much. I had at some point idea to implement BLS12-381 as it is asked for, I'll try it then.

What do you think about having something like algebra/native and algebra/nonnative (or nativealgebra, emulatedalgebra etc). I'd prefer users wouldn't confuse two different approaches for recursion, especially that it is possible to have very efficient 2-chain recursion.

@yelhousni
Copy link
Contributor

What do you think about having something like algebra/native and algebra/nonnative (or nativealgebra, emulatedalgebra etc). I'd prefer users wouldn't confuse two different approaches for recursion, especially that it is possible to have very efficient 2-chain recursion.

Totally agree. That's the best way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants