Skip to content

Commit 02e41ca

Browse files
authored
fix: properly justify round change (#555)
We were making sure that we had reach consensus on the value that the round changes were preparing, which defeats the purpose of the round change justifications. Now, we will find the highest prepared and use that data from the round change directly. If for some reason it is prepared and there is no full data, we fall back to our data, otherwise we just use the start value
1 parent 59d3b50 commit 02e41ca

File tree

1 file changed

+33
-32
lines changed

1 file changed

+33
-32
lines changed

anchor/common/qbft/src/lib.rs

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,6 @@ where
117117
last_prepared_round: Option<Round>,
118118
last_prepared_value: Option<D::Hash>,
119119

120-
/// Past prepare consensus that we have reached
121-
past_consensus: HashMap<Round, D::Hash>,
122-
123120
/// Aggregated commit message
124121
aggregated_commit: Option<SignedSSVMessage>,
125122

@@ -180,8 +177,6 @@ where
180177
last_prepared_round: None,
181178
last_prepared_value: None,
182179

183-
past_consensus: HashMap::new(),
184-
185180
aggregated_commit: None,
186181

187182
message_sender,
@@ -337,46 +332,55 @@ where
337332
}
338333

339334
/// Justify the round change quorum
340-
/// In order to justify a round change quorum, we find the maximum round of the quorum set that
341-
/// had achieved a past consensus. If we have also seen consensus on this round for the
342-
/// suggested data, then it is justified and this function returns that data.
343-
/// If there is no past consensus data in the round change quorum or we disagree with quorum set
344-
/// this function will return None, and we obtain the data as if we were beginning this
345-
/// instance.
335+
/// Finds the highest prepared value from round change messages and returns it
336+
/// for the proposal.
346337
fn justify_round_change_quorum(&self) -> Option<ValidData<D>> {
347-
// Get all round change messages for the current round
348338
let round_change_messages = self
349339
.round_change_container
350340
.get_messages_for_round(self.current_round);
351341

352-
// If we don't have enough messages for quorum, we can't justify anything
342+
// Need quorum to proceed
353343
if round_change_messages.len() < self.config.quorum_size() {
354344
return None;
355345
}
356346

357-
// Find the highest round that any node claims reached preparation
347+
// Find the round change with the highest prepared round
358348
let highest_prepared = round_change_messages
359349
.iter()
360-
.filter(|msg| msg.qbft_message.data_round != 0) // Only consider messages with prepared data
350+
.filter(|msg| msg.qbft_message.data_round != 0)
361351
.max_by_key(|msg| msg.qbft_message.data_round);
362352

363-
// If we found a message with prepared data
364-
if let Some(highest_msg) = highest_prepared {
365-
// Get the prepared data from the message
366-
let prepared_round = Round::from(highest_msg.qbft_message.data_round);
367-
368-
// Verify we have also seen this consensus
369-
if let Some(hash) = self.past_consensus.get(&prepared_round) {
370-
// We have seen consensus on the data, get the value
371-
let our_data = self.data.get(hash).cloned().unwrap_or_else(|| {
372-
warn!("Previous consensus data missing. Using start value");
373-
self.start_data.clone()
374-
});
375-
return Some(ValidData::new(Some(our_data), *hash));
353+
// If no one prepared anything, return None (will use start data)
354+
let highest_prepared = highest_prepared?;
355+
356+
let claimed_hash = highest_prepared.qbft_message.root;
357+
358+
// First, try to get data from the round change message itself
359+
if !highest_prepared.signed_message.full_data().is_empty() {
360+
// The round change includes the full data - decode and use it
361+
if let Ok(data) = D::from_ssz_bytes(highest_prepared.signed_message.full_data()) {
362+
// Verify the data matches the claimed hash
363+
if data.hash() == claimed_hash {
364+
return Some(ValidData::new(Some(Arc::new(data)), claimed_hash));
365+
} else {
366+
warn!("Round change full data doesn't match claimed hash");
367+
}
368+
} else {
369+
warn!("Failed to decode round change full data");
376370
}
377371
}
378372

379-
// No consensus found
373+
// If we don't have the data in the round change, try our local storage
374+
if let Some(data) = self.data.get(&claimed_hash) {
375+
return Some(ValidData::new(Some(data.clone()), claimed_hash));
376+
}
377+
378+
warn!(
379+
"Missing data for highest prepared value with hash {:?}",
380+
claimed_hash
381+
);
382+
383+
// Return None - will fall back to start data
380384
None
381385
}
382386

@@ -705,9 +709,6 @@ where
705709
self.state = InstanceState::Commit { proposal_root };
706710
debug!(state = ?self.state, "Reached a PREPARE consensus. State updated to COMMIT");
707711

708-
// Record that we have come to a consensus on this value
709-
self.past_consensus.insert(round, hash);
710-
711712
// Record as last prepared value and round
712713
self.last_prepared_value = Some(hash);
713714
self.last_prepared_round = Some(self.current_round);

0 commit comments

Comments
 (0)