perf(libsignal): memoize the sender signing key with a pre-warmed XEdDSA cache - #838
Conversation
…DSA cache Every group send re-deserialized the sender signing key from the record's protobuf bytes, so the lazily derived XEdDSA cache (scalar + Edwards point, a full basepoint multiplication) was recomputed for every SenderKeyMessage signature: nearly half the cost of a warm group_encrypt (found via the CodSpeed flamegraph, 18% of the group-send bench window). SenderKeyState now memoizes the parsed PrivateKey in a OnceLock, pre-warming the signing cache before memoizing so the clone handed to each signature carries the warm cache (clones of a cold key would each re-derive it). The record cache stores the live object back after every send, so the memo persists for the cache lifetime; it is never persisted and rebuilds lazily after a cold load. Key creation pre-warms too, since the parsed key is already in hand. Measured on a 2000-send group_encrypt loop (core-pinned perf stat): 24.7 -> 14.0 us/send wall, 868.1M -> 460.9M instructions (-47%). Contract pinned by tests: memo populated at creation and on first use, clones carry the warm cache, protobuf roundtrips reset it, signatures still verify.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds PrivateKey::precompute_signing_cache() to eagerly warm the XEdDSA signing cache and introduces a OnceLock memo in SenderKeyState to cache the deserialized signing key (warmed on new(), lazy-warmed on first use, and tested across clone/roundtrip). ChangesSigning Key Memoization and Cache Warming
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labelsapi-design 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/libsignal/src/protocol/sender_keys.rs`:
- Around line 182-188: The Debug impl for SenderKeyState currently exposes
secret material by printing self.state and the code later stores the serialized
signing private key into sender_signing_key.private; update the Debug for
SenderKeyState to only emit non-secret metadata (e.g., key IDs, version, flags)
and redact or omit any private key bytes instead of including self.state, and
refactor the code that assigns sender_signing_key.private (in the signing key
creation/serialization path) to avoid storing raw private key bytes—store only a
public key, identifier, or a redacted marker. Ensure any Debug/Display
implementations for types used inside SenderKeyState also avoid revealing secret
fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5c50bc64-2ddc-403f-8806-de5a851f7f8b
📒 Files selected for processing (2)
wacore/libsignal/src/core/curve.rswacore/libsignal/src/protocol/sender_keys.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | bench_group_encrypt_message |
246.6 µs | 156.6 µs | +57.54% |
| ⚡ | Simulation | bench_group_send_10 |
325.6 µs | 243.8 µs | +33.54% |
| ⚡ | Simulation | bench_group_send_50 |
361.8 µs | 279.6 µs | +29.42% |
| ⚡ | Simulation | bench_group_send_256 |
558.6 µs | 476.6 µs | +17.2% |
| ⚡ | Simulation | bench_group_send_skdm_10 |
734.7 µs | 653.8 µs | +12.38% |
| 👁 | Memory | bench_group_create_distribution_message |
1.2 KB | 1.8 KB | -31.01% |
| 👁 | Simulation | bench_group_create_distribution_message |
110.4 µs | 195.4 µs | -43.52% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing perf/sender-key-signing-cache (d3ea6e7) with main (b9e0941)
|
About the two flagged regressions on |
CodeRabbit/cubic review: the protobuf state embeds the serialized private signing key, and Debug-printing it (logs, panics) leaked raw key material. This predates the PR (the old derive printed it too); the new manual impl shows chain metadata and an explicit redaction marker.
1 participant
Problem
The CodSpeed flamegraph for
bench_group_send_256(pulled via the new MCP integration) showedOnceLock<EdwardsCacheData>::initializeinsideSenderKeyMessage::newat 18% of the measured window. Tracing the path confirmed it is a real production cost, not a bench artifact:group_encryptcallsSenderKeyState::signing_key_private()on every group message, which runsPrivateKey::deserializeon the record's protobuf bytes, producing a fresh key with a cold XEdDSA cache. The signature then re-derives the cache (scalar reduction + a full Edwards basepoint multiplication) for every single send. The moka record cache keeps the record object alive across sends, but the warm key never survived because it was rebuilt from bytes each time.Change
SenderKeyStatememoizes the parsedPrivateKeyin aOnceLock, and crucially pre-warms the signing cache BEFORE memoizing: the caller receives a clone, and clones of an initializedOnceLockcarry its contents, so a warm memo hands out warm clones while a cold one would make every clone re-derive. Key creation (SenderKeyState::new) seeds the memo directly from the already-parsed key. The memo is never persisted (the protobuf surface is unchanged) and rebuilds lazily after a cold load from the database; since the record cache stores the live object back after every send, the memo effectively lives for the cache lifetime.PrivateKey::precompute_signing_cache()is the new (public, side-effect-only) entry point for the pre-warm.Measurements
2000-iteration
group_encryptloop, core-pinnedperf stat, A/B against main:The Edwards re-derivation was nearly half of a warm group encrypt. On the CodSpeed benches expect
bench_group_send_*(andskdm_*) simulation numbers to drop accordingly.Tests
New test pins the contract: memo populated at creation and on first use, state clones carry the warm cache, protobuf roundtrips reset it, and the memoized key still produces verifiable signatures. Full workspace suite, strict clippy, and both wasm32 CI builds pass. Verified empirically (init-counting instrumentation) that cache derivations move out of the per-send path: same total count, all at setup/creation time.
Notes
group_decryptverifies with the public key and is unaffected. Session (DM) messages authenticate via HMAC, not per-message XEdDSA, so this path is specific to sender-key (group) sends. If a signing-key setter is ever added toSenderKeyState, it must reset the memo (documented on the field).