Skip to content

Commit 54828ac

Browse files
committed
secret: stop using "hashes" dependency for secret debug output
This is the only place we use a sha256 hash. We were using the 'hashes' crate which is a whole extra dependency just to bring in a hash function that already exists in libsecp256k1. libsecp256k1 doesn't expose sha256 directly, but they do expose default hash functions for EDCH, ECDSA and Schnorr, which are just sha2s with extra steps. Since we are not worried about having a specific canonical hash, just one that won't change for different keys, do this. (This change does mean that users will get different hashes when they upgrade this library; but they really shouldn't be persisting Debug output and expecting it to be consistent. I don't think this is a meaningful breaking change.)
1 parent fc912cb commit 54828ac

File tree

2 files changed

+28
-23
lines changed

2 files changed

+28
-23
lines changed

src/key.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1894,10 +1894,7 @@ mod test {
18941894
let s = Secp256k1::new();
18951895
let (sk, _) = s.generate_keypair(&mut StepRng::new(1, 1));
18961896

1897-
assert_eq!(
1898-
&format!("{:?}", sk),
1899-
"<secret key; enable `hashes` feature of `secp256k1` to display fingerprint>"
1900-
);
1897+
assert_eq!(&format!("{:?}", sk), "SecretKey(7ad2d060fb2971d6)");
19011898

19021899
let mut buf = [0u8; constants::SECRET_KEY_SIZE * 2];
19031900
assert_eq!(

src/secret.rs

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,31 +11,39 @@ use crate::to_hex;
1111
macro_rules! impl_display_secret {
1212
// Default hasher exists only in standard library and not alloc
1313
($thing:ident) => {
14-
#[cfg(feature = "hashes")]
1514
impl ::core::fmt::Debug for $thing {
1615
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
17-
use hashes::{sha256, Hash, HashEngine};
16+
use core::fmt;
1817

19-
let tag = "rust-secp256k1DEBUG";
18+
use secp256k1_sys::secp256k1_ecdh_hash_function_default;
2019

21-
let mut engine = sha256::Hash::engine();
22-
let tag_hash = sha256::Hash::hash(tag.as_bytes());
23-
engine.input(&tag_hash.as_ref());
24-
engine.input(&tag_hash.as_ref());
25-
engine.input(&self.secret_bytes());
26-
let hash = sha256::Hash::from_engine(engine);
20+
struct Hex16Writer([u8; 32]);
2721

28-
f.debug_tuple(stringify!($thing)).field(&format_args!("#{:.16}", hash)).finish()
29-
}
30-
}
22+
let mut output = Hex16Writer([0u8; 32]);
23+
unsafe {
24+
// SAFETY: this function pointer is an Option<fn> so we need to unwrap it, even
25+
// though we know that the FFI has provided a non-null value for us.
26+
let ecdh_hash = secp256k1_ecdh_hash_function_default.unwrap();
27+
// SAFETY: output points to >= 32 mutable bytes; both inputs are >= 32 bytes
28+
assert!(self.as_ref().len() >= 32);
29+
ecdh_hash(
30+
output.0.as_mut_ptr(),
31+
self.as_ref().as_ptr(),
32+
"a debug-only byte 'y coordinate'".as_ptr(),
33+
core::ptr::null_mut(),
34+
);
35+
}
3136

32-
#[cfg(not(feature = "hashes"))]
33-
impl ::core::fmt::Debug for $thing {
34-
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
35-
write!(
36-
f,
37-
"<secret key; enable `hashes` feature of `secp256k1` to display fingerprint>"
38-
)
37+
impl fmt::Debug for Hex16Writer {
38+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
39+
for byte in self.0.iter().copied().take(8) {
40+
write!(f, "{:02x}", byte)?;
41+
}
42+
Ok(())
43+
}
44+
}
45+
46+
f.debug_tuple(stringify!($thing)).field(&output).finish()
3947
}
4048
}
4149
};

0 commit comments

Comments
 (0)