From 6d28509f2e515e46407ecfc43a4b6cf0381a9f26 Mon Sep 17 00:00:00 2001 From: manasnagaraj Date: Wed, 2 Oct 2024 14:00:45 -0400 Subject: [PATCH 1/7] idontwant message opitmising --- .../gossipsub/src/behaviour.rs | 11 ++++++--- .../gossipsub/src/config.rs | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs index bf77f30979c..f491dfcb24f 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs @@ -1812,9 +1812,6 @@ where // Calculate the message id on the transformed data. let msg_id = self.config.message_id(&message); - // Broadcast IDONTWANT messages. - self.send_idontwant(&raw_message, &msg_id, propagation_source); - // Check the validity of the message // Peers get penalized if this message is invalid. We don't add it to the duplicate cache // and instead continually penalize peers that repeatedly send this message. @@ -1828,8 +1825,12 @@ where peer_score.duplicated_message(propagation_source, &msg_id, &message.topic); } self.mcache.observe_duplicate(&msg_id, propagation_source); + + // Broadcast IDONTWANT messages. + self.send_idontwant(&raw_message, &msg_id, propagation_source); return; } + tracing::debug!( message=%msg_id, "Put message in duplicate_cache and resolve promises" @@ -2699,6 +2700,10 @@ where return; }; + if message.raw_protobuf_len() < self.config.idontwant_message_size_threshold(){ + return; + } + let iwant_peers = self.gossip_promises.peers_for_message(msg_id); let recipient_peers = mesh_peers diff --git a/beacon_node/lighthouse_network/gossipsub/src/config.rs b/beacon_node/lighthouse_network/gossipsub/src/config.rs index 1296e614c89..edaa4cedeaa 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/config.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/config.rs @@ -98,6 +98,7 @@ pub struct Config { connection_handler_queue_len: usize, connection_handler_publish_duration: Duration, connection_handler_forward_duration: Duration, + idontwant_message_size_threshold: usize, } impl Config { @@ -370,6 +371,16 @@ impl Config { pub fn forward_queue_duration(&self) -> Duration { self.connection_handler_forward_duration } + + // Controls the message size threshold for which IDONTWANT messages are sent. + // Sending IDONTWANT messages for small messages can have a negative effect to the overall + // traffic and CPU load. This acts as a lower bound cutoff for the message size to which + // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.3 + // (see https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.2.md#idontwant-message) + // default is 1kb + pub fn idontwant_message_size_threshold(&self) -> usize{ + self.idontwant_message_size_threshold + } } impl Default for Config { @@ -440,6 +451,7 @@ impl Default for ConfigBuilder { connection_handler_queue_len: 5000, connection_handler_publish_duration: Duration::from_secs(5), connection_handler_forward_duration: Duration::from_millis(1000), + idontwant_message_size_threshold: 1000, }, invalid_protocol: false, } @@ -825,6 +837,17 @@ impl ConfigBuilder { self } + // Controls the message size threshold for which IDONTWANT messages are sent. + // Sending IDONTWANT messages for small messages can have a negative effect to the overall + // traffic and CPU load. This acts as a lower bound cutoff for the message size to which + // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.3 + // (see https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.2.md#idontwant-message) + // default is 1kb + pub fn idontwant_message_size_threshold(&mut self, size: usize) -> &mut Self { + self.config.idontwant_message_size_threshold = size; + self + } + /// Constructs a [`Config`] from the given configuration and validates the settings. pub fn build(&self) -> Result { // check all constraints on config @@ -895,6 +918,7 @@ impl std::fmt::Debug for Config { "published_message_ids_cache_time", &self.published_message_ids_cache_time, ); + let _ = builder.field("idontwant_message_size_threhold",&self.idontwant_message_size_threshold); builder.finish() } } From 205078a7bb3fbcc8290bdfdd423c8493f5b87e5c Mon Sep 17 00:00:00 2001 From: manasnagaraj Date: Wed, 2 Oct 2024 21:30:57 -0400 Subject: [PATCH 2/7] requested changes and linter appeasing --- beacon_node/lighthouse_network/gossipsub/src/behaviour.rs | 5 +++-- beacon_node/lighthouse_network/gossipsub/src/config.rs | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs index f491dfcb24f..a0e5ce5122c 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs @@ -1826,11 +1826,12 @@ where } self.mcache.observe_duplicate(&msg_id, propagation_source); - // Broadcast IDONTWANT messages. - self.send_idontwant(&raw_message, &msg_id, propagation_source); return; } + // Broadcast IDONTWANT messages. + self.send_idontwant(&raw_message, &msg_id, propagation_source); + tracing::debug!( message=%msg_id, "Put message in duplicate_cache and resolve promises" diff --git a/beacon_node/lighthouse_network/gossipsub/src/config.rs b/beacon_node/lighthouse_network/gossipsub/src/config.rs index edaa4cedeaa..a6e47f2c2ca 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/config.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/config.rs @@ -377,7 +377,7 @@ impl Config { // traffic and CPU load. This acts as a lower bound cutoff for the message size to which // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.3 // (see https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.2.md#idontwant-message) - // default is 1kb + // default is 1kB pub fn idontwant_message_size_threshold(&self) -> usize{ self.idontwant_message_size_threshold } @@ -842,7 +842,7 @@ impl ConfigBuilder { // traffic and CPU load. This acts as a lower bound cutoff for the message size to which // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.3 // (see https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.2.md#idontwant-message) - // default is 1kb + // default is 1kB pub fn idontwant_message_size_threshold(&mut self, size: usize) -> &mut Self { self.config.idontwant_message_size_threshold = size; self From b59aee09e703682c7090a5a164b1b42649113a9e Mon Sep 17 00:00:00 2001 From: manasnagaraj Date: Tue, 8 Oct 2024 01:40:58 -0400 Subject: [PATCH 3/7] added the config cli flag --- .../gossipsub/src/behaviour.rs | 11 ++--- .../gossipsub/src/behaviour/tests.rs | 46 ++++++++++++++++++- .../gossipsub/src/config.rs | 19 ++++---- beacon_node/lighthouse_network/src/config.rs | 7 +++ .../lighthouse_network/src/service/mod.rs | 1 + beacon_node/src/cli.rs | 9 +++- beacon_node/src/config.rs | 14 ++++++ 7 files changed, 90 insertions(+), 17 deletions(-) diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs index a0e5ce5122c..c50e76e7f2c 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs @@ -1825,12 +1825,13 @@ where peer_score.duplicated_message(propagation_source, &msg_id, &message.topic); } self.mcache.observe_duplicate(&msg_id, propagation_source); - return; } - // Broadcast IDONTWANT messages. - self.send_idontwant(&raw_message, &msg_id, propagation_source); + // Broadcast IDONTWANT messages + if raw_message.raw_protobuf_len() > self.config.idontwant_message_size_threshold() { + self.send_idontwant(&raw_message, &msg_id, propagation_source); + } tracing::debug!( message=%msg_id, @@ -2701,10 +2702,6 @@ where return; }; - if message.raw_protobuf_len() < self.config.idontwant_message_size_threshold(){ - return; - } - let iwant_peers = self.gossip_promises.peers_for_message(msg_id); let recipient_peers = mesh_peers diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour/tests.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour/tests.rs index 00de3ba2dbc..62f026b568a 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour/tests.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour/tests.rs @@ -5266,13 +5266,14 @@ fn sends_idontwant() { let message = RawMessage { source: Some(peers[1]), - data: vec![12], + data: vec![12u8; 1024], sequence_number: Some(0), topic: topic_hashes[0].clone(), signature: None, key: None, validated: true, }; + gs.handle_received_message(message.clone(), &local_id); assert_eq!( receivers @@ -5292,6 +5293,48 @@ fn sends_idontwant() { ); } +#[test] +fn doesnt_sends_idontwant_for_lower_message_size() { + let (mut gs, peers, receivers, topic_hashes) = inject_nodes1() + .peer_no(5) + .topics(vec![String::from("topic1")]) + .to_subscribe(true) + .gs_config(Config::default()) + .explicit(1) + .peer_kind(PeerKind::Gossipsubv1_2) + .create_network(); + + let local_id = PeerId::random(); + + let message = RawMessage { + source: Some(peers[1]), + data: vec![12], + sequence_number: Some(0), + topic: topic_hashes[0].clone(), + signature: None, + key: None, + validated: true, + }; + + gs.handle_received_message(message.clone(), &local_id); + assert_eq!( + receivers + .into_iter() + .fold(0, |mut idontwants, (peer_id, c)| { + let non_priority = c.non_priority.into_inner(); + while !non_priority.is_empty() { + if let Ok(RpcOut::IDontWant(_)) = non_priority.try_recv() { + assert_ne!(peer_id, peers[1]); + idontwants += 1; + } + } + idontwants + }), + 0, + "IDONTWANT was sent" + ); +} + /// Test that a node doesn't send IDONTWANT messages to the mesh peers /// that don't run Gossipsub v1.2. #[test] @@ -5316,6 +5359,7 @@ fn doesnt_send_idontwant() { key: None, validated: true, }; + gs.handle_received_message(message.clone(), &local_id); assert_eq!( receivers diff --git a/beacon_node/lighthouse_network/gossipsub/src/config.rs b/beacon_node/lighthouse_network/gossipsub/src/config.rs index a6e47f2c2ca..eb8dd432a33 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/config.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/config.rs @@ -372,13 +372,13 @@ impl Config { self.connection_handler_forward_duration } - // Controls the message size threshold for which IDONTWANT messages are sent. + // The message size threshold for which IDONTWANT messages are sent. // Sending IDONTWANT messages for small messages can have a negative effect to the overall - // traffic and CPU load. This acts as a lower bound cutoff for the message size to which - // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.3 + // traffic and CPU load. This acts as a lower bound cutoff for the message size to which + // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.2 // (see https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.2.md#idontwant-message) // default is 1kB - pub fn idontwant_message_size_threshold(&self) -> usize{ + pub fn idontwant_message_size_threshold(&self) -> usize { self.idontwant_message_size_threshold } } @@ -837,10 +837,10 @@ impl ConfigBuilder { self } - // Controls the message size threshold for which IDONTWANT messages are sent. + // The message size threshold for which IDONTWANT messages are sent. // Sending IDONTWANT messages for small messages can have a negative effect to the overall - // traffic and CPU load. This acts as a lower bound cutoff for the message size to which - // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.3 + // traffic and CPU load. This acts as a lower bound cutoff for the message size to which + // IDONTWANT won't be sent to peers. Only works if the peers support Gossipsub1.2 // (see https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.2.md#idontwant-message) // default is 1kB pub fn idontwant_message_size_threshold(&mut self, size: usize) -> &mut Self { @@ -918,7 +918,10 @@ impl std::fmt::Debug for Config { "published_message_ids_cache_time", &self.published_message_ids_cache_time, ); - let _ = builder.field("idontwant_message_size_threhold",&self.idontwant_message_size_threshold); + let _ = builder.field( + "idontwant_message_size_threhold", + &self.idontwant_message_size_threshold, + ); builder.finish() } } diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index 7c95977140e..e19ac35c63c 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -141,6 +141,10 @@ pub struct Config { /// Configuration for the inbound rate limiter (requests received by this node). pub inbound_rate_limiter_config: Option, + + /// Configuration for the minimum message size for which IDONTWANT messages are send in the mesh. + /// Lower the value reduces the optimization effect of the IDONTWANT messages. + pub idontwant_message_size_threshold: usize, } impl Config { @@ -352,6 +356,7 @@ impl Default for Config { outbound_rate_limiter_config: None, invalid_block_storage: None, inbound_rate_limiter_config: None, + idontwant_message_size_threshold: 1000, } } } @@ -433,6 +438,7 @@ pub fn gossipsub_config( gossipsub_config_params: GossipsubConfigParams, seconds_per_slot: u64, slots_per_epoch: u64, + idontwant_message_size_threshold: usize, ) -> gossipsub::Config { fn prefix( prefix: [u8; 4], @@ -504,6 +510,7 @@ pub fn gossipsub_config( .duplicate_cache_time(duplicate_cache_time) .message_id_fn(gossip_message_id) .allow_self_origin(true) + .idontwant_message_size_threshold(idontwant_message_size_threshold) .build() .expect("valid gossipsub configuration") } diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 4cf59e15e10..b12b6714a62 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -205,6 +205,7 @@ impl Network { gossipsub_config_params, ctx.chain_spec.seconds_per_slot, E::slots_per_epoch(), + config.idontwant_message_size_threshold, ); let score_settings = PeerScoreSettings::new(&ctx.chain_spec, gs_config.mesh_n()); diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 1e9611fd1eb..de39ce0348d 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -659,7 +659,14 @@ pub fn cli_app() -> Command { .action(ArgAction::Set) .display_order(0) ) - + .arg( + Arg::new("idontwant-message-size-threshold") + .long("idontwant-message-size-threshold") + .help("Specifies the minimum message size for which IDONTWANT messages are sent. \ + This an optimization strategy to not send IDONTWANT messages for smaller messages.") + .action(ArgAction::Set) + .display_order(0) + ) /* * Monitoring metrics */ diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 0eff8577c4a..f62ccfe3ed9 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -1487,6 +1487,20 @@ pub fn set_network_config( Some(Default::default()) } }; + + if let Some(idontwant_message_size_threshold) = + cli_args.get_one::("idontwant-message-size-threshold") + { + config.idontwant_message_size_threshold = idontwant_message_size_threshold + .parse::() + .map_err(|_| { + format!( + "Invalid idontwant message size threshold value passed: {}", + idontwant_message_size_threshold + ) + })?; + } + Ok(()) } From 98dba970a95ca13631c119d2cd1a2bcc32d5791c Mon Sep 17 00:00:00 2001 From: manasnagaraj Date: Thu, 10 Oct 2024 08:48:02 -0400 Subject: [PATCH 4/7] cli docs generated --- book/src/help_bn.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 733446e5d27..3b29d45cf78 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -196,6 +196,10 @@ Options: --http-tls-key The path of the private key to be used when serving the HTTP API server over TLS. Must not be password-protected. + --idontwant-message-size-threshold + Specifies the minimum message size for which IDONTWANT messages are + sent. This an optimization strategy to not send IDONTWANT messages for + smaller messages. --inbound-rate-limiter-protocols Configures the inbound rate limiter (requests received by this node).Rate limit quotas per protocol can be set in the form of From 9e92d95c4ee822b698f9fcab1f0f9b646831791d Mon Sep 17 00:00:00 2001 From: manasnagaraj Date: Thu, 10 Oct 2024 17:26:28 -0400 Subject: [PATCH 5/7] const declaration --- beacon_node/lighthouse_network/src/config.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index 82830822a44..d70e50b1da3 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -19,6 +19,7 @@ pub const DEFAULT_IPV4_ADDRESS: Ipv4Addr = Ipv4Addr::UNSPECIFIED; pub const DEFAULT_TCP_PORT: u16 = 9000u16; pub const DEFAULT_DISC_PORT: u16 = 9000u16; pub const DEFAULT_QUIC_PORT: u16 = 9001u16; +pub const DEFAULT_IDONTWANT_MESSAGE_SIZE_THRESHOLD: usize = 1000usize; /// The maximum size of gossip messages. pub fn gossip_max_size(is_merge_enabled: bool, gossip_max_size: usize) -> usize { @@ -356,7 +357,7 @@ impl Default for Config { outbound_rate_limiter_config: None, invalid_block_storage: None, inbound_rate_limiter_config: None, - idontwant_message_size_threshold: 1000, + idontwant_message_size_threshold: DEFAULT_IDONTWANT_MESSAGE_SIZE_THRESHOLD, } } } From 4d657e6cf06df1de3099db67882ee6eda786ba64 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Mon, 14 Oct 2024 13:00:40 +1100 Subject: [PATCH 6/7] Hide extra technical cli flag --- beacon_node/src/cli.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index de39ce0348d..d6ed1068036 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -665,6 +665,7 @@ pub fn cli_app() -> Command { .help("Specifies the minimum message size for which IDONTWANT messages are sent. \ This an optimization strategy to not send IDONTWANT messages for smaller messages.") .action(ArgAction::Set) + .hide(true) .display_order(0) ) /* From 1aad5adbc79ca33fa671a99203b3813ae677f332 Mon Sep 17 00:00:00 2001 From: manasnagaraj Date: Mon, 14 Oct 2024 18:09:38 -0400 Subject: [PATCH 7/7] passing ci --- book/src/help_bn.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 3b29d45cf78..733446e5d27 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -196,10 +196,6 @@ Options: --http-tls-key The path of the private key to be used when serving the HTTP API server over TLS. Must not be password-protected. - --idontwant-message-size-threshold - Specifies the minimum message size for which IDONTWANT messages are - sent. This an optimization strategy to not send IDONTWANT messages for - smaller messages. --inbound-rate-limiter-protocols Configures the inbound rate limiter (requests received by this node).Rate limit quotas per protocol can be set in the form of