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

refactor: based on #515 generify groth16 MPC setup for all curves, flatten packages+ refactor #563

Merged
merged 16 commits into from Mar 14, 2023

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Mar 14, 2023

Based on #515 (cherry picked the commits from @HSG88 );

Did mostly stylistic edits; flattened the package structure, avoided couple of allocs, and generify for all curves.

To be merged after #561 ; MPC setup is now gnark/backend/groth16/bn254/mpcsetup .

A future PR (cc @HSG88 ) would probably be nice to ensure APIs don't panic but return errors + struct and public methods are documented (you can test output with godoc) .

On the perf note, the individual scalar mul on affine coordinates are a clear bottleneck, maybe we should use jacobian? (+ optimize the memory usage of the mulGLV path cc @yelhousni in gnark-crypto) .

  • groth16 MPC setup
  • fix Z in preparePhase2
  • parallelize coefficients multiplication
  • parallelize scalar multiplication
  • refactor: expose all typed backends in gnark/backend (moved from internal/)
  • refactor: flatten mpc structure, idomify APIs
  • test: ensure phase2 serialization is tested
  • test: added reference benchmark
  • refactor: setup -> mpcsetup
  • refactor: move utils in mpcsetup; limit api surface
  • refactor: minor code cleaning
  • build: generify mpcsetup for all curves

Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

I think it looks good. Didn't go through the math exactly, hoping that @ThomasPiellard had a look.

Considering that the usage is more complex, I would recommend adding documentation example and a bit of package documentation. If sounds good, then can also do it myself.

Maybe would be good to add type-switched methods into backend/groth16?

@gbotrel gbotrel merged commit 02dca8c into develop Mar 14, 2023
@gbotrel gbotrel deleted the stage/bnb/groth16setup branch March 14, 2023 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants