fix(gossipsub): validate default mesh params in ConfigBuilder::build

protocols/gossipsub/CHANGELOG.md

@@ -1,4 +1,8 @@
 ## 0.51.0
+- Validate the default mesh parameters, the default `max_transmit_size` and all topic mesh
+  configurations in `ConfigBuilder::build`, not only topics with a custom max transmit size.
+  See [PR 6648](https://github.com/libp2p/rust-libp2p/pull/6648).
+
 - Look up the ids of a received IDONTWANT in a `HashSet` when removing them from the peer's send
   queue, so the cost is O(queue + ids) instead of O(queue * ids).
 

protocols/gossipsub/src/behaviour/tests/topic_config.rs

@@ -250,58 +250,64 @@ fn test_mesh_subtraction_with_topic_config() {
     );
 }
 
-/// Tests that if a mesh reaches `mesh_n_high`,
-/// but is only composed of outbound peers, it is not reduced to `mesh_n`.
+/// Tests that if a mesh reaches `mesh_n_high`, it is reduced to `mesh_n` without removing
+/// outbound peers below the topic-specific `mesh_outbound_min`.
 #[test]
 fn test_mesh_subtraction_with_topic_config_min_outbound() {
     let topic = String::from("topic1");
     let topic_hash = TopicHash::from_raw(topic.clone());
 
-    let mesh_n = 5;
-    let mesh_n_high = 7;
+    let mesh_n = 6;
+    let mesh_n_high = 8;
+    let mesh_outbound_min = 3;
 
     let topic_config = TopicMeshConfig {
         mesh_n,
         mesh_n_high,
-        mesh_n_low: 3,
-        mesh_outbound_min: 7,
+        mesh_n_low: 4,
+        mesh_outbound_min,
     };
 
     let config = ConfigBuilder::default()
         .set_topic_config(topic_hash.clone(), topic_config)
         .build()
         .unwrap();
 
-    let peer_no = 12;
-
-    // make all outbound connections.
+    // The first `mesh_outbound_min` peers are outbound connections.
     let (mut gs, peers, _, topics) = DefaultBehaviourTestBuilder::default()
-        .peer_no(peer_no)
+        .peer_no(mesh_n_high)
         .topics(vec![topic])
         .to_subscribe(true)
         .gs_config(config.clone())
-        .outbound(peer_no)
+        .outbound(mesh_outbound_min)
         .create_network();
 
     // graft all peers
-    for peer in peers {
-        gs.handle_graft(&peer, topics.clone());
+    for peer in &peers {
+        gs.handle_graft(peer, topics.clone());
     }
 
     assert_eq!(
         gs.mesh.get(&topics[0]).unwrap().len(),
-        peer_no,
-        "Initially mesh should contain all {peer_no} outbound peers"
+        mesh_n_high,
+        "Initially mesh should be {mesh_n_high}"
     );
 
     // run a heartbeat
     gs.heartbeat();
 
+    let mesh = gs.mesh.get(&topics[0]).unwrap();
     assert_eq!(
-        gs.mesh.get(&topics[0]).unwrap().len(),
-        mesh_n_high,
-        "After heartbeat, mesh should still be {mesh_n_high} as these are all outbound peers"
+        mesh.len(),
+        mesh_n,
+        "After heartbeat, mesh should be reduced to {mesh_n}"
     );
+    for outbound_peer in peers.iter().take(mesh_outbound_min) {
+        assert!(
+            mesh.contains(outbound_peer),
+            "Outbound peers must be kept to satisfy mesh_outbound_min"
+        );
+    }
 }
 
 /// Test behavior with multiple topics having different configs

protocols/gossipsub/src/config.rs

@@ -1111,22 +1111,31 @@ impl ConfigBuilder {
     pub fn build(&self) -> Result<Config, ConfigBuilderError> {
         // check all constraints on config
 
-        let pre_configured_topics = self.config.protocol.max_transmit_sizes.keys();
-        for topic in pre_configured_topics {
-            if self.config.protocol.max_transmit_size_for_topic(topic) < 100 {
-                return Err(ConfigBuilderError::MaxTransmissionSizeTooSmall);
-            }
-
-            let mesh_n = self.config.mesh_n_for_topic(topic);
-            let mesh_n_low = self.config.mesh_n_low_for_topic(topic);
-            let mesh_n_high = self.config.mesh_n_high_for_topic(topic);
-            let mesh_outbound_min = self.config.mesh_outbound_min_for_topic(topic);
+        if self.config.protocol.default_max_transmit_size < 100
+            || self
+                .config
+                .protocol
+                .max_transmit_sizes
+                .values()
+                .any(|size| *size < 100)
+        {
+            return Err(ConfigBuilderError::MaxTransmissionSizeTooSmall);
+        }
 
+        let topic_configuration = &self.config.topic_configuration;
+        for TopicMeshConfig {
+            mesh_n,
+            mesh_n_low,
+            mesh_n_high,
+            mesh_outbound_min,
+        } in std::iter::once(&topic_configuration.default_mesh_params)
+            .chain(topic_configuration.topic_mesh_params.values())
+        {
             if !(mesh_outbound_min <= mesh_n_low && mesh_n_low <= mesh_n && mesh_n <= mesh_n_high) {
                 return Err(ConfigBuilderError::MeshParametersInvalid);
             }
 
-            if mesh_outbound_min * 2 > mesh_n {
+            if mesh_outbound_min * 2 > *mesh_n {
                 return Err(ConfigBuilderError::MeshOutboundInvalid);
             }
         }
@@ -1218,6 +1227,53 @@ mod test {
     use super::*;
     use crate::{Topic, topic::IdentityHash};
 
+    #[test]
+    fn build_rejects_invalid_default_mesh_params() {
+        // mesh_n > mesh_n_high
+        assert!(matches!(
+            ConfigBuilder::default().mesh_n(20).build(),
+            Err(ConfigBuilderError::MeshParametersInvalid)
+        ));
+        // mesh_outbound_min > mesh_n / 2
+        assert!(matches!(
+            ConfigBuilder::default().mesh_outbound_min(4).build(),
+            Err(ConfigBuilderError::MeshOutboundInvalid)
+        ));
+    }
+
+    #[test]
+    fn build_rejects_invalid_topic_mesh_params() {
+        let topic = TopicHash::from_raw("topic");
+        assert!(matches!(
+            ConfigBuilder::default()
+                .set_topic_config(
+                    topic.clone(),
+                    TopicMeshConfig {
+                        mesh_n: 6,
+                        mesh_n_low: 8,
+                        mesh_n_high: 12,
+                        mesh_outbound_min: 2,
+                    },
+                )
+                .build(),
+            Err(ConfigBuilderError::MeshParametersInvalid)
+        ));
+        assert!(matches!(
+            ConfigBuilder::default()
+                .mesh_n_high_for_topic(4, topic)
+                .build(),
+            Err(ConfigBuilderError::MeshParametersInvalid)
+        ));
+    }
+
+    #[test]
+    fn build_rejects_too_small_default_max_transmit_size() {
+        assert!(matches!(
+            ConfigBuilder::default().max_transmit_size(10).build(),
+            Err(ConfigBuilderError::MaxTransmissionSizeTooSmall)
+        ));
+    }
+
     #[test]
     fn create_config_with_message_id_as_plain_function() {
         let config = ConfigBuilder::default()