Block double vote attempts

core/src/consensus/tendermint/message.rs

@@ -18,12 +18,10 @@ use std::cmp;
 
 use ccrypto::blake256;
 use ckey::{verify_schnorr, Error as KeyError, Public, SchnorrSignature};
-use ctypes::Header;
 use primitives::{Bytes, H256};
 use rlp::{Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp};
 use snap;
 
-use super::super::validator_set::DynamicValidator;
 use super::super::BitSet;
 use super::{BlockHash, Height, Step, View};
 
@@ -331,28 +329,6 @@ pub struct ConsensusMessage {
 }
 
 impl ConsensusMessage {
-    /// If a locked node re-proposes locked proposal, the proposed_view is different from the header's view.
-    pub fn new_proposal(
-        signature: SchnorrSignature,
-        validators: &DynamicValidator,
-        proposal_header: &Header,
-        proposed_view: View,
-        prev_proposer_idx: usize,
-    ) -> Result<Self, ::rlp::DecoderError> {
-        let height = proposal_header.number() as Height;
-        let signer_index =
-            validators.proposer_index(*proposal_header.parent_hash(), prev_proposer_idx, proposed_view as usize);
-
-        Ok(ConsensusMessage {
-            signature,
-            signer_index,
-            on: VoteOn {
-                step: VoteStep::new(height, proposed_view, Step::Propose),
-                block_hash: Some(proposal_header.hash()),
-            },
-        })
-    }
-
     pub fn signature(&self) -> SchnorrSignature {
         self.signature
     }

core/src/consensus/tendermint/mod.rs

@@ -22,6 +22,7 @@ mod network;
 mod params;
 pub mod types;
 pub mod vote_collector;
+mod vote_regression_checker;
 mod worker;
 
 use std::sync::atomic::AtomicBool;

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 vote(&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/vote_regression_checker.rs

@@ -0,0 +1,189 @@
+use consensus::{Step, VoteOn};
+use std::cmp::Ordering;
+
+pub struct VoteRegressionChecker {
+    last_vote: Option<VoteOn>,
+}
+
+impl VoteRegressionChecker {
+    pub fn new() -> VoteRegressionChecker {
+        VoteRegressionChecker {
+            last_vote: None,
+        }
+    }
+
+    pub fn check(&mut self, vote_on: &VoteOn) -> bool {
+        assert!(match vote_on.step.step {
+            Step::Propose | Step::Prevote | Step::Precommit => true,
+            _ => false,
+        });
+
+        let monotonic = if let Some(last_vote) = &self.last_vote {
+            match last_vote.step.cmp(&vote_on.step) {
+                Ordering::Less => true,
+                Ordering::Greater => false,
+                Ordering::Equal => last_vote.block_hash == vote_on.block_hash,
+            }
+        } else {
+            true
+        };
+
+        if monotonic {
+            self.last_vote = Some(vote_on.clone());
+        }
+        monotonic
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use consensus::VoteStep;
+    use primitives::H256;
+
+    #[test]
+    fn test_initial_set() {
+        let mut checker = VoteRegressionChecker::new();
+
+        let random_step = VoteStep::new(100, 10, Step::Prevote);
+        let random_hash = Some(H256::random());
+        assert!(checker.check(&VoteOn {
+            step: random_step,
+            block_hash: random_hash
+        }))
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_disallow_commit() {
+        let mut checker = VoteRegressionChecker::new();
+
+        let random_commit_step = VoteStep::new(100, 10, Step::Commit);
+        let random_hash = Some(H256::random());
+        assert!(checker.check(&VoteOn {
+            step: random_commit_step,
+            block_hash: random_hash
+        }))
+    }
+
+    #[test]
+    fn test_allow_height_increase() {
+        let mut checker = VoteRegressionChecker::new();
+
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(1)),
+        });
+
+        assert!(checker.check(&VoteOn {
+            step: VoteStep::new(101, 10, Step::Prevote),
+            block_hash: Some(H256::from(2))
+        }))
+    }
+
+    #[test]
+    fn test_disallow_height_decrease() {
+        let mut checker = VoteRegressionChecker::new();
+
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(1)),
+        });
+
+        assert!(!checker.check(&VoteOn {
+            step: VoteStep::new(99, 10, Step::Prevote),
+            block_hash: Some(H256::from(2))
+        }))
+    }
+
+    #[test]
+    fn test_allow_view_increase() {
+        let mut checker = VoteRegressionChecker::new();
+
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(1)),
+        });
+
+        assert!(checker.check(&VoteOn {
+            step: VoteStep::new(100, 11, Step::Prevote),
+            block_hash: Some(H256::from(2))
+        }))
+    }
+
+    #[test]
+    fn test_disallow_view_decrease() {
+        let mut checker = VoteRegressionChecker::new();
+
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(1)),
+        });
+
+        assert!(!checker.check(&VoteOn {
+            step: VoteStep::new(100, 9, Step::Prevote),
+            block_hash: Some(H256::from(2))
+        }))
+    }
+
+    #[test]
+    fn test_allow_step_increased() {
+        let mut checker = VoteRegressionChecker::new();
+
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(1)),
+        });
+
+        assert!(checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Precommit),
+            block_hash: Some(H256::from(2))
+        }))
+    }
+
+    #[test]
+    fn test_disallow_step_decreased() {
+        let mut checker = VoteRegressionChecker::new();
+
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(1)),
+        });
+
+        assert!(!checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Propose),
+            block_hash: Some(H256::from(2))
+        }))
+    }
+
+    #[test]
+    fn test_allow_same_hash() {
+        let mut checker = VoteRegressionChecker::new();
+
+        let block_hash = Some(H256::random());
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash,
+        });
+
+        assert!(checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash,
+        }))
+    }
+
+    #[test]
+    fn test_disallow_hash_change() {
+        let mut checker = VoteRegressionChecker::new();
+
+        checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(1)),
+        });
+
+        assert!(!checker.check(&VoteOn {
+            step: VoteStep::new(100, 10, Step::Prevote),
+            block_hash: Some(H256::from(2))
+        }))
+    }
+}