Merge pull request #398 from opentensor/fix/clippy-deny-warnings

ci: clippy deny warnings

.github/workflows/check-rust.yml

@@ -71,8 +71,7 @@ jobs:
       - name: cargo fmt
         run: cargo fmt --check --all
 
-  # runs cargo clippy --workspace --all-targets --all-features
-  cargo-clippy:
+  cargo-clippy-default-features:
     name: cargo clippy
     runs-on: SubtensorCI
     strategy:
@@ -117,8 +116,56 @@ jobs:
         with:
           key: ${{ matrix.os }}-${{ env.RUST_BIN_DIR }}
 
-      - name: cargo clippy --workspace --all-targets --all-features
-        run: cargo clippy --workspace --all-targets --all-features
+      - name: cargo clippy --workspace --all-targets -- -D warnings
+        run: cargo clippy --workspace --all-targets -- -D warnings
+
+  cargo-clippy-all-features:
+    name: cargo clippy --all-features
+    runs-on: SubtensorCI
+    strategy:
+      matrix:
+        rust-branch:
+          - nightly-2024-03-05
+        rust-target:
+          - x86_64-unknown-linux-gnu
+          # - x86_64-apple-darwin
+        os:
+          - ubuntu-latest
+          # - macos-latest
+        include:
+          - os: ubuntu-latest
+          # - os: macos-latest
+    env:
+      RELEASE_NAME: development
+      # RUSTFLAGS: -A warnings
+      RUSTV: ${{ matrix.rust-branch }}
+      RUST_BACKTRACE: full
+      RUST_BIN_DIR: target/${{ matrix.rust-target }}
+      SKIP_WASM_BUILD: 1
+      TARGET: ${{ matrix.rust-target }}
+    steps:
+      - name: Check-out repository under $GITHUB_WORKSPACE
+        uses: actions/checkout@v2
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update &&
+          sudo apt-get install -y clang curl libssl-dev llvm libudev-dev protobuf-compiler
+
+      - name: Install Rust ${{ matrix.rust-branch }}
+        uses: actions-rs/toolchain@v1.0.6
+        with:
+          toolchain: ${{ matrix.rust-branch }}
+          components: rustfmt, clippy
+          profile: minimal
+
+      - name: Utilize Shared Rust Cache
+        uses: Swatinem/rust-cache@v2.2.1
+        with:
+          key: ${{ matrix.os }}-${{ env.RUST_BIN_DIR }}
+
+      - name: cargo clippy --workspace --all-targets --all-features -- -D warnings
+        run: cargo clippy --workspace --all-targets --all-features -- -D warnings
 
   # runs cargo test --workspace
   cargo-test:
@@ -279,7 +326,7 @@ jobs:
 
   check-feature-propagation:
     name: zepter run check
-    runs-on: ubuntu-22.04
+    runs-on: SubtensorCI
 
     steps:
       - name: Install stable Rust
@@ -301,7 +348,7 @@ jobs:
 
   check-finney-migrations:
     name: check finney migrations
-    runs-on: ubuntu-22.04
+    runs-on: SubtensorCI
     steps:
       - name: Checkout sources
         uses: actions/checkout@v3

Cargo.toml

@@ -103,8 +103,6 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/polkadot-s
 substrate-fixed = { git = "https://github.com/encointer/substrate-fixed.git", tag = "v0.5.9" }
 substrate-frame-rpc-system = { git = "https://github.com/paritytech/polkadot-sdk.git", tag = "polkadot-v1.10.0" }
 substrate-wasm-builder = { git = "https://github.com/paritytech/polkadot-sdk.git", tag = "polkadot-v1.10.0" }
-
-[workspace.dev-dependencies]
 frame-metadata = "16"
 
 [profile.release]

pallets/admin-utils/tests/tests.rs

@@ -1071,7 +1071,7 @@ fn test_sudo_set_tx_delegate_take_rate_limit() {
                 <<Test as Config>::RuntimeOrigin>::signed(U256::from(1)),
                 to_be_set
             ),
-            Err(DispatchError::BadOrigin.into())
+            Err(DispatchError::BadOrigin)
         );
         assert_eq!(
             SubtensorModule::get_tx_delegate_take_rate_limit(),
@@ -1098,7 +1098,7 @@ fn test_sudo_set_min_delegate_take() {
                 <<Test as Config>::RuntimeOrigin>::signed(U256::from(1)),
                 to_be_set
             ),
-            Err(DispatchError::BadOrigin.into())
+            Err(DispatchError::BadOrigin)
         );
         assert_eq!(SubtensorModule::get_min_delegate_take(), init_value);
         assert_ok!(AdminUtils::sudo_set_min_delegate_take(

pallets/collective/src/tests.rs

@@ -20,9 +20,7 @@
 use super::{Event as CollectiveEvent, *};
 use crate as pallet_collective;
 use frame_support::{
-    assert_noop, assert_ok, derive_impl, parameter_types,
-    traits::{ConstU32, ConstU64},
-    Hashable,
+    assert_noop, assert_ok, derive_impl, parameter_types, traits::ConstU64, Hashable,
 };
 use frame_system::{EnsureRoot, EventRecord, Phase};
 use sp_core::H256;

pallets/subtensor/src/benchmarks.rs

@@ -1,7 +1,6 @@
 //! Subtensor pallet benchmarking.
 
 #![cfg(feature = "runtime-benchmarks")]
-//mod benchmarking;
 
 use crate::Pallet as Subtensor;
 use crate::*;
@@ -10,8 +9,6 @@ use frame_support::assert_ok;
 use frame_system::RawOrigin;
 pub use pallet::*;
 use sp_std::vec;
-use sp_std::vec::Vec;
-//use mock::{Test, new_test_ext};
 
 benchmarks! {
   // Add individual benchmarks here

pallets/subtensor/src/epoch.rs

@@ -215,11 +215,11 @@ impl<T: Config> Pallet<T> {
             // no weights set | outdated weights | self_weights
             if is_zero(&active_stake) {
                 // no active stake
-                normalized_validator_emission = stake.clone(); // do not mask inactive, assumes stake is normalized
-                normalized_combined_emission = stake.clone();
+                normalized_validator_emission.clone_from(&stake); // do not mask inactive, assumes stake is normalized
+                normalized_combined_emission.clone_from(&stake);
             } else {
-                normalized_validator_emission = active_stake.clone(); // emission proportional to inactive-masked normalized stake
-                normalized_combined_emission = active_stake.clone();
+                normalized_validator_emission.clone_from(&active_stake); // emission proportional to inactive-masked normalized stake
+                normalized_combined_emission.clone_from(&active_stake);
             }
         }
 
@@ -575,11 +575,11 @@ impl<T: Config> Pallet<T> {
             // no weights set | outdated weights | self_weights
             if is_zero(&active_stake) {
                 // no active stake
-                normalized_validator_emission = stake.clone(); // do not mask inactive, assumes stake is normalized
-                normalized_combined_emission = stake.clone();
+                normalized_validator_emission.clone_from(&stake); // do not mask inactive, assumes stake is normalized
+                normalized_combined_emission.clone_from(&stake);
             } else {
-                normalized_validator_emission = active_stake.clone(); // emission proportional to inactive-masked normalized stake
-                normalized_combined_emission = active_stake.clone();
+                normalized_validator_emission.clone_from(&active_stake); // emission proportional to inactive-masked normalized stake
+                normalized_combined_emission.clone_from(&active_stake);
             }
         }
 

pallets/subtensor/src/lib.rs

@@ -1924,12 +1924,12 @@ where
                 if Self::check_weights_min_stake(who) {
                     let priority: u64 = Self::get_priority_set_weights(who, *netuid);
                     Ok(ValidTransaction {
-                        priority: priority,
+                        priority,
                         longevity: 1,
                         ..Default::default()
                     })
                 } else {
-                    return Err(InvalidTransaction::Call.into());
+                    Err(InvalidTransaction::Call.into())
                 }
             }
             Some(Call::add_stake { .. }) => Ok(ValidTransaction {

pallets/subtensor/src/registration.rs

@@ -1,5 +1,4 @@
 use super::*;
-use frame_support::pallet_prelude::{DispatchResult, DispatchResultWithPostInfo};
 use frame_support::storage::IterableStorageDoubleMap;
 use sp_core::{Get, H256, U256};
 use sp_io::hashing::{keccak_256, sha2_256};

pallets/subtensor/src/root.rs

@@ -17,7 +17,7 @@
 
 use super::*;
 use crate::math::*;
-use frame_support::dispatch::{DispatchResultWithPostInfo, Pays};
+use frame_support::dispatch::Pays;
 use frame_support::storage::{IterableStorageDoubleMap, IterableStorageMap};
 use frame_support::traits::Get;
 use frame_support::weights::Weight;

pallets/subtensor/src/uids.rs

@@ -5,7 +5,6 @@ use sp_std::vec;
 
 impl<T: Config> Pallet<T> {
     // Returns the number of filled slots on a network.
-    ///
     pub fn get_subnetwork_n(netuid: u16) -> u16 {
         SubnetworkN::<T>::get(netuid)
     }

pallets/subtensor/src/utils.rs

@@ -296,7 +296,7 @@ impl<T: Config> Pallet<T> {
             return false;
         }
 
-        return current_block - prev_tx_block <= rate_limit;
+        current_block - prev_tx_block <= rate_limit
     }
 
     // ========================

runtime/Cargo.toml

@@ -84,7 +84,7 @@ pallet-registry = { default-features = false, path = "../pallets/registry" }
 pallet-commitments = { default-features = false, path = "../pallets/commitments" }
 
 [dev-dependencies]
-frame-metadata = { workspace-dev = true }
+frame-metadata = { workspace = true }
 sp-io = { workspace = true }
 sp-tracing = { workspace = true }
 

runtime/src/lib.rs

@@ -1430,7 +1430,10 @@ impl_runtime_apis! {
             use frame_system_benchmarking::Pallet as SystemBench;
             use baseline::Pallet as BaselineBench;
 
+            #[allow(non_local_definitions)]
             impl frame_system_benchmarking::Config for Runtime {}
+
+            #[allow(non_local_definitions)]
             impl baseline::Config for Runtime {}
 
             use frame_support::traits::WhitelistedStorageKeys;

runtime/src/migrations/account_data_migration.rs

@@ -1,5 +1,4 @@
 use crate::*;
-use log;
 use pallet_balances::ExtraFlags;
 
 #[cfg(feature = "try-runtime")]

runtime/tests/metadata.rs

@@ -2,7 +2,7 @@ use frame_metadata::RuntimeMetadata;
 use node_subtensor_runtime::Runtime;
 use scale_info::TypeDef;
 
-fn is_pallet_error(segments: &Vec<String>) -> bool {
+fn is_pallet_error(segments: &[String]) -> bool {
     let pallet_list: Vec<&str> = vec![
         "pallet_admin_utils",
         "pallet_collective",
@@ -27,29 +27,23 @@ fn test_metadata() {
     // current metadata version should be 14
     assert!(matches!(metadata, RuntimeMetadata::V14(_)));
 
-    match metadata {
-        RuntimeMetadata::V14(value) => {
-            let types = value.types.types;
-            for ty in types.iter() {
-                let segments = &ty.ty.path.segments;
-                if is_pallet_error(segments) {
-                    // error call and event should be enum type
-                    assert!(matches!(ty.ty.type_def, TypeDef::Variant(_)));
-                    match &ty.ty.type_def {
-                        TypeDef::Variant(variants) => {
-                            // check docs not empty
-                            for variant in variants.variants.iter() {
-                                // print name make it easier to find out failed item
-                                println!("{}", variant.name);
-                                assert!(variant.docs.len() > 0);
-                                assert!(!variant.docs[0].is_empty());
-                            }
-                        }
-                        _ => {}
+    if let RuntimeMetadata::V14(value) = metadata {
+        let types = value.types.types;
+        for ty in types.iter() {
+            let segments = &ty.ty.path.segments;
+            if is_pallet_error(segments) {
+                // error call and event should be enum type
+                assert!(matches!(ty.ty.type_def, TypeDef::Variant(_)));
+                if let TypeDef::Variant(variants) = &ty.ty.type_def {
+                    // check docs not empty
+                    for variant in variants.variants.iter() {
+                        // print name make it easier to find out failed item
+                        println!("{}", variant.name);
+                        assert!(!variant.docs.is_empty());
+                        assert!(!variant.docs[0].is_empty());
                     }
                 }
             }
         }
-        _ => {}
     };
 }