Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions beacon_node/lighthouse_network/gossipsub/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand All @@ -5316,6 +5359,7 @@ fn doesnt_send_idontwant() {
key: None,
validated: true,
};

gs.handle_received_message(message.clone(), &local_id);
assert_eq!(
receivers
Expand Down
19 changes: 11 additions & 8 deletions beacon_node/lighthouse_network/gossipsub/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
}
Expand Down
7 changes: 7 additions & 0 deletions beacon_node/lighthouse_network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ pub struct Config {

/// Configuration for the inbound rate limiter (requests received by this node).
pub inbound_rate_limiter_config: Option<InboundRateLimiterConfig>,

/// 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 {
Expand Down Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be good to extract the default value into a IDONTWANT_MESSAGE_SIZE_THRESHOLD const value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds reasonable. made the requested changes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

target_peers few line above also seems to have inline declaration should I extract it too?

}
}
}
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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")
}
Expand Down
1 change: 1 addition & 0 deletions beacon_node/lighthouse_network/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ impl<E: EthSpec> Network<E> {
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());
Expand Down
9 changes: 8 additions & 1 deletion beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
14 changes: 14 additions & 0 deletions beacon_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,20 @@ pub fn set_network_config(
Some(Default::default())
}
};

if let Some(idontwant_message_size_threshold) =
cli_args.get_one::<String>("idontwant-message-size-threshold")
{
config.idontwant_message_size_threshold = idontwant_message_size_threshold
.parse::<usize>()
.map_err(|_| {
format!(
"Invalid idontwant message size threshold value passed: {}",
idontwant_message_size_threshold
)
})?;
}

Ok(())
}

Expand Down
Loading