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

Fix a corner case for ate pairing in BLS12 and BW6 models #460

Merged
merged 5 commits into from
Aug 28, 2022

Conversation

weikengchen
Copy link
Member

@weikengchen weikengchen commented Aug 25, 2022

Description

This PR changes the BLS12 and BW6 model's treatment of the ate pairing count. Otherwise, for certain input, it may fail.

closes: #459


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

@weikengchen weikengchen changed the title Make BLS12 and BW6 ate pairing more stable Fix a corner case for ate pairing in BLS12 and BW6 models Aug 25, 2022
@weikengchen
Copy link
Member Author

weikengchen commented Aug 25, 2022

Reference: introducing two CP curves that embed curve25519 (correction: ed25519, yes, these two curves are different...). Thanks @Pratyush particularly for pointing out the importance of having D = -3 and providing more information about conditions for a sextic twist.
https://github.com/DZK-Labs/ark-yafa/

@weikengchen
Copy link
Member Author

Note: this PR needs to be redone after #447. Since it would introduce a conflict to #447, it should not be merged.

@weikengchen weikengchen added the do-not-merge-now Do not merge due to conflicts with more important PRs label Aug 28, 2022
@Pratyush
Copy link
Member

It's fine to merge this; there should not be any conflict with master.

@Pratyush Pratyush merged commit 77fb6ab into master Aug 28, 2022
@Pratyush Pratyush deleted the fix_ate_pairing branch August 28, 2022 20:48
@codeblooded1729
Copy link

We also need to fix it in g2.rs files for both models. More specifically, you need to do the same edit in the loop where ell coefficients are generated.

@Pratyush
Copy link
Member

Thanks for flagging this @codeblooded1729 ; @weikengchen can you handle that?

@weikengchen
Copy link
Member Author

Yes---I would need to do a pass. But if I remember correctly, this does not affect those two files as their ate loop counts are not hitting this corner case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge-now Do not merge due to conflicts with more important PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BLS curve model's ate pairing implementation is unstable for various X
3 participants