[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,9 +1619,9 @@ 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()),
+ Err(error) => Err(error.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
@@ -131,42 +131,46 @@ impl CliqueBlockState {
  }
 
  // see https://github.com/ethereum/go-ethereum/blob/master/consensus/clique/clique.go#L474
+ //
+ // NOTE(niklasad1): might return EthcoreError without `block bytes`
  fn verify(&self, header: &Header) -> Result<Address, Error> {
  let creator = recover_creator(header)?.clone();
 
  // 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)
  }
 
  /// Verify and apply a new header to current state
+ //
+ // NOTE(niklasad1): might return EthcoreError without `block bytes`
  pub fn apply(&mut self, header: &Header, is_checkpoint: bool) -> Result<Address, Error> {
  let creator = self.verify(header)?;
  self.recent_signers.push_front(creator);
@@ -178,9 +182,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 +201,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 +220,7 @@ impl CliqueBlockState {
  signer: Address,
  beneficiary: Address,
  block_number: u64
- ) -> Result<(), Error> {
+ ) {
 
  trace!(target: "engine", "Attempt vote {:?} {:?}", kind, beneficiary);
 
@@ -239,7 +247,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 +271,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 +292,7 @@ impl CliqueBlockState {
  if self.next_timestamp_inturn.is_some() && self.next_timestamp_noturn.is_some() {
  Ok(())
  } else {
- Err(BlockError::TimestampOverflow)?
+ Err(BlockError::TimestampOverflow)
  }
  }