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

Update AffineCurve equal method to account for points at infinity #281

Merged
merged 21 commits into from
Jul 29, 2024

Conversation

ymekuria
Copy link
Contributor

@ymekuria ymekuria commented Jul 13, 2024

Summary

This PR addresses an issue with the the equal() method in AffineCurve. The method ignores the possibility of either point in the comparison being a point at infinity.

Impact

This issue could lead to equality checks that may succeed when they should fail, or vice versa, when the point at infinity is involved.

Changes

  • New types PointAtInfinity and FinitePoint were added.
  • The type definition of GroupAffine was updated to be a union type of the PointAtInfinity and FinitePoint.
  • The equal method in CreateCurveAffine was implemented to correctly handle the cases when either point is at infinity.

o1js: o1-labs/o1js#1758

…oordinates for better representation of PointAtInfinity
…o use object destructuring for cleaner code and improve readability
…ineOnCurve function to destructure parameters and use them directly in calculations
…ntAtInfinity functions as they are not being used in the codebase
@ymekuria ymekuria marked this pull request as ready for review July 16, 2024 15:37
@ymekuria ymekuria changed the title Add check on infinity flag Update AffineCurve equal method to account for points at infinity Jul 16, 2024
@Trivo25
Copy link
Member

Trivo25 commented Jul 16, 2024

This change causes a bubble-up effect in o1js as well, can you address this in a parent PR into o1js? If this introduces any breaking changes, we should consider targeting V2 if we cannot avoid breaking the API

@ymekuria
Copy link
Contributor Author

This change causes a bubble-up effect in o1js as well, can you address this in a parent PR into o1js? If this introduces any breaking changes, we should consider targeting V2 if we cannot avoid breaking the API

I'll link the parent PR in this PR.

@ymekuria ymekuria merged commit b5e907b into main Jul 29, 2024
1 check passed
@ymekuria ymekuria deleted the feature/infinity-flag branch July 29, 2024 06:06
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