kwantam
commented
2 years ago Thanks for your report! :+1:
With regard to naming: you're totally right, it should be monty_to_edw_generic. We'll be sure to fix this.
With regard to the map: please see the text of Section 6.8.1.
The upshot is: there are many different valid mappings from an Edwards curve to the birationally equivalent Montgomery curve. Sometimes, as in Curve25519, that mapping is given explicitly in the standard, in which case the standardized mapping should be used. For Curve25519, this is the mapping you're referring to, which includes the sqrt(-486664) factor.
Other times, there is no explicit mapping given in a standard, and for these cases Appendix D.1 gives a generic construction. The reason that Section 6.8.1 explicitly states that the standardized mapping should be used when available is that the construction in Appendix D.1 is not guaranteed to agree with that standardized mapping. As you point out, this is the case with Curve25519.
To be clear: it is not possible for us to give a construction in Appendix D.1 that will agree with all possible standards. What's provided in Appendix D.1 is correct modulo the naming issue, which we'll fix---thanks again for pointing it out!
We believe that there is an error in the following section: https://github.com/cfrg/draft-irtf-cfrg-hash-to-curve/blob/main/draft-irtf-cfrg-hash-to-curve.md#generic-montgomery-to-twisted-edwards-map-appx-rational-map-edw Instead of v=s/t, we think it should be v=(s/t)sqrt(-486664), as suggested by https://en.wikipedia.org/wiki/EdDSA#Ed25519. In our code we have generated points on Curve25519, and when mapping them to Ed25519, we get Ed25519 points when using v=(s/t)sqrt(-486664), and we do not get Ed25519 points when using v=s/t. In our implementation of edw_to_monty_generic we must multiply v by sqrt(-486664) at last to get an Ed25519 point. Also, the name of the algorithm edw_to_monty_generic is inconsistent with the parameter and return type, and should be monty_to_edw_generic instead, in accordance with the header.