consensus: check for duplicate transactions in blocks

Change the Merkle root validation logic to also check that a block does not
contain duplicate transactions.  This check is redundant with later
double-spend checks, but is a useful defense-in-depth.

zebra-consensus/src/block/check.rs

@@ -173,12 +173,24 @@ pub fn merkle_root_validity(
     transaction_hashes: &[transaction::Hash],
 ) -> Result<(), BlockError> {
     let merkle_root = transaction_hashes.iter().cloned().collect();
-    if block.header.merkle_root == merkle_root {
+
-        Ok(())
+    if block.header.merkle_root != merkle_root {
-    } else {
+        return Err(BlockError::BadMerkleRoot {
-        Err(BlockError::BadMerkleRoot {
             actual: merkle_root,
             expected: block.header.merkle_root,
-        })
+        });
     }
+
+    // Bitcoin's transaction Merkle trees are malleable, allowing blocks with
+    // duplicate transactions to have the same Merkle root as blocks without
+    // duplicate transactions. Duplicate transactions should cause a block to be
+    // rejected, as duplicate transactions imply that the block contains a
+    // double-spend.  As a defense-in-depth, however, we also check that there
+    // are no duplicate transaction hashes, by collecting into a HashSet.
+    use std::collections::HashSet;
+    if transaction_hashes.len() != transaction_hashes.iter().collect::<HashSet<_>>().len() {
+        return Err(BlockError::DuplicateTransaction);
+    }
+
+    Ok(())
 }

zebra-consensus/src/error.rs

@@ -105,6 +105,9 @@ pub enum BlockError {
         expected: block::merkle::Root,
     },
 
+    #[error("block contains duplicate transactions")]
+    DuplicateTransaction,
+
     #[error("block {0:?} is already in the chain at depth {1:?}")]
     AlreadyInChain(zebra_chain::block::Hash, u32),