Fix to use Err(DoubleVote) from VoteCollector

VoteCollector is changed to return Result<bool, DoubleVote>. Rust
compiler will force you to check whether there was a double vote.

Author: foriequal0

core/src/consensus/tendermint/vote_collector.rs

@@ -61,26 +61,29 @@ impl Encodable for DoubleVote {
 }
 
 impl StepCollector {
-    /// Returns Some(&Address) when validator is double voting.
-    fn insert(&mut self, message: ConsensusMessage) -> Option<DoubleVote> {
+    /// Some(true): a message is new
+    /// Some(false): a message is duplicated
+    /// Err(DoubleVote): a double vote
+    fn insert(&mut self, message: ConsensusMessage) -> Result<bool, DoubleVote> {
         // Do nothing when message was seen.
-        if !self.messages.contains(&message) {
-            self.messages.push(message.clone());
-            if let Some(previous) = self.voted.insert(message.signer_index(), message.clone()) {
-                // Bad validator sent a different message.
-                return Some(DoubleVote {
-                    author_index: message.signer_index(),
-                    vote_one: previous,
-                    vote_two: message,
-                })
-            } else {
-                self.block_votes
-                    .entry(message.block_hash())
-                    .or_default()
-                    .insert(message.signer_index(), message.signature());
-            }
+        if self.messages.contains(&message) {
+            return Ok(false)
+        }
+        self.messages.push(message.clone());
+        if let Some(previous) = self.voted.insert(message.signer_index(), message.clone()) {
+            // Bad validator sent a different message.
+            Err(DoubleVote {
+                author_index: message.signer_index(),
+                vote_one: previous,
+                vote_two: message,
+            })
+        } else {
+            self.block_votes
+                .entry(message.block_hash())
+                .or_default()
+                .insert(message.signer_index(), message.signature());
+            Ok(true)
         }
-        None
     }
 
     /// Count all votes for the given block hash at this round.
@@ -120,7 +123,7 @@ impl Default for VoteCollector {
 
 impl VoteCollector {
     /// Insert vote if it is newer than the oldest one.
-    pub fn collect(&mut self, message: ConsensusMessage) -> Option<DoubleVote> {
+    pub fn collect(&mut self, message: ConsensusMessage) -> Result<bool, DoubleVote> {
         self.votes.entry(*message.round()).or_insert_with(Default::default).insert(message)
     }
 

core/src/consensus/tendermint/worker.rs

@@ -921,7 +921,9 @@ impl Worker {
                 on: on.clone(),
             };
             if !self.votes.is_old_or_known(&message) {
-                self.votes.collect(message);
+                if let Err(double_vote) = self.votes.collect(message) {
+                    cerror!(ENGINE, "Double vote found on_commit_message: {:?}", double_vote);
+                }
             }
         }
 
@@ -1387,7 +1389,7 @@ impl Worker {
                 self.votes_received.set(vote_index);
             }
 
-            if let Some(double) = self.votes.collect(message.clone()) {
+            if let Err(double) = self.votes.collect(message.clone()) {
                 cerror!(ENGINE, "Double vote found {:?}", double);
                 self.report_double_vote(&double);
                 return Err(EngineError::DoubleVote(sender))
@@ -1505,7 +1507,7 @@ impl Worker {
         };
 
         self.votes_received.set(vote.signer_index);
-        self.votes.collect(vote.clone());
+        self.votes.collect(vote.clone()).expect("Must not attempt double vote");
         cinfo!(ENGINE, "Voted {:?} as {}th validator.", vote, signer_index);
         Ok(Some(vote))
     }
@@ -1531,7 +1533,7 @@ impl Worker {
             on,
         };
 
-        self.votes.collect(vote.clone());
+        self.votes.collect(vote.clone()).expect("Must not attempt double vote on proposal");;
         cinfo!(ENGINE, "Voted {:?} as {}th proposer.", vote, signer_index);
         Ok(vote)
     }
@@ -1783,7 +1785,7 @@ impl Worker {
                 );
             }
 
-            if let Some(double) = self.votes.collect(message.clone()) {
+            if let Err(double) = self.votes.collect(message.clone()) {
                 cerror!(ENGINE, "Double Vote found {:?}", double);
                 self.report_double_vote(&double);
                 return None
@@ -2117,7 +2119,9 @@ impl Worker {
         cdebug!(ENGINE, "Commit message-{} is verified", commit_height);
         for vote in votes {
             if !self.votes.is_old_or_known(&vote) {
-                self.votes.collect(vote);
+                if let Err(double_vote) = self.votes.collect(vote) {
+                    cerror!(ENGINE, "Double vote found on_commit_message: {:?}", double_vote);
+                }
             }
         }