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

sidh: updates multiplication and reduction mod p434 #235

Merged
merged 3 commits into from
Aug 27, 2022

Conversation

armfazh
Copy link
Contributor

@armfazh armfazh commented Jun 24, 2021

@armfazh armfazh requested review from wbl and claucece June 24, 2021 23:53
@armfazh armfazh self-assigned this Jun 24, 2021
@armfazh armfazh added the enhancement Improvement over something already in the project label Jun 24, 2021
@lukevalenta lukevalenta self-requested a review June 25, 2021 17:02
Copy link

@lukevalenta lukevalenta left a comment

Choose a reason for hiding this comment

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

Commit 8d9bbdc makes sense to me now, but I'm not sure how to effectively review the other two commits. Approving with that caveat, so please only merge if you're sufficiently confident in the changes or we have a second approver.

Copy link

@wbl wbl left a comment

Choose a reason for hiding this comment

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

Some more comments, or maybe a companion explaining what exactly is happening in the code would help me review. This stuff is tricky, but I think I'm starting to get the idea

dh/sidh/internal/p434/arith_amd64.s Show resolved Hide resolved
dh/sidh/internal/p434/arith_amd64.s Outdated Show resolved Hide resolved
dh/sidh/internal/p434/arith_amd64.s Outdated Show resolved Hide resolved
dh/sidh/internal/p434/arith_amd64.s Show resolved Hide resolved
Copy link
Contributor

@claucece claucece left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you ;) Maybe we can add some documentation, though

@armfazh armfazh changed the base branch from master to main August 26, 2022 23:26
@armfazh armfazh merged commit 00fa63c into cloudflare:main Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement over something already in the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants