-
Notifications
You must be signed in to change notification settings - Fork 493
feat: EIP-7951 for ECDSA on P-256 curve #1649
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package evmprecompiles | ||
|
|
||
| import ( | ||
| "github.com/consensys/gnark/frontend" | ||
| "github.com/consensys/gnark/std/algebra/emulated/sw_emulated" | ||
| "github.com/consensys/gnark/std/math/emulated" | ||
| "github.com/consensys/gnark/std/signature/ecdsa" | ||
| ) | ||
|
|
||
| // P256Verify implements [P256Verify] precompile contract at address 0x100. | ||
| // | ||
| // This circuit performs ECDSA signature verification over the secp256r1 | ||
| // elliptic curve (also known as P-256 or prime256v1). | ||
| // | ||
| // [P256Verify]: https://eips.ethereum.org/EIPS/eip-7951 | ||
| func P256Verify(api frontend.API, | ||
| msgHash emulated.Element[emulated.P256Fr], | ||
| r, s emulated.Element[emulated.P256Fr], | ||
| qx, qy emulated.Element[emulated.P256Fp], | ||
| ) frontend.Variable { | ||
| // Input validation: | ||
| // 1. input_length == 160 ==> checked by the arithmetization | ||
| // 2. 0 < r < n and 0 < s < n ==> checked by the arithmetization/ECDATA and enforced in `IsValid()` | ||
| // 3. 0 ≤ qx < p and 0 ≤ qy < p ==> checked by the arithmetization/ECDATA | ||
| // 4. (qx, qy) is a valid point on the curve P256 ==> checked by the arithmetization/ECDATA | ||
| // 5. (qx, qy) is not (0,0) ==> checked by the arithmetization/ECDATA | ||
| pk := ecdsa.PublicKey[emulated.P256Fp, emulated.P256Fr]{ | ||
| X: qx, | ||
| Y: qy, | ||
| } | ||
| sig := ecdsa.Signature[emulated.P256Fr]{ | ||
| R: r, | ||
| S: s, | ||
| } | ||
| verified := pk.IsValid(api, sw_emulated.GetCurveParams[emulated.P256Fp](), &msgHash, &sig) | ||
| return verified | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,145 @@ | ||||||
| package evmprecompiles | ||||||
|
|
||||||
| import ( | ||||||
| "bytes" | ||||||
| "crypto/rand" | ||||||
| "encoding/hex" | ||||||
| "encoding/json" | ||||||
| "fmt" | ||||||
| "io/ioutil" | ||||||
|
||||||
| "io/ioutil" |
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.
Bug: Assert Context Breaks Subtest Isolation
The assert helper is created with the outer test context but used inside t.Run subtests. When assert.NoError(err) fails, it reports to the parent test rather than the subtest, causing the entire test function to stop instead of just failing the specific subtest. This prevents other test vectors from running and makes it difficult to identify which specific vector failed. The assertion should use the subtest's t parameter instead.
Copilot
AI
Nov 14, 2025
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.
The function splitInput160 silently returns nil values when the input length is not 160 bytes. This could lead to nil pointer dereferences on line 97-101 where the returned values are dereferenced. Consider returning an error or panicking with a descriptive message if the length check fails.
| return nil, nil, nil, nil, nil | |
| panic(fmt.Sprintf("splitInput160: input length is %d bytes, expected 160", len(raw))) |
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.
The comments reference "arithmetization" and "ECDATA" which are zkEVM-specific implementation details. For a general-purpose gnark circuit, these validations are not performed by external systems. The comments should clarify that when used outside a zkEVM context, the caller must ensure: (1) input length is 160 bytes, (2) 0 < r < n and 0 < s < n, (3) 0 ≤ qx < p and 0 ≤ qy < p, (4) (qx, qy) is on the P-256 curve, and (5) (qx, qy) ≠ (0,0). Alternatively, consider adding explicit validation within the circuit for general-purpose use.