Clean up P2pConfig

dns_server/src/main.rs

@@ -57,7 +57,6 @@ async fn run(config: Arc<DnsServerConfig>) -> Result<Never, error::DnsServerErro
         disable_noise: Default::default(),
         boot_nodes: Vec::new(),
         reserved_nodes: Vec::new(),
-        max_inbound_connections: Default::default(),
         ban_threshold: Default::default(),
         ban_duration: Default::default(),
         outbound_connection_timeout: Default::default(),
@@ -66,16 +65,11 @@ async fn run(config: Arc<DnsServerConfig>) -> Result<Never, error::DnsServerErro
         max_clock_diff: Default::default(),
         node_type: NodeType::DnsServer.into(),
         allow_discover_private_ips: Default::default(),
-        msg_header_count_limit: Default::default(),
-        msg_max_locator_count: Default::default(),
-        max_request_blocks_count: Default::default(),
         user_agent,
-        max_message_size: Default::default(),
-        max_peer_tx_announcements: Default::default(),
-        max_singular_unconnected_headers: Default::default(),
         sync_stalling_timeout: Default::default(),
         enable_block_relay_peers: Default::default(),
         connection_count_limits: Default::default(),
+        protocol_config: Default::default(),
     });
 
     let transport = p2p::make_p2p_transport();

node-lib/src/config_files/p2p.rs

@@ -20,6 +20,7 @@ use serde::{Deserialize, Serialize};
 
 use p2p::{
     config::{NodeType, P2pConfig},
+    peer_manager::ConnectionCountLimits,
     types::ip_or_socket_address::IpOrSocketAddress,
 };
 
@@ -95,7 +96,6 @@ impl From<P2pConfigFile> for P2pConfig {
             disable_noise: c.disable_noise,
             boot_nodes: c.boot_nodes.clone().unwrap_or_default(),
             reserved_nodes: c.reserved_nodes.clone().unwrap_or_default(),
-            max_inbound_connections: c.max_inbound_connections.into(),
             ban_threshold: c.ban_threshold.into(),
             ban_duration: c.ban_duration.map(Duration::from_secs).into(),
             max_clock_diff: c.max_clock_diff.map(Duration::from_secs).into(),
@@ -108,19 +108,24 @@ impl From<P2pConfigFile> for P2pConfig {
             node_type: c.node_type.map(Into::into).into(),
 
             allow_discover_private_ips: Default::default(),
-            msg_header_count_limit: Default::default(),
-            msg_max_locator_count: Default::default(),
-            max_request_blocks_count: Default::default(),
             user_agent: mintlayer_core_user_agent(),
-            max_message_size: Default::default(),
-            max_peer_tx_announcements: Default::default(),
-            max_singular_unconnected_headers: Default::default(),
             sync_stalling_timeout: c
                 .sync_stalling_timeout
                 .map(|t| Duration::from_secs(t.into()))
                 .into(),
             enable_block_relay_peers: Default::default(),
-            connection_count_limits: Default::default(),
+            connection_count_limits: ConnectionCountLimits {
+                max_inbound_connections: c.max_inbound_connections.into(),
+
+                preserved_inbound_count_address_group: Default::default(),
+                preserved_inbound_count_ping: Default::default(),
+                preserved_inbound_count_new_blocks: Default::default(),
+                preserved_inbound_count_new_transactions: Default::default(),
+
+                outbound_full_relay_count: Default::default(),
+                outbound_block_relay_count: Default::default(),
+            },
+            protocol_config: Default::default(),
         }
     }
 }

p2p/backend-test-suite/src/block_announcement.rs

@@ -176,24 +176,18 @@ where
         disable_noise: Default::default(),
         boot_nodes: Vec::new(),
         reserved_nodes: Vec::new(),
-        max_inbound_connections: Default::default(),
         ban_threshold: Default::default(),
         ban_duration: Default::default(),
         outbound_connection_timeout: Default::default(),
         ping_check_period: Default::default(),
         ping_timeout: Default::default(),
         max_clock_diff: Default::default(),
         allow_discover_private_ips: Default::default(),
-        msg_header_count_limit: Default::default(),
-        msg_max_locator_count: Default::default(),
-        max_request_blocks_count: Default::default(),
         user_agent: mintlayer_core_user_agent(),
-        max_message_size: Default::default(),
-        max_peer_tx_announcements: Default::default(),
-        max_singular_unconnected_headers: Default::default(),
         sync_stalling_timeout: Default::default(),
         enable_block_relay_peers: Default::default(),
         connection_count_limits: Default::default(),
+        protocol_config: Default::default(),
     });
     let shutdown = Arc::new(SeqCstAtomicBool::new(false));
     let (shutdown_sender_1, shutdown_receiver) = oneshot::channel();

p2p/src/config.rs

@@ -22,9 +22,9 @@ use utils::make_config_setting;
 use crate::{
     net::types::services::{Service, Services},
     peer_manager::ConnectionCountLimits,
+    protocol::ProtocolConfig,
 };
 
-make_config_setting!(MaxInboundConnections, usize, 128);
 make_config_setting!(BanThreshold, u32, 100);
 make_config_setting!(BanDuration, Duration, Duration::from_secs(60 * 60 * 24));
 make_config_setting!(OutboundConnectionTimeout, Duration, Duration::from_secs(10));
@@ -33,12 +33,6 @@ make_config_setting!(AllowDiscoverPrivateIps, bool, false);
 make_config_setting!(PingCheckPeriod, Duration, Duration::from_secs(60));
 make_config_setting!(PingTimeout, Duration, Duration::from_secs(150));
 make_config_setting!(MaxClockDiff, Duration, Duration::from_secs(10));
-make_config_setting!(HeaderLimit, usize, 2000);
-make_config_setting!(MaxLocatorSize, usize, 101);
-make_config_setting!(RequestedBlocksLimit, usize, 500);
-make_config_setting!(MaxMessageSize, usize, 10 * 1024 * 1024);
-make_config_setting!(MaxPeerTxAnnouncements, usize, 5000);
-make_config_setting!(MaxUnconnectedHeaders, usize, 10);
 make_config_setting!(SyncStallingTimeout, Duration, Duration::from_secs(5));
 make_config_setting!(BlockRelayPeers, bool, true);
 
@@ -71,13 +65,6 @@ impl From<NodeType> for Services {
 }
 
 /// The p2p subsystem configuration.
-// TODO: some of these "configuration options" should never be changed in production code,
-// because their values are a part of the protocol, e.g. this includes msg_header_count_limit and
-// max_request_blocks_count. Some other, like msg_max_locator_count, are never changed even
-// in tests. It might be better to separate these "settings" off into a separate struct and/or
-// make some of them constants (and the constant corresponding to msg_max_locator_count may
-// even be moved to chainstate, where locators are actually produced).
-// See the issue https://github.com/mintlayer/mintlayer-core/issues/1201
 #[derive(Debug)]
 pub struct P2pConfig {
     /// Address to bind P2P to.
@@ -93,8 +80,6 @@ pub struct P2pConfig {
     /// PeerManager will try to maintain persistent connections to the reserved nodes.
     /// Ban scores are not adjusted for the reserved nodes.
     pub reserved_nodes: Vec<IpOrSocketAddress>,
-    /// Maximum allowed number of inbound connections.
-    pub max_inbound_connections: MaxInboundConnections,
     /// The score threshold after which a peer is banned.
     pub ban_threshold: BanThreshold,
     /// Duration of bans in seconds.
@@ -112,25 +97,14 @@ pub struct P2pConfig {
     pub node_type: NodeTypeSetting,
     /// Allow announcing and discovering local and private IPs. Should be used for testing only.
     pub allow_discover_private_ips: AllowDiscoverPrivateIps,
-    /// A maximum allowed number of headers in one message.
-    pub msg_header_count_limit: HeaderLimit,
-    /// A maximum number of the elements in the locator.
-    pub msg_max_locator_count: MaxLocatorSize,
-    /// A maximum number of blocks that can be requested from a single peer.
-    pub max_request_blocks_count: RequestedBlocksLimit,
     /// User agent value of this node (sent to peers over the network).
     pub user_agent: UserAgent,
-    /// A maximum size of a p2p message in bytes.
-    pub max_message_size: MaxMessageSize,
-    /// A maximum number of announcements (hashes) for which we haven't receive transactions.
-    pub max_peer_tx_announcements: MaxPeerTxAnnouncements,
-    /// A maximum number of singular unconnected headers that a V1 peer can send before
-    /// it will be considered malicious.
-    pub max_singular_unconnected_headers: MaxUnconnectedHeaders,
     /// A timeout after which a peer is disconnected.
     pub sync_stalling_timeout: SyncStallingTimeout,
     /// Enable/disable block relay peers (only used in unit tests)
     pub enable_block_relay_peers: BlockRelayPeers,
-    /// Various limits for connection counts; should only be overridden by tests.
+    /// Various limits for connection counts; these should only be overridden in tests.
     pub connection_count_limits: ConnectionCountLimits,
+    /// Various limits related to the protocol; these should only be overridden in tests.
+    pub protocol_config: ProtocolConfig,
 }

p2p/src/net/default_backend/peer.rs

@@ -109,7 +109,7 @@ where
         backend_event_rx: mpsc::UnboundedReceiver<BackendEvent>,
         node_protocol_version: ProtocolVersion,
     ) -> Self {
-        let socket = BufferedTranscoder::new(socket, *p2p_config.max_message_size);
+        let socket = BufferedTranscoder::new(socket, *p2p_config.protocol_config.max_message_size);
 
         Self {
             peer_id,
@@ -418,7 +418,8 @@ mod tests {
             peer
         });
 
-        let mut socket2 = BufferedTranscoder::new(socket2, *p2p_config.max_message_size);
+        let mut socket2 =
+            BufferedTranscoder::new(socket2, *p2p_config.protocol_config.max_message_size);
         assert!(socket2.recv().now_or_never().is_none());
         assert!(socket2
             .send(Message::Handshake(HandshakeMessage::Hello {
@@ -499,7 +500,8 @@ mod tests {
             peer
         });
 
-        let mut socket2 = BufferedTranscoder::new(socket2, *p2p_config.max_message_size);
+        let mut socket2 =
+            BufferedTranscoder::new(socket2, *p2p_config.protocol_config.max_message_size);
         socket2.recv().await.unwrap();
         assert!(socket2
             .send(Message::Handshake(HandshakeMessage::HelloAck {
@@ -575,7 +577,8 @@ mod tests {
         let handle =
             logging::spawn_in_current_span(async move { peer.handshake(local_time).await });
 
-        let mut socket2 = BufferedTranscoder::new(socket2, *p2p_config.max_message_size);
+        let mut socket2 =
+            BufferedTranscoder::new(socket2, *p2p_config.protocol_config.max_message_size);
         assert!(socket2.recv().now_or_never().is_none());
         assert!(socket2
             .send(Message::Handshake(HandshakeMessage::Hello {
@@ -640,7 +643,8 @@ mod tests {
         let handle =
             logging::spawn_in_current_span(async move { peer.handshake(local_time).await });
 
-        let mut socket2 = BufferedTranscoder::new(socket2, *p2p_config.max_message_size);
+        let mut socket2 =
+            BufferedTranscoder::new(socket2, *p2p_config.protocol_config.max_message_size);
         assert!(socket2.recv().now_or_never().is_none());
         socket2
             .send(Message::HeaderListRequest(HeaderListRequest::new(

p2p/src/net/default_backend/transport/buffered_transcoder.rs

@@ -187,8 +187,10 @@ mod tests {
 
         let (socket1, socket2) =
             get_two_connected_sockets::<TestTransportChannel, MpscChannelTransport>().await;
-        let mut sender = BufferedTranscoder::new(socket1, *p2p_config.max_message_size);
-        let mut receiver = BufferedTranscoder::new(socket2, *p2p_config.max_message_size);
+        let mut sender =
+            BufferedTranscoder::new(socket1, *p2p_config.protocol_config.max_message_size);
+        let mut receiver =
+            BufferedTranscoder::new(socket2, *p2p_config.protocol_config.max_message_size);
 
         for message in messages {
             sender.send(message.clone()).await.unwrap();

p2p/src/peer_manager/mod.rs

@@ -85,12 +85,23 @@ use self::{
 
 #[derive(Default, Debug)]
 pub struct ConnectionCountLimits {
+    /// Maximum allowed number of inbound connections.
+    pub max_inbound_connections: MaxInboundConnections,
+
+    /// The number of inbound peers to preserve based on the address group.
     pub preserved_inbound_count_address_group: PreservedInboundCountAddressGroup,
+    /// The number of inbound peers to preserve based on ping.
     pub preserved_inbound_count_ping: PreservedInboundCountPing,
+    /// The number of inbound peers to preserve based on the last time they sent us new blocks.
     pub preserved_inbound_count_new_blocks: PreservedInboundCountNewBlocks,
+    /// The number of inbound peers to preserve based on the last time they sent us new transactions.
     pub preserved_inbound_count_new_transactions: PreservedInboundCountNewTransactions,
 
+    /// The desired number of full relay outbound connections.
     pub outbound_full_relay_count: OutboundFullRelayCount,
+    /// The desired number of block relay outbound connections.
+    /// This is supposed to always include one temporary connection, so the value must
+    /// always be greater than zero.
     pub outbound_block_relay_count: OutboundBlockRelayCount,
 }
 
@@ -108,12 +119,8 @@ impl ConnectionCountLimits {
     }
 }
 
-// Desired number of full relay outbound connections.
-// This value is constant because users should not change this.
+make_config_setting!(MaxInboundConnections, usize, 128);
 make_config_setting!(OutboundFullRelayCount, usize, 8);
-
-// Desired number of block relay outbound connections (two permanent and one temporary).
-// This value is constant because users should not change this.
 make_config_setting!(OutboundBlockRelayCount, usize, 3);
 
 /// Lower bound for how often [`PeerManager::heartbeat()`] is called
@@ -668,7 +675,8 @@ where
                 // Outbound peer count is not checked because the node initiates new connections
                 // only when needed or from RPC requests.
                 // TODO: Always allow connections from the whitelisted IPs
-                if self.inbound_peer_count() >= *self.p2p_config.max_inbound_connections
+                if self.inbound_peer_count()
+                    >= *self.p2p_config.connection_count_limits.max_inbound_connections
                     && !self.try_evict_random_inbound_connection()
                 {
                     log::info!("no peer is selected for eviction, new connection is dropped");

p2p/src/peer_manager/peerdb/tests.rs

@@ -45,24 +45,18 @@ fn unban_peer() {
             disable_noise: Default::default(),
             boot_nodes: Default::default(),
             reserved_nodes: Default::default(),
-            max_inbound_connections: Default::default(),
             ban_threshold: Default::default(),
             outbound_connection_timeout: Default::default(),
             ping_check_period: Default::default(),
             ping_timeout: Default::default(),
             max_clock_diff: Default::default(),
             node_type: Default::default(),
             allow_discover_private_ips: Default::default(),
-            msg_header_count_limit: Default::default(),
-            msg_max_locator_count: Default::default(),
-            max_request_blocks_count: Default::default(),
             user_agent: mintlayer_core_user_agent(),
-            max_message_size: Default::default(),
-            max_peer_tx_announcements: Default::default(),
-            max_singular_unconnected_headers: Default::default(),
             sync_stalling_timeout: Default::default(),
             enable_block_relay_peers: Default::default(),
             connection_count_limits: Default::default(),
+            protocol_config: Default::default(),
         }),
         time_getter.get_time_getter(),
         db_store,

p2p/src/peer_manager/tests/addresses.rs

@@ -215,7 +215,6 @@ fn test_addr_list_handling_outbound() {
         disable_noise: Default::default(),
         boot_nodes: Default::default(),
         reserved_nodes: Default::default(),
-        max_inbound_connections: Default::default(),
         ban_threshold: Default::default(),
         ban_duration: Default::default(),
         outbound_connection_timeout: Default::default(),
@@ -224,15 +223,10 @@ fn test_addr_list_handling_outbound() {
         max_clock_diff: Default::default(),
         node_type: Default::default(),
         allow_discover_private_ips: Default::default(),
-        msg_header_count_limit: Default::default(),
-        msg_max_locator_count: Default::default(),
-        max_request_blocks_count: Default::default(),
         user_agent: mintlayer_core_user_agent(),
-        max_message_size: Default::default(),
-        max_peer_tx_announcements: Default::default(),
-        max_singular_unconnected_headers: Default::default(),
         sync_stalling_timeout: Default::default(),
         connection_count_limits: Default::default(),
+        protocol_config: Default::default(),
     });
     let (cmd_tx, mut cmd_rx) = tokio::sync::mpsc::unbounded_channel();
     let (_conn_tx, conn_rx) = tokio::sync::mpsc::unbounded_channel();