Fix clippy suggestions. (#179)

* Fix clippy errors.

* Cargo fmt.

* Enable clippy checks.

* Create if does not exist.

* Fix warnings and enable sccache for clippy.

* chmod +x

* Revert and ignore errors.

* Update cancel-workflow-action.

* Fixes.

* Clippy fixes.

* Fix compilation.

* Fix new clippy warnings.

* fmt --all

* Fix the rest.

* fmt --all

* Conditional.

* Bump smallvec.

* Use separate cache dir for clippy to prevent races.

* Remove unused imports in tests

* Remove "useless conversion"

* Move clippy to main worfklow to avoid clashes.

* Fix clippy error.

* Fix remaning clippy errors.

* cargo fmt --all

Co-authored-by: Hernando Castano <castano.ha@gmail.com>

bridges/modules/ethereum/src/finality.rs

@@ -99,7 +99,7 @@ pub fn finalize_blocks<S: Storage>(
 					*hash == header_validators.0.hash || *hash == best_finalized.hash
 				})
 			})
-			.unwrap_or_else(|| CachedFinalityVotes::default()),
+			.unwrap_or_default(),
 		best_finalized,
 		&validators,
 		id,
@@ -247,7 +247,7 @@ fn empty_steps_signers(header: &Header) -> BTreeSet<Address> {
 	header
 		.empty_steps()
 		.into_iter()
-		.flat_map(|steps| steps)
+		.flatten()
 		.filter_map(|step| empty_step_signer(&step, &header.parent_hash))
 		.collect::<BTreeSet<_>>()
 }
@@ -462,13 +462,9 @@ mod tests {
 			// when we're inserting header#7 and last finalized header is 0:
 			// check that votes at #7 are computed correctly without cache
 			let expected_votes_at_7 = FinalityVotes {
-				votes: vec![
-					(ctx.addresses[0].clone(), 3),
-					(ctx.addresses[1].clone(), 3),
-					(ctx.addresses[2].clone(), 1),
-				]
-				.into_iter()
-				.collect(),
+				votes: vec![(ctx.addresses[0], 3), (ctx.addresses[1], 3), (ctx.addresses[2], 1)]
+					.into_iter()
+					.collect(),
 				ancestry: ancestry[..7].iter().cloned().collect(),
 			};
 			let id7 = headers[6].compute_id();
@@ -491,9 +487,7 @@ mod tests {
 
 			// cached votes at #5
 			let expected_votes_at_5 = FinalityVotes {
-				votes: vec![(ctx.addresses[0].clone(), 3), (ctx.addresses[1].clone(), 2)]
-					.into_iter()
-					.collect(),
+				votes: vec![(ctx.addresses[0], 3), (ctx.addresses[1], 2)].into_iter().collect(),
 				ancestry: ancestry[..5].iter().cloned().collect(),
 			};
 			FinalityCache::<TestRuntime>::insert(hashes[4], expected_votes_at_5);
@@ -520,9 +514,7 @@ mod tests {
 			// when we're inserting header#7 and last finalized header is 3:
 			// check that votes at #7 are computed correctly with cache
 			let expected_votes_at_7 = FinalityVotes {
-				votes: vec![(ctx.addresses[1].clone(), 3), (ctx.addresses[2].clone(), 1)]
-					.into_iter()
-					.collect(),
+				votes: vec![(ctx.addresses[1], 3), (ctx.addresses[2], 1)].into_iter().collect(),
 				ancestry: ancestry[3..7].iter().cloned().collect(),
 			};
 			assert_eq!(

bridges/modules/ethereum/src/import.rs

@@ -70,6 +70,9 @@ pub fn import_headers<S: Storage, PS: PruningStrategy>(
 	Ok((useful, useless))
 }
 
+/// A vector of finalized headers and their submitters.
+pub type FinalizedHeaders<S> = Vec<(HeaderId, Option<<S as Storage>::Submitter>)>;
+
 /// Imports given header and updates blocks finality (if required).
 ///
 /// Transactions receipts must be provided if `header_import_requires_receipts()`
@@ -84,7 +87,7 @@ pub fn import_header<S: Storage, PS: PruningStrategy>(
 	submitter: Option<S::Submitter>,
 	header: Header,
 	receipts: Option<Vec<Receipt>>,
-) -> Result<(HeaderId, Vec<(HeaderId, Option<S::Submitter>)>), Error> {
+) -> Result<(HeaderId, FinalizedHeaders<S>), Error> {
 	// first check that we are able to import this header at all
 	let (header_id, finalized_id) = is_importable_header(storage, &header)?;
 

bridges/modules/ethereum/src/lib.rs

@@ -15,6 +15,8 @@
 // along with Parity Bridges Common.  If not, see <http://www.gnu.org/licenses/>.
 
 #![cfg_attr(not(feature = "std"), no_std)]
+// Runtime-generated enums
+#![allow(clippy::large_enum_variant)]
 
 use crate::finality::{CachedFinalityVotes, FinalityVotes};
 use codec::{Decode, Encode};
@@ -235,6 +237,7 @@ impl<Submitter> ImportContext<Submitter> {
 	}
 
 	/// Converts import context into header we're going to import.
+	#[allow(clippy::too_many_arguments)]
 	pub fn into_import_header(
 		self,
 		is_best: bool,
@@ -503,7 +506,7 @@ impl<T: Trait> Module<T> {
 	}
 
 	/// Verify that transaction is included into given finalized block.
-	pub fn verify_transaction_finalized(block: H256, tx_index: u64, proof: &Vec<RawTransaction>) -> bool {
+	pub fn verify_transaction_finalized(block: H256, tx_index: u64, proof: &[RawTransaction]) -> bool {
 		crate::verify_transaction_finalized(&BridgeStorage::<T>::new(), block, tx_index, proof)
 	}
 }
@@ -616,13 +619,8 @@ impl<T: Trait> BridgeStorage<T> {
 		blocks_at_number: &mut Vec<H256>,
 	) {
 		// ensure that unfinalized headers we want to prune do not have scheduled changes
-		if number > finalized_number {
-			if blocks_at_number
-				.iter()
-				.any(|block| ScheduledChanges::contains_key(block))
-			{
-				return;
-			}
+		if number > finalized_number && blocks_at_number.iter().any(ScheduledChanges::contains_key) {
+			return;
 		}
 
 		// physically remove headers and (probably) obsolete validators sets
@@ -678,11 +676,9 @@ impl<T: Trait> Storage for BridgeStorage<T> {
 		let mut current_id = *parent;
 		loop {
 			// if we have reached finalized block' sibling => stop with special signal
-			if current_id.number == best_finalized.number {
-				if current_id.hash != best_finalized.hash {
-					votes.stopped_at_finalized_sibling = true;
-					return votes;
-				}
+			if current_id.number == best_finalized.number && current_id.hash != best_finalized.hash {
+				votes.stopped_at_finalized_sibling = true;
+				return votes;
 			}
 
 			// if we have reached target header => stop
@@ -834,6 +830,7 @@ impl<T: Trait> Storage for BridgeStorage<T> {
 }
 
 /// Initialize storage.
+#[cfg(any(feature = "std", feature = "runtime-benchmarks"))]
 pub(crate) fn initialize_storage<T: Trait>(
 	initial_header: &Header,
 	initial_difficulty: U256,
@@ -885,7 +882,7 @@ pub fn verify_transaction_finalized<S: Storage>(
 	storage: &S,
 	block: H256,
 	tx_index: u64,
-	proof: &Vec<RawTransaction>,
+	proof: &[RawTransaction],
 ) -> bool {
 	if tx_index >= proof.len() as _ {
 		return false;
@@ -906,9 +903,7 @@ pub fn verify_transaction_finalized<S: Storage>(
 	let is_finalized = match header.number < finalized.number {
 		true => ancestry(storage, finalized.hash)
 			.skip_while(|(_, ancestor)| ancestor.number > header.number)
-			.filter(|&(ancestor_hash, _)| ancestor_hash == block)
-			.next()
-			.is_some(),
+			.any(|(ancestor_hash, _)| ancestor_hash == block),
 		false => block == finalized.hash,
 	};
 	if !is_finalized {
@@ -985,7 +980,7 @@ pub(crate) mod tests {
 						hash,
 						StoredHeader {
 							submitter: None,
-							header: header,
+							header,
 							total_difficulty: 0.into(),
 							next_validators_set_id: 0,
 							last_signal_block: None,
@@ -1266,7 +1261,7 @@ pub(crate) mod tests {
 		run_test_with_genesis(example_header(), TOTAL_VALIDATORS, |_| {
 			let storage = BridgeStorage::<TestRuntime>::new();
 			assert_eq!(
-				verify_transaction_finalized(&storage, example_header().compute_hash(), 0, &vec![example_tx()],),
+				verify_transaction_finalized(&storage, example_header().compute_hash(), 0, &[example_tx()],),
 				true,
 			);
 		});
@@ -1280,7 +1275,7 @@ pub(crate) mod tests {
 			insert_header(&mut storage, example_header());
 			storage.finalize_and_prune_headers(Some(example_header().compute_id()), 0);
 			assert_eq!(
-				verify_transaction_finalized(&storage, example_header_parent().compute_hash(), 0, &vec![example_tx()],),
+				verify_transaction_finalized(&storage, example_header_parent().compute_hash(), 0, &[example_tx()],),
 				true,
 			);
 		});
@@ -1291,7 +1286,7 @@ pub(crate) mod tests {
 		run_test_with_genesis(example_header(), TOTAL_VALIDATORS, |_| {
 			let storage = BridgeStorage::<TestRuntime>::new();
 			assert_eq!(
-				verify_transaction_finalized(&storage, example_header().compute_hash(), 1, &vec![],),
+				verify_transaction_finalized(&storage, example_header().compute_hash(), 1, &[],),
 				false,
 			);
 		});
@@ -1302,7 +1297,7 @@ pub(crate) mod tests {
 		run_test(TOTAL_VALIDATORS, |_| {
 			let storage = BridgeStorage::<TestRuntime>::new();
 			assert_eq!(
-				verify_transaction_finalized(&storage, example_header().compute_hash(), 1, &vec![],),
+				verify_transaction_finalized(&storage, example_header().compute_hash(), 1, &[],),
 				false,
 			);
 		});
@@ -1315,7 +1310,7 @@ pub(crate) mod tests {
 			insert_header(&mut storage, example_header_parent());
 			insert_header(&mut storage, example_header());
 			assert_eq!(
-				verify_transaction_finalized(&storage, example_header().compute_hash(), 0, &vec![example_tx()],),
+				verify_transaction_finalized(&storage, example_header().compute_hash(), 0, &[example_tx()],),
 				false,
 			);
 		});
@@ -1334,7 +1329,7 @@ pub(crate) mod tests {
 			insert_header(&mut storage, finalized_header_sibling);
 			storage.finalize_and_prune_headers(Some(example_header().compute_id()), 0);
 			assert_eq!(
-				verify_transaction_finalized(&storage, finalized_header_sibling_hash, 0, &vec![example_tx()],),
+				verify_transaction_finalized(&storage, finalized_header_sibling_hash, 0, &[example_tx()],),
 				false,
 			);
 		});
@@ -1353,7 +1348,7 @@ pub(crate) mod tests {
 			insert_header(&mut storage, example_header());
 			storage.finalize_and_prune_headers(Some(example_header().compute_id()), 0);
 			assert_eq!(
-				verify_transaction_finalized(&storage, finalized_header_uncle_hash, 0, &vec![example_tx()],),
+				verify_transaction_finalized(&storage, finalized_header_uncle_hash, 0, &[example_tx()],),
 				false,
 			);
 		});
@@ -1368,7 +1363,7 @@ pub(crate) mod tests {
 					&storage,
 					example_header().compute_hash(),
 					0,
-					&vec![example_tx(), example_tx(),],
+					&[example_tx(), example_tx()],
 				),
 				false,
 			);

bridges/modules/ethereum/src/mock.rs

@@ -160,6 +160,6 @@ impl Default for KeepSomeHeadersBehindBest {
 
 impl PruningStrategy for KeepSomeHeadersBehindBest {
 	fn pruning_upper_bound(&mut self, best_number: u64, _: u64) -> u64 {
-		best_number.checked_sub(self.0).unwrap_or(0)
+		best_number.saturating_sub(self.0)
 	}
 }

bridges/modules/ethereum/src/test_utils.rs

@@ -86,7 +86,7 @@ impl HeaderBuilder {
 		use crate::HeadersByNumber;
 		use frame_support::StorageMap;
 
-		let parent_hash = HeadersByNumber::get(parent_number).unwrap()[0].clone();
+		let parent_hash = HeadersByNumber::get(parent_number).unwrap()[0];
 		Self::with_parent_hash_on_runtime::<T>(parent_hash)
 	}
 
@@ -130,7 +130,7 @@ impl HeaderBuilder {
 	/// Adds empty steps to this header.
 	pub fn empty_steps(mut self, empty_steps: &[(&SecretKey, u64)]) -> Self {
 		let sealed_empty_steps = empty_steps
-			.into_iter()
+			.iter()
 			.map(|(author, step)| {
 				let mut empty_step = SealedEmptyStep {
 					step: *step,

bridges/modules/ethereum/src/validators.rs

@@ -20,7 +20,7 @@ use primitives::{Address, Header, HeaderId, LogEntry, Receipt, U256};
 use sp_std::prelude::*;
 
 /// The hash of InitiateChange event of the validators set contract.
-pub(crate) const CHANGE_EVENT_HASH: &'static [u8; 32] = &[
+pub(crate) const CHANGE_EVENT_HASH: &[u8; 32] = &[
 	0x55, 0x25, 0x2f, 0xa6, 0xee, 0xe4, 0x74, 0x1b, 0x4e, 0x24, 0xa7, 0x4a, 0x70, 0xe9, 0xc1, 0x1f, 0xd2, 0xc2, 0x28,
 	0x1d, 0xf8, 0xd6, 0xea, 0x13, 0x12, 0x6f, 0xf8, 0x45, 0xf7, 0x82, 0x5c, 0x89,
 ];
@@ -49,6 +49,9 @@ pub enum ValidatorsSource {
 	Contract(Address, Vec<Address>),
 }
 
+/// A short hand for optional validators change.
+pub type ValidatorsChange = Option<Vec<Address>>;
+
 /// Validators manager.
 pub struct Validators<'a> {
 	config: &'a ValidatorsConfiguration,
@@ -94,7 +97,7 @@ impl<'a> Validators<'a> {
 		&self,
 		header: &Header,
 		receipts: Option<Vec<Receipt>>,
-	) -> Result<(Option<Vec<Address>>, Option<Vec<Address>>), Error> {
+	) -> Result<(ValidatorsChange, ValidatorsChange), Error> {
 		// let's first check if new source is starting from this header
 		let (source_index, _, source) = self.source_at(header.number);
 		let (next_starts_at, next_source) = self.source_at_next_header(source_index, header.number);
@@ -223,7 +226,7 @@ impl<'a> Validators<'a> {
 	}
 
 	/// Returns source of validators that should author the header.
-	fn source_at<'b>(&'b self, header_number: u64) -> (usize, u64, &'b ValidatorsSource) {
+	fn source_at(&self, header_number: u64) -> (usize, u64, &ValidatorsSource) {
 		match self.config {
 			ValidatorsConfiguration::Single(ref source) => (0, 0, source),
 			ValidatorsConfiguration::Multi(ref sources) => sources
@@ -240,11 +243,7 @@ impl<'a> Validators<'a> {
 	}
 
 	/// Returns source of validators that should author the next header.
-	fn source_at_next_header<'b>(
-		&'b self,
-		header_source_index: usize,
-		header_number: u64,
-	) -> (u64, &'b ValidatorsSource) {
+	fn source_at_next_header(&self, header_source_index: usize, header_number: u64) -> (u64, &ValidatorsSource) {
 		match self.config {
 			ValidatorsConfiguration::Single(ref source) => (0, source),
 			ValidatorsConfiguration::Multi(ref sources) => {

bridges/modules/ethereum/src/verification.rs

@@ -22,6 +22,7 @@ use primitives::{
 	public_to_address, step_validator, Address, Header, HeaderId, Receipt, SealedEmptyStep, H256, H520, U128, U256,
 };
 use sp_io::crypto::secp256k1_ecdsa_recover;
+use sp_runtime::transaction_validity::TransactionTag;
 use sp_std::{vec, vec::Vec};
 
 /// Pre-check to see if should try and import this header.
@@ -43,6 +44,8 @@ pub fn is_importable_header<S: Storage>(storage: &S, header: &Header) -> Result<
 }
 
 /// Try accept unsigned aura header into transaction pool.
+///
+/// Returns required and provided tags.
 pub fn accept_aura_header_into_pool<S: Storage>(
 	storage: &S,
 	config: &AuraConfiguration,
@@ -50,7 +53,7 @@ pub fn accept_aura_header_into_pool<S: Storage>(
 	pool_config: &PoolConfiguration,
 	header: &Header,
 	receipts: Option<&Vec<Receipt>>,
-) -> Result<(Vec<Vec<u8>>, Vec<Vec<u8>>), Error> {
+) -> Result<(Vec<TransactionTag>, Vec<TransactionTag>), Error> {
 	// check if we can verify further
 	let (header_id, _) = is_importable_header(storage, header)?;
 
@@ -365,6 +368,7 @@ mod tests {
 	use frame_support::{StorageMap, StorageValue};
 	use primitives::{compute_merkle_root, rlp_encode, TransactionOutcome, H520};
 	use secp256k1::SecretKey;
+	use sp_runtime::transaction_validity::TransactionTag;
 
 	const GENESIS_STEP: u64 = 42;
 	const TOTAL_VALIDATORS: usize = 3;
@@ -386,7 +390,7 @@ mod tests {
 
 	fn default_accept_into_pool(
 		mut make_header: impl FnMut(&[SecretKey]) -> (Header, Option<Vec<Receipt>>),
-	) -> Result<(Vec<Vec<u8>>, Vec<Vec<u8>>), Error> {
+	) -> Result<(Vec<TransactionTag>, Vec<TransactionTag>), Error> {
 		run_test_with_genesis(genesis(), TOTAL_VALIDATORS, |_| {
 			let validators = vec![validator(0), validator(1), validator(2)];
 			let mut storage = BridgeStorage::<TestRuntime>::new();
@@ -429,7 +433,7 @@ mod tests {
 			},
 		);
 
-		let header_hash = HeadersByNumber::get(&number).unwrap()[0].clone();
+		let header_hash = HeadersByNumber::get(&number).unwrap()[0];
 		let mut header = Headers::<TestRuntime>::get(&header_hash).unwrap();
 		header.next_validators_set_id = set_id;
 		if let Some(signalled_set) = signalled_set {
@@ -456,15 +460,15 @@ mod tests {
 		assert_eq!(default_verify(&header), Err(Error::InvalidSealArity));
 
 		// when there's single seal (we expect 2 or 3 seals)
-		header.seal = vec![vec![].into()];
+		header.seal = vec![vec![]];
 		assert_eq!(default_verify(&header), Err(Error::InvalidSealArity));
 
 		// when there's 3 seals (we expect 2 by default)
-		header.seal = vec![vec![].into(), vec![].into(), vec![].into()];
+		header.seal = vec![vec![], vec![], vec![]];
 		assert_eq!(default_verify(&header), Err(Error::InvalidSealArity));
 
 		// when there's 2 seals
-		header.seal = vec![vec![].into(), vec![].into()];
+		header.seal = vec![vec![], vec![]];
 		assert_ne!(default_verify(&header), Err(Error::InvalidSealArity));
 	}
 
@@ -564,7 +568,7 @@ mod tests {
 	fn verifies_step() {
 		// when step is missing from seals
 		let mut header = Header {
-			seal: vec![vec![].into(), vec![].into()],
+			seal: vec![vec![], vec![]],
 			gas_limit: test_aura_config().min_gas_limit,
 			parent_hash: genesis().compute_hash(),
 			..Default::default()
@@ -585,7 +589,7 @@ mod tests {
 
 		// when step is lesser that for the parent block
 		header.seal[0] = rlp_encode(&40u64);
-		header.seal = vec![vec![40].into(), vec![].into()];
+		header.seal = vec![vec![40], vec![]];
 		assert_eq!(verify_with_config(&config, &header), Err(Error::DoubleVote));
 
 		// when step is OK
@@ -691,7 +695,7 @@ mod tests {
 			default_accept_into_pool(|_| (
 				Header {
 					number: 20_000_000,
-					seal: vec![vec![].into(), vec![].into()],
+					seal: vec![vec![], vec![]],
 					gas_limit: test_aura_config().min_gas_limit,
 					log_bloom: (&[0xff; 256]).into(),
 					..Default::default()