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(BW6-761): use revisited Ate pairing instead of Tate #876

Merged
merged 7 commits into from
Oct 24, 2023

Conversation

yelhousni
Copy link
Contributor

@yelhousni yelhousni commented Oct 19, 2023

Description

Revisit the Miller loop algorithm from #846 to allow fixed-argument pairings needed for KZG optimization (see #874). TL;DR: use Eq. (6') instead of (6) in https://hackmd.io/@gnark/BW6-761-changes.

The PR needs gnark-crypto PR Consensys/gnark-crypto#465 for testing.

TODO:

  • Implement revisited ate Miller loop
  • Implement fixed-argument revisited ate Miller loop

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How has this been tested?

Same existing tests work fine + new tests for fixed-argument pairing (compare pairing result to gnark-crypto).

How has this been benchmarked?

Old New
R1CS 2,764,966 2,762,882
SCS 28,191,311 28,215,838

We lost 2084 R1CS and 24527 SRS for a single pairing compared to master but this should be offset in KZG gadget #874 with the fixed-argument pairing that the new Miller loop allows (see below).

Fixed-argument pairing
R1CS 2,133,509
SCS 22,481,220

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

@github-actions
Copy link

📦 github.com/consensys/gnark/backend/plonk
TestProver 0s

TestProver/bn254 0s

📦 github.com/consensys/gnark/internal/stats
TestCircuitStatistics 0s

TestCircuitStatistics/math/bits.ToBinary/bn254/groth16 0s

📦 github.com/consensys/gnark/internal/tinyfield
TestElementAdd 0s

TestElementBitLen 0s

TestElementButterflies 0s

TestElementBytes 0s

TestElementDiv 0s

+ Exp: having the receiver as operand should output the same result: OK,
   passed 200 tests.
+ z.Halve must match z / 2: OK, passed 200 tests.

TestElementDouble 0s

TestElementEqual 0s

TestElementExp 0s

TestElementInverse 0s

TestElementInverseExp 0s

TestElementLegendre 0s

TestElementLexicographicallyLargest 0s

TestElementMul 0s

TestElementMulByConstants 0s

TestElementNeg 0s

TestElementSelect 0s

TestElementSetInt64 0s

TestElementSetInterface 0s

+ Exp: operation result must match big.Int result: OK, passed 200 tests.

TestElementSqrt 0s

TestElementSquare 0s

TestElementSub 0s

@yelhousni yelhousni marked this pull request as draft October 20, 2023 18:00
@yelhousni yelhousni changed the title refactor(bw6-761): use revisited Ate pairing instead of Tate Refactor(BW6-761): use revisited Ate pairing instead of Tate Oct 20, 2023
@yelhousni yelhousni marked this pull request as ready for review October 20, 2023 22:45
@github-actions
Copy link

📦 github.com/consensys/gnark/backend/plonk
TestProver 0s

TestProver/bn254 0s

📦 github.com/consensys/gnark/internal/stats
TestCircuitStatistics 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bls12_377/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bls12_377/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bls12_377/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bls12_381/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bls12_381/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bls12_381/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bn254/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bn254/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bn254/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bw6_761/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bw6_761/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/bw6_761/plonkFRI 0s

TestCircuitStatistics/api/Lookup2/bls12_377/groth16 0s

TestCircuitStatistics/api/Lookup2/bls12_377/plonk 0s

TestCircuitStatistics/api/Lookup2/bls12_377/plonkFRI 0s

TestCircuitStatistics/api/Lookup2/bls12_381/groth16 0s

TestCircuitStatistics/api/Lookup2/bls12_381/plonk 0s

TestCircuitStatistics/api/Lookup2/bls12_381/plonkFRI 0s

TestCircuitStatistics/api/Lookup2/bls24_315/groth16 0s

TestCircuitStatistics/api/Lookup2/bls24_315/plonk 0s

TestCircuitStatistics/api/Lookup2/bls24_315/plonkFRI 0s

TestCircuitStatistics/api/Lookup2/bls24_317/groth16 0s

TestCircuitStatistics/api/Lookup2/bls24_317/plonk 0s

TestCircuitStatistics/api/Lookup2/bls24_317/plonkFRI 0s

TestCircuitStatistics/api/Lookup2/bn254/groth16 0s

TestCircuitStatistics/api/Lookup2/bn254/plonk 0s

TestCircuitStatistics/api/Lookup2/bn254/plonkFRI 0s

TestCircuitStatistics/api/Lookup2/bw6_633/groth16 0s

TestCircuitStatistics/api/Lookup2/bw6_633/plonk 0s

TestCircuitStatistics/api/Lookup2/bw6_633/plonkFRI 0s

TestCircuitStatistics/api/Lookup2/bw6_761/groth16 0s

TestCircuitStatistics/api/Lookup2/bw6_761/plonk 0s

TestCircuitStatistics/api/Lookup2/bw6_761/plonkFRI 0s

@yelhousni yelhousni requested a review from ivokub October 23, 2023 20:23
@yelhousni
Copy link
Contributor Author

@ivokub this is ready for review after we merge Consensys/gnark-crypto#465

ivokub

This comment was marked as outdated.

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.

Actually, I was relooking and there were some errors. Will submit another review after testing and recompiling

@yelhousni
Copy link
Contributor Author

Actually, I was relooking and there were some errors. Will submit another review after testing and recompiling

There was an old test of a function that does not exist anymore (MulBy034) — replaced it by the new test of MulBy014.

@github-actions
Copy link

📦 github.com/consensys/gnark/backend/plonk
TestProver 0s

TestProver/bn254 0s

📦 github.com/consensys/gnark/internal/stats
TestCircuitStatistics 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls12_377/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls12_377/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls12_377/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls12_381/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls12_381/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls12_381/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls24_315/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls24_315/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls24_315/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls24_317/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls24_317/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bls24_317/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bn254/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bn254/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bn254/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bw6_633/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bw6_633/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bw6_633/plonkFRI 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bw6_761/groth16 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bw6_761/plonk 0s

TestCircuitStatistics/api/AssertIsLessOrEqual/constant_bound_64_bits/bw6_761/plonkFRI 0s

TestCircuitStatistics/api/IsZero/bls12_377/groth16 0s

TestCircuitStatistics/api/IsZero/bls12_377/plonk 0s

TestCircuitStatistics/api/IsZero/bls12_377/plonkFRI 0s

TestCircuitStatistics/api/IsZero/bls12_381/groth16 0s

TestCircuitStatistics/api/IsZero/bls12_381/plonk 0s

TestCircuitStatistics/api/IsZero/bls12_381/plonkFRI 0s

TestCircuitStatistics/api/IsZero/bls24_315/groth16 0s

TestCircuitStatistics/api/IsZero/bls24_315/plonk 0s

TestCircuitStatistics/api/IsZero/bls24_315/plonkFRI 0s

TestCircuitStatistics/api/IsZero/bls24_317/groth16 0s

TestCircuitStatistics/api/IsZero/bls24_317/plonk 0s

TestCircuitStatistics/api/IsZero/bls24_317/plonkFRI 0s

TestCircuitStatistics/api/IsZero/bn254/groth16 0s

TestCircuitStatistics/api/IsZero/bn254/plonk 0s

TestCircuitStatistics/api/IsZero/bn254/plonkFRI 0s

TestCircuitStatistics/api/IsZero/bw6_633/groth16 0s

TestCircuitStatistics/api/IsZero/bw6_633/plonk 0s

TestCircuitStatistics/api/IsZero/bw6_633/plonkFRI 0s

TestCircuitStatistics/api/IsZero/bw6_761/groth16 0s

TestCircuitStatistics/api/IsZero/bw6_761/plonk 0s

TestCircuitStatistics/api/IsZero/bw6_761/plonkFRI 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls12_377/groth16 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls12_377/plonk 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls12_377/plonkFRI 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls12_381/groth16 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls12_381/plonk 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls12_381/plonkFRI 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls24_315/groth16 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls24_315/plonk 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls24_315/plonkFRI 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls24_317/groth16 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls24_317/plonk 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bls24_317/plonkFRI 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bn254/groth16 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bn254/plonk 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bn254/plonkFRI 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bw6_633/groth16 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bw6_633/plonk 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bw6_633/plonkFRI 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bw6_761/groth16 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bw6_761/plonk 0s

TestCircuitStatistics/math/bits.ToBinary/unconstrained/bw6_761/plonkFRI 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls12_377/groth16 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls12_377/plonk 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls12_377/plonkFRI 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls12_381/groth16 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls12_381/plonk 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls12_381/plonkFRI 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls24_315/groth16 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls24_315/plonk 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls24_315/plonkFRI 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls24_317/groth16 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls24_317/plonk 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bls24_317/plonkFRI 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bn254/groth16 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bn254/plonk 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bn254/plonkFRI 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bw6_633/groth16 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bw6_633/plonk 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bw6_633/plonkFRI 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bw6_761/groth16 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bw6_761/plonk 0s

TestCircuitStatistics/math/bits.ToNAF/unconstrained/bw6_761/plonkFRI 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls12_377/groth16 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls12_377/plonk 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls12_377/plonkFRI 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls12_381/groth16 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls12_381/plonk 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls12_381/plonkFRI 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls24_315/groth16 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls24_315/plonk 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls24_315/plonkFRI 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls24_317/groth16 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls24_317/plonk 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bls24_317/plonkFRI 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bn254/groth16 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bn254/plonk 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bn254/plonkFRI 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bw6_633/groth16 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bw6_633/plonk 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bw6_633/plonkFRI 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bw6_761/groth16 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bw6_761/plonk 0s

TestCircuitStatistics/math/bits.ToTernary/unconstrained/bw6_761/plonkFRI 0s

TestCircuitStatistics/pairing_bls12377/bw6_761/groth16 0s

📦 github.com/consensys/gnark/internal/tinyfield
TestElementAdd 0s

TestElementBitLen 0s

TestElementButterflies 0s

TestElementBytes 0s

TestElementDiv 0s

+ Div: having the receiver as operand should output the same result: OK,
   passed 200 tests.
+ x⁻ᵏ == 1/xᵏ: OK, passed 200 tests.

TestElementDouble 0s

TestElementEqual 0s

TestElementExp 0s

TestElementHalve 0s

TestElementInverse 0s

TestElementInverseExp 0s

TestElementLegendre 0s

TestElementLexicographicallyLargest 0s

TestElementMul 0s

+ Div: operation result must match big.Int result: OK, passed 200 tests.
+ Mul: having the receiver as operand should output the same result: OK,
   passed 200 tests.

TestElementMulByConstants 0s

TestElementNeg 0s

TestElementSelect 0s

TestElementSetInt64 0s

TestElementSetInterface 0s

TestElementSqrt 0s

TestElementSquare 0s

TestElementSub 0s

+ Sub: having the receiver as operand should output the same result: OK,
   passed 200 tests.
+ Mul: operation result must match big.Int result: OK, passed 200 tests.

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.

Yup, running the KZG and Groth16 examples also timeouted my other PR. And the test with Mul034 was the failing one. Now looks perfect!

@yelhousni yelhousni mentioned this pull request Oct 23, 2023
20 tasks
@yelhousni yelhousni merged commit b1768ff into master Oct 24, 2023
7 checks passed
@yelhousni yelhousni deleted the feat/bw6761-fixed-pairing branch October 24, 2023 13:59
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.

2 participants