From 9cde84ae155722edf2a9ec0ea6511609bc564ddd Mon Sep 17 00:00:00 2001
From: Bo QIU <35757521+boqiu@users.noreply.github.com>
Date: Tue, 24 Sep 2024 11:59:34 +0800
Subject: [PATCH] Refactor network peer db configs (#209)

---
 Cargo.lock                              |  1 +
 node/network/Cargo.toml                 |  1 +
 node/network/src/config.rs              |  4 ++
 node/network/src/peer_manager/peerdb.rs | 88 ++++++++++++++++---------
 node/network/src/service.rs             |  1 +
 node/network/src/types/globals.rs       | 13 +++-
 node/network/tests/pm_tests.rs          |  4 +-
 node/router/src/libp2p_event_handler.rs | 10 ++-
 node/src/config/convert.rs              |  2 +
 node/src/config/mod.rs                  |  3 +
 run/config-testnet-standard.toml        | 12 ++++
 run/config-testnet-turbo.toml           | 12 ++++
 run/config.toml                         | 12 ++++
 13 files changed, 127 insertions(+), 36 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 51cb060..40fedd7 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -5002,6 +5002,7 @@ dependencies = [
  "directory",
  "dirs 4.0.0",
  "discv5",
+ "duration-str",
  "error-chain",
  "eth2_ssz",
  "eth2_ssz_derive",
diff --git a/node/network/Cargo.toml b/node/network/Cargo.toml
index cc0e928..bc1e109 100644
--- a/node/network/Cargo.toml
+++ b/node/network/Cargo.toml
@@ -40,6 +40,7 @@ unsigned-varint = { version = "=0.7.1", features = ["codec"] }
 if-addrs = "0.10.1"
 slog = "2.7.0"
 igd = "0.12.1"
+duration-str = "0.5.1"
 
 [dependencies.libp2p]
 version = "0.45.1"
diff --git a/node/network/src/config.rs b/node/network/src/config.rs
index dffa59b..8778052 100644
--- a/node/network/src/config.rs
+++ b/node/network/src/config.rs
@@ -1,3 +1,4 @@
+use crate::peer_manager::peerdb::PeerDBConfig;
 use crate::types::GossipKind;
 use crate::{Enr, PeerIdSerialized};
 use directory::{
@@ -126,6 +127,8 @@ pub struct Config {
 
     /// The id of the storage network.
     pub network_id: NetworkIdentity,
+
+    pub peer_db: PeerDBConfig,
 }
 
 impl Default for Config {
@@ -204,6 +207,7 @@ impl Default for Config {
             topics: Vec::new(),
             metrics_enabled: false,
             network_id: Default::default(),
+            peer_db: Default::default(),
         }
     }
 }
diff --git a/node/network/src/peer_manager/peerdb.rs b/node/network/src/peer_manager/peerdb.rs
index ef82990..04e0803 100644
--- a/node/network/src/peer_manager/peerdb.rs
+++ b/node/network/src/peer_manager/peerdb.rs
@@ -3,13 +3,15 @@ use crate::{
     multiaddr::{Multiaddr, Protocol},
     Enr, Gossipsub, PeerId,
 };
+use duration_str::deserialize_duration;
 use peer_info::{ConnectionDirection, PeerConnectionStatus, PeerInfo};
 use rand::seq::SliceRandom;
 use score::{PeerAction, ReportSource, Score, ScoreState};
-use std::cmp::Ordering;
+use serde::{Deserialize, Serialize};
 use std::collections::{HashMap, HashSet};
 use std::net::{IpAddr, SocketAddr};
 use std::time::Instant;
+use std::{cmp::Ordering, time::Duration};
 use sync_status::SyncStatus;
 
 pub mod client;
@@ -17,21 +19,41 @@ pub mod peer_info;
 pub mod score;
 pub mod sync_status;
 
-/// Max number of disconnected nodes to remember.
-const MAX_DC_PEERS: usize = 500;
-/// The maximum number of banned nodes to remember.
-pub const MAX_BANNED_PEERS: usize = 1000;
 /// We ban an IP if there are more than `BANNED_PEERS_PER_IP_THRESHOLD` banned peers with this IP.
 const BANNED_PEERS_PER_IP_THRESHOLD: usize = 5;
-/// Relative factor of peers that are allowed to have a negative gossipsub score without penalizing
-/// them in lighthouse.
-const ALLOWED_NEGATIVE_GOSSIPSUB_FACTOR: f32 = 0.1;
-/// The time we allow peers to be in the dialing state in our PeerDb before we revert them to a
-/// disconnected state.
-const DIAL_TIMEOUT: u64 = 15;
+
+#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
+#[serde(default)]
+pub struct PeerDBConfig {
+    /// The maximum number of disconnected nodes to remember.
+    pub max_disconnected_peers: usize,
+    /// The maximum number of banned nodes to remember.
+    pub max_banned_peers: usize,
+    /// We ban an IP if there are more than `BANNED_PEERS_PER_IP_THRESHOLD` banned peers with this IP.
+    pub banned_peers_per_ip_threshold: usize,
+    /// Relative factor of peers that are allowed to have a negative gossipsub score without penalizing them in lighthouse.
+    pub allowed_negative_gossipsub_factor: f32,
+    /// The time we allow peers to be in the dialing state in our PeerDb before we revert them to a disconnected state.
+    #[serde(deserialize_with = "deserialize_duration")]
+    pub dail_timeout: Duration,
+}
+
+impl Default for PeerDBConfig {
+    fn default() -> Self {
+        Self {
+            max_disconnected_peers: 500,
+            max_banned_peers: 1000,
+            banned_peers_per_ip_threshold: 5,
+            allowed_negative_gossipsub_factor: 0.1,
+            dail_timeout: Duration::from_secs(15),
+        }
+    }
+}
 
 /// Storage of known peers, their reputation and information
 pub struct PeerDB {
+    config: PeerDBConfig,
+
     /// The collection of known connected peers, their status and reputation
     peers: HashMap<PeerId, PeerInfo>,
     /// The number of disconnected nodes in the database.
@@ -41,13 +63,14 @@ pub struct PeerDB {
 }
 
 impl PeerDB {
-    pub fn new(trusted_peers: Vec<PeerId>) -> Self {
+    pub fn new(config: PeerDBConfig, trusted_peers: Vec<PeerId>) -> Self {
         // Initialize the peers hashmap with trusted peers
         let peers = trusted_peers
             .into_iter()
             .map(|peer_id| (peer_id, PeerInfo::trusted_peer_info()))
             .collect();
         Self {
+            config,
             disconnected_peers: 0,
             banned_peers_count: BannedPeersCount::default(),
             peers,
@@ -316,9 +339,7 @@ impl PeerDB {
             .iter()
             .filter_map(|(peer_id, info)| {
                 if let PeerConnectionStatus::Dialing { since } = info.connection_status() {
-                    if (*since) + std::time::Duration::from_secs(DIAL_TIMEOUT)
-                        < std::time::Instant::now()
-                    {
+                    if (*since) + self.config.dail_timeout < std::time::Instant::now() {
                         return Some(*peer_id);
                     }
                 }
@@ -422,7 +443,7 @@ impl PeerDB {
         peers.sort_unstable_by(|(.., s1), (.., s2)| s2.partial_cmp(s1).unwrap_or(Ordering::Equal));
 
         let mut to_ignore_negative_peers =
-            (target_peers as f32 * ALLOWED_NEGATIVE_GOSSIPSUB_FACTOR).ceil() as usize;
+            (target_peers as f32 * self.config.allowed_negative_gossipsub_factor).ceil() as usize;
 
         for (peer_id, info, score) in peers {
             let previous_state = info.score_state();
@@ -946,11 +967,11 @@ impl PeerDB {
         let excess_peers = self
             .banned_peers_count
             .banned_peers()
-            .saturating_sub(MAX_BANNED_PEERS);
+            .saturating_sub(self.config.max_banned_peers);
         let mut unbanned_peers = Vec::with_capacity(excess_peers);
 
         // Remove excess banned peers
-        while self.banned_peers_count.banned_peers() > MAX_BANNED_PEERS {
+        while self.banned_peers_count.banned_peers() > self.config.max_banned_peers {
             if let Some((to_drop, unbanned_ips)) = if let Some((id, info, _)) = self
                 .peers
                 .iter()
@@ -982,7 +1003,7 @@ impl PeerDB {
         }
 
         // Remove excess disconnected peers
-        while self.disconnected_peers > MAX_DC_PEERS {
+        while self.disconnected_peers > self.config.max_disconnected_peers {
             if let Some(to_drop) = self
                 .peers
                 .iter()
@@ -1210,7 +1231,7 @@ mod tests {
     }
 
     fn get_db() -> PeerDB {
-        PeerDB::new(vec![])
+        PeerDB::new(PeerDBConfig::default(), vec![])
     }
 
     #[test]
@@ -1265,7 +1286,7 @@ mod tests {
 
         use std::collections::BTreeMap;
         let mut peer_list = BTreeMap::new();
-        for id in 0..MAX_DC_PEERS + 1 {
+        for id in 0..pdb.config.max_disconnected_peers + 1 {
             let new_peer = PeerId::random();
             pdb.connect_ingoing(&new_peer, "/ip4/0.0.0.0".parse().unwrap(), None);
             peer_list.insert(id, new_peer);
@@ -1276,11 +1297,15 @@ mod tests {
             pdb.inject_disconnect(p);
             // Allow the timing to update correctly
         }
-        assert_eq!(pdb.disconnected_peers, MAX_DC_PEERS);
+        assert_eq!(pdb.disconnected_peers, pdb.config.max_disconnected_peers);
         assert_eq!(pdb.disconnected_peers, pdb.disconnected_peers().count());
 
         // Only the oldest peer should have been removed
-        for (id, peer_id) in peer_list.iter().rev().take(MAX_DC_PEERS) {
+        for (id, peer_id) in peer_list
+            .iter()
+            .rev()
+            .take(pdb.config.max_disconnected_peers)
+        {
             println!("Testing id {}", id);
             assert!(
                 pdb.peer_info(peer_id).is_some(),
@@ -1301,7 +1326,7 @@ mod tests {
 
         use std::collections::BTreeMap;
         let mut peer_list = BTreeMap::new();
-        for id in 0..MAX_DC_PEERS + 20 {
+        for id in 0..pdb.config.max_disconnected_peers + 20 {
             let new_peer = PeerId::random();
             pdb.connect_ingoing(&new_peer, "/ip4/0.0.0.0".parse().unwrap(), None);
             peer_list.insert(id, new_peer);
@@ -1314,7 +1339,7 @@ mod tests {
         println!("{}", pdb.disconnected_peers);
 
         peer_list.clear();
-        for id in 0..MAX_DC_PEERS + 20 {
+        for id in 0..pdb.config.max_disconnected_peers + 20 {
             let new_peer = PeerId::random();
             pdb.connect_ingoing(&new_peer, "/ip4/0.0.0.0".parse().unwrap(), None);
             peer_list.insert(id, new_peer);
@@ -1345,7 +1370,7 @@ mod tests {
     fn test_disconnected_are_bounded() {
         let mut pdb = get_db();
 
-        for _ in 0..MAX_DC_PEERS + 1 {
+        for _ in 0..pdb.config.max_disconnected_peers + 1 {
             let p = PeerId::random();
             pdb.connect_ingoing(&p, "/ip4/0.0.0.0".parse().unwrap(), None);
         }
@@ -1356,14 +1381,14 @@ mod tests {
         }
         assert_eq!(pdb.disconnected_peers, pdb.disconnected_peers().count());
 
-        assert_eq!(pdb.disconnected_peers, MAX_DC_PEERS);
+        assert_eq!(pdb.disconnected_peers, pdb.config.max_disconnected_peers);
     }
 
     #[test]
     fn test_banned_are_bounded() {
         let mut pdb = get_db();
 
-        for _ in 0..MAX_BANNED_PEERS + 1 {
+        for _ in 0..pdb.config.max_banned_peers + 1 {
             let p = PeerId::random();
             pdb.connect_ingoing(&p, "/ip4/0.0.0.0".parse().unwrap(), None);
         }
@@ -1374,7 +1399,10 @@ mod tests {
             pdb.inject_disconnect(&p);
         }
 
-        assert_eq!(pdb.banned_peers_count.banned_peers(), MAX_BANNED_PEERS);
+        assert_eq!(
+            pdb.banned_peers_count.banned_peers(),
+            pdb.config.max_banned_peers
+        );
     }
 
     #[test]
@@ -1908,7 +1936,7 @@ mod tests {
     #[allow(clippy::float_cmp)]
     fn test_trusted_peers_score() {
         let trusted_peer = PeerId::random();
-        let mut pdb: PeerDB = PeerDB::new(vec![trusted_peer]);
+        let mut pdb: PeerDB = PeerDB::new(PeerDBConfig::default(), vec![trusted_peer]);
 
         pdb.connect_ingoing(&trusted_peer, "/ip4/0.0.0.0".parse().unwrap(), None);
 
diff --git a/node/network/src/service.rs b/node/network/src/service.rs
index fda729e..8b46126 100644
--- a/node/network/src/service.rs
+++ b/node/network/src/service.rs
@@ -84,6 +84,7 @@ impl<AppReqId: ReqId> Service<AppReqId> {
                 .iter()
                 .map(|x| PeerId::from(x.clone()))
                 .collect(),
+            config.peer_db,
             config.network_id.clone(),
         ));
 
diff --git a/node/network/src/types/globals.rs b/node/network/src/types/globals.rs
index fc2b5df..4a41055 100644
--- a/node/network/src/types/globals.rs
+++ b/node/network/src/types/globals.rs
@@ -1,5 +1,6 @@
 //! A collection of variables that are accessible outside of the network thread itself.
 use crate::peer_manager::peerdb::PeerDB;
+use crate::peer_manager::peerdb::PeerDBConfig;
 use crate::Client;
 use crate::EnrExt;
 use crate::{Enr, GossipTopic, Multiaddr, PeerId};
@@ -34,6 +35,7 @@ impl NetworkGlobals {
         tcp_port: u16,
         udp_port: u16,
         trusted_peers: Vec<PeerId>,
+        peer_db_config: PeerDBConfig,
         network_id: NetworkIdentity,
     ) -> Self {
         NetworkGlobals {
@@ -42,7 +44,7 @@ impl NetworkGlobals {
             listen_multiaddrs: RwLock::new(Vec::new()),
             listen_port_tcp: AtomicU16::new(tcp_port),
             listen_port_udp: AtomicU16::new(udp_port),
-            peers: RwLock::new(PeerDB::new(trusted_peers)),
+            peers: RwLock::new(PeerDB::new(peer_db_config, trusted_peers)),
             gossipsub_subscriptions: RwLock::new(HashSet::new()),
             network_id: RwLock::new(network_id),
         }
@@ -110,6 +112,13 @@ impl NetworkGlobals {
         let enr_key: discv5::enr::CombinedKey =
             discv5::enr::CombinedKey::from_libp2p(&keypair).unwrap();
         let enr = discv5::enr::EnrBuilder::new("v4").build(&enr_key).unwrap();
-        NetworkGlobals::new(enr, 9000, 9000, vec![], Default::default())
+        NetworkGlobals::new(
+            enr,
+            9000,
+            9000,
+            vec![],
+            Default::default(),
+            Default::default(),
+        )
     }
 }
diff --git a/node/network/tests/pm_tests.rs b/node/network/tests/pm_tests.rs
index 414e1aa..99f0c7b 100644
--- a/node/network/tests/pm_tests.rs
+++ b/node/network/tests/pm_tests.rs
@@ -9,7 +9,7 @@ use common::{
     swarm,
 };
 use network::{
-    peer_manager::{self, config::Config, PeerManagerEvent},
+    peer_manager::{config::Config, peerdb::PeerDBConfig, PeerManagerEvent},
     NetworkGlobals, PeerAction, PeerInfo, PeerManager, ReportSource,
 };
 
@@ -101,7 +101,7 @@ async fn banned_peers_consistency() {
     };
 
     let excess_banned_peers = 15;
-    let peers_to_ban = peer_manager::peerdb::MAX_BANNED_PEERS + excess_banned_peers;
+    let peers_to_ban = PeerDBConfig::default().max_banned_peers + excess_banned_peers;
 
     // Build all the dummy peers needed.
     let (mut swarm_pool, peers) = {
diff --git a/node/router/src/libp2p_event_handler.rs b/node/router/src/libp2p_event_handler.rs
index ae80c11..5e3943e 100644
--- a/node/router/src/libp2p_event_handler.rs
+++ b/node/router/src/libp2p_event_handler.rs
@@ -948,8 +948,14 @@ mod tests {
             let keypair = Keypair::generate_secp256k1();
             let enr_key = CombinedKey::from_libp2p(&keypair).unwrap();
             let enr = EnrBuilder::new("v4").build(&enr_key).unwrap();
-            let network_globals =
-                NetworkGlobals::new(enr, 30000, 30000, vec![], Default::default());
+            let network_globals = NetworkGlobals::new(
+                enr,
+                30000,
+                30000,
+                vec![],
+                Default::default(),
+                Default::default(),
+            );
 
             let listen_addr: Multiaddr = "/ip4/127.0.0.1/tcp/30000".parse().unwrap();
             network_globals.listen_multiaddrs.write().push(listen_addr);
diff --git a/node/src/config/convert.rs b/node/src/config/convert.rs
index 58c0152..865c4f1 100644
--- a/node/src/config/convert.rs
+++ b/node/src/config/convert.rs
@@ -96,6 +96,8 @@ impl ZgsConfig {
         network_config.target_peers = self.network_target_peers;
         network_config.private = self.network_private;
 
+        network_config.peer_db = self.network_peer_db;
+
         Ok(network_config)
     }
 
diff --git a/node/src/config/mod.rs b/node/src/config/mod.rs
index 0373fca..376f2e3 100644
--- a/node/src/config/mod.rs
+++ b/node/src/config/mod.rs
@@ -91,6 +91,9 @@ build_config! {
 pub struct ZgsConfig {
     pub raw_conf: RawConfiguration,
 
+    /// Network peer db config, configured by [network_peer_db] section by `config` crate.
+    pub network_peer_db: network::peer_manager::peerdb::PeerDBConfig,
+
     // router config, configured by [router] section by `config` crate.
     pub router: router::Config,
 
diff --git a/run/config-testnet-standard.toml b/run/config-testnet-standard.toml
index c4363c7..a5eaac5 100644
--- a/run/config-testnet-standard.toml
+++ b/run/config-testnet-standard.toml
@@ -218,6 +218,18 @@ reward_contract_address = "0x0496D0817BD8519e0de4894Dc379D35c35275609"
 #
 # prune_batch_wait_time_ms = 1000
 
+#######################################################################
+###                Network Peer DB Config Options                   ###
+#######################################################################
+
+# [network_peer_db]
+
+# The maximum number of disconnected nodes to remember.
+# max_disconnected_peers = 500
+
+# The maximum number of banned nodes to remember.
+# max_banned_peers = 1000
+
 #######################################################################
 ###                   Router Config Options                         ###
 #######################################################################
diff --git a/run/config-testnet-turbo.toml b/run/config-testnet-turbo.toml
index e9d54ac..52979a5 100644
--- a/run/config-testnet-turbo.toml
+++ b/run/config-testnet-turbo.toml
@@ -230,6 +230,18 @@ reward_contract_address = "0x51998C4d486F406a788B766d93510980ae1f9360"
 #
 # prune_batch_wait_time_ms = 1000
 
+#######################################################################
+###                Network Peer DB Config Options                   ###
+#######################################################################
+
+# [network_peer_db]
+
+# The maximum number of disconnected nodes to remember.
+# max_disconnected_peers = 500
+
+# The maximum number of banned nodes to remember.
+# max_banned_peers = 1000
+
 #######################################################################
 ###                   Router Config Options                         ###
 #######################################################################
diff --git a/run/config.toml b/run/config.toml
index ebc113c..fa75141 100644
--- a/run/config.toml
+++ b/run/config.toml
@@ -232,6 +232,18 @@
 #
 # prune_batch_wait_time_ms = 1000
 
+#######################################################################
+###                Network Peer DB Config Options                   ###
+#######################################################################
+
+# [network_peer_db]
+
+# The maximum number of disconnected nodes to remember.
+# max_disconnected_peers = 500
+
+# The maximum number of banned nodes to remember.
+# max_banned_peers = 1000
+
 #######################################################################
 ###                   Router Config Options                         ###
 #######################################################################