Reduce number of unwraps in chain crate (#2679)

chain/src/chain.rs

@@ -765,11 +765,15 @@ impl Chain {
 	}
 
 	/// Check chain status whether a txhashset downloading is needed
-	pub fn check_txhashset_needed(&self, caller: String, hashes: &mut Option<Vec<Hash>>) -> bool {
+	pub fn check_txhashset_needed(
+		&self,
+		caller: String,
+		hashes: &mut Option<Vec<Hash>>,
+	) -> Result<bool, Error> {
 		let horizon = global::cut_through_horizon() as u64;
-		let body_head = self.head().unwrap();
+		let body_head = self.head()?;
-		let header_head = self.header_head().unwrap();
+		let header_head = self.header_head()?;
-		let sync_head = self.get_sync_head().unwrap();
+		let sync_head = self.get_sync_head()?;
 
 		debug!(
 			"{}: body_head - {}, {}, header_head - {}, {}, sync_head - {}, {}",
@@ -787,7 +791,7 @@ impl Chain {
 				"{}: no need txhashset. header_head.total_difficulty: {} <= body_head.total_difficulty: {}",
 				caller, header_head.total_difficulty, body_head.total_difficulty,
 			);
-			return false;
+			return Ok(false);
 		}
 
 		let mut oldest_height = 0;
@@ -828,13 +832,14 @@ impl Chain {
 					"{}: need a state sync for txhashset. oldest block which is not on local chain: {} at {}",
 					caller, oldest_hash, oldest_height,
 				);
-				return true;
+				Ok(true)
 			} else {
 				error!("{}: something is wrong! oldest_height is 0", caller);
-				return false;
+				Ok(false)
-			};
+			}
+		} else {
+			Ok(false)
 		}
-		return false;
 	}
 
 	/// Writes a reading view on a txhashset state that's been provided to us.
@@ -851,7 +856,7 @@ impl Chain {
 
 		// Initial check whether this txhashset is needed or not
 		let mut hashes: Option<Vec<Hash>> = None;
-		if !self.check_txhashset_needed("txhashset_write".to_owned(), &mut hashes) {
+		if !self.check_txhashset_needed("txhashset_write".to_owned(), &mut hashes)? {
 			warn!("txhashset_write: txhashset received but it's not needed! ignored.");
 			return Err(ErrorKind::InvalidTxHashSet("not needed".to_owned()).into());
 		}
@@ -1230,10 +1235,10 @@ impl Chain {
 	/// Builds an iterator on blocks starting from the current chain head and
 	/// running backward. Specialized to return information pertaining to block
 	/// difficulty calculation (timestamp and previous difficulties).
-	pub fn difficulty_iter(&self) -> store::DifficultyIter<'_> {
+	pub fn difficulty_iter(&self) -> Result<store::DifficultyIter<'_>, Error> {
-		let head = self.head().unwrap();
+		let head = self.head()?;
 		let store = self.store.clone();
-		store::DifficultyIter::from(head.last_block_h, store)
+		Ok(store::DifficultyIter::from(head.last_block_h, store))
 	}
 
 	/// Check whether we have a block without reading it

chain/src/pipe.rs

@@ -183,16 +183,21 @@ pub fn sync_block_headers(
 	headers: &[BlockHeader],
 	ctx: &mut BlockContext<'_>,
 ) -> Result<Option<Tip>, Error> {
-	if let Some(header) = headers.first() {
+	let first_header = match headers.first() {
+		Some(header) => {
 			debug!(
 				"pipe: sync_block_headers: {} headers from {} at {}",
 				headers.len(),
 				header.hash(),
 				header.height,
 			);
-	} else {
+			header
+		}
+		None => {
+			error!("failed to get the first header");
 			return Ok(None);
 		}
+	};
 
 	let all_known = if let Some(last_header) = headers.last() {
 		ctx.batch.get_block_header(&last_header.hash()).is_ok()
@@ -201,7 +206,6 @@ pub fn sync_block_headers(
 	};
 
 	if !all_known {
-		let first_header = headers.first().unwrap();
 		let prev_header = ctx.batch.get_previous_header(&first_header)?;
 		txhashset::sync_extending(&mut ctx.txhashset, &mut ctx.batch, |extension| {
 			extension.rewind(&prev_header)?;

chain/src/store.rs

@@ -272,7 +272,7 @@ impl<'a> Batch<'a> {
 	/// Clear all entries from the output_pos index (must be rebuilt after).
 	pub fn clear_output_pos(&self) -> Result<(), Error> {
 		let key = to_key(COMMIT_POS_PREFIX, &mut "".to_string().into_bytes());
-		for (k, _) in self.db.iter::<u64>(&key).unwrap() {
+		for (k, _) in self.db.iter::<u64>(&key)? {
 			self.db.delete(&k)?;
 		}
 		Ok(())

chain/src/txhashset/txhashset.rs

@@ -66,7 +66,10 @@ impl<T: PMMRable> PMMRHandle<T> {
 	) -> Result<PMMRHandle<T>, Error> {
 		let path = Path::new(root_dir).join(sub_dir).join(file_name);
 		fs::create_dir_all(path.clone())?;
-		let backend = PMMRBackend::new(path.to_str().unwrap().to_string(), prunable, header)?;
+		let path_str = path.to_str().ok_or(Error::from(ErrorKind::Other(
+			"invalid file path".to_owned(),
+		)))?;
+		let backend = PMMRBackend::new(path_str.to_string(), prunable, header)?;
 		let last_pos = backend.unpruned_size();
 		Ok(PMMRHandle { backend, last_pos })
 	}
@@ -1470,7 +1473,7 @@ fn expected_file(path: &Path) -> bool {
 			)
 			.as_str()
 		)
-		.unwrap();
+		.expect("invalid txhashset regular expression");
 	}
 	RE.is_match(&s_path)
 }

chain/tests/data_file_integrity.rs

@@ -81,7 +81,7 @@ fn data_files() {
 
 		for n in 1..4 {
 			let prev = chain.head_header().unwrap();
-			let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+			let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 			let pk = ExtKeychainPath::new(1, n as u32, 0, 0, 0).to_identifier();
 			let reward = libtx::reward::output(&keychain, &pk, 0).unwrap();
 			let mut b =

chain/tests/mine_simple_chain.rs

@@ -101,7 +101,7 @@ where
 
 	for n in 1..4 {
 		let prev = chain.head_header().unwrap();
-		let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+		let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 		let pk = ExtKeychainPath::new(1, n as u32, 0, 0, 0).to_identifier();
 		let reward = libtx::reward::output(keychain, &pk, 0).unwrap();
 		let mut b =
@@ -409,7 +409,7 @@ fn output_header_mappings() {
 
 	for n in 1..15 {
 		let prev = chain.head_header().unwrap();
-		let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+		let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 		let pk = ExtKeychainPath::new(1, n as u32, 0, 0, 0).to_identifier();
 		let reward = libtx::reward::output(&keychain, &pk, 0).unwrap();
 		reward_outputs.push(reward.0.clone());
@@ -545,7 +545,7 @@ fn actual_diff_iter_output() {
 		Arc::new(Mutex::new(StopState::new())),
 	)
 	.unwrap();
-	let iter = chain.difficulty_iter();
+	let iter = chain.difficulty_iter().unwrap();
 	let mut last_time = 0;
 	let mut first = true;
 	for elem in iter.into_iter() {

chain/tests/test_coinbase_maturity.rs

@@ -66,7 +66,7 @@ fn test_coinbase_maturity() {
 	let key_id3 = ExtKeychainPath::new(1, 3, 0, 0, 0).to_identifier();
 	let key_id4 = ExtKeychainPath::new(1, 4, 0, 0, 0).to_identifier();
 
-	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 	let reward = libtx::reward::output(&keychain, &key_id1, 0).unwrap();
 	let mut block = core::core::Block::new(&prev, vec![], Difficulty::min(), reward).unwrap();
 	block.header.timestamp = prev.timestamp + Duration::seconds(60);
@@ -113,7 +113,7 @@ fn test_coinbase_maturity() {
 	let fees = txs.iter().map(|tx| tx.fee()).sum();
 	let reward = libtx::reward::output(&keychain, &key_id3, fees).unwrap();
 	let mut block = core::core::Block::new(&prev, txs, Difficulty::min(), reward).unwrap();
-	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 	block.header.timestamp = prev.timestamp + Duration::seconds(60);
 	block.header.pow.secondary_scaling = next_header_info.secondary_scaling;
 
@@ -147,7 +147,7 @@ fn test_coinbase_maturity() {
 
 		let reward = libtx::reward::output(&keychain, &pk, 0).unwrap();
 		let mut block = core::core::Block::new(&prev, vec![], Difficulty::min(), reward).unwrap();
-		let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+		let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 		block.header.timestamp = prev.timestamp + Duration::seconds(60);
 		block.header.pow.secondary_scaling = next_header_info.secondary_scaling;
 
@@ -172,7 +172,7 @@ fn test_coinbase_maturity() {
 
 	let txs = vec![coinbase_txn];
 	let fees = txs.iter().map(|tx| tx.fee()).sum();
-	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 	let reward = libtx::reward::output(&keychain, &key_id4, fees).unwrap();
 	let mut block = core::core::Block::new(&prev, txs, Difficulty::min(), reward).unwrap();
 

servers/src/grin/server.rs

@@ -413,7 +413,7 @@ impl Server {
 		// for release
 		let diff_stats = {
 			let last_blocks: Vec<consensus::HeaderInfo> =
-				global::difficulty_data_to_vector(self.chain.difficulty_iter())
+				global::difficulty_data_to_vector(self.chain.difficulty_iter()?)
 					.into_iter()
 					.collect();
 

servers/src/grin/sync/body_sync.rs

@@ -69,10 +69,17 @@ impl BodySync {
 	/// Return true if txhashset download is needed (when requested block is under the horizon).
 	fn body_sync(&mut self) -> bool {
 		let mut hashes: Option<Vec<Hash>> = Some(vec![]);
-		if self
+		let txhashset_needed = match self
 			.chain
 			.check_txhashset_needed("body_sync".to_owned(), &mut hashes)
 		{
+			Ok(v) => v,
+			Err(e) => {
+				error!("body_sync: failed to call txhashset_needed: {:?}", e);
+				return false;
+			}
+		};
+		if txhashset_needed {
 			debug!(
 				"body_sync: cannot sync full blocks earlier than horizon. will request txhashset",
 			);

servers/src/grin/sync/syncer.rs

@@ -209,12 +209,20 @@ impl SyncRunner {
 			}
 		} else {
 			// sum the last 5 difficulties to give us the threshold
-			let threshold = self
+			let threshold = {
-				.chain
+				let diff_iter = match self.chain.difficulty_iter() {
-				.difficulty_iter()
+					Ok(v) => v,
+					Err(e) => {
+						error!("failed to get difficulty iterator: {:?}", e);
+						// we handle 0 height in the caller
+						return (false, 0);
+					}
+				};
+				diff_iter
 					.map(|x| x.difficulty)
 					.take(5)
-				.fold(Difficulty::zero(), |sum, val| sum + val);
+					.fold(Difficulty::zero(), |sum, val| sum + val)
+			};
 
 			let peer_diff = peer_info.total_difficulty();
 			if peer_diff > local_diff.clone() + threshold.clone() {

servers/src/mining/mine_block.rs

@@ -105,7 +105,7 @@ fn build_block(
 
 	// Determine the difficulty our block should be at.
 	// Note: do not keep the difficulty_iter in scope (it has an active batch).
-	let difficulty = consensus::next_difficulty(head.height + 1, chain.difficulty_iter());
+	let difficulty = consensus::next_difficulty(head.height + 1, chain.difficulty_iter()?);
 
 	// Extract current "mineable" transactions from the pool.
 	// If this fails for *any* reason then fallback to an empty vec of txs.

wallet/src/test_framework/mod.rs

@@ -83,7 +83,7 @@ fn get_outputs_by_pmmr_index_local(
 /// Adds a block with a given reward to the chain and mines it
 pub fn add_block_with_reward(chain: &Chain, txs: Vec<&Transaction>, reward: CbData) {
 	let prev = chain.head_header().unwrap();
-	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter());
+	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 	let out_bin = util::from_hex(reward.output).unwrap();
 	let kern_bin = util::from_hex(reward.kernel).unwrap();
 	let output = ser::deserialize(&mut &out_bin[..]).unwrap();