Skip to content

[new-protocol] seed parent proposal from anchor leaf so leader can propose after restart #4440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
imabdulbasit merged 2 commits into from
Jun 5, 2026
+37 −38
Merged

[new-protocol] seed parent proposal from anchor leaf so leader can propose after restart #4440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91381b9
seed parent proposal from anchor leaf so leader can propose after res…
imabdulbasit Jun 5, 2026
ee6f1e5
use proposal view number
imabdulbasit Jun 5, 2026
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
2 changes: 1 addition & 1 deletion crates/hotshot/new-protocol/bench/src/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ async fn build_coordinator(
genesis_state,
Leaf2::from(genesis_proposal.clone()),
);
consensus.seed_genesis(genesis_cert1, genesis_proposal);
consensus.seed_parent(genesis_cert1, genesis_proposal);

let proposal_validator =
ProposalValidator::new(membership.clone(), epoch_height, upgrade_lock.clone());
Expand Down
22 changes: 9 additions & 13 deletions crates/hotshot/new-protocol/src/consensus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,20 +272,16 @@ impl<T: NodeType> Consensus<T> {
}
}

/// Seed the genesis state so that the view-1 leader can propose without
/// any external bootstrap injection.
///
/// Stores a genesis certificate and proposal at view 0, sets the locked
/// certificate, and sets the current epoch. After calling this, a
/// subsequent `apply` that triggers `maybe_propose(view=1)` will find the
/// Seed a parent certificate and proposal so the leader of the *next* view
/// can propose without any external bootstrap injection.
/// Sets the locked certificate and current epoch. After calling this, a
/// subsequent `apply` that triggers `maybe_propose` will find the
/// parent cert and proposal it needs.
pub fn seed_genesis(&mut self, genesis_cert1: Certificate1<T>, genesis_proposal: Proposal<T>) {
self.current_epoch = Some(genesis_proposal.epoch);
self.certs
.insert(ViewNumber::genesis(), genesis_cert1.clone());
self.locked_cert = Some(genesis_cert1);
self.proposals
.insert(ViewNumber::genesis(), genesis_proposal);
pub fn seed_parent(&mut self, cert1: Certificate1<T>, proposal: Proposal<T>) {
self.current_epoch = Some(proposal.epoch);
self.certs.insert(cert1.view_number(), cert1.clone());
self.locked_cert = Some(cert1);
self.proposals.insert(proposal.view_number, proposal);
}
Comment on lines +275 to 285
Copy link
Copy Markdown
Contributor

@claude claude Bot Jun 5, 2026

Choose a reason for hiding this comment

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

The generalization from seed_genesis to seed_parent looks clean. One thing the doc could mention: this method assumes cert1.view_number() matches proposal.view_number (i.e., the cert certifies the leaf derived from this proposal). If they can legitimately differ, the method will store the proposal under a view key that doesn't match its own view_number field, which will trip the commitment check in maybe_propose.


/// Apply a [`PreCutoverSeed`] to bridge legacy state into the new
Expand Down
47 changes: 25 additions & 22 deletions crates/hotshot/new-protocol/src/coordinator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,38 +134,41 @@ where
initializer.epoch_height,
);

let genesis_cert1 = initializer.high_qc.clone();
let genesis_proposal = message::Proposal {
block_header: initializer.anchor_leaf.block_header().clone(),
view_number: ViewNumber::genesis(),
epoch: EpochNumber::genesis(),
justify_qc: genesis_cert1.clone(),
let anchor_leaf = &initializer.anchor_leaf;
let anchor_view = anchor_leaf.view_number();
let anchor_epoch = anchor_leaf
.epoch(initializer.epoch_height)
.unwrap_or(EpochNumber::genesis());
let cert1 = initializer.high_qc.clone();
let parent_proposal = message::Proposal {
block_header: anchor_leaf.block_header().clone(),
view_number: anchor_view,
epoch: anchor_epoch,
justify_qc: anchor_leaf.justify_qc(),
next_epoch_justify_qc: None,
Copy link
Copy Markdown
Contributor

@claude claude Bot Jun 5, 2026

Choose a reason for hiding this comment

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

Nit (correctness at epoch boundaries): next_epoch_justify_qc is hardcoded to None here, and the Proposal -> QuorumProposalWrapper conversion also hardcodes it to None. Since next_epoch_justify_qc contributes to the Leaf2 commitment (when with_epoch = true, see data.rs:1755), this means proposal_commitment(parent_proposal) won't match anchor_leaf.commit() if the anchor_leaf has a non-None next_epoch_justify_qc.

This likely doesn't matter if the anchor_leaf is never at an epoch boundary when this code path runs, but worth keeping in mind.

upgrade_certificate: None,
view_change_evidence: None,
next_drb_result: None,
upgrade_certificate: anchor_leaf.upgrade_certificate(),
view_change_evidence: anchor_leaf
.view_change_evidence
.clone()
.and_then(|e| match e {
hotshot_types::data::ViewChangeEvidence2::Timeout(tc) => Some(tc),
hotshot_types::data::ViewChangeEvidence2::ViewSync(_) => None,
}),
next_drb_result: anchor_leaf.next_drb_result,
state_cert: None,
};

let mut state_manager = StateManager::new(
Arc::new(initializer.instance_state.clone()),
upgrade_lock.clone(),
);
state_manager.seed_state(
initializer.anchor_leaf.view_number(),
initializer.anchor_state.clone(),
initializer.anchor_leaf.clone(),
);
// The synthetic genesis proposal has a non-null justify_qc (the genesis
// cert1) so the leaf derived from it has a different commitment than
// the anchor leaf produced by `Leaf2::genesis`. `request_header` for
// view 1 looks up the parent state by the *proposal's* leaf
// commitment, so seed the same state under that commitment too.
state_manager.seed_state(
ViewNumber::genesis(),
anchor_view,
initializer.anchor_state.clone(),
Leaf2::from(genesis_proposal.clone()),
anchor_leaf.clone(),
);
consensus.seed_genesis(genesis_cert1, genesis_proposal);
consensus.seed_parent(cert1, parent_proposal);
Comment on lines +142 to +170
Copy link
Copy Markdown
Contributor

@claude claude Bot Jun 5, 2026

Choose a reason for hiding this comment

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

Question (correctness): seed_parent stores both the cert and proposal at cert1.view_number(), but the proposal's own view_number field is anchor_view. If cert1.view_number() != anchor_view — which can happen when high_qc is ahead of the anchor leaf (the HotShotInitializer comment says "It should be for a view greater than inner's view number for the non genesis case") — then the commitment check in maybe_propose (consensus.rs:1201) will fail:

} else if proposal_commitment(proposal) != parent_cert.data.leaf_commit {
    warn!("stored proposal at parent_view does not match parent cert's leaf_commit; refusing to propose with mismatched parent");
    return;

proposal_commitment derives a leaf from the proposal (which carries view_number = anchor_view), while cert1.data.leaf_commit is the commitment of whatever leaf cert1 actually certified (at cert1.view_number()). If these views differ, the commitments will differ and the leader silently refuses to propose.

Is there an invariant guaranteeing cert1.view_number() == anchor_view in the new protocol context? If so, a debug_assert or comment documenting this would help future readers. If not, this may need to be handled.

consensus.set_view(anchor_view, anchor_epoch);
Copy link
Copy Markdown
Contributor

@claude claude Bot Jun 5, 2026

Choose a reason for hiding this comment

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

Good addition — previously current_view was stuck at genesis after seed_genesis, which would prevent the node from proposing at the correct view after a non-genesis restart.


let lock = upgrade_lock.clone();
Self::builder()
Expand Down
2 changes: 1 addition & 1 deletion crates/hotshot/new-protocol/src/tests/common/coordinator_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub async fn build_test_coordinator<N: Network<TestTypes>>(
genesis_state,
Leaf2::from(genesis_proposal.clone()),
);
consensus.seed_genesis(genesis_cert1.clone(), genesis_proposal.clone());
consensus.seed_parent(genesis_cert1.clone(), genesis_proposal.clone());

if let Some(seed) = pre_cutover_seed {
consensus.apply_pre_cutover_seed(seed);
Expand Down
2 changes: 1 addition & 1 deletion crates/hotshot/new-protocol/src/tests/legacy_cutover.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ async fn build_cutover_coordinator(
genesis_state,
Leaf2::from(genesis_proposal.clone()),
);
consensus.seed_genesis(genesis_cert1, genesis_proposal);
consensus.seed_parent(genesis_cert1, genesis_proposal);

let block_builder = BlockBuilder::new(
instance.clone(),
Expand Down
Loading