Skip to content

Conversation

@cpu
Copy link
Member

@cpu cpu commented Dec 1, 2025

This is an extension of #196 with a few differences. Please check my work :-)

  1. The x25519_test.json test case (tcId 101) that @XoifaiI flagged was also present in the x25519_pem_test.json and x25519_jwk_test.json variations. I amended their commit to fix these instances in the same manner as was done in x25519_test.json (Another point in favour of having less formats...)
  2. I believe both the x25519 and x448 test cases had some acceptable result test cases that were missing the "Twist" flag.
  3. I believe the x25519 test cases had some acceptable result test cases that had the "Twist" flag, but that weren't twist points.

I've included the program I wrote to lint these files, and integrated it with CI. This is mainly so others can check my work. I could be convinced it's not worth including the tool + CI integration. Please chime in if you have opinions here.

XoifaiI and others added 3 commits December 1, 2025 12:00
Several low order public key tests specify twist x points but were
missing the appropriate flag.
These testcases are not on the twist.
@XoifaiI
Copy link

XoifaiI commented Dec 1, 2025

Yep looks good, nice touch adding support for all test formats in twist check.
Very minor thing I can see is doing if legendre.Cmp(big.NewInt(0)) == 0 || legendre.Cmp(big.NewInt(1)) == 0 { rather than explicitly checking for -1, but its the same logically so I'd leave it.

And yeah I forgot forking the repo and then comparing would mean you can't push the change, I just did that so I could use the forks json link until it was fixed, thanks!

@XoifaiI
Copy link

XoifaiI commented Dec 1, 2025

Can confirm all the tests (including x25519) now pass for this now
image

@cpu cpu force-pushed the cpu-do-the-twist branch from 04920b5 to 4bda03b Compare December 1, 2025 21:18
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

Successfully merging this pull request may close these issues.

3 participants