diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 88f9467..d21d56e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -73,3 +73,19 @@ jobs: env: DO_WASM: true run: ./contrib/test.sh + + Clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + - run: rustup component add clippy + - uses: actions-rs/cargo@v1 + with: + command: clippy + args: --features=rand-std,recovery,lowmemory,global-context --all-targets -- -D warnings diff --git a/README.md b/README.md index a2c85b2..bd19fb6 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,17 @@ Contributions to this library are welcome. A few guidelines: * No crypto should be implemented in Rust, with the possible exception of hash functions. Cryptographic contributions should be directed upstream to libsecp256k1. * This library should always compile with any combination of features on **Rust 1.41.1**. +### Githooks + +To assist devs in catching errors _before_ running CI we provide some githooks. If you do not +already have locally configured githooks you can use the ones in this repository by running, in the +root directory of the repository: +``` +git config --local core.hooksPath githooks/ +``` + +Alternatively add symlinks in your `.git/hooks` directory to any of the githooks we provide. + ## Fuzzing If you want to fuzz this library, or any library which depends on it, you will diff --git a/githooks/pre-commit b/githooks/pre-commit new file mode 100755 index 0000000..cabd06f --- /dev/null +++ b/githooks/pre-commit @@ -0,0 +1,50 @@ +#!/bin/sh +# +# A hook script to verify what is about to be committed. +# Called by "git commit" with no arguments. The hook should +# exit with non-zero status after issuing an appropriate message if +# it wants to stop the commit. + +if git rev-parse --verify HEAD >/dev/null 2>&1 +then + against=HEAD +else + # Initial commit: diff against an empty tree object + against=$(git hash-object -t tree /dev/null) +fi + +# If you want to allow non-ASCII filenames set this variable to true. +allownonascii=$(git config --bool hooks.allownonascii) + +# Redirect output to stderr. +exec 1>&2 + +# Cross platform projects tend to avoid non-ASCII filenames; prevent +# them from being added to the repository. We exploit the fact that the +# printable range starts at the space character and ends with tilde. +if [ "$allownonascii" != "true" ] && + # Note that the use of brackets around a tr range is ok here, (it's + # even required, for portability to Solaris 10's /usr/bin/tr), since + # the square bracket bytes happen to fall in the designated range. + test $(git diff --cached --name-only --diff-filter=A -z $against | + LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 +then + cat <<\EOF +Error: Attempt to add a non-ASCII file name. + +This can cause problems if you want to work with people on other platforms. + +To be portable it is advisable to rename the file. + +If you know what you are doing you can disable this check using: + + git config hooks.allownonascii true +EOF + exit 1 +fi + +# If there are whitespace errors, print the offending file names and fail. +git diff-index --check --cached $against -- || exit 1 + +# Check that code lints cleanly. +cargo clippy --features=rand-std,recovery,lowmemory,global-context --all-targets -- -D warnings || exit 1 diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 61d4646..ae6698e 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -19,6 +19,8 @@ // Coding conventions #![deny(non_upper_case_globals, non_camel_case_types, non_snake_case, unused_mut)] +#![allow(clippy::missing_safety_doc)] + #![cfg_attr(all(not(test), not(feature = "std")), no_std)] #![cfg_attr(docsrs, feature(doc_cfg))] diff --git a/src/context.rs b/src/context.rs index b5b5525..12afb0f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -114,9 +114,6 @@ mod alloc_only { use crate::ffi::{self, types::{c_uint, c_void}}; use crate::{Secp256k1, Signing, Verification, Context, AlignedType}; - #[cfg(feature = "rand-std")] - use rand; - impl private::Sealed for SignOnly {} impl private::Sealed for All {} impl private::Sealed for VerifyOnly {} @@ -191,7 +188,7 @@ mod alloc_only { /// ctx.seeded_randomize(&seed); /// # } /// ``` - #[allow(unused_mut)] // Unused when `rand-std` is not enabled. + #[cfg_attr(not(feature = "rand-std"), allow(clippy::let_and_return, unused_mut))] pub fn gen_new() -> Secp256k1 { #[cfg(target_arch = "wasm32")] ffi::types::sanity_checks_for_wasm(); diff --git a/src/ecdh.rs b/src/ecdh.rs index 3ebf948..70ad705 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -176,7 +176,7 @@ impl ::serde::Serialize for SharedSecret { let mut buf = [0u8; SHARED_SECRET_SIZE * 2]; s.serialize_str(crate::to_hex(&self.0, &mut buf).expect("fixed-size hex serialization")) } else { - s.serialize_bytes(&self.as_ref()[..]) + s.serialize_bytes(self.as_ref()) } } } @@ -272,9 +272,7 @@ mod tests { 0xff, 0xff, 0, 0, 0xff, 0xff, 0, 0, 99, 99, 99, 99, 99, 99, 99, 99 ]; - static STR: &'static str = "\ - 01010101010101010001020304050607ffff0000ffff00006363636363636363\ - "; + static STR: &str = "01010101010101010001020304050607ffff0000ffff00006363636363636363"; let secret = SharedSecret::from_slice(&BYTES).unwrap(); diff --git a/src/ecdsa/mod.rs b/src/ecdsa/mod.rs index 8c0f814..d9a8ba4 100644 --- a/src/ecdsa/mod.rs +++ b/src/ecdsa/mod.rs @@ -1,6 +1,6 @@ //! Structs and functionality related to the ECDSA signature algorithm. -use core::{fmt, str, ops, ptr, mem}; +use core::{fmt, str, ops, ptr}; use crate::{Signing, Verification, Message, PublicKey, Secp256k1, SecretKey, from_hex, Error, ffi}; use crate::ffi::CPtr; @@ -398,14 +398,8 @@ impl Secp256k1 { } counter += 1; - // From 1.32 can use `to_le_bytes` instead - let le_counter = counter.to_le(); - let le_counter_bytes : [u8; 4] = mem::transmute(le_counter); - for (i, b) in le_counter_bytes.iter().enumerate() { - extra_entropy[i] = *b; - } - - entropy_p = extra_entropy.as_ptr() as *const ffi::types::c_void; + extra_entropy[..4].copy_from_slice(&counter.to_le_bytes()); + entropy_p = extra_entropy.as_ptr().cast::(); // When fuzzing, these checks will usually spinloop forever, so just short-circuit them. #[cfg(fuzzing)] diff --git a/src/key.rs b/src/key.rs index c3ddfab..787859d 100644 --- a/src/key.rs +++ b/src/key.rs @@ -431,7 +431,7 @@ impl PublicKey { #[cfg(feature = "global-context")] #[cfg_attr(docsrs, doc(cfg(feature = "global-context")))] pub fn from_secret_key_global(sk: &SecretKey) -> PublicKey { - PublicKey::from_secret_key(&SECP256K1, sk) + PublicKey::from_secret_key(SECP256K1, sk) } /// Creates a public key directly from a slice. @@ -1621,7 +1621,7 @@ pub mod serde_keypair { let secret_key = SecretKey::deserialize(deserializer)?; Ok(KeyPair::from_secret_key( - &crate::SECP256K1, + crate::SECP256K1, &secret_key, )) } @@ -1875,7 +1875,7 @@ mod test { assert!(PublicKey::from_str("0218845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd1").is_err()); assert!(PublicKey::from_str("xx0218845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd1").is_err()); - let long_str: String = core::iter::repeat('a').take(1024 * 1024).collect(); + let long_str = "a".repeat(1024 * 1024); assert!(SecretKey::from_str(&long_str).is_err()); assert!(PublicKey::from_str(&long_str).is_err()); } @@ -2077,6 +2077,7 @@ mod test { #[cfg(not(fuzzing))] #[test] + #[allow(clippy::nonminimal_bool)] fn pubkey_equal() { let pk1 = PublicKey::from_slice( &hex!("0241cc121c419921942add6db6482fb36243faf83317c866d2a28d8c6d7089f7ba"), @@ -2108,9 +2109,8 @@ mod test { 0xff, 0xff, 0, 0, 0xff, 0xff, 0, 0, 99, 99, 99, 99, 99, 99, 99, 99 ]; - static SK_STR: &'static str = "\ - 01010101010101010001020304050607ffff0000ffff00006363636363636363\ - "; + static SK_STR: &str = "01010101010101010001020304050607ffff0000ffff00006363636363636363"; + #[cfg(fuzzing)] static PK_BYTES: [u8; 33] = [ 0x02, @@ -2119,9 +2119,7 @@ mod test { 0x06, 0x83, 0x7f, 0x30, 0xaa, 0x0c, 0xd0, 0x54, 0x4a, 0xc8, 0x87, 0xfe, 0x91, 0xdd, 0xd1, 0x66, ]; - static PK_STR: &'static str = "\ - 0218845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166\ - "; + static PK_STR: &str = "0218845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166"; #[cfg(not(fuzzing))] let s = Secp256k1::new(); @@ -2236,9 +2234,7 @@ mod test { 0xff, 0xff, 0, 0, 0xff, 0xff, 0, 0, 99, 99, 99, 99, 99, 99, 99, 99 ]; - static SK_STR: &'static str = "\ - 01010101010101010001020304050607ffff0000ffff00006363636363636363\ - "; + static SK_STR: &str = "01010101010101010001020304050607ffff0000ffff00006363636363636363"; let sk = KeyPairWrapper(KeyPair::from_seckey_slice(&crate::SECP256K1, &SK_BYTES).unwrap()); assert_tokens(&sk.compact(), &[ diff --git a/src/lib.rs b/src/lib.rs index aa1c38f..c2dc65f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -152,6 +152,8 @@ #![deny(non_upper_case_globals, non_camel_case_types, non_snake_case)] #![warn(missing_docs, missing_copy_implementations, missing_debug_implementations)] +#![allow(clippy::missing_safety_doc)] + #![cfg_attr(all(not(test), not(feature = "std")), no_std)] #![cfg_attr(all(test, feature = "unstable"), feature(test))] #![cfg_attr(docsrs, feature(doc_cfg))] @@ -729,10 +731,10 @@ mod tests { assert_eq!( ecdsa::Signature::from_der(&byte_str).expect("byte str decode"), - ecdsa::Signature::from_str(&hex_str).expect("byte str decode") + ecdsa::Signature::from_str(hex_str).expect("byte str decode") ); - let sig = ecdsa::Signature::from_str(&hex_str).expect("byte str decode"); + let sig = ecdsa::Signature::from_str(hex_str).expect("byte str decode"); assert_eq!(&sig.to_string(), hex_str); assert_eq!(&format!("{:?}", sig), hex_str); @@ -759,7 +761,7 @@ mod tests { // 71 byte signature let hex_str = "30450221009d0bad576719d32ae76bedb34c774866673cbde3f4e12951555c9408e6ce774b02202876e7102f204f6bfee26c967c3926ce702cf97d4b010062e193f763190f6776"; - let sig = ecdsa::Signature::from_str(&hex_str).expect("byte str decode"); + let sig = ecdsa::Signature::from_str(hex_str).expect("byte str decode"); assert_eq!(&format!("{}", sig), hex_str); } @@ -1001,7 +1003,7 @@ mod tests { 226, 108, 150, 124, 57, 38, 206, 112, 44, 249, 125, 75, 1, 0, 98, 225, 147, 247, 99, 25, 15, 103, 118 ]; - static SIG_STR: &'static str = "\ + static SIG_STR: &str = "\ 30450221009d0bad576719d32ae76bedb34c774866673cbde3f4e12951555c9408e6ce77\ 4b02202876e7102f204f6bfee26c967c3926ce702cf97d4b010062e193f763190f6776\ "; @@ -1026,7 +1028,7 @@ mod tests { let msg = Message::from_slice(&msg_data).unwrap(); // Check usage as explicit parameter - let pk = PublicKey::from_secret_key(&SECP256K1, &sk); + let pk = PublicKey::from_secret_key(SECP256K1, &sk); // Check usage as self let sig = SECP256K1.sign_ecdsa(&msg, &sk); diff --git a/src/schnorr.rs b/src/schnorr.rs index cefaee4..f5f602d 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -505,7 +505,7 @@ mod tests { ) .is_err()); - let long_str: String = core::iter::repeat('a').take(1024 * 1024).collect(); + let long_str: String = "a".repeat(1024 * 1024); assert!(XOnlyPublicKey::from_str(&long_str).is_err()); } @@ -548,7 +548,7 @@ mod tests { 0x9e, 0x4d, 0x4e, 0xf5, 0x67, 0x8a, 0xd0, 0xd9, 0xd5, 0x32, 0xc0, 0xdf, 0xa9, 0x07, 0xb5, 0x68, 0x72, 0x2d, 0x0b, 0x01, 0x19, 0xba, ]; - static SIG_STR: &'static str = "\ + static SIG_STR: &str = "\ 14d0bf1a8953506fb460f58be141af767fd112535fb3922ef217308e2c26706f1eeb432b3dba9a01082f9e4d4ef5678ad0d9d532c0dfa907b568722d0b0119ba\ "; @@ -556,9 +556,7 @@ mod tests { 24, 132, 87, 129, 246, 49, 196, 143, 28, 151, 9, 226, 48, 146, 6, 125, 6, 131, 127, 48, 170, 12, 208, 84, 74, 200, 135, 254, 145, 221, 209, 102 ]; - static PK_STR: &'static str = "\ - 18845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166\ - "; + static PK_STR: &str = "18845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166"; let pk = XOnlyPublicKey::from_slice(&PK_BYTES).unwrap(); assert_tokens(&sig.compact(), &[Token::BorrowedBytes(&SIG_BYTES[..])]);