-
Notifications
You must be signed in to change notification settings - Fork 222
Feat: Add P-256 (secp256r1) elliptic curve + ECDSA #767
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
Conversation
ivokub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! I left a few comments, but I think they are suitable for future PRs.
However the only thing I would do is to remove the SizeG1AffineCompressed constant or make it equal 64.
Or perhaps we could look if there is any other compression method?
|
@ivokub Added Hash-to-G1 and MSM and fixed review comments 👍 |
ivokub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the changes. Looks good now.
However, the comment by cursor is valid for v not being reset in the loops. It is fixed in #725 but that one isn't merged yet as it breaks test generation on gnark side.
Lets confirm in what order we should merge the PRs.
We can merge this one |
You're right. I'll merge this branch and then in #725 will just regenerate to fix the |
Description
Add P-256 (secp256r1) elliptic curve (Fr, Fp and G arithmetics + ECDSA).
Type of change
How has this been tested?
How has this been benchmarked?
Checklist:
golangci-lintdoes not output errors locallyNote
Introduce the P-256 (secp256r1) curve with full field/group ops, MSM, hash-to-curve, and ECDSA, and wire it into the registry, docs, and generators.
secp256r1(P-256)ecc/secp256r1:fp/frfield arithmetic, G1 (affine/jacobian), multiexp, hash-to-curve (SVDW), ECDSA (sign/verify/recover), (de)serialization, tests/benchmarks.ecc: addSECP256R1ID, base/scalar moduli, string mapping; expose generators and curve coefficients.ecc.md) to list P-256; refresh package header to include ECDSA, Poseidon2, etc.secp256r1(multiexp, ECDSA, marshal, hash-to-curve).Written by Cursor Bugbot for commit 5cb46b2. This will update automatically on new commits. Configure here.