Skip to content

Commit

Permalink
resolved nits
Browse files Browse the repository at this point in the history
  • Loading branch information
zkxuerb committed Nov 1, 2024
1 parent b0b89c3 commit 2bdc60b
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 43 deletions.
4 changes: 1 addition & 3 deletions node/bft/src/gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1165,13 +1165,11 @@ impl<N: Network> Handshake for Gateway<N> {
let num_attempts = self.cache.insert_inbound_connection(peer_addr.ip(), CONNECTION_ATTEMPTS_SINCE_SECS);

debug!("Number of connection attempts from '{}': {}", peer_addr.ip(), num_attempts);
if num_attempts >= MAX_CONNECTION_ATTEMPTS
{
if num_attempts > MAX_CONNECTION_ATTEMPTS {
self.update_ip_ban(peer_addr.ip());
trace!("{CONTEXT} Gateway rejected a consecutive connection request from IP '{}'", peer_addr.ip());
return Err(error(format!("'{}' appears to be spamming connections", peer_addr.ip())));
}

}

let stream = self.borrow_stream(&mut connection);
Expand Down
2 changes: 1 addition & 1 deletion node/bft/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
};
use snarkos_node_bft_events::{CertificateRequest, CertificateResponse, Event};
use snarkos_node_bft_ledger_service::LedgerService;
use snarkos_node_sync::{locators::BlockLocators, BlockSync, BlockSyncMode};
use snarkos_node_sync::{BlockSync, BlockSyncMode, locators::BlockLocators};
use snarkos_node_tcp::P2P;
use snarkvm::{
console::{network::Network, types::Field},
Expand Down
6 changes: 3 additions & 3 deletions node/router/src/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ impl<N: Network> Router<N> {
return Err(error(format!("'{}' is a banned IP address", peer_addr.ip())));
}

let num_attempts = self.cache.insert_inbound_connection(peer_addr.ip(), Router::<N>::CONNECTION_ATTEMPTS_SINCE_SECS);
let num_attempts =
self.cache.insert_inbound_connection(peer_addr.ip(), Router::<N>::CONNECTION_ATTEMPTS_SINCE_SECS);

debug!("Number of connection attempts from '{}': {}", peer_addr.ip(), num_attempts);
if num_attempts >= Router::<N>::MAX_CONNECTION_ATTEMPTS
{
if num_attempts > Router::<N>::MAX_CONNECTION_ATTEMPTS {
self.update_ip_ban(peer_addr.ip());
trace!("Rejected a consecutive connection request from IP '{}'", peer_addr.ip());
return Err(error(format!("'{}' appears to be spamming connections", peer_addr.ip())));
Expand Down
11 changes: 2 additions & 9 deletions node/router/src/heartbeat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
use snarkvm::prelude::Network;

use colored::Colorize;
use rand::{prelude::IteratorRandom, rngs::OsRng, Rng};
use rand::{Rng, prelude::IteratorRandom, rngs::OsRng};

/// A helper function to compute the maximum of two numbers.
/// See Rust issue 92391: https://github.com/rust-lang/rust/issues/92391.
Expand Down Expand Up @@ -237,16 +237,9 @@ pub trait Heartbeat<N: Network>: Outbound<N> {
if num_deficient > 0 {
// Initialize an RNG.
let rng = &mut OsRng;
let banned_ips = self.tcp().banned_peers().get_banned_ips();

// Attempt to connect to more peers.
for peer_ip in self
.router()
.candidate_peers()
.into_iter()
.filter(|peer| !banned_ips.contains(&peer.ip()))
.choose_multiple(rng, num_deficient)
{
for peer_ip in self.router().candidate_peers().into_iter().choose_multiple(rng, num_deficient) {
self.router().connect(peer_ip);
}

Expand Down
12 changes: 7 additions & 5 deletions node/router/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ pub use routing::*;

use crate::messages::NodeType;
use snarkos_account::Account;
use snarkos_node_tcp::{Config, Tcp, is_bogon_ip, is_unspecified_or_broadcast_ip};
use snarkos_node_tcp::{Config, P2P, Tcp, is_bogon_ip, is_unspecified_or_broadcast_ip};

use snarkvm::prelude::{Address, Network, PrivateKey, ViewKey};

use anyhow::{Result, bail};
Expand Down Expand Up @@ -105,16 +106,16 @@ pub struct InnerRouter<N: Network> {
}

impl<N: Network> Router<N> {
/// The minimum permitted interval between connection attempts for an IP; anything shorter is considered malicious.
#[cfg(not(any(test, feature = "test")))]
const CONNECTION_ATTEMPTS_SINCE_SECS: i64 = 10;
/// The maximum number of candidate peers permitted to be stored in the node.
const MAXIMUM_CANDIDATE_PEERS: usize = 10_000;
/// The maximum number of connection failures permitted by an inbound connecting peer.
const MAXIMUM_CONNECTION_FAILURES: usize = 5;
/// The maximum amount of connection attempts withing a 10 second threshold
#[cfg(not(any(test, feature = "test")))]
const MAX_CONNECTION_ATTEMPTS: usize = 10;
/// The minimum permitted interval between connection attempts for an IP; anything shorter is considered malicious.
#[cfg(not(any(test, feature = "test")))]
const CONNECTION_ATTEMPTS_SINCE_SECS: i64 = 10;
/// The duration in seconds after which a connected peer is considered inactive or
/// disconnected if no message has been received in the meantime.
const RADIO_SILENCE_IN_SECS: u64 = 150; // 2.5 minutes
Expand Down Expand Up @@ -395,7 +396,8 @@ impl<N: Network> Router<N> {

/// Returns the list of candidate peers.
pub fn candidate_peers(&self) -> HashSet<SocketAddr> {
self.candidate_peers.read().clone()
let banned_ips = self.tcp().banned_peers().get_banned_ips();
self.candidate_peers.read().iter().filter(|peer| !banned_ips.contains(&peer.ip())).copied().collect()
}

/// Returns the list of restricted peers.
Expand Down
2 changes: 1 addition & 1 deletion node/sync/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ version = "=3.0.0"

[dependencies.snarkos-node-tcp]
path = "../tcp"
version = "=2.2.7"
version = "=3.0.0"

[dependencies.snarkvm]
workspace = true
Expand Down
13 changes: 6 additions & 7 deletions node/sync/src/block_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ use snarkos_node_router::messages::DataBlocks;
use snarkos_node_sync_communication_service::CommunicationService;
use snarkos_node_sync_locators::{CHECKPOINT_INTERVAL, NUM_RECENT_BLOCKS};
use snarkos_node_tcp::Tcp;
use snarkvm::prelude::{block::Block, Network};
use snarkvm::prelude::{Network, block::Block};

use anyhow::{Result, bail, ensure};
use indexmap::{IndexMap, IndexSet};
use itertools::Itertools;
use parking_lot::{Mutex, RwLock};
use rand::{CryptoRng, Rng, prelude::IteratorRandom};
use std::{
collections::{BTreeMap, HashSet},
collections::{BTreeMap, HashMap, HashSet},
net::{IpAddr, Ipv4Addr, SocketAddr},
sync::{
Arc,
Expand Down Expand Up @@ -95,7 +95,7 @@ pub struct BlockSync<N: Network> {
tcp: Tcp,
/// The map of peer IP to their block locators.
/// The block locators are consistent with the canonical map and every other peer's block locators.
locators: Arc<RwLock<IndexMap<SocketAddr, BlockLocators<N>>>>,
locators: Arc<RwLock<HashMap<SocketAddr, BlockLocators<N>>>>,
/// The map of peer-to-peer to their common ancestor.
/// This map is used to determine which peers to request blocks from.
common_ancestors: Arc<RwLock<IndexMap<PeerPair, u32>>>,
Expand Down Expand Up @@ -447,7 +447,7 @@ impl<N: Network> BlockSync<N> {
/// Removes the peer from the sync pool, if they exist.
pub fn remove_peer(&self, peer_ip: &SocketAddr) {
// Remove the locators entry for the given peer IP.
self.locators.write().swap_remove(peer_ip);
self.locators.write().remove(peer_ip);
// Remove all block requests to the peer.
self.remove_block_requests_to_peer(peer_ip);
}
Expand Down Expand Up @@ -712,7 +712,7 @@ impl<N: Network> BlockSync<N> {
if let Some((_, _, peer_ips)) = requests.get(height) { peer_ips.iter().for_each(|peer_ip| {
debug!("Removing peer {peer_ip} from block request {height}");
// Remove the locators entry for the given peer IP.
locators.swap_remove(peer_ip);
locators.remove(peer_ip);
if is_timeout {
peers_to_ban.insert(*peer_ip);
}
Expand All @@ -738,7 +738,6 @@ impl<N: Network> BlockSync<N> {
let tcp = self.tcp.clone();
tokio::spawn(async move {
tcp.disconnect(peer_ip).await;
trace!("Peer disconnected!");
});
}

Expand Down Expand Up @@ -982,7 +981,7 @@ mod tests {
use snarkos_node_bft_ledger_service::MockLedgerService;
use snarkvm::prelude::{Field, TestRng};

use indexmap::{indexset, IndexSet};
use indexmap::{IndexSet, indexset};
use snarkos_node_tcp::Config;
use snarkvm::ledger::committee::Committee;
use std::net::{IpAddr, Ipv4Addr};
Expand Down
25 changes: 14 additions & 11 deletions node/tcp/src/helpers/banned_peers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,27 @@ use parking_lot::RwLock;

/// Contains the ban details for a banned peer.
#[derive(Clone)]
pub struct BanConfig {
pub struct BanDetails {
/// The time when the ban was created.
banned_at: Instant,
}

impl BanConfig {
/// Creates a new ban config.
pub fn new(banned_at: Instant) -> Self {
Self { banned_at }
impl BanDetails {
/// Creates a new ban at the given time.
pub fn new() -> Self {
Self { banned_at: Instant::now() }
}
}

impl Default for BanDetails {
fn default() -> Self {
Self::new()
}
}

/// Contains the set of peers currently banned by IP.
#[derive(Default)]
pub struct BannedPeers(RwLock<HashMap<IpAddr, BanConfig>>);
pub struct BannedPeers(RwLock<HashMap<IpAddr, BanDetails>>);

impl BannedPeers {
/// Check whether the given IP address is currently banned.
Expand All @@ -48,19 +53,17 @@ impl BannedPeers {
}

/// Get ban config for the given IP address.
pub fn get_ban_config(&self, ip: IpAddr) -> Option<BanConfig> {
pub fn get_ban_config(&self, ip: IpAddr) -> Option<BanDetails> {
self.0.read().get(&ip).cloned()
}

/// Insert or update a banned IP.
pub fn update_ip_ban(&self, ip: IpAddr) {
self.0.write().insert(ip, BanConfig::new(Instant::now()));
self.0.write().insert(ip, BanDetails::default());
}

/// Remove the expired entries
pub fn remove_old_bans(&self, ban_time_in_secs: u64) {
self.0.write().retain(|_, ban_config| {
ban_config.banned_at.elapsed().as_secs() < ban_time_in_secs
});
self.0.write().retain(|_, ban_config| ban_config.banned_at.elapsed().as_secs() < ban_time_in_secs);
}
}
2 changes: 1 addition & 1 deletion node/tcp/src/helpers/known_peers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// limitations under the License.

use std::{
collections::{hash_map::Entry, HashMap},
collections::{HashMap, hash_map::Entry},
net::IpAddr,
sync::Arc,
time::Instant,
Expand Down
2 changes: 0 additions & 2 deletions node/tcp/src/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ use tokio::{
use tracing::*;

use crate::{
connections::{Connection, ConnectionSide, Connections},
protocols::{Protocol, Protocols},
BannedPeers,
Config,
KnownPeers,
Expand Down

0 comments on commit 2bdc60b

Please sign in to comment.