nostr: reduce allocations in the NIP-44 v2 message path - #1421
Merged
Conversation
JSKitty
force-pushed
the
nip44-allocations
branch
from
August 3, 2026 23:51
4d3fa24 to
0f48c16
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
JSKitty
force-pushed
the
nip44-allocations
branch
from
August 4, 2026 00:03
0f48c16 to
2ccb447
Compare
Contributor
Author
|
Coverage note: the remaining three lines are test-only. Two are the length guards inside The two real misses in Good for review! 🙏 |
Member
|
@JSKitty, can you rebase on master? |
Three sources of per-message allocation, none of which the algorithm needs: HKDF-expand built a `Vec` for the output and another for each block of the PRF chain. `expand_into` fills a caller-owned buffer instead, so the 76-byte message keys now come off the stack, and the T(i-1) | info | i concatenation is fed to the engine part by part rather than assembled in a buffer first. The keyed engine also depends only on the PRK, so it is built once and its midstate cloned per block instead of re-absorbing the ipad/opad key schedule three times per message. Cloning a `HashEngine` is possible because `bitcoin_hashes` 1.x made `Clone` a supertrait of it. `encrypt_to_bytes_with_nonce` padded into one buffer, encrypted it, then copied it into a second buffer alongside the version, nonce and MAC. It now lays out [version | nonce | length | plaintext | padding | MAC] once and encrypts the ciphertext region where it already sits. The MAC input is contiguous in that buffer too, so it needs no second engine input. `get_message_keys` can no longer fail, so it returns `MessageKeys` directly and the `HkdfLength` and `TryFromSlice` error variants go away with it. Per operation on an Apple M4: | op | allocs | bytes | time | |---------------|-------------|---------------|-----------------| | encrypt 32B | 11 -> 1 | 448 -> 99 | 1097 -> 710 ns | | decrypt 32B | 8 -> 2 | 346 -> 66 | 987 -> 767 ns | | encrypt 1024B | 11 -> 1 | 3424 -> 1091 | | Output is unchanged: a differential harness over every padding boundary, both tamper paths and all rejection error strings produces an identical digest, and the payload is asserted byte-equal to the previous pad-encrypt-append construction across sixteen plaintext lengths. Adds the RFC 5869 SHA-256 vectors, which the HKDF helpers had no coverage for.
`generate_shared_key` called `PublicKey::xonly()`, which parses the key, and then `from_x_only_public_key(pk, Parity::Even)`, which rebuilds the even-parity compressed form and parses it again. Both parses run the modular square root in `ge_set_xo_var`, the dominant cost of the conversion. Building the compressed form directly parses once for the same result: both paths accept exactly the keys whose x coordinate lies on the curve, and both take the even-parity point. `ConversationKey::derive` on an Apple M4: 19712 -> 17472 ns (-11%).
JSKitty
force-pushed
the
nip44-allocations
branch
from
August 4, 2026 09:37
2ccb447 to
5368934
Compare
JSKitty
marked this pull request as ready for review
August 4, 2026 09:39
TheAwiteb
reviewed
Aug 4, 2026
TheAwiteb
left a comment
Member
There was a problem hiding this comment.
It looks good, but I don't understand cryptography that much
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #1419, stacked on it. The diff is the two commits on top.
Three sources of per-message allocation in the NIP-44 v2 path:
Vecfor the output and one per block of the PRF chain.expand_intofills a caller-owned buffer, so the 76-byte message keys come off the stack, andT(i-1) | info | iis fed to the engine part by part rather than assembled first. The keyed engine depends only on the PRK, so it's built once and its midstate cloned per block, which 1.x makes possible by puttingCloneonHashEngine.encrypt_to_bytes_with_noncepadded into one buffer, encrypted it, then copied it into a second alongside the version, nonce and MAC. It now lays out[version | nonce | length | plaintext | padding | MAC]once and encrypts the ciphertext region in place.generate_shared_keyparsed the public key twice:xonly(), thenfrom_x_only_public_key(pk, Parity::Even)rebuilding the even-parity compressed form and parsing it again, running the modular square root inge_set_xo_varboth times. Building that form directly parses once.get_message_keyscan no longer fail, so it returnsMessageKeysdirectly and theHkdfLength/TryFromSlicevariants go with it.Measured on an Apple M4, all three trees in one run. "upstream" is master before #1419:
ConversationKey::derive#1419 does the bulk of it on the hashing, this PR does the rest on the allocations and the ECDH parse. Allocation traffic is all from this PR, since #1419 changed none of it:
Nothing here trades memory for speed; both go down.
Notes to the reviewers
Output is unchanged. Same differential harness as #1419 (every padding boundary, MAC and ciphertext tampering, all rejection paths by exact error string, on- and off-curve keys), identical digest
c95d027371418aa7d71c0d86ea7cec49.The in-place payload construction is the riskiest change, so
padis kept as a test oracle and the new payload is asserted byte-equal to the previous pad-encrypt-append form across sixteen plaintext lengths. Also adds the RFC 5869 SHA-256 vectors, which the HKDF helpers had no coverage for.Verified with the full
just precommitset pluscargo build --workspace --all-targetsandcargo clippy --all-targets -- -D warnings, on both commits and the tip.Checklist
CHANGELOG.md(if applicable)