Merge rust-bitcoin/rust-secp256k1#364: Add lints to catch missing traits

69f44d9301 Manually implement Debug for SerializedSignature (Tobin Harding)
26921a31b8 Add lints to catch missing traits (Tobin Harding)
35556e22f2 Remove useless call to format (Tobin Harding)
0ad414a982 Remove unneeded return statements (Tobin Harding)

Pull request description:

  We can use the linters to help us catch type definitions that are missing 'standard' derives. 'standard' is project defined to be

  - Copy
  - Clone
  - Debug
  - PartialEq and Eq
  - PartialOrd and Ord
  - Hash

  (I've assumed this to be true based on the code and an open [PR](https://github.com/rust-bitcoin/rust-bitcoin/pull/587) in rust-bitcoin.)

  While neither Rustc nor Clippy can find all of these, Rustc can warn for missing `Copy` and `Debug` implementations and these warnings can assist us find types that may need additional derives.

  First two patches are trivial Clippy fixes in preparation for using the linter to improve type definitions crate wide.

  Patch 3 adds
  ```
  #![warn(missing_copy_implementations)]
  #![warn(missing_debug_implementations)]
  ```
  and fixes newly emitted warnings.

ACKs for top commit:
  thomaseizinger:
    ACK 69f44d9301
  apoelstra:
    ACK 69f44d9301

Tree-SHA512: 18f2c52d207f962ef7d6749a57a35e48eb18a18fac82d4df4ff3dce549b69661cb27f66c4cae516ae5477f5b919d9197f70a5c924955605c73f8545f430c3b42

src/context.rs

@@ -69,16 +69,19 @@ pub trait Signing: Context {}
 pub trait Verification: Context {}
 
 /// Represents the set of capabilities needed for signing with a user preallocated memory.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct SignOnlyPreallocated<'buf> {
     phantom: PhantomData<&'buf ()>,
 }
 
 /// Represents the set of capabilities needed for verification with a user preallocated memory.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct VerifyOnlyPreallocated<'buf> {
     phantom: PhantomData<&'buf ()>,
 }
 
 /// Represents the set of all capabilities with a user preallocated memory.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct AllPreallocated<'buf> {
     phantom: PhantomData<&'buf ()>,
 }
@@ -112,14 +115,17 @@ mod alloc_only {
 
     /// Represents the set of capabilities needed for signing.
     #[cfg_attr(docsrs, doc(cfg(any(feature = "std", feature = "alloc"))))]
+    #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
     pub enum SignOnly {}
 
     /// Represents the set of capabilities needed for verification.
     #[cfg_attr(docsrs, doc(cfg(any(feature = "std", feature = "alloc"))))]
+    #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
     pub enum VerifyOnly {}
 
     /// Represents the set of all capabilities.
     #[cfg_attr(docsrs, doc(cfg(any(feature = "std", feature = "alloc"))))]
+    #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
     pub enum All {}
 
     impl Signing for SignOnly {}

src/ecdsa/mod.rs

@@ -32,7 +32,19 @@ impl fmt::Debug for Signature {
 impl fmt::Display for Signature {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let sig = self.serialize_der();
-        for v in sig.iter() {
+        sig.fmt(f)
+    }
+}
+
+impl fmt::Debug for SerializedSignature {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(self, f)
+    }
+}
+
+impl fmt::Display for SerializedSignature {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        for v in self.data.iter().take(self.len) {
             write!(f, "{:02x}", v)?;
         }
         Ok(())
@@ -386,7 +398,7 @@ impl<C: Signing> Secp256k1<C> {
     /// Requires a signing capable context.
     pub fn sign_ecdsa_grind_r(&self, msg: &Message, sk: &SecretKey, bytes_to_grind: usize) -> Signature {
         let len_check = |s : &ffi::Signature| der_length_check(s, 71 - bytes_to_grind);
-        return self.sign_grind_with_check(msg, sk, len_check);
+        self.sign_grind_with_check(msg, sk, len_check)
     }
 
     /// Constructs a signature for `msg` using the secret key `sk`, RFC6979 nonce
@@ -397,7 +409,7 @@ impl<C: Signing> Secp256k1<C> {
     /// Requires a signing capable context.
     #[deprecated(since = "0.21.0", note = "Use sign_ecdsa_grind_r instead.")]
     pub fn sign_low_r(&self, msg: &Message, sk: &SecretKey) -> Signature {
-        return self.sign_grind_with_check(msg, sk, compact_sig_has_zero_first_bit)
+        self.sign_grind_with_check(msg, sk, compact_sig_has_zero_first_bit)
     }
 
     /// Constructs a signature for `msg` using the secret key `sk`, RFC6979 nonce
@@ -407,7 +419,7 @@ impl<C: Signing> Secp256k1<C> {
     /// will perform two signing operations.
     /// Requires a signing capable context.
     pub fn sign_ecdsa_low_r(&self, msg: &Message, sk: &SecretKey) -> Signature {
-        return self.sign_grind_with_check(msg, sk, compact_sig_has_zero_first_bit)
+        self.sign_grind_with_check(msg, sk, compact_sig_has_zero_first_bit)
     }
 }
 

src/key.rs

@@ -511,7 +511,8 @@ impl Ord for PublicKey {
 }
 
 /// Opaque data structure that holds a keypair consisting of a secret and a public key.
-#[derive(Clone)]
+// Should secrets implement Copy: https://github.com/rust-bitcoin/rust-secp256k1/issues/363
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct KeyPair(ffi::KeyPair);
 impl_display_secret!(KeyPair);
 
@@ -1084,7 +1085,7 @@ mod test {
         // Zero
         assert_eq!(SecretKey::from_slice(&[0; 32]), Err(InvalidSecretKey));
         assert_eq!(
-            SecretKey::from_str(&format!("0000000000000000000000000000000000000000000000000000000000000000")),
+            SecretKey::from_str("0000000000000000000000000000000000000000000000000000000000000000"),
             Err(InvalidSecretKey)
         );
         // -1

src/lib.rs

@@ -132,6 +132,8 @@
 #![deny(non_snake_case)]
 #![deny(unused_mut)]
 #![warn(missing_docs)]
+#![warn(missing_copy_implementations)]
+#![warn(missing_debug_implementations)]
 
 
 #![cfg_attr(all(not(test), not(feature = "std")), no_std)]

src/secret.rs

@@ -57,6 +57,7 @@ macro_rules! impl_display_secret {
 ///
 /// [`Display`]: fmt::Display
 /// [`Debug`]: fmt::Debug
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct DisplaySecret {
     secret: [u8; SECRET_KEY_SIZE]
 }