Merge pull request #155 from semiotic-ai/ci_clippy_fail

ci: fix clippy

Author: aasseman

.github/workflows/tests.yml

@@ -35,7 +35,7 @@ jobs:
           key: ${{ runner.os }}-cargo-clippy
       - run: |
            rustup component add clippy
-           cargo clippy --all-features -- -D clippy::all
+           cargo clippy --all-targets --all-features -- -D warnings
   
   test-and-coverage:
     name: cargo test and coverage

tap_aggregator/src/aggregator.rs

@@ -149,7 +149,7 @@ mod tests {
                 .await
                 .unwrap();
         receipts.push(receipt.clone());
-        receipts.push(receipt.clone());
+        receipts.push(receipt);
 
         let res = aggregator::check_signatures_unique(&receipts);
         assert!(res.is_err());

tap_aggregator/src/main.rs

@@ -8,7 +8,7 @@ use clap::Parser;
 use ethers_signers::{coins_bip39::English, MnemonicBuilder};
 use tokio::signal::unix::{signal, SignalKind};
 
-use log::{debug, info, warn};
+use log::{debug, info};
 use tap_aggregator::metrics;
 use tap_aggregator::server;
 

tap_core/src/adapters/test/receipt_checks_adapter_test.rs

@@ -68,10 +68,7 @@ mod receipt_checks_adapter_unit_test {
             .await;
         let receipt_storage = Arc::new(RwLock::new(receipts));
 
-        let query_appraisals = (0..11)
-            .into_iter()
-            .map(|id| (id, 100u128))
-            .collect::<HashMap<_, _>>();
+        let query_appraisals = (0..11).map(|id| (id, 100u128)).collect::<HashMap<_, _>>();
 
         let query_appraisals_storage = Arc::new(RwLock::new(query_appraisals));
 

tap_core/src/tap_manager/test/manager_test.rs

@@ -1,8 +1,8 @@
 // Copyright 2023-, Semiotic AI, Inc.
 // SPDX-License-Identifier: Apache-2.0
-#[cfg(test)]
 
-mod manager_test {
+#[cfg(test)]
+mod manager_unit_test {
     use std::{
         collections::{HashMap, HashSet},
         str::FromStr,

tap_integration_tests/clippy.toml

@@ -0,0 +1,2 @@
+# Since this whole crate is only for testing, we relax some of the clippy rules
+too-many-arguments-threshold = 15

tap_integration_tests/tests/indexer_mock/mod.rs

@@ -96,7 +96,7 @@ impl<
             receipt_count: Arc::new(AtomicU64::new(0)),
             threshold,
             aggregator_client: (
-                HttpClientBuilder::default().build(format!("{}", aggregate_server_address))?,
+                HttpClientBuilder::default().build(aggregate_server_address)?,
                 aggregate_server_api_version,
             ),
         })
@@ -234,8 +234,7 @@ async fn request_rav<
         .await?;
     {
         let mut manager_guard = manager.lock().await;
-        let _result =
-            manager_guard.verify_and_store_rav(rav_request.expected_rav, remote_rav_result.data)?;
+        manager_guard.verify_and_store_rav(rav_request.expected_rav, remote_rav_result.data)?;
     }
 
     // For these tests, we expect every receipt to be valid, i.e. there should be no invalid receipts, nor any missing receipts (less than the expected threshold).
@@ -259,5 +258,5 @@ fn get_current_timestamp_u64_ns() -> Result<u64> {
 }
 
 fn to_rpc_error(e: Box<dyn std::error::Error>, msg: &str) -> jsonrpsee::types::ErrorObjectOwned {
-    jsonrpsee::types::ErrorObject::owned(-32000, format!("{} - {}", e.to_string(), msg), None::<()>)
+    jsonrpsee::types::ErrorObject::owned(-32000, format!("{} - {}", e, msg), None::<()>)
 }

tap_integration_tests/tests/showcase.rs

@@ -639,22 +639,24 @@ async fn test_manager_wrong_aggregator_keys(
 
     let mut counter = 1;
     for (receipt_1, id) in requests {
-        let result = client_1.request("request", (id, receipt_1)).await;
+        let result: Result<(), jsonrpsee::core::Error> =
+            client_1.request("request", (id, receipt_1)).await;
         // The rav request is being made with messages that have been signed with a key that differs from the gateway aggregator's.
         // So the Gateway Aggregator should send an error to the requesting Indexer.
         // And so the Indexer should then return an error to the clinet when a rav request is made.
         // A rav request is made when the number of receipts sent = receipt_threshold_1.
         // result should be an error when counter = multiple of receipt_threshold_1 and Ok otherwise.
         if (counter % receipt_threshold_1) == 0 {
-            match result {
-                Ok(()) => panic!("Gateway Aggregator should have sent an error to the Indexer."),
-                Err(_) => {}
-            }
+            assert!(
+                result.is_err(),
+                "Gateway Aggregator should have sent an error to the Indexer."
+            );
         } else {
-            match result {
-                Ok(()) => {}
-                Err(e) => panic!("Error making receipt request: {:?}", e),
-            }
+            assert!(
+                result.is_ok(),
+                "Error making receipt request: {:?}",
+                result.unwrap_err()
+            );
         }
         counter += 1;
     }
@@ -680,21 +682,20 @@ async fn test_manager_wrong_requestor_keys(
 
     let mut counter = 1;
     for (receipt_1, id) in requests {
-        let result = client_1.request("request", (id, receipt_1)).await;
+        let result: Result<(), jsonrpsee::core::Error> =
+            client_1.request("request", (id, receipt_1)).await;
         // The receipts have been signed with a key that the Indexer is not expecting.
         // So the Indexer should return an error when a rav request is made, because they will not have any valid receipts for the request.
         // A rav request is made when the number of receipts sent = receipt_threshold_1.
         // result should be an error when counter = multiple of receipt_threshold_1 and Ok otherwise.
         if (counter % receipt_threshold_1) == 0 {
-            match result {
-                Ok(()) => panic!("Should have failed signature verification"),
-                Err(_) => {}
-            }
+            assert!(result.is_err(), "Should have failed signature verification");
         } else {
-            match result {
-                Ok(()) => {}
-                Err(e) => panic!("Error making receipt request: {:?}", e),
-            }
+            assert!(
+                result.is_ok(),
+                "Error making receipt request: {:?}",
+                result.unwrap_err()
+            );
         }
         counter += 1;
     }
@@ -740,22 +741,21 @@ async fn test_tap_manager_rav_timestamp_cuttoff(
 
     let mut counter = 1;
     for (receipt_1, id) in requests {
-        let result = client_1.request("request", (id, receipt_1)).await;
+        let result: Result<(), jsonrpsee::core::Error> =
+            client_1.request("request", (id, receipt_1)).await;
 
         // The first receipt in the second batch has the same timestamp as the last receipt in the first batch.
         // TAP manager should ignore this receipt when creating the second RAV request.
         // The indexer_mock will throw an error if the number of receipts in RAV request is less than the expected number.
         // An error is expected when requesting the second RAV.
         if counter == 2 * receipt_threshold_1 {
-            match result {
-                Ok(()) => panic!("Should have failed RAV request"),
-                Err(_) => {}
-            }
+            assert!(result.is_err(), "Should have failed RAV request");
         } else {
-            match result {
-                Ok(()) => {}
-                Err(e) => panic!("Error making receipt request: {:?}", e),
-            }
+            assert!(
+                result.is_ok(),
+                "Error making receipt request: {:?}",
+                result.unwrap_err()
+            );
         }
         counter += 1;
     }
@@ -796,8 +796,7 @@ async fn test_tap_aggregator_rav_timestamp_cuttoff(
         http_max_concurrent_connections,
     )
     .await?;
-    let client =
-        HttpClientBuilder::default().build(format!("http://{}", gateway_addr.to_string()))?;
+    let client = HttpClientBuilder::default().build(format!("http://{}", gateway_addr))?;
 
     // This is the first part of the test, two batches of receipts are sent to the aggregator.
     // The second batch has one receipt with the same timestamp as the latest receipt in the first batch.
@@ -828,10 +827,10 @@ async fn test_tap_aggregator_rav_timestamp_cuttoff(
         jsonrpsee_helpers::JsonRpcResponse<SignedRAV>,
         jsonrpsee::core::Error,
     > = client.request("aggregate_receipts", params).await;
-    match second_rav_response {
-        Ok(_) => panic!("Should have failed RAV request"),
-        Err(_) => {}
-    }
+    assert!(
+        second_rav_response.is_err(),
+        "Should have failed RAV request"
+    );
 
     // This is the second part of the test, two batches of receipts are sent to the aggregator.
     // The second batch has one receipt with the timestamp = timestamp+1 of the latest receipt in the first batch.