Skip to content

Commit

Permalink
p2p ban tests were rewritten
Browse files Browse the repository at this point in the history
  • Loading branch information
ImplOfAnImpl committed Jan 15, 2024
1 parent 00552ba commit d1a4811
Show file tree
Hide file tree
Showing 12 changed files with 872 additions and 353 deletions.
18 changes: 17 additions & 1 deletion p2p/p2p-test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl P2pBasicTestTimeGetter {
}

/// A timeout for blocking calls.
pub const LONG_TIMEOUT: Duration = Duration::from_secs(600);
pub const LONG_TIMEOUT: Duration = Duration::from_secs(30); // FIXME
/// A short timeout for events that shouldn't occur.
pub const SHORT_TIMEOUT: Duration = Duration::from_millis(500);

Expand Down Expand Up @@ -221,6 +221,7 @@ macro_rules! expect_no_recv {
};
}

/// Wait until the specified value is received from the channel.
pub async fn wait_for_recv<T: Eq>(receiver: &mut mpsc::UnboundedReceiver<T>, value: &T) {
let wait_loop = async {
loop {
Expand All @@ -232,3 +233,18 @@ pub async fn wait_for_recv<T: Eq>(receiver: &mut mpsc::UnboundedReceiver<T>, val

expect_future_val!(wait_loop);
}

/// Wait until the sender stops putting messages into the channel.
pub async fn wait_for_no_recv<T>(receiver: &mut mpsc::UnboundedReceiver<T>) {
let wait_loop = async {
loop {
let wait_result =
tokio::time::timeout(Duration::from_millis(100), receiver.recv()).await;
if wait_result.is_err() {
break;
}
}
};

expect_future_val!(wait_loop);
}
2 changes: 1 addition & 1 deletion p2p/src/ban_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ make_config_setting!(
);

/// Settings related to banning and discouragement.
#[derive(Default, Debug)]
#[derive(Default, Debug, Clone)]
pub struct BanConfig {
/// The score threshold after which a peer is banned.
pub ban_threshold: BanThreshold,
Expand Down
15 changes: 15 additions & 0 deletions p2p/src/peer_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1291,6 +1291,8 @@ where

peer.announced_addresses.insert(&address, &mut make_pseudo_rng());

// FIXME: check for ban etc here

self.peerdb.peer_discovered(address);

let peer_ids = self
Expand Down Expand Up @@ -1902,6 +1904,9 @@ pub trait Observer {
pub trait PeerManagerQueryInterface {
#[cfg(test)]
fn peers(&self) -> &BTreeMap<PeerId, PeerContext>;

#[cfg(test)]
fn peer_db(&self) -> &dyn peerdb::PeerDbQueryInterface;
}

impl<T, S> PeerManagerQueryInterface for PeerManager<T, S>
Expand All @@ -1914,7 +1919,17 @@ where
fn peers(&self) -> &BTreeMap<PeerId, PeerContext> {
&self.peers
}

#[cfg(test)]
fn peer_db(&self) -> &dyn peerdb::PeerDbQueryInterface {
&self.peerdb
}
}

#[cfg(test)]
mod tests;

#[cfg(test)]
pub mod test_utils {
pub use super::tests::utils::*;
}
15 changes: 15 additions & 0 deletions p2p/src/peer_manager/peerdb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,21 @@ impl<S: PeerDbStorage> PeerDb<S> {
}
}

pub trait PeerDbQueryInterface {
fn is_address_banned(&self, address: &BannableAddress) -> bool;
fn is_address_discouraged(&self, address: &BannableAddress) -> bool;
}

impl<S: PeerDbStorage> PeerDbQueryInterface for PeerDb<S> {
fn is_address_banned(&self, address: &BannableAddress) -> bool {
self.is_address_banned(address)
}

fn is_address_discouraged(&self, address: &BannableAddress) -> bool {
self.is_address_discouraged(address)
}
}

#[cfg(test)]
mod tests;

Expand Down
49 changes: 18 additions & 31 deletions p2p/src/peer_manager/tests/addr_list_response_caching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,28 @@ use crate::{
default_backend::{
transport::TcpTransportSocket,
types::{Command, Message},
ConnectivityHandle, DefaultNetworkingService,
DefaultNetworkingService,
},
types::{PeerInfo, Role},
ConnectivityService, NetworkingService,
},
peer_manager::{
addr_list_response_cache,
peerdb::{
address_tables::table::test_utils::make_non_colliding_addresses,
storage::PeerDbStorage, storage_impl::PeerDbStorageImpl,
address_tables::table::test_utils::make_non_colliding_addresses, storage::PeerDbStorage,
},
PeerManager,
},
protocol::ProtocolConfig,
testing_utils::{peerdb_inmemory_store, TestAddressMaker, TEST_PROTOCOL_VERSION},
testing_utils::{TestAddressMaker, TEST_PROTOCOL_VERSION},
types::peer_id::PeerId,
PeerManagerEvent,
};

use super::make_standalone_peer_manager;

// Note: addr list requests are only handled for inbound peers, so we only test this variant.

type TestNetworkingService = DefaultNetworkingService<TcpTransportSocket>;
type TestPeerManager = PeerManager<
DefaultNetworkingService<TcpTransportSocket>,
PeerDbStorageImpl<storage::inmemory::InMemory>,
>;

#[tracing::instrument(skip(seed))]
#[rstest]
Expand Down Expand Up @@ -272,28 +268,19 @@ fn setup_peer_mgr(
p2p_config: &Arc<P2pConfig>,
time_getter: &P2pBasicTestTimeGetter,
rng: &mut impl Rng,
) -> (TestPeerManager, UnboundedReceiver<Command>) {
let (cmd_sender, cmd_receiver) = tokio::sync::mpsc::unbounded_channel();
let (_conn_event_sender, conn_event_receiver) = tokio::sync::mpsc::unbounded_channel();
let (_peer_mgr_event_sender, peer_mgr_event_receiver) =
tokio::sync::mpsc::unbounded_channel::<PeerManagerEvent>();
let connectivity_handle = ConnectivityHandle::<TestNetworkingService>::new(
// Note: technically, we should pass the bind addresses here, but since we don't
// establish real connections, it doesn't really matter.
vec![],
cmd_sender,
conn_event_receiver,
);

let mut peer_mgr = PeerManager::<TestNetworkingService, _>::new(
Arc::clone(chain_config),
Arc::clone(p2p_config),
connectivity_handle,
peer_mgr_event_receiver,
time_getter.get_time_getter(),
peerdb_inmemory_store(),
)
.unwrap();
) -> (
PeerManager<TestNetworkingService, impl PeerDbStorage>,
UnboundedReceiver<Command>,
) {
let (mut peer_mgr, _conn_event_sender, _peer_mgr_event_sender, cmd_receiver, _) =
make_standalone_peer_manager(
Arc::clone(chain_config),
Arc::clone(p2p_config),
// Note: technically, we should pass the bind addresses here, but since we don't
// establish real connections, it doesn't really matter.
vec![],
time_getter.get_time_getter(),
);

let addresses_in_db = make_non_colliding_addresses(
&[peer_mgr.peerdb.address_tables().new_addr_table()],
Expand Down
Loading

0 comments on commit d1a4811

Please sign in to comment.