Introduce extending_readonly to simplify a forcing and cancelling rollbacks (#945)

readonly views of the txhashset

chain/src/chain.rs

@@ -435,7 +435,7 @@ impl Chain {
 		// against the latest block header.
 		// We will rewind the extension internally to the pos for
 		// the block header to ensure the view is consistent.
-		txhashset::extending(&mut txhashset, |extension| {
+		txhashset::extending_readonly(&mut txhashset, |extension| {
 			extension.validate(&header, skip_rproofs)
 		})
 	}
@@ -460,13 +460,12 @@ impl Chain {
 		let mut txhashset = self.txhashset.write().unwrap();
 		let store = self.store.clone();
 
-		let roots = txhashset::extending(&mut txhashset, |extension| {
+		let roots = txhashset::extending_readonly(&mut txhashset, |extension| {
 			// apply the block on the txhashset and check the resulting root
 			if is_fork {
 				pipe::rewind_and_apply_fork(b, store, extension)?;
 			}
 			extension.apply_block(b)?;
-			extension.force_rollback();
 			Ok(extension.roots())
 		})?;
 
@@ -484,7 +483,7 @@ impl Chain {
 	) -> Result<MerkleProof, Error> {
 		let mut txhashset = self.txhashset.write().unwrap();
 
-		let merkle_proof = txhashset::extending(&mut txhashset, |extension| {
+		let merkle_proof = txhashset::extending_readonly(&mut txhashset, |extension| {
 			extension.merkle_proof(output, block_header)
 		})?;
 
@@ -549,10 +548,6 @@ impl Chain {
 			txhashset::TxHashSet::open(self.db_root.clone(), self.store.clone(), None)?;
 		txhashset::extending(&mut txhashset, |extension| {
 			extension.validate(&header, false)?;
-
-			// validate rewinds and rollbacks, in this specific case we want to
-			// apply the rewind
-			extension.cancel_rollback();
 			extension.rebuild_index()?;
 			Ok(())
 		})?;
@@ -600,7 +595,7 @@ impl Chain {
 			txhashes.compact()?;
 
 			// print out useful debug info after compaction
-			txhashset::extending(&mut txhashes, |extension| {
+			txhashset::extending_readonly(&mut txhashes, |extension| {
 				extension.dump_output_pmmr();
 				Ok(())
 			})?;

chain/src/txhashset.rs

@@ -236,6 +236,42 @@ impl TxHashSet {
 	}
 }
 
+/// Starts a new unit of work to extend (or rewind) the chain with additional blocks.
+/// Accepts a closure that will operate within that unit of work.
+/// The closure has access to an Extension object that allows the addition
+/// of blocks to the txhashset and the checking of the current tree roots.
+///
+/// The unit of work is always discarded (always rollback) as this is read-only.
+pub fn extending_readonly<'a, F, T>(trees: &'a mut TxHashSet, inner: F) -> Result<T, Error>
+where
+	F: FnOnce(&mut Extension) -> Result<T, Error>,
+{
+	let sizes: (u64, u64, u64);
+	let res: Result<T, Error>;
+	{
+		let commit_index = trees.commit_index.clone();
+
+		trace!(LOGGER, "Starting new txhashset (readonly) extension.");
+		let mut extension = Extension::new(trees, commit_index);
+		res = inner(&mut extension);
+
+		sizes = extension.sizes();
+	}
+
+	debug!(
+		LOGGER,
+		"Rollbacking txhashset (readonly) extension. sizes {:?}", sizes
+	);
+
+	trees.output_pmmr_h.backend.discard();
+	trees.rproof_pmmr_h.backend.discard();
+	trees.kernel_pmmr_h.backend.discard();
+
+	trace!(LOGGER, "TxHashSet (readonly) extension done.");
+
+	res
+}
+
 /// Starts a new unit of work to extend the chain with additional blocks,
 /// accepting a closure that will work within that unit of work. The closure
 /// has access to an Extension object that allows the addition of blocks to
@@ -485,8 +521,8 @@ impl<'a> Extension<'a> {
 			block_header.hash()
 		);
 
-		// rewind to the specified block and set the force_rollback flag (read-only)
+		// rewind to the specified block for a consistent view
-		self.rewind_readonly(block_header)?;
+		self.rewind(block_header)?;
 
 		// then calculate the Merkle Proof based on the known pos
 		let pos = self.get_output_pos(&output.commit)?;
@@ -510,22 +546,6 @@ impl<'a> Extension<'a> {
 		Ok(())
 	}
 
-	/// Rewinds the MMRs to the provided block, using the last output and
-	/// last kernel of the block we want to rewind to.
-	pub fn rewind_readonly(&mut self, block_header: &BlockHeader) -> Result<(), Error> {
-		// first make sure we set the rollback flag (read-only use of the extension)
-		self.force_rollback();
-
-		let hash = block_header.hash();
-		let height = block_header.height;
-		debug!(
-			LOGGER,
-			"Rewinding (readonly) to header {} at {}", hash, height
-		);
-
-		self.rewind(block_header)
-	}
-
 	/// Rewinds the MMRs to the provided positions, given the output and
 	/// kernel we want to rewind to.
 	fn rewind_pos(
@@ -576,9 +596,8 @@ impl<'a> Extension<'a> {
 	/// Note: this is an expensive operation and sets force_rollback
 	/// so the extension is read-only.
 	pub fn validate(&mut self, header: &BlockHeader, skip_rproofs: bool) -> Result<(), Error> {
-		// first rewind to the provided header and
+		// rewind to the provided header for a consistent view
-		// set the force_rollback flag (read-only)
+		&self.rewind(header)?;
-		&self.rewind_readonly(header)?;
 
 		// validate all hashes and sums within the trees
 		if let Err(e) = self.output_pmmr.validate() {
@@ -650,11 +669,6 @@ impl<'a> Extension<'a> {
 		self.rollback = true;
 	}
 
-	/// Cancel a previous rollback, to apply this extension
-	pub fn cancel_rollback(&mut self) {
-		self.rollback = false;
-	}
-
 	/// Dumps the output MMR.
 	/// We use this after compacting for visual confirmation that it worked.
 	pub fn dump_output_pmmr(&self) {