Merge #726: key: don't use Hasher to generate fingerprints; just us…

…e `hashes` crate

b8ac971 keypair: use public key for Debug output (Andrew Poelstra)
a16e5ec secret keys: debug output only when `hashes` is enabled (Andrew Poelstra)

Pull request description:

  In addition to changing `SecretKey` and `SharedSecret` to use `hashes`, we also unconditionally use the public half of `KeyPair` as a fingerprint, since that's always available and does not need extra deps.

  This patches the existing unit tests but doesn't add more. Maybe they should be removed; it's a bit weird to have unit tests for `Debug` output. But in this case we're doing some nontrivial logic and I guess we wanted to double-check that it was taking effect.

  I'd also like to change the manual tagged-hash implementation to use `bitcoin_hashes` methods but those are under construction rust-bitcoin/rust-bitcoin#3184 and the existing stuff is neither faster nor less code than what's currently done. So we'll live with it.

  Fixes #725

ACKs for top commit:
  Kixunil:
    ACK b8ac971

Tree-SHA512: d0a65e0a0069bcbc663c1d3e7f98b75868355c4db48e9a9c905cdcd2af1606ac86090cdf0aae5caa23337c5d565e6420d7c956dd0a65a1877004840075bc08e9

src/key.rs

@@ -772,7 +772,6 @@ impl<'de> serde::Deserialize<'de> for PublicKey {
 /// [`cbor`]: https://docs.rs/cbor
 #[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
 pub struct Keypair(ffi::Keypair);
-impl_display_secret!(Keypair);
 impl_fast_comparisons!(Keypair);
 
 impl Keypair {
@@ -972,6 +971,15 @@ impl Keypair {
     pub fn non_secure_erase(&mut self) { self.0.non_secure_erase(); }
 }
 
+impl fmt::Debug for Keypair {
+    fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
+        f.debug_struct("Keypair")
+            .field("pubkey", &self.public_key())
+            .field("secret", &"<hidden>")
+            .finish()
+    }
+}
+
 impl From<Keypair> for SecretKey {
     #[inline]
     fn from(pair: Keypair) -> Self { SecretKey::from_keypair(&pair) }
@@ -1705,12 +1713,15 @@ mod test {
     }
 
     #[test]
-    #[cfg(all(feature = "rand", feature = "alloc"))]
+    #[cfg(all(feature = "rand", feature = "alloc", not(feature = "hashes")))]
     fn test_debug_output() {
         let s = Secp256k1::new();
         let (sk, _) = s.generate_keypair(&mut StepRng::new(1, 1));
 
-        assert_eq!(&format!("{:?}", sk), "SecretKey(#d3e0c51a23169bb5)");
+        assert_eq!(
+            &format!("{:?}", sk),
+            "<secret key; enable `hashes` feature of `secp256k1` to display fingerprint>"
+        );
 
         let mut buf = [0u8; constants::SECRET_KEY_SIZE * 2];
         assert_eq!(

src/secret.rs

@@ -11,28 +11,7 @@ use crate::to_hex;
 macro_rules! impl_display_secret {
     // Default hasher exists only in standard library and not alloc
     ($thing:ident) => {
-        #[cfg(feature = "std")]
-        impl core::fmt::Debug for $thing {
-            fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
-                use core::hash::Hasher;
-                const DEBUG_HASH_TAG: &[u8] = &[
-                    0x66, 0xa6, 0x77, 0x1b, 0x9b, 0x6d, 0xae, 0xa1, 0xb2, 0xee, 0x4e, 0x07, 0x49,
-                    0x4a, 0xac, 0x87, 0xa9, 0xb8, 0x5b, 0x4b, 0x35, 0x02, 0xaa, 0x6d, 0x0f, 0x79,
-                    0xcb, 0x63, 0xe6, 0xf8, 0x66, 0x22,
-                ]; // =SHA256(b"rust-secp256k1DEBUG");
-
-                let mut hasher = std::collections::hash_map::DefaultHasher::new();
-
-                hasher.write(DEBUG_HASH_TAG);
-                hasher.write(DEBUG_HASH_TAG);
-                hasher.write(&self.secret_bytes());
-                let hash = hasher.finish();
-
-                f.debug_tuple(stringify!($thing)).field(&format_args!("#{:016x}", hash)).finish()
-            }
-        }
-
-        #[cfg(all(not(feature = "std"), feature = "hashes"))]
+        #[cfg(feature = "hashes")]
         impl ::core::fmt::Debug for $thing {
             fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
                 use hashes::{sha256, Hash, HashEngine};
@@ -50,10 +29,13 @@ macro_rules! impl_display_secret {
             }
         }
 
-        #[cfg(all(not(feature = "std"), not(feature = "hashes")))]
+        #[cfg(not(feature = "hashes"))]
         impl ::core::fmt::Debug for $thing {
             fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
-                write!(f, "<secret requires std or hashes feature to display>")
+                write!(
+                    f,
+                    "<secret key; enable `hashes` feature of `secp256k1` to display fingerprint>"
+                )
             }
         }
     };