feat(swarm): remove deprecated banning API

This is replaced by the `libp2p::allow_block_list` module.

Related: #3647.

Pull-Request: #3886.
This commit is contained in:
Thomas Eizinger
2023-05-08 07:50:27 +02:00
committed by GitHub
parent b507fe298f
commit b4e724dd72
5 changed files with 7 additions and 239 deletions

View File

@ -41,7 +41,6 @@ pub(crate) struct Metrics {
dial_attempt: Counter, dial_attempt: Counter,
outgoing_connection_error: Family<OutgoingConnectionErrorLabels, Counter>, outgoing_connection_error: Family<OutgoingConnectionErrorLabels, Counter>,
connected_to_banned_peer: Family<AddressLabels, Counter>,
} }
impl Metrics { impl Metrics {
@ -104,13 +103,6 @@ impl Metrics {
outgoing_connection_error.clone(), outgoing_connection_error.clone(),
); );
let connected_to_banned_peer = Family::default();
sub_registry.register(
"connected_to_banned_peer",
"Number of connection attempts to banned peer",
connected_to_banned_peer.clone(),
);
let connections_established = Family::default(); let connections_established = Family::default();
sub_registry.register( sub_registry.register(
"connections_established", "connections_established",
@ -145,7 +137,6 @@ impl Metrics {
listener_error, listener_error,
dial_attempt, dial_attempt,
outgoing_connection_error, outgoing_connection_error,
connected_to_banned_peer,
connections_establishment_duration, connections_establishment_duration,
} }
} }
@ -224,8 +215,6 @@ impl<TBvEv, THandleErr> super::Recorder<libp2p_swarm::SwarmEvent<TBvEv, THandleE
} }
} }
#[allow(deprecated)] #[allow(deprecated)]
libp2p_swarm::DialError::Banned => record(OutgoingConnectionError::Banned),
#[allow(deprecated)]
libp2p_swarm::DialError::ConnectionLimit(_) => { libp2p_swarm::DialError::ConnectionLimit(_) => {
record(OutgoingConnectionError::ConnectionLimit) record(OutgoingConnectionError::ConnectionLimit)
} }
@ -250,14 +239,6 @@ impl<TBvEv, THandleErr> super::Recorder<libp2p_swarm::SwarmEvent<TBvEv, THandleE
} }
}; };
} }
#[allow(deprecated)]
libp2p_swarm::SwarmEvent::BannedPeer { endpoint, .. } => {
self.connected_to_banned_peer
.get_or_create(&AddressLabels {
protocols: protocol_stack::as_string(endpoint.get_remote_address()),
})
.inc();
}
libp2p_swarm::SwarmEvent::NewListenAddr { address, .. } => { libp2p_swarm::SwarmEvent::NewListenAddr { address, .. } => {
self.new_listen_addr self.new_listen_addr
.get_or_create(&AddressLabels { .get_or_create(&AddressLabels {
@ -339,7 +320,6 @@ enum PeerStatus {
#[derive(EncodeLabelValue, Hash, Clone, Eq, PartialEq, Debug)] #[derive(EncodeLabelValue, Hash, Clone, Eq, PartialEq, Debug)]
enum OutgoingConnectionError { enum OutgoingConnectionError {
Banned,
ConnectionLimit, ConnectionLimit,
LocalPeerId, LocalPeerId,
NoAddresses, NoAddresses,

View File

@ -1905,9 +1905,7 @@ where
}; };
match error { match error {
#[allow(deprecated)] DialError::LocalPeerId { .. }
DialError::Banned
| DialError::LocalPeerId { .. }
| DialError::InvalidPeerId { .. } | DialError::InvalidPeerId { .. }
| DialError::WrongPeerId { .. } | DialError::WrongPeerId { .. }
| DialError::Aborted | DialError::Aborted

View File

@ -10,6 +10,10 @@
- Return a bool from `ExternalAddresses::on_swarm_event` and `ListenAddresses::on_swarm_event` indicating whether any state was changed. - Return a bool from `ExternalAddresses::on_swarm_event` and `ListenAddresses::on_swarm_event` indicating whether any state was changed.
See [PR 3865]. See [PR 3865].
- Remove deprecated banning API from `Swarm`.
Users should migrate to `libp2p::allow_block_list`.
See [PR 3886].
- Remove `ConnectionHandlerUpgrErr::Timer` variant. - Remove `ConnectionHandlerUpgrErr::Timer` variant.
This variant was never constructed and thus dead code. This variant was never constructed and thus dead code.
See [PR 3605]. See [PR 3605].
@ -19,6 +23,7 @@
[PR 3715]: https://github.com/libp2p/rust-libp2p/pull/3715 [PR 3715]: https://github.com/libp2p/rust-libp2p/pull/3715
[PR 3746]: https://github.com/libp2p/rust-libp2p/pull/3746 [PR 3746]: https://github.com/libp2p/rust-libp2p/pull/3746
[PR 3865]: https://github.com/libp2p/rust-libp2p/pull/3865 [PR 3865]: https://github.com/libp2p/rust-libp2p/pull/3865
[PR 3886]: https://github.com/libp2p/rust-libp2p/pull/3886
## 0.42.2 ## 0.42.2

View File

@ -223,8 +223,7 @@ pub enum SwarmEvent<TBehaviourOutEvent, THandlerErr> {
}, },
/// A new connection arrived on a listener and is in the process of protocol negotiation. /// A new connection arrived on a listener and is in the process of protocol negotiation.
/// ///
/// A corresponding [`ConnectionEstablished`](SwarmEvent::ConnectionEstablished), /// A corresponding [`ConnectionEstablished`](SwarmEvent::ConnectionEstablished) or
/// [`BannedPeer`](SwarmEvent::BannedPeer), or
/// [`IncomingConnectionError`](SwarmEvent::IncomingConnectionError) event will later be /// [`IncomingConnectionError`](SwarmEvent::IncomingConnectionError) event will later be
/// generated for this connection. /// generated for this connection.
IncomingConnection { IncomingConnection {
@ -256,14 +255,6 @@ pub enum SwarmEvent<TBehaviourOutEvent, THandlerErr> {
/// Error that has been encountered. /// Error that has been encountered.
error: DialError, error: DialError,
}, },
/// We connected to a peer, but we immediately closed the connection because that peer is banned.
#[deprecated(note = "Use `libp2p::allow_block_list` instead.", since = "0.42.1")]
BannedPeer {
/// Identity of the banned peer.
peer_id: PeerId,
/// Endpoint of the connection that has been closed.
endpoint: ConnectedPoint,
},
/// One of our listeners has reported a new local listening address. /// One of our listeners has reported a new local listening address.
NewListenAddr { NewListenAddr {
/// The listener that is listening on the new address. /// The listener that is listening on the new address.
@ -349,9 +340,6 @@ where
/// similar mechanisms. /// similar mechanisms.
external_addrs: Addresses, external_addrs: Addresses,
/// List of nodes for which we deny any incoming connection.
banned_peers: HashSet<PeerId>,
/// Pending event to be delivered to connection handlers /// Pending event to be delivered to connection handlers
/// (or dropped if the peer disconnected) before the `behaviour` /// (or dropped if the peer disconnected) before the `behaviour`
/// can be polled again. /// can be polled again.
@ -560,22 +548,6 @@ where
return Err(e); return Err(e);
} }
if let Some(peer_id) = peer_id {
// Check if peer is banned.
if self.banned_peers.contains(&peer_id) {
#[allow(deprecated)]
let error = DialError::Banned;
self.behaviour
.on_swarm_event(FromSwarm::DialFailure(DialFailure {
peer_id: Some(peer_id),
error: &error,
connection_id,
}));
return Err(error);
}
}
let addresses = { let addresses = {
let mut addresses_from_opts = dial_opts.get_addresses(); let mut addresses_from_opts = dial_opts.get_addresses();
@ -742,27 +714,6 @@ where
} }
} }
/// Bans a peer by its peer ID.
///
/// Any incoming connection and any dialing attempt will immediately be rejected.
/// This function has no effect if the peer is already banned.
#[deprecated(note = "Use `libp2p::allow_block_list` instead.", since = "0.42.1")]
pub fn ban_peer_id(&mut self, peer_id: PeerId) {
if self.banned_peers.insert(peer_id) {
// Note that established connections to the now banned peer are closed but not
// added to [`Swarm::banned_peer_connections`]. They have been previously reported
// as open to the behaviour and need be reported as closed once closing the
// connection finishes.
self.pool.disconnect(peer_id);
}
}
/// Unbans a peer.
#[deprecated(note = "Use `libp2p::allow_block_list` instead.", since = "0.42.1")]
pub fn unban_peer_id(&mut self, peer_id: PeerId) {
self.banned_peers.remove(&peer_id);
}
/// Disconnects a peer by its peer ID, closing all connections to said peer. /// Disconnects a peer by its peer ID, closing all connections to said peer.
/// ///
/// Returns `Ok(())` if there was one or more established connections to the peer. /// Returns `Ok(())` if there was one or more established connections to the peer.
@ -818,11 +769,6 @@ where
concurrent_dial_errors, concurrent_dial_errors,
established_in, established_in,
} => { } => {
if self.banned_peers.contains(&peer_id) {
#[allow(deprecated)]
return Some(SwarmEvent::BannedPeer { peer_id, endpoint });
}
let handler = match endpoint.clone() { let handler = match endpoint.clone() {
ConnectedPoint::Dialer { ConnectedPoint::Dialer {
address, address,
@ -1715,7 +1661,6 @@ where
supported_protocols: Default::default(), supported_protocols: Default::default(),
listened_addrs: HashMap::new(), listened_addrs: HashMap::new(),
external_addrs: Addresses::default(), external_addrs: Addresses::default(),
banned_peers: HashSet::new(),
pending_event: None, pending_event: None,
} }
} }
@ -1724,9 +1669,6 @@ where
/// Possible errors when trying to establish or upgrade an outbound connection. /// Possible errors when trying to establish or upgrade an outbound connection.
#[derive(Debug)] #[derive(Debug)]
pub enum DialError { pub enum DialError {
/// The peer is currently banned.
#[deprecated(note = "Use `libp2p::allow_block_list` instead.", since = "0.42.1")]
Banned,
/// The configured limit for simultaneous outgoing connections /// The configured limit for simultaneous outgoing connections
/// has been reached. /// has been reached.
#[deprecated( #[deprecated(
@ -1786,8 +1728,6 @@ impl fmt::Display for DialError {
f, f,
"Dial error: tried to dial local peer id at {endpoint:?}." "Dial error: tried to dial local peer id at {endpoint:?}."
), ),
#[allow(deprecated)]
DialError::Banned => write!(f, "Dial error: peer is banned."),
DialError::DialPeerConditionFalse(c) => { DialError::DialPeerConditionFalse(c) => {
write!(f, "Dial error: condition {c:?} for dialing peer was false.") write!(f, "Dial error: condition {c:?} for dialing peer was false.")
} }
@ -1838,8 +1778,6 @@ impl error::Error for DialError {
DialError::ConnectionLimit(err) => Some(err), DialError::ConnectionLimit(err) => Some(err),
DialError::LocalPeerId { .. } => None, DialError::LocalPeerId { .. } => None,
DialError::NoAddresses => None, DialError::NoAddresses => None,
#[allow(deprecated)]
DialError::Banned => None,
DialError::DialPeerConditionFalse(_) => None, DialError::DialPeerConditionFalse(_) => None,
DialError::Aborted => None, DialError::Aborted => None,
DialError::InvalidPeerId { .. } => None, DialError::InvalidPeerId { .. } => None,
@ -2121,136 +2059,6 @@ mod tests {
&& !swarm2.is_connected(swarm1.local_peer_id()) && !swarm2.is_connected(swarm1.local_peer_id())
} }
/// Establishes multiple connections between two peers,
/// after which one peer bans the other.
///
/// The test expects both behaviours to be notified via calls to [`NetworkBehaviour::on_swarm_event`]
/// with pairs of [`FromSwarm::ConnectionEstablished`] / [`FromSwarm::ConnectionClosed`]
/// while unbanned.
///
/// While the ban is in effect, further dials occur. For these connections no
/// [`FromSwarm::ConnectionEstablished`], [`FromSwarm::ConnectionClosed`]
/// calls should be registered.
#[test]
#[allow(deprecated)]
fn test_connect_disconnect_ban() {
let _ = env_logger::try_init();
// Since the test does not try to open any substreams, we can
// use keep alive protocols handler.
let handler_proto = keep_alive::ConnectionHandler;
let mut swarm1 = new_test_swarm::<_, ()>(handler_proto.clone()).build();
let mut swarm2 = new_test_swarm::<_, ()>(handler_proto).build();
let addr1: Multiaddr = multiaddr::Protocol::Memory(rand::random::<u64>()).into();
let addr2: Multiaddr = multiaddr::Protocol::Memory(rand::random::<u64>()).into();
swarm1.listen_on(addr1).unwrap();
swarm2.listen_on(addr2.clone()).unwrap();
let swarm1_id = *swarm1.local_peer_id();
#[derive(Debug)]
enum Stage {
/// Waiting for the peers to connect. Banning has not occurred.
Connecting,
/// Ban occurred.
Banned,
// Ban is in place and a dial is ongoing.
BannedDial,
// Mid-ban dial was registered and the peer was unbanned.
Unbanned,
// There are dial attempts ongoing for the no longer banned peers.
Reconnecting,
}
let num_connections = 10;
for _ in 0..num_connections {
swarm1.dial(addr2.clone()).unwrap();
}
let mut s1_expected_conns = num_connections;
let mut s2_expected_conns = num_connections;
let mut stage = Stage::Connecting;
executor::block_on(future::poll_fn(move |cx| loop {
let poll1 = Swarm::poll_next_event(Pin::new(&mut swarm1), cx);
let poll2 = Swarm::poll_next_event(Pin::new(&mut swarm2), cx);
match stage {
Stage::Connecting => {
if swarm1.behaviour.assert_connected(s1_expected_conns, 1)
&& swarm2.behaviour.assert_connected(s2_expected_conns, 1)
{
// Setup to test that already established connections are correctly closed
// and reported as such after the peer is banned.
swarm2.ban_peer_id(swarm1_id);
stage = Stage::Banned;
}
}
Stage::Banned => {
if swarm1.behaviour.assert_disconnected(s1_expected_conns, 1)
&& swarm2.behaviour.assert_disconnected(s2_expected_conns, 1)
{
// Setup to test that new connections of banned peers are not reported.
swarm1.dial(addr2.clone()).unwrap();
s1_expected_conns += 1;
stage = Stage::BannedDial;
}
}
Stage::BannedDial => {
if swarm1.behaviour.assert_disconnected(s1_expected_conns, 2) {
// The banned connection was established. Given the ban, swarm2 closed the
// connection. Check that it was not reported to the behaviour of the
// banning swarm.
assert_eq!(
swarm2.behaviour.on_connection_established.len(),
s2_expected_conns,
"No additional closed connections should be reported for the banned peer"
);
// Setup to test that the banned connection is not reported upon closing
// even if the peer is unbanned.
swarm2.unban_peer_id(swarm1_id);
stage = Stage::Unbanned;
}
}
Stage::Unbanned => {
if swarm1.network_info().num_peers() == 0
&& swarm2.network_info().num_peers() == 0
{
// The banned connection has closed. Check that it was not reported.
assert_eq!(
swarm2.behaviour.on_connection_closed.len(), s2_expected_conns,
"No additional closed connections should be reported for the banned peer"
);
// Setup to test that a ban lifted does not affect future connections.
for _ in 0..num_connections {
swarm1.dial(addr2.clone()).unwrap();
}
s1_expected_conns += num_connections;
s2_expected_conns += num_connections;
stage = Stage::Reconnecting;
}
}
Stage::Reconnecting => {
if swarm1.behaviour.on_connection_established.len() == s1_expected_conns
&& swarm2.behaviour.assert_connected(s2_expected_conns, 2)
{
return Poll::Ready(());
}
}
}
if poll1.is_pending() && poll2.is_pending() {
return Poll::Pending;
}
}))
}
/// Establishes multiple connections between two peers, /// Establishes multiple connections between two peers,
/// after which one peer disconnects the other using [`Swarm::disconnect_peer_id`]. /// after which one peer disconnects the other using [`Swarm::disconnect_peer_id`].
/// ///

View File

@ -235,29 +235,6 @@ where
.count() .count()
} }
/// Checks that when the expected number of closed connection notifications are received, a
/// given number of expected disconnections have been received as well.
///
/// Returns if the first condition is met.
pub(crate) fn assert_disconnected(
&self,
expected_closed_connections: usize,
expected_disconnections: usize,
) -> bool {
if self.on_connection_closed.len() == expected_closed_connections {
assert_eq!(
self.on_connection_closed
.iter()
.filter(|(.., remaining_established)| { *remaining_established == 0 })
.count(),
expected_disconnections
);
return true;
}
false
}
/// Checks that when the expected number of established connection notifications are received, /// Checks that when the expected number of established connection notifications are received,
/// a given number of expected connections have been received as well. /// a given number of expected connections have been received as well.
/// ///