crypto: Improve bn254 point "on curve" check #1300
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the "on curve" validation check for bn254 elliptic curve points by consolidating the logic and removing a TODO comment. The change modifies the point validation to check both curve membership and the point-at-infinity case in a single return statement.
- Consolidates the point validation logic into a more concise form
- Removes the TODO comment about reverse order checking
- Maintains the same validation behavior while improving code structure
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const auto yy = pt.y * pt.y; | ||
| const auto xxx = pt.x * pt.x * pt.x; | ||
| return yy == xxx + Curve::B; | ||
| const auto on_curve = yy == xxx + Curve::B; | ||
| return on_curve || pt == 0; |
There was a problem hiding this comment.
The refactored code now computes the expensive curve equation (yy == xxx + Curve::B) even for the point at infinity (pt == 0). The original early return for pt == 0 was more efficient as it avoided unnecessary field arithmetic operations.
There was a problem hiding this comment.
We optimize for the average case. Point at infinity as the input is an edge case so checking it up front was a pessimization.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1300 +/- ##
==========================================
- Coverage 87.16% 87.16% -0.01%
==========================================
Files 167 167
Lines 24752 24751 -1
Branches 4063 4063
==========================================
- Hits 21574 21573 -1
Misses 537 537
Partials 2641 2641
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
No description provided.