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

feat: elliptic-curves p256/k256 integration, universal Fp ops hooks #2027

Closed
wants to merge 39 commits into from

Conversation

nhtyy
Copy link
Collaborator

@nhtyy nhtyy commented Feb 4, 2025

No description provided.

@ratankaliani
Copy link
Member

Can you update the documentation with the new patches as well? Or create another PR which shows the updated patch tags?

Copy link
Contributor

github-actions bot commented Feb 6, 2025

Test Old New
curve25519_dalek_test_zero_mul 197994 197994
k256_test_verify_rand_lte_100 461701381 5281873
bn_test_bn_test_g1_add_100 811485 811492
curve25519_dalek_test_ed25519_verify 32286963 32287003
curve25519_dalek_test_decompressed_noncanonical 9263 9263
rustcrypto_bigint_test_bigint_mul_mod_special 2332628 2332628
curve25519_dalek_test_decompressed_expected_value 15848831 15693506
bls12_381_tests_test_sqrt_fp_100 941136 1057511
rustcrypto_bigint_test_bigint_mul_add_residue 2250012 2250012
bn_test_bn_test_g1_double_100 624788 624788
secp256k1_program_test_recover_rand_lte_100 10938750 7301005
curve25519_dalek_ng_test_zero_mul 197964 197964
bn_test_bn_test_fr_inverse_100 990649 990649
rust_crypto_rsa_test_pkcs_verify_100 174348604 174535015
p256_test_verify_rand_lte_100 697528817 6089925
bls12_381_tests_test_bls_add_100 15580049 15580049
curve25519_dalek_ng_test_zero_msm 217587 217587
k256_test_recover_pubkey_infinity 365074 131738
k256_test_recover_rand_lte_100 15590890 9446949
bn_test_bn_test_fq_inverse_100 969449 969449
curve25519_dalek_test_zero_msm 213931 213931
k256_test_recover_high_hash_high_recid 985457706 3948677
bls12_381_tests_test_bls_double_100 9670245 9670245
curve25519_dalek_ng_test_add_then_multiply 7654700 7686220
p256_test_recover_rand_lte_100 51465312 11248550
bn_test_bn_test_fq_sqrt_100 989049 989049
bls12_381_tests_test_sqrt_fp2_100 1882575 1985491
curve25519_dalek_test_add_then_multiply 7798254 8113398
keccack_test_expected_digest_lte_100 2024057 2028621
bls12_381_tests_test_inverse_fp2_100 3085079 3085079
p256_test_recover_high_hash_high_recid 2730149191 4966698
secp256k1_program_test_verify_rand_lte_100 179310406 16118493
sha_test_sha3_expected_digest_lte_100_times 1793395 1793101
bls12_381_tests_test_inverse_fp_100 1607625 1607625
p256_test_recover_pubkey_infinity 417622 137981
sha_test_sha2_expected_digest_lte_100_times 3761329 3764287
curve25519_dalek_ng_test_decompressed_noncanonical 204793 204793

//! Weierstrass precompiles.
//!
//! In summary, SP1 overrides curve arithmetic entirely, and patches upstream field operations
//! to be more effcient in the VM, such as `sqrt` or `inverse`.
Copy link
Contributor

Choose a reason for hiding this comment

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

typos:

Suggested change
//! to be more effcient in the VM, such as `sqrt` or `inverse`.
//! to be more efficient in the VM, such as `sqrt` or `inverse`.

type FIELD_BYTES_SIZE = U32;

/// A [`CurveArithmetic`] implementation for SP1 acceleration.
/// Patched crates will implement this trait to expose thier field element type to us.
Copy link
Contributor

Choose a reason for hiding this comment

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

typos:

Suggested change
/// Patched crates will implement this trait to expose thier field element type to us.
/// Patched crates will implement this trait to expose their field element type to us.

///
/// This "newtype" is needed due to some limitations of GATs.
///
/// Specfically, its impossible to generically implement
Copy link
Contributor

Choose a reason for hiding this comment

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

typos:

Suggested change
/// Specfically, its impossible to generically implement
/// Specifically, its impossible to generically implement
++ b/crates/zkvm/lib/src/ecdsa/projective.rs

// type `Scalar = <C as CurveArithmetic>::Scalar`,
//
// For more information:
// see the rationale in the documention of `<C as ECDSACruve>::ScalarImpl`.
Copy link
Contributor

Choose a reason for hiding this comment

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

typos:

Suggested change
// see the rationale in the documention of `<C as ECDSACruve>::ScalarImpl`.
// see the rationale in the documentation of `<C as ECDSACruve>::ScalarImpl`.

@nhtyy nhtyy closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants