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

Add pairing BLS12 377 #933

Merged
merged 18 commits into from
Nov 11, 2024
Merged

Add pairing BLS12 377 #933

merged 18 commits into from
Nov 11, 2024

Conversation

jotabulacios
Copy link
Contributor

@jotabulacios jotabulacios commented Oct 25, 2024

BLS 12-377 pairing

Description

This PR aims to add the pairing for the BLS 12-377 curve.

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2024

Codecov Report

Attention: Patch coverage is 99.15966% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.03%. Comparing base (8dfddac) to head (3a3b85d).

Files with missing lines Patch % Lines
...rt_weierstrass/curves/bls12_377/field_extension.rs 93.87% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #933      +/-   ##
==========================================
+ Coverage   71.62%   72.03%   +0.41%     
==========================================
  Files         149      150       +1     
  Lines       32560    32909     +349     
==========================================
+ Hits        23321    23707     +386     
+ Misses       9239     9202      -37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jotabulacios jotabulacios marked this pull request as ready for review October 25, 2024 20:14
@jotabulacios jotabulacios requested a review from a team as a code owner October 25, 2024 20:14
// Pairings over Families of Elliptic Curves" (https://eprint.iacr.org/2020/875.pdf)

pub fn final_exponentiation(f: &Fp12E) -> Fp12E {
let f_easy_aux = f.conjugate() * f.inv().unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This unwrap should be removed, perhaps pairing error could have another field DivisionByZero or InvZeroError? (Maybe converting it from the FieldError?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are prior validations that ensure that f won't be zero, so there's no risk of a division by zero or related error occurring here

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is ok, but even though we are sure that Result will never be Err, handling the error is always better than using unwrap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Changed the final_exponentiation to handle the error

Copy link
Collaborator

@ilitteri ilitteri left a comment

Choose a reason for hiding this comment

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

The PR looks good. Left some comments and suggestions.

Copy link
Collaborator

@ilitteri ilitteri left a comment

Choose a reason for hiding this comment

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

LGTM

@jotabulacios jotabulacios added this pull request to the merge queue Nov 11, 2024
Merged via the queue into main with commit e650e0f Nov 11, 2024
8 checks passed
@jotabulacios jotabulacios deleted the add_pairing_bls12_377 branch November 11, 2024 19:21
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.

5 participants