Support Secp256r1 #1885
Support Secp256r1 #1885
Conversation
|
@Trivo25 can you get me the necessary reviewers for this? |
|
Thanks for the review @Geometer1729! I also need one here and then I need you to merge both PRs since I don't have permission to |
|
Oh and @Trivo25 I also need admin review for changing the bindings commit |
|
@mitschabaude Can you run |
no @Geometer1729, I didn't change anything that needs compiling. also, CI isn't failing, just the benchmark job is, and that one always fails on forks |
|
updating the bindings is just needed when you need to recompile ocaml or rust sources |
|
Oh, I thought it was also affected by some of the content of o1js-bindings. Fair enough sorry about that. |
| }); | ||
|
|
||
| console.time('ecdsa verify, no endomorphism (build constraint system)'); | ||
| let csNoEndo = (await programNoEndo.analyzeMethods()).ecdsa; |
There was a problem hiding this comment.
Could we add some test vectors to verify this implementation?
There was a problem hiding this comment.
yes! I'll do that
|
Amazing work! Florian has final approval, but it LGTM |
|
the test is failing because we're still running node 18 in CI, where the web crypto API is missing 😬 do you mind if I change it to node 20? |
|
need workflow approval again 😢 @Trivo25 |
|
and again @Trivo25! |
|
@nicc @mrmr1993 that's why I need write access to the repo ^^ #1885 (comment) #1885 (comment) |
|
CI is green, ready to approve & merge 🚀 EDIT: and the bindings PR! |
|
@hattyhattington17 can we get your eyes on this too? |
|
@mitschabaude fyi I will get this merged before the next release |
| @@ -129,7 +128,7 @@ function double(p1: Point, Curve: { modulus: bigint; a: bigint }) { | |||
| let [x1_, y1_] = Field3.toBigints(x1, y1); | |||
| let denom = inverse(mod(2n * y1_, f), f) ?? 0n; | |||
|
|
|||
| let m = mod(3n * mod(x1_ ** 2n, f) * denom, f); | |||
| let m = mod((3n * mod(x1_ ** 2n, f) + Curve.a) * denom, f); | |||
There was a problem hiding this comment.
In PR310 this m is computed with a single modular operation, meaning: let m = mod((3n * x * x + a) * d, p);. Cannot we remove one mod from this line?
There was a problem hiding this comment.
good question! I think using mod() twice is better actually, because both x and d are large field elements. so x^2 is already a bigint of 2x the length, and it's more efficient to first reduce it (with mod()) back to 1x the length before doing another bigint multiplication with d. In the code you mention, we get an intermediate result of 2x * 1x = 3x the length
closes #1724
this adds ECDSA (and general) support for the curve secp256r1, also known as P-256, which is the standard curve used in ECDSA outside the crypto world, due to its recommendation by NIST; see P-256 in this document
Most changes are in o1-labs/o1js-bindings#310
changes here:
ain EC addition and doubling methodsEfficiency
Sadly, secp256r1 doesn't support the endomorphism we have implemented, because that endomorphism crucially relies on the
a = 0property. Therefore, in our ECDSA circuit the GLV method is not used, which means significant constraint savings get lost: