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

Legendre trait now uses accelerated Jacobi #102

Merged

Conversation

mratsim
Copy link
Contributor

@mratsim mratsim commented Nov 17, 2023

Followup on #95

As discussed with @CPerezz at ProgCrypto, without a trait for Legendre it would not be possible to use it generically in algorithms like hash-to-curve or for testing, requiring a macro or individual implementation on a per-curve basis.

Unfortunately the new implementation is incompatible with pasta-curves as those are private, in general it's extremely hard to work with them (impossible to add assembly for example), this might be problematic for Nova over a cycle of curve (cc @huitseeker )

This PR reworks the fast modular inversion and Legendre symbol in a ff_ext folder as they definitely should be upstreamed.

The Legendre trait has been changed to not derive from ff::Field and output i64

Copy link
Contributor

@davidnevadoc davidnevadoc 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 the improvement. LGTM!

Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot for working on this @mratsim <3

@CPerezz CPerezz added this pull request to the merge queue Nov 24, 2023
Merged via the queue into privacy-scaling-explorations:main with commit 2e7f8eb Nov 24, 2023
7 checks passed
@mratsim mratsim deleted the pse-pr-cleanup-legendre branch November 24, 2023 16:07
jonathanpwang pushed a commit to axiom-crypto/halo2curves that referenced this pull request Nov 29, 2023
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.

3 participants