fix(blockchain): warm XMSS keys after the slot's signatures, in parallel - #621
Conversation
A leanVM key caches a single bottom subtree (512 slots on devnet-5's key range), and the actor warmed both keys of every validator for slot + 1 at the end of every tick. At slot 512k-1 that meant: - interval 0: 64 serial subtree rebuilds (~1.7 s) held the actor, so the proposer's block reached gossip ~1.7 s late; - interval 1: that warm had evicted the subtree slot 512k-1 signs with, so every attestation rebuilt it on the signing path, and the next warm rebuilt the new one again. Votes split between the late block and its parent, the next block carried 192 of 1024, and devnet-5 lost 2-3 slots of finality every 512 slots. Warm from interval 2 on, once the slot's attestations are signed, and once per slot; the next slot's block is signed at interval 4, after it. Warm the upcoming proposer's proposal key alone, since the others sign nothing before their turn. Spread the rebuilds over scoped threads of four keys each, joined inline: across a boundary the warm drops from 1.54 s (64 keys, serial) to 142 ms (33 keys) on an 11-core machine, and costs 0.2 ms when every key is already warm.
🤖 Kimi Code ReviewI'll review this PR focusing on the key manager optimization for XMSS signing cache warming. Let me analyze the changes carefully. Overall AssessmentThis is a well-structured optimization PR that reduces unnecessary XMSS subtree rebuilds by:
Issues Found1. Race Condition in
|
| Severity | Count | Key Issues |
|---|---|---|
| Medium | 1 | warmed_slot updated before success (Point 1) |
| Low | 2 | Missing import check (Point 2), test seed overflow (Point 5) |
| Info | 4 | Thread panic handling, lifetime fragility, doc clarity, interval assumption |
The core logic is sound. Point 1 should be fixed before merge as it creates a correctness issue on failure paths.
Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt
🤖 Codex Code ReviewFindings:
Consensus-wise, I didn’t see changes to fork choice, justification/finalization, attestation validation, state transition, or SSZ handling in this PR. Testing gap: I couldn’t run the Rust tests in this environment because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewI'll skip running the build (would need separate approval) since the diff is small and I've traced the types/borrows by hand. Here's the review. Review:
|
KeyManager remembered the last slot it warmed so the interval-3 and interval-4 repeats could skip the keys. The keys already know: leanVM's `prepare` compares the requested slot's subtree with the one it caches and rebuilds only on a miss. The slot memo duplicated that state one level up, and could disagree with it, since a `sign` on a different subtree swaps the cache without the memo seeing it. A repeat call now pays a lock per key plus the helper threads' spawns, about 0.2 ms, and a key past its range warns on each interval-2..4 call instead of once per slot.
Intervals 3 and 4 re-asked in case the actor overran interval 2. That fallback bought little: when the interval-2 warm is missed, `sign` rebuilds the subtree itself on a miss, so the cost is latency on the next slot's duties, the same as before warming existed. Warming at one place also puts it next to the duty it follows, and brings the warning for a key past its range back to once per slot.
The attestations are signed on the actor at interval 1, before `produce_attestations` returns, so the end of that tick is the first point where nothing is left to sign at this slot. Warming there instead of at interval 2 leaves interval 2's tick to aggregation, and gives the next slot's proposer an extra interval of margin before it signs at interval 4. The warm runs after the early-aggregation timer is armed, so a slow warm across a subtree boundary cannot push that timer back.
Nothing in ethlambda-blockchain uses it. The benchmark in bin/ethlambda still does, so the workspace entry stays.
…ctor `Scope::spawn` panics when the OS refuses a thread, so a warm that only shifts latency could take the BlockChain actor down with it. Spawn with `Builder::spawn_scoped` instead, and warm a batch on the calling thread when its thread cannot be created. The fan-out also spawned one thread per four keys: a node with 1000 validators started ~250 threads every slot, and a lone validator one thread that added no parallelism. Cap the threads at the core count, keep each batch at four keys or more, and let the calling thread take the first batch, so a single batch spawns nothing. With the fan-out in its own function, its test checks that every key is visited exactly once, which the old smoke test could not fail on. Alongside: `KeyRole` is public and replaces the binary's two copies of the same enum, the per-key trace names its validator and role, the log fields put the slot first, and the test key seeds no longer overflow past 127 validators.
The first tick fires as soon as the actor starts and runs the current interval's duty. The startup warm always targeted the current slot's attestation keys, so a node started from interval 2 on warmed a slot it no longer attests at, and never warmed the proposal key that signs the next slot's block at this slot's interval 4. A restarted key has an empty cache, so that build rebuilt a whole subtree. Target the current slot up to interval 1, and the next slot plus its proposer's key from interval 2 on. `get_our_proposer` now takes the validator count: it cloned the whole head state on every call, and the tick already reads it.
13d20b5
into
build/leanvm-unified-aggregate-api
## 🗒️ Description / Motivation Follow-up to #621. After that PR, crossing a 512-slot subtree boundary still blocks the blockchain actor for ~142 ms at interval 1, while `prepare_keys_for` rebuilds the next subtree of every attestation key and joins the rebuilds inline. This moves that warm off the actor, so the tick returns immediately. It is safe to overlap with signing because of how leanVM caches the subtree (`xmss::XmssSecretKey`): the cache is `Mutex<Option<BottomSubtree>>`, and both `prepare` and `sign` go through `cached_bottom_subtree`. That takes the lock, rebuilds only on a miss, and holds the lock while rebuilding. A signature that races the warm of the same key therefore waits for that rebuild and reuses it. It never rebuilds twice, so the worst case is the current inline cost. ## What Changed - **`crates/blockchain/src/key_manager.rs`** - `KeyManager` stores `Arc<ValidatorKeyPair>`, so a background thread can own clones of the keys. `KeyManager::new` keeps its signature. - New `prepare_keys_in_background(slot, proposer)`: spawns an `xmss-warm` thread running the same batched warm, and keeps its `JoinHandle`. - The warm body (`available_parallelism` + `for_each_batched` from #621) moved into a free function `warm_keys`, shared by the blocking `prepare_keys_for` and the background path. The background thread takes `for_each_batched`'s inline first batch, and any further batches still go to its capped scoped threads. - **`crates/blockchain/src/lib.rs`** - The interval-1 tick calls `prepare_keys_in_background`. - The startup warm in `BlockChain::spawn` stays blocking: it runs before the first tick, so there is nothing to overlap with. ## Correctness / Behavior Guarantees - **Eviction ordering is unchanged.** The warm is spawned after `produce_attestations`, which signs synchronously in the same tick, so nothing signs with the old subtree after the warm starts. - **Interval-4 proposal signing:** if it lands before the warm finishes, it waits on the key's lock. That is the same cost as today, never double. - **A warm still running at the next call is skipped**, with a warning. A finished warm that panicked is logged when it is joined. A failed thread spawn warns instead of panicking. In every case the only cost is latency, since `sign` rebuilds a missing subtree itself. - **No runtime dependency:** it uses `std::thread`, not `spawn_blocking`, so `KeyManager` stays usable outside tokio (the benchmark uses it). ## Tests Added / Run - New `prepare_keys_in_background_warms_without_blocking_signing`: signs while the warm may still be running, joins the worker, and checks that a later call spawns a fresh warm. - `make fmt` and `make lint` are clean. All `key_manager` tests pass. - `cargo test --workspace --profile release-fast --no-fail-fast`: 516 passed. The 205 failures all come from the base branch (#606), where the spec fixtures don't decode: 204 are `ValidatorPubkey length != 32` and 1 is a 1208-vs-2536-byte signature length mismatch. #621's CI fails the same way. ## Related Issues / PRs - Stacked on #621 (itself on #606) ## ✅ Verification Checklist - [x] Ran `make fmt` — clean - [x] Ran `make lint` (clippy with `-D warnings`) — clean - [ ] Ran `make test` (`cargo test --workspace --profile release-fast`) — all passing (spec fixtures fail on the base branch, see above) --------- Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com>
2 participants
Why
A leanVM XMSS key caches a single bottom subtree (
split_level = ceil(log2(range)) / 2, so 512 slots for a key range of about 2^18), andon_tickwarmed both keys of every validator forslot + 1at the end of every tick. At slot512k - 1:prepare(512k)rebuilt 64 subtrees one after another (~1.7 s on the actor). The proposer's block reached gossip ~1.7 s late512k - 1signs with, so every attestation rebuilt it on the signing path (32 signatures over 1.9 s instead of 0.5 s), and the next warm rebuilt the new subtree againSome nodes attested before importing the late block and some after, so the votes split across two sources. Block
512kthen carried 192 of the 1024 votes it needed 683 of. On a 32-node, 1024-validator devnet with 8 s slots, this was the only deviation from head - finalized = 3 in 16 hours: every 512 slots, it lost 2 to 3 slots of finality.What
signrebuilds the subtree when it isn't cached. The next slot's block is signed at interval 4, after this warm.Measured
32 validators, 512-slot subtrees, crossing a boundary, on an 11-core machine: