rust-bitcoin / rust-secp256k1

Rust language bindings for Bitcoin secp256k1 library.
Creative Commons Zero v1.0 Universal
350 stars 270 forks source link

key: don't use `Hasher` to generate fingerprints; just use `hashes` crate #726

Closed apoelstra closed 2 months ago

apoelstra commented 2 months ago

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 https://github.com/rust-bitcoin/rust-bitcoin/pull/3184 and the existing stuff is neither faster nor less code than what's currently done. So we'll live with it.

Fixes #725

tcharding commented 2 months ago

conceptACK. You missed setting the PR title.

apoelstra commented 2 months ago

CI failure looks like some unrelated xargo thing. We definitely gotta start pinning nightly here.