Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

p521: projective arithmetic tests #950

Merged
merged 1 commit into from
Nov 2, 2023
Merged

p521: projective arithmetic tests #950

merged 1 commit into from
Nov 2, 2023

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Nov 2, 2023

Uses impl_projective_arithmetic_tests! to test projective arithmetic against a set of test vectors.

Includes test vectors from:

http://point-at-infinity.org/ecc/nisttv

@tarcieri
p521: projective arithmetic tests
0d88b84 
Uses `impl_projective_arithmetic_tests!` to test projective arithmetic
against a set of test vectors.

Includes test vectors from:

http://point-at-infinity.org/ecc/nisttv
@tarcieri tarcieri merged commit f9916b2 into master Nov 2, 2023
122 checks passed
@tarcieri tarcieri deleted the p521/projective-arithmetic-tests branch November 2, 2023 21:58
@MasterAwesome
Copy link
Contributor

MasterAwesome commented Nov 2, 2023 •
edited
Loading

Just noticed the projective tests didn't run as a part of CI, probably due to the wrong feature being used arithmetic when only wip-arithmetic-do-not-use exists

elliptic-curves/p521/tests/projective.rs

Line 3 in f9916b2

#![cfg(all(feature = "arithmetic", feature = "test-vectors"))]

@tarcieri
Copy link
Member Author

tarcieri commented Nov 2, 2023

Oh wow, you're right: they're not running at all

@MasterAwesome
Copy link
Contributor

On the bright side,

test affine_to_projective ... ok
test projective_add_and_sub ... ok
test projective_double_and_sub ... ok
test projective_add_vs_double ... ok
test projective_mixed_addition ... ok
test projective_identity_addition ... ok
test test_vector_add_mixed_identity ... ok
test test_vector_double_generator ... ok
test test_vector_repeated_add_mixed ... ok
test test_vector_repeated_add ... ok
test test_vector_scalar_mult ... ok

🎉

tarcieri added a commit that referenced this pull request Nov 2, 2023
@tarcieri
p521: fix projective arithmetic tests
ac88fd3 
The tests added in #950 weren't actually running due to feature gating
(the feature is named `wip-arithmetic-do-not-use`, but the conditional
gating was using `arithmetic` instead).

It turns out a small modification to `primeorder` was needed, since the
`impl_projective_arithmetic_tests` macro was relying on `Into`
conversions from core arrays to `GenericArray` which top out at
64-element arrays, and `p521`'s field elements are 66-bytes.

With the modification in place, the tests pass!
@tarcieri tarcieri mentioned this pull request Nov 2, 2023
Merged
@tarcieri
Copy link
Member Author

tarcieri commented Nov 2, 2023

Yep, they do pass! See #951

tarcieri added a commit that referenced this pull request Nov 2, 2023
@tarcieri
p521: fix projective arithmetic tests (#951)
e77b798 
The tests added in #950 weren't actually running due to feature gating
(the feature is named `wip-arithmetic-do-not-use`, but the conditional
gating was using `arithmetic` instead).

It turns out a small modification to `primeorder` was needed, since the
`impl_projective_arithmetic_tests` macro was relying on `Into`
conversions from core arrays to `GenericArray` which top out at
64-element arrays, and `p521`'s field elements are 66-bytes.

With the modification in place, the tests pass!
tarcieri added a commit that referenced this pull request Nov 2, 2023
@tarcieri
p521: arithmetic feature
c1c7dbf 
Now that #946, #950, and #951 have landed it seems prudent to add a real
`arithmetic` feature.

This adds a feature similar to the other crates in this repo which
exposes the following types which provide a curve arithmetic
implementation:

- `AffinePoint`
- `ProjectivePoint`
- `Scalar`

The `wip-arithmetic-do-not-use` feature is now removed as well. While
technically SemVer breaking, it wasn't supposed to be used in the first
place!

Closes #947
@tarcieri tarcieri mentioned this pull request Nov 2, 2023
Merged
tarcieri added a commit that referenced this pull request Nov 2, 2023
@tarcieri
p521: arithmetic feature
a6db83b 
Now that #946, #950, and #951 have landed it seems prudent to add a real
`arithmetic` feature.

This adds a feature similar to the other crates in this repo which
exposes the following types which provide a curve arithmetic
implementation:

- `AffinePoint`
- `ProjectivePoint`
- `Scalar`

The `wip-arithmetic-do-not-use` feature is now removed as well. While
technically SemVer breaking, it wasn't supposed to be used in the first
place!

Closes #947
tarcieri added a commit that referenced this pull request Nov 3, 2023
@tarcieri
p521: arithmetic feature (#953)
3a19153 
Now that #946, #950, and #951 have landed it seems prudent to add a real
`arithmetic` feature.

This adds a feature similar to the other crates in this repo which
exposes the following types which provide a curve arithmetic
implementation:

- `AffinePoint`
- `ProjectivePoint`
- `Scalar`

The `wip-arithmetic-do-not-use` feature is now removed as well. While
technically SemVer breaking, it wasn't supposed to be used in the first
place!

Closes #947
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tarcieri @MasterAwesome