Skip to content

nostr: upgrade bitcoin_hashes to 1.x - #1419

Merged
yukibtc merged 1 commit into
nostrdevkit:masterfrom
JSKitty:bitcoin-hashes-1x
Aug 4, 2026
Merged

nostr: upgrade bitcoin_hashes to 1.x#1419
yukibtc merged 1 commit into
nostrdevkit:masterfrom
JSKitty:bitcoin-hashes-1x

Conversation

@JSKitty

@JSKitty JSKitty commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Bumps bitcoin_hashes from 0.14 to 1.1.

0.21.0 added SIMD SHA-256 Hardware Acceleration (ARMv8 crypto extensions, x86 SHA-NI, AVX2, SSE4.1), picked
by runtime feature detection with software fallback. The 0.14 line we currently pin is pure software on every target,

Measured on an Apple M4 Max:

0.14 1.1
SHA-256, 64 bytes 226 ns 34.7 ns
NIP-44 v2 encrypt, 32-byte plaintext 2870 ns 1091 ns
NIP-44 v2 decrypt, 32-byte plaintext 2734 ns 995 ns

Machines without the relevant extensions (every Raspberry Pi, pre-Ice Lake Intel) fall back to
software and are unaffected either way. 1.x also wipes secret data in HMAC and HKDF, which the
0.14 engines did not.

These API changes were needed, all introduced by 0.21.0:

  • HmacEngine<T> takes the engine type now instead of the hash type, so HmacEngine<sha256::Hash>
    becomes HmacEngine<sha256::HashEngine>.
  • Hmac::from_engine(engine) becomes engine.finalize(), since HmacEngine implements
    HashEngine and finalizes to Hmac<T::Hash>.
  • Hash::from_slice was removed, so ConversationKey::from_slice checks the length itself and
    calls Hash::from_byte_array.
  • sha256::Hash::hash is an inherent method now, so the Hash trait import it used to need is
    dead in a few places.

Notes to the reviewers

This is a breaking change, however, given my obsession with low-level optimisation (particularly on the NIP44-v2 crypto paths; which are very heavily used in Concord Protocol and Vector Messenger), I believe this is a worthwhile change.

Behaviour is unchanged. Checked with a differential harness rather than relying on the
existing suite: 64 conversation-key derivations, 27 plaintext lengths covering every padding
boundary up to the 65408 maximum, MAC tampering, ciphertext tampering, all ten rejection paths
compared by exact error string, and 64 on- and off-curve x-coordinates, all folded into one
order-sensitive digest. Identical before and after
(c95d027371418aa7d71c0d86ea7cec49, 30 off-curve rejected / 34 accepted). Happy to include that
harness as a test if you'd find it useful.

I have a follow-up PR that cuts allocations in the NIP-44 v2 path (stack HKDF expansion, one buffer
for the payload, and a single curve parse in the ECDH derive instead of two). It's independent of
this PR and I'd rather keep them separate, so I'll open it once this is settled.


This PR was created from a deep performance R&D project, including the use of AI for automated benchmarking, with the mission of maximizing performance for Vector and Concord Protocol, reviewed by myself, Claude Fable 5 and DeepSeek-v4-Flash, I am aware of the contribution guidelines and I fully understand the changes being proposed.

Thanks for your consideration, there are many areas of performance enhancement I'd like to upstream from Vector to the Nostr Rust SDK / NostrDevKit. 🙏

Checklist

  • I followed the contribution guidelines
  • I updated the relevant CHANGELOG.md (if applicable)
  • I understand and can explain all code in this PR

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@TheAwiteb TheAwiteb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, only couple changes

Comment thread nostr/src/nips/nip44/v2.rs Outdated
Comment thread nostr/CHANGELOG.md Outdated
Comment thread nostr/CHANGELOG.md Outdated
@JSKitty

JSKitty commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Handled the Codecov report, clippy, and applied your suggestions. 🙏 💚

@TheAwiteb

Copy link
Copy Markdown
Member

@JSKitty Request my review when you are done

@JSKitty
JSKitty requested a review from TheAwiteb August 3, 2026 22:47
@TheAwiteb

Copy link
Copy Markdown
Member

I have a follow-up PR that cuts allocations in the NIP-44 v2 path (stack HKDF expansion, one buffer for the payload, and a single curve parse in the ECDH derive instead of two). It's independent of this PR and I'd rather keep them separate, so I'll open it once this is settled.

Open a draft if you have something ready, we plan to release v0.45 in the next couple days. So we include your PR in the milestone, it's good one

@TheAwiteb TheAwiteb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Thanks

`bitcoin_hashes` 0.21.0 added SIMD SHA-256 backends (ARMv8 crypto
extensions, x86 SHA-NI, AVX2, SSE4.1) chosen by runtime feature detection
with a software fallback. The 0.14 line we pin is pure software on every
target.

The bump needs these API changes that 0.21.0 introduced:

- `HmacEngine<T>` takes the engine type now instead of the hash type, so
  `HmacEngine<sha256::Hash>` becomes `HmacEngine<sha256::HashEngine>`.
- `Hmac::from_engine(engine)` becomes `engine.finalize()`. `HmacEngine`
  implements `HashEngine` and finalizes to `Hmac<T::Hash>`.
- `Hash::from_slice` is gone, so `ConversationKey::from_slice` checks the
  length itself and calls `Hash::from_byte_array`.
- `sha256::Hash::hash` is an inherent method now, so the `Hash` trait
  import it needed is dead in a few places.

It breaks the public API wherever `bitcoin_hashes` types leak out:
`nip44::v2::ConversationKey` via its `Deref` to `Hmac<sha256::Hash>`, the
`nip94::FileMetadata` hash fields and their builders, and the
`nip98::HttpData` payload. `EventId` is unaffected, it holds a plain
`[u8; 32]`.

1.x also wipes secret data in HMAC and HKDF. The 0.14 engines did not.

Measured on an Apple M4: SHA-256 of 64 bytes goes from 226ns to 34.7ns.
NIP-44 v2 encrypt of a 32-byte plaintext goes from 2870ns to 1091ns,
decrypt from 2734ns to 995ns.

Output is unchanged. A differential harness covering conversation key
derivation, every padding boundary, both tamper paths and all ten
rejection error strings produces the same digest before and after.

One caveat: `bip39` still depends on `bitcoin_hashes` 0.14, so building
with `nip06` pulls in both versions. Nothing crosses that boundary, since
`Mnemonic::to_seed_normalized` returns `[u8; 64]`, and a stripped test
binary exercising both paths grew by 16 bytes.

Pull-Request: nostrdevkit#1419
Acked-by: Awiteb <a@4rs.nl>
Acked-by: Yuki Kishimoto <yukikishimoto@protonmail.com>
Signed-off-by: Yuki Kishimoto <yukikishimoto@protonmail.com>

@yukibtc yukibtc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@yukibtc
yukibtc force-pushed the bitcoin-hashes-1x branch from b176402 to 8c7e9f6 Compare August 4, 2026 06:54
@yukibtc
yukibtc merged commit 8c7e9f6 into nostrdevkit:master Aug 4, 2026
42 checks passed
@yukibtc

yukibtc commented Aug 4, 2026

Copy link
Copy Markdown
Member

Squashed and merged at 8c7e9f6

@JSKitty
JSKitty deleted the bitcoin-hashes-1x branch August 4, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants