Skip to content

Commit

Permalink
applied comments
Browse files Browse the repository at this point in the history
  • Loading branch information
pompon0 committed Dec 13, 2023
1 parent 8768646 commit 27a37b3
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 32 deletions.
17 changes: 5 additions & 12 deletions node/actors/bft/src/leader/replica_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,12 @@ impl StateMachine {
// ----------- All checks finished. Now we process the message. --------------

// We store the message in our cache.
self.commit_message_cache
.entry(message.view)
.or_default()
.insert(author.clone(), signed_message);
let cache_entry = self.commit_message_cache.entry(message.view).or_default();
cache_entry.insert(author.clone(), signed_message);

// Now we check if we have enough messages to continue.
let mut by_proposal: HashMap<_, Vec<_>> = HashMap::new();
for msg in self
.commit_message_cache
.get(&message.view)
.unwrap()
.values()
{
for msg in cache_entry.values() {
by_proposal.entry(msg.msg.proposal).or_default().push(msg);
}
let Some((_, replica_messages)) = by_proposal
Expand All @@ -127,7 +120,7 @@ impl StateMachine {
else {
return Ok(());
};
debug_assert!(replica_messages.len() == self.inner.threshold());
debug_assert_eq!(replica_messages.len(), self.inner.threshold());

// ----------- Update the state machine --------------

Expand All @@ -143,7 +136,7 @@ impl StateMachine {

// Create the justification for our message.
let justification = validator::CommitQC::from(
&replica_messages.into_iter().cloned().collect::<Vec<_>>()[..],
&replica_messages.into_iter().cloned().collect::<Vec<_>>(),
&self.inner.validator_set,
)
.expect("Couldn't create justification from valid replica messages!");
Expand Down
2 changes: 1 addition & 1 deletion node/actors/bft/src/leader/replica_prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ impl StateMachine {
.into_values()
.collect();

debug_assert!(num_messages == self.inner.threshold());
debug_assert_eq!(num_messages, self.inner.threshold());

// ----------- Update the state machine --------------

Expand Down
8 changes: 3 additions & 5 deletions node/actors/bft/src/leader/state_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl StateMachine {
if prepare_qc.view() < next_view {
continue;
};
next_view = prepare_qc.view();
next_view = prepare_qc.view().next();
Self::propose(ctx, inner, payload_source, prepare_qc).await?;
}
}
Expand All @@ -128,16 +128,14 @@ impl StateMachine {
// Get the highest block voted for and check if there's a quorum of votes for it. To have a quorum
// in this situation, we require 2*f+1 votes, where f is the maximum number of faulty replicas.
let mut count: HashMap<_, usize> = HashMap::new();

for (vote, signers) in justification.map.iter() {
for (vote, signers) in &justification.map {
*count.entry(vote.high_vote.proposal).or_default() += signers.len();
}

let highest_vote: Option<validator::BlockHeader> = count
.iter()
// We only take one value from the iterator because there can only be at most one block with a quorum of 2f+1 votes.
.find(|(_, v)| **v > 2 * inner.faulty_replicas())
.map(|(h, _)| h)
.find_map(|(h, v)| (*v > 2 * inner.faulty_replicas()).then_some(h))
.cloned();

// Get the highest CommitQC.
Expand Down
2 changes: 1 addition & 1 deletion node/actors/bft/src/leader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ async fn replica_prepare_during_commit() {
current_view,
current_phase: Phase::Commit,
}) => {
assert_eq!(current_view,util.replica.view);
assert_eq!(current_view, util.replica.view);
}
);
}
Expand Down
4 changes: 2 additions & 2 deletions node/actors/bft/src/testonly/make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ impl PayloadSource for RandomPayloadSource {
}

/// Never provides a payload.
pub struct InavailablePayloadSource;
pub struct UnavailablePayloadSource;

#[async_trait::async_trait]
impl PayloadSource for InavailablePayloadSource {
impl PayloadSource for UnavailablePayloadSource {
async fn propose(
&self,
ctx: &ctx::Ctx,
Expand Down
2 changes: 1 addition & 1 deletion node/actors/bft/src/testonly/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub(crate) enum Behavior {
impl Behavior {
pub(crate) fn payload_source(&self) -> Box<dyn crate::PayloadSource> {
match self {
Self::HonestNotProposing => Box::new(testonly::InavailablePayloadSource),
Self::HonestNotProposing => Box::new(testonly::UnavailablePayloadSource),
_ => Box::new(testonly::RandomPayloadSource),
}
}
Expand Down
10 changes: 0 additions & 10 deletions node/actors/bft/src/testonly/ut_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,16 +251,6 @@ impl UTHarness {
match (i + 1).cmp(&want_threshold) {
Ordering::Equal => res.unwrap(),
Ordering::Less => res.unwrap(),
/*assert_matches!(
res,
Err(ReplicaCommitError::NumReceivedBelowThreshold {
num_messages,
threshold,
}) => {
assert_eq!(num_messages, i+1);
assert_eq!(threshold, want_threshold)
}
),*/
Ordering::Greater => assert_matches!(res, Err(ReplicaCommitError::Old { .. })),
}
}
Expand Down

0 comments on commit 27a37b3

Please sign in to comment.