Skip to content

k256+p256+p384: add PrimeField constant tests#737

Merged
tarcieri merged 9 commits into
masterfrom
k256+p256+p384/add-constant-tests
Feb 3, 2023
Merged

k256+p256+p384: add PrimeField constant tests#737
tarcieri merged 9 commits into
masterfrom
k256+p256+p384/add-constant-tests

Conversation

@tarcieri

@tarcieri tarcieri commented Feb 1, 2023

Copy link
Copy Markdown
Member

Adds tests for the TWO_INV, ROOT_OF_UNITY, and ROOT_OF_UNITY_INV constants, based on the tests generated via ff_derive.

Some of them are currently failing and have been annotated with #[ignore] and a TODO comment.

Adds tests for the `TWO_INV`, `ROOT_OF_UNITY`, and `ROOT_OF_UNITY_INV`
constants, based on the tests generated via `ff_derive`.

Some of them are currently failing and have been annotated with
`#[ignore]` and a TODO comment.
@tarcieri tarcieri requested review from fjarri and str4d February 1, 2023 22:33
Comment thread k256/src/arithmetic/field.rs
@tarcieri

tarcieri commented Feb 1, 2023

Copy link
Copy Markdown
Member Author

Note: they're copy-pasted for now, but once I get these all working (including DELTA) I can extract them into elliptic-curve::dev as a macro or something

@fjarri

fjarri commented Feb 1, 2023

Copy link
Copy Markdown
Contributor

I think there are a few tests missing here:

  • MULTIPLICATIVE_GENERATOR ^ ((modulus - 1) >> S) == ROOT_OF_UNITY
  • MULTIPLICATIVE_GENERATOR ^ (2^S) == DELTA

@tarcieri

tarcieri commented Feb 2, 2023

Copy link
Copy Markdown
Member Author

Yeah as implied in the comment above yours the DELTA constants aren't defined yet

@tarcieri

tarcieri commented Feb 2, 2023

Copy link
Copy Markdown
Member Author

Whoa, just hit an ICE on stable Rust (looks like while compiling the dependencies):

https://github.com/RustCrypto/elliptic-curves/actions/runs/4078778175/jobs/7029397528

Edit: filed an issue rust-lang/rust#107613

@tarcieri

tarcieri commented Feb 3, 2023

Copy link
Copy Markdown
Member Author

Okay, after an ICE and failures possibly related to GitHub rewriting OS package tarballs I'm going to land this while it's green.

Will backfill a few additional tests and try to consolidate the definitions of the test into a reusable macro.

@tarcieri tarcieri merged commit 231fcaf into master Feb 3, 2023
@tarcieri tarcieri deleted the k256+p256+p384/add-constant-tests branch February 3, 2023 00:11
This was referenced Mar 3, 2023
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.

2 participants