From 34e096f458aab72cb2deeaa579ce37ca4b671d8e Mon Sep 17 00:00:00 2001 From: Marcin Date: Wed, 11 Dec 2024 15:10:31 +0100 Subject: [PATCH 01/10] First working version --- consensus/src/dag/reconstruction/dag.rs | 1 - consensus/src/dag/reconstruction/mod.rs | 49 ++++++++++++++++----- consensus/src/dag/reconstruction/parents.rs | 12 ++--- consensus/src/dissemination/responder.rs | 5 +-- consensus/src/extension/election.rs | 18 ++++---- consensus/src/extension/extender.rs | 2 +- consensus/src/extension/units.rs | 2 +- consensus/src/testing/dag.rs | 6 +-- consensus/src/units/mod.rs | 7 ++- consensus/src/units/testing.rs | 5 ++- 10 files changed, 70 insertions(+), 37 deletions(-) diff --git a/consensus/src/dag/reconstruction/dag.rs b/consensus/src/dag/reconstruction/dag.rs index e488b49c..f3b07d8a 100644 --- a/consensus/src/dag/reconstruction/dag.rs +++ b/consensus/src/dag/reconstruction/dag.rs @@ -96,7 +96,6 @@ impl Dag { } let missing_parents = unit .parents() - .values() .filter(|parent| !self.dag_units.contains(parent)) .cloned() .collect(); diff --git a/consensus/src/dag/reconstruction/mod.rs b/consensus/src/dag/reconstruction/mod.rs index ec7b8c1e..fced83c9 100644 --- a/consensus/src/dag/reconstruction/mod.rs +++ b/consensus/src/dag/reconstruction/mod.rs @@ -1,5 +1,5 @@ use std::collections::HashMap; - +use aleph_bft_rmc::NodeCount; use crate::{ units::{ControlHash, FullUnit, HashFor, Unit, UnitCoord, UnitWithParents, WrappedUnit}, Hasher, NodeMap, SessionId, @@ -8,7 +8,7 @@ use crate::{ mod dag; mod parents; -use aleph_bft_types::{Data, MultiKeychain, OrderedUnit, Signed}; +use aleph_bft_types::{Data, MultiKeychain, NodeIndex, OrderedUnit, Round, Signed}; use dag::Dag; use parents::Reconstruction as ParentReconstruction; @@ -16,7 +16,7 @@ use parents::Reconstruction as ParentReconstruction; #[derive(Debug, PartialEq, Eq, Clone)] pub struct ReconstructedUnit { unit: U, - parents: NodeMap>, + parents: NodeMap<(HashFor, Round)>, } impl ReconstructedUnit { @@ -25,7 +25,17 @@ impl ReconstructedUnit { match unit.control_hash().combined_hash == ControlHash::::combine_hashes(&parents) { - true => Ok(ReconstructedUnit { unit, parents }), + true => { + let unit_round = unit.round(); + let mut parents_with_rounds = NodeMap::with_size(parents.size()); + for (parent_index, hash) in parents.into_iter() { + parents_with_rounds.insert(parent_index, (hash, unit_round - 1)); + } + Ok(ReconstructedUnit { + unit, + parents: parents_with_rounds, + }) + }, false => Err(unit), } } @@ -72,8 +82,25 @@ impl WrappedUnit for ReconstructedUnit { } impl UnitWithParents for ReconstructedUnit { - fn parents(&self) -> &NodeMap> { - &self.parents + fn parents(&self) -> impl Iterator> { + self.parents.values().map(|(hash, _)| hash) + } + + fn direct_parents(&self) -> impl Iterator> { + self.parents.values().filter_map(|(hash, parent_round)| { + match self.unit.coord().round() { + 0 => None, + unit_round => if unit_round - 1 == *parent_round { Some(hash) } else { None }, + } + }) + } + + fn parent_for(&self, index: NodeIndex) -> Option<&HashFor> { + self.parents.get(index).map(|(hash, _)| hash) + } + + fn parents_size(&self) -> NodeCount { + self.parents.size() } } @@ -89,7 +116,7 @@ impl From { fn from(unit: ReconstructedUnit, K>>) -> Self { - let parents = unit.parents().values().cloned().collect(); + let parents = unit.parents().cloned().collect(); let unit = unit.unpack(); let creator = unit.creator(); let round = unit.round(); @@ -233,7 +260,7 @@ mod test { assert_eq!(units.len(), 1); let reconstructed_unit = units.pop().expect("just checked its there"); assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone())); - assert_eq!(reconstructed_unit.parents().item_count(), 0); + assert_eq!(reconstructed_unit.parents().count(), 0); } } @@ -254,15 +281,15 @@ mod test { match round { 0 => { assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone())); - assert_eq!(reconstructed_unit.parents().item_count(), 0); + assert_eq!(reconstructed_unit.parents().count(), 0); } round => { - assert_eq!(reconstructed_unit.parents().item_count(), 4); + assert_eq!(reconstructed_unit.parents().count(), 4); let parents = dag .get((round - 1) as usize) .expect("the parents are there"); for (parent, reconstructed_parent) in - parents.iter().zip(reconstructed_unit.parents().values()) + parents.iter().zip(reconstructed_unit.parents()) { assert_eq!(&parent.hash(), reconstructed_parent); } diff --git a/consensus/src/dag/reconstruction/parents.rs b/consensus/src/dag/reconstruction/parents.rs index ac1ea954..820c5bf8 100644 --- a/consensus/src/dag/reconstruction/parents.rs +++ b/consensus/src/dag/reconstruction/parents.rs @@ -237,7 +237,7 @@ mod test { assert_eq!(units.len(), 1); let reconstructed_unit = units.pop().expect("just checked its there"); assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone())); - assert_eq!(reconstructed_unit.parents().item_count(), 0); + assert_eq!(reconstructed_unit.parents().count(), 0); } } @@ -258,15 +258,15 @@ mod test { match round { 0 => { assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone())); - assert_eq!(reconstructed_unit.parents().item_count(), 0); + assert_eq!(reconstructed_unit.parents().count(), 0); } round => { - assert_eq!(reconstructed_unit.parents().item_count(), 4); + assert_eq!(reconstructed_unit.parents().count(), 4); let parents = dag .get((round - 1) as usize) .expect("the parents are there"); for (parent, reconstructed_parent) in - parents.iter().zip(reconstructed_unit.parents().values()) + parents.iter().zip(reconstructed_unit.parents()) { assert_eq!(&parent.hash(), reconstructed_parent); } @@ -371,11 +371,11 @@ mod test { assert!(requests.is_empty()); assert_eq!(units.len(), 1); let reconstructed_unit = units.pop().expect("just checked its there"); - assert_eq!(reconstructed_unit.parents().item_count(), 4); + assert_eq!(reconstructed_unit.parents().count(), 4); for (coord, parent_hash) in parent_hashes { assert_eq!( Some(&parent_hash), - reconstructed_unit.parents().get(coord.creator()) + reconstructed_unit.parent_for(coord.creator()) ); } } diff --git a/consensus/src/dissemination/responder.rs b/consensus/src/dissemination/responder.rs index ae01602e..95ab5de8 100644 --- a/consensus/src/dissemination/responder.rs +++ b/consensus/src/dissemination/responder.rs @@ -57,7 +57,6 @@ impl Responder { .map(|unit| { let parents = unit .parents() - .values() .map(|parent_hash| { units .unit(parent_hash) @@ -270,8 +269,8 @@ mod test { match response { Response::Parents(response_hash, parents) => { assert_eq!(response_hash, requested_unit.hash()); - assert_eq!(parents.len(), requested_unit.parents().size().0); - for (parent, parent_hash) in zip(parents, requested_unit.parents().values()) { + assert_eq!(parents.len(), requested_unit.parents().count()); + for (parent, parent_hash) in zip(parents, requested_unit.parents()) { assert_eq!(&parent.as_signable().hash(), parent_hash); } } diff --git a/consensus/src/extension/election.rs b/consensus/src/extension/election.rs index c2afc173..ba9804dd 100644 --- a/consensus/src/extension/election.rs +++ b/consensus/src/extension/election.rs @@ -49,11 +49,11 @@ impl CandidateElection { fn parent_votes( &mut self, - parents: &NodeMap>, + parents: Vec>, ) -> Result<(NodeCount, NodeCount), CandidateOutcome> { let (mut votes_for, mut votes_against) = (NodeCount(0), NodeCount(0)); - for parent in parents.values() { - match self.votes.get(parent).expect("units are added in order") { + for parent in parents { + match self.votes.get(&parent).expect("units are added in order") { true => votes_for += NodeCount(1), false => votes_against += NodeCount(1), } @@ -63,11 +63,12 @@ impl CandidateElection { fn vote_from_parents( &mut self, - parents: &NodeMap>, + parents: Vec>, + parents_size: NodeCount, relative_round: Round, ) -> Result> { use CandidateOutcome::*; - let threshold = parents.size().consensus_threshold(); + let threshold = parents_size.consensus_threshold(); // Gather parents' votes. let (votes_for, votes_against) = self.parent_votes(parents)?; assert!(votes_for + votes_against >= threshold); @@ -102,12 +103,13 @@ impl CandidateElection { return Ok(()); } let relative_round = voter.round() - self.round; + let direct_parents = voter.direct_parents().cloned().collect::>(); let vote = match relative_round { 0 => unreachable!("just checked that voter and election rounds are not equal"), // Direct descendands vote for, all other units of that round against. - 1 => voter.parents().get(self.candidate_creator) == Some(&self.candidate_hash), + 1 => voter.parent_for(self.candidate_creator) == Some(&self.candidate_hash), // Otherwise we compute the vote based on the parents' votes. - _ => self.vote_from_parents(voter.parents(), relative_round)?, + _ => self.vote_from_parents(direct_parents, voter.parents_size(), relative_round)?, }; self.votes.insert(voter.hash(), vote); Ok(()) @@ -360,7 +362,7 @@ mod test { #[test] #[ignore] - // TODO(A0-4563) Uncomment after changes to parent voting code + // TODO(A0-4559) Uncomment fn given_minimal_dag_with_orphaned_node_when_electing_then_orphaned_node_is_not_head() { use ElectionResult::*; let mut units = Units::new(); diff --git a/consensus/src/extension/extender.rs b/consensus/src/extension/extender.rs index bf556023..7a8f5fb6 100644 --- a/consensus/src/extension/extender.rs +++ b/consensus/src/extension/extender.rs @@ -101,7 +101,7 @@ mod test { #[test] #[ignore] - // TODO(A0-4563) Uncomment after changes to parent voting code + // TODO(A0-4559) Uncomment fn given_minimal_dag_with_orphaned_node_when_producing_batches_have_correct_length() { let mut extender = Extender::new(); let n_members = NodeCount(4); diff --git a/consensus/src/extension/units.rs b/consensus/src/extension/units.rs index ca2b42f6..b7e5721d 100644 --- a/consensus/src/extension/units.rs +++ b/consensus/src/extension/units.rs @@ -64,7 +64,7 @@ impl Units { .expect("head is picked among units we have"), ); while let Some(u) = queue.pop_front() { - for u_hash in u.parents().clone().into_values() { + for u_hash in u.parents() { if let Some(v) = self.units.remove(&u_hash) { queue.push_back(v); } diff --git a/consensus/src/testing/dag.rs b/consensus/src/testing/dag.rs index 30023b26..8cd85d58 100644 --- a/consensus/src/testing/dag.rs +++ b/consensus/src/testing/dag.rs @@ -139,7 +139,7 @@ impl DagFeeder { fn on_reconstructed_unit(&mut self, unit: ReconstructedUnit) { let h = unit.hash(); - let parents = unit.parents(); + let parents = unit.parents().collect::>(); let expected_hashes: HashSet<_> = self .units_map .get(&h) @@ -147,8 +147,8 @@ impl DagFeeder { .parent_hashes() .into_iter() .collect(); - assert_eq!(parents.item_count(), expected_hashes.len()); - for (_, hash) in parents { + assert_eq!(parents.len(), expected_hashes.len()); + for hash in parents { assert!(expected_hashes.contains(hash)); } self.result.push(unit.clone()); diff --git a/consensus/src/units/mod.rs b/consensus/src/units/mod.rs index 285bfa13..a36e23ba 100644 --- a/consensus/src/units/mod.rs +++ b/consensus/src/units/mod.rs @@ -215,9 +215,14 @@ pub trait WrappedUnit: Unit { } pub trait UnitWithParents: Unit { - fn parents(&self) -> &NodeMap>; + fn parents(&self) -> impl Iterator>; + fn direct_parents(&self) -> impl Iterator>; + fn parent_for(&self, index: NodeIndex) -> Option<&HashFor>; + fn parents_size(&self) -> NodeCount; } + + impl Unit for FullUnit { type Hasher = H; diff --git a/consensus/src/units/testing.rs b/consensus/src/units/testing.rs index 14ad0076..1e6bd420 100644 --- a/consensus/src/units/testing.rs +++ b/consensus/src/units/testing.rs @@ -241,7 +241,7 @@ pub fn minimal_reconstructed_dag_units_up_to( .clone(); let inactive_node = inactive_node_first_and_last_seen_unit.creator(); for r in 1..=round { - let mut parents: Vec = dag + let direct_parents: Vec = dag .last() .expect("previous round present") .clone() @@ -250,6 +250,7 @@ pub fn minimal_reconstructed_dag_units_up_to( .choose_multiple(&mut rng, threshold) .into_iter() .collect(); + let mut parents = direct_parents.clone(); if r == round { let ancestor_unit = dag .first() @@ -262,7 +263,7 @@ pub fn minimal_reconstructed_dag_units_up_to( .into_iterator() .filter(|node_id| node_id != &inactive_node) .map(|node_id| { - random_reconstructed_unit_with_parents(node_id, &parents, &keychains[node_id.0], r) + random_reconstructed_unit_with_parents(node_id, &parents, &keychains[node_id.0], r) }) .collect(); dag.push(units); From dd5a1124b26cf84c198199785acc678f8aff1772 Mon Sep 17 00:00:00 2001 From: Marcin Date: Wed, 11 Dec 2024 15:10:58 +0100 Subject: [PATCH 02/10] fmt --- consensus/src/dag/reconstruction/mod.rs | 28 +++++++++++++++---------- consensus/src/units/mod.rs | 2 -- consensus/src/units/testing.rs | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/consensus/src/dag/reconstruction/mod.rs b/consensus/src/dag/reconstruction/mod.rs index fced83c9..05fc4f37 100644 --- a/consensus/src/dag/reconstruction/mod.rs +++ b/consensus/src/dag/reconstruction/mod.rs @@ -1,9 +1,9 @@ -use std::collections::HashMap; -use aleph_bft_rmc::NodeCount; use crate::{ units::{ControlHash, FullUnit, HashFor, Unit, UnitCoord, UnitWithParents, WrappedUnit}, Hasher, NodeMap, SessionId, }; +use aleph_bft_rmc::NodeCount; +use std::collections::HashMap; mod dag; mod parents; @@ -27,15 +27,15 @@ impl ReconstructedUnit { { true => { let unit_round = unit.round(); - let mut parents_with_rounds = NodeMap::with_size(parents.size()); + let mut parents_with_rounds = NodeMap::with_size(parents.size()); for (parent_index, hash) in parents.into_iter() { parents_with_rounds.insert(parent_index, (hash, unit_round - 1)); } Ok(ReconstructedUnit { - unit, - parents: parents_with_rounds, + unit, + parents: parents_with_rounds, }) - }, + } false => Err(unit), } } @@ -87,12 +87,18 @@ impl UnitWithParents for ReconstructedUnit { } fn direct_parents(&self) -> impl Iterator> { - self.parents.values().filter_map(|(hash, parent_round)| { - match self.unit.coord().round() { + self.parents + .values() + .filter_map(|(hash, parent_round)| match self.unit.coord().round() { 0 => None, - unit_round => if unit_round - 1 == *parent_round { Some(hash) } else { None }, - } - }) + unit_round => { + if unit_round - 1 == *parent_round { + Some(hash) + } else { + None + } + } + }) } fn parent_for(&self, index: NodeIndex) -> Option<&HashFor> { diff --git a/consensus/src/units/mod.rs b/consensus/src/units/mod.rs index a36e23ba..a3618ecb 100644 --- a/consensus/src/units/mod.rs +++ b/consensus/src/units/mod.rs @@ -221,8 +221,6 @@ pub trait UnitWithParents: Unit { fn parents_size(&self) -> NodeCount; } - - impl Unit for FullUnit { type Hasher = H; diff --git a/consensus/src/units/testing.rs b/consensus/src/units/testing.rs index 1e6bd420..8092473b 100644 --- a/consensus/src/units/testing.rs +++ b/consensus/src/units/testing.rs @@ -263,7 +263,7 @@ pub fn minimal_reconstructed_dag_units_up_to( .into_iterator() .filter(|node_id| node_id != &inactive_node) .map(|node_id| { - random_reconstructed_unit_with_parents(node_id, &parents, &keychains[node_id.0], r) + random_reconstructed_unit_with_parents(node_id, &parents, &keychains[node_id.0], r) }) .collect(); dag.push(units); From 817653ab25890b4c88ab361a8fd1f253dd29f6c2 Mon Sep 17 00:00:00 2001 From: Marcin Date: Wed, 11 Dec 2024 15:12:18 +0100 Subject: [PATCH 03/10] clippy --- consensus/src/extension/election.rs | 2 +- consensus/src/extension/units.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/src/extension/election.rs b/consensus/src/extension/election.rs index ba9804dd..2f57d8cb 100644 --- a/consensus/src/extension/election.rs +++ b/consensus/src/extension/election.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use crate::{ extension::units::Units, units::{HashFor, UnitWithParents}, - Hasher, NodeCount, NodeIndex, NodeMap, Round, + Hasher, NodeCount, NodeIndex, Round, }; fn common_vote(relative_round: Round) -> bool { diff --git a/consensus/src/extension/units.rs b/consensus/src/extension/units.rs index b7e5721d..e9deacf9 100644 --- a/consensus/src/extension/units.rs +++ b/consensus/src/extension/units.rs @@ -65,7 +65,7 @@ impl Units { ); while let Some(u) = queue.pop_front() { for u_hash in u.parents() { - if let Some(v) = self.units.remove(&u_hash) { + if let Some(v) = self.units.remove(u_hash) { queue.push_back(v); } } From c3017cf9243a75cc7c5827e2020e76155a9669f2 Mon Sep 17 00:00:00 2001 From: Marcin Date: Thu, 12 Dec 2024 08:41:04 +0100 Subject: [PATCH 04/10] Bump consensus crate version --- Cargo.lock | 2 +- consensus/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe8bf2fb..27450abc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -28,7 +28,7 @@ dependencies = [ [[package]] name = "aleph-bft" -version = "0.39.1" +version = "0.40.0" dependencies = [ "aleph-bft-mock", "aleph-bft-rmc", diff --git a/consensus/Cargo.toml b/consensus/Cargo.toml index 300a89a3..609ae3c1 100644 --- a/consensus/Cargo.toml +++ b/consensus/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "aleph-bft" -version = "0.39.1" +version = "0.40.0" edition = "2021" authors = ["Cardinal Cryptography"] categories = ["algorithms", "data-structures", "cryptography", "database"] From cad83b913c7cbdc3493c1ce4edaa96649e256a1d Mon Sep 17 00:00:00 2001 From: Marcin Date: Thu, 12 Dec 2024 08:42:14 +0100 Subject: [PATCH 05/10] Updated readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0a78b290..0a70d547 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ More details are available [in the book][reference-link-implementation-details]. - Import AlephBFT in your crate ```toml [dependencies] - aleph-bft = "^0.39" + aleph-bft = "^0.40" ``` - The main entry point is the `run_session` function, which returns a Future that runs the consensus algorithm. From 73b4233bc70a54d80e28880ef816f21ecd5cbf17 Mon Sep 17 00:00:00 2001 From: Marcin Date: Thu, 12 Dec 2024 08:43:40 +0100 Subject: [PATCH 06/10] Revered changes to unit testing code --- consensus/src/units/testing.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/consensus/src/units/testing.rs b/consensus/src/units/testing.rs index 8092473b..14ad0076 100644 --- a/consensus/src/units/testing.rs +++ b/consensus/src/units/testing.rs @@ -241,7 +241,7 @@ pub fn minimal_reconstructed_dag_units_up_to( .clone(); let inactive_node = inactive_node_first_and_last_seen_unit.creator(); for r in 1..=round { - let direct_parents: Vec = dag + let mut parents: Vec = dag .last() .expect("previous round present") .clone() @@ -250,7 +250,6 @@ pub fn minimal_reconstructed_dag_units_up_to( .choose_multiple(&mut rng, threshold) .into_iter() .collect(); - let mut parents = direct_parents.clone(); if r == round { let ancestor_unit = dag .first() From 6ab0ddb8dabf794f461479aae66d2a7f9d78dbeb Mon Sep 17 00:00:00 2001 From: Marcin Date: Mon, 16 Dec 2024 12:10:58 +0100 Subject: [PATCH 07/10] Review --- consensus/src/dag/reconstruction/mod.rs | 7 +++++-- consensus/src/extension/election.rs | 10 ++++++---- consensus/src/testing/dag.rs | 8 +++----- consensus/src/units/mod.rs | 2 +- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/consensus/src/dag/reconstruction/mod.rs b/consensus/src/dag/reconstruction/mod.rs index 05fc4f37..a069029b 100644 --- a/consensus/src/dag/reconstruction/mod.rs +++ b/consensus/src/dag/reconstruction/mod.rs @@ -29,7 +29,8 @@ impl ReconstructedUnit { let unit_round = unit.round(); let mut parents_with_rounds = NodeMap::with_size(parents.size()); for (parent_index, hash) in parents.into_iter() { - parents_with_rounds.insert(parent_index, (hash, unit_round - 1)); + // we cannot have here round 0 units + parents_with_rounds.insert(parent_index, (hash, unit_round.saturating_sub(1))); } Ok(ReconstructedUnit { unit, @@ -90,7 +91,9 @@ impl UnitWithParents for ReconstructedUnit { self.parents .values() .filter_map(|(hash, parent_round)| match self.unit.coord().round() { + // round 0 units cannot have non-empty parents 0 => None, + unit_round => { if unit_round - 1 == *parent_round { Some(hash) @@ -105,7 +108,7 @@ impl UnitWithParents for ReconstructedUnit { self.parents.get(index).map(|(hash, _)| hash) } - fn parents_size(&self) -> NodeCount { + fn node_count(&self) -> NodeCount { self.parents.size() } } diff --git a/consensus/src/extension/election.rs b/consensus/src/extension/election.rs index 2f57d8cb..b69d7bc9 100644 --- a/consensus/src/extension/election.rs +++ b/consensus/src/extension/election.rs @@ -64,11 +64,10 @@ impl CandidateElection { fn vote_from_parents( &mut self, parents: Vec>, - parents_size: NodeCount, + threshold: NodeCount, relative_round: Round, ) -> Result> { use CandidateOutcome::*; - let threshold = parents_size.consensus_threshold(); // Gather parents' votes. let (votes_for, votes_against) = self.parent_votes(parents)?; assert!(votes_for + votes_against >= threshold); @@ -103,13 +102,16 @@ impl CandidateElection { return Ok(()); } let relative_round = voter.round() - self.round; - let direct_parents = voter.direct_parents().cloned().collect::>(); + let direct_parents = voter.direct_parents().cloned().collect(); let vote = match relative_round { 0 => unreachable!("just checked that voter and election rounds are not equal"), // Direct descendands vote for, all other units of that round against. 1 => voter.parent_for(self.candidate_creator) == Some(&self.candidate_hash), // Otherwise we compute the vote based on the parents' votes. - _ => self.vote_from_parents(direct_parents, voter.parents_size(), relative_round)?, + _ => { + let threshold = voter.node_count().consensus_threshold(); + self.vote_from_parents(direct_parents, threshold, relative_round)? + }, }; self.votes.insert(voter.hash(), vote); Ok(()) diff --git a/consensus/src/testing/dag.rs b/consensus/src/testing/dag.rs index 8cd85d58..5d894380 100644 --- a/consensus/src/testing/dag.rs +++ b/consensus/src/testing/dag.rs @@ -139,7 +139,7 @@ impl DagFeeder { fn on_reconstructed_unit(&mut self, unit: ReconstructedUnit) { let h = unit.hash(); - let parents = unit.parents().collect::>(); + let parents = unit.parents().cloned().collect::>(); let expected_hashes: HashSet<_> = self .units_map .get(&h) @@ -147,10 +147,8 @@ impl DagFeeder { .parent_hashes() .into_iter() .collect(); - assert_eq!(parents.len(), expected_hashes.len()); - for hash in parents { - assert!(expected_hashes.contains(hash)); - } + + assert_eq!(parents.into_iter().collect::>(), expected_hashes); self.result.push(unit.clone()); self.store.insert(unit); } diff --git a/consensus/src/units/mod.rs b/consensus/src/units/mod.rs index a3618ecb..ea251aac 100644 --- a/consensus/src/units/mod.rs +++ b/consensus/src/units/mod.rs @@ -218,7 +218,7 @@ pub trait UnitWithParents: Unit { fn parents(&self) -> impl Iterator>; fn direct_parents(&self) -> impl Iterator>; fn parent_for(&self, index: NodeIndex) -> Option<&HashFor>; - fn parents_size(&self) -> NodeCount; + fn node_count(&self) -> NodeCount; } impl Unit for FullUnit { From 1a5dcf859503763b3f80bca0426272473043cb5b Mon Sep 17 00:00:00 2001 From: Marcin Date: Mon, 16 Dec 2024 12:11:42 +0100 Subject: [PATCH 08/10] fmt --- consensus/src/dag/reconstruction/mod.rs | 2 +- consensus/src/extension/election.rs | 2 +- consensus/src/testing/dag.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/consensus/src/dag/reconstruction/mod.rs b/consensus/src/dag/reconstruction/mod.rs index a069029b..120853d0 100644 --- a/consensus/src/dag/reconstruction/mod.rs +++ b/consensus/src/dag/reconstruction/mod.rs @@ -93,7 +93,7 @@ impl UnitWithParents for ReconstructedUnit { .filter_map(|(hash, parent_round)| match self.unit.coord().round() { // round 0 units cannot have non-empty parents 0 => None, - + unit_round => { if unit_round - 1 == *parent_round { Some(hash) diff --git a/consensus/src/extension/election.rs b/consensus/src/extension/election.rs index b69d7bc9..e60ff8d8 100644 --- a/consensus/src/extension/election.rs +++ b/consensus/src/extension/election.rs @@ -111,7 +111,7 @@ impl CandidateElection { _ => { let threshold = voter.node_count().consensus_threshold(); self.vote_from_parents(direct_parents, threshold, relative_round)? - }, + } }; self.votes.insert(voter.hash(), vote); Ok(()) diff --git a/consensus/src/testing/dag.rs b/consensus/src/testing/dag.rs index 5d894380..ce8ae023 100644 --- a/consensus/src/testing/dag.rs +++ b/consensus/src/testing/dag.rs @@ -147,7 +147,7 @@ impl DagFeeder { .parent_hashes() .into_iter() .collect(); - + assert_eq!(parents.into_iter().collect::>(), expected_hashes); self.result.push(unit.clone()); self.store.insert(unit); From b60105a2e2ae543dbff6cb72c0e9d262131da5c8 Mon Sep 17 00:00:00 2001 From: Marcin Date: Mon, 23 Dec 2024 14:10:18 +0100 Subject: [PATCH 09/10] Review --- consensus/src/extension/election.rs | 2 +- consensus/src/testing/dag.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/consensus/src/extension/election.rs b/consensus/src/extension/election.rs index e60ff8d8..8ef87bca 100644 --- a/consensus/src/extension/election.rs +++ b/consensus/src/extension/election.rs @@ -102,7 +102,6 @@ impl CandidateElection { return Ok(()); } let relative_round = voter.round() - self.round; - let direct_parents = voter.direct_parents().cloned().collect(); let vote = match relative_round { 0 => unreachable!("just checked that voter and election rounds are not equal"), // Direct descendands vote for, all other units of that round against. @@ -110,6 +109,7 @@ impl CandidateElection { // Otherwise we compute the vote based on the parents' votes. _ => { let threshold = voter.node_count().consensus_threshold(); + let direct_parents = voter.direct_parents().cloned().collect(); self.vote_from_parents(direct_parents, threshold, relative_round)? } }; diff --git a/consensus/src/testing/dag.rs b/consensus/src/testing/dag.rs index ce8ae023..86f7b227 100644 --- a/consensus/src/testing/dag.rs +++ b/consensus/src/testing/dag.rs @@ -139,7 +139,7 @@ impl DagFeeder { fn on_reconstructed_unit(&mut self, unit: ReconstructedUnit) { let h = unit.hash(); - let parents = unit.parents().cloned().collect::>(); + let parents: HashSet<_> = unit.parents().cloned().collect(); let expected_hashes: HashSet<_> = self .units_map .get(&h) @@ -148,7 +148,7 @@ impl DagFeeder { .into_iter() .collect(); - assert_eq!(parents.into_iter().collect::>(), expected_hashes); + assert_eq!(parents, expected_hashes); self.result.push(unit.clone()); self.store.insert(unit); } From f569e47305d70f82d7211800415ecebcbdebb872 Mon Sep 17 00:00:00 2001 From: Marcin Date: Mon, 23 Dec 2024 14:40:01 +0100 Subject: [PATCH 10/10] Try to fix cargo audit pipeline --- .github/workflows/cargo-audit.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/cargo-audit.yml b/.github/workflows/cargo-audit.yml index c6c9a2a6..ee92cc64 100644 --- a/.github/workflows/cargo-audit.yml +++ b/.github/workflows/cargo-audit.yml @@ -20,6 +20,11 @@ jobs: - name: Checkout source code uses: actions/checkout@v4 + - name: Install cargo audit + shell: bash + run: | + cargo install cargo-audit --locked + - name: Run `cargo-audit` uses: actions-rs/audit-check@v1 with: