Fix tendermint distinct precommit bug (#517)

* fix tendermint distinct precommit bug

* remove conflicting precommit error

coordinator/src/tributary/scanner.rs

@@ -216,8 +216,7 @@ impl<
           // Since the evidence is on the chain, it should already have been validated
           // We can just punish the signer
           let data = match ev {
-            Evidence::ConflictingMessages(first, second) |
+            Evidence::ConflictingMessages(first, second) => (first, Some(second)),
-            Evidence::ConflictingPrecommit(first, second) => (first, Some(second)),
             Evidence::InvalidPrecommit(first) | Evidence::InvalidValidRound(first) => (first, None),
           };
           let msgs = (

coordinator/tributary/src/tests/transaction/tendermint.rs

@@ -156,14 +156,6 @@ async fn evidence_with_prevote() {
           .await
           .encode(),
       )));
-      txs.push(TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit(
-        signed_from_data::<N>(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id))
-          .await
-          .encode(),
-        signed_from_data::<N>(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id))
-          .await
-          .encode(),
-      )));
       txs
     }
   };
@@ -263,34 +255,6 @@ async fn conflicting_msgs_evidence_tx() {
     verify_tendermint_tx::<N>(&tx, &validators, commit).unwrap_err();
   }
 
-  // Precommit
-  {
-    let sig = signer.sign(&[]).await; // the inner signature doesn't matter
-
-    let signed_1 = signed_for_b_r(0, 0, Data::Precommit(Some(([0x11; 32], sig)))).await;
-    let tx = TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit(
-      signed_1.encode(),
-      signed_1.encode(),
-    ));
-    assert!(verify_tendermint_tx::<N>(&tx, &validators, commit).is_err());
-
-    // For precommit, the round number is ignored
-    let signed_2 = signed_for_b_r(0, 1, Data::Precommit(Some(([0x22; 32], sig)))).await;
-    let tx = TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit(
-      signed_1.encode(),
-      signed_2.encode(),
-    ));
-    verify_tendermint_tx::<N>(&tx, &validators, commit).unwrap();
-
-    // Yet the block number isn't
-    let signed_2 = signed_for_b_r(1, 0, Data::Precommit(Some(([0x22; 32], sig)))).await;
-    let tx = TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit(
-      signed_1.encode(),
-      signed_2.encode(),
-    ));
-    assert!(verify_tendermint_tx::<N>(&tx, &validators, commit).is_err());
-  }
-
   // msgs from different senders should fail
   {
     let signed_1 = signed_for_b_r(0, 0, Data::Proposal(None, TendermintBlock(vec![0x11]))).await;

coordinator/tributary/tendermint/src/lib.rs

@@ -136,7 +136,6 @@ pub enum SlashReason {
 #[derive(Clone, PartialEq, Eq, Debug, Encode, Decode)]
 pub enum Evidence {
   ConflictingMessages(Vec<u8>, Vec<u8>),
-  ConflictingPrecommit(Vec<u8>, Vec<u8>),
   InvalidPrecommit(Vec<u8>),
   InvalidValidRound(Vec<u8>),
 }
@@ -179,30 +178,6 @@ pub fn verify_tendermint_evience<N: Network>(
         Err(TendermintError::InvalidEvidence)?;
       }
     }
-    Evidence::ConflictingPrecommit(first, second) => {
-      let first = decode_and_verify_signed_message::<N>(first, schema)?.msg;
-      let second = decode_and_verify_signed_message::<N>(second, schema)?.msg;
-
-      if (first.sender != second.sender) || (first.block != second.block) {
-        Err(TendermintError::InvalidEvidence)?;
-      }
-
-      // check whether messages are precommits to different blocks
-      // The inner signatures don't need to be verified since the outer signatures were
-      // While the inner signatures may be invalid, that would've yielded a invalid precommit
-      // signature slash instead of distinct precommit slash
-      if let Data::Precommit(Some((h1, _))) = first.data {
-        if let Data::Precommit(Some((h2, _))) = second.data {
-          if h1 == h2 {
-            Err(TendermintError::InvalidEvidence)?;
-          }
-          return Ok(());
-        }
-      }
-
-      // No fault identified
-      Err(TendermintError::InvalidEvidence)?;
-    }
     Evidence::InvalidPrecommit(msg) => {
       let msg = decode_and_verify_signed_message::<N>(msg, schema)?.msg;
 

coordinator/tributary/tendermint/src/message_log.rs

@@ -3,18 +3,17 @@ use std::{sync::Arc, collections::HashMap};
 use log::debug;
 use parity_scale_codec::Encode;
 
-use crate::{ext::*, RoundNumber, Step, Data, DataFor, TendermintError, SignedMessageFor, Evidence};
+use crate::{ext::*, RoundNumber, Step, DataFor, TendermintError, SignedMessageFor, Evidence};
 
 type RoundLog<N> = HashMap<<N as Network>::ValidatorId, HashMap<Step, SignedMessageFor<N>>>;
 pub(crate) struct MessageLog<N: Network> {
   weights: Arc<N::Weights>,
-  precommitted: HashMap<N::ValidatorId, SignedMessageFor<N>>,
   pub(crate) log: HashMap<RoundNumber, RoundLog<N>>,
 }
 
 impl<N: Network> MessageLog<N> {
   pub(crate) fn new(weights: Arc<N::Weights>) -> MessageLog<N> {
-    MessageLog { weights, precommitted: HashMap::new(), log: HashMap::new() }
+    MessageLog { weights, log: HashMap::new() }
   }
 
   // Returns true if it's a new message
@@ -40,24 +39,6 @@ impl<N: Network> MessageLog<N> {
       return Ok(false);
     }
 
-    // If they already precommitted to a distinct hash, error
-    if let Data::Precommit(Some((hash, _))) = msg.data {
-      if let Some(prev) = self.precommitted.get(&msg.sender) {
-        if let Data::Precommit(Some((prev_hash, _))) = prev.msg.data {
-          if hash != prev_hash {
-            debug!(target: "tendermint", "Validator precommitted to multiple blocks");
-            Err(TendermintError::Malicious(
-              msg.sender,
-              Some(Evidence::ConflictingPrecommit(prev.encode(), signed.encode())),
-            ))?;
-          }
-        } else {
-          panic!("message in precommitted wasn't Precommit");
-        }
-      }
-      self.precommitted.insert(msg.sender, signed.clone());
-    }
-
     msgs.insert(step, signed);
     Ok(true)
   }