Conversation
ivokub
left a comment
There was a problem hiding this comment.
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.
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.