Introduce cargo clippy CI check (#70)

* Introduce clippy

* Clippy findings fixes

* Taplo formatting

* Update .github/workflows/check.yml

Co-authored-by: Branislav Kontur <[email protected]>

* Fix clippy warning

---------

Co-authored-by: Branislav Kontur <[email protected]>

Author: karolk91

.github/workflows/check.yml

@@ -74,6 +74,31 @@ jobs:
         run: |
           cargo check --workspace --features=runtime-benchmarks
 
+  clippy:
+    name: Cargo clippy
+    # TODO: replace `parity-default` with `ubuntu-latest` when the repo becomes public.
+    runs-on: parity-default
+    needs: [set-image]
+    container:
+      image: ${{ needs.set-image.outputs.CI_IMAGE }}
+    env:
+      RUSTFLAGS: "-D warnings"
+      SKIP_WASM_BUILD: 1
+    steps:
+      - name: Checkout sources
+        uses: actions/checkout@v4
+
+      - name: Rust cache
+        uses: Swatinem/rust-cache@v2
+        with:
+          cache-on-failure: true
+          cache-all-crates: true
+
+      - name: Cargo clippy
+        run: |
+          cargo clippy --all-targets --locked --workspace --quiet
+          cargo clippy --all-targets --all-features --locked --workspace --quiet
+
   test:
     name: Test
     # TODO: replace `parity-default` with `ubuntu-latest` when the repo becomes public.

Cargo.lock

@@ -24,6 +24,7 @@ serde = { workspace = true }
 sc-chain-spec = { workspace = true }
 frame-support = { workspace = true, optional = true }
 frame-system-rpc-runtime-api = { workspace = true }
+frame-try-runtime = { workspace = true, optional = true }
 pallet-transaction-payment = { workspace = true }
 sc-cli = { workspace = true }
 sc-client-api = { workspace = true }
@@ -99,6 +100,7 @@ runtime-benchmarks = [
 try-runtime = [
 	"bulletin-polkadot-runtime/try-runtime",
 	"frame-support/try-runtime",
+	"frame-try-runtime/try-runtime",
 	"pallet-transaction-payment/try-runtime",
 	"polkadot-bulletin-chain-runtime/try-runtime",
 	"sp-runtime/try-runtime",

node/Cargo.toml

@@ -5,12 +5,13 @@ use std::time::Duration;
 /// Generates inherent data for the `benchmark overhead` command.
 ///
 /// Note: Should only be used for benchmarking.
+#[allow(clippy::result_large_err)]
 pub fn inherent_benchmark_data() -> Result<InherentData> {
 	let mut inherent_data = InherentData::new();
 	let d = Duration::from_millis(0);
 	let timestamp = sp_timestamp::InherentDataProvider::new(d.into());
 
 	futures::executor::block_on(timestamp.provide_inherent_data(&mut inherent_data))
-		.map_err(|e| format!("creating inherent data: {:?}", e))?;
+		.map_err(|e| format!("creating inherent data: {e:?}"))?;
 	Ok(inherent_data)
 }

node/src/benchmarking.rs

@@ -63,6 +63,7 @@ impl SubstrateCli for Cli {
 }
 
 /// Parse and run command line arguments
+#[allow(clippy::result_large_err)]
 pub fn run() -> sc_cli::Result<()> {
 	let cli = Cli::from_args();
 
@@ -163,8 +164,7 @@ pub fn run() -> sc_cli::Result<()> {
 							return Err(sc_cli::Error::Input(
 								"Bulletin binary does not support `--runtime` flag for `benchmark overhead`. Please provide a chain spec or use the `frame-omni-bencher`."
 									.into(),
-							)
-								.into())
+							))
 						}
 
 						cmd.run_with_default_builder_and_spec::<Block, ()>(

node/src/command.rs

@@ -12,6 +12,7 @@ mod fake_runtime_api;
 mod node_primitives;
 mod rpc;
 
+#[allow(clippy::result_large_err)]
 fn main() -> sc_cli::Result<()> {
 	command::run()
 }

node/src/main.rs

@@ -15,10 +15,3 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 pub use polkadot_primitives::{AccountId, Balance, Block, BlockNumber, Hash, Nonce};
-
-/// Should be aligned with runtimes.
-#[cfg(feature = "try-runtime")]
-pub const MILLISECS_PER_BLOCK: u64 = 6000;
-/// Should be aligned with runtimes.
-#[cfg(feature = "try-runtime")]
-pub const SLOT_DURATION: u64 = MILLISECS_PER_BLOCK;

node/src/node_primitives.rs

@@ -25,6 +25,7 @@ type FullGrandpaBlockImport =
 const GRANDPA_JUSTIFICATION_PERIOD: u32 = 512;
 
 #[allow(clippy::type_complexity)]
+#[allow(clippy::result_large_err)]
 pub fn new_partial(
 	config: &Configuration,
 ) -> Result<
@@ -141,6 +142,7 @@ pub fn new_partial(
 }
 
 /// Builds a new service for a full client.
+#[allow(clippy::result_large_err)]
 pub fn new_full<
 	N: sc_network::NetworkBackend<Block, <Block as sp_runtime::traits::Block>::Hash>,
 >(
@@ -211,7 +213,7 @@ pub fn new_full<
 		);
 	}
 
-	let role = config.role.clone();
+	let role = config.role;
 	let force_authoring = config.force_authoring;
 	let backoff_authoring_blocks: Option<()> = None;
 	let name = config.network.node_name.clone();

node/src/service.rs

@@ -225,15 +225,11 @@ mod benchmarks {
 		let hash = [0u8; 32];
 		let max_size: u64 = 1024 * 1024;
 		let origin2 = origin.clone();
-		TransactionStorage::<T>::authorize_preimage(
-			origin2 as T::RuntimeOrigin,
-			hash.clone(),
-			max_size,
-		)
-		.map_err(|_| BenchmarkError::Stop("unable to authorize account"))?;
+		TransactionStorage::<T>::authorize_preimage(origin2 as T::RuntimeOrigin, hash, max_size)
+			.map_err(|_| BenchmarkError::Stop("unable to authorize account"))?;
 
 		#[extrinsic_call]
-		_(origin as T::RuntimeOrigin, hash.clone());
+		_(origin as T::RuntimeOrigin, hash);
 
 		assert_last_event::<T>(Event::PreimageAuthorizationRefreshed { hash }.into());
 		Ok(())

pallets/transaction-storage/src/benchmarking.rs

@@ -140,7 +140,7 @@ impl CheckContext {
 }
 
 fn num_chunks(bytes: u32) -> u32 {
-	((bytes as u64 + CHUNK_SIZE as u64 - 1) / CHUNK_SIZE as u64) as u32
+	(bytes as u64).div_ceil(CHUNK_SIZE as u64) as u32
 }
 
 #[polkadot_sdk_frame::pallet]
@@ -742,7 +742,7 @@ pub mod pallet {
 					Ok(())
 				} else {
 					// No previous authorization to refresh.
-					return Err(Error::<T>::AuthorizationNotFound.into())
+					Err(Error::<T>::AuthorizationNotFound.into())
 				}
 			})
 		}
@@ -818,7 +818,7 @@ pub mod pallet {
 		/// block.
 		fn block_transactions_full() -> bool {
 			BlockTransactions::<T>::decode_len()
-				.map_or(false, |len| len >= T::MaxBlockTransactions::get() as usize)
+				.is_some_and(|len| len >= T::MaxBlockTransactions::get() as usize)
 		}
 
 		/// Check that authorization exists for data of the given size to be stored in a single