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

fix: multi_scalar_multiplication #1542

Merged
merged 47 commits into from
Oct 3, 2024

Conversation

ratankaliani
Copy link
Member

@ratankaliani ratankaliani commented Sep 25, 2024

Handle the complete addition for secp256k1_add using a new WeierstrassAffinePoint trait that's implemented for secp256k1, bls12-381 and bn254. Easy to extend to secpr1.

  • Modify multi_scalar_multiplication and mul_assign to use complete_add_assign, which handles all of the potential edge cases.
  • Add generic test for secp256k1_add, bls12381_add and bn254_add that covers the complete addition cases.

Copy link

github-actions bot commented Sep 25, 2024

SP1 Performance Test Results

Branch: ratan/impl-add-assign-fixes
Commit: 27926bba
Author: ratankaliani

program cycles execute (mHz) core (kHZ) compress (KHz) time success
fibonacci 11291 0.19 4.67 0.43 26s
ssz-withdrawals 2757356 11.22 61.68 34.38 1m20s
tendermint 12593597 5.40 168.12 100.83 2m7s

@ratankaliani ratankaliani changed the title fix: impl add assign fixes fix: secp256k1 addition Sep 26, 2024
@ratankaliani ratankaliani changed the title fix: secp256k1 addition fix: weierstrass_add Sep 30, 2024
@ratankaliani ratankaliani marked this pull request as ready for review September 30, 2024 18:43
@ratankaliani ratankaliani changed the title fix: weierstrass_add fix: multi_scalar_multiplication Oct 1, 2024
@ratankaliani ratankaliani merged commit a8db6b1 into tamir/v1.3.0-rc2 Oct 3, 2024
21 checks passed
@ratankaliani ratankaliani deleted the ratan/impl-add-assign-fixes branch October 3, 2024 18:20
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.

2 participants