-
Notifications
You must be signed in to change notification settings - Fork 234
Use untwisted Edwards Curve for Ed448 scalar multiplication #1337
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
Conversation
I think we should delete the whole The only alternative I see is to somehow make all this code generic to be able to share it between Decaf and Edwards, but it seems that will actually produce more code than it will save. |
I forgot to add #1332, which makes the conversion to twisted form and back much faster. So now we are only seeing a 8% improvement, not sure if its worth it. If further optimizations to inversion are incoming this all might not be worth it. |
I think instead of this we should use a better conversion to/from the twisted form which avoids inversions: #1349 |
Yeah, with #1316 the performance of this PR is actually worse. |
Just wanted to report back: with all optimizations applied the difference between the twisted and untwisted curve is only one extra multiplication when doing additions. In total this adds up to 120 extra multiplications in the untwisted curve. So in total doing the multiplication in the twisted curve saves 112 multiplications but adds 4 squarings. |
The current scalar multiplication algorithm for
EdwardsPoint
does the computation on the twisted Edwards curve.However, the conversion is quite expensive.(turns out with #1332 the conversion is significantly faster)This PR moves the computation to happen on the untwisted Edwards curve:
Based on #1303, #1313, #1314, #1329, #1330, #1332 and #1334 to make sure all optimizations are pulled in before comparing performance.
With further improvements to inversion, the difference could go away entirely, so we might want to hold off until we get a clearer picture from crypto-bigint.