Return None from merkle_root functions

The merkle_root of an empty tree is undefined, this is the only error
case we have for the two `bitcoin_merkle_root*` functions. We can fully
describe this error case by returning an `Option` if args are found to
be empty. We can do the same for the wrapper functions in `block`
module.

While we are at it, refactor out a recursive helper function to make
reading the code between the two functions easier.

Author: tcharding

src/blockdata/block.rs

@@ -176,7 +176,10 @@ impl Block {
 
     /// check if merkle root of header matches merkle root of the transaction list
     pub fn check_merkle_root (&self) -> bool {
-        self.header.merkle_root == self.merkle_root()
+        match self.merkle_root() {
+            Some(merkle_root) => self.header.merkle_root == merkle_root,
+            None => false,
+        }
     }
 
     /// check if witness commitment in coinbase is matching the transaction list
@@ -197,8 +200,10 @@ impl Block {
                     let commitment = WitnessCommitment::from_slice(&coinbase.output[pos].script_pubkey.as_bytes()[6..38]).unwrap();
                     // witness reserved value is in coinbase input witness
                     if coinbase.input[0].witness.len() == 1 && coinbase.input[0].witness[0].len() == 32 {
-                        let witness_root = self.witness_root();
-                        return commitment == Self::compute_witness_commitment(&witness_root, coinbase.input[0].witness[0].as_slice())
+                        match self.witness_root() {
+                            Some(witness_root) => return commitment == Self::compute_witness_commitment(&witness_root, coinbase.input[0].witness[0].as_slice()),
+                            None => return false,
+                        }
                     }
                 }
             }
@@ -207,9 +212,9 @@ impl Block {
     }
 
     /// Calculate the transaction merkle root.
-    pub fn merkle_root(&self) -> TxMerkleNode {
+    pub fn merkle_root(&self) -> Option<TxMerkleNode> {
         let hashes = self.txdata.iter().map(|obj| obj.txid().as_hash());
-        bitcoin_merkle_root(hashes).into()
+        bitcoin_merkle_root(hashes).map(|h| h.into())
     }
 
     /// compute witness commitment for the transaction list
@@ -221,7 +226,7 @@ impl Block {
     }
 
     /// Merkle root of transactions hashed for witness
-    pub fn witness_root(&self) -> WitnessMerkleNode {
+    pub fn witness_root(&self) -> Option<WitnessMerkleNode> {
         let hashes = self.txdata.iter().enumerate().map(|(i, t)|
             if i == 0 {
                 // Replace the first hash with zeroes.
@@ -230,7 +235,7 @@ impl Block {
                 t.wtxid().as_hash()
             }
         );
-        bitcoin_merkle_root(hashes).into()
+        bitcoin_merkle_root(hashes).map(|h| h.into())
     }
 
     /// The size of the header + the size of the varint with the tx count + the txs themselves
@@ -371,7 +376,7 @@ mod tests {
         let real_decode = decode.unwrap();
         assert_eq!(real_decode.header.version, 1);
         assert_eq!(serialize(&real_decode.header.prev_blockhash), prevhash);
-        assert_eq!(real_decode.header.merkle_root, real_decode.merkle_root());
+        assert_eq!(real_decode.header.merkle_root, real_decode.merkle_root().unwrap());
         assert_eq!(serialize(&real_decode.header.merkle_root), merkle);
         assert_eq!(real_decode.header.time, 1231965655);
         assert_eq!(real_decode.header.bits, 486604799);
@@ -407,7 +412,7 @@ mod tests {
         assert_eq!(real_decode.header.version, 0x20000000);  // VERSIONBITS but no bits set
         assert_eq!(serialize(&real_decode.header.prev_blockhash), prevhash);
         assert_eq!(serialize(&real_decode.header.merkle_root), merkle);
-        assert_eq!(real_decode.header.merkle_root, real_decode.merkle_root());
+        assert_eq!(real_decode.header.merkle_root, real_decode.merkle_root().unwrap());
         assert_eq!(real_decode.header.time, 1472004949);
         assert_eq!(real_decode.header.bits, 0x1a06d450);
         assert_eq!(real_decode.header.nonce, 1879759182);

src/util/hash.rs

@@ -17,6 +17,8 @@
 //! merkleization.
 //!
 
+use core::iter;
+
 use prelude::*;
 
 use io;
@@ -25,59 +27,81 @@ use core::cmp::min;
 use hashes::Hash;
 use consensus::encode::Encodable;
 
-/// Calculates the merkle root of a list of hashes inline
-/// into the allocated slice.
+/// Calculates the merkle root of a list of *hashes*, inline (in place) in `hashes`.
 ///
 /// In most cases, you'll want to use [bitcoin_merkle_root] instead.
-pub fn bitcoin_merkle_root_inline<T>(data: &mut [T]) -> T
+///
+/// # Returns
+/// - `None` if `hashes` is empty. The merkle root of an empty tree of hashes is undefined.
+/// - `Some(hash)` if `hashes` contains one element. A single hash is by definition the merkle root.
+/// - `Some(merkle_root)` if length of `hashes` is greater than one.
+pub fn bitcoin_merkle_root_inline<T>(hashes: &mut [T]) -> Option<T>
     where T: Hash + Encodable,
           <T as Hash>::Engine: io::Write,
 {
-    // Base case
-    if data.is_empty() {
-        return Default::default();
-    }
-    if data.len() < 2 {
-        return T::from_inner(data[0].into_inner());
-    }
-    // Recursion
-    for idx in 0..((data.len() + 1) / 2) {
-        let idx1 = 2 * idx;
-        let idx2 = min(idx1 + 1, data.len() - 1);
-        let mut encoder = T::engine();
-        data[idx1].consensus_encode(&mut encoder).expect("in-memory writers don't error");
-        data[idx2].consensus_encode(&mut encoder).expect("in-memory writers don't error");
-        data[idx] = T::from_engine(encoder);
+    match hashes.len() {
+        0 => None,
+        1 => Some(hashes[0]),
+        _ => Some(merkle_root_r(hashes)),
     }
-    let half_len = data.len() / 2 + data.len() % 2;
-    bitcoin_merkle_root_inline(&mut data[0..half_len])
 }
 
-/// Calculates the merkle root of an iterator of hashes.
-pub fn bitcoin_merkle_root<T, I>(mut iter: I) -> T
+/// Calculates the merkle root of an iterator of *hashes*.
+///
+/// # Returns
+/// - `None` if `hashes` is empty. The merkle root of an empty tree of hashes is undefined.
+/// - `Some(hash)` if `hashes` contains one element. A single hash is by definition the merkle root.
+/// - `Some(merkle_root)` if length of `hashes` is greater than one.
+pub fn bitcoin_merkle_root<T, I>(mut hashes: I) -> Option<T>
     where T: Hash + Encodable,
           <T as Hash>::Engine: io::Write,
-          I: ExactSizeIterator<Item = T>,
+          I: Iterator<Item = T>,
 {
-    // Base case
-    if iter.len() == 0 {
-        return Default::default();
-    }
-    if iter.len() == 1 {
-        return T::from_inner(iter.next().unwrap().into_inner());
-    }
-    // Recursion
-    let half_len = iter.len() / 2 + iter.len() % 2;
-    let mut alloc = Vec::with_capacity(half_len);
-    while let Some(hash1) = iter.next() {
+    let first = hashes.next()?;
+    let second = match hashes.next() {
+        Some(second) => second,
+        None => return Some(first),
+    };
+
+    let mut hashes = iter::once(first).chain(iter::once(second)).chain(hashes);
+
+    // We need a local copy to pass to `merkle_root_r`. It's more efficient to do the first loop of
+    // processing as we make the copy instead of copying the whole iterator.
+    let (min, max) = hashes.size_hint();
+    let mut alloc = Vec::with_capacity(max.unwrap_or(min) / 2 + 1);
+
+    while let Some(hash1) = hashes.next() {
         // If the size is odd, use the last element twice.
-        let hash2 = iter.next().unwrap_or(hash1);
+        let hash2 = hashes.next().unwrap_or(hash1);
         let mut encoder = T::engine();
         hash1.consensus_encode(&mut encoder).expect("in-memory writers don't error");
         hash2.consensus_encode(&mut encoder).expect("in-memory writers don't error");
         alloc.push(T::from_engine(encoder));
     }
-    bitcoin_merkle_root_inline(&mut alloc)
+
+    Some(merkle_root_r(&mut alloc))
+}
+
+// `hashes` must contain at least one hash.
+fn merkle_root_r<T>(hashes: &mut [T]) -> T
+    where T: Hash + Encodable,
+          <T as Hash>::Engine: io::Write,
+{
+    if hashes.len() == 1 {
+        return hashes[0]
+    }
+
+    for idx in 0..((hashes.len() + 1) / 2) {
+        let idx1 = 2 * idx;
+        let idx2 = min(idx1 + 1, hashes.len() - 1);
+        let mut encoder = T::engine();
+        hashes[idx1].consensus_encode(&mut encoder).expect("in-memory writers don't error");
+        hashes[idx2].consensus_encode(&mut encoder).expect("in-memory writers don't error");
+        hashes[idx] = T::from_engine(encoder);
+    }
+    let half_len = hashes.len() / 2 + hashes.len() % 2;
+
+    merkle_root_r(&mut hashes[0..half_len])
 }
 
 #[cfg(test)]

src/util/merkleblock.rs

@@ -543,7 +543,7 @@ mod tests {
 
             // Calculate the merkle root and height
             let hashes = txids.iter().map(|t| t.as_hash());
-            let merkle_root_1: TxMerkleNode = bitcoin_merkle_root(hashes).into();
+            let merkle_root_1: TxMerkleNode = bitcoin_merkle_root(hashes).expect("hashes is not empty").into();
             let mut height = 1;
             let mut ntx = num_tx;
             while ntx > 1 {