-
Notifications
You must be signed in to change notification settings - Fork 29
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(sigmap-EDAP-06): Missing IsOnCurve & IsInSubgroup Checks For Elli… #229
Conversation
…ptic Curve Point - remove unnecessary function
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.
Change LGTM but I'd prefer to change the variable name.
Even the documentation comment mentions "commitment in the certificate". Why are we using something abstract like expectedCommit
? Can we just change to certificateCommitment
?
…ptic Curve Point - address PR feedback
verify/verifier.go
Outdated
actualCommit, err := v.Commit(blob) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
expectedX := &fp.Element{} | ||
expectedX.Unmarshal(expectedCommit.X) | ||
expectedX.Unmarshal(certCommitment.X) |
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.
can we change all expected -> certCommitment
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.
…ptic Curve Point - address PR feedback
…proxy into epociask--fix-EDAP-06
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.
Approving but would still like to get the last variable name change in. Night ;)
Fixes Issue
Fixes #
Changes proposed
Screenshots (Optional)
Note to reviewers