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 compilation errors if only the feature hash2curve is enabled #996

Closed

Conversation

alexanderkjall
Copy link
Contributor

Without this patch:

$ cargo test --no-default-features --features hash2curve
   Compiling p256 v0.13.2 (/home/capitol/project/elliptic-curves/p256)
error[E0432]: unresolved import `crate::FieldElement`
  --> p256/src/arithmetic/hash2curve.rs:96:45
   |
96 |     use crate::{arithmetic::field::MODULUS, FieldElement, NistP256, Scalar, U256};
   |                                             ^^^^^^^^^^^^ no `FieldElement` in the root
   |
   = help: consider importing this struct through its public re-export instead:
           crate::arithmetic::FieldElement
note: found an item that was configured out
  --> p256/src/lib.rs:47:28
   |
47 | pub use arithmetic::field::FieldElement;
   |                            ^^^^^^^^^^^^
   = note: the item is gated behind the `expose-field` feature

error[E0432]: unresolved import `sha2`
   --> p256/src/arithmetic/hash2curve.rs:108:9
    |
108 |     use sha2::Sha256;
    |         ^^^^ use of undeclared crate or module `sha2`

error[E0432]: unresolved import `p256::ecdsa`
 --> p256/tests/ecdsa.rs:7:5
  |
7 |     ecdsa::{SigningKey, VerifyingKey},
  |     ^^^^^ could not find `ecdsa` in `p256`

error[E0283]: type annotations needed
  --> p256/tests/ecdsa.rs:14:70
   |
14 |         <NonZeroScalar as Reduce<U256>>::reduce_bytes(&bytes.into()).into()
   |                                                                      ^^^^
   |
   = note: cannot satisfy `_: From<NonZeroScalar<NistP256>>`
   = note: required for `NonZeroScalar<NistP256>` to implement `Into<_>`
help: try using a fully qualified path to specify the expected types
   |
14 |         <NonZeroScalar<NistP256> as Into<T>>::into(<NonZeroScalar as Reduce<U256>>::reduce_bytes(&bytes.into()))
   |         +++++++++++++++++++++++++++++++++++++++++++                                                            ~

error[E0282]: type annotations needed
  --> p256/tests/ecdsa.rs:22:30
   |
22 |         let (signature, v) = sk.sign_recoverable(msg).unwrap();
   |                              ^^ cannot infer type

Some errors have detailed explanations: E0282, E0283, E0432.
For more information about an error, try `rustc --explain E0282`.
error: could not compile `p256` (test "ecdsa") due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
warning: unused import: `group::cofactor::CofactorGroup`
   --> p256/src/arithmetic/hash2curve.rs:101:9
    |
101 |         group::cofactor::CofactorGroup,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_imports)]` on by default

warning: unused import: `MapToCurve`
   --> p256/src/arithmetic/hash2curve.rs:102:64
    |
102 |         hash2curve::{self, ExpandMsgXmd, FromOkm, GroupDigest, MapToCurve, OsswuMap},
    |                                                                ^^^^^^^^^^

For more information about this error, try `rustc --explain E0432`.
warning: `p256` (lib test) generated 2 warnings
error: could not compile `p256` (lib test) due to 2 previous errors; 2 warnings emitted

@@ -93,7 +93,9 @@ impl FromOkm for Scalar {

#[cfg(test)]
mod tests {
use crate::{arithmetic::field::MODULUS, FieldElement, NistP256, Scalar, U256};
#![cfg(feature = "expose-field")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#![cfg(feature = "expose-field")]
#[cfg(feature = "expose-field")]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, that increased the diff a bit, but made it more correct I think. Thanks for the feedback

@@ -1,6 +1,6 @@
//! ECDSA tests.

#![cfg(feature = "arithmetic")]
#![cfg(all(feature = "arithmetic", feature = "ecdsa-core"))]
Copy link
Member

Choose a reason for hiding this comment

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

How is this related to hash2curve?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reworked this a bit, it was an error that showed up after I fixed the one in the hash2curve.rs file. Hopefully more correct now.

@tarcieri
Copy link
Member

tarcieri commented Dec 1, 2023

I took a slightly different approach in #998

@tarcieri
Copy link
Member

tarcieri commented Dec 1, 2023

Merged #996 instead which fixes the tests and adds them to CI

@tarcieri tarcieri closed this Dec 1, 2023
@alexanderkjall alexanderkjall deleted the gate-hash2curve-tests branch December 2, 2023 08:06
@alexanderkjall
Copy link
Contributor Author

That was a much better approach, thanks :)

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