Finish `CommitteeId` wrapper (#249)

* CommitteeId type wrapper

* rewrite committee id logic

* remove unused convert

* use correct way to get committe id by member id

* cleanup and add comment

* Simplify child committees method

---------

Co-authored-by: danielsanchezq <sanchez.quiros.daniel@gmail.com>

consensus-engine/Cargo.toml

@@ -3,8 +3,6 @@ name = "consensus-engine"
 version = "0.1.0"
 edition = "2021"
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-
 [dependencies]
 serde = { version = "1.0", features = ["derive"], optional = true }
 blake2 = "0.10"
@@ -18,6 +16,7 @@ fraction = { version = "0.13" }
 
 [features]
 default = []
+serde = ["dep:serde"]
 
 [dev-dependencies]
 proptest = "1.2.0"

consensus-engine/src/overlay/tree_overlay/overlay.rs

@@ -89,14 +89,26 @@ where
     }
 
     fn child_committees(&self, id: NodeId) -> Vec<Committee> {
-        match self.carnot_tree.child_committees(&id) {
+        // Lookup committee index by member id, then committee id by index.
-            (None, None) => vec![],
+        self.carnot_tree
-            (None, Some(c)) | (Some(c), None) => vec![std::iter::once(*c).collect()],
+            .committees_by_member
-            (Some(c1), Some(c2)) => vec![
+            .get(&id)
-                std::iter::once(*c1).collect(),
+            .and_then(|committee_idx| self.carnot_tree.inner_committees.get(*committee_idx))
-                std::iter::once(*c2).collect(),
+            .map(|committee_id| {
-            ],
+                let (l, r) = self.carnot_tree.child_committees(committee_id);
-        }
+                let extract_committee = |committee_id| {
+                    self.carnot_tree
+                        .committee_id_to_index
+                        .get(committee_id)
+                        .and_then(|committee_idx| {
+                            self.carnot_tree.membership_committees.get(committee_idx)
+                        })
+                };
+                let l = l.and_then(extract_committee).into_iter().cloned();
+                let r = r.and_then(extract_committee).into_iter().cloned();
+                l.chain(r).collect()
+            })
+            .expect("NodeId not found in overlay")
     }
 
     fn leaf_committees(&self, _id: NodeId) -> Vec<Committee> {
@@ -191,7 +203,6 @@ mod tests {
     use crate::Overlay;
 
     use super::*;
-    use std::collections::HashSet;
 
     #[test]
     fn test_carnot_overlay_leader() {
@@ -235,7 +246,7 @@ mod tests {
             leader: RoundRobin::new(),
         });
 
-        let mut expected_root = HashSet::new();
+        let mut expected_root = Committee::new();
         expected_root.insert(overlay.nodes[9]);
         expected_root.extend(overlay.nodes[0..3].iter());
 

consensus-engine/src/overlay/tree_overlay/tree.rs

@@ -1,22 +1,22 @@
-use crate::{Committee, NodeId};
+use crate::{Committee, CommitteeId, NodeId};
 use blake2::{digest::typenum::U32, Blake2b, Digest};
 use std::collections::{HashMap, HashSet};
 
-fn blake2b_hash(committee: &Committee) -> [u8; 32] {
+fn blake2b_hash(committee: &Committee) -> CommitteeId {
     let mut hasher = Blake2b::<U32>::new();
     let mut tmp = committee.iter().collect::<Vec<_>>();
     tmp.sort();
     for member in tmp {
         hasher.update(member);
     }
-    hasher.finalize().into()
+    CommitteeId::new(hasher.finalize().into())
 }
 
 #[derive(Debug, Clone)]
 pub(super) struct Tree {
-    pub(super) inner_committees: Vec<NodeId>,
+    pub(super) inner_committees: Vec<CommitteeId>,
     pub(super) membership_committees: HashMap<usize, Committee>,
-    pub(super) committee_id_to_index: HashMap<NodeId, usize>,
+    pub(super) committee_id_to_index: HashMap<CommitteeId, usize>,
     pub(super) committees_by_member: HashMap<NodeId, usize>,
 }
 
@@ -50,15 +50,16 @@ impl Tree {
     pub(super) fn build_committee_from_nodes_with_size(
         nodes: &[NodeId],
         number_of_committees: usize,
-    ) -> (Vec<[u8; 32]>, HashMap<usize, Committee>) {
+    ) -> (Vec<CommitteeId>, HashMap<usize, Committee>) {
         let committee_size = nodes.len() / number_of_committees;
         let remainder = nodes.len() % number_of_committees;
 
-        let mut committees: Vec<HashSet<NodeId>> = (0..number_of_committees)
+        let mut committees: Vec<Committee> = (0..number_of_committees)
             .map(|n| {
                 nodes[n * committee_size..(n + 1) * committee_size]
                     .iter()
                     .cloned()
+                    .map(From::from)
                     .collect()
             })
             .collect();
@@ -76,7 +77,7 @@ impl Tree {
         (hashes, committees.into_iter().enumerate().collect())
     }
 
-    pub(super) fn parent_committee(&self, committee_id: &NodeId) -> Option<&[u8; 32]> {
+    pub(super) fn parent_committee(&self, committee_id: &CommitteeId) -> Option<&CommitteeId> {
         if committee_id == &self.inner_committees[0] {
             None
         } else {
@@ -96,8 +97,8 @@ impl Tree {
 
     pub(super) fn child_committees(
         &self,
-        committee_id: &NodeId,
+        committee_id: &CommitteeId,
-    ) -> (Option<&[u8; 32]>, Option<&[u8; 32]>) {
+    ) -> (Option<&CommitteeId>, Option<&CommitteeId>) {
         let Some(base) = self
             .committee_id_to_index
             .get(committee_id)
@@ -110,7 +111,7 @@ impl Tree {
         )
     }
 
-    pub(super) fn leaf_committees(&self) -> HashMap<&[u8; 32], &Committee> {
+    pub(super) fn leaf_committees(&self) -> HashMap<&CommitteeId, &Committee> {
         let total_leafs = (self.inner_committees.len() + 1) / 2;
         let mut leaf_committees = HashMap::new();
         for i in (self.inner_committees.len() - total_leafs)..self.inner_committees.len() {
@@ -131,7 +132,7 @@ impl Tree {
         self.committees_by_member.get(member_id).copied()
     }
 
-    pub(super) fn committee_id_by_member_id(&self, member_id: &NodeId) -> Option<&[u8; 32]> {
+    pub(super) fn committee_id_by_member_id(&self, member_id: &NodeId) -> Option<&CommitteeId> {
         self.committees_by_member
             .get(member_id)
             .map(|&idx| &self.inner_committees[idx])
@@ -142,7 +143,10 @@ impl Tree {
             .and_then(|idx| self.committee_by_committee_idx(idx))
     }
 
-    pub(super) fn committee_by_committee_id(&self, committee_id: &NodeId) -> Option<&Committee> {
+    pub(super) fn committee_by_committee_id(
+        &self,
+        committee_id: &CommitteeId,
+    ) -> Option<&Committee> {
         self.committee_id_to_index
             .get(committee_id)
             .and_then(|&idx| self.committee_by_committee_idx(idx))

consensus-engine/src/types.rs

@@ -5,6 +5,9 @@ use std::hash::Hash;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+mod committee;
+pub use committee::CommitteeId;
+
 pub type View = i64;
 pub type NodeId = [u8; 32];
 pub type BlockId = [u8; 32];
@@ -125,7 +128,7 @@ impl Block {
 /// Possible output events.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Send {
-    pub to: HashSet<NodeId>,
+    pub to: Committee,
     pub payload: Payload,
 }
 

consensus-engine/src/types/committee.rs

@@ -0,0 +1,37 @@
+#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct CommitteeId(pub(crate) [u8; 32]);
+
+impl CommitteeId {
+    pub const fn new(val: [u8; 32]) -> Self {
+        Self(val)
+    }
+}
+
+impl From<[u8; 32]> for CommitteeId {
+    fn from(id: [u8; 32]) -> Self {
+        Self(id)
+    }
+}
+
+impl From<&[u8; 32]> for CommitteeId {
+    fn from(id: &[u8; 32]) -> Self {
+        Self(*id)
+    }
+}
+
+impl From<CommitteeId> for [u8; 32] {
+    fn from(id: CommitteeId) -> Self {
+        id.0
+    }
+}
+
+impl core::fmt::Display for CommitteeId {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        write!(f, "0x")?;
+        for v in self.0 {
+            write!(f, "{:02x}", v)?;
+        }
+        Ok(())
+    }
+}

nomos-services/consensus/src/tally/mod.rs

@@ -3,17 +3,16 @@ pub mod timeout;
 pub mod unhappy;
 
 // std
-use std::collections::HashSet;
 
 // crates
 use serde::{Deserialize, Serialize};
 
 // internal
-use consensus_engine::NodeId;
+use consensus_engine::Committee;
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct CarnotTallySettings {
     pub threshold: usize,
     // TODO: this probably should be dynamic and should change with the view (?)
-    pub participating_nodes: HashSet<NodeId>,
+    pub participating_nodes: Committee,
 }