[Merged by Bors] - Fix Rust 1.61 clippy lints

.github/workflows/test-suite.yml

@@ -12,7 +12,7 @@ env:
   # Deny warnings in CI
   RUSTFLAGS: "-D warnings"
   # The Nightly version used for cargo-udeps, might need updating from time to time.
-  PINNED_NIGHTLY: nightly-2021-12-01
+  PINNED_NIGHTLY: nightly-2022-05-20
 jobs:
   target-branch-check:
     name: target-branch-check

beacon_node/beacon_chain/src/block_verification.rs

@@ -53,6 +53,7 @@ use crate::{
     },
     metrics, BeaconChain, BeaconChainError, BeaconChainTypes,
 };
+use derivative::Derivative;
 use eth2::types::EventKind;
 use execution_layer::PayloadStatus;
 use fork_choice::{ForkChoice, ForkChoiceStore, PayloadVerificationStatus};
@@ -537,7 +538,8 @@ pub fn signature_verify_chain_segment<T: BeaconChainTypes>(
 
 /// A wrapper around a `SignedBeaconBlock` that indicates it has been approved for re-gossiping on
 /// the p2p network.
-#[derive(Debug)]
+#[derive(Derivative)]
+#[derivative(Debug(bound = "T: BeaconChainTypes"))]
 pub struct GossipVerifiedBlock<T: BeaconChainTypes> {
     pub block: SignedBeaconBlock<T::EthSpec>,
     pub block_root: Hash256,

beacon_node/network/Cargo.toml

@@ -43,3 +43,4 @@ lru_cache = { path = "../../common/lru_cache" }
 if-addrs = "0.6.4"
 strum = "0.24.0"
 tokio-util = { version = "0.6.3", features = ["time"] }
+derivative = "2.2.0"

beacon_node/network/src/beacon_processor/mod.rs

@@ -42,6 +42,7 @@ use crate::sync::manager::BlockProcessType;
 use crate::{metrics, service::NetworkMessage, sync::SyncMessage};
 use beacon_chain::parking_lot::Mutex;
 use beacon_chain::{BeaconChain, BeaconChainTypes, GossipVerifiedBlock};
+use derivative::Derivative;
 use futures::stream::{Stream, StreamExt};
 use futures::task::Poll;
 use lighthouse_network::{
@@ -51,7 +52,6 @@ use lighthouse_network::{
 use logging::TimeLatch;
 use slog::{crit, debug, error, trace, warn, Logger};
 use std::collections::VecDeque;
-use std::fmt;
 use std::pin::Pin;
 use std::sync::{Arc, Weak};
 use std::task::Context;
@@ -331,17 +331,13 @@ impl DuplicateCache {
 }
 
 /// An event to be processed by the manager task.
+#[derive(Derivative)]
+#[derivative(Debug(bound = "T: BeaconChainTypes"))]
 pub struct WorkEvent<T: BeaconChainTypes> {
     drop_during_sync: bool,
     work: Work<T>,
 }
 
-impl<T: BeaconChainTypes> fmt::Debug for WorkEvent<T> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
-    }
-}
-
 impl<T: BeaconChainTypes> WorkEvent<T> {
     /// Create a new `Work` event for some unaggregated attestation.
     pub fn unaggregated_attestation(
@@ -615,7 +611,8 @@ impl<T: BeaconChainTypes> std::convert::From<ReadyWork<T>> for WorkEvent<T> {
 }
 
 /// A consensus message (or multiple) from the network that requires processing.
-#[derive(Debug)]
+#[derive(Derivative)]
+#[derivative(Debug(bound = "T: BeaconChainTypes"))]
 pub enum Work<T: BeaconChainTypes> {
     GossipAttestation {
         message_id: MessageId,

beacon_node/network/src/sync/range_sync/chain.rs

@@ -26,8 +26,9 @@ const BATCH_BUFFER_SIZE: u8 = 5;
 /// A return type for functions that act on a `Chain` which informs the caller whether the chain
 /// has been completed and should be removed or to be kept if further processing is
 /// required.
-#[must_use = "Should be checked, since a failed chain must be removed. A chain that requested
- being removed and continued is now in an inconsistent state"]
+///
+/// Should be checked, since a failed chain must be removed. A chain that requested being removed
+/// and continued is now in an inconsistent state.
 pub type ProcessingResult = Result<KeepChain, RemoveChain>;
 
 /// Reasons for removing a chain

testing/simulator/src/local_network.rs

@@ -107,15 +107,16 @@ impl<E: EthSpec> LocalNetwork<E> {
             beacon_config.network.discv5_config.table_filter = |_| true;
         }
 
-        let mut write_lock = self_1.beacon_nodes.write();
-        let index = write_lock.len();
-
+        // We create the beacon node without holding the lock, so that the lock isn't held
+        // across the await. This is only correct if this function never runs in parallel
+        // with itself (which at the time of writing, it does not).
+        let index = self_1.beacon_nodes.read().len();
         let beacon_node = LocalBeaconNode::production(
             self.context.service_context(format!("node_{}", index)),
             beacon_config,
         )
         .await?;
-        write_lock.push(beacon_node);
+        self_1.beacon_nodes.write().push(beacon_node);
         Ok(())
     }
 

validator_client/src/preparation_service.rs

@@ -199,7 +199,8 @@ impl<T: SlotClock + 'static, E: EthSpec> PreparationService<T, E> {
                     .map_err(|e| {
                         error!(
                             log,
-                            "{}", format!("Error loading fee-recipient file: {:?}", e);
+                            "Error loading fee-recipient file";
+                            "error" => ?e
                         );
                     })
                     .unwrap_or(());
@@ -213,44 +214,39 @@ impl<T: SlotClock + 'static, E: EthSpec> PreparationService<T, E> {
         all_pubkeys
             .into_iter()
             .filter_map(|pubkey| {
-                let validator_index = self.validator_store.validator_index(&pubkey);
-                if let Some(validator_index) = validator_index {
-                    let fee_recipient = if let Some(from_validator_defs) =
-                        self.validator_store.suggested_fee_recipient(&pubkey)
-                    {
-                        // If there is a `suggested_fee_recipient` in the validator definitions yaml
-                        // file, use that value.
-                        Some(from_validator_defs)
-                    } else {
-                        // If there's nothing in the validator defs file, check the fee recipient
-                        // file.
+                // Ignore fee recipients for keys without indices, they are inactive.
+                let validator_index = self.validator_store.validator_index(&pubkey)?;
+
+                // If there is a `suggested_fee_recipient` in the validator definitions yaml
+                // file, use that value.
+                let fee_recipient = self
+                    .validator_store
+                    .suggested_fee_recipient(&pubkey)
+                    .or_else(|| {
+                        // If there's nothing in the validator defs file, check the fee
+                        // recipient file.
                         fee_recipient_file
-                            .as_ref()
-                            .and_then(|f| match f.get_fee_recipient(&pubkey) {
-                                Ok(f) => f,
-                                Err(_e) => None,
-                            })
-                            // If there's nothing in the file, try the process-level default value.
-                            .or(self.fee_recipient)
-                    };
-
-                    if let Some(fee_recipient) = fee_recipient {
-                        Some(ProposerPreparationData {
-                            validator_index,
-                            fee_recipient,
-                        })
-                    } else {
-                        if spec.bellatrix_fork_epoch.is_some() {
-                            error!(
-                                log,
-                                "Validator is missing fee recipient";
-                                "msg" => "update validator_definitions.yml",
-                                "pubkey" => ?pubkey
-                            );
-                        }
-                        None
-                    }
+                            .as_ref()?
+                            .get_fee_recipient(&pubkey)
+                            .ok()?
+                    })
+                    // If there's nothing in the file, try the process-level default value.
+                    .or(self.fee_recipient);
+
+                if let Some(fee_recipient) = fee_recipient {
+                    Some(ProposerPreparationData {
+                        validator_index,
+                        fee_recipient,
+                    })
                 } else {
+                    if spec.bellatrix_fork_epoch.is_some() {
+                        error!(
+                            log,
+                            "Validator is missing fee recipient";
+                            "msg" => "update validator_definitions.yml",
+                            "pubkey" => ?pubkey
+                        );
+                    }
                     None
                 }
             })

validator_client/src/validator_store.rs

@@ -171,6 +171,8 @@ impl<T: SlotClock + 'static, E: EthSpec> ValidatorStore<T, E> {
     /// - Adding the validator definition to the YAML file, saving it to the filesystem.
     /// - Enabling the validator with the slashing protection database.
     /// - If `enable == true`, starting to perform duties for the validator.
+    // FIXME: ignore this clippy lint until the validator store is refactored to use async locks
+    #[allow(clippy::await_holding_lock)]
     pub async fn add_validator(
         &self,
         validator_def: ValidatorDefinition,