Skip to content

Commit 748327a

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 748327a

File tree

2 files changed

+27
-23
lines changed

2 files changed

+27
-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: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,31 +11,38 @@ 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;
17+
use secp256k1_sys::secp256k1_ecdh_hash_function_default;
1818

19-
let tag = "rust-secp256k1DEBUG";
19+
struct Hex16Writer([u8; 32]);
2020

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

28-
f.debug_tuple(stringify!($thing)).field(&format_args!("#{:.16}", hash)).finish()
29-
}
30-
}
36+
impl fmt::Debug for Hex16Writer {
37+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
38+
for byte in self.0.iter().copied().take(8) {
39+
write!(f, "{:02x}", byte)?;
40+
}
41+
Ok(())
42+
}
43+
}
3144

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-
)
45+
f.debug_tuple(stringify!($thing)).field(&output).finish()
3946
}
4047
}
4148
};

0 commit comments

Comments
 (0)