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.

Sign up for GitHub

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

Jump to bottom

feat: Add BLS signature verification for BN254 #89

Merged
merged 7 commits into from
Aug 10, 2023
Merged

feat: Add BLS signature verification for BN254 #89

merged 7 commits into from
Aug 10, 2023

Conversation

flyingnobita
Copy link
Contributor

No description provided.

@jonathanpwang jonathanpwang self-requested a review July 7, 2023 21:15
jonathanpwang
jonathanpwang requested changes Jul 22, 2023
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

Some code quality comments and one change regarding add_unequal.

halo2-ecc/src/bn254/bls_signature.rs Outdated Show resolved Hide resolved
halo2-ecc/src/bn254/bls_signature.rs Outdated Show resolved Hide resolved
jonathanpwang
jonathanpwang requested changes Jul 25, 2023
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes so quickly.

Some minor change around avoiding (0,0) identity point.

Also missed last time that this new function should be returning 0 or 1, not an Fp12 point.

halo2-ecc/src/bn254/bls_signature.rs Outdated Show resolved Hide resolved
halo2-ecc/src/bn254/bls_signature.rs Outdated Show resolved Hide resolved
halo2-ecc/src/bn254/bls_signature.rs Show resolved Hide resolved
jonathanpwang
jonathanpwang requested changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Code looks good - why was the halo2_proofs repo changed? We'll need to change it back for it to merge.

Also please add your tests to the halo2-ecc git CI workflow.

halo2-base/Cargo.toml Outdated Show resolved Hide resolved
halo2-base/Cargo.toml Outdated Show resolved Hide resolved
flyingnobita
flyingnobita commented Aug 8, 2023
View reviewed changes
halo2-ecc/src/bn254/tests/bls_signature.rs Outdated Show resolved Hide resolved
halo2-base/Cargo.toml Outdated Show resolved Hide resolved
@jonathanpwang
Copy link
Contributor

LGTM

@jonathanpwang jonathanpwang merged commit f2eacb1 into axiom-crypto:community-edition Aug 10, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanpwang jonathanpwang jonathanpwang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@flyingnobita @jonathanpwang