Merge pull request #1451 from mintlayer/p2p_peer_discouragement

P2p: peer discouragement

Cargo.toml

@@ -139,6 +139,7 @@ criterion = "0.5"
 crossterm = "0.27"
 derive_more = "0.99"
 directories = "5.0"
+humantime = "2.1"
 dyn-clone = "1.0"
 enum-iterator = "1.4"
 expect-test = "1.3"

chainstate/src/detail/mod.rs

@@ -537,7 +537,7 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
                 bi.block_id(),
                 bi.block_height(),
                 bi.block_timestamp(),
-                bi.block_timestamp().into_time().as_standard_printable_time(),
+                bi.block_timestamp().into_time(),
             );
 
             self.update_initial_block_download_flag()

common/src/chain/transaction/printout.rs

@@ -61,10 +61,7 @@ pub fn transaction_summary(tx: &Transaction, chain_config: &ChainConfig) -> Stri
     };
     let fmt_timelock = |tl: &OutputTimeLock| match tl {
         OutputTimeLock::UntilHeight(h) => format!("OutputTimeLock::UntilHeight({h})"),
-        OutputTimeLock::UntilTime(t) => format!(
-            "OutputTimeLock::UntilTime({})",
-            t.into_time().as_standard_printable_time()
-        ),
+        OutputTimeLock::UntilTime(t) => format!("OutputTimeLock::UntilTime({})", t.into_time()),
         OutputTimeLock::ForBlockCount(n) => format!("OutputTimeLock::ForBlockCount({n} blocks)"),
         OutputTimeLock::ForSeconds(secs) => {
             format!("OutputTimeLock::ForSeconds({secs} seconds)")
@@ -166,7 +163,7 @@ pub fn transaction_summary(tx: &Transaction, chain_config: &ChainConfig) -> Stri
                     fmt_timelock(timelock)
                 )
             }
-            TxOutput::Burn(val) => fmt_val(val),
+            TxOutput::Burn(val) => format!("Burn({})", fmt_val(val)),
             TxOutput::CreateStakePool(id, data) => {
                 format!(
                     "CreateStakePool(Id({}), {})",

common/src/primitives/time.rs

@@ -13,9 +13,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::fmt::{Debug, Display};
 use std::sync::atomic::{AtomicU64, Ordering};
 use std::time::{Duration, SystemTime};
 
+use chrono::TimeZone;
+use serde::{Deserialize, Serialize};
+
 pub fn duration_to_int(d: &Duration) -> Result<u64, std::num::TryFromIntError> {
     let r = d.as_millis().try_into()?;
     Ok(r)
@@ -61,7 +65,7 @@ pub fn get_time() -> Time {
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
 pub struct Time {
     /// Time, stored as duration since SystemTime::UNIX_EPOCH
     time: Duration,
@@ -102,13 +106,13 @@ impl Time {
         self.time.saturating_sub(t.time)
     }
 
-    pub fn as_absolute_time(&self) -> SystemTime {
-        SystemTime::UNIX_EPOCH + self.time
-    }
-
-    pub fn as_standard_printable_time(&self) -> String {
-        let datetime: chrono::DateTime<chrono::Utc> = self.as_absolute_time().into();
-        format!("{}", datetime.format("%Y-%m-%d %H:%M:%S"))
+    pub fn as_absolute_time(&self) -> Option<chrono::DateTime<chrono::Utc>> {
+        TryInto::<i64>::try_into(self.time.as_secs()).ok().and_then(|secs| {
+            // Note: chrono::DateTime supports time values up to about 262,000 years away
+            // from the common era, which is still way below i64::MAX; i.e. timestamp_opt
+            // may still return None here.
+            chrono::Utc.timestamp_opt(secs, self.time.subsec_nanos()).single()
+        })
     }
 }
 
@@ -136,6 +140,33 @@ impl std::ops::Sub<Time> for Time {
     }
 }
 
+impl Debug for Time {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let utc_time = self.as_absolute_time();
+
+        if let Some(time) = utc_time {
+            write!(f, "{time:?}")
+        } else {
+            write!(f, "Time({:?})", self.time)
+        }
+    }
+}
+
+impl Display for Time {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let utc_time = self.as_absolute_time();
+
+        if let Some(time) = utc_time {
+            write!(f, "{time}")
+        } else {
+            // Note: we could use humantime::format_duration here, but the output won't be
+            // very nice, e.g. for Duration::MAX it'll be:
+            // "584542046090years 7months 15days 17h 5m 3s 999ms 999us 999ns"
+            write!(f, "{:?} since Unix epoch", self.time)
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use logging::log;
@@ -189,9 +220,23 @@ mod tests {
     }
 
     #[test]
-    fn format_absolute_time() {
+    fn debug_display() {
         let t = Time::from_secs_since_epoch(1705064092);
-        let s = t.as_standard_printable_time().to_string();
-        assert_eq!(&s, "2024-01-12 12:54:52");
+        let s = format!("{t:?}");
+        assert_eq!(s, "2024-01-12T12:54:52Z");
+        let s = format!("{t}");
+        assert_eq!(s, "2024-01-12 12:54:52 UTC");
+
+        let t = Time::from_duration_since_epoch(Duration::from_millis(1705064092123));
+        let s = format!("{t:?}");
+        assert_eq!(s, "2024-01-12T12:54:52.123Z");
+        let s = format!("{t}");
+        assert_eq!(s, "2024-01-12 12:54:52.123 UTC");
+
+        let t = Time::from_duration_since_epoch(Duration::MAX);
+        let s = format!("{t:?}");
+        assert_eq!(s, "Time(18446744073709551615.999999999s)");
+        let s = format!("{t}");
+        assert_eq!(s, "18446744073709551615.999999999s since Unix epoch");
     }
 }

dns-server/src/crawler_p2p/crawler/mod.rs

@@ -39,38 +39,30 @@ use common::{chain::ChainConfig, primitives::time::Time};
 use crypto::random::{seq::IteratorRandom, Rng};
 use logging::log;
 use p2p::{
-    config::{BanDuration, BanThreshold},
     error::P2pError,
     net::types::PeerInfo,
     peer_manager::{ADDR_RATE_BUCKET_SIZE, ADDR_RATE_INITIAL_SIZE, MAX_ADDR_RATE_PER_SECOND},
     types::{bannable_address::BannableAddress, peer_id::PeerId, socket_address::SocketAddress},
     utils::rate_limiter::RateLimiter,
 };
+use utils::make_config_setting;
 
 use crate::crawler_p2p::crawler::address_data::AddressStateTransitionTo;
 
 use self::address_data::{AddressData, AddressState};
 
 /// How many outbound connection attempts can be made per heartbeat
 const MAX_CONNECTS_PER_HEARTBEAT: usize = 25;
-const DEFAULT_BAN_THRESHOLD: u32 = 100;
-const DEFAULT_BAN_DURATION: Duration = Duration::from_secs(60 * 60 * 24);
 
-#[derive(Clone)]
+make_config_setting!(BanThreshold, u32, 100);
+make_config_setting!(BanDuration, Duration, Duration::from_secs(60 * 60 * 24));
+
+#[derive(Default, Clone)]
 pub struct CrawlerConfig {
     pub ban_threshold: BanThreshold,
     pub ban_duration: BanDuration,
 }
 
-impl Default for CrawlerConfig {
-    fn default() -> Self {
-        Self {
-            ban_threshold: BanThreshold::from(DEFAULT_BAN_THRESHOLD),
-            ban_duration: BanDuration::from(DEFAULT_BAN_DURATION),
-        }
-    }
-}
-
 /// The `Crawler` is the component that communicates with Mintlayer peers using p2p,
 /// and based on the results, commands the DNS server to add/remove ip addresses.
 /// The `Crawler` emits events that communicate whether addresses were reached or,

dns-server/src/crawler_p2p/crawler/tests/mod.rs

@@ -27,7 +27,7 @@ use common::{
     primitives::{time::Time, user_agent::mintlayer_core_user_agent},
 };
 use p2p::{
-    config::{BanDuration, BanThreshold, NodeType},
+    config::NodeType,
     error::{DialError, P2pError, ProtocolError},
     net::types::PeerInfo,
     testing_utils::TEST_PROTOCOL_VERSION,
@@ -312,8 +312,8 @@ fn ban_misbehaved_peer(#[case] seed: Seed) {
     let chain_config = common::chain::config::create_mainnet();
     let mut crawler = test_crawler(
         CrawlerConfig {
-            ban_duration: BanDuration::new(BAN_DURATION),
-            ban_threshold: BanThreshold::new(ban_threshold),
+            ban_duration: BAN_DURATION.into(),
+            ban_threshold: ban_threshold.into(),
         },
         BTreeSet::new(),
         BTreeMap::new(),
@@ -442,8 +442,8 @@ fn ban_misbehaved_peers_with_same_address(#[case] seed: Seed) {
     let chain_config = common::chain::config::create_mainnet();
     let mut crawler = test_crawler(
         CrawlerConfig {
-            ban_duration: BanDuration::new(BAN_DURATION),
-            ban_threshold: BanThreshold::new(ban_threshold),
+            ban_duration: BAN_DURATION.into(),
+            ban_threshold: ban_threshold.into(),
         },
         BTreeSet::new(),
         BTreeMap::new(),
@@ -589,8 +589,8 @@ fn ban_on_misbehavior_during_handshake(#[case] seed: Seed) {
     let chain_config = common::chain::config::create_mainnet();
     let mut crawler = test_crawler(
         CrawlerConfig {
-            ban_duration: BanDuration::new(BAN_DURATION),
-            ban_threshold: BanThreshold::new(ban_threshold),
+            ban_duration: BAN_DURATION.into(),
+            ban_threshold: ban_threshold.into(),
         },
         BTreeSet::new(),
         BTreeMap::new(),
@@ -665,8 +665,8 @@ fn no_ban_on_connection_error(#[case] seed: Seed) {
     let chain_config = common::chain::config::create_mainnet();
     let mut crawler = test_crawler(
         CrawlerConfig {
-            ban_duration: BanDuration::new(BAN_DURATION),
-            ban_threshold: BanThreshold::new(ban_threshold),
+            ban_duration: BAN_DURATION.into(),
+            ban_threshold: ban_threshold.into(),
         },
         BTreeSet::new(),
         BTreeMap::new(),
@@ -712,8 +712,8 @@ const BAN_THRESHOLD: u32 = 100;
 
 fn make_config() -> CrawlerConfig {
     CrawlerConfig {
-        ban_duration: BanDuration::new(BAN_DURATION),
-        ban_threshold: BanThreshold::new(BAN_THRESHOLD),
+        ban_duration: BAN_DURATION.into(),
+        ban_threshold: BAN_THRESHOLD.into(),
     }
 }
 

dns-server/src/crawler_p2p/crawler_manager/tests/mock_manager.rs

@@ -35,7 +35,7 @@ use common::{
     time_getter::TimeGetter,
 };
 use p2p::{
-    config::{BanDuration, BanThreshold, NodeType, P2pConfig},
+    config::{NodeType, P2pConfig},
     error::{DialError, P2pError},
     message::{AnnounceAddrRequest, PeerManagerMessage},
     net::{
@@ -295,8 +295,8 @@ pub fn test_crawler(
         default_p2p_port: 3031,
     };
     let crawler_config = CrawlerConfig {
-        ban_duration: BanDuration::default(),
-        ban_threshold: BanThreshold::default(),
+        ban_duration: Default::default(),
+        ban_threshold: Default::default(),
     };
 
     let state = MockStateRef {

dns-server/src/crawler_p2p/crawler_manager/tests/mod.rs

@@ -19,16 +19,18 @@ use std::time::Duration;
 
 use chainstate::ban_score::BanScore;
 use p2p::{
-    config::{BanDuration, BanThreshold},
     error::{P2pError, ProtocolError},
     types::socket_address::SocketAddress,
 };
 use p2p_test_utils::{expect_no_recv, expect_recv};
 
 use crate::{
-    crawler_p2p::crawler_manager::tests::mock_manager::{
-        advance_time, assert_banned_addresses, assert_known_addresses, test_crawler,
-        ErraticNodeConnectError,
+    crawler_p2p::{
+        crawler::{BanDuration, BanThreshold},
+        crawler_manager::tests::mock_manager::{
+            advance_time, assert_banned_addresses, assert_known_addresses, test_crawler,
+            ErraticNodeConnectError,
+        },
     },
     dns_server::DnsServerCommand,
 };

dns-server/src/main.rs

@@ -58,8 +58,9 @@ async fn run(config: Arc<DnsServerConfig>) -> Result<Never, error::DnsServerErro
         boot_nodes: Vec::new(),
         reserved_nodes: Vec::new(),
         whitelisted_addresses: Default::default(),
-        ban_threshold: Default::default(),
-        ban_duration: Default::default(),
+        // Note: this ban config (as well as any other settings related to the peer or sync manager)
+        // won't have any effect on the dns server.
+        ban_config: Default::default(),
         outbound_connection_timeout: Default::default(),
         ping_check_period: Default::default(),
         ping_timeout: Default::default(),

node-lib/src/config_files/mod.rs

@@ -172,8 +172,8 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
         reserved_nodes,
         whitelisted_addresses,
         max_inbound_connections,
-        ban_threshold,
-        ban_duration,
+        discouragement_threshold,
+        discouragement_duration,
         max_clock_diff,
         outbound_connection_timeout,
         ping_check_period,
@@ -190,7 +190,9 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
     let reserved_nodes = options.p2p_reserved_node.clone().or(reserved_nodes);
     let whitelisted_addresses = options.p2p_whitelist_addr.clone().or(whitelisted_addresses);
     let max_inbound_connections = options.p2p_max_inbound_connections.or(max_inbound_connections);
-    let ban_threshold = options.p2p_ban_threshold.or(ban_threshold);
+    let discouragement_threshold =
+        options.p2p_discouragement_threshold.or(discouragement_threshold);
+    let discouragement_duration = options.p2p_discouragement_duration.or(discouragement_duration);
     let ping_check_period = options.p2p_ping_check_period.or(ping_check_period);
     let ping_timeout = options.p2p_ping_timeout.or(ping_timeout);
     let max_clock_diff = options.p2p_max_clock_diff.or(max_clock_diff);
@@ -210,8 +212,8 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
         reserved_nodes,
         whitelisted_addresses,
         max_inbound_connections,
-        ban_threshold,
-        ban_duration,
+        discouragement_threshold,
+        discouragement_duration,
         max_clock_diff,
         outbound_connection_timeout,
         ping_check_period,