From 9f7145912a3f4e4ada8bd6802392dc49e5acc18e Mon Sep 17 00:00:00 2001 From: Anton Date: Thu, 26 Jan 2023 11:20:23 +0400 Subject: [PATCH] feat(swarm)!: report connections to our own `PeerId` in separate error (#3377) Previously, inbound connections that happened to resolve to our own `PeerId` were reported as `WrongPeerId`. With this patch, we now report those in a dedicated `LocalPeerId` error. Related: #3205. --- misc/metrics/src/swarm.rs | 6 +++++- protocols/kad/src/behaviour.rs | 2 +- swarm/CHANGELOG.md | 3 +++ swarm/src/connection/error.rs | 12 +++++++++++- swarm/src/connection/pool.rs | 3 +-- swarm/src/lib.rs | 14 +++++++++----- 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/misc/metrics/src/swarm.rs b/misc/metrics/src/swarm.rs index cb376cbaeb2..17de48b634c 100644 --- a/misc/metrics/src/swarm.rs +++ b/misc/metrics/src/swarm.rs @@ -228,7 +228,7 @@ impl super::Recorder { record(OutgoingConnectionErrorError::ConnectionLimit) } - libp2p_swarm::DialError::LocalPeerId => { + libp2p_swarm::DialError::LocalPeerId { .. } => { record(OutgoingConnectionErrorError::LocalPeerId) } libp2p_swarm::DialError::NoAddresses => { @@ -361,6 +361,7 @@ struct IncomingConnectionErrorLabels { #[derive(EncodeLabelValue, Hash, Clone, Eq, PartialEq, Debug)] enum PendingInboundConnectionError { WrongPeerId, + LocalPeerId, TransportErrorMultiaddrNotSupported, TransportErrorOther, Aborted, @@ -373,6 +374,9 @@ impl From<&libp2p_swarm::PendingInboundConnectionError> for PendingInboundConnec libp2p_swarm::PendingInboundConnectionError::WrongPeerId { .. } => { PendingInboundConnectionError::WrongPeerId } + libp2p_swarm::PendingInboundConnectionError::LocalPeerId { .. } => { + PendingInboundConnectionError::LocalPeerId + } libp2p_swarm::PendingInboundConnectionError::ConnectionLimit(_) => { PendingInboundConnectionError::ConnectionLimit } diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index 66e1b426ad7..27450b2a6b4 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -1932,7 +1932,7 @@ where match error { DialError::Banned | DialError::ConnectionLimit(_) - | DialError::LocalPeerId + | DialError::LocalPeerId { .. } | DialError::InvalidPeerId { .. } | DialError::WrongPeerId { .. } | DialError::Aborted diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index b05af853d39..9666daa4d75 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -22,6 +22,8 @@ The default values remains 7. If you have previously set `connection_event_buffer_size` you should re-evaluate what a good size for a _per connection_ buffer is. See [PR 3188]. + +- Add `PendingConnectionError::LocalPeerId` to differentiate wrong VS local peer ID errors. See [PR 3377]. - Remove `PendingConnectionError:::IO` variant. This was never constructed. @@ -35,6 +37,7 @@ [PR 3272]: https://github.com/libp2p/rust-libp2p/pull/3272 [PR 3327]: https://github.com/libp2p/rust-libp2p/pull/3327 [PR 3188]: https://github.com/libp2p/rust-libp2p/pull/3188 +[PR 3377]: https://github.com/libp2p/rust-libp2p/pull/3377 [PR 3373]: https://github.com/libp2p/rust-libp2p/pull/3373 # 0.41.1 diff --git a/swarm/src/connection/error.rs b/swarm/src/connection/error.rs index 7b9217ad352..a5a6136f0b1 100644 --- a/swarm/src/connection/error.rs +++ b/swarm/src/connection/error.rs @@ -96,11 +96,14 @@ pub enum PendingConnectionError { Aborted, /// The peer identity obtained on the connection did not - /// match the one that was expected or is the local one. + /// match the one that was expected. WrongPeerId { obtained: PeerId, endpoint: ConnectedPoint, }, + + /// The connection was dropped because it resolved to our own [`PeerId`]. + LocalPeerId { endpoint: ConnectedPoint }, } impl PendingConnectionError { @@ -114,6 +117,9 @@ impl PendingConnectionError { PendingConnectionError::WrongPeerId { obtained, endpoint } => { PendingConnectionError::WrongPeerId { obtained, endpoint } } + PendingConnectionError::LocalPeerId { endpoint } => { + PendingConnectionError::LocalPeerId { endpoint } + } } } } @@ -140,6 +146,9 @@ where "Pending connection: Unexpected peer ID {obtained} at {endpoint:?}." ) } + PendingConnectionError::LocalPeerId { endpoint } => { + write!(f, "Pending connection: Local peer ID at {endpoint:?}.") + } } } } @@ -152,6 +161,7 @@ where match self { PendingConnectionError::Transport(_) => None, PendingConnectionError::WrongPeerId { .. } => None, + PendingConnectionError::LocalPeerId { .. } => None, PendingConnectionError::Aborted => None, PendingConnectionError::ConnectionLimit(..) => None, } diff --git a/swarm/src/connection/pool.rs b/swarm/src/connection/pool.rs index 1396354f24d..c993afeaa02 100644 --- a/swarm/src/connection/pool.rs +++ b/swarm/src/connection/pool.rs @@ -686,8 +686,7 @@ where // Check peer is not local peer. .and_then(|()| { if self.local_id == obtained_peer_id { - Err(PendingConnectionError::WrongPeerId { - obtained: obtained_peer_id, + Err(PendingConnectionError::LocalPeerId { endpoint: endpoint.clone(), }) } else { diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 6026d8d3eb3..66ef8e7419d 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1580,8 +1580,8 @@ pub enum DialError { /// The configured limit for simultaneous outgoing connections /// has been reached. ConnectionLimit(ConnectionLimit), - /// The peer being dialed is the local peer and thus the dial was aborted. - LocalPeerId, + /// The peer identity obtained on the connection matches the local peer. + LocalPeerId { endpoint: ConnectedPoint }, /// [`NetworkBehaviour::addresses_of_peer`] returned no addresses /// for the peer to dial. NoAddresses, @@ -1611,6 +1611,7 @@ impl From for DialError { PendingConnectionError::WrongPeerId { obtained, endpoint } => { DialError::WrongPeerId { obtained, endpoint } } + PendingConnectionError::LocalPeerId { endpoint } => DialError::LocalPeerId { endpoint }, PendingConnectionError::Transport(e) => DialError::Transport(e), } } @@ -1621,7 +1622,10 @@ impl fmt::Display for DialError { match self { DialError::ConnectionLimit(err) => write!(f, "Dial error: {err}"), DialError::NoAddresses => write!(f, "Dial error: no addresses for peer."), - DialError::LocalPeerId => write!(f, "Dial error: tried to dial local peer id."), + DialError::LocalPeerId { endpoint } => write!( + f, + "Dial error: tried to dial local peer id at {endpoint:?}." + ), DialError::Banned => write!(f, "Dial error: peer is banned."), DialError::DialPeerConditionFalse(c) => { write!(f, "Dial error: condition {c:?} for dialing peer was false.") @@ -1671,7 +1675,7 @@ impl error::Error for DialError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { match self { DialError::ConnectionLimit(err) => Some(err), - DialError::LocalPeerId => None, + DialError::LocalPeerId { .. } => None, DialError::NoAddresses => None, DialError::Banned => None, DialError::DialPeerConditionFalse(_) => None, @@ -2487,7 +2491,7 @@ mod tests { match swarm.poll_next_unpin(cx) { Poll::Ready(Some(SwarmEvent::OutgoingConnectionError { peer_id, - error: DialError::WrongPeerId { .. }, + error: DialError::LocalPeerId { .. }, .. })) => { assert_eq!(&peer_id.unwrap(), swarm.local_peer_id());