rewind to header as part of txhashset validation (#808)

* rewind to header as part of txhashset validation
otherwise we risk including a new block and the roots do not match

* fix bug in rm_log rewind (wants to be inclusive of provided index)

* put block marker in the index so we can rewind correctly
during validation of the new txhashset

* rustfmt

chain/src/chain.rs

@@ -515,10 +515,14 @@ impl Chain {
 		let header = self.store.get_block_header(&h)?;
 		txhashset::zip_write(self.db_root.clone(), txhashset_data)?;
 
+		// write the block marker so we can safely rewind to
+		// the pos for that block when we validate the extension below
+		self.store
+			.save_block_marker(&h, &(rewind_to_output, rewind_to_kernel))?;
+
 		let mut txhashset =
 			txhashset::TxHashSet::open(self.db_root.clone(), self.store.clone(), None)?;
 		txhashset::extending(&mut txhashset, |extension| {
-			extension.rewind_pos(header.height, rewind_to_output, rewind_to_kernel)?;
 			extension.validate(&header)?;
 			// TODO validate kernels and their sums with Outputs
 			extension.rebuild_index()?;

chain/src/txhashset.rs

@@ -550,8 +550,13 @@ impl<'a> Extension<'a> {
 		}
 	}
 
-	/// Validate the current txhashset state against a block header
-	pub fn validate(&self, header: &BlockHeader) -> Result<(), Error> {
+	/// Validate the txhashset state against the provided block header.
+	/// Rewinds to that pos for the header first so we see a consistent
+	/// view of the world.
+	pub fn validate(&mut self, header: &BlockHeader) -> Result<(), Error> {
+		// first rewind to the provided header
+		&self.rewind(header)?;
+
 		// validate all hashes and sums within the trees
 		if let Err(e) = self.output_pmmr.validate() {
 			return Err(Error::InvalidTxHashSet(e));

chain/src/types.rs

@@ -44,6 +44,7 @@ bitflags! {
 
 /// A helper to hold the roots of the txhashset in order to keep them
 /// readable
+#[derive(Debug)]
 pub struct TxHashSetRoots {
 	/// Output root
 	pub output_root: Hash,

store/src/pmmr.rs

@@ -215,10 +215,11 @@ where
 	fn dump_stats(&self) {
 		debug!(
 			LOGGER,
-			"pmmr backend: unpruned - {}, hashes - {}, data - {}",
+			"pmmr backend: unpruned - {}, hashes - {}, data - {}, rm_log - {:?}",
 			self.unpruned_size().unwrap_or(0),
 			self.hash_size().unwrap_or(0),
-			self.data_size().unwrap_or(0)
+			self.data_size().unwrap_or(0),
+			self.rm_log.removed
 		);
 	}
 }

store/src/types.rs

@@ -250,21 +250,21 @@ impl RemoveLog {
 		Ok(rl)
 	}
 
-	/// Truncate and empties the remove log.
-	pub fn rewind(&mut self, last_offs: u32) -> io::Result<()> {
+	/// Rewinds the remove log back to the provided index.
+	/// We keep everything in the rm_log from that index and earlier.
+	/// In practice the index is a block height, so we rewind back to that block
+	/// keeping everything in the rm_log up to and including that block.
+	pub fn rewind(&mut self, idx: u32) -> io::Result<()> {
 		// simplifying assumption: we always remove older than what's in tmp
 		self.removed_tmp = vec![];
 		// backing it up before truncating
 		self.removed_bak = self.removed.clone();
 
-		if last_offs == 0 {
+		if idx == 0 {
 			self.removed = vec![];
 		} else {
-			self.removed = self.removed
-				.iter()
-				.filter(|&&(_, idx)| idx < last_offs)
-				.map(|x| *x)
-				.collect();
+			// retain rm_log entries up to and including those at the provided index
+			self.removed.retain(|&(_, x)| x <= idx);
 		}
 		Ok(())
 	}