Greptile Summary

This PR reworks block proposal timing: instead of building and signing the next slot's block at interval 0 of that slot, the actor now does it at interval 4 of the previous slot, then waits out the remainder of the interval before publishing at the slot boundary. A companion fix removes the get_proposal_head call (which ticked the store clock prematurely) and replaces it with a read-only store.head() lookup, resolving a finalization stall.

• The old propose_block / produce_block_with_signatures monolith is split into three focused helpers: build_signed_block, assemble_signed_block, and process_and_publish_block, with propose_block now async to accommodate the slot-boundary sleep.
• store::produce_block_on_head replaces the removed produce_block_with_signatures, taking an explicit head_root instead of advancing the store clock internally.
• A new build_overran_publish_window predicate (with unit tests) determines whether the build finished before or after the target slot opened, driving the publish-or-wait decision.

Confidence Score: 5/5

Safe to merge; the core build-at-interval-4 logic is sound, the finalization fix is correct, and devnet validation showed zero stale discards and zero skipped ticks.

The refactor is well-structured with clear comments explaining every invariant. The single-threaded actor guarantees cited in the code are accurate for the current architecture, making the rebuild guard genuinely inert today. The only findings are a key-advance ordering nit and a missing must_use annotation.

crates/blockchain/src/lib.rs — the key-advance ordering and the ignored process_and_publish_block return value are both in this file, though neither blocks the change.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant HT as handle_tick
    participant PB as propose_block
    participant ST as store
    participant KM as key_manager
    participant P2P as gossip

    Note over HT: Slot S, Interval 4
    HT->>ST: "on_tick(interval-4 timestamp, has_proposal=false)"
    Note over ST: Promotes attestations
    HT->>HT: get_our_proposer(S+1)
    HT->>PB: propose_block(S+1, validator_id)
    PB->>ST: "on_tick(slot_S+1_start_ms, has_proposal=true)"
    Note over ST: Advances to slot S+1 interval 0
    PB->>ST: store.head() parent_root
    PB->>KM: build_signed_block(S+1, validator_id, parent_root)
    Note over KM: advance_key(S+1) + sign inline
    PB->>PB: build_overran_publish_window()?
    alt build finished early
        PB->>PB: tokio::time::sleep(remaining_ms)
    end
    PB->>ST: process_block(signed_block)
    PB->>P2P: publish_block(signed_block)
    PB-->>HT: return
    HT->>KM: advance_keys_to(S+1) -- no-op, already advanced
    Note over HT: Slot S+1, Interval 0
    HT->>ST: on_tick(slot_S+1_start_ms, is_proposer)
    Note over ST: Idempotency guard -- no-op

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant HT as handle_tick
    participant PB as propose_block
    participant ST as store
    participant KM as key_manager
    participant P2P as gossip

    Note over HT: Slot S, Interval 4
    HT->>ST: "on_tick(interval-4 timestamp, has_proposal=false)"
    Note over ST: Promotes attestations
    HT->>HT: get_our_proposer(S+1)
    HT->>PB: propose_block(S+1, validator_id)
    PB->>ST: "on_tick(slot_S+1_start_ms, has_proposal=true)"
    Note over ST: Advances to slot S+1 interval 0
    PB->>ST: store.head() parent_root
    PB->>KM: build_signed_block(S+1, validator_id, parent_root)
    Note over KM: advance_key(S+1) + sign inline
    PB->>PB: build_overran_publish_window()?
    alt build finished early
        PB->>PB: tokio::time::sleep(remaining_ms)
    end
    PB->>ST: process_block(signed_block)
    PB->>P2P: publish_block(signed_block)
    PB-->>HT: return
    HT->>KM: advance_keys_to(S+1) -- no-op, already advanced
    Note over HT: Slot S+1, Interval 0
    HT->>ST: on_tick(slot_S+1_start_ms, is_proposer)
    Note over ST: Idempotency guard -- no-op

_{Reviews (2): Last reviewed commit: "refactor: inline function" | Re-trigger Greptile}

🤖 Codex Code Review

1. High — Prebuilding now blocks the blockchain actor for the rest of interval 4 and even across the boundary sleep, so the code no longer builds on the true slot-boundary view it claims to use. handle_tick awaits the prebuild at lib.rs, propose_block explicitly relies on the actor processing no other messages during build/sleep at lib.rs, and gossip/block ingestion is on the same actor at lib.rs. As a result, late blocks or attestations arriving after interval 4 starts cannot affect the manual store::on_tick(..., slot_start_ms, true) at lib.rs. That can make the proposer build on a stale fork-choice head and miss attestations that the old interval-0 path would have seen.

2. Medium — The failure path advances store.time() into the next slot before the wall-clock boundary and then returns immediately. store::on_tick(..., slot_start_ms, true) runs first at lib.rs, but a build failure exits at lib.rs. Future-slot attestation validation uses store.time() at store.rs, so if prebuild fails early, next-slot attestations can be accepted up to one interval early. That is a consensus-time validation bug on the proposer-failure path.

Aside from those two points, the helper split (build_signed_block / assemble_signed_block / process_and_publish_block) is cleaner and the XMSS aggregation changes look mechanically sound.

I couldn’t run cargo test here because cargo tried to sync the Rust toolchain into read-only /home/runner/.rustup.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

🤖 Claude Code Review

Here is my review of PR #445.

Review: feat(blockchain): pre-build proposer block one interval early

This PR shifts the expensive leanVM block-building work from interval 0 (where it blocked the first attestation window) to interval 4 of the previous slot. The core machinery — propose_block, build_signed_block, process_and_publish_block, and the build_overran_publish_window alignment helper — is sound, and the finalization-clock-advance bug fix (replacing get_proposal_head with a plain store.head()) is correct and well-motivated. The devnet validation is reassuring.

Four findings survived verification, ranked by severity:

crates/blockchain/src/lib.rs:685 — Unconditional signed_block.clone() wasted on every build failure

process_and_publish_block clones the block up front and passes the clone to process_block:

if let Err(err) = self.process_block(signed_block.clone()) {
    ...
    return false;
}
// original signed_block used for publish_block here

If process_block returns Err, the function returns early and the retained original signed_block is dropped without ever reaching publish_block. SignedBlock holds a MultiMessageAggregate backed by a ByteList512KiB — the clone can be several KB of XMSS proof material. Swap the roles: pass ownership to process_block and clone for the publish path (on the success path only), or restructure so the clone is made after confirming local import succeeded.

crates/blockchain/src/lib.rs:328–330 — Silent proposal skip when node is syncing

The old interval-0 code had an explicit else branch logging "Skipping block proposal while syncing". The new interval-4 code folds the sync guard into a .filter() with no fallback:

let next_proposer = self
    .get_our_proposer(next_slot)
    .filter(|_| self.sync_status.duties_allowed());

When a validator is elected proposer but duties_allowed() returns false, the slot is silently skipped. The attestation path at interval 1 (lines 293–297) uses an explicit else if branch with info!(%slot, "Skipping attestations while syncing") as the established pattern. An operator diagnosing missing proposals during a sync stall gets no signal. Add a log (gated on !self.key_manager.validator_ids().is_empty(), matching the attestation pattern) when a proposer is elected but suppressed by sync.

crates/blockchain/src/lib.rs:439–441 — unix_now_ms() sampled twice in the sleep path

if !build_overran_publish_window(unix_now_ms(), genesis_time_ms, slot) {
    let wait_ms = slot_start_ms.saturating_sub(unix_now_ms());
    tokio::time::sleep(Duration::from_millis(wait_ms)).await;
}

unix_now_ms() wraps SystemTime::UNIX_EPOCH.elapsed() — a non-monotonic wall clock. If the system clock is stepped backward by NTP between the two calls (possible on a validator host under heavy leanVM load), the second sample yields a smaller timestamp than the first, causing saturating_sub to compute a larger-than-correct wait_ms and delaying publication past the slot boundary. Capture once:

let now_ms = unix_now_ms();
if !build_overran_publish_window(now_ms, genesis_time_ms, slot) {
    let wait_ms = slot_start_ms.saturating_sub(now_ms);
    tokio::time::sleep(Duration::from_millis(wait_ms)).await;
}

crates/blockchain/src/lib.rs:683,456,459 — published_msg: &'static str is dead variation

Both call sites pass the identical literal:

self.process_and_publish_block(slot, validator_id, signed_block, "Published block");
// ...
self.process_and_publish_block(slot, validator_id, rebuilt, "Published block");

The parameter exists to vary the log message but never does. Remove it and hardcode "Published block" inside the function. If the rebuild path ever needs a distinct message, add an info! at the call site before the call — tunneling it as a parameter adds signature noise for no benefit.

Automated review by Claude (Anthropic) · sonnet · custom prompt