Skip to content
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

Jump to bottom

[dag][bugfix] fix bitmask for incomplete first round #9723

Merged
merged 1 commit into from
Aug 24, 2023
Merged

[dag][bugfix] fix bitmask for incomplete first round #9723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 9 additions & 9 deletions consensus/src/dag/dag_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,22 +271,22 @@ impl Dag {
}
}

pub fn lowest_incomplete_round(&self) -> Option<Round> {
pub fn lowest_incomplete_round(&self) -> Round {
if self.nodes_by_round.is_empty() {
return self.lowest_round();
}
Copy link
Contributor

@sasha8 sasha8 Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

nit: if the DAG is empty you return Some(lowest_round), but if the DAG is full you return None. The question is why empty DAG is not considered full. In the sense that all rounds either have all nodes or none?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, removed the option and now I return the actual round that is incomplete. For a full dag, its highest_round+1, and an empty dag, its lowest_round.


for (round, round_nodes) in &self.nodes_by_round {
if round_nodes.iter().any(|node| node.is_none()) {
return Some(*round);
return *round;
}
}
None

self.highest_round() + 1
}

pub fn bitmask(&self, target_round: Round) -> DagSnapshotBitmask {
let lowest_round = match self.lowest_incomplete_round() {
Some(round) => round,
None => {
return DagSnapshotBitmask::new(self.highest_round() + 1, vec![]);
},
};
let lowest_round = self.lowest_incomplete_round();

let bitmask = self
.nodes_by_round
Expand Down
5 changes: 2 additions & 3 deletions consensus/src/dag/tests/dag_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,14 @@ fn test_dag_recover_from_storage() {
fn test_dag_bitmask() {
let (signers, epoch_state, mut dag, _) = setup();

let mut metadatas = vec![];
assert_eq!(dag.bitmask(15), DagSnapshotBitmask::new(1, vec![]));

for round in 1..5 {
let parents = dag
.get_strong_links_for_round(round, &epoch_state.verifier)
.unwrap_or_default();
for signer in &signers[0..3] {
let node = new_certified_node(round, signer.author(), parents.clone());
metadatas.push(node.metadata().clone());
assert!(dag.add_node(node).is_ok());
}
}
Expand All @@ -226,12 +225,12 @@ fn test_dag_bitmask() {
DagSnapshotBitmask::new(1, vec![vec![true, true, true, false]; 4])
);

// Populate the fourth author for all rounds
for round in 1..5 {
let parents = dag
.get_strong_links_for_round(round, &epoch_state.verifier)
.unwrap_or_default();
let node = new_certified_node(round, signers[3].author(), parents.clone());
metadatas.push(node.metadata().clone());
assert!(dag.add_node(node).is_ok());
}
assert_eq!(dag.bitmask(15), DagSnapshotBitmask::new(5, vec![]));
Expand Down
Loading