use head (not header_head) when rewinding full blocks (mimblewimble#3017

)

* use head (not header_head) when rewinding to find fork point for full blocks

* get rid of shortcut when nothing to rewind - always something to rewind

chain/src/chain.rs

@@ -528,8 +528,8 @@ impl Chain {
 		// latest block header. Rewind the extension to the specified header to
 		// ensure the view is consistent.
 		txhashset::extending_readonly(&mut txhashset, |extension| {
-			let header_head = extension.batch.header_head()?;
-			pipe::rewind_and_apply_fork(&header, &header_head, extension)?;
+			let head = extension.batch.head()?;
+			pipe::rewind_and_apply_fork(&header, &head, extension)?;
 			extension.validate(fast_validation, &NoStatus)?;
 			Ok(())
 		})
@@ -542,8 +542,8 @@ impl Chain {
 		let (prev_root, roots, sizes) =
 			txhashset::extending_readonly(&mut txhashset, |extension| {
 				let previous_header = extension.batch.get_previous_header(&b.header)?;
-				let header_head = extension.batch.header_head()?;
-				pipe::rewind_and_apply_fork(&previous_header, &header_head, extension)?;
+				let head = extension.batch.head()?;
+				pipe::rewind_and_apply_fork(&previous_header, &head, extension)?;
 
 				// Retrieve the header root before we apply the new block
 				let prev_root = extension.header_root()?;
@@ -581,8 +581,8 @@ impl Chain {
 	) -> Result<MerkleProof, Error> {
 		let mut txhashset = self.txhashset.write();
 		let merkle_proof = txhashset::extending_readonly(&mut txhashset, |extension| {
-			let header_head = extension.batch.header_head()?;
-			pipe::rewind_and_apply_fork(&header, &header_head, extension)?;
+			let head = extension.batch.head()?;
+			pipe::rewind_and_apply_fork(&header, &head, extension)?;
 			extension.merkle_proof(output)
 		})?;
 
@@ -636,8 +636,8 @@ impl Chain {
 		{
 			let mut txhashset = self.txhashset.write();
 			txhashset::extending_readonly(&mut txhashset, |extension| {
-				let header_head = extension.batch.header_head()?;
-				pipe::rewind_and_apply_fork(&header, &header_head, extension)?;
+				let head = extension.batch.head()?;
+				pipe::rewind_and_apply_fork(&header, &head, extension)?;
 
 				extension.snapshot()?;
 				Ok(())
@@ -1517,8 +1517,8 @@ fn setup_head(
 				})?;
 
 				let res = txhashset::extending(txhashset, &mut batch, |extension| {
-					let header_head = extension.batch.header_head()?;
-					pipe::rewind_and_apply_fork(&header, &header_head, extension)?;
+					let head = extension.batch.head()?;
+					pipe::rewind_and_apply_fork(&header, &head, extension)?;
 					extension.validate_roots()?;
 
 					// now check we have the "block sums" for the block in question

chain/src/pipe.rs

@@ -87,7 +87,6 @@ pub fn process_block(b: &Block, ctx: &mut BlockContext<'_>) -> Result<Option<Tip
 	check_known(&b.header, ctx)?;
 
 	let head = ctx.batch.head()?;
-	let header_head = ctx.batch.header_head()?;
 
 	let is_next = b.header.prev_hash == head.last_block_h;
 
@@ -115,7 +114,7 @@ pub fn process_block(b: &Block, ctx: &mut BlockContext<'_>) -> Result<Option<Tip
 	// Start a chain extension unit of work dependent on the success of the
 	// internal validation and saving operations
 	let block_sums = txhashset::extending(&mut ctx.txhashset, &mut ctx.batch, |mut extension| {
-		rewind_and_apply_fork(&prev, &header_head, extension)?;
+		rewind_and_apply_fork(&prev, &head, extension)?;
 
 		// Check any coinbase being spent have matured sufficiently.
 		// This needs to be done within the context of a potentially
@@ -527,12 +526,6 @@ pub fn rewind_and_apply_header_fork(
 	header: &BlockHeader,
 	ext: &mut txhashset::HeaderExtension<'_>,
 ) -> Result<(), Error> {
-	let head = ext.head();
-	if header.hash() == head.last_block_h {
-		// Nothing to rewind and nothing to reapply. Done.
-		return Ok(());
-	}
-
 	let mut fork_hashes = vec![];
 	let mut current = header.clone();
 	while current.height > 0 && !ext.is_on_current_chain(&current).is_ok() {

chain/tests/mine_simple_chain.rs

@@ -89,77 +89,238 @@ fn mine_short_chain() {
 	clean_output_dir(chain_dir);
 }
 
+// Convenience wrapper for processing a full block on the test chain.
+fn process_header(chain: &Chain, header: &BlockHeader) {
+	chain
+		.process_block_header(header, chain::Options::SKIP_POW)
+		.unwrap();
+}
+
+// Convenience wrapper for processing a block header on the test chain.
+fn process_block(chain: &Chain, block: &Block) {
+	chain
+		.process_block(block.clone(), chain::Options::SKIP_POW)
+		.unwrap();
+}
+
+//
+// a - b - c
+//  \
+//   - b'
+//
+// Process in the following order -
+// 1. block_a
+// 2. block_b
+// 3. block_b'
+// 4. header_c
+// 5. block_c
+//
 #[test]
-fn process_headers_first_with_fork() {
-	let chain_dir = ".grin.fork_headers";
+fn test_block_a_block_b_block_b_fork_header_c_fork_block_c() {
+	let chain_dir = ".grin.block_a_block_b_block_b_fork_header_c_fork_block_c";
 	clean_output_dir(chain_dir);
-
 	global::set_mining_mode(ChainTypes::AutomatedTesting);
 	let kc = ExtKeychain::from_random_seed(false).unwrap();
 	let genesis = pow::mine_genesis_block().unwrap();
+	let last_status = RwLock::new(None);
+	let adapter = Arc::new(StatusAdapter::new(last_status));
+	let chain = setup_with_status_adapter(chain_dir, genesis.clone(), adapter.clone());
+
+	let block_a = prepare_block(&kc, &chain.head_header().unwrap(), &chain, 1);
+	process_block(&chain, &block_a);
+
+	let block_b = prepare_block(&kc, &block_a.header, &chain, 2);
+	let block_b_fork = prepare_block(&kc, &block_a.header, &chain, 2);
+
+	process_block(&chain, &block_b);
+	process_block(&chain, &block_b_fork);
+
+	let block_c = prepare_block(&kc, &block_b.header, &chain, 3);
+	process_header(&chain, &block_c.header);
 
+	assert_eq!(chain.head().unwrap(), Tip::from_header(&block_b.header));
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_c.header)
+	);
+
+	process_block(&chain, &block_c);
+
+	assert_eq!(chain.head().unwrap(), Tip::from_header(&block_c.header));
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_c.header)
+	);
+
+	clean_output_dir(chain_dir);
+}
+
+//
+// a - b
+//  \
+//   - b' - c'
+//
+// Process in the following order -
+// 1. block_a
+// 2. block_b
+// 3. block_b'
+// 4. header_c'
+// 5. block_c'
+//
+#[test]
+fn test_block_a_block_b_block_b_fork_header_c_fork_block_c_fork() {
+	let chain_dir = ".grin.block_a_block_b_block_b_fork_header_c_fork_block_c_fork";
+	clean_output_dir(chain_dir);
+	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	let kc = ExtKeychain::from_random_seed(false).unwrap();
+	let genesis = pow::mine_genesis_block().unwrap();
 	let last_status = RwLock::new(None);
 	let adapter = Arc::new(StatusAdapter::new(last_status));
 	let chain = setup_with_status_adapter(chain_dir, genesis.clone(), adapter.clone());
 
-	let prev = chain.head_header().unwrap();
-	assert_eq!(prev, genesis.header);
+	let block_a = prepare_block(&kc, &chain.head_header().unwrap(), &chain, 1);
+	process_block(&chain, &block_a);
 
-	// First process the header for a block mined on top of our previous block (genesis).
-	let b1 = prepare_block(&kc, &prev, &chain, 1);
-	chain
-		.process_block_header(&b1.header, chain::Options::SKIP_POW)
-		.unwrap();
+	let block_b = prepare_block(&kc, &block_a.header, &chain, 2);
+	let block_b_fork = prepare_block(&kc, &block_a.header, &chain, 2);
 
-	// Now mine a fork block and process this header.
-	// Note: We have not yet processed the competing full block.
-	// This header should also be accepted on the header MMR (after necessary rewind).
-	// But this should not update header_head as this is a losing fork.
-	let b2 = prepare_block(&kc, &prev, &chain, 1);
-	chain
-		.process_block_header(&b2.header, chain::Options::SKIP_POW)
-		.unwrap();
+	process_block(&chain, &block_b);
+	process_block(&chain, &block_b_fork);
 
-	// Check our header_head reflects b1 (first one wins).
-	let head_header = chain.header_head().unwrap();
-	assert_eq!(head_header, Tip::from_header(&b1.header));
+	let block_c_fork = prepare_block(&kc, &block_b_fork.header, &chain, 3);
+	process_header(&chain, &block_c_fork.header);
 
-	// Now process the full block for b2.
-	chain
-		.process_block(b2.clone(), chain::Options::SKIP_POW)
-		.unwrap();
+	assert_eq!(chain.head().unwrap(), Tip::from_header(&block_b.header));
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_c_fork.header)
+	);
 
-	// Check head reflects b2 as this is the winning full block at this height.
-	let head = chain.head().unwrap();
-	assert_eq!(head, Tip::from_header(&b2.header));
+	process_block(&chain, &block_c_fork);
 
-	// BUT - header_head *still* references b1 (this is weird but ok).
-	let head_header = chain.header_head().unwrap();
-	assert_eq!(head_header, Tip::from_header(&b1.header));
+	assert_eq!(
+		chain.head().unwrap(),
+		Tip::from_header(&block_c_fork.header)
+	);
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_c_fork.header)
+	);
 
-	// Now process the full block for b1.
-	chain
-		.process_block(b1.clone(), chain::Options::SKIP_POW)
-		.unwrap();
+	clean_output_dir(chain_dir);
+}
 
-	// Check head still reflects b2 as this is the winning full block at this height.
-	let head = chain.head().unwrap();
-	assert_eq!(head, Tip::from_header(&b2.header));
+//
+// a - b - c
+//  \
+//   - b'
+//
+// Process in the following order -
+// 1. block_a
+// 2. header_b
+// 3. header_b_fork
+// 4. block_b_fork
+// 5. block_b
+// 6. block_c
+//
+#[test]
+fn test_block_a_header_b_header_b_fork_block_b_fork_block_b_block_c() {
+	let chain_dir = ".grin.test_block_a_header_b_header_b_fork_block_b_fork_block_b_block_c";
+	clean_output_dir(chain_dir);
+	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	let kc = ExtKeychain::from_random_seed(false).unwrap();
+	let genesis = pow::mine_genesis_block().unwrap();
+	let last_status = RwLock::new(None);
+	let adapter = Arc::new(StatusAdapter::new(last_status));
+	let chain = setup_with_status_adapter(chain_dir, genesis.clone(), adapter.clone());
 
-	// Check header_head *still* references b1 (still weird but ok).
-	let head_header = chain.header_head().unwrap();
-	assert_eq!(head_header, Tip::from_header(&b1.header));
+	let block_a = prepare_block(&kc, &chain.head_header().unwrap(), &chain, 1);
+	process_block(&chain, &block_a);
 
-	let b3 = prepare_block(&kc, &b1.header, &chain, 2);
-	chain
-		.process_block(b3.clone(), chain::Options::SKIP_POW)
-		.unwrap();
+	let block_b = prepare_block(&kc, &block_a.header, &chain, 2);
+	let block_b_fork = prepare_block(&kc, &block_a.header, &chain, 2);
+
+	process_header(&chain, &block_b.header);
+	process_header(&chain, &block_b_fork.header);
+	process_block(&chain, &block_b_fork);
+	process_block(&chain, &block_b);
+
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_b.header)
+	);
+	assert_eq!(
+		chain.head().unwrap(),
+		Tip::from_header(&block_b_fork.header)
+	);
+
+	let block_c = prepare_block(&kc, &block_b.header, &chain, 3);
+	process_block(&chain, &block_c);
+
+	assert_eq!(chain.head().unwrap(), Tip::from_header(&block_c.header));
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_c.header)
+	);
+
+	clean_output_dir(chain_dir);
+}
+
+//
+// a - b
+//  \
+//   - b' - c'
+//
+// Process in the following order -
+// 1. block_a
+// 2. header_b
+// 3. header_b_fork
+// 4. block_b_fork
+// 5. block_b
+// 6. block_c_fork
+//
+#[test]
+fn test_block_a_header_b_header_b_fork_block_b_fork_block_b_block_c_fork() {
+	let chain_dir = ".grin.test_block_a_header_b_header_b_fork_block_b_fork_block_b_block_c_fork";
+	clean_output_dir(chain_dir);
+	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	let kc = ExtKeychain::from_random_seed(false).unwrap();
+	let genesis = pow::mine_genesis_block().unwrap();
+	let last_status = RwLock::new(None);
+	let adapter = Arc::new(StatusAdapter::new(last_status));
+	let chain = setup_with_status_adapter(chain_dir, genesis.clone(), adapter.clone());
 
-	// Check head and header_head both reflect b3.
-	let head = chain.head().unwrap();
-	assert_eq!(head, Tip::from_header(&b3.header));
-	let header_head = chain.header_head().unwrap();
-	assert_eq!(header_head, Tip::from_header(&b3.header));
+	let block_a = prepare_block(&kc, &chain.head_header().unwrap(), &chain, 1);
+	process_block(&chain, &block_a);
+
+	let block_b = prepare_block(&kc, &block_a.header, &chain, 2);
+	let block_b_fork = prepare_block(&kc, &block_a.header, &chain, 2);
+
+	process_header(&chain, &block_b.header);
+	process_header(&chain, &block_b_fork.header);
+	process_block(&chain, &block_b_fork);
+	process_block(&chain, &block_b);
+
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_b.header)
+	);
+	assert_eq!(
+		chain.head().unwrap(),
+		Tip::from_header(&block_b_fork.header)
+	);
+
+	let block_c_fork = prepare_block(&kc, &block_b_fork.header, &chain, 3);
+	process_block(&chain, &block_c_fork);
+
+	assert_eq!(
+		chain.head().unwrap(),
+		Tip::from_header(&block_c_fork.header)
+	);
+	assert_eq!(
+		chain.header_head().unwrap(),
+		Tip::from_header(&block_c_fork.header)
+	);
 
 	clean_output_dir(chain_dir);
 }