Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
48 changes: 47 additions & 1 deletion crates/hotshot/task-impls/src/quorum_proposal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,14 +651,60 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>
},
HotShotEvent::QuorumProposalPreliminarilyValidated(proposal) => {
let view_number = proposal.data.view_number();

// All nodes get the latest proposed view as a proxy of `cur_view` of old.
if !self.update_latest_proposed_view(view_number).await {
tracing::trace!("Failed to update latest proposed view");
}

// Use the epoch from the proposal's justify_qc rather than self.cur_epoch.
// This avoids a race condition where we receive a proposal for the first view
// of a new epoch before ViewChange has updated our cur_epoch.
// If the QC is for the last block of an epoch (EQC), the next view is in epoch + 1.
let justify_qc = proposal.data.justify_qc();
let qc_epoch = justify_qc.data.epoch;
let is_eqc = justify_qc
.data
.block_number
.is_some_and(|bn| is_last_block(bn, self.epoch_height));
let target_epoch = if is_eqc {
qc_epoch.map(|e| e + 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

To make sure that it's a valid extended QC we should also check whether the proposal has the corresponding next_epoch_justify_qc. You can get it with the method next_epoch_justify_qc() and you can check whether the two certificates correspond to each other by comparing their leaf commits. They should be the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should actually be able to use handle_eqc_formed method to also properly store the eQC received in this way.

} else {
qc_epoch
};

// If the justify_qc is an EQC, verify and store the extended QC
if is_eqc {
let next_epoch_qc = proposal.data.next_epoch_justify_qc().as_ref();
ensure!(
next_epoch_qc.is_some(),
"Proposal has EQC but no next_epoch_justify_qc present"
);
let next_epoch_qc = next_epoch_qc.unwrap();
ensure!(
justify_qc.data.leaf_commit == next_epoch_qc.data.leaf_commit,
"Proposal has EQC but leaf commits don't match: justify_qc={:?}, \
next_epoch_qc={:?}",
justify_qc.data.leaf_commit,
next_epoch_qc.data.leaf_commit
);
self.formed_quorum_certificates
.insert(justify_qc.view_number(), justify_qc.clone());
self.formed_next_epoch_quorum_certificates
.insert(next_epoch_qc.view_number(), next_epoch_qc.clone());
handle_eqc_formed(
justify_qc.view_number(),
justify_qc.data.leaf_commit,
justify_qc.data.block_number,
self,
&event_sender,
)
.await;
}

self.create_dependency_task_if_new(
view_number + 1,
epoch_number,
target_epoch,
event_receiver,
event_sender,
Arc::clone(&event),
Expand Down
2 changes: 1 addition & 1 deletion crates/hotshot/testing/tests/test_da_committees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ cross_tests!(
// allow more time to pass in CI
completion_task_description: CompletionTaskDescription::TimeBasedCompletionTaskBuilder(
TimeBasedCompletionTaskDescription {
duration: Duration::from_secs(200),
duration: Duration::from_secs(240),
},
),
..TestDescription::default()
Expand Down
2 changes: 1 addition & 1 deletion hotshot.just
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ test-ci-rest *args:
# Build and run an integration test target
test-ci test *args:
echo Running integration test group {{test}}
RUST_LOG=error cargo nextest run -p hotshot-testing --profile hotshot --test {{test}} {{args}}
RUST_LOG=debug cargo nextest run -p hotshot-testing --profile hotshot --test {{test}} --nocapture {{args}}

# Run all hotshot integration tests
test-all *args:
Expand Down