[ethcore cleanup]: various unrelated fixes from #11493 (#11507)

* [cleanup]: various unrelated fixes from `#11493`

* [revert]: too verbose logging `seal`

* fix nit: don't mix from() and into()

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,8 +267,6 @@ 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
@@ -272,7 +274,7 @@ impl CliqueBlockState {
 	// 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)
 		}
 	}
 

ethcore/engines/clique/src/lib.rs

@@ -104,7 +104,6 @@ use crate::{
 	params::CliqueParams,
 };
 
-
 mod params;
 mod block_state;
 mod util;
@@ -157,7 +156,7 @@ impl VoteType {
 		} else if nonce == NONCE_DROP_VOTE {
 			Ok(VoteType::Remove)
 		} else {
-			Err(EngineError::CliqueInvalidNonce(nonce))?
+			Err(EngineError::CliqueInvalidNonce(nonce).into())
 		}
 	}
 
@@ -250,17 +249,14 @@ impl Clique {
 		}
 	}
 
-	fn sign_header(&self, header: &Header) -> Result<(Signature, H256), Error> {
-
+	fn sign_header(&self, header: &Header) -> Result<(Signature, H256), EngineError> {
 		match self.signer.read().as_ref() {
-			None => {
-				Err(EngineError::RequiresSigner)?
-			}
+			None => Err(EngineError::RequiresSigner),
 			Some(signer) => {
 				let digest = header.hash();
 				match signer.sign(digest) {
 					Ok(sig) => Ok((sig, digest)),
-					Err(e) => Err(EngineError::Custom(e.into()))?,
+					Err(e) => Err(EngineError::Custom(e.to_string())),
 				}
 			}
 		}
@@ -296,9 +292,7 @@ impl Clique {
 		}
 		// BlockState is not found in memory, which means we need to reconstruct state from last checkpoint.
 		match self.client.read().as_ref().and_then(|w| w.upgrade()) {
-			None => {
-				return Err(EngineError::RequiresClient)?;
-			}
+			None => return Err(EngineError::RequiresClient.into()),
 			Some(c) => {
 				let last_checkpoint_number = header.number() - header.number() % self.epoch_length as u64;
 				debug_assert_ne!(last_checkpoint_number, header.number());
@@ -327,7 +321,7 @@ impl Clique {
 					}
 					match c.block_header(BlockId::Hash(last_parent_hash)) {
 						None => {
-							return Err(BlockError::UnknownParent(last_parent_hash))?;
+							return Err(Error::Block(BlockError::UnknownParent(last_parent_hash)));
 						}
 						Some(next) => {
 							chain.push_front(next.decode()?);
@@ -341,7 +335,7 @@ impl Clique {
 					.parent_hash();
 
 				let last_checkpoint_header = match c.block_header(BlockId::Hash(last_checkpoint_hash)) {
-					None => return Err(EngineError::CliqueMissingCheckpoint(last_checkpoint_hash))?,
+					None => return Err(EngineError::CliqueMissingCheckpoint(last_checkpoint_hash).into()),
 					Some(header) => header.decode()?,
 				};
 
@@ -353,15 +347,14 @@ impl Clique {
 				block_state_by_hash.insert(last_checkpoint_header.hash(), last_checkpoint_state.clone());
 
 				// Backfill!
-				let mut new_state = last_checkpoint_state.clone();
+				let mut new_state = last_checkpoint_state;
 				for item in &chain {
 					new_state.apply(item, false)?;
 				}
 				new_state.calc_next_timestamp(header.timestamp(), self.period)?;
 				block_state_by_hash.insert(header.hash(), new_state.clone());
 
-				let elapsed = backfill_start.elapsed();
-				trace!(target: "engine", "Back-filling succeed, took {} ms.", elapsed.as_millis());
+				trace!(target: "engine", "Back-filling succeed, took {} ms.", backfill_start.elapsed().as_millis());
 				Ok(new_state)
 			}
 		}
@@ -409,9 +402,8 @@ impl Engine for Clique {
 		trace!(target: "engine", "on_seal_block");
 
 		let header = &mut block.header;
-
-		let state = self.state_no_backfill(header.parent_hash())
-			.ok_or_else(|| BlockError::UnknownParent(*header.parent_hash()))?;
+		let mut state = self.state_no_backfill(header.parent_hash())
+			.ok_or_else(|| Error::Block(BlockError::UnknownParent(*header.parent_hash())))?;
 
 		let is_checkpoint = header.number() % self.epoch_length == 0;
 
@@ -438,16 +430,15 @@ impl Engine for Clique {
 		}
 
 		// Work on clique seal.
-
 		let mut seal: Vec<u8> = Vec::with_capacity(VANITY_LENGTH + SIGNATURE_LENGTH);
 
 		// At this point, extra_data should only contain miner vanity.
 		if header.extra_data().len() != VANITY_LENGTH {
-			Err(BlockError::ExtraDataOutOfBounds(OutOfBounds {
+			return Err(Error::Block(BlockError::ExtraDataOutOfBounds(OutOfBounds {
 				min: Some(VANITY_LENGTH),
 				max: Some(VANITY_LENGTH),
 				found: header.extra_data().len()
-			}))?;
+			})));
 		}
 		// vanity
 		{
@@ -467,15 +458,14 @@ impl Engine for Clique {
 		// append signature onto extra_data
 		let (sig, _msg) = self.sign_header(&header)?;
 		seal.extend_from_slice(&sig[..]);
-		header.set_extra_data(seal.clone());
+		header.set_extra_data(seal);
 
 		header.compute_hash();
 
 		// locally sealed block don't go through valid_block_family(), so we have to record state here.
-		let mut new_state = state.clone();
-		new_state.apply(&header, is_checkpoint)?;
-		new_state.calc_next_timestamp(header.timestamp(), self.period)?;
-		self.block_state_by_hash.write().insert(header.hash(), new_state);
+		state.apply(&header, is_checkpoint)?;
+		state.calc_next_timestamp(header.timestamp(), self.period)?;
+		self.block_state_by_hash.write().insert(header.hash(), state);
 
 		trace!(target: "engine", "on_seal_block: finished, final header: {:?}", header);
 
@@ -574,18 +564,18 @@ impl Engine for Clique {
 
 			// This should succeed under the constraints that the system clock works
 			let limit_as_dur = limit.duration_since(UNIX_EPOCH).map_err(|e| {
-				Box::new(format!("Converting SystemTime to Duration failed: {}", e))
+				format!("Converting SystemTime to Duration failed: {}", e)
 			})?;
 
 			let hdr = Duration::from_secs(header.timestamp());
 			if hdr > limit_as_dur {
 				let found = CheckedSystemTime::checked_add(UNIX_EPOCH, hdr).ok_or(BlockError::TimestampOverflow)?;
 
-				Err(BlockError::TemporarilyInvalid(OutOfBounds {
+				return Err(Error::Block(BlockError::TemporarilyInvalid(From::from(OutOfBounds {
 					min: None,
 					max: Some(limit),
 					found,
-				}.into()))?
+				}))));
 			}
 		}
 
@@ -600,10 +590,10 @@ impl Engine for Clique {
 
 		let seal_fields = header.decode_seal::<Vec<_>>()?;
 		if seal_fields.len() != 2 {
-			Err(BlockError::InvalidSealArity(Mismatch {
+			return Err(Error::Block(BlockError::InvalidSealArity(Mismatch {
 				expected: 2,
 				found: seal_fields.len(),
-			}))?
+			})));
 		}
 
 		let mixhash = H256::from_slice(seal_fields[0]);
@@ -611,58 +601,58 @@ impl Engine for Clique {
 
 		// Nonce must be 0x00..0 or 0xff..f
 		if nonce != NONCE_DROP_VOTE && nonce != NONCE_AUTH_VOTE {
-			Err(EngineError::CliqueInvalidNonce(nonce))?;
+			return Err(EngineError::CliqueInvalidNonce(nonce).into());
 		}
 
 		if is_checkpoint && nonce != NULL_NONCE {
-			Err(EngineError::CliqueInvalidNonce(nonce))?;
+			return Err(EngineError::CliqueInvalidNonce(nonce).into());
 		}
 
 		// Ensure that the mix digest is zero as Clique don't have fork protection currently
 		if mixhash != NULL_MIXHASH {
-			Err(BlockError::MismatchedH256SealElement(Mismatch {
+			return Err(Error::Block(BlockError::MismatchedH256SealElement(Mismatch {
 				expected: NULL_MIXHASH,
 				found: mixhash,
-			}))?
+			})));
 		}
 
 		let extra_data_len = header.extra_data().len();
 
 		if extra_data_len < VANITY_LENGTH {
-			Err(EngineError::CliqueMissingVanity)?
+			return Err(EngineError::CliqueMissingVanity.into());
 		}
 
 		if extra_data_len < VANITY_LENGTH + SIGNATURE_LENGTH {
-			Err(EngineError::CliqueMissingSignature)?
+			return Err(EngineError::CliqueMissingSignature.into());
 		}
 
 		let signers = extra_data_len - (VANITY_LENGTH + SIGNATURE_LENGTH);
 
 		// Checkpoint blocks must at least contain one signer
 		if is_checkpoint && signers == 0 {
-			Err(EngineError::CliqueCheckpointNoSigner)?
+			return Err(EngineError::CliqueCheckpointNoSigner.into());
 		}
 
 		// Addresses must be be divisable by 20
 		if is_checkpoint && signers % ADDRESS_LENGTH != 0 {
-			Err(EngineError::CliqueCheckpointInvalidSigners(signers))?
+			return Err(EngineError::CliqueCheckpointInvalidSigners(signers).into());
 		}
 
 		// Ensure that the block doesn't contain any uncles which are meaningless in PoA
 		if *header.uncles_hash() != NULL_UNCLES_HASH {
-			Err(BlockError::InvalidUnclesHash(Mismatch {
+			return Err(Error::Block(BlockError::InvalidUnclesHash(Mismatch {
 				expected: NULL_UNCLES_HASH,
 				found: *header.uncles_hash(),
-			}))?
+			})));
 		}
 
 		// Ensure that the block's difficulty is meaningful (may not be correct at this point)
 		if *header.difficulty() != DIFF_INTURN && *header.difficulty() != DIFF_NOTURN {
-			Err(BlockError::DifficultyOutOfBounds(OutOfBounds {
+			return Err(Error::Block(BlockError::DifficultyOutOfBounds(OutOfBounds {
 				min: Some(DIFF_NOTURN),
 				max: Some(DIFF_INTURN),
 				found: *header.difficulty(),
-			}))?
+			})));
 		}
 
 		// All basic checks passed, continue to next phase
@@ -684,7 +674,7 @@ impl Engine for Clique {
 
 		// parent sanity check
 		if parent.hash() != *header.parent_hash() || header.number() != parent.number() + 1 {
-			Err(BlockError::UnknownParent(parent.hash()))?
+			return Err(Error::Block(BlockError::UnknownParent(parent.hash())));
 		}
 
 		// Ensure that the block's timestamp isn't too close to it's parent
@@ -694,17 +684,16 @@ impl Engine for Clique {
 			let found = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(limit))
 				.ok_or(BlockError::TimestampOverflow)?;
 
-			Err(BlockError::InvalidTimestamp(OutOfBounds {
+			return Err(Error::Block(BlockError::InvalidTimestamp(From::from(OutOfBounds {
 				min: None,
 				max,
 				found,
-			}.into()))?
+			}))));
 		}
 
 		// Retrieve the parent state
-		let parent_state = self.state(&parent)?;
 		// Try to apply current state, apply() will further check signer and recent signer.
-		let mut new_state = parent_state.clone();
+		let mut new_state = self.state(&parent)?;
 		new_state.apply(header, header.number() % self.epoch_length == 0)?;
 		new_state.calc_next_timestamp(header.timestamp(), self.period)?;
 		self.block_state_by_hash.write().insert(header.hash(), new_state);
@@ -714,7 +703,7 @@ impl Engine for Clique {
 
 	fn genesis_epoch_data(&self, header: &Header, _call: &Call) -> Result<Vec<u8>, String> {
 		let mut state = self.new_checkpoint_state(header).expect("Unable to parse genesis data.");
-		state.calc_next_timestamp(header.timestamp(), self.period).map_err(|e| format!("{}", e))?;
+		state.calc_next_timestamp(header.timestamp(), self.period).map_err(|e| e.to_string())?;
 		self.block_state_by_hash.write().insert(header.hash(), state);
 
 		// no proof.

ethcore/engines/ethash/src/lib.rs

@@ -179,7 +179,7 @@ fn verify_block_unordered(pow: &Arc<EthashManager>, header: &Header) -> Result<(
 	let seal = EthashSeal::parse_seal(header.seal())?;
 
 	let result = pow.compute_light(
-		header.number() as u64,
+		header.number(),
 		&header.bare_hash().0,
 		seal.nonce.to_low_u64_be()
 	);
@@ -193,10 +193,17 @@ fn verify_block_unordered(pow: &Arc<EthashManager>, header: &Header) -> Result<(
 	       mix = H256(result.mix_hash),
 	       res = H256(result.value));
 	if mix != seal.mix_hash {
-		return Err(From::from(BlockError::MismatchedH256SealElement(Mismatch { expected: mix, found: seal.mix_hash })));
+		return Err(From::from(BlockError::MismatchedH256SealElement(Mismatch {
+			expected: mix,
+			found: seal.mix_hash
+		})));
 	}
 	if &difficulty < header.difficulty() {
-		return Err(From::from(BlockError::InvalidProofOfWork(OutOfBounds { min: Some(header.difficulty().clone()), max: None, found: difficulty })));
+		return Err(From::from(BlockError::InvalidProofOfWork(OutOfBounds {
+			min: Some(*header.difficulty()),
+			max: None,
+			found: difficulty
+		})));
 	}
 	Ok(())
 }
@@ -324,7 +331,11 @@ impl Engine for Ethash {
 		// TODO: consider removing these lines.
 		let min_difficulty = self.ethash_params.minimum_difficulty;
 		if header.difficulty() < &min_difficulty {
-			return Err(From::from(BlockError::DifficultyOutOfBounds(OutOfBounds { min: Some(min_difficulty), max: None, found: header.difficulty().clone() })))
+			return Err(From::from(BlockError::DifficultyOutOfBounds(OutOfBounds {
+				min: Some(min_difficulty),
+				max: None,
+				found: *header.difficulty(),
+			})))
 		}
 
 		let difficulty = ethash::boundary_to_difficulty(&H256(quick_get_difficulty(
@@ -335,7 +346,11 @@ impl Engine for Ethash {
 		)));
 
 		if &difficulty < header.difficulty() {
-			return Err(From::from(BlockError::InvalidProofOfWork(OutOfBounds { min: Some(header.difficulty().clone()), max: None, found: difficulty })));
+			return Err(From::from(BlockError::InvalidProofOfWork(OutOfBounds {
+				min: Some(*header.difficulty()),
+				max: None,
+				found: difficulty
+			})));
 		}
 
 		Ok(())
@@ -348,13 +363,20 @@ impl Engine for Ethash {
 	fn verify_block_family(&self, header: &Header, parent: &Header) -> Result<(), Error> {
 		// we should not calculate difficulty for genesis blocks
 		if header.number() == 0 {
-			return Err(From::from(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, found: header.number() })));
+			return Err(From::from(BlockError::RidiculousNumber(OutOfBounds {
+				min: Some(1),
+				max: None,
+				found: header.number()
+			})));
 		}
 
 		// Check difficulty is correct given the two timestamps.
 		let expected_difficulty = self.calculate_difficulty(header, parent);
 		if header.difficulty() != &expected_difficulty {
-			return Err(From::from(BlockError::InvalidDifficulty(Mismatch { expected: expected_difficulty, found: header.difficulty().clone() })))
+			return Err(From::from(BlockError::InvalidDifficulty(Mismatch {
+				expected: expected_difficulty,
+				found: *header.difficulty()
+			})))
 		}
 
 		Ok(())

ethcore/engines/validator-set/src/safe_contract.rs

@@ -503,9 +503,10 @@ impl ValidatorSet for ValidatorSafeContract {
 				receipts.iter().map(::rlp::encode)
 			);
 			if found_root != *old_header.receipts_root() {
-				return Err(BlockError::InvalidReceiptsRoot(
-					Mismatch { expected: *old_header.receipts_root(), found: found_root }
-				).into());
+				return Err(EthcoreError::Block(BlockError::InvalidReceiptsRoot(Mismatch {
+					expected: *old_header.receipts_root(),
+					found: found_root
+				})));
 			}
 
 			let bloom = self.expected_bloom(&old_header);

ethcore/src/block.rs

@@ -102,7 +102,7 @@ pub trait Drain {
 
 impl<'x> OpenBlock<'x> {
 	/// Create a new `OpenBlock` ready for transaction pushing.
-	pub fn new<'a>(
+	pub fn new(
 		engine: &'x dyn Engine,
 		factories: Factories,
 		tracing: bool,
@@ -204,12 +204,11 @@ impl<'x> OpenBlock<'x> {
 			let hash = t.hash();
 			let start = time::Instant::now();
 			self.push_transaction(t)?;
-			let took = start.elapsed();
-			let took_ms = took.as_secs() * 1000 + took.subsec_nanos() as u64 / 1000000;
-			if took > time::Duration::from_millis(slow_tx) {
-				warn!("Heavy ({} ms) transaction in block {:?}: {:?}", took_ms, self.block.header.number(), hash);
+			let elapsed_millis = start.elapsed().as_millis();
+			if elapsed_millis > slow_tx {
+				warn!("Heavy ({} ms) transaction in block {:?}: {:?}", elapsed_millis, self.block.header.number(), hash);
 			}
-			debug!(target: "tx", "Transaction {:?} took: {} ms", hash, took_ms);
+			debug!(target: "tx", "Transaction {:?} took: {} ms", hash, elapsed_millis);
 		}
 
 		Ok(())
@@ -347,10 +346,10 @@ impl LockedBlock {
 		let expected_seal_fields = engine.seal_fields(&self.header);
 		let mut s = self;
 		if seal.len() != expected_seal_fields {
-			Err(BlockError::InvalidSealArity(Mismatch {
+			return Err(Error::Block(BlockError::InvalidSealArity(Mismatch {
 				expected: expected_seal_fields,
 				found: seal.len()
-			}))?;
+			})));
 		}
 
 		s.block.header.set_seal(seal);
@@ -504,7 +503,6 @@ mod tests {
 		verification::Unverified,
 	};
 	use hash_db::EMPTY_PREFIX;
-	use spec;
 
 	/// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header
 	fn enact_bytes(

ethcore/src/client/client.rs

@@ -2388,7 +2388,7 @@ impl ImportSealedBlock for Client {
 			// Do a super duper basic verification to detect potential bugs
 			if let Err(e) = self.engine.verify_block_basic(&header) {
 				self.importer.bad_blocks.report(
-					block.rlp_bytes(),
+					raw,
 					format!("Detected an issue with locally sealed block: {}", e),
 				);
 				return Err(e);

ethcore/src/miner/miner.rs

@@ -716,7 +716,6 @@ impl Miner {
 			},
 		};
 
-		let sealing_result =
 		match self.engine.generate_seal(&block, &parent_header) {
 			// Directly import a regular sealed block.
 			Seal::Regular(seal) => {
@@ -744,8 +743,7 @@ impl Miner {
 					})
 			},
 			Seal::None => false,
-			};
-		sealing_result
+		}
 	}
 
 	/// Prepares work which has to be done to seal.
@@ -1276,7 +1274,6 @@ impl miner::MinerService for Miner {
 				if self.seal_and_import_block_internally(chain, block) {
 					trace!(target: "miner", "update_sealing: imported internally sealed block");
 				}
-				return
 			},
 			SealingState::NotReady => unreachable!("We returned right after sealing_state was computed. qed."),
 			SealingState::External => {

ethcore/sync/src/block_sync.rs

@@ -577,7 +577,7 @@ impl BlockDownloader {
 				},
 				Ok(_) => {
 					trace_sync!(self, "Block queued {:?}", h);
-					imported.insert(h.clone());
+					imported.insert(h);
 					self.block_imported(&h, number, &parent);
 				},
 				Err(EthcoreError::Block(BlockError::UnknownParent(_))) if allow_out_of_order => {

ethcore/types/src/engines/mod.rs

@@ -18,11 +18,13 @@
 
 use ethereum_types::{Address, H256, H64};
 use bytes::Bytes;
-use ethjson;
 use rlp::Rlp;
 use unexpected::Mismatch;
 
-use crate::{BlockNumber, errors::{BlockError, EthcoreError}};
+use crate::{
+	BlockNumber,
+	errors::{BlockError, EthcoreError}
+};
 
 pub mod epoch;
 pub mod params;
@@ -56,21 +58,18 @@ impl EthashSeal {
 	/// Tries to parse rlp encoded bytes as an Ethash/Clique seal.
 	pub fn parse_seal<T: AsRef<[u8]>>(seal: &[T]) -> Result<Self, EthcoreError> {
 		if seal.len() != 2 {
-			return Err(BlockError::InvalidSealArity(
-				Mismatch {
+			Err(EthcoreError::Block(BlockError::InvalidSealArity(Mismatch {
 				expected: 2,
 				found: seal.len()
-				}
-			).into());
-		}
-
+			})))
+		} else {
 			let mix_hash = Rlp::new(seal[0].as_ref()).as_val::<H256>()?;
 			let nonce = Rlp::new(seal[1].as_ref()).as_val::<H64>()?;
 			Ok(EthashSeal { mix_hash, nonce })
 		}
+	}
 }
 
-
 /// Seal type.
 #[derive(Debug, PartialEq, Eq)]
 pub enum Seal {
@@ -96,7 +95,7 @@ pub const MAX_UNCLE_AGE: u64 = 6;
 
 /// Default EIP-210 contract code.
 /// As defined in https://github.com/ethereum/EIPs/pull/210
-pub const DEFAULT_BLOCKHASH_CONTRACT: &'static [u8] = &[
+pub const DEFAULT_BLOCKHASH_CONTRACT: &[u8] = &[
 	0x73, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
 	0xff, 0xff, 0xff, 0xff, 0xfe, 0x33, 0x14, 0x15, 0x61, 0x00, 0x6a, 0x57, 0x60, 0x01, 0x43, 0x03,
 	0x60, 0x00, 0x35, 0x61, 0x01, 0x00, 0x82, 0x07, 0x55, 0x61, 0x01, 0x00, 0x81, 0x07, 0x15, 0x15,

ethcore/verification/src/verification.rs

@@ -163,8 +163,8 @@ fn verify_uncles(block: &PreverifiedBlock, bc: &dyn BlockProvider, engine: &dyn
 
 		let mut excluded = HashSet::new();
 		excluded.insert(header.hash());
-		let mut hash = header.parent_hash().clone();
-		excluded.insert(hash.clone());
+		let mut hash = *header.parent_hash();
+		excluded.insert(hash);
 		for _ in 0..MAX_UNCLE_AGE {
 			match bc.block_details(&hash) {
 				Some(details) => {
@@ -213,7 +213,7 @@ fn verify_uncles(block: &PreverifiedBlock, bc: &dyn BlockProvider, engine: &dyn
 			// cB.p^6	-----------/  6
 			// cB.p^7	-------------/
 			// cB.p^8
-			let mut expected_uncle_parent = header.parent_hash().clone();
+			let mut expected_uncle_parent = *header.parent_hash();
 			let uncle_parent = bc.block_header_data(&uncle.parent_hash())
 				.ok_or_else(|| BlockError::UnknownUncleParent(*uncle.parent_hash()))?;
 			for _ in 0..depth {
@@ -440,7 +440,6 @@ mod tests {
 	use keccak_hash::keccak;
 	use engine::Engine;
 	use parity_crypto::publickey::{Random, Generator};
-	use spec;
 	use ethcore::test_helpers::{
 		create_test_block_with_data, create_test_block, TestBlockChainClient
 	};
@@ -449,7 +448,6 @@ mod tests {
 		errors::BlockError::*,
 		transaction::{SignedTransaction, Transaction, UnverifiedTransaction, Action},
 	};
-	use rlp;
 	use triehash::ordered_trie_root;
 	use machine::Machine;
 	use null_engine::NullEngine;