fix bad-block reporting no reason (#9638)

Author: debris

ethcore/src/client/client.rs

@@ -296,7 +296,6 @@ impl Importer {
 					continue;
 				}
 
-				let raw = block.bytes.clone();
 				match self.check_and_lock_block(block, client) {
 					Ok(closed_block) => {
 						if self.engine.is_proposal(&header) {
@@ -314,8 +313,8 @@ impl Importer {
 						}
 					},
 					Err(err) => {
-						self.bad_blocks.report(raw, format!("{:?}", err));
-						invalid_blocks.insert(header.hash());
+						self.bad_blocks.report(bytes, format!("{:?}", err));
+						invalid_blocks.insert(hash);
 					},
 				}
 			}
@@ -356,23 +355,23 @@ impl Importer {
 		imported
 	}
 
-	fn check_and_lock_block(&self, block: PreverifiedBlock, client: &Client) -> Result<LockedBlock, ()> {
+	fn check_and_lock_block(&self, block: PreverifiedBlock, client: &Client) -> EthcoreResult<LockedBlock> {
 		let engine = &*self.engine;
 		let header = block.header.clone();
 
 		// Check the block isn't so old we won't be able to enact it.
 		let best_block_number = client.chain.read().best_block_number();
 		if client.pruning_info().earliest_state > header.number() {
 			warn!(target: "client", "Block import failed for #{} ({})\nBlock is ancient (current best block: #{}).", header.number(), header.hash(), best_block_number);
-			return Err(());
+			bail!("Block is ancient");
 		}
 
 		// Check if parent is in chain
 		let parent = match client.block_header_decoded(BlockId::Hash(*header.parent_hash())) {
 			Some(h) => h,
 			None => {
 				warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash());
-				return Err(());
+				bail!("Parent not found");
 			}
 		};
 
@@ -391,13 +390,13 @@ impl Importer {
 
 		if let Err(e) = verify_family_result {
 			warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
-			return Err(());
+			bail!(e);
 		};
 
 		let verify_external_result = self.verifier.verify_block_external(&header, engine);
 		if let Err(e) = verify_external_result {
 			warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
-			return Err(());
+			bail!(e);
 		};
 
 		// Enact Verified Block
@@ -417,9 +416,13 @@ impl Importer {
 			&mut chain.ancestry_with_metadata_iter(*header.parent_hash()),
 		);
 
-		let mut locked_block = enact_result.map_err(|e| {
-			warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
-		})?;
+		let mut locked_block = match enact_result {
+			Ok(b) => b,
+			Err(e) => {
+				warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
+				bail!(e);
+			}
+		};
 
 		// Strip receipts for blocks before validate_receipts_transition,
 		// if the expected receipts root header does not match.
@@ -433,7 +436,7 @@ impl Importer {
 		// Final Verification
 		if let Err(e) = self.verifier.verify_block_final(&header, locked_block.block().header()) {
 			warn!(target: "client", "Stage 5 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
-			return Err(());
+			bail!(e);
 		}
 
 		Ok(locked_block)
@@ -443,7 +446,7 @@ impl Importer {
 	///
 	/// The block is guaranteed to be the next best blocks in the
 	/// first block sequence. Does no sealing or transaction validation.
-	fn import_old_block(&self, unverified: Unverified, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> Result<(), ::error::Error> {
+	fn import_old_block(&self, unverified: Unverified, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> EthcoreResult<()> {
 		let receipts = ::rlp::decode_list(receipts_bytes);
 		let _import_lock = self.import_lock.lock();
 

ethcore/src/verification/queue/mod.rs

@@ -465,19 +465,19 @@ impl<K: Kind> VerificationQueue<K> {
 
 	/// Add a block to the queue.
 	pub fn import(&self, input: K::Input) -> ImportResult {
-		let h = input.hash();
+		let hash = input.hash();
 		{
-			if self.processing.read().contains_key(&h) {
+			if self.processing.read().contains_key(&hash) {
 				bail!(ErrorKind::Import(ImportErrorKind::AlreadyQueued));
 			}
 
 			let mut bad = self.verification.bad.lock();
-			if bad.contains(&h) {
+			if bad.contains(&hash) {
 				bail!(ErrorKind::Import(ImportErrorKind::KnownBad));
 			}
 
 			if bad.contains(&input.parent_hash()) {
-				bad.insert(h.clone());
+				bad.insert(hash);
 				bail!(ErrorKind::Import(ImportErrorKind::KnownBad));
 			}
 		}
@@ -486,21 +486,21 @@ impl<K: Kind> VerificationQueue<K> {
 			Ok(item) => {
 				self.verification.sizes.unverified.fetch_add(item.heap_size_of_children(), AtomicOrdering::SeqCst);
 
-				self.processing.write().insert(h.clone(), item.difficulty());
+				self.processing.write().insert(hash, item.difficulty());
 				{
 					let mut td = self.total_difficulty.write();
 					*td = *td + item.difficulty();
 				}
 				self.verification.unverified.lock().push_back(item);
 				self.more_to_verify.notify_all();
-				Ok(h)
+				Ok(hash)
 			},
 			Err(err) => {
 				match err {
 					// Don't mark future blocks as bad.
 					Error(ErrorKind::Block(BlockError::TemporarilyInvalid(_)), _) => {},
 					_ => {
-						self.verification.bad.lock().insert(h.clone());
+						self.verification.bad.lock().insert(hash);
 					}
 				}
 				Err(err)