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

ff_derive fails with no sqrt for p = 5 (mod 8) and p = 9 (mod 16) #84

Open
bgillesp opened this issue May 12, 2022 · 2 comments · May be fixed by #115
Open

ff_derive fails with no sqrt for p = 5 (mod 8) and p = 9 (mod 16) #84

bgillesp opened this issue May 12, 2022 · 2 comments · May be fixed by #115

Comments

@bgillesp
Copy link

The ff_derive derived PrimeField implementation fails to derive a sqrt function for primes p = 5 (mod 8) and p = 9 (mod 16), resulting in a compile-time error for these cases. According to the introduction of IACR Preprint 2012/685 (the cited reference for the algorithms used for the p = 3 (mod 4) and p = 1 (mod 16) cases), efficient algorithms do exist for computing square roots over these primes; however, these algorithms are not currently implemented here.

In Issue #33, this limitation is noted explicitly, so it may be that the desired use cases for this library don't require full coverage of odd primes. I just wanted to check whether this is an intentional omission for maintainability, or if it's simply a feature that hasn't been added yet. If it's the latter and maintainers are interested, I might be able to assemble a pull request.

@str4d
Copy link
Member

str4d commented Oct 31, 2022

This is just a feature no one has requested before. If someone wants to provide square root helpers for the uncovered cases, I'd be happy to review a PR. #93 adds helper methods for Tonelli-Shanks and for a generic sqrt_ratio; we could also have similar helper methods for other primes, and then use them in ff_derive.

@daira
Copy link
Contributor

daira commented Oct 31, 2022

Tonelli–Shanks works for these primes, so why don't we just use that? The performance loss is relatively small I think.

@VictorArcium VictorArcium linked a pull request Oct 10, 2024 that will close this issue
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 a pull request may close this issue.

3 participants