From 77fd4791f90bd689292541e136d9ebd345627d37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tuomas=20M=C3=A4kinen?= <1947505+tuommaki@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:58:03 +0200 Subject: [PATCH] Address all warnings/errors from `clippy` (#27) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tuomas Mäkinen --- crates/node/src/asset_manager/mod.rs | 6 +-- crates/node/src/cli.rs | 1 + crates/node/src/main.rs | 4 +- crates/node/src/mempool/mod.rs | 2 +- crates/node/src/networking/p2p.rs | 2 +- crates/node/src/rpc_client/mod.rs | 6 +-- crates/node/src/rpc_server/mod.rs | 6 +-- crates/node/src/scheduler/mod.rs | 8 +-- crates/node/src/storage/database/postgres.rs | 10 ++-- crates/node/src/types/hash.rs | 6 +-- crates/node/src/types/key_capsule.rs | 2 +- crates/node/src/types/rpc.rs | 2 +- crates/node/src/types/transaction.rs | 2 +- crates/node/src/workflow/mod.rs | 56 +++++++++----------- crates/tests/e2e-tests/src/main.rs | 10 ++-- crates/tests/e2e-tests/src/server.rs | 6 +-- 16 files changed, 61 insertions(+), 68 deletions(-) diff --git a/crates/node/src/asset_manager/mod.rs b/crates/node/src/asset_manager/mod.rs index e0333e2b..7c001dd4 100644 --- a/crates/node/src/asset_manager/mod.rs +++ b/crates/node/src/asset_manager/mod.rs @@ -98,7 +98,7 @@ impl AssetManager { prover, verifier, } => (Program::from(prover), Program::from(verifier)), - _ => return Err(AssetManagerError::IncompatibleTxPayload(tx.hash.clone()).into()), + _ => return Err(AssetManagerError::IncompatibleTxPayload(tx.hash).into()), }; self.process_program(&prover).await?; @@ -125,7 +125,7 @@ impl AssetManager { let workflow = match tx.payload.clone() { Payload::Run { workflow } => workflow, - _ => return Err(AssetManagerError::IncompatibleTxPayload(tx.hash.clone()).into()), + _ => return Err(AssetManagerError::IncompatibleTxPayload(tx.hash).into()), }; // TODO: Ideally the following would happen concurrently for each file... @@ -138,7 +138,7 @@ impl AssetManager { checksum, } => { let f = types::File { - tx: tx.hash.clone(), + tx: tx.hash, name: file_name, url: file_url, }; diff --git a/crates/node/src/cli.rs b/crates/node/src/cli.rs index 92646e2d..f894963f 100644 --- a/crates/node/src/cli.rs +++ b/crates/node/src/cli.rs @@ -147,6 +147,7 @@ pub enum GenerateCommand { }, } +#[allow(clippy::large_enum_variant)] #[derive(Debug, Subcommand)] pub enum Command { /// Generate objects. diff --git a/crates/node/src/main.rs b/crates/node/src/main.rs index 7e6c6fb6..4eaf74c0 100644 --- a/crates/node/src/main.rs +++ b/crates/node/src/main.rs @@ -242,8 +242,8 @@ async fn run(config: Arc) -> Result<()> { for addr in config.p2p_discovery_addrs.clone() { tracing::info!("connecting to p2p peer {}", addr); match addr.to_socket_addrs() { - Ok(socket_iter) => { - for peer in socket_iter { + Ok(mut socket_iter) => { + if let Some(peer) = socket_iter.next() { p2p.node().connect(peer).await?; break; } diff --git a/crates/node/src/mempool/mod.rs b/crates/node/src/mempool/mod.rs index b3acc938..285250c5 100644 --- a/crates/node/src/mempool/mod.rs +++ b/crates/node/src/mempool/mod.rs @@ -56,7 +56,7 @@ impl Mempool { // Broadcast new transaction to P2P network if it's configured. if let Some(ref tx_chan) = self.tx_chan { - if let Ok(_) = tx_chan.send_tx(&tx).await { + if tx_chan.send_tx(&tx).await.is_ok() { tx.propagated = true; self.storage.set(&tx).await?; } else { diff --git a/crates/node/src/networking/p2p.rs b/crates/node/src/networking/p2p.rs index 6cdf0013..0f849feb 100644 --- a/crates/node/src/networking/p2p.rs +++ b/crates/node/src/networking/p2p.rs @@ -161,7 +161,7 @@ mod tests { struct Sink(Arc>); impl Sink { fn new(tx: Arc>) -> Self { - Self { 0: tx } + Self(tx) } } diff --git a/crates/node/src/rpc_client/mod.rs b/crates/node/src/rpc_client/mod.rs index bfa81fd0..eaffb24b 100644 --- a/crates/node/src/rpc_client/mod.rs +++ b/crates/node/src/rpc_client/mod.rs @@ -24,7 +24,7 @@ impl RpcClient { tx_hash: &Hash, ) -> Result, Box> { let mut params = ArrayParams::new(); - params.insert(&tx_hash).expect("rpc params"); + params.insert(tx_hash).expect("rpc params"); let resp = self .client @@ -40,7 +40,7 @@ impl RpcClient { pub async fn send_transaction(&self, tx: &Transaction) -> Result<(), Box> { let mut params = ArrayParams::new(); - params.insert(&tx).expect("rpc params"); + params.insert(tx).expect("rpc params"); let resp = self .client @@ -57,7 +57,7 @@ impl RpcClient { pub async fn get_tx_tree(&self, tx_hash: &Hash) -> Result> { let mut params = ArrayParams::new(); - params.insert(&tx_hash).expect("rpc params"); + params.insert(tx_hash).expect("rpc params"); let resp = self .client diff --git a/crates/node/src/rpc_server/mod.rs b/crates/node/src/rpc_server/mod.rs index 251ce425..9716b962 100644 --- a/crates/node/src/rpc_server/mod.rs +++ b/crates/node/src/rpc_server/mod.rs @@ -116,13 +116,11 @@ async fn get_transaction(params: Params<'static>, ctx: Arc) -> RpcRespo tracing::info!("JSON-RPC: get_transaction()"); - let resp = match ctx.database.find_transaction(&tx_hash).await { + match ctx.database.find_transaction(&tx_hash).await { Ok(Some(tx)) => RpcResponse::Ok(tx), Ok(None) => RpcResponse::Err(RpcError::NotFound(tx_hash.to_string())), Err(e) => RpcResponse::Err(RpcError::NotFound(tx_hash.to_string())), - }; - - resp + } } #[tracing::instrument(level = "info")] diff --git a/crates/node/src/scheduler/mod.rs b/crates/node/src/scheduler/mod.rs index bb2ece98..dc20be8a 100644 --- a/crates/node/src/scheduler/mod.rs +++ b/crates/node/src/scheduler/mod.rs @@ -298,8 +298,8 @@ impl TaskManager for Scheduler { TaskKind::Proof => Transaction { hash: Hash::default(), payload: Payload::Proof { - parent: running_task.task.tx.clone(), - prover: program.clone(), + parent: running_task.task.tx, + prover: program, proof: result.data, }, nonce, @@ -309,8 +309,8 @@ impl TaskManager for Scheduler { TaskKind::Verification => Transaction { hash: Hash::default(), payload: Payload::Verification { - parent: running_task.task.tx.clone(), - verifier: program.clone(), + parent: running_task.task.tx, + verifier: program, verification: result.data, }, nonce, diff --git a/crates/node/src/storage/database/postgres.rs b/crates/node/src/storage/database/postgres.rs index 447ea319..ec480b6b 100644 --- a/crates/node/src/storage/database/postgres.rs +++ b/crates/node/src/storage/database/postgres.rs @@ -168,7 +168,7 @@ impl Database { .fetch_optional(&self.pool) .await?; - return Ok(res.is_some()); + Ok(res.is_some()) } pub async fn get_incomplete_assets(&self) -> Result> { @@ -352,8 +352,8 @@ impl Database { let mut txs = Vec::with_capacity(refs.len()); for tx_hash in refs { let tx = self.find_transaction(&tx_hash).await?; - if tx.is_some() { - txs.push(tx.unwrap()); + if let Some(tx) = tx { + txs.push(tx); } } @@ -494,11 +494,11 @@ impl Database { let mut db_tx = self.pool.begin().await?; sqlx::query("DELETE FROM program USING deploy WHERE (program.hash = deploy.prover OR program.hash = deploy.verifier) AND deploy.tx = $1") - .bind(&tx_hash) + .bind(tx_hash) .execute(&mut *db_tx) .await?; sqlx::query("DELETE FROM transaction WHERE hash = $1") - .bind(&tx_hash) + .bind(tx_hash) .execute(&mut *db_tx) .await?; diff --git a/crates/node/src/types/hash.rs b/crates/node/src/types/hash.rs index 89ea7501..3b769e88 100644 --- a/crates/node/src/types/hash.rs +++ b/crates/node/src/types/hash.rs @@ -17,7 +17,7 @@ impl Hash { pub fn random(rng: &mut R) -> Hash { let mut bs = [0u8; HASH_SIZE]; rng.fill_bytes(&mut bs); - Hash { 0: bs } + Hash(bs) } } @@ -58,9 +58,7 @@ impl From for Hash { impl From<&[u8]> for Hash { fn from(value: &[u8]) -> Self { - Self { - 0: value.try_into().expect("copy"), - } + Self(value.try_into().expect("copy")) } } diff --git a/crates/node/src/types/key_capsule.rs b/crates/node/src/types/key_capsule.rs index b75aca10..aeefb3aa 100644 --- a/crates/node/src/types/key_capsule.rs +++ b/crates/node/src/types/key_capsule.rs @@ -35,7 +35,7 @@ impl KeyCapsule { match self.keys.iter().find(|(recipient, _)| recipient == pk) { Some((_, key)) => { - let key = ecies::decrypt(sk, &key)?; + let key = ecies::decrypt(sk, key)?; let msg = ecies::decrypt(&key, self.msg.as_slice())?; Ok(msg.to_vec()) } diff --git a/crates/node/src/types/rpc.rs b/crates/node/src/types/rpc.rs index c2e05695..6f2295da 100644 --- a/crates/node/src/types/rpc.rs +++ b/crates/node/src/types/rpc.rs @@ -23,7 +23,7 @@ pub enum RpcResponse { impl RpcResponse { pub fn unwrap(&self) -> T { if let RpcResponse::Ok(v) = self { - return v.clone(); + v.clone() } else { // TODO: Consider better debug print here? panic!("unwrap"); diff --git a/crates/node/src/types/transaction.rs b/crates/node/src/types/transaction.rs index 2cce92a5..5c1de4d9 100644 --- a/crates/node/src/types/transaction.rs +++ b/crates/node/src/types/transaction.rs @@ -214,7 +214,7 @@ impl Payload { } => { buf.append(&mut parent.to_vec()); buf.append(&mut verifier.to_vec()); - buf.append(&mut verification.clone().as_mut()); + buf.append(verification.clone().as_mut()); } Payload::Cancel { parent } => { buf.append(&mut parent.to_vec()); diff --git a/crates/node/src/workflow/mod.rs b/crates/node/src/workflow/mod.rs index 7bab3c0a..79c6891d 100644 --- a/crates/node/src/workflow/mod.rs +++ b/crates/node/src/workflow/mod.rs @@ -52,12 +52,12 @@ impl WorkflowEngine { Payload::Run { workflow } => { tracing::debug!("creating next task from Run tx {}", &cur_tx.hash); - if workflow.steps.len() == 0 { + if workflow.steps.is_empty() { Ok(None) } else { Ok(Some( self.workflow_step_to_task( - cur_tx.hash.clone(), + cur_tx.hash, &workflow.steps[0], TaskKind::Proof, ) @@ -72,11 +72,7 @@ impl WorkflowEngine { } => { tracing::debug!("creating next task from Proof tx {}", &cur_tx.hash); - match workflow - .steps - .iter() - .position(|s| s.program == prover.clone()) - { + match workflow.steps.iter().position(|s| s.program == *prover) { Some(proof_step_idx) => { if workflow.steps.len() <= proof_step_idx { Err(WorkflowError::WorkflowStepMissing(format!( @@ -87,7 +83,7 @@ impl WorkflowEngine { } else { Ok(Some( self.workflow_step_to_task( - cur_tx.hash.clone(), + cur_tx.hash, &workflow.steps[proof_step_idx + 1], TaskKind::Verification, ) @@ -109,12 +105,12 @@ impl WorkflowEngine { Ok(None) => { return Err(WorkflowError::WorkflowTransactionMissing(format!( "Proof tx, hash {}", - parent.to_string() + parent )) .into()); } Ok(Some(tx)) => tx, - Err(err) => return Err(err.into()), + Err(err) => return Err(err), }; // TODO: Rewrite this spaghetti! @@ -136,7 +132,7 @@ impl WorkflowEngine { } else { Ok(Some( self.workflow_step_to_task( - proof_tx.hash.clone(), + proof_tx.hash, &workflow.steps[proof_step_idx + 1], TaskKind::Verification, ) @@ -162,7 +158,7 @@ impl WorkflowEngine { } async fn find_parent_tx_for_program(&self, tx_hash: &Hash, program: &Hash) -> Result { - let mut cur_tx = tx_hash.clone(); + let mut cur_tx = *tx_hash; tracing::debug!("finding workflow for transaction {}", tx_hash); @@ -180,7 +176,7 @@ impl WorkflowEngine { return Err(WorkflowError::TransactionNotFound(cur_tx).into()); } - if workflow.steps.get(0).unwrap().program == *program { + if workflow.steps.first().unwrap().program == *program { return Ok(cur_tx); } else { return Err(WorkflowError::TransactionNotFound(cur_tx).into()); @@ -191,11 +187,11 @@ impl WorkflowEngine { return Ok(cur_tx); } - cur_tx = parent.clone(); + cur_tx = parent; continue; } Payload::ProofKey { parent, .. } => { - cur_tx = parent.clone(); + cur_tx = parent; continue; } Payload::Verification { @@ -205,7 +201,7 @@ impl WorkflowEngine { return Ok(cur_tx); } - cur_tx = parent.clone(); + cur_tx = parent; continue; } _ => { @@ -220,7 +216,7 @@ impl WorkflowEngine { } async fn workflow_for_transaction(&self, tx_hash: &Hash) -> Result { - let mut tx_hash = tx_hash.clone(); + let mut tx_hash = *tx_hash; tracing::debug!("finding workflow for transaction {}", tx_hash); @@ -240,17 +236,17 @@ impl WorkflowEngine { } Payload::Proof { parent, .. } => { tracing::debug!("finding workflow from parent {} of {}", &parent, tx_hash); - tx_hash = parent.clone(); + tx_hash = parent; continue; } Payload::ProofKey { parent, .. } => { tracing::debug!("finding workflow from parent {} of {}", &parent, tx_hash); - tx_hash = parent.clone(); + tx_hash = parent; continue; } Payload::Verification { parent, .. } => { tracing::debug!("finding workflow from parent {} of {}", &parent, tx_hash); - tx_hash = parent.clone(); + tx_hash = parent; continue; } _ => { @@ -281,7 +277,7 @@ impl WorkflowEngine { file_url, .. } => File { - tx: tx.clone(), + tx, name: file_name.clone(), url: file_url.clone(), }, @@ -290,10 +286,10 @@ impl WorkflowEngine { file_name, } => { // Make record of file that needs transfer from source tx to current tx's files. - file_transfers.push((source_program.clone(), file_name.clone())); + file_transfers.push((*source_program, file_name.clone())); File { - tx: tx.clone(), + tx, name: file_name.clone(), url: "".to_string(), } @@ -316,7 +312,7 @@ impl WorkflowEngine { Ok(Task { id, tx, - name: format!("{}-{}", id.to_string(), step.program.to_string()), + name: format!("{}-{}", id, step.program), kind, program_id: step.program, args: step.args.clone(), @@ -358,7 +354,7 @@ mod tests { #[async_trait] impl TransactionStore for TxStore { async fn find_transaction(&self, tx_hash: &Hash) -> Result> { - Ok(self.txs.get(tx_hash).map(|e| e.clone())) + Ok(self.txs.get(tx_hash).cloned()) } } @@ -467,8 +463,8 @@ mod tests { let mut tx = Transaction { hash: Hash::default(), payload: Payload::Proof { - parent: parent.clone(), - prover: program.clone(), + parent: *parent, + prover: *program, proof: "proof.".into(), }, nonce: 1, @@ -485,7 +481,7 @@ mod tests { let mut tx = Transaction { hash: Hash::default(), payload: Payload::ProofKey { - parent: parent.clone(), + parent: *parent, key: "key.".into(), }, nonce: 1, @@ -502,8 +498,8 @@ mod tests { let mut tx = Transaction { hash: Hash::default(), payload: Payload::Verification { - parent: parent.clone(), - verifier: program.clone(), + parent: *parent, + verifier: *program, verification: b"verification.".to_vec(), }, nonce: 1, diff --git a/crates/tests/e2e-tests/src/main.rs b/crates/tests/e2e-tests/src/main.rs index 2ea7c14c..2f68a000 100644 --- a/crates/tests/e2e-tests/src/main.rs +++ b/crates/tests/e2e-tests/src/main.rs @@ -70,8 +70,8 @@ async fn deploy_programs( file_server: Arc, key: &SecretKey, deployment_name: &str, - prover_img: &PathBuf, - verifier_img: &PathBuf, + prover_img: &Path, + verifier_img: &Path, ) -> Result<(Hash, Hash, Hash)> { let prover = from_img_file_to_metadata(prover_img, &file_server.register_file(prover_img).await); @@ -114,16 +114,16 @@ async fn send_proving_task( verifier_hash: &Hash, ) -> Result { let proving_step = WorkflowStep { - program: prover_hash.clone(), + program: *prover_hash, args: vec!["--nonce".to_string(), nonce.to_string()], inputs: vec![], }; let verifying_step = WorkflowStep { - program: verifier_hash.clone(), + program: *verifier_hash, args: vec!["--nonce".to_string(), nonce.to_string()], inputs: vec![ProgramData::Output { - source_program: prover_hash.clone(), + source_program: *prover_hash, file_name: "/workspace/proof.dat".to_string(), }], }; diff --git a/crates/tests/e2e-tests/src/server.rs b/crates/tests/e2e-tests/src/server.rs index cf44b05f..67877405 100644 --- a/crates/tests/e2e-tests/src/server.rs +++ b/crates/tests/e2e-tests/src/server.rs @@ -9,7 +9,7 @@ use hyper_util::rt::TokioIo; use sha3::{Digest, Sha3_256}; use std::collections::HashMap; use std::net::SocketAddr; -use std::path::PathBuf; +use std::path::Path; use std::sync::Arc; use tokio::fs::File; use tokio::net::TcpListener; @@ -36,13 +36,13 @@ impl FileServer { server } - pub async fn register_file(&self, file: &PathBuf) -> String { + pub async fn register_file(&self, file: &Path) -> String { let file_name = file.to_string_lossy().to_string(); let mut file_map = self.file_map.write().await; let mut hasher = Sha3_256::new(); hasher.update(file_name.as_bytes()); let digest = hex::encode(hasher.finalize()); - let url = format!("http://{}/{}", self.listen_addr.to_string(), digest); + let url = format!("http://{}/{}", self.listen_addr, digest); file_map.insert(digest, file_name); url }