consensus: tidy merkle validation in checkpointer

This moves it in with the existing `check_block` method and expands that
method's contract to cover general block validation checks.

zebra-consensus/src/checkpoint.rs

@@ -423,15 +423,40 @@ where
         }
     }
 
-    /// If the block height of `block` is valid, returns that height.
+    /// Check that the block height and Merkle root are valid.
     ///
-    /// Returns an error if the block's height is invalid, see `check_height()`
-    /// for details.
+    /// Checking the Merkle root ensures that the block hash binds the block
+    /// contents. To prevent malleability (CVE-2012-2459), we also need to check
+    /// whether the transaction hashes are unique.
     fn check_block(&self, block: &Block) -> Result<block::Height, VerifyCheckpointError> {
         let block_height = block
             .coinbase_height()
             .ok_or(VerifyCheckpointError::CoinbaseHeight { hash: block.hash() })?;
         self.check_height(block_height)?;
+
+        let transaction_hashes = block
+            .transactions
+            .iter()
+            .map(|tx| tx.hash())
+            .collect::<Vec<_>>();
+        let merkle_root = transaction_hashes.iter().cloned().collect();
+
+        // Check that the Merkle root is valid.
+        if block.header.merkle_root != merkle_root {
+            return Err(VerifyCheckpointError::BadMerkleRoot {
+                expected: block.header.merkle_root,
+                actual: merkle_root,
+            });
+        }
+
+        // To prevent malleability (CVE-2012-2459), we also need to check
+        // whether the transaction hashes are unique. Collecting into a HashSet
+        // deduplicates, so this checks that there are no duplicate transaction
+        // hashes, preventing Merkle root malleability.
+        if transaction_hashes.len() != transaction_hashes.iter().collect::<HashSet<_>>().len() {
+            return Err(VerifyCheckpointError::DuplicateTransaction);
+        }
+
         Ok(block_height)
     }
 
@@ -450,7 +475,7 @@ where
         // Set up a oneshot channel to send results
         let (tx, rx) = oneshot::channel();
 
-        // Check for a valid height
+        // Check that the height and Merkle roots are valid.
         let height = match self.check_block(&block) {
             Ok(height) => height,
             Err(error) => {
@@ -459,33 +484,6 @@ where
             }
         };
 
-        // Check for a valid Merkle root. To prevent malleability (CVE-2012-2459),
-        // we also need to check whether the transaction hashes are unique.
-
-        let transaction_hashes = block
-            .transactions
-            .iter()
-            .map(|tx| tx.hash())
-            .collect::<Vec<_>>();
-        let merkle_root = transaction_hashes.iter().cloned().collect();
-
-        if block.header.merkle_root != merkle_root {
-            tx.send(Err(VerifyCheckpointError::BadMerkleRoot {
-                expected: block.header.merkle_root,
-                actual: merkle_root,
-            }))
-            .expect("rx has not been dropped yet");
-            return rx;
-        }
-
-        // Collecting into a HashSet deduplicates, so this checks that there
-        // are no duplicate transaction hashes, preventing Merkle root malleability.
-        if transaction_hashes.len() != transaction_hashes.iter().collect::<HashSet<_>>().len() {
-            tx.send(Err(VerifyCheckpointError::DuplicateTransaction))
-                .expect("rx has not been dropped yet");
-            return rx;
-        }
-
         // Since we're using Arc<Block>, each entry is a single pointer to the
         // Arc. But there are a lot of QueuedBlockLists in the queue, so we keep
         // allocations as small as possible.