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.

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

Fix missing features found by Rust 1.80 #1062

Merged
merged 1 commit into from
Jul 26, 2024
Merged

Conversation

tarcieri
Copy link
Member

Some crates were defining features but not using them:

  • bign256: bits, serde, test-vectors
  • p521: bits
  • sm2: bits

In fixing this, I also uncovered that bign256's test vectors are failing.

Comment on lines 3 to 23
// TODO(tarcieri): these are failing

use elliptic_curve::{
group::{ff::PrimeField, GroupEncoding},
sec1::{self, ToEncodedPoint},
};
use p256::{
test_vectors::group::{ADD_TEST_VECTORS, MUL_TEST_VECTORS},
AffinePoint, ProjectivePoint, Scalar,
};
use primeorder::{impl_projective_arithmetic_tests, Double};
// #![cfg(all(feature = "arithmetic", feature = "test-vectors"))]
//
// use bign256::{
// test_vectors::group::{ADD_TEST_VECTORS, MUL_TEST_VECTORS},
// AffinePoint, ProjectivePoint, Scalar,
// };
// use elliptic_curve::{
// group::{ff::PrimeField, GroupEncoding},
// sec1::{self, ToEncodedPoint},
// };
// use primeorder::{impl_projective_arithmetic_tests, Double};
//
// impl_projective_arithmetic_tests!(
// AffinePoint,
// ProjectivePoint,
// Scalar,
// ADD_TEST_VECTORS,
// MUL_TEST_VECTORS
// );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @makavity, these weren't actually running before apparently:

test test_vector_double_generator ... FAILED
test test_vector_repeated_add ... FAILED
test test_vector_repeated_add_mixed ... FAILED
test test_vector_scalar_mult ... FAILED

failures:

---- test_vector_double_generator stdout ----
thread 'test_vector_double_generator' panicked at bign256/tests/projective.rs:17:1:
assertion `left == right` failed
  left: [45, 132, 25, 227, 217, 5, 210, 30, 54, 17, 38, 29, 173, 201, 91, 179, 85, 131, 9, 12, 44, 173, 208, 200, 7, 245, 59, 179, 130, 143, 9, 149]
 right: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_vector_repeated_add stdout ----
thread 'test_vector_repeated_add' panicked at bign256/tests/projective.rs:17:1:
assertion `left == right` failed
  left: [45, 132, 25, 227, 217, 5, 210, 30, 54, 17, 38, 29, 173, 201, 91, 179, 85, 131, 9, 12, 44, 173, 208, 200, 7, 245, 59, 179, 130, 143, 9, 149]
 right: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

---- test_vector_repeated_add_mixed stdout ----
thread 'test_vector_repeated_add_mixed' panicked at bign256/tests/projective.rs:17:1:
assertion `left == right` failed
  left: [45, 132, 25, 227, 217, 5, 210, 30, 54, 17, 38, 29, 173, 201, 91, 179, 85, 131, 9, 12, 44, 173, 208, 200, 7, 245, 59, 179, 130, 143, 9, 149]
 right: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

---- test_vector_scalar_mult stdout ----
thread 'test_vector_scalar_mult' panicked at bign256/tests/projective.rs:17:1:
assertion `left == right` failed
  left: [45, 132, 25, 227, 217, 5, 210, 30, 54, 17, 38, 29, 173, 201, 91, 179, 85, 131, 9, 12, 44, 173, 208, 200, 7, 245, 59, 179, 130, 143, 9, 149]
 right: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]


failures:
    test_vector_double_generator
    test_vector_repeated_add
    test_vector_repeated_add_mixed
    test_vector_scalar_mult

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed ADD_TEST_VECTORS, because of test specific.

// k = 1,..,20, p += G
    (
        hex!("0000000000000000000000000000000000000000000000000000000000000000"),
        hex!("6BF7FC3CFB16D69F5CE4C9A351D6835D78913966C408F6521E29CF1804516A93"),
    ),
    ...

But have no idea, how to fix MUL_TEST_VECTORS. Do you have snippet for sagemath, to generate it?

Copy link
Contributor

@makavity makavity Jul 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm. Just generated and fixed :)

p = 2**256 - 189
a = 2**256 - 192
b = 0x77CE6C1515F3A8EDD2C13AABE4D8FBBE4CF55069978B9253B22E7D6BD69C03F1
Gx = 0
Gy = 0x6BF7FC3CFB16D69F5CE4C9A351D6835D78913966C408F6521E29CF1804516A93
h = 1

# Create the finite field and the elliptic curve
F = GF(p)
E = EllipticCurve(F, [a, b])

# Define the base point
G = E(Gx, Gy)
n = G.order()

# Generate test vectors
def generate_mul_test_vectors(num_vectors):
    vectors = []
    for _ in range(num_vectors):
        k = ZZ.random_element(1, n)
        P = k * G
        vectors.append((k, P))
    return vectors

# Number of test vectors to generate
num_vectors = 20
test_vectors = generate_mul_test_vectors(num_vectors)

# Print the test vectors in the required format
for k, P in test_vectors:
    k_hex = f"{k:064X}"
    Px_hex = f"{int(P[0]):064X}"
    Py_hex = f"{int(P[1]):064X}"
    print(f'    (')
    print(f'        hex!("{k_hex}"),')
    print(f'        hex!("{Px_hex}"),')
    print(f'        hex!("{Py_hex}"),')
    print(f'    ),')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #1065.
Thank you

Some crates were defining features but not using them:

- bign256: added `bits`, `serde`, `test-vectors` features
- p521: removed `bits` vestiges (too hard with weird modulus size)
- sm2: added `bits` feature

In fixing this, I also uncovered that bign256's test vectors are
failing.
@tarcieri tarcieri merged commit 235e401 into master Jul 26, 2024
36 checks passed
@tarcieri tarcieri deleted the fix-missing-features branch July 26, 2024 15:08
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