Remove unwrap from client module (#2554)

* remove unwraps in client

* imporve block hash expect message

ethcore/src/client/client.rs

@@ -147,13 +147,6 @@ pub struct Client {
 /// assume finality of a given candidate.
 pub const HISTORY: u64 = 1200;
 
-/// Append a path element to the given path and return the string.
-pub fn append_path<P>(path: P, item: &str) -> String where P: AsRef<Path> {
-	let mut p = path.as_ref().to_path_buf();
-	p.push(item);
-	p.to_str().unwrap().to_owned()
-}
-
 impl Client {
 	/// Create a new client with given spec and DB path and custom verifier.
 	pub fn new(
@@ -167,7 +160,7 @@ impl Client {
 		let path = path.to_path_buf();
 		let gb = spec.genesis_block();
 
-		let db = Arc::new(try!(Database::open(&db_config, &path.to_str().unwrap()).map_err(ClientError::Database)));
+		let db = Arc::new(try!(Database::open(&db_config, &path.to_str().expect("DB path could not be converted to string.")).map_err(ClientError::Database)));
 		let chain = Arc::new(BlockChain::new(config.blockchain.clone(), &gb, db.clone()));
 		let tracedb = RwLock::new(TraceDB::new(config.tracing.clone(), db.clone(), chain.clone()));
 
@@ -296,30 +289,27 @@ impl Client {
 
 		// Check if Parent is in chain
 		let chain_has_parent = chain.block_header(header.parent_hash());
-		if let None = chain_has_parent {
+		if let Some(parent) = chain_has_parent {
+			// Enact Verified Block
+			let last_hashes = self.build_last_hashes(header.parent_hash().clone());
+			let db = self.state_db.lock().boxed_clone_canon(&header.parent_hash());
+
+			let enact_result = enact_verified(block, engine, self.tracedb.read().tracing_enabled(), db, &parent, last_hashes, self.factories.clone());
+			let locked_block = try!(enact_result.map_err(|e| {
+				warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
+			}));
+
+			// Final Verification
+			if let Err(e) = self.verifier.verify_block_final(header, locked_block.block().header()) {
+				warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
+				return Err(());
+			}
+
+			Ok(locked_block)
+		} else {
 			warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash());
-			return Err(());
+			Err(())
-		};
-
-		// Enact Verified Block
-		let parent = chain_has_parent.unwrap();
-		let last_hashes = self.build_last_hashes(header.parent_hash().clone());
-		let db = self.state_db.lock().boxed_clone_canon(&header.parent_hash());
-
-		let enact_result = enact_verified(block, engine, self.tracedb.read().tracing_enabled(), db, &parent, last_hashes, self.factories.clone());
-		if let Err(e) = enact_result {
-			warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
-			return Err(());
-		};
-
-		// Final Verification
-		let locked_block = enact_result.unwrap();
-		if let Err(e) = self.verifier.verify_block_final(header, locked_block.block().header()) {
-			warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e);
-			return Err(());
 		}
-
-		Ok(locked_block)
 	}
 
 	fn calculate_enacted_retracted(&self, import_results: &[ImportRoute]) -> (Vec<H256>, Vec<H256>) {
@@ -363,23 +353,17 @@ impl Client {
 
 			for block in blocks {
 				let header = &block.header;
-				if invalid_blocks.contains(header.parent_hash()) {
+				let is_invalid = invalid_blocks.contains(header.parent_hash());
+				if let (false, Ok(closed_block)) = (is_invalid, self.check_and_close_block(&block)) {
+					imported_blocks.push(header.hash());
+
+					let route = self.commit_block(closed_block, &header.hash(), &block.bytes);
+					import_results.push(route);
+
+					self.report.write().accrue_block(&block);
+				} else {
 					invalid_blocks.insert(header.hash());
-					continue;
 				}
-				let closed_block = self.check_and_close_block(&block);
-				if let Err(_) = closed_block {
-					invalid_blocks.insert(header.hash());
-					continue;
-				}
-
-				let closed_block = closed_block.unwrap();
-				imported_blocks.push(header.hash());
-
-				let route = self.commit_block(closed_block, &header.hash(), &block.bytes);
-				import_results.push(route);
-
-				self.report.write().accrue_block(&block);
 			}
 
 			let imported = imported_blocks.len();
@@ -429,7 +413,7 @@ impl Client {
 		// Are we committing an era?
 		let ancient = if number >= HISTORY {
 			let n = number - HISTORY;
-			Some((n, chain.block_hash(n).unwrap()))
+			Some((n, chain.block_hash(n).expect("only verified blocks can be commited; verified block has hash; qed")))
 		} else {
 			None
 		};
@@ -900,8 +884,10 @@ impl BlockChainClient for Client {
 					BodyView::new(&block).localized_transaction_at(&address.block_hash, block_number, address.index)
 				});
 
-			match (t, chain.transaction_receipt(&address)) {
+			let tx_and_sender = t.and_then(|tx| tx.sender().ok().map(|sender| (tx, sender)));
-				(Some(tx), Some(receipt)) => {
+
+			match (tx_and_sender, chain.transaction_receipt(&address)) {
+				(Some((tx, sender)), Some(receipt)) => {
 					let block_hash = tx.block_hash.clone();
 					let block_number = tx.block_number.clone();
 					let transaction_hash = tx.hash();
@@ -923,7 +909,7 @@ impl BlockChainClient for Client {
 						gas_used: receipt.gas_used - prior_gas_used,
 						contract_address: match tx.action {
 							Action::Call(_) => None,
-							Action::Create => Some(contract_address(&tx.sender().unwrap(), &tx.nonce))
+							Action::Create => Some(contract_address(&sender, &tx.nonce))
 						},
 						logs: receipt.logs.into_iter().enumerate().map(|(i, log)| LocalizedLogEntry {
 							entry: log,
@@ -1024,17 +1010,18 @@ impl BlockChainClient for Client {
 		let start = self.block_number(filter.range.start);
 		let end = self.block_number(filter.range.end);
 
-		if start.is_some() && end.is_some() {
+		match (start, end) {
-			let filter = trace::Filter {
+			(Some(s), Some(e)) => {
-				range: start.unwrap() as usize..end.unwrap() as usize,
+				let filter = trace::Filter {
-				from_address: From::from(filter.from_address),
+					range: s as usize..e as usize,
-				to_address: From::from(filter.to_address),
+					from_address: From::from(filter.from_address),
-			};
+					to_address: From::from(filter.to_address),
+				};
 
-			let traces = self.tracedb.read().filter(&filter);
+				let traces = self.tracedb.read().filter(&filter);
-			Some(traces)
+				Some(traces)
-		} else {
+			},
-			None
+			_ => None,
 		}
 	}
 
@@ -1106,11 +1093,15 @@ impl MiningBlockChainClient for Client {
 		// Add uncles
 		chain
 			.find_uncle_headers(&h, engine.maximum_uncle_age())
-			.unwrap()
+			.unwrap_or_else(Vec::new)
 			.into_iter()
 			.take(engine.maximum_uncle_count())
 			.foreach(|h| {
-				open_block.push_uncle(h).unwrap();
+				open_block.push_uncle(h).expect("pushing maximum_uncle_count;
+												open_block was just created;
+												push_uncle is not ok only if more than maximum_uncle_count is pushed;
+												so all push_uncle are Ok;
+												qed");
 			});
 
 		open_block