Skip to content
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

SIMD-0075: Precompile for Secp256r1 #3152

Open
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

0xRigel
Copy link

@0xRigel 0xRigel commented Oct 13, 2024

Problem

There currently isn't support for signature verification of signatures on the NIST-P256 curve (primary curve used by Passkeys). Motivation and details can be found in SIMD-0075

Summary of Changes

  • Adds a precompile for secp256r1 signature verification that implements the structure defined in SIMD-0075 and that abides by the rules set forth in SIMD-0152
  • Adds a separate secp256r1 crate to avoid further bloat of solana_sdk
  • Adds tests to check functionality of the precompile, as well as the computed curve order values

@0xRigel 0xRigel force-pushed the secp256r1-precompile branch 2 times, most recently from 0f4876e to 8f18340 Compare October 13, 2024 17:29
sdk/src/lib.rs Outdated Show resolved Hide resolved
@kevinheavey kevinheavey added the CI Pull Request is ready to enter CI label Oct 15, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Oct 15, 2024
@samkim-crypto samkim-crypto self-requested a review October 16, 2024 08:20
sdk/src/lib.rs Outdated Show resolved Hide resolved
@0xRigel 0xRigel marked this pull request as ready for review October 16, 2024 17:28
sdk/feature-set/src/lib.rs Outdated Show resolved Hide resolved
Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

I think I looked at just about everything except the actual cryptography. I saw @samkim-crypto self-requested review.

I spotted a handful of small nits, mostly around naming and semantics. Let me know what you think. Other than that nice work!

sdk/secp256r1/Cargo.toml Outdated Show resolved Hide resolved
sdk/secp256r1/Cargo.toml Outdated Show resolved Hide resolved
sdk/secp256r1/Cargo.toml Outdated Show resolved Hide resolved
sdk/program/src/lib.rs Outdated Show resolved Hide resolved
sdk/program/src/lib.rs Outdated Show resolved Hide resolved
sdk/program/src/lib.rs Outdated Show resolved Hide resolved
sdk/Cargo.toml Outdated Show resolved Hide resolved
sdk/src/precompiles.rs Outdated Show resolved Hide resolved
sdk/secp256r1/src/lib.rs Outdated Show resolved Hide resolved
sdk/secp256r1/src/lib.rs Outdated Show resolved Hide resolved
@samkim-crypto samkim-crypto added the CI Pull Request is ready to enter CI label Oct 22, 2024
@0xRigel
Copy link
Author

0xRigel commented Nov 6, 2024

Thanks for another set of updates. The crypto parts should look good after these set up updates. Let's try to fix the CI issues and the conflicts to master. In the meanwhile, I just wanted to spend a day or two exploring other pure rust variants that we can potentially use. If none of them suffice, then let's just stick with openssl.

Got it, sounds good 👍 Reverted the new error and fixed the conflicts. I still have the p256 implementation from the first SIMD attempt and for purposes of benchmarking, so if you want to take a look at that just let me know.

@yihau yihau added the CI Pull Request is ready to enter CI label Nov 6, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Nov 6, 2024
@yihau yihau added the CI Pull Request is ready to enter CI label Nov 6, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Nov 6, 2024
@samkim-crypto samkim-crypto added the CI Pull Request is ready to enter CI label Nov 6, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Nov 6, 2024
@jordaaash jordaaash added the CI Pull Request is ready to enter CI label Nov 6, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Nov 6, 2024
@samkim-crypto samkim-crypto added the CI Pull Request is ready to enter CI label Nov 7, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Nov 7, 2024
@jordaaash jordaaash added the CI Pull Request is ready to enter CI label Nov 7, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants