Fix clippy warnings (#1385)

## Issue Addressed

NA

## Proposed Changes

Fixes most clippy warnings and ignores the rest of them, see issue #1388.

Makefile

@@ -53,7 +53,7 @@ test-full: cargo-fmt test-release test-debug test-ef
 # Lints the code for bad style and potentially unsafe arithmetic using Clippy.
 # Clippy lints are opt-in per-crate for now. By default, everything is allowed except for performance and correctness lints.
 lint:
-	cargo clippy --all -- -A clippy::all --D clippy::perf --D clippy::correctness
+	cargo clippy --all -- -D warnings
 
 # Runs the makefile in the `ef_tests` repo.
 #

account_manager/src/upgrade_legacy_keypairs.rs

@@ -112,7 +112,7 @@ fn upgrade_keypair<P: AsRef<Path>>(
     let validator_dir = validator_dir.as_ref();
     let secrets_dir = secrets_dir.as_ref();
 
-    let keypair: Keypair = load_unencrypted_keypair(validator_dir.join(input_filename))?.into();
+    let keypair: Keypair = load_unencrypted_keypair(validator_dir.join(input_filename))?;
 
     let password = rand::thread_rng()
         .sample_iter(&Alphanumeric)
@@ -136,7 +136,7 @@ fn upgrade_keypair<P: AsRef<Path>>(
         .to_json_writer(&mut file)
         .map_err(|e| format!("Cannot write keystore to {:?}: {:?}", keystore_path, e))?;
 
-    let password_path = secrets_dir.join(format!("{}", keypair.pk.as_hex_string()));
+    let password_path = secrets_dir.join(keypair.pk.as_hex_string());
 
     if password_path.exists() {
         return Err(format!("{:?} already exists", password_path));

account_manager/src/validator/deposit.rs

@@ -1,6 +1,5 @@
 use crate::VALIDATOR_DIR_FLAG;
 use clap::{App, Arg, ArgMatches};
-use clap_utils;
 use deposit_contract::DEPOSIT_GAS;
 use environment::Environment;
 use futures::compat::Future01CompatExt;

account_manager/src/validator/mod.rs

@@ -28,11 +28,9 @@ pub fn cli_run<T: EthSpec>(matches: &ArgMatches, env: Environment<T>) -> Result<
     match matches.subcommand() {
         (create::CMD, Some(matches)) => create::cli_run::<T>(matches, env, base_wallet_dir),
         (deposit::CMD, Some(matches)) => deposit::cli_run::<T>(matches, env),
-        (unknown, _) => {
-            return Err(format!(
-                "{} does not have a {} command. See --help",
-                CMD, unknown
-            ));
-        }
+        (unknown, _) => Err(format!(
+            "{} does not have a {} command. See --help",
+            CMD, unknown
+        )),
     }
 }

account_manager/src/wallet/create.rs

@@ -117,24 +117,24 @@ pub fn cli_run(matches: &ArgMatches, base_dir: PathBuf) -> Result<(), String> {
     }
 
     println!("Your wallet's 12-word BIP-39 mnemonic is:");
-    println!("");
+    println!();
     println!("\t{}", mnemonic.phrase());
-    println!("");
+    println!();
     println!("This mnemonic can be used to fully restore your wallet, should ");
     println!("you lose the JSON file or your password. ");
-    println!("");
+    println!();
     println!("It is very important that you DO NOT SHARE this mnemonic as it will ");
     println!("reveal the private keys of all validators and keys generated with  ");
     println!("this wallet. That would be catastrophic.");
-    println!("");
+    println!();
     println!("It is also important to store a backup of this mnemonic so you can ");
     println!("recover your private keys in the case of data loss. Writing it on ");
     println!("a piece of paper and storing it in a safe place would be prudent.");
-    println!("");
+    println!();
     println!("Your wallet's UUID is:");
-    println!("");
+    println!();
     println!("\t{}", wallet.wallet().uuid());
-    println!("");
+    println!();
     println!("You do not need to backup your UUID or keep it secret.");
 
     Ok(())

account_manager/src/wallet/mod.rs

@@ -30,11 +30,9 @@ pub fn cli_run(matches: &ArgMatches) -> Result<(), String> {
     match matches.subcommand() {
         (create::CMD, Some(matches)) => create::cli_run(matches, base_dir),
         (list::CMD, Some(_)) => list::cli_run(base_dir),
-        (unknown, _) => {
-            return Err(format!(
-                "{} does not have a {} command. See --help",
-                CMD, unknown
-            ));
-        }
+        (unknown, _) => Err(format!(
+            "{} does not have a {} command. See --help",
+            CMD, unknown
+        )),
     }
 }

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -287,7 +287,7 @@ impl<T: BeaconChainTypes> VerifiedAggregatedAttestation<T> {
         if chain
             .observed_aggregators
             .observe_validator(&attestation, aggregator_index as usize)
-            .map_err(|e| BeaconChainError::from(e))?
+            .map_err(BeaconChainError::from)?
         {
             return Err(Error::PriorAttestationKnown {
                 validator_index: aggregator_index,
@@ -370,7 +370,7 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
         if chain
             .observed_attesters
             .validator_has_been_observed(&attestation, validator_index as usize)
-            .map_err(|e| BeaconChainError::from(e))?
+            .map_err(BeaconChainError::from)?
         {
             return Err(Error::PriorAttestationKnown {
                 validator_index,
@@ -390,7 +390,7 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
         if chain
             .observed_attesters
             .observe_validator(&attestation, validator_index as usize)
-            .map_err(|e| BeaconChainError::from(e))?
+            .map_err(BeaconChainError::from)?
         {
             return Err(Error::PriorAttestationKnown {
                 validator_index,
@@ -504,7 +504,7 @@ pub fn verify_attestation_signature<T: BeaconChainTypes>(
         .canonical_head
         .try_read_for(HEAD_LOCK_TIMEOUT)
         .ok_or_else(|| BeaconChainError::CanonicalHeadLockTimeout)
-        .map(|head| head.beacon_state.fork.clone())?;
+        .map(|head| head.beacon_state.fork)?;
 
     let signature_set = indexed_attestation_signature_set_from_pubkeys(
         |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed),
@@ -559,7 +559,7 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
         .canonical_head
         .try_read_for(HEAD_LOCK_TIMEOUT)
         .ok_or_else(|| BeaconChainError::CanonicalHeadLockTimeout)
-        .map(|head| head.beacon_state.fork.clone())?;
+        .map(|head| head.beacon_state.fork)?;
 
     let signature_sets = vec![
         signed_aggregate_selection_proof_signature_set(
@@ -694,7 +694,7 @@ where
             // The state roots are not useful for the shuffling, so there's no need to
             // compute them.
             per_slot_processing(&mut state, Some(Hash256::zero()), &chain.spec)
-                .map_err(|e| BeaconChainError::from(e))?;
+                .map_err(BeaconChainError::from)?;
         }
 
         metrics::stop_timer(state_skip_timer);
@@ -706,11 +706,11 @@ where
 
         state
             .build_committee_cache(relative_epoch, &chain.spec)
-            .map_err(|e| BeaconChainError::from(e))?;
+            .map_err(BeaconChainError::from)?;
 
         let committee_cache = state
             .committee_cache(relative_epoch)
-            .map_err(|e| BeaconChainError::from(e))?;
+            .map_err(BeaconChainError::from)?;
 
         chain
             .shuffling_cache

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -199,6 +199,8 @@ pub struct BeaconChain<T: BeaconChainTypes> {
     pub genesis_block_root: Hash256,
     /// The root of the list of genesis validators, used during syncing.
     pub genesis_validators_root: Hash256,
+
+    #[allow(clippy::type_complexity)]
     /// A state-machine that is updated with information from the network and chooses a canonical
     /// head block.
     pub fork_choice: RwLock<
@@ -493,9 +495,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             slot: head.beacon_block.slot(),
             block_root: head.beacon_block_root,
             state_root: head.beacon_state_root,
-            current_justified_checkpoint: head.beacon_state.current_justified_checkpoint.clone(),
-            finalized_checkpoint: head.beacon_state.finalized_checkpoint.clone(),
-            fork: head.beacon_state.fork.clone(),
+            current_justified_checkpoint: head.beacon_state.current_justified_checkpoint,
+            finalized_checkpoint: head.beacon_state.finalized_checkpoint,
+            fork: head.beacon_state.fork,
             genesis_time: head.beacon_state.genesis_time,
             genesis_validators_root: head.beacon_state.genesis_validators_root,
         })
@@ -853,8 +855,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             data: AttestationData {
                 slot,
                 index,
-                beacon_block_root: beacon_block_root,
-                source: state.current_justified_checkpoint.clone(),
+                beacon_block_root,
+                source: state.current_justified_checkpoint,
                 target: Checkpoint {
                     epoch,
                     root: target_root,
@@ -986,8 +988,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 .try_read_for(HEAD_LOCK_TIMEOUT)
                 .ok_or_else(|| Error::CanonicalHeadLockTimeout)?
                 .beacon_state
-                .fork
-                .clone();
+                .fork;
 
             self.op_pool
                 .insert_attestation(
@@ -1348,7 +1349,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         };
 
         // Verify and import the block.
-        let result = match import_block(unverified_block) {
+        match import_block(unverified_block) {
             // The block was successfully verified and imported. Yay.
             Ok(block_root) => {
                 trace!(
@@ -1362,7 +1363,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES);
 
                 let _ = self.event_handler.register(EventKind::BeaconBlockImported {
-                    block_root: block_root,
+                    block_root,
                     block: Box::new(block),
                 });
 
@@ -1399,9 +1400,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
                 Err(other)
             }
-        };
-
-        result
+        }
     }
 
     /// Accepts a fully-verified block and imports it into the chain without performing any
@@ -1642,7 +1641,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 body: BeaconBlockBody {
                     randao_reveal,
                     eth1_data,
-                    graffiti: self.graffiti.clone(),
+                    graffiti: self.graffiti,
                     proposer_slashings: proposer_slashings.into(),
                     attester_slashings: attester_slashings.into(),
                     attestations: self
@@ -1718,7 +1717,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             .snapshot_cache
             .try_read_for(BLOCK_PROCESSING_CACHE_LOCK_TIMEOUT)
             .and_then(|snapshot_cache| snapshot_cache.get_cloned(beacon_block_root))
-            .map::<Result<_, Error>, _>(|snapshot| Ok(snapshot))
+            .map::<Result<_, Error>, _>(Ok)
             .unwrap_or_else(|| {
                 let beacon_block = self
                     .get_block(&beacon_block_root)?
@@ -2010,8 +2009,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let mut finalized_blocks: HashSet<Hash256> = HashSet::new();
 
         let genesis_block_hash = Hash256::zero();
-        write!(output, "digraph beacon {{\n").unwrap();
-        write!(output, "\t_{:?}[label=\"genesis\"];\n", genesis_block_hash).unwrap();
+        writeln!(output, "digraph beacon {{").unwrap();
+        writeln!(output, "\t_{:?}[label=\"genesis\"];", genesis_block_hash).unwrap();
 
         // Canonical head needs to be processed first as otherwise finalized blocks aren't detected
         // properly.
@@ -2045,44 +2044,44 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 }
 
                 if block_hash == canonical_head_hash {
-                    write!(
+                    writeln!(
                         output,
-                        "\t_{:?}[label=\"{} ({})\" shape=box3d];\n",
+                        "\t_{:?}[label=\"{} ({})\" shape=box3d];",
                         block_hash,
                         block_hash,
                         signed_beacon_block.slot()
                     )
                     .unwrap();
                 } else if finalized_blocks.contains(&block_hash) {
-                    write!(
+                    writeln!(
                         output,
-                        "\t_{:?}[label=\"{} ({})\" shape=Msquare];\n",
+                        "\t_{:?}[label=\"{} ({})\" shape=Msquare];",
                         block_hash,
                         block_hash,
                         signed_beacon_block.slot()
                     )
                     .unwrap();
                 } else {
-                    write!(
+                    writeln!(
                         output,
-                        "\t_{:?}[label=\"{} ({})\" shape=box];\n",
+                        "\t_{:?}[label=\"{} ({})\" shape=box];",
                         block_hash,
                         block_hash,
                         signed_beacon_block.slot()
                     )
                     .unwrap();
                 }
-                write!(
+                writeln!(
                     output,
-                    "\t_{:?} -> _{:?};\n",
+                    "\t_{:?} -> _{:?};",
                     block_hash,
                     signed_beacon_block.parent_root()
                 )
                 .unwrap();
             }
         }
 
-        write!(output, "}}\n").unwrap();
+        writeln!(output, "}}").unwrap();
     }
 
     // Used for debugging
@@ -2135,7 +2134,7 @@ impl From<BeaconStateError> for Error {
 }
 
 impl ChainSegmentResult {
-    pub fn to_block_error(self) -> Result<(), BlockError> {
+    pub fn into_block_error(self) -> Result<(), BlockError> {
         match self {
             ChainSegmentResult::Failed { error, .. } => Err(error),
             ChainSegmentResult::Successful { .. } => Ok(()),

beacon_node/beacon_chain/src/block_verification.rs

@@ -584,8 +584,9 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
                 state_root
             };
 
-            per_slot_processing(&mut state, Some(state_root), &chain.spec)?
-                .map(|summary| summaries.push(summary));
+            if let Some(summary) = per_slot_processing(&mut state, Some(state_root), &chain.spec)? {
+                summaries.push(summary)
+            }
         }
 
         expose_participation_metrics(&summaries);

beacon_node/beacon_chain/src/builder.rs

@@ -93,6 +93,7 @@ where
 ///
 /// See the tests for an example of a complete working example.
 pub struct BeaconChainBuilder<T: BeaconChainTypes> {
+    #[allow(clippy::type_complexity)]
     store: Option<Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>>,
     store_migrator: Option<T::StoreMigrator>,
     canonical_head: Option<BeaconSnapshot<T::EthSpec>>,
@@ -461,13 +462,10 @@ where
             .pubkey_cache_path
             .ok_or_else(|| "Cannot build without a pubkey cache path".to_string())?;
 
-        let validator_pubkey_cache = self
-            .validator_pubkey_cache
-            .map(|cache| Ok(cache))
-            .unwrap_or_else(|| {
-                ValidatorPubkeyCache::new(&canonical_head.beacon_state, pubkey_cache_path)
-                    .map_err(|e| format!("Unable to init validator pubkey cache: {:?}", e))
-            })?;
+        let validator_pubkey_cache = self.validator_pubkey_cache.map(Ok).unwrap_or_else(|| {
+            ValidatorPubkeyCache::new(&canonical_head.beacon_state, pubkey_cache_path)
+                .map_err(|e| format!("Unable to init validator pubkey cache: {:?}", e))
+        })?;
 
         let persisted_fork_choice = store
             .get_item::<PersistedForkChoice>(&Hash256::from_slice(&FORK_CHOICE_DB_KEY))

beacon_node/beacon_chain/src/eth1_chain.rs

@@ -331,7 +331,7 @@ impl<T: EthSpec> Eth1ChainBackend<T> for CachingEth1Backend<T> {
             //
             // Here we choose the eth1_data corresponding to the latest block in our voting window.
             // If no votes exist, choose `state.eth1_data` as default vote.
-            let default_vote = votes_to_consider
+            votes_to_consider
                 .iter()
                 .max_by(|(_, x), (_, y)| x.cmp(y))
                 .map(|vote| {
@@ -355,8 +355,7 @@ impl<T: EthSpec> Eth1ChainBackend<T> for CachingEth1Backend<T> {
                     );
                     metrics::inc_counter(&metrics::DEFAULT_ETH1_VOTES);
                     vote
-                });
-            default_vote
+                })
         };
 
         debug!(