Skip to content

Commit

Permalink
[Narwhal] Add checks to the DAG and panic if fail (MystenLabs#5173)
Browse files Browse the repository at this point in the history
* Add checks to the DAG and panic if fail
* Take into account gc bound
* Add more log lines to understand
* Do not panic if the round does not exist

Co-authored-by: George Danezis <george@danez.is>
  • Loading branch information
gdanezis and George Danezis authored Oct 13, 2022
1 parent 04e1e18 commit 25f5a9f
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 7 deletions.
10 changes: 5 additions & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 38 additions & 2 deletions narwhal/consensus/src/bullshark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ use crate::{
};
use config::{Committee, Stake};
use fastcrypto::{traits::EncodeDecodeBase64, Hash};
use std::{collections::HashMap, sync::Arc};
use tracing::debug;
use std::{
collections::{BTreeSet, HashMap},
sync::Arc,
};
use tracing::{debug, error};
use types::{Certificate, CertificateDigest, ConsensusStore, Round, SequenceNumber, StoreResult};

#[cfg(test)]
Expand All @@ -35,6 +38,37 @@ impl ConsensusProtocol for Bullshark {
let round = certificate.round();
let mut consensus_index = consensus_index;

// We must have stored already the parents of this certiciate!
if round > 0 {
let parents = certificate.header.parents.clone();
if let Some(round_table) = state.dag.get(&(round - 1)) {
let store_parents: BTreeSet<&CertificateDigest> =
round_table.iter().map(|(_, (digest, _))| digest).collect();

for parent_digest in parents {
if !store_parents.contains(&parent_digest) {
if round - 1 + self.gc_depth > state.last_committed_round {
error!(
"The store does not contain the parent of {:?}: Missing item digest={:?}",
certificate, parent_digest
);
} else {
debug!(
"The store does not contain the parent of {:?}: Missing item digest={:?} (but below GC round)",
certificate, parent_digest
);
}
}
}
} else {
error!(
"Round not present in Dag store: {:?} when looking for parents of {:?}",
round - 1,
certificate
);
}
}

// Add the new certificate to the local storage.
state
.dag
Expand Down Expand Up @@ -90,6 +124,8 @@ impl ConsensusProtocol for Bullshark {
.iter()
.rev()
{
debug!("Previous Leader {:?} has enough support", leader);

// Starting from the oldest leader, flatten the sub-dag referenced by the leader.
for x in utils::order_dag(self.gc_depth, leader, state) {
let digest = x.digest();
Expand Down
2 changes: 2 additions & 0 deletions narwhal/consensus/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ where
// Output the sequence in the right order.
for output in sequence {
let certificate = &output.certificate;
tracing::debug!("Commit in Sequence {:?}", output);

#[cfg(not(feature = "benchmark"))]
if output.consensus_index % 5_000 == 0 {
tracing::debug!("Committed {}", certificate.header);
Expand Down

0 comments on commit 25f5a9f

Please sign in to comment.