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

Generic pairings for BW6 #633

Merged
merged 19 commits into from
Aug 30, 2023
Merged

Generic pairings for BW6 #633

merged 19 commits into from
Aug 30, 2023

Conversation

swasilyev
Copy link
Contributor

@swasilyev swasilyev commented Mar 27, 2023

Part of the project of getting BW6-767 the outer curve for BLS12-381 into arkworks, supersedes (fixes) #624

Implements pairings for the BW6 family in a generic way, following https://yelhousni.github.io/phd.pdf, section 4.4.2

  1. Introduces generic BW6 curve parameters: CM lifting cofactors and "trace modulo r"
  2. Makes the existing miller loop implementation support both BW6-761 and BW6-767. It conflicts with Bw6 miller loop optimization #617. I suggest we adapt the latter later.
  3. Implements the hard part of the final exponentiation in generic way for curves with "trace modulo r" = 0 (this is BW6-767)
  4. The existing hand-optimized hard part for BW6-761 moved to the curve specification.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the GitHub PR explorer

@swasilyev swasilyev requested review from a team as code owners March 27, 2023 23:07
@swasilyev swasilyev requested review from Pratyush, mmagician and weikengchen and removed request for a team March 27, 2023 23:07
@swasilyev
Copy link
Contributor Author

A fun observation. In the Miller loop computation we are currently free to get rid of

        if Self::ATE_LOOP_COUNT_1_IS_NEGATIVE {
            f_1.cyclotomic_inverse_in_place();
        }

For BW6-761 nothing changes as it has X > 0. BW6-767 isn't used afaik, so we have freedom here. @yelhousni wdyt

@mmagician
Copy link
Member

mmagician commented Mar 28, 2023

The existing hand-optimized hard part for BW6-761 moved to the curve specification.

That's still missing right? Would be good to have that PR open for this to test too, at least locally.

Edit: NVM, just saw this in the 767 PR too.

@swasilyev
Copy link
Contributor Author

swasilyev commented Mar 28, 2023

Surprisingly, on my machine

Generic implementation:

Pairing for BW6_761/Final Exponentiation for BW6_761
time: [2.9952 ms 3.0381 ms 3.0840 ms]

Curve specific one:

Pairing for BW6_761/Final Exponentiation for BW6_761
time: [3.1242 ms 3.1320 ms 3.1402 ms]
change: [+1.5416% +3.0911% +4.6056%] (p = 0.00 < 0.05)
Performance has regressed.

what slightly contradicts https://hackmd.io/@gnark/BW6-761-changes#Final-exponentiation. Moreover, the addition chains optimization (#634) is less applicable to the curve specific implementation.

The question then is if we want to support curve-specific final exp implementation for BW-761 for compatibility reasons? The results of the pairing computation will be different, but I don't know any project that would use it for smth other than bilinearity, that still holds.

ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
@swasilyev
Copy link
Contributor Author

Thank you for the review @mmagician Really exciting findings!

Copy link
Member

@mmagician mmagician left a comment

Choose a reason for hiding this comment

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

Looks good 👍🏼

@mmagician
Copy link
Member

@swasilyev On second thoughts this should go into breaking changes not features section in the CHANGELOG, WDYT?

ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
ec/src/models/bw6/mod.rs Outdated Show resolved Hide resolved
@Pratyush Pratyush added T-feature Type: new features T-refactor Type: cleanup/refactor labels Apr 1, 2023
@Pratyush Pratyush added the breaking-change This PR contains a breaking change label Apr 1, 2023
@swasilyev
Copy link
Contributor Author

Btw, i think that currently the change is not breaking, as i moved the existing curve-specific implementation to the curve definition in arkworks-rs/curves#156. Though i would remove it later in another pr.

@Pratyush
Copy link
Member

I think it's still a breaking change because defining a BW6 curve now requires specifying more constants.

@mmagician
Copy link
Member

@swasilyev I think the last part is to move the CHANGELOG entry from features to breaking changes: I agree with Pratyush that is indeed a breaking change on the API.

@Pratyush Pratyush merged commit 9db6cf6 into arkworks-rs:master Aug 30, 2023
7 of 15 checks passed
@mmagician mmagician mentioned this pull request Sep 1, 2023
6 tasks
@Pratyush Pratyush linked an issue Sep 6, 2023 that may be closed by this pull request
aleasims pushed a commit to NilFoundation/arkworks-algebra that referenced this pull request Oct 18, 2023
Co-authored-by: mmagician <[email protected]>
Co-authored-by: Pratyush Mishra <[email protected]>
aleasims added a commit to NilFoundation/arkworks-algebra that referenced this pull request Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This PR contains a breaking change T-feature Type: new features T-refactor Type: cleanup/refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BW6 generic final exponentiation (hard part)
3 participants