From 9e7d2ce9fd747c12ed47517bddfcb5be17c9fa13 Mon Sep 17 00:00:00 2001 From: Marcin Date: Tue, 14 Jan 2025 11:34:14 +0100 Subject: [PATCH] Moved ControlHash to separate module, added some basic tests --- consensus/src/alerts/handler.rs | 3 +- consensus/src/units/control_hash.rs | 93 +++++++++++++++++++++++++++++ consensus/src/units/mod.rs | 59 ++---------------- 3 files changed, 101 insertions(+), 54 deletions(-) create mode 100644 consensus/src/units/control_hash.rs diff --git a/consensus/src/alerts/handler.rs b/consensus/src/alerts/handler.rs index 214f462e..3cb0b61c 100644 --- a/consensus/src/alerts/handler.rs +++ b/consensus/src/alerts/handler.rs @@ -277,12 +277,13 @@ impl Handler { #[cfg(test)] mod tests { + use crate::units::ControlHash; use crate::{ alerts::{ handler::{Error, Handler, RmcResponse}, Alert, AlertMessage, ForkProof, ForkingNotification, }, - units::{ControlHash, FullUnit, PreUnit}, + units::{FullUnit, PreUnit}, PartiallyMultisigned, Recipient, Round, }; use aleph_bft_mock::{Data, Hasher64, Keychain, Signature}; diff --git a/consensus/src/units/control_hash.rs b/consensus/src/units/control_hash.rs new file mode 100644 index 00000000..cc721ead --- /dev/null +++ b/consensus/src/units/control_hash.rs @@ -0,0 +1,93 @@ +use crate::{units::UnitCoord, Hasher, NodeCount, NodeMap, Round}; +use codec::{Decode, Encode}; + +/// Combined hashes of the parents of a unit together with the set of indices of creators of the +/// parents +#[derive(Clone, Eq, PartialEq, Hash, Debug, Decode, Encode)] +pub struct ControlHash { + pub(crate) parents: NodeMap, + pub(crate) combined_hash: H::Hash, +} + +impl ControlHash { + pub(crate) fn new(parents_with_rounds_and_hashes: &NodeMap<(H::Hash, Round)>) -> Self { + let mut parents_with_rounds = NodeMap::with_size(parents_with_rounds_and_hashes.size()); + for (parent_index, (_, parent_round)) in parents_with_rounds_and_hashes.iter() { + parents_with_rounds.insert(parent_index, *parent_round); + } + ControlHash { + parents: parents_with_rounds, + combined_hash: Self::create_control_hash(parents_with_rounds_and_hashes), + } + } + + /// Calculate parent control hash, which includes all parent hashes and their rounds into account. + pub(crate) fn create_control_hash(parent_map: &NodeMap<(H::Hash, Round)>) -> H::Hash { + parent_map.using_encoded(H::hash) + } + + pub(crate) fn parents(&self) -> impl Iterator + '_ { + self.parents + .iter() + .map(|(node_index, &round)| UnitCoord::new(round, node_index)) + } + + pub(crate) fn n_parents(&self) -> NodeCount { + NodeCount(self.parents().count()) + } + + pub(crate) fn n_members(&self) -> NodeCount { + self.parents.size() + } +} + +#[cfg(test)] +pub mod tests { + use crate::units::UnitCoord; + use crate::{units::ControlHash, NodeCount, NodeIndex}; + use aleph_bft_mock::Hasher64; + use codec::{Decode, Encode}; + + #[test] + fn given_control_hash_is_encoded_when_same_control_hash_is_decoded_then_results_are_the_same() { + let ch = + ControlHash::::new(&vec![Some(([0; 8], 2)), None, Some(([1; 8], 2))].into()); + let encoded = ch.encode(); + let decoded = + ControlHash::decode(&mut encoded.as_slice()).expect("should decode correctly"); + assert_eq!(decoded, ch); + } + + #[test] + fn given_control_hash_then_basic_properties_are_correct() { + let parent_map = vec![ + Some(([0; 8], 2)), + None, + Some(([2; 8], 2)), + Some(([3; 8], 2)), + Some(([4; 8], 2)), + Some(([5; 8], 2)), + Some(([5; 8], 1)), + ] + .into(); + let ch = ControlHash::::new(&parent_map); + assert_eq!( + ControlHash::::create_control_hash(&parent_map), + [119, 216, 234, 178, 169, 143, 117, 127] + ); + + assert_eq!(ch.n_parents(), NodeCount(6)); + assert_eq!(ch.n_members(), NodeCount(7)); + + let parents: Vec<_> = ch.parents().collect(); + let expected_parents = vec![ + UnitCoord::new(2, NodeIndex(0)), + UnitCoord::new(2, NodeIndex(2)), + UnitCoord::new(2, NodeIndex(3)), + UnitCoord::new(2, NodeIndex(4)), + UnitCoord::new(2, NodeIndex(5)), + UnitCoord::new(1, NodeIndex(6)), + ]; + assert_eq!(parents, expected_parents); + } +} diff --git a/consensus/src/units/mod.rs b/consensus/src/units/mod.rs index 88cf5a01..1f53826a 100644 --- a/consensus/src/units/mod.rs +++ b/consensus/src/units/mod.rs @@ -1,17 +1,20 @@ use std::fmt::{Display, Formatter, Result as FmtResult}; use crate::{ - Data, Hasher, Index, MultiKeychain, NodeCount, NodeIndex, NodeMap, Round, SessionId, Signable, - Signed, UncheckedSigned, + Data, Hasher, Index, MultiKeychain, NodeCount, NodeIndex, Round, SessionId, Signable, Signed, + UncheckedSigned, }; use codec::{Decode, Encode}; use derivative::Derivative; use parking_lot::RwLock; +mod control_hash; mod store; #[cfg(test)] mod testing; mod validator; + +pub use control_hash::ControlHash; pub(crate) use store::*; #[cfg(test)] pub use testing::{ @@ -52,46 +55,6 @@ impl Display for UnitCoord { } } -/// Combined hashes of the parents of a unit together with the set of indices of creators of the -/// parents -#[derive(Clone, Eq, PartialEq, Hash, Debug, Decode, Encode)] -pub struct ControlHash { - pub(crate) parents: NodeMap, - pub(crate) combined_hash: H::Hash, -} - -impl ControlHash { - pub(crate) fn new(parents_with_rounds_and_hashes: &NodeMap<(H::Hash, Round)>) -> Self { - let mut parents_with_rounds = NodeMap::with_size(parents_with_rounds_and_hashes.size()); - for (parent_index, (_, parent_round)) in parents_with_rounds_and_hashes.iter() { - parents_with_rounds.insert(parent_index, *parent_round); - } - ControlHash { - parents: parents_with_rounds, - combined_hash: Self::create_control_hash(parents_with_rounds_and_hashes), - } - } - - /// Calculate parent control hash, which includes all parent hashes and their rounds into account. - pub(crate) fn create_control_hash(parent_map: &NodeMap<(H::Hash, Round)>) -> H::Hash { - parent_map.using_encoded(H::hash) - } - - pub(crate) fn parents(&self) -> impl Iterator + '_ { - self.parents - .iter() - .map(|(node_index, &round)| UnitCoord::new(round, node_index)) - } - - pub(crate) fn n_parents(&self) -> NodeCount { - NodeCount(self.parents().count()) - } - - pub(crate) fn n_members(&self) -> NodeCount { - self.parents.size() - } -} - /// The simplest type representing a unit, consisting of coordinates and a control hash #[derive(Clone, Eq, PartialEq, Hash, Debug, Decode, Encode)] pub struct PreUnit { @@ -282,7 +245,7 @@ pub type HashFor = <::Hasher as Hasher>::Hash; #[cfg(test)] pub mod tests { use crate::{ - units::{random_full_parent_units_up_to, ControlHash, FullUnit, Unit}, + units::{random_full_parent_units_up_to, FullUnit, Unit}, Hasher, NodeCount, }; use aleph_bft_mock::{Data, Hasher64}; @@ -301,16 +264,6 @@ pub mod tests { } } - #[test] - fn test_control_hash_codec() { - let ch = - ControlHash::::new(&vec![Some(([0; 8], 2)), None, Some(([1; 8], 2))].into()); - let encoded = ch.encode(); - let decoded = - ControlHash::decode(&mut encoded.as_slice()).expect("should decode correctly"); - assert_eq!(decoded, ch); - } - #[test] fn test_full_unit_codec() { for full_unit in random_full_parent_units_up_to(3, NodeCount(4), 43)