code: Count & Keeper improvements

.gitignore

@@ -15,3 +15,7 @@ Cargo.lock
 
 # Code coverage artifacts
 lcov.info
+
+
+# IDEA related fluff
+.idea

Code/vote/src/count.rs

@@ -25,12 +25,12 @@ impl ValuesWeights {
         *entry
     }
 
-    /// Return the value with the highest weight and said weight, if any.
-    pub fn highest_weighted_value(&self) -> Option<(&ValueId, Weight)> {
+    /// Return the cumulative weight associated with all the values.
+    /// Returns None if there are no weights associated with any value.
+    pub fn cumulative_weights(&self) -> Option<Weight> {
         self.value_weights
             .iter()
-            .max_by_key(|(_, weight)| *weight)
-            .map(|(value, weight)| (value.as_ref(), *weight))
+            .fold(None, |mac, (_, w)| mac.map(|acc| acc + w))
     }
 }
 
@@ -63,6 +63,8 @@ impl VoteCount {
 
     /// Add vote to internal counters and return the highest threshold.
     pub fn add_vote(&mut self, vote: Vote, weight: Weight) -> Threshold {
+        // Note: The ordering among these if clauses is important
+        // We first check if there is a quorum for a specific value
         if let Some(value) = vote.value {
             let value = Arc::new(value);
             let new_weight = self.values_weights.add_weight(value.clone(), weight);
@@ -80,44 +82,54 @@ impl VoteCount {
             }
         }
 
-        // Check if we have a quorum for any value, using the highest weighted value, if any.
-        if let Some((_max_value, max_weight)) = self.values_weights.highest_weighted_value() {
-            if is_quorum(max_weight + self.nil, self.total) {
-                return Threshold::Any;
-            }
+        // Check if we have a quorum for all values and for nil votes,
+        if self.is_quorum_any() {
+            return Threshold::Any;
         }
 
-        // No quorum
-        Threshold::Init
+        // No quorum was reached
+        Threshold::Unreached
     }
+
     pub fn check_threshold(&self, threshold: Threshold) -> bool {
         match threshold {
-            Threshold::Init => false,
-            Threshold::Any => self.values_weights.highest_weighted_value().is_some(),
+            Threshold::Unreached => false,
+            Threshold::Any => self.is_quorum_any(),
             Threshold::Nil => self.nil > 0,
             Threshold::Value(value) => self.values_weights.value_weights.contains_key(&value),
         }
     }
+
+    // Checks if the cumulative weight associated to all votes and nil is enough for a quorum
+    fn is_quorum_any(&self) -> bool {
+        if let Some(cm_weight) = self.values_weights.cumulative_weights() {
+            if is_quorum(cm_weight + self.nil, self.total) {
+                return true;
+            }
+        }
+        false
+    }
 }
 
 //-------------------------------------------------------------------------
 // Round votes
 //-------------------------------------------------------------------------
 
-// Thresh represents the different quorum thresholds.
+// Threshold represents the different quorum thresholds.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum Threshold {
-    /// No quorum
-    Init, // no quorum
-    /// Qorum of votes but not for the same value
-    Any,
-    /// Quorum for nil
-    Nil,
-    /// Quorum for a value
+    /// No quorum was reached
+    Unreached,
+    /// Quorum reached for a specific value
     Value(Arc<ValueId>),
+    /// Quorum reached for a nil value
+    /// This happens eg if the proposer missed their time to send proposal
+    Nil,
+    /// Qorum of votes reached but for multiple different values, including nil
+    Any,
 }
 
-/// Returns whether or note `value > (2/3)*total`.
+/// Returns whether or not `value > (2/3)*total`.
 pub fn is_quorum(value: Weight, total: Weight) -> bool {
     3 * value > 2 * total
 }
@@ -139,11 +151,11 @@ mod tests {
         // add a vote for nil. nothing changes.
         let vote = Vote::new_prevote(Round::new(0), None, Address::new(1));
         let thresh = round_votes.add_vote(vote.clone(), 1);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add it again, nothing changes.
         let thresh = round_votes.add_vote(vote.clone(), 1);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add it again, get Nil
         let thresh = round_votes.add_vote(vote.clone(), 1);
@@ -162,11 +174,11 @@ mod tests {
         // add a vote. nothing changes.
         let vote = Vote::new_prevote(Round::new(0), val, Address::new(1));
         let thresh = round_votes.add_vote(vote.clone(), weight);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add it again, nothing changes.
         let thresh = round_votes.add_vote(vote.clone(), weight);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add a vote for nil, get Thresh::Any
         let vote_nil = Vote::new_prevote(Round::new(0), None, Address::new(2));
@@ -191,25 +203,25 @@ mod tests {
         // add a vote for v1. nothing changes.
         let vote1 = Vote::new_precommit(Round::new(0), val1, Address::new(1));
         let thresh = round_votes.add_vote(vote1.clone(), 1);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add a vote for v2. nothing changes.
         let vote2 = Vote::new_precommit(Round::new(0), val2, Address::new(2));
         let thresh = round_votes.add_vote(vote2.clone(), 1);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add a vote for nil. nothing changes.
         let vote_nil = Vote::new_precommit(Round::new(0), None, Address::new(3));
         let thresh = round_votes.add_vote(vote_nil.clone(), 1);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add a vote for v1. nothing changes
         let thresh = round_votes.add_vote(vote1.clone(), 1);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add a vote for v2. nothing changes
         let thresh = round_votes.add_vote(vote2.clone(), 1);
-        assert_eq!(thresh, Threshold::Init);
+        assert_eq!(thresh, Threshold::Unreached);
 
         // add a big vote for v2. get Value(v2)
         let thresh = round_votes.add_vote(vote2.clone(), 10);

Code/vote/src/keeper.rs

@@ -62,15 +62,15 @@ impl VoteKeeper {
     /// Map a vote type and a threshold to a state machine event.
     fn to_event(typ: VoteType, threshold: Threshold) -> Option<Event> {
         match (typ, threshold) {
-            (_, Threshold::Init) => None,
+            (_, Threshold::Unreached) => None,
 
-            (VoteType::Prevote, Threshold::Any) => Some(Event::PolkaAny),
-            (VoteType::Prevote, Threshold::Nil) => Some(Event::PolkaNil),
             (VoteType::Prevote, Threshold::Value(v)) => Some(Event::PolkaValue(*v.as_ref())),
+            (VoteType::Prevote, Threshold::Nil) => Some(Event::PolkaNil),
+            (VoteType::Prevote, Threshold::Any) => Some(Event::PolkaAny),
 
-            (VoteType::Precommit, Threshold::Any) => Some(Event::PrecommitAny),
-            (VoteType::Precommit, Threshold::Nil) => None,
             (VoteType::Precommit, Threshold::Value(v)) => Some(Event::PrecommitValue(*v.as_ref())),
+            (VoteType::Precommit, Threshold::Nil) => None,
+            (VoteType::Precommit, Threshold::Any) => Some(Event::PrecommitAny),
         }
     }
 }