Skip to content

perf: groth16 verifier circuit uses precomputed lines for all curves#1016

Merged
yelhousni merged 2 commits intomasterfrom
perf/g16-circuit
Jan 22, 2024
Merged

perf: groth16 verifier circuit uses precomputed lines for all curves#1016
yelhousni merged 2 commits intomasterfrom
perf/g16-circuit

Conversation

@yelhousni
Copy link
Copy Markdown
Contributor

@yelhousni yelhousni commented Jan 22, 2024

Description

We forgot to update Groth16 verifier circuit to use NewG2AffineFixed for deltaNeg and gammaNeg in ValueOfVerifyingKeyFixed for all curves.

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Tests in verifier_test.go in std/recursion/groth16 pass.

How has this been benchmarked?

(WIP: should benchmark for example bn254-over-bn254 and bn254-over-bw6-761)

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copy link
Copy Markdown
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.

LGTM!

@yelhousni yelhousni added type: perf type: consolidate strengthen an existing feature labels Jan 22, 2024
@yelhousni yelhousni added this to the v0.9.0 milestone Jan 22, 2024
@yelhousni yelhousni merged commit 986081a into master Jan 22, 2024
@yelhousni yelhousni deleted the perf/g16-circuit branch January 22, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: consolidate strengthen an existing feature type: perf

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants