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

Addition for twisted edwards curves assumes a = -1 #87

Closed
kevaundray opened this issue Oct 19, 2021 · 2 comments
Closed

Addition for twisted edwards curves assumes a = -1 #87

kevaundray opened this issue Oct 19, 2021 · 2 comments

Comments

@kevaundray
Copy link

kevaundray commented Oct 19, 2021

Summary

When instantiating the curve parameters for an embedded curve like JubJub, the A parameter needs to be set, however the group arithmetic formulas assume that A = -1.

A is set here: https://github.com/ConsenSys/gnark-crypto/blob/1572c4e3cda6663b8d00ae05291e4b1d4f585cd8/ecc/bls12-381/twistededwards/twistededwards.go#L52

point addition is computed here, implicitly assuming a = -1 : https://github.com/ConsenSys/gnark-crypto/blob/1572c4e3cda6663b8d00ae05291e4b1d4f585cd8/ecc/bls12-381/twistededwards/point.go#L199

Disadvantages

Explicitly adding the support for various As will add an extra field multiplication per affine point addition and other various costs.

Advantages

If someone uses your library and does not use A = -1, it will produce the correct results. All of the embedded curves used in your library so far use A=-1.

Solutions

  • One can assume that A will always be -1 for all curves of interest to gnark-crypto, and not allow the A variable to be configurable.
  • Change the group arithmetic to use any specified a value. I'm not familiar with go-generate; it may be possible to generate code when a=-1 and otherwise generate the general formulas
@kevaundray kevaundray changed the title Affine addition for twisted edwards curves assumes a = -1 Addition for twisted edwards curves assumes a = -1 Oct 19, 2021
@yelhousni
Copy link
Collaborator

Thank you for raising this issue @kevaundray. Indeed we assume that for all curves of interest to gnark-crypto A=-1. Previously, we had arithmetic for configurable A (commit 06804c5) but we changed that as we were able to find faster reduced form (A=-1) for all curves we implement. I just made a PR (#88) to get rid of A to avoid ambiguity. That is said, we might go back to old code if there is need for a new twisted Edwards curve that do not support reduced form.

@kevaundray
Copy link
Author

Thanks for the PR! Will close this issue as it is merged

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

No branches or pull requests

2 participants