fix(consensus): cutover QC handling and parent proposal fetch

[bfish713]

This PR hardens the legacy → new-protocol cutover so the last legacy QC always reaches the new protocol and the boundary views decide deterministically, and fixes a legacy proposal-path bug where a DA-only state-map entry aborted the leader's proposal.

This PR:

Cutover QC handling (fix cutover qc handling):

• Captures the last-legacy QC keyed on the QC's own view instead of cur_view (task-impls/src/consensus/mod.rs). Qc2Formed fires once, and the aggregator can form this QC before processing its own ViewChange into the cutover view; keying on cur_view dropped it, so the cutover seed's high_qc (and the LegacyHighQcFormed bridge) missed it.
• Bridges legacy timeout votes only for views strictly before new_version_first_view (task-impls/src/consensus/handlers.rs, <= → <). Legacy re-times-out the parked boundary view forever; bridging those votes could form a TC2 that skips the first new-protocol view even when its leader is online.
• Re-requests the block/header when a TC-backed proposal finds no header for the current lock (new-protocol/src/consensus.rs, maybe_propose). The TC-time header request targets the lock held at that moment; if a bridged legacy QC moves the lock afterwards, the leader now re-issues RequestBlockAndHeader for the adopted lock instead of hitting "no block header" until the view times out. The block builder dedups by (view, parent).
• Adds state-machine tests for both behaviors: a timeout-backed proposal chains only from the locked cert (never a cert1 of the timed-out view), and a bridged legacy QC is adopted as lock, the header re-requested, and the view re-proposed justifying the adopted QC with the TC attached.
• Makes the cutover test suite exact: the loose permitted-failures mode is removed, every test (including the permutation sweep) asserts the precise attributable failure set in both directions, and the happy path asserts zero failed views. Silencers now watch new-protocol coordinator progress (published via AtomicU64) in addition to legacy cur_view, so post-cutover kills land punctually.

Parent proposal fetch (fetch leaf if we only have a DA proposal in leaf and state map, task-impls/src/helpers.rs):

• A Da/Failed state-map entry carries no leaf/state, so it cannot be proposed on; parent_leaf_and_state now treats it like a missing view and fetches the parent proposal. This matters when the leader forms the QC from unicast votes before validating the parent proposal itself — previously the DA entry masked the fetch and the proposal aborted.

This PR does not:

• Change the coordinator's seed extraction or SubmitLegacyHighQc/SubmitTimeoutVote handling — only how legacy feeds the bridge and how the new-protocol state machine reacts once the lock moves.
• Restrict the header re-request to the cutover: it also fires for a normal TC-backed view whose header is still building. This is benign (deduped by the builder, and the epoch it computes matches the eventual proposal's), but it is broader than the comment suggests.

Key places to review:

• task-impls/src/consensus/mod.rs, Qc2Formed arm — the guard new_protocol_active(qc.view_number() + 1) && !new_protocol_active(qc.view_number()) selects exactly the last-legacy QC, independent of when the aggregator's own view advances.
• new-protocol/src/consensus.rs, maybe_propose — the re-request's request_epoch intentionally matches the eventual proposal_epoch derivation below it (not the raw TC epoch used by the original request site in handle_timeout_certificate); that is what makes the leader check correct for a first-of-epoch proposal.
• new-protocol/src/tests/legacy_cutover.rs, expected_failures — the exact failure model the whole suite now leans on: the silenced view, legacy unicast-vote spillover to the preceding view (for silenced views at or before the cutover), and the dead node's later leader slots.

How to test this PR:

• cargo nextest run -p hotshot-new-protocol — cutover suite (tests::legacy_cutover) and the new state-machine tests (tests::consensus).

[gemini-code-assist]

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

[claude]

Claude finished @bfish713's task in 2m 19s —— View job

---

Claude Code Review — PR #4702 (Fix FF Cutover Tests)

• Read existing PR comments (only the Gemini bot intro; no prior review feedback to dedup against)
• Review production code changes
• Review test changes
• Post review feedback

---

Reviewed the full diff against origin/main. This is a focused, well-reasoned set of fixes to the legacy→0.8 cutover path plus a substantial tightening of the cutover test harness. Overall it looks correct and high-quality — the comments do a genuinely good job explaining the why behind each subtle change. No blocking issues found.

Production changes

consensus/mod.rs — Qc2Formed boundary guard
Re-keying the guard from self.cur_view to qc.view_number() + 1 is the right call. The old check was racy: the aggregator can form the last-legacy QC before it processes its own ViewChange into the cutover view, so cur_view could still be pre-cutover and the QC would be dropped. new_protocol_active(qc.view_number() + 1) && !new_protocol_active(qc.view_number()) precisely pins "the QC whose view is the last legacy view," independent of local view timing. 👍

consensus/handlers.rs — handle_timeout bridge bound (<= → <)
Correct. new_version_first_view is run by the new protocol; legacy only parks-and-re-times-out that boundary view, so bridging those votes would feed the new-protocol coordinator timeout evidence that prematurely TC2-skips its first view. Excluding view_number == new_version_first_view is the right boundary.

helpers.rs — parent_leaf_and_state fetch gate
Good fix. Switching from contains_key to get(...).is_some_and(|v| v.leaf_and_state().is_some()) correctly treats a Da/Failed state-map entry as "no proposable parent" and triggers the fetch, instead of the DA entry masking it and aborting the proposal. The narrower name vsm_contains_parent_leaf reflects the real predicate.

consensus.rs — re-request header when the lock moved (maybe_propose)
The new block re-emits RequestBlockAndHeader when a TC exists but the header lookup misses, gated on view_change_evidence.is_some() and leadership. This closes the cutover gap where the original TC-time request targeted the old lock and a bridged legacy QC then moved locked_cert past it. Two things I checked that hold up:

• The request_epoch computation (is_last_block(proposal.block_header.block_number(), …)) is consistent with the later proposal_epoch derivation (is_last_block(header.block_number().saturating_sub(1), …)), since the child header's block number is parent + 1. So the re-request targets the same epoch the eventual proposal will use.
• It re-fires on every maybe_propose call while the header is outstanding, but that's bounded by proposed_views short-circuiting once proposed and the block builder deduping by (view, parent) — which the comment calls out. Fine as-is.

Test changes

• test_timeout_proposal_chains_from_lock_not_timed_out_cert1 and test_bridged_legacy_qc_adopts_lock_and_reproposes are good, targeted unit tests that lock down exactly the two behaviors above (chain-from-lock, and adopt-lock-then-re-request). The manual-outbox approach in the second test is a clean way to freeze the "header outstanding when the legacy QC arrives" state.
• The legacy_cutover.rs rework is the strongest part: dropping the loose flag and replacing permitted_failures (a superset) with expected_failures (an exact, per-silenced-leader attribution) turns previously-permissive assertions into exact ones. The await_node_at_view change to watch both legacy cur_view and the new-protocol coordinator view is what makes the exact assertions viable — post-cutover, legacy only creeps past the boundary via timeouts, so silencing keyed off legacy alone would fire late. The reasoning in the expected_failures doc comment (pre-cutover unicast vote spillover to view-1, none post-cutover) matches the vote-routing model.

Minor notes (non-blocking)

• expected_failures(..., 50) hardcodes max_view = 50 as the slot-enumeration ceiling. It's commented as "only needs to exceed the highest decided view," and with target_decisions = 6 that's safe, but a brief inline note tying 50 to the deadline/target would help future readers who bump the decision count.
• I did not run the cutover suite here (multi-thread integration tests, long runtime). Worth confirming the tightened exact-assertion permutation sweep is stable across a few CI runs given it previously relied on the loose check for post-cutover timing — exact failure-set assertions can be sensitive to scheduler jitter.

Nice work — the fixes are minimal and each is backed by a test that would have caught the original flake.

[github-actions]

Created backport PR for release-ff:

• [Backport release-ff] fix(consensus): cutover QC handling and parent proposal fetch #4710 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-4702-to-release-ff
git worktree add --checkout .worktree/backport-4702-to-release-ff backport-4702-to-release-ff
cd .worktree/backport-4702-to-release-ff
git reset --hard HEAD^
git cherry-pick -x 37385605d677b1e0f2cd3f9d4d9233e7e2ec56a3
git push --force-with-lease