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

chore(curves): memoize EC constants, fix curve tests #1608

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

nhtyy
Copy link

@nhtyy nhtyy commented Oct 5, 2024

Added memoization to some curve parameters to avoid string parsing more than once during runtime

The tests in crates/curves also werent working so I enabled the needed feature in dev-deps

@nhtyy nhtyy changed the title chore(curves): memoize EC params, fix curve tests chore(curves): memoize EC constants, fix curve tests Oct 5, 2024
@jtguibas
Copy link
Contributor

jtguibas commented Oct 6, 2024

Did you add this to increase performance of the elliptic curve precompiles?

@nhtyy
Copy link
Author

nhtyy commented Oct 6, 2024

Ya, it doenst look like some of these functions like generator() are used much, but modulus does at least seem to be used from FieldParameters in a few places

Some moduli are parsed as strings (overriding the default of behavior of FieldParameters) rather than as bytes from the const, but I didnt want to change too much

@ctian1
Copy link
Member

ctian1 commented Oct 11, 2024

@nhtyy Could you use lazy_static instead of defining a new macro? Otherwise seems like a good addition, though it's not clear how much effect it really has on performance

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