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: double method for FieldElement #547

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

tcoratger
Copy link
Collaborator

Description

This pull request proposes a performance enhancement in the implementation of elliptic curve operations. Instead of using a full multiplication operation like FieldElement::TWO * a to handle doubling (2 * a), it suggests utilizing the double method available in the FieldElement struct.

The rationale behind this enhancement is that the doubling operation (a + a) requires only one addition between field elements, which is more performant compared to a full multiplication. Therefore, replacing multiplication with addition can improve the efficiency of various computations involving point doubling in elliptic curve arithmetic.

Changes Made

  • Added logic to utilize the double method from the FieldElement struct for doubling operations.
  • Updated relevant computations to use the double method where appropriate.

@xJonathanLEI
Copy link
Owner

xJonathanLEI commented Feb 17, 2024

Ran the benchmark on this PR branch against master:

class_hash              time:   [17.715 ms 17.755 ms 17.804 ms]
                        change: [+0.0908% +0.6897% +1.1519%] (p = 0.01 < 0.05)

ecdsa_get_public_key    time:   [91.500 µs 91.624 µs 91.803 µs]
                        change: [-12.552% -11.651% -11.003%] (p = 0.00 < 0.05)

ecdsa_recover           time:   [376.48 µs 378.87 µs 381.03 µs]
                        change: [-4.9179% -4.3248% -3.7237%] (p = 0.00 < 0.05)

ecdsa_sign              time:   [136.55 µs 136.85 µs 137.24 µs]
                        change: [-11.674% -11.152% -10.675%] (p = 0.00 < 0.05)

ecdsa_verify            time:   [389.95 µs 390.62 µs 391.22 µs]
                        change: [-0.6257% -0.2373% +0.1272%] (p = 0.23 > 0.05)

pedersen_hash           time:   [24.607 µs 24.649 µs 24.700 µs]
                        change: [-0.6410% +0.2009% +1.1220%] (p = 0.66 > 0.05)

poseidon_hash           time:   [7.9407 µs 7.9519 µs 7.9660 µs]
                        change: [-15.201% -14.834% -14.310%] (p = 0.00 < 0.05)

poseidon_hash_single    time:   [7.5840 µs 7.6086 µs 7.6400 µs]
                        change: [-17.138% -16.698% -16.290%] (p = 0.00 < 0.05)

poseidon_hash_many      time:   [15.201 µs 15.218 µs 15.239 µs]
                        change: [-19.208% -19.062% -18.923%] (p = 0.00 < 0.05)

rfc6979_generate_k      time:   [1.6403 µs 1.6422 µs 1.6459 µs]
                        change: [+8.1256% +8.6450% +9.3945%] (p = 0.00 < 0.05)

So we got some pretty decent improvements on ECDSA stuff, and even more impressive ones on Poseidon. Thank you!

Copy link
Owner

@xJonathanLEI xJonathanLEI left a comment

Choose a reason for hiding this comment

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

LGTM

@xJonathanLEI xJonathanLEI changed the title Implement double method for FieldElement feat: double method for FieldElement Feb 17, 2024
@xJonathanLEI xJonathanLEI merged commit 6ebaa9b into xJonathanLEI:master Feb 17, 2024
24 checks passed
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