-
Notifications
You must be signed in to change notification settings - Fork 596
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
Improved performance for ecdsa. #4985
Conversation
87040db
to
e28ccf6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 6 files at r1, all commit messages.
Reviewable status: 1 of 6 files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware and @orizi)
corelib/src/ec.cairo
line 92 at r1 (raw file):
ec_state_add(ref self, :p); } /// Adds a point to the computation.
Subs
Code quote:
Adds
corelib/src/ec.cairo
line 129 at r1 (raw file):
Option::Some(EcPointTrait::new_nz(:x, :y)?.into()) } /// Creates a new EC point from its (x, y) coordinates.
Same below.
Suggestion:
new NonZero EC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 6 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware and @orizi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware and @ilyalesokhin-starkware)
corelib/src/ec.cairo
line 92 at r1 (raw file):
Previously, gilbens-starkware (Gil Ben-Shachar) wrote…
Subs
Done.
corelib/src/ec.cairo
line 129 at r1 (raw file):
Previously, gilbens-starkware (Gil Ben-Shachar) wrote…
Same below.
Done.
11c9566
to
935d069
Compare
e28ccf6
to
8a7e76f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware)
935d069
to
19d49d5
Compare
8a7e76f
to
ce2ace2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)
ce2ace2
to
58700f5
Compare
why clone instead of creating a new one? Code quote: let mut zG_state = init_ec.clone(); |
why clone? Code quote: let mut zG_plus_eQ_state = zG_state.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware)
corelib/src/ecdsa.cairo
line 82 at r3 (raw file):
Previously, ilyalesokhin-starkware wrote…
why clone instead of creating a new one?
much cheaper - it is copyable - but would not like to let people use without noting the differences.
Previously, ilyalesokhin-starkware wrote…
nvm I see that you are using it twice. |
Do you need to clone here? isn't this the last use? Code quote: zG_state.clone(); |
commit-id:b55c2287
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware)
corelib/src/ecdsa.cairo
line 108 at r3 (raw file):
Previously, ilyalesokhin-starkware wrote…
Do you need to clone here? isn't this the last use?
Done.
58700f5
to
7627e89
Compare
This change is