[Merged by Bors] - Fix Rust 1.61 clippy lints

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/checks.rs

@@ -63,7 +63,7 @@ pub async fn verify_all_finalized_at<E: EthSpec>(
 ) -> Result<(), String> {
     let epochs = {
         let mut epochs = Vec::new();
-        for remote_node in network.remote_nodes()? {
+        for remote_node in network.remote_nodes().await? {
             epochs.push(
                 remote_node
                     .get_beacon_states_finality_checkpoints(StateId::Head)
@@ -93,7 +93,7 @@ async fn verify_validator_count<E: EthSpec>(
 ) -> Result<(), String> {
     let validator_counts = {
         let mut validator_counts = Vec::new();
-        for remote_node in network.remote_nodes()? {
+        for remote_node in network.remote_nodes().await? {
             let vc = remote_node
                 .get_debug_beacon_states::<E>(StateId::Head)
                 .await
@@ -126,7 +126,7 @@ pub async fn verify_full_block_production_up_to<E: EthSpec>(
     slot_duration: Duration,
 ) -> Result<(), String> {
     slot_delay(slot, slot_duration).await;
-    let beacon_nodes = network.beacon_nodes.read();
+    let beacon_nodes = network.beacon_nodes.read().await;
     let beacon_chain = beacon_nodes[0].client.beacon_chain().unwrap();
     let num_blocks = beacon_chain
         .chain_dump()
@@ -152,7 +152,7 @@ pub async fn verify_fork_version<E: EthSpec>(
     altair_fork_version: [u8; 4],
 ) -> Result<(), String> {
     epoch_delay(fork_epoch, slot_duration, E::slots_per_epoch()).await;
-    for remote_node in network.remote_nodes()? {
+    for remote_node in network.remote_nodes().await? {
         let fork_version = remote_node
             .get_beacon_states_fork(StateId::Head)
             .await
@@ -177,7 +177,7 @@ pub async fn verify_full_sync_aggregates_up_to<E: EthSpec>(
     slot_duration: Duration,
 ) -> Result<(), String> {
     slot_delay(upto_slot, slot_duration).await;
-    let remote_nodes = network.remote_nodes()?;
+    let remote_nodes = network.remote_nodes().await?;
     let remote_node = remote_nodes.first().unwrap();
 
     for slot in sync_committee_start_slot.as_u64()..=upto_slot.as_u64() {

testing/simulator/src/eth1_sim.rs

@@ -248,8 +248,8 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
          */
         println!(
             "Simulation complete. Finished with {} beacon nodes and {} validator clients",
-            network.beacon_node_count(),
-            network.validator_client_count()
+            network.beacon_node_count().await,
+            network.validator_client_count().await
         );
 
         // Be explicit about dropping the network, as this kills all the nodes. This ensures

testing/simulator/src/local_network.rs

@@ -3,13 +3,13 @@ use node_test_rig::{
     eth2::{types::StateId, BeaconNodeHttpClient},
     ClientConfig, LocalBeaconNode, LocalValidatorClient, ValidatorConfig, ValidatorFiles,
 };
-use parking_lot::RwLock;
 use sensitive_url::SensitiveUrl;
 use std::{
     ops::Deref,
     time::{SystemTime, UNIX_EPOCH},
 };
 use std::{sync::Arc, time::Duration};
+use tokio::sync::RwLock;
 use types::{Epoch, EthSpec};
 
 const BOOTNODE_PORT: u16 = 42424;
@@ -72,24 +72,24 @@ impl<E: EthSpec> LocalNetwork<E> {
     ///
     /// Note: does not count nodes that are external to this `LocalNetwork` that may have connected
     /// (e.g., another Lighthouse process on the same machine.)
-    pub fn beacon_node_count(&self) -> usize {
-        self.beacon_nodes.read().len()
+    pub async fn beacon_node_count(&self) -> usize {
+        self.beacon_nodes.read().await.len()
     }
 
     /// Returns the number of validator clients in the network.
     ///
     /// Note: does not count nodes that are external to this `LocalNetwork` that may have connected
     /// (e.g., another Lighthouse process on the same machine.)
-    pub fn validator_client_count(&self) -> usize {
-        self.validator_clients.read().len()
+    pub async fn validator_client_count(&self) -> usize {
+        self.validator_clients.read().await.len()
     }
 
     /// Adds a beacon node to the network, connecting to the 0'th beacon node via ENR.
     pub async fn add_beacon_node(&self, mut beacon_config: ClientConfig) -> Result<(), String> {
         let self_1 = self.clone();
         println!("Adding beacon node..");
         {
-            let read_lock = self.beacon_nodes.read();
+            let read_lock = self.beacon_nodes.read().await;
 
             let boot_node = read_lock.first().expect("should have at least one node");
 
@@ -99,15 +99,15 @@ impl<E: EthSpec> LocalNetwork<E> {
                     .enr()
                     .expect("bootnode must have a network"),
             );
-            let count = self.beacon_node_count() as u16;
+            let count = self.beacon_node_count().await as u16;
             beacon_config.network.discovery_port = BOOTNODE_PORT + count;
             beacon_config.network.libp2p_port = BOOTNODE_PORT + count;
             beacon_config.network.enr_udp_port = Some(BOOTNODE_PORT + count);
             beacon_config.network.enr_tcp_port = Some(BOOTNODE_PORT + count);
             beacon_config.network.discv5_config.table_filter = |_| true;
         }
 
-        let mut write_lock = self_1.beacon_nodes.write();
+        let mut write_lock = self_1.beacon_nodes.write().await;
         let index = write_lock.len();
 
         let beacon_node = LocalBeaconNode::production(
@@ -133,7 +133,7 @@ impl<E: EthSpec> LocalNetwork<E> {
             .service_context(format!("validator_{}", beacon_node));
         let self_1 = self.clone();
         let socket_addr = {
-            let read_lock = self.beacon_nodes.read();
+            let read_lock = self.beacon_nodes.read().await;
             let beacon_node = read_lock
                 .get(beacon_node)
                 .ok_or_else(|| format!("No beacon node for index {}", beacon_node))?;
@@ -158,13 +158,17 @@ impl<E: EthSpec> LocalNetwork<E> {
             validator_files,
         )
         .await?;
-        self_1.validator_clients.write().push(validator_client);
+        self_1
+            .validator_clients
+            .write()
+            .await
+            .push(validator_client);
         Ok(())
     }
 
     /// For all beacon nodes in `Self`, return a HTTP client to access each nodes HTTP API.
-    pub fn remote_nodes(&self) -> Result<Vec<BeaconNodeHttpClient>, String> {
-        let beacon_nodes = self.beacon_nodes.read();
+    pub async fn remote_nodes(&self) -> Result<Vec<BeaconNodeHttpClient>, String> {
+        let beacon_nodes = self.beacon_nodes.read().await;
 
         beacon_nodes
             .iter()
@@ -174,7 +178,10 @@ impl<E: EthSpec> LocalNetwork<E> {
 
     /// Return current epoch of bootnode.
     pub async fn bootnode_epoch(&self) -> Result<Epoch, String> {
-        let nodes = self.remote_nodes().expect("Failed to get remote nodes");
+        let nodes = self
+            .remote_nodes()
+            .await
+            .expect("Failed to get remote nodes");
         let bootnode = nodes.first().expect("Should contain bootnode");
         bootnode
             .get_beacon_states_finality_checkpoints(StateId::Head)
@@ -184,7 +191,10 @@ impl<E: EthSpec> LocalNetwork<E> {
     }
 
     pub async fn duration_to_genesis(&self) -> Duration {
-        let nodes = self.remote_nodes().expect("Failed to get remote nodes");
+        let nodes = self
+            .remote_nodes()
+            .await
+            .expect("Failed to get remote nodes");
         let bootnode = nodes.first().expect("Should contain bootnode");
         let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
         let genesis_time = Duration::from_secs(

testing/simulator/src/no_eth1_sim.rs

@@ -150,8 +150,8 @@ pub fn run_no_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
          */
         println!(
             "Simulation complete. Finished with {} beacon nodes and {} validator clients",
-            network.beacon_node_count(),
-            network.validator_client_count()
+            network.beacon_node_count().await,
+            network.validator_client_count().await
         );
 
         // Be explicit about dropping the network, as this kills all the nodes. This ensures

testing/simulator/src/sync_sim.rs

@@ -134,8 +134,8 @@ fn syncing_sim(
          */
         println!(
             "Simulation complete. Finished with {} beacon nodes and {} validator clients",
-            network.beacon_node_count(),
-            network.validator_client_count()
+            network.beacon_node_count().await,
+            network.validator_client_count().await
         );
 
         // Be explicit about dropping the network, as this kills all the nodes. This ensures
@@ -370,7 +370,7 @@ pub async fn verify_syncing<E: EthSpec>(
 pub async fn check_still_syncing<E: EthSpec>(network: &LocalNetwork<E>) -> Result<bool, String> {
     // get syncing status of nodes
     let mut status = Vec::new();
-    for remote_node in network.remote_nodes()? {
+    for remote_node in network.remote_nodes().await? {
         status.push(
             remote_node
                 .get_node_syncing()

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,