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.

Sign up for GitHub

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

Jump to bottom

Updated comments on madd/add/dbl functionf for curve operations #298

Merged
merged 1 commit into from
Aug 3, 2024
Merged

Updated comments on madd/add/dbl functionf for curve operations #298

merged 1 commit into from
Aug 3, 2024

Conversation

vo-nil
Copy link
Contributor

@vo-nil vo-nil commented Jul 31, 2024

Closes #297

@vo-nil vo-nil requested a review from martun July 31, 2024 08:28
@vo-nil vo-nil self-assigned this Jul 31, 2024
@vo-nil vo-nil force-pushed the 297-check-curve-operations branch 2 times, most recently from b31b4a2 to cf87360 Compare August 2, 2024 08:18
martun
martun approved these changes Aug 2, 2024
View reviewed changes
libs/algebra/include/nil/crypto3/algebra/curves/detail/forms/edwards/inverted/element_g1.hpp Outdated
{
/* Check whether constructed point has proper order */
if ( *this * (scalar_field_type::modulus - 1) + *this != zero()) {
throw std::runtime_error("Point has wrong order");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change the text to something like "EC points of order other than the group order are not allowed."

libs/algebra/include/nil/crypto3/algebra/curves/detail/forms/edwards/inverted/madd_2007_bl.hpp Outdated
@@ -44,6 +43,10 @@ namespace nil {
constexpr static inline void process(ElementType &first,
const ElementType &second) {

/* NOTE: does not handle O and pts of order 2,4, i.e.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move these back above the class definition, where it was. But keep the new text, it's more clear now.

@vo-nil
Comments reviewed and removed #297
9d88934 
Added test for points of order 2 and 4, removed unused forms and coordinates #297

Reverted constructors for inverted coordinates #297

Exception thrown in release also #297

Removed edwards<183> curve and inverted coordinates #297

Further cleanup of inverted coordinates #297
@vo-nil vo-nil force-pushed the 297-check-curve-operations branch from cf87360 to 9d88934 Compare August 2, 2024 13:26
@vo-nil vo-nil merged commit b8bad9d into master Aug 3, 2024
2 checks passed
@vo-nil vo-nil deleted the 297-check-curve-operations branch August 3, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martun martun martun approved these changes

Assignees

@vo-nil vo-nil

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Review NOTEs on curve operations
2 participants
@vo-nil @martun