Here are some more comments, all minor. I will probably need one more
round of review.
- src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java
362 /* XDH does not require native implementation */
Looks like a cut-paste error. I think you can remove this line.
- src/java.base/share/classes/java/security/interfaces/EdECPrivateKey.java
- src/java.base/share/classes/java/security/interfaces/EdECPublicKey.java
- src/java.base/share/classes/java/security/spec/EdECPoint.java
In the class description of these 3 classes, you are missing a <p> at
the start of the 2nd paragraph. Otherwise when you generate javadoc, it
won't create a new paragraph.
- src/java.base/share/classes/java/security/spec/EdDSAParameterSpec.java
71 * @param context the context that is bound to the signature
This should probably say that the parameter is copied. Also getContext()
should say that it returns a copy (if not null).
92 * Get the context that should be used, if any
Missing period at end of sentence.
- src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdDSAPublicKeyImpl.java
58 // set the high-order bit of the
.. of the ?
73 // construct the EdECPoint representation
74
This comment applies to code after line 74, so I think it would make
more sense if you removed line 74 and added a blank line before 73.
- src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdECOperations.java
It would be useful to have some additional comments in this code.
--Sean
On 2/25/20 3:49 PM, Anthony Scarpino wrote:
Hi
I need a code review for the EdDSA support in JEP 339. The code builds
on the existing java implemented constant time classes used for XDH and
the NIST curves. The change also adds classes to the public API to
support EdDSA operations.
All information about the JEP is located at:
JEP 339: https://bugs.openjdk.java.net/browse/JDK-8199231
CSR: https://bugs.openjdk.java.net/browse/JDK-8190219
webrev: https://cr.openjdk.java.net/~ascarpino/8166597/webrev/
thanks
Tony
