Don't add blocks which aren't valid

Previously, Tendermint needed to be live more than it needed to be correct.
Under the original intention for it, correctness would fail if any coin
desynced, which would cause the node to fail entirely. By accepting a
supermajority's view of state, despite its own, a single coin's failure would
only lead to inability to participate with that single coin.

Now that Tendermint is solely for Tributary, nodes should halt a coin-specific
chain if their view of the chain differs. They are unable to meaningless
participate regardless.

This also means a supermajority of validators can no longer fake messages from
other validators, allowing the Tributary chain to use uniform weights with much
less impact. There is still enough impact they can't be used (ability to cause
a fork), yet they should allow uniform block production (as that's solely a DoS
concern).

While we prior could've simply additionally checked signatures, add_block's
lack of a failure case would've meant it had to panic. This would've been a DoS
possible a minority-weight *which affected the entire coordinator* and
therefore *the entire validator for all coins*.

coordinator/tributary/src/blockchain.rs

@@ -54,16 +54,23 @@ impl<T: Transaction> Blockchain<T> {
     block.verify(self.genesis, self.tip, locally_provided, self.next_nonces.clone())
   }
 
-  /// Add a block, assuming it's valid.
+  /// Add a block.
-  ///
+  #[must_use]
-  /// Do not call this without either verifying the block or having it confirmed under consensus.
+  pub fn add_block(&mut self, block: &Block<T>) -> bool {
-  /// Doing so will cause a panic or action an invalid transaction.
+    // TODO: Handle desyncs re: provided transactions properly
-  pub fn add_block(&mut self, block: &Block<T>) {
+    if self.verify_block(block).is_err() {
+      return false;
+    }
+
+    // None of the following assertions should be reachable since we verified the block
     self.tip = block.hash();
     for tx in &block.transactions {
       match tx.kind() {
         TransactionKind::Provided => {
-          self.provided.withdraw(tx.hash());
+          assert!(
+            self.provided.withdraw(tx.hash()),
+            "verified block had a provided transaction we didn't have"
+          );
         }
         TransactionKind::Unsigned => {}
         TransactionKind::Signed(Signed { signer, nonce, .. }) => {
@@ -72,10 +79,12 @@ impl<T: Transaction> Blockchain<T> {
             .insert(*signer, nonce + 1)
             .expect("block had signed transaction from non-participant");
           if prev != *nonce {
-            panic!("block had an invalid nonce");
+            panic!("verified block had an invalid nonce");
           }
         }
       }
     }
+
+    true
   }
 }

coordinator/tributary/src/tests/blockchain.rs

@@ -35,7 +35,7 @@ fn block_addition() {
   assert_eq!(block.header.parent, genesis);
   assert_eq!(block.header.transactions, [0; 32]);
   blockchain.verify_block(&block).unwrap();
-  blockchain.add_block(&block);
+  assert!(blockchain.add_block(&block));
   assert_eq!(blockchain.tip(), block.hash());
 }
 
@@ -155,7 +155,7 @@ fn signed_transaction() {
 
     // Verify and add the block
     blockchain.verify_block(&block).unwrap();
-    blockchain.add_block(&block);
+    assert!(blockchain.add_block(&block));
     assert_eq!(blockchain.tip(), block.hash());
   };
 
@@ -194,11 +194,11 @@ fn provided_transaction() {
   blockchain.verify_block(&block).unwrap();
 
   // add_block should work for verified blocks
-  blockchain.add_block(&block);
+  assert!(blockchain.add_block(&block));
 
   let block = Block::new(blockchain.tip(), &txs, HashMap::new());
   // The provided transaction should no longer considered provided, causing this error
   assert!(blockchain.verify_block(&block).is_err());
-  // add_block should also work for unverified provided transactions if told to add them
+  // add_block should fail for unverified provided transactions if told to add them
-  blockchain.add_block(&block);
+  assert!(!blockchain.add_block(&block));
 }

coordinator/tributary/src/transaction.rs

@@ -58,8 +58,6 @@ impl ReadWrite for Signed {
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum TransactionKind<'a> {
   /// This tranaction should be provided by every validator, solely ordered by the block producer.
-  ///
-  /// This transaction is only valid if a supermajority of validators provided it.
   Provided,
 
   /// An unsigned transaction, only able to be included by the block producer.