Merge upstream changes for 'process_block_single'

- Essentially buckets the operations into more readable functions, removes unnecessary and hard-to-read code
- Maybe a small speedup here

chain/src/chain.rs

@@ -284,8 +284,9 @@ impl Chain {
 		Ok(head)
 	}
 
-	/// Processes a single block, then checks for orphans, processing
-	/// those as well if they're found
+	/// Processes a single block, then checks for an orphan, regardless
+	/// of first block processing successfully. Ensures we check orphans
+	/// once, each time we receive a new block from peers
 	pub fn process_block(&self, b: Block, opts: Options) -> Result<Option<Tip>, Error> {
 		let block_height = b.header.height;
 		let orphan_height;
@@ -328,10 +329,76 @@ impl Chain {
 		}
 	}
 
+	/// Quick check for "known" duplicate block up to and including current chain head.
+	/// Returns an error if this block is "known".
+	pub fn is_known(&self, header: &BlockHeader) -> Result<(), Error> {
+		let head = self.head()?;
+		if head.hash() == header.hash() {
+			return Err(ErrorKind::Unfit("duplicate block".to_owned()).into());
+		}
+		if header.total_difficulty() <= head.total_difficulty {
+			if self.block_exists(header.hash())? {
+				debug!(
+					"Block {} at {} is unfit at this time, duplicate block",
+					header.hash(),
+					header.height,
+				);
+				return Err(ErrorKind::Unfit("duplicate block".to_owned()).into());
+			}
+		}
+		Ok(())
+	}
+
+	// Check if the provided block is an orphan.
+	// If block is an orphan add it to our orphan block pool for deferred processing.
+	// If this is the "next" block immediately following current head then not an orphan.
+	// Or if we have the previous full block then not an orphan.
+	fn check_orphan(&self, block: &Block, opts: Options) -> Result<(), Error> {
+		let head = self.head()?;
+		let is_next = block.header.prev_hash == head.last_block_h;
+		if is_next || self.block_exists(block.header.prev_hash)? {
+			return Ok(());
+		}
+
+		let block_hash = block.hash();
+		let orphan = Orphan {
+			block: block.clone(),
+			opts,
+			added: Instant::now(),
+		};
+		self.orphans.add(orphan);
+
+		debug!(
+			"is_orphan: {:?}, # orphans {}{}",
+			block_hash,
+			self.orphans.len(),
+			if self.orphans.len_evicted() > 0 {
+				format!(", # evicted {}", self.orphans.len_evicted())
+			} else {
+				String::new()
+			},
+		);
+
+		Err(ErrorKind::Orphan.into())
+	}
+
 	/// Attempt to add a new block to the chain.
 	/// Returns true if it has been added to the longest chain
 	/// or false if it has added to a fork (or orphan?).
 	fn process_block_single(&self, b: Block, opts: Options) -> Result<Option<Tip>, Error> {
+		// Process the header first.
+		// If invalid then fail early.
+		// If valid then continue with block processing with header_head committed to db etc.
+
+		self.process_block_header(&b.header, opts)?;
+
+		// Check if we already know about this full block.
+		self.is_known(&b.header)?;
+
+		// Check if this block is an orphan.
+		// Only do this once we know the header PoW is valid.
+		self.check_orphan(&b, opts)?;
+
 		let (maybe_new_head, prev_head) = {
 			let mut header_pmmr = self.header_pmmr.write();
 			let mut txhashset = self.txhashset.write();
@@ -340,72 +407,24 @@ impl Chain {
 
 			let prev_head = ctx.batch.head()?;
 
-			let maybe_new_head = pipe::process_block(&b, &mut ctx);
+			let maybe_new_head = pipe::process_block(&b, &mut ctx)?;
 
 			// We have flushed txhashset extension changes to disk
 			// but not yet committed the batch.
 			// A node shutdown at this point can be catastrophic...
 			// We prevent this via the stop_lock (see above).
-			if maybe_new_head.is_ok() {
-				ctx.batch.commit()?;
-			}
+			ctx.batch.commit()?;
 
 			// release the lock and let the batch go before post-processing
 			(maybe_new_head, prev_head)
 		};
 
-		match maybe_new_head {
-			Ok(head) => {
-				let status = self.determine_status(head.clone(), prev_head);
-
-				// notifying other parts of the system of the update
-				self.adapter.block_accepted(&b, status, opts);
-
-				Ok(head)
-			}
-			Err(e) => match e.kind() {
-				ErrorKind::Orphan => {
-					let block_hash = b.hash();
-					let orphan = Orphan {
-						block: b,
-						opts: opts,
-						added: Instant::now(),
-					};
+		let status = self.determine_status(maybe_new_head.clone(), prev_head);
 
-					self.orphans.add(orphan);
+		// notifying other parts of the system of the update
+		self.adapter.block_accepted(&b, status, opts);
 
-					debug!(
-						"process_block: orphan: {:?}, # orphans {}{}",
-						block_hash,
-						self.orphans.len(),
-						if self.orphans.len_evicted() > 0 {
-							format!(", # evicted {}", self.orphans.len_evicted())
-						} else {
-							String::new()
-						},
-					);
-					Err(ErrorKind::Orphan.into())
-				}
-				ErrorKind::Unfit(ref msg) => {
-					debug!(
-						"Block {} at {} is unfit at this time: {}",
-						b.hash(),
-						b.header.height,
-						msg
-					);
-					Err(ErrorKind::Unfit(msg.clone()).into())
-				}
-				_ => {
-					info!(
-						"Rejected block {} at {}: {:?}",
-						b.hash(),
-						b.header.height,
-						e
-					);
-					Err(ErrorKind::Other(format!("{:?}", e)).into())
-				}
-			},
-		}
+		Ok(maybe_new_head)
 	}
 
 	/// Process a block header received during "header first" propagation.