Clippy fixes (#57)

* Clippy fixes

* oops

.github/workflows/clippy.yml

@@ -0,0 +1,37 @@
+name:                  Check clippy
+
+on:
+  pull_request:
+  push:
+    branches:
+      - master
+      - stable
+    tags:
+      - v*
+    paths-ignore:
+      - 'README.md'
+jobs:
+  check-clippy:
+    name:              Check clippy
+    runs-on:           ubuntu-latest
+    env:
+      RUST_BACKTRACE:  full
+    steps:
+      - name:          Checkout sources & submodules
+        uses:          actions/checkout@master
+        with:
+          fetch-depth: 5
+          submodules:  recursive
+      - name:          Install toolchain
+        uses:          actions-rs/toolchain@master
+        with:
+          profile:     minimal
+          toolchain:   stable
+          components:  clippy, rustfmt
+          override:    true
+      - name:          Checking clippy
+        uses:          actions-rs/cargo@master
+        with:
+          command:     clippy
+          toolchain:   stable
+          args:        --all --all-features

core/src/der.rs

@@ -15,6 +15,10 @@ impl SignatureArray {
     pub fn len(&self) -> usize {
         self.1
     }
+
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
 }
 
 impl AsRef<[u8]> for SignatureArray {

core/src/ecdsa.rs

@@ -52,7 +52,7 @@ impl ECMultContext {
         if pr.eq_x_var(&xr) {
             return true;
         }
-        return false;
+        false
     }
 
     pub fn recover_raw(
@@ -96,9 +96,9 @@ impl ECMultContext {
         pubkey.set_gej_var(&qj);
 
         if pubkey.is_infinity() {
-            return Err(Error::InvalidSignature);
+            Err(Error::InvalidSignature)
         } else {
-            return Ok(pubkey);
+            Ok(pubkey)
         }
     }
 }
@@ -135,8 +135,8 @@ impl ECMultGenContext {
         }
         if sigs.is_high() {
             sigs = -sigs;
-            recid = recid ^ 1;
+            recid ^= 1;
         }
-        return Ok((sigr, sigs, recid));
+        Ok((sigr, sigs, recid))
     }
 }

core/src/ecmult.rs

@@ -45,6 +45,7 @@ pub struct ECMultContext {
 impl ECMultContext {
     /// Create a new `ECMultContext` from raw values.
     ///
+    /// # Safety
     /// The function is unsafe because incorrect value of `pre_g` can lead to
     /// crypto logic failure. You most likely do not want to use this function,
     /// but `ECMultContext::new_boxed`.
@@ -86,9 +87,9 @@ impl ECMultContext {
 /// coordinates. Not constant time.
 pub fn set_all_gej_var(a: &[Jacobian]) -> Vec<Affine> {
     let mut az: Vec<Field> = Vec::with_capacity(a.len());
-    for i in 0..a.len() {
-        if !a[i].is_infinity() {
-            az.push(a[i].z);
+    for point in a {
+        if !point.is_infinity() {
+            az.push(point.z);
         }
     }
     let azi: Vec<Field> = inv_all_var(&az);
@@ -111,7 +112,7 @@ pub fn set_all_gej_var(a: &[Jacobian]) -> Vec<Affine> {
 /// output magnitudes are 1 (but not guaranteed to be
 /// normalized).
 pub fn inv_all_var(fields: &[Field]) -> Vec<Field> {
-    if fields.len() == 0 {
+    if fields.is_empty() {
         return Vec::new();
     }
 
@@ -129,7 +130,7 @@ pub fn inv_all_var(fields: &[Field]) -> Vec<Field> {
         let j = i;
         let i = i - 1;
         ret[j] = ret[i] * u;
-        u = u * fields[j];
+        u *= fields[j];
     }
 
     ret[0] = u;
@@ -165,6 +166,7 @@ pub struct ECMultGenContext {
 impl ECMultGenContext {
     /// Create a new `ECMultGenContext` from raw values.
     ///
+    /// # Safety
     /// The function is unsafe because incorrect value of `pre_g` can lead to
     /// crypto logic failure. You most likely do not want to use this function,
     /// but `ECMultGenContext::new_boxed`.
@@ -208,12 +210,8 @@ impl ECMultGenContext {
 
         // Construct a group element with no known corresponding scalar (nothing up my sleeve).
         let mut nums_32 = [0u8; 32];
-        debug_assert!("The scalar for this x is unknown".as_bytes().len() == 32);
-        for (i, v) in "The scalar for this x is unknown"
-            .as_bytes()
-            .iter()
-            .enumerate()
-        {
+        debug_assert!(b"The scalar for this x is unknown".len() == 32);
+        for (i, v) in b"The scalar for this x is unknown".iter().enumerate() {
             nums_32[i] = *v;
         }
         let mut nums_x = Field::default();
@@ -260,7 +258,7 @@ impl ECMultGenContext {
 
 pub fn odd_multiples_table(prej: &mut [Jacobian], zr: &mut [Field], a: &Jacobian) {
     debug_assert!(prej.len() == zr.len());
-    debug_assert!(prej.len() > 0);
+    debug_assert!(!prej.is_empty());
     debug_assert!(!a.is_infinity());
 
     let d = a.double_var(None);
@@ -316,8 +314,9 @@ fn table_get_ge_const(r: &mut Affine, pre: &[Affine], n: i32, w: usize) {
     debug_assert!(n >= -((1 << (w - 1)) - 1));
     debug_assert!(n <= ((1 << (w - 1)) - 1));
     for m in 0..pre.len() {
-        r.x.cmov(&pre[m].x, m == idx_n as usize);
-        r.y.cmov(&pre[m].y, m == idx_n as usize);
+        let flag = m == idx_n as usize;
+        r.x.cmov(&pre[m].x, flag);
+        r.y.cmov(&pre[m].y, flag);
     }
     r.infinity = false;
     let neg_y = r.y.neg(1);

core/src/field.rs

@@ -394,7 +394,7 @@ impl Field {
 
         debug_assert!(t9 >> 23 == 0);
 
-        return z0 == 0 || z1 == 0x3ffffff;
+        z0 == 0 || z1 == 0x3ffffff
     }
 
     /// Verify whether a field element represents zero i.e. would
@@ -479,7 +479,7 @@ impl Field {
 
         debug_assert!(t9 >> 23 == 0);
 
-        return z0 == 0 || z1 == 0x3ffffff;
+        z0 == 0 || z1 == 0x3ffffff
     }
 
     /// Set a field element equal to a small integer. Resulting field
@@ -496,7 +496,7 @@ impl Field {
     pub fn is_zero(&self) -> bool {
         debug_assert!(self.normalized);
         debug_assert!(self.verify());
-        return (self.n[0]
+        (self.n[0]
             | self.n[1]
             | self.n[2]
             | self.n[3]
@@ -506,15 +506,15 @@ impl Field {
             | self.n[7]
             | self.n[8]
             | self.n[9])
-            == 0;
+            == 0
     }
 
     /// Check the "oddness" of a field element. Requires the input to
     /// be normalized.
     pub fn is_odd(&self) -> bool {
         debug_assert!(self.normalized);
         debug_assert!(self.verify());
-        return self.n[0] & 1 != 0;
+        self.n[0] & 1 != 0
     }
 
     /// Sets a field element equal to zero, initializing all fields.
@@ -578,7 +578,7 @@ impl Field {
         self.normalized = true;
         debug_assert!(self.verify());
 
-        return true;
+        true
     }
 
     pub fn fill_b32(&self, r: &mut [u8; 32]) {
@@ -694,13 +694,13 @@ impl Field {
                 return Ordering::Less;
             }
         }
-        return Ordering::Equal;
+        Ordering::Equal
     }
 
     pub fn eq_var(&self, other: &Field) -> bool {
         let mut na = self.neg(1);
         na += other;
-        return na.normalizes_to_zero_var();
+        na.normalizes_to_zero_var()
     }
 
     fn mul_inner(&mut self, a: &Field, b: &Field) {
@@ -1620,8 +1620,7 @@ impl Field {
         for _ in 0..2 {
             t1 = t1.sqr();
         }
-        let r = self * &t1;
-        r
+        self * &t1
     }
 
     /// Potentially faster version of secp256k1_fe_inv, without
@@ -1751,7 +1750,7 @@ impl PartialEq for Field {
     fn eq(&self, other: &Field) -> bool {
         let mut na = self.neg(self.magnitude);
         na += other;
-        return na.normalizes_to_zero();
+        na.normalizes_to_zero()
     }
 }
 

core/src/group.rs

@@ -126,7 +126,7 @@ impl Affine {
         if self.y.is_odd() != odd {
             self.y = self.y.neg(1);
         }
-        return true;
+        true
     }
 
     /// Check whether a group element is the point at infinity.
@@ -220,7 +220,7 @@ pub fn set_table_gej_var(r: &mut [Affine], a: &[Jacobian], zr: &[Field]) {
     let mut i = r.len() - 1;
     let mut zi: Field;
 
-    if r.len() > 0 {
+    if !r.is_empty() {
         zi = a[i].z.inv();
         r[i].set_gej_zinv(&a[i], &zi);
 
@@ -238,7 +238,7 @@ pub fn globalz_set_table_gej(r: &mut [Affine], globalz: &mut Field, a: &[Jacobia
     let mut i = r.len() - 1;
     let mut zs: Field;
 
-    if r.len() > 0 {
+    if !r.is_empty() {
         r[i].x = a[i].x;
         r[i].y = a[i].y;
         *globalz = a[i].z;
@@ -296,7 +296,7 @@ impl Jacobian {
         r *= x;
         let mut r2 = self.x;
         r2.normalize_weak();
-        return r.eq_var(&r2);
+        r.eq_var(&r2)
     }
 
     /// Set r equal to the inverse of a (i.e., mirrored around the X
@@ -328,7 +328,7 @@ impl Jacobian {
         }
 
         let yz = self.y * self.z;
-        return yz.is_quad_var();
+        yz.is_quad_var()
     }
 
     /// Set r equal to the double of a. If rzr is not-NULL, r->z =

core/src/lib.rs

@@ -1,5 +1,14 @@
 //! Core libraries for libsecp256k1.
 
+#![allow(
+    clippy::cast_ptr_alignment,
+    clippy::identity_op,
+    clippy::many_single_char_names,
+    clippy::needless_range_loop,
+    clippy::suspicious_op_assign_impl,
+    clippy::too_many_arguments,
+    clippy::type_complexity
+)]
 #![deny(
     unused_import_braces,
     unused_imports,

core/src/scalar.rs

@@ -57,12 +57,12 @@ impl Scalar {
         debug_assert!(count < 32);
         debug_assert!(offset + count <= 256);
         if (offset + count - 1) >> 5 == offset >> 5 {
-            return self.bits(offset, count);
+            self.bits(offset, count)
         } else {
             debug_assert!((offset >> 5) + 1 < 8);
-            return ((self.0[offset >> 5] >> (offset & 0x1f))
+            ((self.0[offset >> 5] >> (offset & 0x1f))
                 | (self.0[(offset >> 5) + 1] << (32 - (offset & 0x1f))))
-                & ((1 << count) - 1);
+                & ((1 << count) - 1)
         }
     }
 
@@ -283,7 +283,7 @@ impl Scalar {
         no |= Choice::from((self.0[1] < SECP256K1_N_H_1) as u8) & !yes;
         yes |= Choice::from((self.0[1] > SECP256K1_N_H_1) as u8) & !no;
         yes |= Choice::from((self.0[0] >= SECP256K1_N_H_0) as u8) & !no;
-        return yes.into();
+        yes.into()
     }
 
     /// Conditionally negate a number, in constant time.
@@ -749,8 +749,8 @@ impl Scalar {
         self.0[4] = (self.0[4] >> n) + (self.0[5] << (32 - n));
         self.0[5] = (self.0[5] >> n) + (self.0[6] << (32 - n));
         self.0[6] = (self.0[6] >> n) + (self.0[7] << (32 - n));
-        self.0[7] = self.0[7] >> n;
-        return ret;
+        self.0[7] >>= n;
+        ret
     }
 
     pub fn sqr_in_place(&mut self, a: &Scalar) {
@@ -922,7 +922,7 @@ impl Scalar {
     }
 
     pub fn is_even(&self) -> bool {
-        return self.0[0] & 1 == 0;
+        self.0[0] & 1 == 0
     }
 }
 

gen/genmult/src/lib.rs

@@ -1,4 +1,6 @@
-use libsecp256k1_core::curve::{AffineStorage, ECMultGenContext};
+#![allow(clippy::needless_range_loop)]
+
+use libsecp256k1_core::curve::ECMultGenContext;
 use std::{
     fs::File,
     io::{Error, Write},
@@ -12,7 +14,7 @@ pub fn generate_to(file: &mut File) -> Result<(), Error> {
     for j in 0..64 {
         file.write_fmt(format_args!("    ["))?;
         for i in 0..16 {
-            let pg: AffineStorage = prec[j][i].into();
+            let pg = prec[j][i];
             file.write_fmt(format_args!(
                 "        crate::curve::AffineStorage::new(crate::curve::FieldStorage::new({}, {}, {}, {}, {}, {}, {}, {}), crate::curve::FieldStorage::new({}, {}, {}, {}, {}, {}, {}, {})),",
                 pg.x.0[7], pg.x.0[6], pg.x.0[5], pg.x.0[4], pg.x.0[3], pg.x.0[2], pg.x.0[1], pg.x.0[0],