ed25519,x25519: run a subset of Wycheproof test vectors#36
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Wycheproof-based test coverage for Ed25519 signature verification and X25519 key exchange by embedding JSON vector sets and running them in package tests.
Changes:
- Introduce
crypto/ed25519/testvectorsandcrypto/x25519/testvectorshelper packages that embed and parse Wycheproof JSON into flattened vector slices. - Add Ed25519 verification tests that iterate Wycheproof vectors and assert expected validity outcomes.
- Add X25519 ECDH tests that iterate Wycheproof vectors and validate shared secret outputs for “valid” (and conditionally for “acceptable”) cases.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crypto/x25519/testvectors/vectors.go | New embedded-vector loader + flag-based selector for X25519 Wycheproof vectors. |
| crypto/x25519/key_test.go | New Wycheproof-driven X25519 key exchange test. |
| crypto/ed25519/testvectors/vectors.go | New embedded-vector loader + flag-based selector for Ed25519 Wycheproof vectors. |
| crypto/ed25519/testvectors/ed25519_test.json | Adds the Wycheproof Ed25519 verify vector JSON payload for embedding. |
| crypto/ed25519/key_test.go | New Wycheproof-driven Ed25519 VerifyBytes test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Select returns vectors that have at least one of the given flags. | ||
| func Select(vectors []Vector, flags ...string) []Vector { | ||
| var out []Vector | ||
| for _, v := range vectors { | ||
| if hasAnyFlag(v.Flags, flags) { | ||
| out = append(out, v) | ||
| } | ||
| } | ||
| return out |
There was a problem hiding this comment.
Select(vectors) currently returns an empty slice when called with no flags (because hasAnyFlag(..., nil) is always false). This is an easy footgun for callers and differs from the existing Wycheproof selector pattern in crypto/secp256k1/testvectors (SelectECDSA returns all vectors when flags is empty). Consider returning the input vectors unchanged when len(flags)==0, or update the docstring to explicitly state that passing no flags selects none.
| // Select returns vectors that have at least one of the given flags. | ||
| func Select(vectors []Vector, flags ...string) []Vector { | ||
| var out []Vector | ||
| for _, v := range vectors { | ||
| if hasAnyFlag(v.Flags, flags) { | ||
| out = append(out, v) | ||
| } | ||
| } | ||
| return out |
There was a problem hiding this comment.
Select(vectors) currently returns an empty slice when called with no flags (because hasAnyFlag(..., nil) is always false). This is an easy footgun for callers and differs from the existing Wycheproof selector pattern in crypto/secp256k1/testvectors (SelectECDSA returns all vectors when flags is empty). Consider returning the input vectors unchanged when len(flags)==0, or update the docstring to explicitly state that passing no flags selects none.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
smoyer64
left a comment
smoyer64
left a comment
There was a problem hiding this comment.
Traditionally, the ed25519_test.json file would be in the testdata folder right? And the associated vectors.go would be xxx_test.go. What's the benefit to not follow conventions here? (2 typ.)
MichaelMure
force-pushed
the
25518-wycheproof
branch
from
bffba15 to
7298447
Compare
Because I want the data loading to be at the same place as the vectors (ie, not duplicated). To be imported, that can't be a |
|
Also, that pattern already exist in the repo. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Note
Low Risk
Changes are limited to adding test coverage and embedded Wycheproof JSON vectors; no production crypto logic is modified, but the large vector files may increase repo/test runtime and CI resource usage.
Overview
Adds Wycheproof-based conformance tests for
ed25519.PublicKey.VerifyBytesandx25519ECDH (PrivateKey.KeyExchange), validating expected accept/reject behavior and shared-secret outputs across official edge-case vectors.Introduces embedded Wycheproof vector bundles under
crypto/ed25519/testvectorsandcrypto/x25519/testvectorswith small loaders (Load/Select) to parse and flatten the JSON into per-test cases.Written by Cursor Bugbot for commit 7298447. This will update automatically on new commits. Configure here.