chore!: adds clippy lints config and fix lints (#91)

- adds lint config for [`cargo-lints`](https://crates.io/crates/cargo-lints)
- uses `cargo-lints` in CI
- fixes clippy errors
- Breaking: changes method signature of RistrettoComSig::sign to pass in references (clippy::needless-pass-by-value)

.github/workflows/clippy-check.yml

@@ -14,7 +14,13 @@ jobs:
         with:
           command: fmt
           args: --all -- --check
-      - uses: actions-rs/clippy-check@v1
+      - name: Install cargo-lints
+        uses: actions-rs/cargo@v1
         with:
-          token: ${{ secrets.GITHUB_TOKEN }}
-          args: --all-features
+          command: install
+          args: cargo-lints
+      - name: Clippy lints
+        uses: actions-rs/cargo@v1
+        with:
+          command: lints
+          args: clippy --all-targets

benches/signatures.rs

@@ -12,7 +12,7 @@ fn generate_secret_key(c: &mut Criterion) {
     c.bench_function("Generate secret key", |b| {
         let mut rng = thread_rng();
         b.iter(|| {
-            let _ = RistrettoSecretKey::random(&mut rng);
+            let _key = RistrettoSecretKey::random(&mut rng);
         });
     });
 }
@@ -46,7 +46,7 @@ fn sign_message(c: &mut Criterion) {
         b.iter_batched(
             gen_keypair,
             |d| {
-                let _ = RistrettoSchnorr::sign(d.k, d.r, &d.m.to_vec()).unwrap();
+                let _sig = RistrettoSchnorr::sign(d.k, d.r, &d.m.to_vec()).unwrap();
             },
             BatchSize::SmallInput,
         );

lints.toml

@@ -0,0 +1,54 @@
+deny = [
+    # Prevent spelling mistakes in lints
+    'unknown_lints',
+    # clippy groups:
+    'clippy::correctness',
+    # All clippy allows must have a reason
+    # TODO: enable lint-reasons feature
+    #    'clippy::allow_attributes_without_reason',
+    # Docs
+    # 'missing_docs',
+    #    'clippy::missing_errors_doc',
+    #    'clippy::missing_safety_doc',
+    #    'clippy::missing_panics_doc',
+
+    # Common mistakes
+    'clippy::await_holding_lock',
+    'unused_variables',
+    'unused_imports',
+    'dead_code',
+    'unused_extern_crates',
+    'unused_must_use',
+    'unreachable_patterns',
+    'clippy::cloned_instead_of_copied',
+    'clippy::create_dir',
+    'clippy::dbg_macro',
+    'clippy::else_if_without_else',
+    'clippy::enum_glob_use',
+    'clippy::inline_always',
+    'clippy::let_underscore_drop',
+    'clippy::let_unit_value',
+    'clippy::match_on_vec_items',
+    'clippy::match_wild_err_arm',
+    # In crypto code, it is fairly common to have similar names e.g. `owner_pk` and `owner_k`
+    # 'clippy::similar_names',
+    'clippy::needless_borrow',
+
+    # style
+    'clippy::explicit_into_iter_loop',
+    'clippy::explicit_iter_loop',
+    'clippy::if_not_else',
+    'clippy::match_bool',
+    'clippy::needless_pass_by_value',
+    'clippy::range_plus_one',
+    'clippy::struct_excessive_bools',
+    'clippy::too_many_lines',
+    'clippy::trivially_copy_pass_by_ref',
+
+    # casting mistakes
+    'clippy::cast_lossless',
+    'clippy::cast_possible_truncation',
+    'clippy::cast_possible_wrap',
+    'clippy::cast_precision-loss',
+    'clippy::cast_sign_loss'
+]

src/ffi/keys.rs

@@ -215,7 +215,7 @@ pub unsafe extern "C" fn sign_comsig(
     };
     let challenge = Blake256::digest(msg.as_bytes()).to_vec();
     let factory = PedersenCommitmentFactory::default();
-    let sig = match RistrettoComSig::sign(secret_a, secret_x, nonce_a, nonce_x, &challenge, &factory) {
+    let sig = match RistrettoComSig::sign(&secret_a, &secret_x, &nonce_a, &nonce_x, &challenge, &factory) {
         Ok(sig) => sig,
         _ => return SIGNING_ERROR,
     };

src/lib.rs

@@ -1,6 +1,3 @@
-extern crate serde;
-extern crate serde_json;
-
 #[macro_use]
 extern crate lazy_static;
 

src/ristretto/constants.rs

@@ -69,6 +69,7 @@ pub const RISTRETTO_NUMS_POINTS_COMPRESSED: [CompressedRistretto; 10] = [
 ];
 
 lazy_static! {
+    /// A static array of pre-generated NUMS points
     pub static ref RISTRETTO_NUMS_POINTS: [RistrettoPoint; 10] = {
         let mut arr = [RistrettoPoint::default(); 10];
         for i in 0..10 {
@@ -78,6 +79,7 @@ lazy_static! {
     };
 }
 
+/// The NUMS Ristretto point `H`
 pub const RISTRETTO_PEDERSEN_H: CompressedRistretto = RISTRETTO_NUMS_POINTS_COMPRESSED[0];
 
 #[cfg(test)]

src/ristretto/dalek_range_proof.rs

@@ -246,14 +246,14 @@ mod test {
         // Invalid value
         let v2 = RistrettoSecretKey::from(43);
         let c = commitment_factory.commit(&k, &v2);
-        assert_eq!(prover.verify(&proof, &c), false);
+        assert!(!prover.verify(&proof, &c));
         // Invalid key
         let k = RistrettoSecretKey::random(&mut rng);
         let c = commitment_factory.commit(&k, &v);
-        assert_eq!(prover.verify(&proof, &c), false);
+        assert!(!prover.verify(&proof, &c));
         // Both invalid
         let c = commitment_factory.commit(&k, &v2);
-        assert_eq!(prover.verify(&proof, &c), false);
+        assert!(!prover.verify(&proof, &c));
     }
 
     #[test]
@@ -277,7 +277,7 @@ mod test {
         let c = commitment_factory.commit(&k, &v);
         let message = b"testing12345678910111";
         let proof = prover
-            .construct_proof_with_rewind_key(&k, 42, &rewind_k, &rewind_blinding_k, &message)
+            .construct_proof_with_rewind_key(&k, 42, &rewind_k, &rewind_blinding_k, message)
             .unwrap();
 
         // test Debug impl
@@ -347,7 +347,7 @@ mod test {
         for i in 0..257 {
             let v = RistrettoSecretKey::from(i);
             let c = commitment_factory.commit(&k, &v);
-            assert_eq!(prover.verify(&proof, &c), false);
+            assert!(!prover.verify(&proof, &c));
         }
     }
 }

src/ristretto/ristretto_com_sig.rs

@@ -67,7 +67,7 @@ use crate::{
 /// let e = Blake256::digest(b"Maskerade");
 /// let factory = PedersenCommitmentFactory::default();
 /// let commitment = factory.commit(&x_val, &a_val);
-/// let sig = RistrettoComSig::sign(a_val, x_val, a_nonce, x_nonce, &e, &factory).unwrap();
+/// let sig = RistrettoComSig::sign(&a_val, &x_val, &a_nonce, &x_nonce, &e, &factory).unwrap();
 /// assert!(sig.verify_challenge(&commitment, &e, &factory));
 /// ```
 ///
@@ -151,7 +151,7 @@ mod test {
         let e_key = RistrettoSecretKey::from_bytes(&challenge).unwrap();
         let u_value = &k_1 + e_key.clone() * &x_value;
         let v_value = &k_2 + e_key * &a_value;
-        let sig = RistrettoComSig::sign(a_value, x_value, k_2, k_1, &challenge, &factory).unwrap();
+        let sig = RistrettoComSig::sign(&a_value, &x_value, &k_2, &k_1, &challenge, &factory).unwrap();
         let R_calc = sig.public_nonce();
         assert_eq!(nonce_commitment, *R_calc);
         let (_, sig_1, sig_2) = sig.complete_signature_tuple();
@@ -194,10 +194,18 @@ mod test {
             .chain(b"Moving Pictures")
             .finalize();
         // Calculate Alice's signature
-        let sig_alice =
-            RistrettoComSig::sign(a_value_alice, x_value_alice, k_2_alice, k_1_alice, &challenge, &factory).unwrap();
+        let sig_alice = RistrettoComSig::sign(
+            &a_value_alice,
+            &x_value_alice,
+            &k_2_alice,
+            &k_1_alice,
+            &challenge,
+            &factory,
+        )
+        .unwrap();
         // Calculate Bob's signature
-        let sig_bob = RistrettoComSig::sign(a_value_bob, x_value_bob, k_2_bob, k_1_bob, &challenge, &factory).unwrap();
+        let sig_bob =
+            RistrettoComSig::sign(&a_value_bob, &x_value_bob, &k_2_bob, &k_1_bob, &challenge, &factory).unwrap();
         // Now add the two signatures together
         let s_agg = &sig_alice + &sig_bob;
         // Check that the multi-sig verifies
@@ -216,7 +224,7 @@ mod test {
         let message = from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap();
         let k_1 = RistrettoSecretKey::random(&mut rng);
         let k_2 = RistrettoSecretKey::random(&mut rng);
-        assert!(RistrettoComSig::sign(a_value, x_value, k_2, k_1, &message, &factory).is_ok());
+        assert!(RistrettoComSig::sign(&a_value, &x_value, &k_2, &k_1, &message, &factory).is_ok());
     }
 
     #[test]

src/ristretto/ristretto_keys.rs

@@ -496,7 +496,7 @@ mod test {
         ];
         let mut bytes = [0u8; 32];
         for i in 0u8..16 {
-            let pk = RistrettoPublicKey::from_hex(&encodings_of_small_multiples[i as usize]).unwrap();
+            let pk = RistrettoPublicKey::from_hex(encodings_of_small_multiples[i as usize]).unwrap();
             bytes[0] = i;
             let sk = RistrettoSecretKey::from_bytes(&bytes).unwrap();
             let pk2 = RistrettoPublicKey::from_secret_key(&sk);

src/signatures/commitment_signature.rs

@@ -92,10 +92,10 @@ where
     /// Sign the provided challenge with the value commitment's value and blinding factor. The two nonces should be
     /// completely random and never reused - that responsibility lies with the calling function.
     pub fn sign<C>(
-        secret_a: K,
-        secret_x: K,
-        nonce_a: K,
-        nonce_x: K,
+        secret_a: &K,
+        secret_x: &K,
+        nonce_a: &K,
+        nonce_x: &K,
         challenge: &[u8],
         factory: &C,
     ) -> Result<Self, CommitmentSignatureError>
@@ -109,13 +109,13 @@ where
             Ok(e) => e,
             Err(_) => return Err(CommitmentSignatureError::InvalidChallenge),
         };
-        let ea = &e * &secret_a;
-        let ex = &e * &secret_x;
+        let ea = &e * secret_a;
+        let ex = &e * secret_x;
 
-        let v = &nonce_a + &ea;
-        let u = &nonce_x + &ex;
+        let v = nonce_a + &ea;
+        let u = nonce_x + &ex;
 
-        let public_commitment_nonce = factory.commit(&nonce_x, &nonce_a);
+        let public_commitment_nonce = factory.commit(nonce_x, nonce_a);
 
         Ok(Self::new(public_commitment_nonce, u, v))
     }
@@ -142,7 +142,7 @@ where
     }
 
     /// Verify if the commitment signature signed the commitment using the specified challenge (as secret key).
-    ///   v*H + u*G = R + e.C
+    ///  v*H + u*G = R + e.C
     pub fn verify<'a, C>(&self, public_commitment: &'a HomomorphicCommitment<P>, challenge: &K, factory: &C) -> bool
     where
         for<'b> &'a HomomorphicCommitment<P>: Mul<&'b K, Output = HomomorphicCommitment<P>>,

src/wasm/key_utils.rs

@@ -314,7 +314,7 @@ pub(crate) fn sign_comsig_with_key(
         None => RistrettoSecretKey::random(&mut OsRng),
     };
 
-    let sig = match RistrettoComSig::sign(private_key_a.clone(), private_key_x.clone(), r_2, r_1, e, &factory) {
+    let sig = match RistrettoComSig::sign(&private_key_a, &private_key_x, &r_2, &r_1, e, &factory) {
         Ok(s) => s,
         Err(e) => {
             result.error = format!("Could not create signature. {}", e.to_string());
@@ -483,15 +483,9 @@ mod test {
         let (sk_x, _) = random_keypair();
         let (nonce_a, _) = random_keypair();
         let (nonce_x, _) = random_keypair();
-        let sig = CommitmentSignature::<RistrettoPublicKey, _>::sign(
-            sk_a.clone(),
-            sk_x.clone(),
-            nonce_a,
-            nonce_x,
-            &hash(msg),
-            &factory,
-        )
-        .unwrap();
+        let sig =
+            CommitmentSignature::<RistrettoPublicKey, _>::sign(&sk_a, &sk_x, &nonce_a, &nonce_x, &hash(msg), &factory)
+                .unwrap();
         let commitment = factory.commit(&sk_x, &sk_a);
 
         (sig, commitment)