From 4906a191f945e23d9f38ffffc9ba611af51d1f36 Mon Sep 17 00:00:00 2001
From: Henry de Valence <hdevalence@hdevalence.ca>
Date: Wed, 25 Nov 2020 12:17:16 -0800
Subject: [PATCH] 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 | 22 +++++++++++++++++-----
 zebra-consensus/src/error.rs       |  3 +++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs
index e59c24b3..46701b18 100644
--- a/zebra-consensus/src/block/check.rs
+++ b/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(())
-    } else {
-        Err(BlockError::BadMerkleRoot {
+
+    if block.header.merkle_root != merkle_root {
+        return 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(())
 }
diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs
index d0fb1603..9adf31fe 100644
--- a/zebra-consensus/src/error.rs
+++ b/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),