Skip to content

Conversation

@yelhousni
Copy link
Contributor

@yelhousni yelhousni commented Nov 13, 2025

Description

This PR implements a circuit corresponding to https://eips.ethereum.org/EIPS/eip-7951 alongside test against Consensys/gnark-crypto#767 and Wycheproof test vectors (https://eips.ethereum.org/assets/eip-7951/test-vectors.json).

Needs Consensys/gnark-crypto#767 to be merged first.

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Tests against gnark-crypto pass but against Wycheproof some edge cases fail because they are checked at the arithmetization level not the gnark circuit level. Currently, p256verify_vectors_clean.json contains some data that passes gnark circuit test and p256verify_vectors.json is the entire data.

How has this been benchmarked?

In a BN254 circuit:

  • SCS: 705 923
  • R1CS: 195 897

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Note

Implements the P-256 ECDSA verification circuit (EIP-7951) and updates curve ops to support complete/unified arithmetic, with comprehensive tests and vectors.

  • Precompile (zkEVM):
    • std/evmprecompiles/P256Verify: ECDSA verification over P-256 using JointScalarMulBase with complete arithmetic; limbwise field conversion and modular equality checks.
  • Algebra (curve ops):
    • Add doubleGeneric, tripleGeneric, doubleAndAddGeneric with unified handling to avoid edge cases (zero denominators) via Select guards.
    • Wire complete arithmetic through scalar mul paths: use AddUnified/generic doubling in loops based on cfg.CompleteArithmetic.
  • Tests & Vectors:
    • New circuit tests (including mocked arithmetization) and Wycheproof/cleaned test vectors under std/evmprecompiles/test_vectors/.

Written by Cursor Bugbot for commit 5fef998. This will update automatically on new commits. Configure here.

@yelhousni yelhousni marked this pull request as draft November 13, 2025 00:47
@yelhousni yelhousni requested review from Copilot and ivokub November 14, 2025 16:26
@yelhousni yelhousni added the dep: linea Issues affecting Linea downstream label Nov 14, 2025
@yelhousni yelhousni added this to the v0.14.N milestone Nov 14, 2025
@yelhousni yelhousni marked this pull request as ready for review November 14, 2025 16:27
@yelhousni yelhousni changed the title feat: eip-7951 ecdsa p256 feat: EIP-7951 for ECDSA on P-256 curve Nov 14, 2025
Copilot finished reviewing on behalf of yelhousni November 14, 2025 16:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the P256Verify precompile (EIP-7951) for ECDSA signature verification over the secp256r1 (P-256) elliptic curve. The implementation provides a gnark circuit for verifying ECDSA signatures at the EVM precompile address 0x100.

Key changes:

  • Adds P256Verify function implementing EIP-7951 using gnark's emulated P256 curve and ECDSA signature verification
  • Includes comprehensive test coverage with both basic functional tests and Wycheproof test vectors
  • Provides 60+ test vectors from the Wycheproof project covering edge cases and malleability scenarios

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File Description
std/evmprecompiles/256-p256verify.go Core implementation of P256Verify precompile using emulated ECDSA signature verification
std/evmprecompiles/256-p256verify_test.go Test suite with basic circuit tests and EIP vector validation
std/evmprecompiles/test_vectors/p256verify_vectors_clean.json Wycheproof test vectors for comprehensive edge case coverage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ivokub
ivokub previously approved these changes Nov 16, 2025
Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a few changes. See if it makes sense. Its good to merge on my side

@yelhousni
Copy link
Contributor Author

It looks good and passes all the test vectors, so good to merge on my end too. But I will do an optimization PR for to use JointScalarMul and see if all the edge cases in the test vectors make sense in the context of the EIP.

@yelhousni
Copy link
Contributor Author

@ivokub Good catch! Actually JointScalarMulBase is better in terms of constraints than ScalarMul+ScalarMulBase. Because ScalarMulBase (scalarMulBaseGeneric) is less efficient than scalarMulFakeGLV. We had it before FakeGLV and we should remove/update it now IMO.

@ivokub
Copy link
Collaborator

ivokub commented Nov 19, 2025

@ivokub Good catch! Actually JointScalarMulBase is better in terms of constraints than ScalarMul+ScalarMulBase. Because ScalarMulBase (scalarMulBaseGeneric) is less efficient than scalarMulFakeGLV. We had it before FakeGLV and we should remove/update it now IMO.

Indeed! I actually changed to JointScalarMulBase and improved from 1M to 700k constraints. And seems all tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dep: linea Issues affecting Linea downstream

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants