Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions crates/blockchain/src/key_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ethlambda_types::{
primitives::{H256, HashTreeRoot as _},
signature::{ValidatorSecretKey, ValidatorSignature},
};
use tracing::info;

use crate::metrics;

Expand Down Expand Up @@ -102,6 +103,21 @@ impl KeyManager {
.get_mut(&validator_id)
.ok_or(KeyManagerError::ValidatorKeyNotFound(validator_id))?;

// Advance XMSS key preparation window if the slot is outside the current window.
// Each bottom tree covers 65,536 slots; the window holds 2 at a time.
if !secret_key.is_prepared_for(slot) {

@MegaRedHand MegaRedHand Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I opened #262 to tackle this later

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Got it, thanks for tracking it.

info!(validator_id, slot, "Advancing XMSS key preparation window");
while !secret_key.is_prepared_for(slot) {
Comment thread
MegaRedHand marked this conversation as resolved.
secret_key.advance_preparation();
if !secret_key.is_prepared_for(slot) {
return Err(KeyManagerError::SigningError(format!(
"XMSS key exhausted for validator {validator_id}: \
slot {slot} is beyond the key's activation interval"
)));
}
}
Comment on lines +111 to +120

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 while loop only ever advances once

The inner if !is_prepared_for guard returns an error immediately after the first advance_preparation() call, so the while never actually iterates a second time. If the slot happens to be two or more windows ahead of the current prepared interval (e.g., after an extended outage), the key would be incorrectly reported as exhausted even though another advance would bring it in range.

To detect true exhaustion (where advance_preparation() is a no-op), capture the interval before and after and compare:

while !secret_key.is_prepared_for(slot) {
    let before = secret_key.get_prepared_interval();
    secret_key.advance_preparation();
    if secret_key.get_prepared_interval() == before {
        // No-op: key activation interval is fully exhausted
        return Err(KeyManagerError::SigningError(format!(
            "XMSS key exhausted for validator {validator_id}: \
             slot {slot} is beyond the key's activation interval"
        )));
    }
}

This requires exposing get_prepared_interval() on ValidatorSecretKey, but correctly distinguishes "key exhausted" from "slot is multiple windows ahead".

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/blockchain/src/key_manager.rs
Line: 110-118

Comment:
**`while` loop only ever advances once**

The inner `if !is_prepared_for` guard returns an error immediately after the first `advance_preparation()` call, so the `while` never actually iterates a second time. If the slot happens to be two or more windows ahead of the current prepared interval (e.g., after an extended outage), the key would be incorrectly reported as exhausted even though another advance would bring it in range.

To detect true exhaustion (where `advance_preparation()` is a no-op), capture the interval before and after and compare:

```rust
while !secret_key.is_prepared_for(slot) {
    let before = secret_key.get_prepared_interval();
    secret_key.advance_preparation();
    if secret_key.get_prepared_interval() == before {
        // No-op: key activation interval is fully exhausted
        return Err(KeyManagerError::SigningError(format!(
            "XMSS key exhausted for validator {validator_id}: \
             slot {slot} is beyond the key's activation interval"
        )));
    }
}
```

This requires exposing `get_prepared_interval()` on `ValidatorSecretKey`, but correctly distinguishes "key exhausted" from "slot is multiple windows ahead".

How can I resolve this? If you propose a fix, please make it concise.

}

let signature: ValidatorSignature = {
let _timing = metrics::time_pq_sig_attestation_signing();
secret_key
Expand Down
19 changes: 18 additions & 1 deletion crates/common/types/src/signature.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use leansig::{
serialization::Serializable,
signature::{SignatureScheme, SigningError},
signature::{SignatureScheme, SignatureSchemeSecretKey as _, SigningError},
};

use crate::primitives::H256;
Expand Down Expand Up @@ -97,4 +97,21 @@ impl ValidatorSecretKey {
let sig = LeanSignatureScheme::sign(&self.inner, slot, &message.0)?;
Ok(ValidatorSignature { inner: sig })
}

/// Returns true if the key is prepared to sign at the given slot.
///
/// XMSS keys maintain a sliding window of two bottom trees. Only slots
/// within this window can be signed without advancing the preparation.
pub fn is_prepared_for(&self, slot: u32) -> bool {
self.inner.get_prepared_interval().contains(&(slot as u64))
}

/// Advance the prepared window forward by one bottom tree.
///
/// Each call slides the window by sqrt(LIFETIME) = 65,536 slots.
/// If the window is already at the end of the key's activation interval,
/// this is a no-op.
pub fn advance_preparation(&mut self) {
self.inner.advance_preparation();
}
}
Loading