Skip to content

Commit

Permalink
feat(p2p): remove low_watermark
Browse files Browse the repository at this point in the history
  • Loading branch information
CHr15F0x committed Sep 24, 2024
1 parent 78270d0 commit be4761b
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 85 deletions.
4 changes: 2 additions & 2 deletions crates/p2p/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Builder {
let client = Client::new(command_sender, local_peer_id);

let (behaviour, relay_transport) = behaviour_builder
.unwrap_or_else(|| Behaviour::builder(keypair.clone(), chain_id, cfg.clone()))
.unwrap_or_else(|| Behaviour::builder(keypair.clone(), chain_id, cfg))
.build(client.clone());

let swarm = Swarm::new(
Expand All @@ -65,7 +65,7 @@ impl Builder {
(
client,
event_receiver,
MainLoop::new(swarm, command_receiver, event_sender, cfg),
MainLoop::new(swarm, command_receiver, event_sender),
)
}
}
4 changes: 0 additions & 4 deletions crates/p2p/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ pub struct Config {
pub max_inbound_relayed_peers: usize,
/// Maximum number of outbound peers.
pub max_outbound_peers: usize,
/// The minimum number of peers to maintain. If the number of outbound peers
/// drops below this number, the node will attempt to connect to more
/// peers.
pub low_watermark: usize,
/// How long to prevent evicted peers from reconnecting.
pub eviction_timeout: Duration,
pub ip_whitelist: Vec<IpNet>,
Expand Down
24 changes: 6 additions & 18 deletions crates/p2p/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ use tokio::time::Duration;

#[cfg(test)]
use crate::test_utils;
use crate::{behaviour, Command, Config, EmptyResultSender, Event, TestCommand, TestEvent};
use crate::{behaviour, Command, EmptyResultSender, Event, TestCommand, TestEvent};

pub struct MainLoop {
cfg: crate::Config,
swarm: libp2p::swarm::Swarm<behaviour::Behaviour>,
command_receiver: mpsc::Receiver<Command>,
event_sender: mpsc::Sender<Event>,
Expand Down Expand Up @@ -77,10 +76,8 @@ impl MainLoop {
swarm: libp2p::swarm::Swarm<behaviour::Behaviour>,
command_receiver: mpsc::Receiver<Command>,
event_sender: mpsc::Sender<Event>,
cfg: Config,
) -> Self {
Self {
cfg,
swarm,
command_receiver,
event_sender,
Expand Down Expand Up @@ -438,20 +435,11 @@ impl MainLoop {
tracing::debug!("Checking low watermark");
// Starting from libp2p-v0.54.1 bootstrap queries are started
// automatically in the kad behaviour:
// - periodically,
// - after a peer is added to the routing table.
// If we have enough peers, we just stop any ongoing bootstrap
// query initiated by libp2p.
if self.swarm.behaviour_mut().outbound_peers().count()
>= self.cfg.low_watermark
{
self.swarm
.behaviour_mut()
.kademlia_mut()
.query_mut(&id)
.expect("Query to be active")
.finish();
} else if step.count == NonZeroUsize::new(1).expect("1>0") {
// 1. periodically,
// 2. after a peer is added to the routing table, if the number of
// peers in the DHT is lower than 20. See `bootstrap_on_low_peers` for more details:
// https://github.com/libp2p/rust-libp2p/blob/d7beb55f672dce54017fa4b30f67ecb8d66b9810/protocols/kad/src/behaviour.rs#L1401).
if step.count == NonZeroUsize::new(1).expect("1>0") {
send_test_event(
&self.event_sender,
TestEvent::KademliaBootstrapStarted,
Expand Down
2 changes: 0 additions & 2 deletions crates/p2p/src/test_utils/peer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ impl Config {
max_inbound_direct_peers: 10,
max_inbound_relayed_peers: 10,
max_outbound_peers: 10,
// Don't open connections automatically.
low_watermark: 0,
ip_whitelist: vec!["::1/0".parse().unwrap(), "0.0.0.0/0".parse().unwrap()],
bootstrap_period: Duration::from_millis(500),
eviction_timeout: Duration::from_secs(15 * 60),
Expand Down
36 changes: 0 additions & 36 deletions crates/p2p/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ async fn periodic_bootstrap() {

const BOOTSTRAP_PERIOD: Duration = Duration::from_millis(500);
let cfg = Config {
low_watermark: 3,
bootstrap_period: BOOTSTRAP_PERIOD,
..Config::for_test()
};
Expand Down Expand Up @@ -254,41 +253,6 @@ async fn periodic_bootstrap() {
peer2.client.for_test().get_peers_from_dht().await,
[boot.peer_id, peer1.peer_id].into()
);

// Start a new peer and connect to the other peers, immediately reaching the low
// watermark.
let mut peer3 = TestPeer::new(cfg);

peer3
.client
.dial(boot.peer_id, boot_addr.clone())
.await
.unwrap();
peer3
.client
.dial(peer1.peer_id, addr1.clone())
.await
.unwrap();
peer3
.client
.dial(peer2.peer_id, addr2.clone())
.await
.unwrap();

consume_accumulated_events(&mut peer3.event_receiver).await;

// The low watermark is reached for peer3, so no more bootstrap attempts are
// made.
let timeout = tokio::time::timeout(
BOOTSTRAP_PERIOD + Duration::from_millis(100),
wait_for_event(&mut peer3.event_receiver, |event| match event {
Event::Test(TestEvent::KademliaBootstrapStarted) => Some(()),
_ => None,
}),
)
.await;

assert!(timeout.is_err());
}

/// Test that if a peer attempts to reconnect too quickly, the connection is
Expand Down
22 changes: 0 additions & 22 deletions crates/pathfinder/src/bin/pathfinder/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,17 +449,6 @@ Example:
)]
max_outbound_connections: u32,

#[arg(
long = "p2p.low-watermark",
long_help = "The minimum number of outbound peers to maintain. If the number of outbound \
peers drops below this number, the node will attempt to connect to more \
peers.",
value_name = "LOW_WATERMARK",
env = "PATHFINDER_LOW_WATERMARK",
default_value = "20"
)]
low_watermark: u32,

#[arg(
long = "p2p.ip-whitelist",
long_help = "Comma separated list of IP addresses or IP address ranges (in CIDR) to \
Expand Down Expand Up @@ -729,7 +718,6 @@ pub struct P2PConfig {
pub max_inbound_relayed_connections: usize,
pub max_outbound_connections: usize,
pub ip_whitelist: Vec<IpNet>,
pub low_watermark: usize,
pub kad_name: Option<String>,
pub l1_checkpoint_override: Option<pathfinder_ethereum::EthereumStateUpdate>,
pub stream_timeout: Duration,
Expand Down Expand Up @@ -840,15 +828,6 @@ impl P2PConfig {
.exit()
}

if args.low_watermark > args.max_outbound_connections {
Cli::command()
.error(
ErrorKind::ValueValidation,
"p2p.low-watermark must be less than or equal to p2p.max_outbound_connections",
)
.exit()
}

if args.kad_name.iter().any(|x| !x.starts_with('/')) {
Cli::command()
.error(
Expand Down Expand Up @@ -876,7 +855,6 @@ impl P2PConfig {
),
predefined_peers: parse_multiaddr_vec("p2p.predefined-peers", args.predefined_peers),
ip_whitelist: args.ip_whitelist,
low_watermark: 0,
kad_name: args.kad_name,
l1_checkpoint_override,
stream_timeout: Duration::from_secs(args.stream_timeout.into()),
Expand Down
1 change: 0 additions & 1 deletion crates/pathfinder/src/bin/pathfinder/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,6 @@ async fn start_p2p(
max_inbound_direct_peers: config.max_inbound_direct_connections,
max_inbound_relayed_peers: config.max_inbound_relayed_connections,
max_outbound_peers: config.max_outbound_connections,
low_watermark: config.low_watermark,
ip_whitelist: config.ip_whitelist,
bootstrap_period: Duration::from_secs(2 * 60),
eviction_timeout: config.eviction_timeout,
Expand Down

0 comments on commit be4761b

Please sign in to comment.