384 add clippy to ci

.github/workflows/test-code.yml

@@ -9,29 +9,6 @@ on:
       - main
 
 jobs:
-  build-check:
-    runs-on: ubuntu-latest
-
-    steps:
-      - uses: actions/checkout@v3
-      - uses: ./.github/actions/shared
-
-      - name: Install toolchain
-        # Call `rustup show` as a hack so that the toolchain defined in rust-toolchain.toml is installed
-        run: rustup show
-
-      # Enable this for clippy linting.
-      # - name: Check and Lint Code
-      #   run: cargo +nightly-2021-12-01 clippy -- -D warnings
-
-      - name: Install Protoc
-        uses: arduino/setup-protoc@v1
-        with:
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
-
-      - name: Check Code
-        run: cargo check
-
   test-code:
     runs-on: ubuntu-latest
 
@@ -64,3 +41,27 @@ jobs:
         with:
           toolchain: nightly-2024-04-18
           command: test
+
+      - name: Clippy -- Main
+        uses: actions-rs/cargo@v1
+        with:
+          toolchain: nightly-2024-04-18
+          command: clippy
+          args: --all-features -- -W clippy::all -A clippy::style -A forgetting_copy_types -A forgetting_references
+
+      - name: Clippy -- All Targets (except integration)
+        uses: actions-rs/cargo@v1
+        with:
+          toolchain: nightly-2024-04-18
+          command: clippy
+          # We are a bit more forgiving when it comes to the code in tests and only check for correctness
+          args: --workspace --all-features --all-targets --exclude runtime-integration-tests -- -A clippy::all -W clippy::correctness -A forgetting_copy_types -A forgetting_references
+
+      - name: Clippy -- Integration
+        uses: actions-rs/cargo@v1
+        with:
+          toolchain: nightly-2024-04-18
+          command: clippy
+          # We are a bit more forgiving when it comes to the code in tests and only check for correctness
+          args: --package runtime-integration-tests --all-features --all-targets -- -A clippy::all -W clippy::correctness -A forgetting_copy_types -A forgetting_references
+

chain-extensions/token/src/lib.rs

@@ -73,7 +73,7 @@ where
 		+ orml_currencies_allowance_extension::Config,
 	<T as SysConfig>::AccountId: UncheckedFrom<<T as SysConfig>::Hash> + AsRef<[u8]>,
 	Tokens: orml_traits::MultiCurrency<AccountId, CurrencyId = CurrencyId>,
-	AccountId: sp_std::fmt::Debug + Decode,
+	AccountId: sp_std::fmt::Debug + Decode + core::clone::Clone,
 {
 	fn call<E: Ext>(&mut self, env: Environment<E, InitState>) -> Result<RetVal, DispatchError>
 	where
@@ -92,15 +92,14 @@ where
 			0,
 		);
 
-		let result = match func_id {
+		match func_id {
 			FuncId::TotalSupply => total_supply(env, overhead_weight),
 			FuncId::BalanceOf => balance_of(env, overhead_weight),
 			FuncId::Transfer => transfer(env, overhead_weight),
 			FuncId::Allowance => allowance(env, overhead_weight),
 			FuncId::Approve => approve(env, overhead_weight),
 			FuncId::TransferFrom => transfer_from(env, overhead_weight),
-		};
-		result
+		}
 	}
 
 	fn enabled() -> bool {
@@ -200,7 +199,7 @@ where
 		+ orml_currencies::Config<MultiCurrency = Tokens, AccountId = AccountId>
 		+ orml_currencies_allowance_extension::Config,
 	E: Ext<T = T>,
-	AccountId: sp_std::fmt::Debug,
+	AccountId: sp_std::fmt::Debug + core::clone::Clone,
 	Tokens: orml_traits::MultiCurrency<AccountId, CurrencyId = CurrencyId>,
 	(CurrencyId, AccountId, <Tokens as MultiCurrency<AccountId>>::Balance): Decode,
 {
@@ -229,7 +228,7 @@ where
 
 	<orml_currencies::Pallet<T> as MultiCurrency<T::AccountId>>::transfer(
 		currency_id,
-		&env.ext().caller(),
+		env.ext().caller(),
 		&recipient,
 		amount,
 	)?;
@@ -292,7 +291,7 @@ where
 		+ orml_currencies::Config<MultiCurrency = Tokens, AccountId = AccountId>
 		+ orml_currencies_allowance_extension::Config,
 	E: Ext<T = T>,
-	AccountId: sp_std::fmt::Debug,
+	AccountId: sp_std::fmt::Debug + core::clone::Clone,
 	Tokens: orml_traits::MultiCurrency<AccountId, CurrencyId = CurrencyId>,
 	(CurrencyId, AccountId, <Tokens as MultiCurrency<AccountId>>::Balance): Decode,
 {
@@ -320,7 +319,7 @@ where
 
 	orml_currencies_allowance_extension::Pallet::<T>::do_approve_transfer(
 		currency_id,
-		&env.ext().caller(),
+		env.ext().caller(),
 		&spender,
 		amount,
 	)?;
@@ -338,7 +337,7 @@ where
 		+ orml_currencies::Config<MultiCurrency = Tokens, AccountId = AccountId>
 		+ orml_currencies_allowance_extension::Config,
 	E: Ext<T = T>,
-	AccountId: sp_std::fmt::Debug,
+	AccountId: sp_std::fmt::Debug + core::clone::Clone,
 	Tokens: orml_traits::MultiCurrency<AccountId, CurrencyId = CurrencyId>,
 	(AccountId, CurrencyId, AccountId, <Tokens as MultiCurrency<AccountId>>::Balance): Decode,
 {
@@ -372,7 +371,7 @@ where
 	orml_currencies_allowance_extension::Pallet::<T>::do_transfer_approved(
 		currency_id,
 		&owner,
-		&env.ext().caller(),
+		env.ext().caller(),
 		&recipient,
 		amount,
 	)?;

node/src/command.rs

@@ -404,19 +404,19 @@ pub fn run() -> Result<()> {
 						.map_err(|e| format!("Error: {:?}", e))?;
 
 				match runner.config().chain_spec.identify() {
-					ChainIdentity::Amplitude => runner.async_run(|config| {
+					ChainIdentity::Amplitude => runner.async_run(|_config| {
 						Ok((cmd.run::<Block, ExtendedHostFunctions<
 							sp_io::SubstrateHostFunctions,
 							<AmplitudeRuntimeExecutor as NativeExecutionDispatch>::ExtendHostFunctions,
 						>, _>(Some(substrate_info(BLOCK_TIME_MILLIS))), task_manager))
 					}),
-					ChainIdentity::Foucoco => runner.async_run(|config| {
+					ChainIdentity::Foucoco => runner.async_run(|_config| {
 						Ok((cmd.run::<Block, ExtendedHostFunctions<
 							sp_io::SubstrateHostFunctions,
 							<FoucocoRuntimeExecutor as NativeExecutionDispatch>::ExtendHostFunctions,
 						>, _>(Some(substrate_info(BLOCK_TIME_MILLIS))), task_manager))
 					}),
-					ChainIdentity::Pendulum => runner.async_run(|config| {
+					ChainIdentity::Pendulum => runner.async_run(|_config| {
 						Ok((cmd.run::<Block, ExtendedHostFunctions<
 							sp_io::SubstrateHostFunctions,
 							<PendulumRuntimeExecutor as NativeExecutionDispatch>::ExtendHostFunctions,

pallets/orml-currencies-allowance-extension/src/lib.rs

@@ -238,7 +238,7 @@ pub mod pallet {
 	}
 }
 
-#[allow(clippy::forget_non_drop, clippy::swap_ptr_to_ref, clippy::forget_ref, clippy::forget_copy)]
+#[allow(clippy::forget_non_drop, clippy::swap_ptr_to_ref, forgetting_references, forgetting_copy_types)]
 #[cfg_attr(test, mockable)]
 impl<T: Config> Pallet<T> {
 	// Check the amount approved to be spent by an owner to a delegate

pallets/orml-tokens-management-extension/src/lib.rs

@@ -124,14 +124,14 @@ pub mod pallet {
 				T::CurrencyIdChecker::is_valid_currency_id(&currency_id),
 				Error::<T>::NotOwnableCurrency
 			);
-			ensure!(!CurrencyData::<T>::contains_key(&currency_id), Error::<T>::AlreadyCreated);
+			ensure!(!CurrencyData::<T>::contains_key(currency_id), Error::<T>::AlreadyCreated);
 
 			let deposit = T::AssetDeposit::get();
 			ext::orml_tokens::reserve::<T>(T::DepositCurrency::get(), &creator, deposit)
 				.map_err(|_| Error::<T>::InsufficientBalance)?;
 
 			CurrencyData::<T>::insert(
-				currency_id.clone(),
+				currency_id,
 				CurrencyDetails {
 					owner: creator.clone(),
 					issuer: creator.clone(),
@@ -175,7 +175,7 @@ pub mod pallet {
 			ensure!(origin == currency_data.issuer, Error::<T>::NoPermission);
 
 			// do mint via orml-currencies
-			let _ = ext::orml_tokens::mint::<T>(currency_id, &to, amount)?;
+			ext::orml_tokens::mint::<T>(currency_id, &to, amount)?;
 
 			Self::deposit_event(Event::Mint { currency_id, to, amount });
 			Ok(())
@@ -207,7 +207,7 @@ pub mod pallet {
 			ensure!(origin == currency_data.admin, Error::<T>::NoPermission);
 
 			// do burn via orml-currencies
-			let _ = ext::orml_tokens::burn::<T>(currency_id, &from, amount)?;
+			ext::orml_tokens::burn::<T>(currency_id, &from, amount)?;
 
 			Self::deposit_event(Event::Burned { currency_id, from, amount });
 			Ok(())
@@ -231,7 +231,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			let origin = ensure_signed(origin)?;
 
-			CurrencyData::<T>::try_mutate(currency_id.clone(), |maybe_details| {
+			CurrencyData::<T>::try_mutate(currency_id, |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T>::NotCreated)?;
 				ensure!(origin == details.owner, Error::<T>::NoPermission);
 
@@ -241,7 +241,7 @@ pub mod pallet {
 				details.owner = new_owner.clone();
 
 				// move reserved balance to the new owner's account
-				let _ = ext::orml_tokens::repatriate_reserve::<T>(
+				ext::orml_tokens::repatriate_reserve::<T>(
 					T::DepositCurrency::get(),
 					&origin,
 					&new_owner,
@@ -271,13 +271,13 @@ pub mod pallet {
 		) -> DispatchResult {
 			ensure_root(origin)?;
 
-			CurrencyData::<T>::try_mutate(currency_id.clone(), |maybe_details| {
+			CurrencyData::<T>::try_mutate(currency_id, |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T>::NotCreated)?;
 
 				if details.owner == new_owner {
 					return Ok(())
 				}
-				let _ = ext::orml_tokens::repatriate_reserve::<T>(
+				ext::orml_tokens::repatriate_reserve::<T>(
 					T::DepositCurrency::get(),
 					&details.owner,
 					&new_owner,
@@ -311,7 +311,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			let origin = ensure_signed(origin)?;
 
-			CurrencyData::<T>::try_mutate(currency_id.clone(), |maybe_details| {
+			CurrencyData::<T>::try_mutate(currency_id, |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T>::NotCreated)?;
 
 				ensure!(origin == details.owner, Error::<T>::NoPermission);

pallets/parachain-staking/Cargo.toml

@@ -36,9 +36,10 @@ frame-benchmarking = {git = "https://github.com/paritytech/substrate", branch =
 [features]
 default = ["std"]
 runtime-benchmarks = [
-  "frame-benchmarking",
+  "frame-benchmarking/runtime-benchmarks",
   "frame-support/runtime-benchmarks",
   "frame-system/runtime-benchmarks",
+  "pallet-balances/runtime-benchmarks",
 ]
 std = [
   "frame-support/std",

pallets/parachain-staking/src/benchmarking.rs

@@ -307,7 +307,7 @@ benchmarks! {
 		let n in (T::MinCollators::get() + 1) .. T::MaxTopCandidates::get() - 1;
 		let m in 0 .. T::MaxDelegatorsPerCollator::get();
 
-		let u = T::MaxUnstakeRequests::get() as u32 - 1;
+		let u = T::MaxUnstakeRequests::get() - 1;
 		let candidates = setup_collator_candidates::<T>(n, None);
 		for (i, c) in candidates.iter().enumerate() {
 			fill_delegators::<T>(m, c.clone(), i.saturated_into::<u32>());
@@ -532,7 +532,7 @@ benchmarks! {
 	}
 
 	unlock_unstaked {
-		let u in 1 .. (T::MaxUnstakeRequests::get() as u32 - 1);
+		let u in 1 .. (T::MaxUnstakeRequests::get() - 1);
 
 		let candidate = account("collator", 0u32, COLLATOR_ACCOUNT_SEED);
 		let free_balance = T::CurrencyBalance::from(u128::MAX);

pallets/parachain-staking/src/try_state.rs

@@ -35,6 +35,7 @@ pub fn log_and_return_error_message(error_message: String) -> &'static str {
 	"Sanity test error"
 }
 
+#[allow(dead_code)]
 pub(crate) fn do_try_state<T: Config>() -> Result<(), &'static str> {
 	validate_candiate_pool::<T>()?;
 	validate_delegators::<T>()?;

pallets/parachain-staking/src/types.rs

@@ -84,20 +84,17 @@ impl<AccountId: Ord, Balance: PartialEq + Ord> Ord for Stake<AccountId, Balance>
 }
 
 /// The activity status of the collator.
-#[derive(Copy, Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)]
+#[derive(
+	Copy, Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen, Default,
+)]
 pub enum CandidateStatus {
 	/// Committed to be online and producing valid blocks (not equivocating)
+	#[default]
 	Active,
 	/// Staked until the inner round
 	Leaving(SessionIndex),
 }
 
-impl Default for CandidateStatus {
-	fn default() -> CandidateStatus {
-		CandidateStatus::Active
-	}
-}
-
 #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, TypeInfo, MaxEncodedLen)]
 #[scale_info(skip_type_params(MaxDelegatorsPerCandidate))]
 #[codec(mel_bound(AccountId: MaxEncodedLen, Balance: MaxEncodedLen))]