From 3f750d8c13c7540159c9b7118aa4925a1298c250 Mon Sep 17 00:00:00 2001 From: Itay-Tsabary-Starkware <106665835+Itay-Tsabary-Starkware@users.noreply.github.com> Date: Mon, 28 Oct 2024 22:07:44 +0200 Subject: [PATCH] fix: fix dump to config (#1637) commit-id:6da233c9 Co-Authored-By: nadin-Starkware --- config/mempool/default_config.json | 17 ++++--------- config/papyrus/default_config.json | 19 ++++----------- .../src/discovery/discovery_test.rs | 2 +- crates/papyrus_network/src/discovery/mod.rs | 8 +++---- .../papyrus_network/src/peer_manager/mod.rs | 12 +++++----- .../papyrus_network/src/peer_manager/test.rs | 16 +++++++++---- ...fig__config_test__dump_default_config.snap | 24 ++++--------------- 7 files changed, 36 insertions(+), 62 deletions(-) diff --git a/config/mempool/default_config.json b/config/mempool/default_config.json index a4e8a78cbe..d0ae8e5481 100644 --- a/config/mempool/default_config.json +++ b/config/mempool/default_config.json @@ -690,12 +690,9 @@ "value": 5 }, "mempool_p2p_config.network_config.discovery_config.bootstrap_dial_retry_config.max_delay": { - "description": "The maximum delay for the exponential backoff strategy.", + "description": "The maximum delay in seconds for the exponential backoff strategy.", "privacy": "Public", - "value": { - "nanos": 0, - "secs": 5 - } + "value": 5 }, "mempool_p2p_config.network_config.discovery_config.heartbeat_interval": { "description": "The interval between each discovery (Kademlia) query in milliseconds.", @@ -710,18 +707,12 @@ "mempool_p2p_config.network_config.peer_manager_config.malicious_timeout": { "description": "The duration a peer is blacklisted after being marked as malicious.", "privacy": "Public", - "value": { - "nanos": 0, - "secs": 31536000 - } + "value": 31536000 }, "mempool_p2p_config.network_config.peer_manager_config.unstable_timeout": { "description": "The duration a peer blacklisted after being reported as unstable.", "privacy": "Public", - "value": { - "nanos": 0, - "secs": 1 - } + "value": 1 }, "mempool_p2p_config.network_config.quic_port": { "description": "The port that the node listens on for incoming quic connections.", diff --git a/config/papyrus/default_config.json b/config/papyrus/default_config.json index 80ef39ea46..60178e9662 100644 --- a/config/papyrus/default_config.json +++ b/config/papyrus/default_config.json @@ -185,12 +185,9 @@ "value": 5 }, "network.discovery_config.bootstrap_dial_retry_config.max_delay": { - "description": "The maximum delay for the exponential backoff strategy.", + "description": "The maximum delay in seconds for the exponential backoff strategy.", "privacy": "Public", - "value": { - "nanos": 0, - "secs": 5 - } + "value": 5 }, "network.discovery_config.heartbeat_interval": { "description": "The interval between each discovery (Kademlia) query in milliseconds.", @@ -205,18 +202,12 @@ "network.peer_manager_config.malicious_timeout": { "description": "The duration a peer is blacklisted after being marked as malicious.", "privacy": "Public", - "value": { - "nanos": 0, - "secs": 31536000 - } + "value": 31536000 }, "network.peer_manager_config.unstable_timeout": { "description": "The duration a peer blacklisted after being reported as unstable.", "privacy": "Public", - "value": { - "nanos": 0, - "secs": 1 - } + "value": 1 }, "network.quic_port": { "description": "The port that the node listens on for incoming quic connections.", @@ -433,4 +424,4 @@ "privacy": "Public", "value": true } -} \ No newline at end of file +} diff --git a/crates/papyrus_network/src/discovery/discovery_test.rs b/crates/papyrus_network/src/discovery/discovery_test.rs index c03cced903..eb9ed25113 100644 --- a/crates/papyrus_network/src/discovery/discovery_test.rs +++ b/crates/papyrus_network/src/discovery/discovery_test.rs @@ -36,7 +36,7 @@ const BOOTSTRAP_DIAL_SLEEP: Duration = Duration::from_secs(1); const CONFIG: DiscoveryConfig = DiscoveryConfig { bootstrap_dial_retry_config: RetryConfig { base_delay_millis: BOOTSTRAP_DIAL_SLEEP.as_millis() as u64, - max_delay: BOOTSTRAP_DIAL_SLEEP, + max_delay: BOOTSTRAP_DIAL_SLEEP.as_secs(), factor: 1, }, heartbeat_interval: Duration::ZERO, diff --git a/crates/papyrus_network/src/discovery/mod.rs b/crates/papyrus_network/src/discovery/mod.rs index 0464bad374..7c5333324f 100644 --- a/crates/papyrus_network/src/discovery/mod.rs +++ b/crates/papyrus_network/src/discovery/mod.rs @@ -221,13 +221,13 @@ impl SerializeConfig for DiscoveryConfig { #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct RetryConfig { pub base_delay_millis: u64, - pub max_delay: Duration, + pub max_delay: u64, pub factor: u64, } impl Default for RetryConfig { fn default() -> Self { - Self { base_delay_millis: 2, max_delay: Duration::from_secs(5), factor: 5 } + Self { base_delay_millis: 2, max_delay: 5, factor: 5 } } } @@ -243,7 +243,7 @@ impl SerializeConfig for RetryConfig { ser_param( "max_delay", &self.max_delay, - "The maximum delay for the exponential backoff strategy.", + "The maximum delay in seconds for the exponential backoff strategy.", ParamPrivacyInput::Public, ), ser_param( @@ -259,7 +259,7 @@ impl SerializeConfig for RetryConfig { impl RetryConfig { fn strategy(&self) -> ExponentialBackoff { ExponentialBackoff::from_millis(self.base_delay_millis) - .max_delay(self.max_delay) + .max_delay(Duration::from_secs(self.max_delay)) .factor(self.factor) } } diff --git a/crates/papyrus_network/src/peer_manager/mod.rs b/crates/papyrus_network/src/peer_manager/mod.rs index 7ee1ed68c4..25c43bc8dc 100644 --- a/crates/papyrus_network/src/peer_manager/mod.rs +++ b/crates/papyrus_network/src/peer_manager/mod.rs @@ -52,8 +52,8 @@ pub struct PeerManager { #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] pub struct PeerManagerConfig { - malicious_timeout: Duration, - unstable_timeout: Duration, + malicious_timeout: u64, + unstable_timeout: u64, } #[derive(thiserror::Error, Debug)] @@ -70,8 +70,8 @@ impl Default for PeerManagerConfig { fn default() -> Self { Self { // 1 year. - malicious_timeout: Duration::from_secs(3600 * 24 * 365), - unstable_timeout: Duration::from_secs(1), + malicious_timeout: 3600 * 24 * 365, + unstable_timeout: 1, } } } @@ -216,12 +216,12 @@ impl PeerManager { ReputationModifier::Misconduct { misconduct_score } => { peer.report(misconduct_score); if peer.is_malicious() { - peer.blacklist_peer(self.config.malicious_timeout); + peer.blacklist_peer(Duration::from_secs(self.config.malicious_timeout)); peer.reset_misconduct_score(); } } ReputationModifier::Unstable => { - peer.blacklist_peer(self.config.unstable_timeout); + peer.blacklist_peer(Duration::from_secs(self.config.unstable_timeout)); } } Ok(()) diff --git a/crates/papyrus_network/src/peer_manager/test.rs b/crates/papyrus_network/src/peer_manager/test.rs index 4ba0e1b1c6..0e16c2a26f 100644 --- a/crates/papyrus_network/src/peer_manager/test.rs +++ b/crates/papyrus_network/src/peer_manager/test.rs @@ -153,8 +153,8 @@ async fn peer_assignment_no_peers() { #[tokio::test] async fn peer_assignment_no_unblocked_peers() { - const BLOCKED_UNTIL: Duration = Duration::from_secs(5); - const TIMEOUT: Duration = Duration::from_secs(1); + const BLOCKED_UNTIL: u64 = 5; + const TIMEOUT: u64 = 1; // Create a new peer manager let config = PeerManagerConfig { malicious_timeout: TIMEOUT, unstable_timeout: TIMEOUT }; let mut peer_manager: PeerManager = PeerManager::new(config.clone()); @@ -172,7 +172,10 @@ async fn peer_assignment_no_unblocked_peers() { peer_manager.report_peer(peer_id, ReputationModifier::Unstable).unwrap(); // Consume the peer blacklisted event - let event = tokio::time::timeout(TIMEOUT, peer_manager.next()).await.unwrap().unwrap(); + let event = tokio::time::timeout(Duration::from_secs(TIMEOUT), peer_manager.next()) + .await + .unwrap() + .unwrap(); assert_matches!( event, ToSwarm::GenerateEvent(ToOtherBehaviourEvent::PeerBlacklisted { peer_id: event_peer_id }) @@ -185,11 +188,14 @@ async fn peer_assignment_no_unblocked_peers() { // Simulate that BLOCKED_UNTIL has passed. tokio::time::pause(); - tokio::time::advance(BLOCKED_UNTIL).await; + tokio::time::advance(Duration::from_secs(BLOCKED_UNTIL)).await; tokio::time::resume(); // After BLOCKED_UNTIL has passed, the peer manager can assign the session. - let event = tokio::time::timeout(TIMEOUT, peer_manager.next()).await.unwrap().unwrap(); + let event = tokio::time::timeout(Duration::from_secs(TIMEOUT), peer_manager.next()) + .await + .unwrap() + .unwrap(); assert_matches!( event, ToSwarm::GenerateEvent(ToOtherBehaviourEvent::SessionAssigned { diff --git a/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap b/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap index 6e5d95a68e..dc23f1312c 100644 --- a/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap +++ b/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap @@ -1,5 +1,6 @@ --- source: crates/papyrus_node/src/config/config_test.rs +assertion_line: 101 expression: dumped_default_config --- { @@ -211,14 +212,9 @@ expression: dumped_default_config "privacy": "Public" }, "network.discovery_config.bootstrap_dial_retry_config.max_delay": { - "description": "The maximum delay for the exponential backoff strategy.", + "description": "The maximum delay in seconds for the exponential backoff strategy.", "value": { - "nanos": { - "$serde_json::private::Number": "0" - }, - "secs": { - "$serde_json::private::Number": "5" - } + "$serde_json::private::Number": "5" }, "privacy": "Public" }, @@ -239,24 +235,14 @@ expression: dumped_default_config "network.peer_manager_config.malicious_timeout": { "description": "The duration a peer is blacklisted after being marked as malicious.", "value": { - "nanos": { - "$serde_json::private::Number": "0" - }, - "secs": { - "$serde_json::private::Number": "31536000" - } + "$serde_json::private::Number": "31536000" }, "privacy": "Public" }, "network.peer_manager_config.unstable_timeout": { "description": "The duration a peer blacklisted after being reported as unstable.", "value": { - "nanos": { - "$serde_json::private::Number": "0" - }, - "secs": { - "$serde_json::private::Number": "1" - } + "$serde_json::private::Number": "1" }, "privacy": "Public" },