On Tue, 12 Nov 2024 22:45:33 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Ben Perez has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> no classpath exception in test copyright header
>
> src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 471:
>
>> 469: }
>> 470: }
>> 471: return null;
>
> Why return null? Why not just use `void` as return type? Same for the
> `checkPrivateKey(...)` method.
This is because the `NamedKEM` methods for checking keys can optionally return
the key. I can pass that key along if that makes more sense
> src/java.base/share/classes/com/sun/crypto/provider/SHA3Parallel.java line 37:
>
>> 35: import static sun.security.provider.SHA3.keccak;
>> 36:
>> 37: public class SHA3Parallel {
>
> Why not merge this with `sun.security.provider.SHA3` class? A separate class
> in a different package seems harder to track...
@ferakocz might have some more insight here
> src/java.base/share/classes/com/sun/crypto/provider/SHA3Parallel.java line
> 122:
>
>> 120: }
>> 121:
>> 122: public static final class Shake256Parallel extends SHA3Parallel {
>
> I didn't find usage of this class in this PR? Is this for future usage?
This is used in ML-DSA, but I can remove it from this PR
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1841166694
PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1841169108
PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1841168499
