chore(*): Add msim clippy check and fix clippy lints (#4180)

* chore(*): Add msim clippy check and fix clippy lints

* Update config patch

* fix script

* 1.81 fixes and remove copyright

* review

.github/workflows/_rust_lints.yml

@@ -62,3 +62,8 @@ jobs:
       # See '.cargo/config' for list of enabled/disabled clippy lints
       - name: Check Clippy Lints
         run: cargo ci-clippy
+
+      - name: Check Clippy Lints with msim
+        run: |
+          git clean -fd
+          ./scripts/simtest/clippy.sh

Cargo.toml

@@ -461,5 +461,3 @@ transaction-fuzzer = { path = "crates/transaction-fuzzer" }
 typed-store = { path = "crates/typed-store" }
 typed-store-derive = { path = "crates/typed-store-derive" }
 typed-store-error = { path = "crates/typed-store-error" }
-
-[patch.crates-io]

crates/iota-benchmark/tests/simtest.rs

@@ -569,7 +569,7 @@ mod test {
                     .collect()
             })
             .unwrap_or_else(|_| vec![]);
-        assert!(checkpoint_files.len() > 0);
+        assert!(!checkpoint_files.is_empty());
         let bytes = std::fs::read(checkpoint_files.first().unwrap()).unwrap();
 
         let _checkpoint: CheckpointData =
@@ -792,11 +792,7 @@ mod test {
             return false; // don't fail ineligible nodes
         }
         let mut rng = thread_rng();
-        if rng.gen_range(0.0..1.0) < probability {
-            true
-        } else {
-            false
-        }
+        rng.gen_range(0.0..1.0) < probability
     }
 
     async fn build_test_cluster(
@@ -901,7 +897,7 @@ mod test {
         let bank = BenchmarkBank::new(proxy.clone(), primary_coin);
         let system_state_observer = {
             let mut system_state_observer = SystemStateObserver::new(proxy.clone());
-            if let Ok(_) = system_state_observer.state.changed().await {
+            if system_state_observer.state.changed().await.is_ok() {
                 info!(
                     "Got the new state (reference gas price and/or protocol config) from system state object"
                 );

crates/iota-config/src/local_ip_utils.rs

@@ -18,13 +18,20 @@ pub struct SimAddressManager {
 }
 
 #[cfg(msim)]
-impl SimAddressManager {
-    pub fn new() -> Self {
+impl Default for SimAddressManager {
+    fn default() -> Self {
         Self {
             next_ip_offset: AtomicI16::new(1),
             next_port: AtomicI16::new(9000),
         }
     }
+}
+
+#[cfg(msim)]
+impl SimAddressManager {
+    pub fn new() -> Self {
+        Self::default()
+    }
 
     pub fn get_next_ip(&self) -> String {
         let offset = self

crates/iota-core/src/authority.rs

@@ -1284,7 +1284,7 @@ impl AuthorityState {
         let digest = *certificate.digest();
 
         fail_point_if!("correlated-crash-process-certificate", || {
-            if iota_simulator::random::deterministic_probability_once(&digest, 0.01) {
+            if iota_simulator::random::deterministic_probability_once(digest, 0.01) {
                 iota_simulator::task::kill_current_node(None);
             }
         });
@@ -4185,7 +4185,7 @@ impl AuthorityState {
         #[cfg(msim)]
         let extra_packages = framework_injection::get_extra_packages(self.name);
         #[cfg(msim)]
-        let system_packages = system_packages.map(|p| p).chain(extra_packages.iter());
+        let system_packages = system_packages.chain(&extra_packages);
 
         for system_package in system_packages {
             let modules = system_package.modules().to_vec();
@@ -5105,7 +5105,7 @@ pub mod framework_injection {
     }
 
     pub fn get_extra_packages(name: AuthorityName) -> Vec<SystemPackage> {
-        let built_in = BTreeSet::from_iter(BuiltInFramework::all_package_ids().into_iter());
+        let built_in = BTreeSet::from_iter(BuiltInFramework::all_package_ids());
         let extra: Vec<ObjectID> = OVERRIDE.with(|cfg| {
             cfg.borrow()
                 .keys()

crates/iota-core/src/checkpoints/mod.rs

@@ -1698,18 +1698,12 @@ impl CheckpointBuilder {
         let ccps = root_txs
             .iter()
             .filter_map(|tx| {
-                if let Some(tx) = tx {
-                    if matches!(
+                tx.as_ref().filter(|tx| {
+                    matches!(
                         tx.transaction_data().kind(),
                         TransactionKind::ConsensusCommitPrologueV1(_)
-                    ) {
-                        Some(tx)
-                    } else {
-                        None
-                    }
-                } else {
-                    None
-                }
+                    )
+                })
             })
             .collect::<Vec<_>>();
 
@@ -1724,22 +1718,20 @@ impl CheckpointBuilder {
             .multi_get_transaction_blocks(
                 &sorted
                     .iter()
-                    .map(|tx| tx.transaction_digest().clone())
+                    .map(|tx| *tx.transaction_digest())
                     .collect::<Vec<_>>(),
             )
             .unwrap();
 
-        if ccps.len() == 0 {
+        if ccps.is_empty() {
             // If there is no consensus commit prologue transaction in the roots, then there
             // should be no consensus commit prologue transaction in the
             // checkpoint.
-            for tx in txs.iter() {
-                if let Some(tx) = tx {
-                    assert!(!matches!(
-                        tx.transaction_data().kind(),
-                        TransactionKind::ConsensusCommitPrologueV1(_)
-                    ));
-                }
+            for tx in txs.iter().flatten() {
+                assert!(!matches!(
+                    tx.transaction_data().kind(),
+                    TransactionKind::ConsensusCommitPrologueV1(_)
+                ));
             }
         } else {
             // If there is one consensus commit prologue, it must be the first one in the
@@ -1751,13 +1743,11 @@ impl CheckpointBuilder {
 
             assert_eq!(ccps[0].digest(), txs[0].as_ref().unwrap().digest());
 
-            for tx in txs.iter().skip(1) {
-                if let Some(tx) = tx {
-                    assert!(!matches!(
-                        tx.transaction_data().kind(),
-                        TransactionKind::ConsensusCommitPrologueV1(_)
-                    ));
-                }
+            for tx in txs.iter().skip(1).flatten() {
+                assert!(!matches!(
+                    tx.transaction_data().kind(),
+                    TransactionKind::ConsensusCommitPrologueV1(_)
+                ));
             }
         }
     }

crates/iota-core/src/consensus_handler.rs

@@ -403,7 +403,7 @@ impl<C: CheckpointServiceNotify + Send + Sync> ConsensusHandler<C> {
 
         fail_point_if!("correlated-crash-after-consensus-commit-boundary", || {
             let key = [commit_sub_dag_index, self.epoch_store.epoch()];
-            if iota_simulator::random::deterministic_probability(&key, 0.01) {
+            if iota_simulator::random::deterministic_probability(key, 0.01) {
                 iota_simulator::task::kill_current_node(None);
             }
         });

crates/iota-e2e-tests/tests/object_deletion_tests.rs

@@ -30,10 +30,9 @@ mod sim_only_tests {
         // root object.
         let (package_id, object_id) = publish_package_and_create_parent_object(&test_cluster).await;
         let child_id = create_owned_child(&test_cluster, package_id).await;
-        let wrap_child_txn_digest = wrap_child(&test_cluster, package_id, object_id, child_id)
+        let wrap_child_txn_digest = *wrap_child(&test_cluster, package_id, object_id, child_id)
             .await
-            .transaction_digest()
-            .clone();
+            .transaction_digest();
 
         fullnode
             .with_async(|node| async {
@@ -46,7 +45,7 @@ mod sim_only_tests {
                 .unwrap();
 
                 // Wait until the above checkpoint is pruned.
-                let _ = timeout(
+                timeout(
                     Duration::from_secs(60),
                     wait_until_checkpoint_pruned(node, checkpoint),
                 )
@@ -80,14 +79,12 @@ mod sim_only_tests {
         // Next, we unwrap and delete the child object, as well as delete the root
         // object.
         let unwrap_delete_txn_digest =
-            unwrap_and_delete_child(&test_cluster, package_id, object_id)
+            *unwrap_and_delete_child(&test_cluster, package_id, object_id)
                 .await
-                .transaction_digest()
-                .clone();
-        let delete_root_obj_txn_digest = delete_object(&test_cluster, package_id, object_id)
+                .transaction_digest();
+        let delete_root_obj_txn_digest = *delete_object(&test_cluster, package_id, object_id)
             .await
-            .transaction_digest()
-            .clone();
+            .transaction_digest();
 
         fullnode
             .with_async(|node| async {
@@ -105,7 +102,7 @@ mod sim_only_tests {
                 .await
                 .unwrap();
 
-                let _ = timeout(
+                timeout(
                     Duration::from_secs(60),
                     wait_until_checkpoint_pruned(node, std::cmp::max(checkpoint1, checkpoint2)),
                 )
@@ -269,7 +266,7 @@ mod sim_only_tests {
             if let Some(seq) = node
                 .state()
                 .epoch_store_for_testing()
-                .get_transaction_checkpoint(&digest)
+                .get_transaction_checkpoint(digest)
                 .unwrap()
             {
                 return seq;

crates/iota-e2e-tests/tests/protocol_version_tests.rs

@@ -129,7 +129,7 @@ mod sim_only_tests {
             .build()
             .await;
 
-        let validator = test_cluster.get_validator_pubkeys()[0].clone();
+        let validator = test_cluster.get_validator_pubkeys()[0];
         test_cluster.stop_node(&validator);
 
         assert_eq!(
@@ -482,7 +482,7 @@ mod sim_only_tests {
     }
 
     async fn create_obj(cluster: &TestCluster) -> ObjectRef {
-        execute_creating(cluster, {
+        *execute_creating(cluster, {
             let mut builder = ProgrammableTransactionBuilder::new();
             builder
                 .move_call(
@@ -500,11 +500,10 @@ mod sim_only_tests {
         .await
         .first()
         .unwrap()
-        .clone()
     }
 
     async fn wrap_obj(cluster: &TestCluster, obj: ObjectRef) -> ObjectRef {
-        execute_creating(cluster, {
+        *execute_creating(cluster, {
             let mut builder = ProgrammableTransactionBuilder::new();
             builder
                 .move_call(
@@ -521,7 +520,6 @@ mod sim_only_tests {
         .await
         .first()
         .unwrap()
-        .clone()
     }
 
     async fn transfer_obj(
@@ -569,9 +567,9 @@ mod sim_only_tests {
             .unwrap();
 
         let results = response.results.unwrap();
-        let return_ = &results.first().unwrap().return_values.first().unwrap().0;
+        let return_val = &results.first().unwrap().return_values.first().unwrap().0;
 
-        bcs::from_bytes(&return_).unwrap()
+        bcs::from_bytes(return_val).unwrap()
     }
 
     async fn execute_creating(
@@ -610,11 +608,11 @@ mod sim_only_tests {
     }
 
     async fn expect_upgrade_failed(cluster: &TestCluster) {
-        monitor_version_change(&cluster, START /* expected proto version */).await;
+        monitor_version_change(cluster, START /* expected proto version */).await;
     }
 
     async fn expect_upgrade_succeeded(cluster: &TestCluster) {
-        monitor_version_change(&cluster, FINISH /* expected proto version */).await;
+        monitor_version_change(cluster, FINISH /* expected proto version */).await;
     }
 
     async fn get_framework_upgrade_versions(

crates/iota-framework/src/lib.rs

@@ -106,7 +106,7 @@ macro_rules! define_system_packages {
 
 pub struct BuiltInFramework;
 impl BuiltInFramework {
-    pub fn iter_system_packages() -> impl Iterator<Item = &'static SystemPackage> {
+    pub fn iter_system_packages<'a>() -> impl Iterator<Item = &'a SystemPackage> {
         // All system packages in the current build should be registered here, and this
         // is the only place we need to worry about if any of them changes.
         // TODO: Is it possible to derive dependencies from the bytecode instead of

crates/iota-node/src/handle.rs

@@ -135,7 +135,9 @@ impl Drop for IotaNodeHandle {
     fn drop(&mut self) {
         if self.shutdown_on_drop {
             let node_id = self.inner().sim_state.sim_node.id();
-            iota_simulator::runtime::Handle::try_current().map(|h| h.delete_node(node_id));
+            if let Some(h) = iota_simulator::runtime::Handle::try_current() {
+                h.delete_node(node_id);
+            }
         }
     }
 }

crates/iota-protocol-config/src/lib.rs

@@ -1053,12 +1053,12 @@ impl ProtocolConfig {
 }
 
 #[cfg(not(msim))]
-static POISON_VERSION_METHODS: AtomicBool = AtomicBool::new(false);
+static POISON_VERSION_METHODS: AtomicBool = const { AtomicBool::new(false) };
 
 // Use a thread local in sim tests for test isolation.
 #[cfg(msim)]
 thread_local! {
-    static POISON_VERSION_METHODS: AtomicBool = AtomicBool::new(false);
+    static POISON_VERSION_METHODS: AtomicBool = const { AtomicBool::new(false) };
 }
 
 // Instantiations for each protocol version.

crates/iota-simulator/src/lib.rs

@@ -87,7 +87,7 @@ pub mod configs {
         env_configs: impl IntoIterator<Item = (&'static str, SimConfig)>,
     ) -> SimConfig {
         let mut env_configs = HashMap::<&'static str, SimConfig>::from_iter(env_configs);
-        if let Some(env) = std::env::var("IOTA_SIM_CONFIG").ok() {
+        if let Ok(env) = std::env::var("IOTA_SIM_CONFIG") {
             if let Some(cfg) = env_configs.remove(env.as_str()) {
                 info!("Using test config for IOTA_SIM_CONFIG={}", env);
                 cfg

crates/iota-storage/tests/key_value_tests.rs

@@ -439,10 +439,9 @@ mod simtests {
 
     use super::*;
 
-    async fn svc(
-        State(state): State<Arc<Mutex<HashMap<String, Vec<u8>>>>>,
-        request: Request<Body>,
-    ) -> Response {
+    type Storage = HashMap<String, Vec<u8>>;
+
+    async fn svc(State(state): State<Arc<Mutex<Storage>>>, request: Request<Body>) -> Response {
         let path = request.uri().path().to_string();
         let key = path.trim_start_matches('/');
         let value = state.lock().unwrap().get(key).cloned();
@@ -456,7 +455,7 @@ mod simtests {
         }
     }
 
-    async fn test_server(data: Arc<Mutex<HashMap<String, Vec<u8>>>>) {
+    async fn test_server(data: Arc<Mutex<Storage>>) {
         let handle = iota_simulator::runtime::Handle::current();
         let builder = handle.create_node();
         let (startup_sender, mut startup_receiver) = tokio::sync::watch::channel(false);

crates/iota-swarm/src/memory/container-sim.rs

@@ -34,7 +34,9 @@ impl Drop for Container {
     fn drop(&mut self) {
         if let Some(handle) = self.handle.take() {
             tracing::info!("shutting down {}", handle.node_id);
-            iota_simulator::runtime::Handle::try_current().map(|h| h.delete_node(handle.node_id));
+            if let Some(h) = iota_simulator::runtime::Handle::try_current() {
+                h.delete_node(handle.node_id);
+            }
         }
     }
 }
@@ -57,7 +59,7 @@ impl Container {
         let startup_sender = Arc::new(startup_sender);
         let node = builder
             .ip(ip)
-            .name(&format!("{:?}", config.authority_public_key().concise()))
+            .name(format!("{:?}", config.authority_public_key().concise()))
             .init(move || {
                 info!("Node restarted");
                 let config = config.clone();

crates/iota-types/src/iota_system_state/mod.rs

@@ -438,7 +438,7 @@ pub mod advance_epoch_result_injection {
 
     thread_local! {
         /// Override the result of advance_epoch in the range [start, end).
-        static OVERRIDE: RefCell<Option<(EpochId, EpochId)>>  = RefCell::new(None);
+        static OVERRIDE: RefCell<Option<(EpochId, EpochId)>>  = const { RefCell::new(None) };
     }
 
     /// Override the result of advance_epoch transaction if new epoch is in the