refactor(blockchain): consolidate XMSS preparation window advance logic

[conache]

🗒️ Description / Motivation

This PR addresses review comments from #332.

sign_with_attestation_key and sign_with_proposal_key had inline copies of the same advance loop that lives in the advance_key helper. This PR routes them through the helper so there's a single implementation.

Also switches the tracing field for elapsed durations from elapsed_ms = ... as u64 to elapsed = ?duration so the Duration's Debug impl is used.

What Changed

crates/blockchain/src/key_manager.rs

• advance_key now returns Result<(), KeyManagerError> and returns the same SigningError("XMSS key exhausted...") the inline loops previously produced.
• sign_with_attestation_key and sign_with_proposal_key replace their inline advance loops with advance_key(...)?.
• advance_keys_to swallows the new Result via inspect_err so one exhausted key doesn't abort iteration; logs a warn per failure.
• All three elapsed_ms = ... as u64 fields → elapsed = ?start.elapsed().

Correctness / Behavior Guarantees

• Signing-path behavior is unchanged: same control flow, same is_prepared_for check, same advance_preparation loop, same exhaustion detection, same returned error string.
• The exhaustion error message no longer distinguishes attestation vs proposal in its text (both say "XMSS key exhausted..."). validator_id and slot are still in the message.
• Tracing field change is observability-only — printed value is now human-readable (1.234s) instead of a raw millisecond count.

Tests Added / Run

• make fmt clean
• make lint clean
• make test passes

The refactor is logic-preserving, so the existing signature_spectests and forkchoice_spectests continue to exercise the signing paths through the new advance_key indirection.

Related Issues / PRs

• Closes Address comments from #332 #383
• Follow-up to feat(blockchain): advance XMSS preparation window preemtively #332

✅ Verification Checklist

• make fmt
• make lint
• make test

[greptile-apps]

Greptile Summary

This PR removes duplicated XMSS key-advance loops from sign_with_attestation_key and sign_with_proposal_key by routing both through the shared advance_key helper, which now returns Result<(), KeyManagerError> instead of (). The tracing change from elapsed_ms = ... as u64 to elapsed = ?duration is a straightforward observability improvement.

• advance_key now returns Err(SigningError(...)) on exhaustion instead of logging a warning and breaking; signing callers propagate the error with ?, and advance_keys_to discards it via inspect_err with a warn log.
• The exhaustion error message no longer distinguishes attestation vs. proposal key by name (both use the generic text), though validator_id and slot remain in the message.
• One minor observability gap: the warn! closures in advance_keys_to log %err only, dropping validator_id and slot as structured tracing fields that were present before the refactor.

Confidence Score: 4/5

The signing paths are logic-equivalent to the code they replace; the only real delta is that advance_keys_to now logs a warning per exhausted key instead of silently continuing, and the tracing field format improves readability.

The refactor correctly unifies control flow and the error-propagation wiring through both sign_with_* callers and advance_keys_to looks right. The one gap is that the new warn! closures in advance_keys_to emit validator_id and slot only via the formatted error string rather than as structured fields, which is a minor observability regression worth addressing.

crates/blockchain/src/key_manager.rs — the advance_keys_to warning logs

Important Files Changed

Filename	Overview
crates/blockchain/src/key_manager.rs	Consolidates duplicate XMSS advance loops into a single advance_key helper that now returns Result; advance_keys_to swallows errors with inspect_err; tracing switches to elapsed = ?duration. The warn logs in advance_keys_to omit structured validator_id/slot fields.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[sign_with_attestation_key / sign_with_proposal_key] --> B[advance_key]
    C[advance_keys_to loop] --> B

    B --> D{is_prepared_for slot?}
    D -- yes --> E[return Ok]
    D -- no --> F[log Advancing...]
    F --> G[while !is_prepared_for]
    G --> H[advance_preparation]
    H --> I{interval changed?}
    I -- no --> J[return Err SigningError]
    I -- yes --> G
    G -- prepared --> K[log Advanced + elapsed]
    K --> L[return Ok]

    J --> M{caller}
    M -- sign_with_*_key --> N[propagate ? to caller]
    M -- advance_keys_to --> O[inspect_err: warn log, discard]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/blockchain/src/key_manager.rs:55-60
The `warn!` calls inside `inspect_err` only capture the error string; `validator_id` and `slot` are embedded in the formatted message but are no longer emitted as structured tracing fields. Before this PR, the exhaustion warning included them as first-class fields (`warn!(validator_id, slot, "...")`), making it easy to filter logs by validator or slot. With `%err` alone, that filtering is lost.

```suggestion
            let _ = advance_key(*validator_id, &mut key_pair.attestation_key, slot).inspect_err(
                |err| warn!(validator_id, slot, %err, "Failed to advance attestation key preparation window"),
            );
            let _ = advance_key(*validator_id, &mut key_pair.proposal_key, slot).inspect_err(
                |err| warn!(validator_id, slot, %err, "Failed to advance proposal key preparation window"),
            );
```

_{Reviews (1): Last reviewed commit: "Cleanup: advance_key method in sign_with..." | Re-trigger Greptile}