Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootstrapper Mode #268

Merged
merged 5 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
18 changes: 14 additions & 4 deletions src/config_models/cli_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,17 @@ pub struct Args {
/// Will not prevent outgoing connections made with `--peers`.
/// Set this value to 0 to refuse all incoming connections.
#[clap(long, default_value = "10", value_name = "COUNT")]
pub max_peers: u16,
pub max_num_peers: u16,

/// Whether to act as bootstrapper node.
///
/// Bootstrapper nodes ensure that the maximum number of peers is never
/// reached by disconnecting from existing peers when the maximum is about
/// to be reached. As a result, they will respond with high likelihood to
/// incoming connection requests -- in contrast to regular nodes, which
/// refuse incoming connections when the max is reached.
#[clap(long)]
pub bootstrap: bool,

/// If this flag is set, the node will refuse to initiate a transaction.
/// This flag makes sense for machines whose resources are dedicated to
Expand Down Expand Up @@ -229,7 +239,7 @@ impl Args {

/// Indicates if all incoming peer connections are disallowed.
pub(crate) fn disallow_all_incoming_peer_connections(&self) -> bool {
self.max_peers.is_zero()
self.max_num_peers.is_zero()
}

/// Return the port that peer can connect on. None if incoming connections
Expand Down Expand Up @@ -319,7 +329,7 @@ mod cli_args_tests {
let default_args = Args::default();

assert_eq!(1000, default_args.peer_tolerance);
assert_eq!(10, default_args.max_peers);
assert_eq!(10, default_args.max_num_peers);
assert_eq!(9798, default_args.peer_port);
assert_eq!(9799, default_args.rpc_port);
assert_eq!(
Expand All @@ -342,7 +352,7 @@ mod cli_args_tests {
#[test]
fn max_peers_0_means_no_incoming_connections() {
let args = Args {
max_peers: 0,
max_num_peers: 0,
..Default::default()
};
assert!(args.disallow_all_incoming_peer_connections());
Expand Down
71 changes: 53 additions & 18 deletions src/connect_to_peers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ async fn check_if_connection_is_allowed(

if let Some(status) = {
// Disallow connection if max number of &peers has been attained
if (cli_arguments.max_peers as usize) <= global_state.net.peer_map.len() {
if (cli_arguments.max_num_peers as usize) <= global_state.net.peer_map.len() {
Some(ConnectionStatus::Refused(
ConnectionRefusedReason::MaxPeerNumberExceeded,
))
Expand Down Expand Up @@ -134,10 +134,20 @@ async fn check_if_connection_is_allowed(
return ConnectionStatus::Refused(ConnectionRefusedReason::IncompatibleVersion);
}

// If this connection touches the maximum number of peer connections, say
// so with special OK code.
if cli_arguments.max_num_peers as usize == global_state.net.peer_map.len() + 1 {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't you also check that the node is running as a bootstrapper node here? It seems to me that all nodes will display this "disconnect" behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • All nodes (bootstrapper or not) run through the highlighted logic.
  • The P2P message ConnectionStatus is only ever Refused(..) or Accepted; the value AcceptedMaxReached is filtered out on line 260.
  • Reading the CLI flag should happen on line 279. Will add that momentarily.

info!("ConnectionStatus::Accepted, but max # connections is now reached");
return ConnectionStatus::AcceptedMaxReached;
}

info!("ConnectionStatus::Accepted");
ConnectionStatus::Accepted
}

/// Respond to an incoming connection initiation.
///
/// Catch and process errors (if any) gracefully.
pub(crate) async fn answer_peer_wrapper<S>(
stream: S,
state_lock: GlobalStateLock,
Expand Down Expand Up @@ -183,7 +193,7 @@ where
inner_ret
}

async fn answer_peer<S>(
pub(crate) async fn answer_peer<S>(
stream: S,
state: GlobalStateLock,
peer_address: std::net::SocketAddr,
Expand All @@ -206,9 +216,9 @@ where
> = SymmetricallyFramed::new(length_delimited, SymmetricalBincode::default());

// Complete Neptune handshake
let peer_handshake_data: HandshakeData = match peer.try_next().await? {
let (peer_handshake_data, acceptance_code) = match peer.try_next().await? {
Some(PeerMessage::Handshake(payload)) => {
let (v, hsd) = *payload;
let (v, peer_handshake_data) = *payload;
if v != crate::MAGIC_STRING_REQUEST {
bail!("Expected magic value, got {:?}", v);
}
Expand All @@ -220,11 +230,11 @@ where
.await?;

// Verify peer network before moving on
if hsd.network != own_handshake_data.network {
if peer_handshake_data.network != own_handshake_data.network {
bail!(
"Cannot connect with {}: Peer runs {}, this client runs {}.",
peer_address,
hsd.network,
peer_handshake_data.network,
own_handshake_data.network,
);
}
Expand All @@ -233,28 +243,46 @@ where
let connection_status = check_if_connection_is_allowed(
state.clone(),
&own_handshake_data,
&hsd,
&peer_handshake_data,
&peer_address,
)
.await;

peer.send(PeerMessage::ConnectionStatus(connection_status))
.await?;
if let ConnectionStatus::Refused(refused_reason) = connection_status {
warn!("Incoming connection refused: {:?}", refused_reason);
bail!("Refusing incoming connection. Reason: {:?}", refused_reason);
match connection_status {
ConnectionStatus::Refused(refused_reason) => {
peer.send(PeerMessage::ConnectionStatus(ConnectionStatus::Refused(
refused_reason,
)))
.await?;
warn!("Incoming connection refused: {:?}", refused_reason);
bail!("Refusing incoming connection. Reason: {:?}", refused_reason);
}
ConnectionStatus::AcceptedMaxReached | ConnectionStatus::Accepted => {
peer.send(PeerMessage::ConnectionStatus(ConnectionStatus::Accepted))
.await?;
}
}

debug!("Got correct magic value request!");
hsd
(peer_handshake_data, connection_status)
}
_ => {
bail!("Didn't get handshake on connection attempt");
}
};

// Whether the incoming connection comes from a peer in bad standing is checked in `get_connection_status`
// Whether the incoming connection comes from a peer in bad standing is
// checked in `check_if_connection_is_allowed`. So if we get here, we are
// good to go.
info!("Connection accepted from {}", peer_address);

// If necessary, disconnect from another, existing peer.
if acceptance_code == ConnectionStatus::AcceptedMaxReached && state.cli().bootstrap {
info!("Maximum # peers reached, so disconnecting from an existing peer.");
peer_task_to_main_tx
.send(PeerTaskToMain::DisconnectFromLongestLivedPeer)
.await?;
}

let peer_distance = 1; // All incoming connections have distance 1
let mut peer_loop_handler = PeerLoopHandler::new(
peer_task_to_main_tx,
Expand Down Expand Up @@ -411,6 +439,12 @@ where
bail!("Attempted to connect to peer that was not allowed. This connection attempt should not have been made.");
}

// By default, start by asking the peer for its peers. In an adversarial
// context, we want the network topology to be as robust as possible.
// Blockchain data can be obtained from other peers, if this connection
// fails.
peer.send(PeerMessage::PeerListRequest).await?;

let mut peer_loop_handler = PeerLoopHandler::new(
peer_task_to_main_tx,
state,
Expand Down Expand Up @@ -516,6 +550,7 @@ mod connect_tests {
.read(&to_bytes(&PeerMessage::ConnectionStatus(
ConnectionStatus::Accepted,
))?)
.write(&to_bytes(&PeerMessage::PeerListRequest)?)
.read(&to_bytes(&PeerMessage::Bye)?)
.build();

Expand Down Expand Up @@ -582,7 +617,7 @@ mod connect_tests {

// pretend --max_peers is 1.
let mut cli = state_lock.cli().clone();
cli.max_peers = 1;
cli.max_num_peers = 1;
state_lock.set_cli(cli.clone()).await;

status = check_if_connection_is_allowed(
Expand All @@ -599,7 +634,7 @@ mod connect_tests {
}

// pretend --max-peers is 100
cli.max_peers = 100;
cli.max_num_peers = 100;
state_lock.set_cli(cli.clone()).await;

// Attempt to connect to already connected peer
Expand Down Expand Up @@ -890,7 +925,7 @@ mod connect_tests {

// set max_peers to 2 to ensure failure on next connection attempt
let mut cli = state_lock.cli().clone();
cli.max_peers = 2;
cli.max_num_peers = 2;
state_lock.set_cli(cli).await;

let answer = answer_peer(
Expand Down
Loading
Loading