Peer manager cfg (#2766)

## Issue Addressed
I've done this change in a couple of WIPs already so I might as well submit it on its own. This changes no functionality but reduces coupling in a 0.0001%. It also helps new people who need to work in the peer manager to better understand what it actually needs from the outside

## Proposed Changes

Add a config to the peer manager

beacon_node/http_api/tests/common.rs

@@ -8,7 +8,7 @@ use lighthouse_network::{
     discv5::enr::{CombinedKey, EnrBuilder},
     rpc::methods::{MetaData, MetaDataV2},
     types::{EnrAttestationBitfield, EnrSyncCommitteeBitfield, SyncState},
-    ConnectedPoint, Enr, NetworkConfig, NetworkGlobals, PeerId, PeerManager,
+    ConnectedPoint, Enr, NetworkGlobals, PeerId, PeerManager,
 };
 use network::NetworkMessage;
 use sensitive_url::SensitiveUrl;
@@ -106,8 +106,8 @@ pub async fn create_api_server<T: BeaconChainTypes>(
     ));
 
     // Only a peer manager can add peers, so we create a dummy manager.
-    let network_config = NetworkConfig::default();
+    let config = lighthouse_network::peer_manager::config::Config::default();
-    let mut pm = PeerManager::new(&network_config, network_globals.clone(), &log)
+    let mut pm = PeerManager::new(config, network_globals.clone(), &log)
         .await
         .unwrap();
 

beacon_node/lighthouse_network/src/behaviour/mod.rs

@@ -4,8 +4,8 @@ use crate::behaviour::gossipsub_scoring_parameters::{
 use crate::config::gossipsub_config;
 use crate::discovery::{subnet_predicate, Discovery, DiscoveryEvent, TARGET_SUBNET_PEERS};
 use crate::peer_manager::{
-    peerdb::score::PeerAction, peerdb::score::ReportSource, ConnectionDirection, PeerManager,
+    config::Config as PeerManagerCfg, peerdb::score::PeerAction, peerdb::score::ReportSource,
-    PeerManagerEvent,
+    ConnectionDirection, PeerManager, PeerManagerEvent,
 };
 use crate::rpc::*;
 use crate::service::METADATA_FILENAME;
@@ -265,6 +265,12 @@ impl<TSpec: EthSpec> Behaviour<TSpec> {
             .with_peer_score(params, thresholds)
             .expect("Valid score params and thresholds");
 
+        let peer_manager_cfg = PeerManagerCfg {
+            discovery_enabled: !config.disable_discovery,
+            target_peer_count: config.target_peers,
+            ..Default::default()
+        };
+
         Ok(Behaviour {
             // Sub-behaviours
             gossipsub,
@@ -272,7 +278,7 @@ impl<TSpec: EthSpec> Behaviour<TSpec> {
             discovery,
             identify: Identify::new(identify_config),
             // Auxiliary fields
-            peer_manager: PeerManager::new(&config, network_globals.clone(), log).await?,
+            peer_manager: PeerManager::new(peer_manager_cfg, network_globals.clone(), log).await?,
             events: VecDeque::new(),
             internal_events: VecDeque::new(),
             network_globals,

beacon_node/lighthouse_network/src/lib.rs

@@ -11,7 +11,7 @@ mod config;
 #[allow(clippy::mutable_key_type)] // PeerId in hashmaps are no longer permitted by clippy
 pub mod discovery;
 mod metrics;
-mod peer_manager;
+pub mod peer_manager;
 pub mod rpc;
 mod service;
 pub mod types;

beacon_node/lighthouse_network/src/peer_manager/config.rs

@@ -0,0 +1,43 @@
+/// The time in seconds between re-status's peers.
+pub const DEFAULT_STATUS_INTERVAL: u64 = 300;
+
+/// Default ping interval for outbound connections, in seconds.
+pub const DEFAULT_PING_INTERVAL_OUTBOUND: u64 = 15;
+
+/// Default interval for inbound connections.
+pub const DEFAULT_PING_INTERVAL_INBOUND: u64 = 20;
+
+/// Default number of peers to connect to.
+pub const DEFAULT_TARGET_PEERS: usize = 50;
+
+/// Configurations for the PeerManager.
+#[derive(Debug)]
+pub struct Config {
+    /* Peer count related configurations */
+    /// Whether discovery is enabled.
+    pub discovery_enabled: bool,
+    /// Target number of peers to connect to.
+    pub target_peer_count: usize,
+
+    /* RPC related configurations */
+    /// Time in seconds between status requests sent to peers.
+    pub status_interval: u64,
+    /// The time in seconds between PING events. We do not send a ping if the other peer has PING'd
+    /// us within this time frame (Seconds). This is asymmetric to avoid simultaneous pings. This
+    /// interval applies to inbound connections: those in which we are not the dialer.
+    pub ping_interval_inbound: u64,
+    /// Interval between PING events for peers dialed by us.
+    pub ping_interval_outbound: u64,
+}
+
+impl Default for Config {
+    fn default() -> Self {
+        Config {
+            discovery_enabled: true,
+            target_peer_count: DEFAULT_TARGET_PEERS,
+            status_interval: DEFAULT_STATUS_INTERVAL,
+            ping_interval_inbound: DEFAULT_PING_INTERVAL_INBOUND,
+            ping_interval_outbound: DEFAULT_PING_INTERVAL_OUTBOUND,
+        }
+    }
+}

beacon_node/lighthouse_network/src/peer_manager/mod.rs

@@ -4,7 +4,7 @@ use crate::discovery::TARGET_SUBNET_PEERS;
 use crate::rpc::{GoodbyeReason, MetaData, Protocol, RPCError, RPCResponseErrorCode};
 use crate::types::SyncState;
 use crate::{error, metrics, Gossipsub};
-use crate::{NetworkConfig, NetworkGlobals, PeerId};
+use crate::{NetworkGlobals, PeerId};
 use crate::{Subnet, SubnetDiscovery};
 use discv5::Enr;
 use futures::prelude::*;
@@ -35,16 +35,7 @@ use peerdb::score::{PeerAction, ReportSource};
 pub use peerdb::sync_status::{SyncInfo, SyncStatus};
 use std::collections::{hash_map::Entry, HashMap};
 use std::net::IpAddr;
-
+pub mod config;
-/// The time in seconds between re-status's peers.
-const STATUS_INTERVAL: u64 = 300;
-/// The time in seconds between PING events. We do not send a ping if the other peer has PING'd us
-/// within this time frame (Seconds)
-/// This is asymmetric to avoid simultaneous pings.
-/// The interval for outbound connections.
-const PING_INTERVAL_OUTBOUND: u64 = 15;
-/// The interval for inbound connections.
-const PING_INTERVAL_INBOUND: u64 = 20;
 
 /// The heartbeat performs regular updates such as updating reputations and performing discovery
 /// requests. This defines the interval in seconds.
@@ -118,23 +109,31 @@ pub enum PeerManagerEvent {
 impl<TSpec: EthSpec> PeerManager<TSpec> {
     // NOTE: Must be run inside a tokio executor.
     pub async fn new(
-        config: &NetworkConfig,
+        cfg: config::Config,
         network_globals: Arc<NetworkGlobals<TSpec>>,
         log: &slog::Logger,
     ) -> error::Result<Self> {
+        let config::Config {
+            discovery_enabled,
+            target_peer_count,
+            status_interval,
+            ping_interval_inbound,
+            ping_interval_outbound,
+        } = cfg;
+
         // Set up the peer manager heartbeat interval
         let heartbeat = tokio::time::interval(tokio::time::Duration::from_secs(HEARTBEAT_INTERVAL));
 
         Ok(PeerManager {
             network_globals,
             events: SmallVec::new(),
-            inbound_ping_peers: HashSetDelay::new(Duration::from_secs(PING_INTERVAL_INBOUND)),
+            inbound_ping_peers: HashSetDelay::new(Duration::from_secs(ping_interval_inbound)),
-            outbound_ping_peers: HashSetDelay::new(Duration::from_secs(PING_INTERVAL_OUTBOUND)),
+            outbound_ping_peers: HashSetDelay::new(Duration::from_secs(ping_interval_outbound)),
-            status_peers: HashSetDelay::new(Duration::from_secs(STATUS_INTERVAL)),
+            status_peers: HashSetDelay::new(Duration::from_secs(status_interval)),
-            target_peers: config.target_peers,
+            target_peers: target_peer_count,
             sync_committee_subnets: Default::default(),
             heartbeat,
-            discovery_enabled: !config.disable_discovery,
+            discovery_enabled,
             log: log.clone(),
         })
     }
@@ -1056,22 +1055,11 @@ enum ConnectingType {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::discovery::enr::build_enr;
     use crate::discovery::enr_ext::CombinedKeyExt;
     use crate::rpc::methods::{MetaData, MetaDataV2};
-    use crate::Enr;
     use discv5::enr::CombinedKey;
     use slog::{o, Drain};
-    use std::net::UdpSocket;
+    use types::MinimalEthSpec as E;
-    use types::{EnrForkId, MinimalEthSpec};
-
-    type E = MinimalEthSpec;
-
-    pub fn unused_port() -> u16 {
-        let socket = UdpSocket::bind("127.0.0.1:0").expect("should create udp socket");
-        let local_addr = socket.local_addr().expect("should read udp socket");
-        local_addr.port()
-    }
 
     pub fn build_log(level: slog::Level, enabled: bool) -> slog::Logger {
         let decorator = slog_term::TermDecorator::new().build();
@@ -1085,17 +1073,18 @@ mod tests {
         }
     }
 
-    async fn build_peer_manager(target: usize) -> PeerManager<E> {
+    async fn build_peer_manager(target_peer_count: usize) -> PeerManager<E> {
-        let keypair = libp2p::identity::Keypair::generate_secp256k1();
+        let config = config::Config {
-        let config = NetworkConfig {
+            target_peer_count,
-            discovery_port: unused_port(),
+            discovery_enabled: false,
-            target_peers: target,
             ..Default::default()
         };
-        let enr_key: CombinedKey = CombinedKey::from_libp2p(&keypair).unwrap();
-        let enr: Enr = build_enr::<E>(&enr_key, &config, EnrForkId::default()).unwrap();
         let log = build_log(slog::Level::Debug, false);
-        let globals = NetworkGlobals::new(
+        let globals = {
+            let keypair = libp2p::identity::Keypair::generate_secp256k1();
+            let enr_key: CombinedKey = CombinedKey::from_libp2p(&keypair).unwrap();
+            let enr = discv5::enr::EnrBuilder::new("v4").build(&enr_key).unwrap();
+            NetworkGlobals::new(
                 enr,
                 9000,
                 9000,
@@ -1106,8 +1095,9 @@ mod tests {
                 }),
                 vec![],
                 &log,
-        );
+            )
-        PeerManager::new(&config, Arc::new(globals), &log)
+        };
+        PeerManager::new(config, Arc::new(globals), &log)
             .await
             .unwrap()
     }

beacon_node/lighthouse_network/src/peer_manager/peerdb.rs

@@ -52,7 +52,7 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
         Self {
             log: log.clone(),
             disconnected_peers: 0,
-            banned_peers_count: BannedPeersCount::new(),
+            banned_peers_count: BannedPeersCount::default(),
             peers,
         }
     }
@@ -923,7 +923,7 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
                     "banned_peers > MAX_BANNED_PEERS despite no banned peers in db!"
                 );
                 // reset banned_peers this will also exit the loop
-                self.banned_peers_count = BannedPeersCount::new();
+                self.banned_peers_count = BannedPeersCount::default();
                 None
             } {
                 debug!(self.log, "Removing old banned peer"; "peer_id" => %to_drop);
@@ -1087,6 +1087,7 @@ impl BanResult {
     }
 }
 
+#[derive(Default)]
 pub struct BannedPeersCount {
     /// The number of banned peers in the database.
     banned_peers: usize,
@@ -1132,13 +1133,6 @@ impl BannedPeersCount {
             .get(ip)
             .map_or(false, |count| *count > BANNED_PEERS_PER_IP_THRESHOLD)
     }
-
-    pub fn new() -> Self {
-        BannedPeersCount {
-            banned_peers: 0,
-            banned_peers_per_ip: HashMap::new(),
-        }
-    }
 }
 
 #[cfg(test)]