Fix all Clippy warnings (#244)

- Add Clippy to CI
- Rename InternalError variants without redundant Error suffix
- Rename to_bytes to as_bytes on well known naming
- Fix Redundant refs
- Fix redundant lifetimes
- Fix late declarations

.github/workflows/rust.yml

@@ -67,3 +67,13 @@ jobs:
     - uses: actions/checkout@v3
     - uses: dtolnay/rust-toolchain@stable
     - run: cargo build --benches --features batch
+
+  clippy:
+    name: Check that clippy is happy
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: dtolnay/rust-toolchain@1.65
+        with:
+          components: clippy
+      - run: cargo clippy

src/batch.rs

@@ -218,7 +218,7 @@ pub fn verify_batch(
         || signatures.len() != verifying_keys.len()
         || verifying_keys.len() != messages.len()
     {
-        return Err(InternalError::ArrayLengthError {
+        return Err(InternalError::ArrayLength {
             name_a: "signatures",
             length_a: signatures.len(),
             name_b: "messages",
@@ -292,11 +292,11 @@ pub fn verify_batch(
         once(-B_coefficient).chain(zs.iter().cloned()).chain(zhrams),
         B.chain(Rs).chain(As),
     )
-    .ok_or(InternalError::VerifyError)?;
+    .ok_or(InternalError::Verify)?;
 
     if id.is_identity() {
         Ok(())
     } else {
-        Err(InternalError::VerifyError.into())
+        Err(InternalError::Verify.into())
     }
 }

src/errors.rs

@@ -23,23 +23,23 @@ use std::error::Error;
 /// need to pay any attention to these.
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 pub(crate) enum InternalError {
-    PointDecompressionError,
+    PointDecompression,
-    ScalarFormatError,
+    ScalarFormat,
     /// An error in the length of bytes handed to a constructor.
     ///
     /// To use this, pass a string specifying the `name` of the type which is
     /// returning the error, and the `length` in bytes which its constructor
     /// expects.
-    BytesLengthError {
+    BytesLength {
         name: &'static str,
         length: usize,
     },
     /// The verification equation wasn't satisfied
-    VerifyError,
+    Verify,
     /// Two arrays did not match in size, making the called signature
     /// verification method impossible.
     #[cfg(any(feature = "batch", feature = "batch_deterministic"))]
-    ArrayLengthError {
+    ArrayLength {
         name_a: &'static str,
         length_a: usize,
         name_b: &'static str,
@@ -48,22 +48,22 @@ pub(crate) enum InternalError {
         length_c: usize,
     },
     /// An ed25519ph signature can only take up to 255 octets of context.
-    PrehashedContextLengthError,
+    PrehashedContextLength,
     /// A mismatched (public, secret) key pair.
-    MismatchedKeypairError,
+    MismatchedKeypair,
 }
 
 impl Display for InternalError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
-            InternalError::PointDecompressionError => write!(f, "Cannot decompress Edwards point"),
+            InternalError::PointDecompression => write!(f, "Cannot decompress Edwards point"),
-            InternalError::ScalarFormatError => write!(f, "Cannot use scalar with high-bit set"),
+            InternalError::ScalarFormat => write!(f, "Cannot use scalar with high-bit set"),
-            InternalError::BytesLengthError { name: n, length: l } => {
+            InternalError::BytesLength { name: n, length: l } => {
                 write!(f, "{} must be {} bytes in length", n, l)
             }
-            InternalError::VerifyError => write!(f, "Verification equation was not satisfied"),
+            InternalError::Verify => write!(f, "Verification equation was not satisfied"),
             #[cfg(any(feature = "batch", feature = "batch_deterministic"))]
-            InternalError::ArrayLengthError {
+            InternalError::ArrayLength {
                 name_a: na,
                 length_a: la,
                 name_b: nb,
@@ -76,11 +76,11 @@ impl Display for InternalError {
                               {} has length {}, {} has length {}.",
                 na, la, nb, lb, nc, lc
             ),
-            InternalError::PrehashedContextLengthError => write!(
+            InternalError::PrehashedContextLength => write!(
                 f,
                 "An ed25519ph signature can only take up to 255 octets of context"
             ),
-            InternalError::MismatchedKeypairError => write!(f, "Mismatched Keypair detected"),
+            InternalError::MismatchedKeypair => write!(f, "Mismatched Keypair detected"),
         }
     }
 }

src/signature.rs

@@ -73,7 +73,7 @@ fn check_scalar(bytes: [u8; 32]) -> Result<Scalar, SignatureError> {
     // This is compatible with ed25519-donna and libsodium when
     // -DED25519_COMPAT is NOT specified.
     if bytes[31] & 224 != 0 {
-        return Err(InternalError::ScalarFormatError.into());
+        return Err(InternalError::ScalarFormat.into());
     }
 
     Ok(Scalar::from_bits(bytes))
@@ -95,15 +95,15 @@ fn check_scalar(bytes: [u8; 32]) -> Result<Scalar, SignatureError> {
     }
 
     match Scalar::from_canonical_bytes(bytes).into() {
-        None => return Err(InternalError::ScalarFormatError.into()),
+        None => Err(InternalError::ScalarFormat.into()),
-        Some(x) => return Ok(x),
+        Some(x) => Ok(x),
-    };
+    }
 }
 
 impl InternalSignature {
     /// Convert this `Signature` to a byte array.
     #[inline]
-    pub fn to_bytes(&self) -> [u8; SIGNATURE_LENGTH] {
+    pub fn as_bytes(&self) -> [u8; SIGNATURE_LENGTH] {
         let mut signature_bytes: [u8; SIGNATURE_LENGTH] = [0u8; SIGNATURE_LENGTH];
 
         signature_bytes[..32].copy_from_slice(&self.R.as_bytes()[..]);
@@ -182,6 +182,6 @@ impl TryFrom<&ed25519::Signature> for InternalSignature {
 
 impl From<InternalSignature> for ed25519::Signature {
     fn from(sig: InternalSignature) -> ed25519::Signature {
-        ed25519::Signature::from_bytes(&sig.to_bytes()).unwrap()
+        ed25519::Signature::from_bytes(&sig.as_bytes()).unwrap()
     }
 }

src/signing.rs

@@ -124,7 +124,7 @@ impl SigningKey {
         let verifying_key = VerifyingKey::from_bytes(verifying_key.try_into().unwrap())?;
 
         if verifying_key != VerifyingKey::from(&secret_key) {
-            return Err(InternalError::MismatchedKeypairError.into());
+            return Err(InternalError::MismatchedKeypair.into());
         }
 
         Ok(SigningKey {
@@ -300,9 +300,7 @@ impl SigningKey {
     {
         let expanded: ExpandedSecretKey = (&self.secret_key).into(); // xxx thanks i hate this
 
-        expanded
+        expanded.sign_prehashed(prehashed_message, &self.verifying_key, context)
-            .sign_prehashed(prehashed_message, &self.verifying_key, context)
-            .into()
     }
 
     /// Verify a signature on a message with this signing key's public key.
@@ -473,7 +471,7 @@ impl Signer<ed25519::Signature> for SigningKey {
     /// Sign a message with this signing key's secret key.
     fn try_sign(&self, message: &[u8]) -> Result<ed25519::Signature, SignatureError> {
         let expanded: ExpandedSecretKey = (&self.secret_key).into();
-        Ok(expanded.sign(&message, &self.verifying_key).into())
+        Ok(expanded.sign(message, &self.verifying_key))
     }
 }
 
@@ -505,7 +503,7 @@ impl TryFrom<&[u8]> for SigningKey {
         SecretKey::try_from(bytes)
             .map(|bytes| Self::from_bytes(&bytes))
             .map_err(|_| {
-                InternalError::BytesLengthError {
+                InternalError::BytesLength {
                     name: "SecretKey",
                     length: SECRET_KEY_LENGTH,
                 }
@@ -695,24 +693,20 @@ impl ExpandedSecretKey {
     #[allow(non_snake_case)]
     pub(crate) fn sign(&self, message: &[u8], verifying_key: &VerifyingKey) -> ed25519::Signature {
         let mut h: Sha512 = Sha512::new();
-        let R: CompressedEdwardsY;
-        let r: Scalar;
-        let s: Scalar;
-        let k: Scalar;
 
-        h.update(&self.nonce);
+        h.update(self.nonce);
-        h.update(&message);
+        h.update(message);
 
-        r = Scalar::from_hash(h);
+        let r = Scalar::from_hash(h);
-        R = (&r * &ED25519_BASEPOINT_TABLE).compress();
+        let R: CompressedEdwardsY = (&r * &ED25519_BASEPOINT_TABLE).compress();
 
         h = Sha512::new();
         h.update(R.as_bytes());
         h.update(verifying_key.as_bytes());
-        h.update(&message);
+        h.update(message);
 
-        k = Scalar::from_hash(h);
+        let k = Scalar::from_hash(h);
-        s = &(&k * &self.key) + &r;
+        let s: Scalar = (k * self.key) + r;
 
         InternalSignature { R, s }.into()
     }
@@ -749,17 +743,11 @@ impl ExpandedSecretKey {
     {
         let mut h: Sha512;
         let mut prehash: [u8; 64] = [0u8; 64];
-        let R: CompressedEdwardsY;
-        let r: Scalar;
-        let s: Scalar;
-        let k: Scalar;
 
         let ctx: &[u8] = context.unwrap_or(b""); // By default, the context is an empty string.
 
         if ctx.len() > 255 {
-            return Err(SignatureError::from(
+            return Err(SignatureError::from(InternalError::PrehashedContextLength));
-                InternalError::PrehashedContextLengthError,
-            ));
         }
 
         let ctx_len: u8 = ctx.len() as u8;
@@ -781,26 +769,26 @@ impl ExpandedSecretKey {
         // still bleeding from malleability, for fuck's sake.
         h = Sha512::new()
             .chain_update(b"SigEd25519 no Ed25519 collisions")
-            .chain_update(&[1]) // Ed25519ph
+            .chain_update([1]) // Ed25519ph
-            .chain_update(&[ctx_len])
+            .chain_update([ctx_len])
             .chain_update(ctx)
-            .chain_update(&self.nonce)
+            .chain_update(self.nonce)
             .chain_update(&prehash[..]);
 
-        r = Scalar::from_hash(h);
+        let r = Scalar::from_hash(h);
-        R = (&r * &ED25519_BASEPOINT_TABLE).compress();
+        let R: CompressedEdwardsY = (&r * &ED25519_BASEPOINT_TABLE).compress();
 
         h = Sha512::new()
             .chain_update(b"SigEd25519 no Ed25519 collisions")
-            .chain_update(&[1]) // Ed25519ph
+            .chain_update([1]) // Ed25519ph
-            .chain_update(&[ctx_len])
+            .chain_update([ctx_len])
             .chain_update(ctx)
             .chain_update(R.as_bytes())
             .chain_update(verifying_key.as_bytes())
             .chain_update(&prehash[..]);
 
-        k = Scalar::from_hash(h);
+        let k = Scalar::from_hash(h);
-        s = &(&k * &self.key) + &r;
+        let s: Scalar = (k * self.key) + r;
 
         Ok(InternalSignature { R, s }.into())
     }

src/verifying.rs

@@ -90,7 +90,7 @@ impl VerifyingKey {
 
     /// View this public key as a byte array.
     #[inline]
-    pub fn as_bytes<'a>(&'a self) -> &'a [u8; PUBLIC_KEY_LENGTH] {
+    pub fn as_bytes(&self) -> &[u8; PUBLIC_KEY_LENGTH] {
         &(self.0).0
     }
 
@@ -133,7 +133,7 @@ impl VerifyingKey {
         let compressed = CompressedEdwardsY(*bytes);
         let point = compressed
             .decompress()
-            .ok_or(InternalError::PointDecompressionError)?;
+            .ok_or(InternalError::PointDecompression)?;
 
         Ok(VerifyingKey(compressed, point))
     }
@@ -185,8 +185,6 @@ impl VerifyingKey {
         let signature = InternalSignature::try_from(signature)?;
 
         let mut h: Sha512 = Sha512::default();
-        let R: EdwardsPoint;
-        let k: Scalar;
 
         let ctx: &[u8] = context.unwrap_or(b"");
         debug_assert!(
@@ -197,20 +195,21 @@ impl VerifyingKey {
         let minus_A: EdwardsPoint = -self.1;
 
         h.update(b"SigEd25519 no Ed25519 collisions");
-        h.update(&[1]); // Ed25519ph
+        h.update([1]); // Ed25519ph
-        h.update(&[ctx.len() as u8]);
+        h.update([ctx.len() as u8]);
         h.update(ctx);
         h.update(signature.R.as_bytes());
         h.update(self.as_bytes());
         h.update(prehashed_message.finalize().as_slice());
 
-        k = Scalar::from_hash(h);
+        let k = Scalar::from_hash(h);
-        R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s);
+        let R: EdwardsPoint =
+            EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s);
 
         if R.compress() == signature.R {
             Ok(())
         } else {
-            Err(InternalError::VerifyError.into())
+            Err(InternalError::Verify.into())
         }
     }
 
@@ -285,32 +284,30 @@ impl VerifyingKey {
         let signature = InternalSignature::try_from(signature)?;
 
         let mut h: Sha512 = Sha512::new();
-        let R: EdwardsPoint;
-        let k: Scalar;
         let minus_A: EdwardsPoint = -self.1;
-        let signature_R: EdwardsPoint;
 
-        match signature.R.decompress() {
+        let signature_R: EdwardsPoint = match signature.R.decompress() {
-            None => return Err(InternalError::VerifyError.into()),
+            None => return Err(InternalError::Verify.into()),
-            Some(x) => signature_R = x,
+            Some(x) => x,
-        }
+        };
 
         // Logical OR is fine here as we're not trying to be constant time.
         if signature_R.is_small_order() || self.1.is_small_order() {
-            return Err(InternalError::VerifyError.into());
+            return Err(InternalError::Verify.into());
         }
 
         h.update(signature.R.as_bytes());
         h.update(self.as_bytes());
-        h.update(&message);
+        h.update(message);
 
-        k = Scalar::from_hash(h);
+        let k = Scalar::from_hash(h);
-        R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s);
+        let R: EdwardsPoint =
+            EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s);
 
         if R == signature_R {
             Ok(())
         } else {
-            Err(InternalError::VerifyError.into())
+            Err(InternalError::Verify.into())
         }
     }
 }
@@ -326,21 +323,20 @@ impl Verifier<ed25519::Signature> for VerifyingKey {
         let signature = InternalSignature::try_from(signature)?;
 
         let mut h: Sha512 = Sha512::new();
-        let R: EdwardsPoint;
-        let k: Scalar;
         let minus_A: EdwardsPoint = -self.1;
 
         h.update(signature.R.as_bytes());
         h.update(self.as_bytes());
-        h.update(&message);
+        h.update(message);
 
-        k = Scalar::from_hash(h);
+        let k = Scalar::from_hash(h);
-        R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s);
+        let R: EdwardsPoint =
+            EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s);
 
         if R.compress() == signature.R {
             Ok(())
         } else {
-            Err(InternalError::VerifyError.into())
+            Err(InternalError::Verify.into())
         }
     }
 }
@@ -350,17 +346,14 @@ impl TryFrom<&[u8]> for VerifyingKey {
 
     #[inline]
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
-        let bytes = bytes.try_into().map_err(|_| {
+        let bytes = bytes.try_into().map_err(|_| InternalError::BytesLength {
-            InternalError::BytesLengthError {
+            name: "VerifyingKey",
-                name: "VerifyingKey",
+            length: PUBLIC_KEY_LENGTH,
-                length: PUBLIC_KEY_LENGTH,
-            }
         })?;
         Self::from_bytes(bytes)
     }
 }
 
-
 #[cfg(feature = "pkcs8")]
 impl DecodePublicKey for VerifyingKey {}
 

tests/ed25519.rs

@@ -66,8 +66,7 @@ mod vectors {
             let pub_bytes = &pub_bytes[..PUBLIC_KEY_LENGTH].try_into().unwrap();
 
             let signing_key = SigningKey::from_bytes(sec_bytes);
-            let expected_verifying_key =
+            let expected_verifying_key = VerifyingKey::from_bytes(pub_bytes).unwrap();
-                VerifyingKey::from_bytes(pub_bytes).unwrap();
             assert_eq!(expected_verifying_key, signing_key.verifying_key());
 
             // The signatures in the test vectors also include the message
@@ -93,8 +92,7 @@ mod vectors {
         let sig_bytes = hex!("98a70222f0b8121aa9d30f813d683f809e462b469c7ff87639499bb94e6dae4131f85042463c2a355a2003d062adf5aaa10b8c61e636062aaad11c2a26083406");
 
         let signing_key = SigningKey::from_bytes(&sec_bytes);
-        let expected_verifying_key =
+        let expected_verifying_key = VerifyingKey::from_bytes(&pub_bytes).unwrap();
-            VerifyingKey::from_bytes(&pub_bytes).unwrap();
         assert_eq!(expected_verifying_key, signing_key.verifying_key());
         let sig1 = Signature::try_from(&sig_bytes[..]).unwrap();