[ethcore cleanup]: various unrelated fixes from #11493

ethcore/engines/authority-round/src/lib.rs

@@ -874,8 +874,7 @@ impl AuthorityRound {
 			};
 			durations.push(dur_info);
 			for (time, dur) in our_params.step_durations.iter().skip(1) {
-				let (step, time) = next_step_time_duration(dur_info, *time)
-					.ok_or(BlockError::TimestampOverflow)?;
+				let (step, time) = next_step_time_duration(dur_info, *time).ok_or(BlockError::TimestampOverflow)?;
 				dur_info.transition_step = step;
 				dur_info.transition_timestamp = time;
 				dur_info.step_duration = *dur;
@@ -1595,9 +1594,11 @@ impl Engine for AuthorityRound {
 	/// Check the number of seal fields.
 	fn verify_block_basic(&self, header: &Header) -> Result<(), Error> {
 		if header.number() >= self.validate_score_transition && *header.difficulty() >= U256::from(U128::max_value()) {
-			return Err(From::from(BlockError::DifficultyOutOfBounds(
-				OutOfBounds { min: None, max: Some(U256::from(U128::max_value())), found: *header.difficulty() }
-			)));
+			return Err(Error::Block(BlockError::DifficultyOutOfBounds(OutOfBounds {
+				min: None,
+				max: Some(U256::from(U128::max_value())),
+				found: *header.difficulty()
+			})));
 		}
 
 		match verify_timestamp(&self.step.inner, header_step(header, self.empty_steps_transition)?) {
@@ -1618,7 +1619,7 @@ impl Engine for AuthorityRound {
 					self.validators.report_benign(header.author(), set_number, header.number());
 				}
 
-				Err(BlockError::InvalidSeal.into())
+				Err(Error::Block(BlockError::InvalidSeal))
 			}
 			Err(e) => Err(e.into()),
 			Ok(()) => Ok(()),
@@ -1724,9 +1725,9 @@ impl Engine for AuthorityRound {
 		if header.number() >= self.validate_score_transition {
 			let expected_difficulty = calculate_score(parent_step.into(), step.into(), empty_steps_len.into());
 			if header.difficulty() != &expected_difficulty {
-				return Err(From::from(BlockError::InvalidDifficulty(Mismatch {
+				return Err(Error::Block(BlockError::InvalidDifficulty(Mismatch {
 					expected: expected_difficulty,
-					found: header.difficulty().clone()
+					found: *header.difficulty()
 				})));
 			}
 		}

ethcore/engines/basic-authority/src/lib.rs

@@ -75,7 +75,7 @@ fn verify_external(header: &Header, validators: &dyn ValidatorSet) -> Result<(),
 	}
 
 	match validators.contains(header.parent_hash(), &signer) {
-		false => Err(BlockError::InvalidSeal.into()),
+		false => Err(Error::Block(BlockError::InvalidSeal)),
 		true => Ok(())
 	}
 }
@@ -91,7 +91,7 @@ impl BasicAuthority {
 	/// Create a new instance of BasicAuthority engine
 	pub fn new(our_params: BasicAuthorityParams, machine: Machine) -> Self {
 		BasicAuthority {
-			machine: machine,
+			machine,
 			signer: RwLock::new(None),
 			validators: new_validator_set(our_params.validators),
 		}

ethcore/engines/clique/src/block_state.rs

@@ -33,7 +33,7 @@ use unexpected::Mismatch;
 
 use crate::{
 	util::{extract_signers, recover_creator},
-	{VoteType, DIFF_INTURN, DIFF_NOTURN, NULL_AUTHOR, SIGNING_DELAY_NOTURN_MS},
+	VoteType, DIFF_INTURN, DIFF_NOTURN, NULL_AUTHOR, SIGNING_DELAY_NOTURN_MS,
 };
 
 /// Type that keeps track of the state for a given vote
@@ -137,30 +137,30 @@ impl CliqueBlockState {
 		// The signer is not authorized
 		if !self.signers.contains(&creator) {
 			trace!(target: "engine", "current state: {}", self);
-			Err(EngineError::NotAuthorized(creator))?
+			return Err(EngineError::NotAuthorized(creator).into());
 		}
 
 		// The signer has signed a block too recently
 		if self.recent_signers.contains(&creator) {
 			trace!(target: "engine", "current state: {}", self);
-			Err(EngineError::CliqueTooRecentlySigned(creator))?
+			return Err(EngineError::CliqueTooRecentlySigned(creator).into());
 		}
 
 		// Wrong difficulty
 		let inturn = self.is_inturn(header.number(), &creator);
 
 		if inturn && *header.difficulty() != DIFF_INTURN {
-			Err(BlockError::InvalidDifficulty(Mismatch {
+			return Err(Error::Block(BlockError::InvalidDifficulty(Mismatch {
 				expected: DIFF_INTURN,
 				found: *header.difficulty(),
-			}))?
+			})));
 		}
 
 		if !inturn && *header.difficulty() != DIFF_NOTURN {
-			Err(BlockError::InvalidDifficulty(Mismatch {
+			return Err(Error::Block(BlockError::InvalidDifficulty(Mismatch {
 				expected: DIFF_NOTURN,
 				found: *header.difficulty(),
-			}))?
+			})));
 		}
 
 		Ok(creator)
@@ -178,9 +178,10 @@ impl CliqueBlockState {
 			if self.signers != signers {
 				let invalid_signers: Vec<String> = signers.into_iter()
 					.filter(|s| !self.signers.contains(s))
-					.map(|s| format!("{}", s))
+					.map(|s| s.to_string())
 					.collect();
-				Err(EngineError::CliqueFaultyRecoveredSigners(invalid_signers))?
+
+				return Err(EngineError::CliqueFaultyRecoveredSigners(invalid_signers).into());
 			};
 
 			// TODO(niklasad1): I'm not sure if we should shrink here because it is likely that next epoch
@@ -196,11 +197,14 @@ impl CliqueBlockState {
 		if *header.author() != NULL_AUTHOR {
 			let decoded_seal = header.decode_seal::<Vec<_>>()?;
 			if decoded_seal.len() != 2 {
-				Err(BlockError::InvalidSealArity(Mismatch { expected: 2, found: decoded_seal.len() }))?
+				return Err(Error::Block(BlockError::InvalidSealArity(Mismatch {
+					expected: 2,
+					found: decoded_seal.len()
+				})));
 			}
 
 			let nonce = H64::from_slice(decoded_seal[1]);
-			self.update_signers_on_vote(VoteType::from_nonce(nonce)?, creator, *header.author(), header.number())?;
+			self.update_signers_on_vote(VoteType::from_nonce(nonce)?, creator, *header.author(), header.number());
 		}
 
 		Ok(creator)
@@ -212,7 +216,7 @@ impl CliqueBlockState {
 		signer: Address,
 		beneficiary: Address,
 		block_number: u64
-	) -> Result<(), Error> {
+	) {
 
 		trace!(target: "engine", "Attempt vote {:?} {:?}", kind, beneficiary);
 
@@ -239,7 +243,7 @@ impl CliqueBlockState {
 		// If no vote was found for the beneficiary return `early` but don't propogate an error
 		let (votes, vote_kind) = match self.get_current_votes_and_kind(beneficiary) {
 			Some((v, k)) => (v, k),
-			None => return Ok(()),
+			None => return,
 		};
 		let threshold = self.signers.len() / 2;
 
@@ -263,16 +267,14 @@ impl CliqueBlockState {
 			self.rotate_recent_signers();
 			self.remove_all_votes_from(beneficiary);
 		}
-
-		Ok(())
 	}
 
 	/// Calculate the next timestamp for `inturn` and `noturn` fails if any of them can't be represented as
 	/// `SystemTime`
 	// TODO(niklasad1): refactor this method to be in constructor of `CliqueBlockState` instead.
 	// This is a quite bad API because we must mutate both variables even when already `inturn` fails
 	// That's why we can't return early and must have the `if-else` in the end
-	pub fn calc_next_timestamp(&mut self, timestamp: u64, period: u64) -> Result<(), Error> {
+	pub fn calc_next_timestamp(&mut self, timestamp: u64, period: u64) -> Result<(), BlockError> {
 		let inturn = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(timestamp.saturating_add(period)));
 
 		self.next_timestamp_inturn = inturn;
@@ -286,7 +288,7 @@ impl CliqueBlockState {
 		if self.next_timestamp_inturn.is_some() && self.next_timestamp_noturn.is_some() {
 			Ok(())
 		} else {
-			Err(BlockError::TimestampOverflow)?
+			Err(BlockError::TimestampOverflow)
 		}
 	}