refactor txhashset roots, call validate() on roots during block valid…

…ation

api/src/types.rs

@@ -119,7 +119,7 @@ impl TxHashSet {
 	pub fn from_head(head: Arc<chain::Chain>) -> TxHashSet {
 		let roots = head.get_txhashset_roots();
 		TxHashSet {
-			output_root_hash: roots.output_root.to_hex(),
+			output_root_hash: roots.output_root().to_hex(),
 			range_proof_root_hash: roots.rproof_root.to_hex(),
 			kernel_root_hash: roots.kernel_root.to_hex(),
 		}

chain/src/chain.rs

@@ -594,7 +594,7 @@ impl Chain {
 		b.header.prev_root = prev_root;
 
 		// Set the output, rangeproof and kernel MMR roots.
-		b.header.output_root = roots.output_root;
+		b.header.output_root = roots.output_root();
 		b.header.range_proof_root = roots.rproof_root;
 		b.header.kernel_root = roots.kernel_root;
 

chain/src/txhashset/txhashset.rs

@@ -25,7 +25,7 @@ use crate::error::{Error, ErrorKind};
 use crate::store::{Batch, ChainStore};
 use crate::txhashset::bitmap_accumulator::BitmapAccumulator;
 use crate::txhashset::{RewindableKernelView, UTXOView};
-use crate::types::{OutputMMRPosition, Tip, TxHashSetRoots, TxHashsetWriteStatus};
+use crate::types::{OutputMMRPosition, OutputRoots, Tip, TxHashSetRoots, TxHashsetWriteStatus};
 use crate::util::secp::pedersen::{Commitment, RangeProof};
 use crate::util::{file, secp_static, zip};
 use croaring::Bitmap;
@@ -337,7 +337,10 @@ impl TxHashSet {
 			ReadonlyPMMR::at(&self.kernel_pmmr_h.backend, self.kernel_pmmr_h.last_pos);
 
 		TxHashSetRoots {
-			output_root: output_pmmr.root(),
+			output_roots: OutputRoots {
+				pmmr_root: output_pmmr.root(),
+				bitmap_root: self.bitmap_accumulator.root(),
+			},
 			rproof_root: rproof_pmmr.root(),
 			kernel_root: kernel_pmmr.root(),
 		}
@@ -1131,10 +1134,13 @@ impl<'a> Extension<'a> {
 	/// and kernel sum trees.
 	pub fn roots(&self) -> Result<TxHashSetRoots, Error> {
 		Ok(TxHashSetRoots {
-			output_root: self
-				.output_pmmr
-				.root()
-				.map_err(|_| ErrorKind::InvalidRoot)?,
+			output_roots: OutputRoots {
+				pmmr_root: self
+					.output_pmmr
+					.root()
+					.map_err(|_| ErrorKind::InvalidRoot)?,
+				bitmap_root: self.bitmap_accumulator.root(),
+			},
 			rproof_root: self
 				.rproof_pmmr
 				.root()
@@ -1152,26 +1158,7 @@ impl<'a> Extension<'a> {
 			return Ok(());
 		}
 		let head_header = self.batch.get_block_header(&self.head.hash())?;
-		let header_roots = TxHashSetRoots {
-			output_root: head_header.output_root,
-			rproof_root: head_header.range_proof_root,
-			kernel_root: head_header.kernel_root,
-		};
-		if header_roots != self.roots()? {
-			Err(ErrorKind::InvalidRoot.into())
-		} else {
-			// TODO - just log this for now.
-			// We eventually want to build a combined root
-			// for the output MMR and the bitmnap accumulator.
-			debug!(
-				"{} at {}, bitmap accumulator root: {:?}",
-				head_header.hash(),
-				head_header.height,
-				self.bitmap_accumulator.root(),
-			);
-
-			Ok(())
-		}
+		self.roots()?.validate(&head_header)
 	}
 
 	/// Validate the header, output and kernel MMR sizes against the block header.

chain/src/types.rs

@@ -21,7 +21,7 @@ use crate::core::core::hash::{Hash, Hashed, ZERO_HASH};
 use crate::core::core::{Block, BlockHeader};
 use crate::core::pow::Difficulty;
 use crate::core::ser;
-use crate::error::Error;
+use crate::error::{Error, ErrorKind};
 use crate::util::RwLock;
 
 bitflags! {
@@ -203,18 +203,55 @@ impl TxHashsetWriteStatus for SyncState {
 	}
 }
 
-/// A helper to hold the roots of the txhashset in order to keep them
-/// readable.
-#[derive(Debug, PartialEq)]
+/// A helper for the various txhashset MMR roots.
+#[derive(Debug)]
 pub struct TxHashSetRoots {
-	/// Output root
-	pub output_root: Hash,
+	/// Output roots
+	pub output_roots: OutputRoots,
 	/// Range Proof root
 	pub rproof_root: Hash,
 	/// Kernel root
 	pub kernel_root: Hash,
 }
 
+impl TxHashSetRoots {
+	/// Accessor for the underlying output PMMR root
+	pub fn output_root(&self) -> Hash {
+		self.output_roots.pmmr_root
+	}
+
+	/// Validate roots against the provided block header.
+	pub fn validate(&self, header: &BlockHeader) -> Result<(), Error> {
+		debug!(
+			"validate roots: {} at {}, output_root: {}, output pmmr: {} (bitmap: {})",
+			header.hash(),
+			header.height,
+			header.output_root,
+			self.output_roots.pmmr_root,
+			self.output_roots.bitmap_root,
+		);
+
+		if header.output_root != self.output_roots.pmmr_root {
+			Err(ErrorKind::InvalidRoot.into())
+		} else if header.range_proof_root != self.rproof_root {
+			Err(ErrorKind::InvalidRoot.into())
+		} else if header.kernel_root != self.kernel_root {
+			Err(ErrorKind::InvalidRoot.into())
+		} else {
+			Ok(())
+		}
+	}
+}
+
+/// A helper for the various output roots.
+#[derive(Debug)]
+pub struct OutputRoots {
+	/// The output PMMR root
+	pub pmmr_root: Hash,
+	/// The bitmap accumulator root
+	pub bitmap_root: Hash,
+}
+
 /// A helper to hold the output pmmr position of the txhashset in order to keep them
 /// readable.
 #[derive(Debug)]

chain/tests/bitmap_accumulator.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use self::chain::txhashset::BitmapAccumulator;
-use self::core::core::hash;
+use self::core::core::hash::Hash;
 use self::core::ser::PMMRIndexHashable;
 use bit_vec::BitVec;
 use grin_chain as chain;
@@ -26,7 +26,7 @@ fn test_bitmap_accumulator() {
 
 	let mut bit_vec = BitVec::from_elem(1024, false);
 	let mut accumulator = BitmapAccumulator::new();
-	assert_eq!(accumulator.root(), hash::ZERO_HASH);
+	assert_eq!(accumulator.root(), Hash::default());
 
 	// 1000... (rebuild from 0, setting [0] true)
 	accumulator.apply(vec![0], vec![0]).unwrap();
@@ -38,7 +38,7 @@ fn test_bitmap_accumulator() {
 	// Check that removing the last bit in a chunk removes the now empty chunk
 	// if it is the rightmost chunk.
 	accumulator.apply(vec![0], vec![]).unwrap();
-	assert_eq!(accumulator.root(), hash::ZERO_HASH);
+	assert_eq!(accumulator.root(), Hash::default());
 
 	// 1100... (rebuild from 0, setting [0, 1] true)
 	accumulator.apply(vec![0], vec![0, 1]).unwrap();
@@ -126,7 +126,7 @@ fn test_bitmap_accumulator() {
 	// Here we trim all the "empty" chunks leaving an empty accumulator.
 	// 0000...0000, 0000...0000 (rebuild from 1025, setting [] true)
 	accumulator.apply(vec![1025], vec![]).unwrap();
-	assert_eq!(accumulator.root(), hash::ZERO_HASH);
+	assert_eq!(accumulator.root(), Hash::default());
 
 	// Make sure we pad appropriately with 0s if we set a distant bit to 1.
 	// 0000...0000, 0100...0000 (rebuild from 1025, setting [1025] true)