Skip to content

Commit

Permalink
feat(swarm)!: report connections to our own PeerId in separate error (
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
melekes authored Jan 26, 2023
1 parent 90af08c commit 9f71459
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 10 deletions.
6 changes: 5 additions & 1 deletion misc/metrics/src/swarm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl<TBvEv, THandleErr> super::Recorder<libp2p_swarm::SwarmEvent<TBvEv, THandleE
libp2p_swarm::DialError::ConnectionLimit(_) => {
record(OutgoingConnectionErrorError::ConnectionLimit)
}
libp2p_swarm::DialError::LocalPeerId => {
libp2p_swarm::DialError::LocalPeerId { .. } => {
record(OutgoingConnectionErrorError::LocalPeerId)
}
libp2p_swarm::DialError::NoAddresses => {
Expand Down Expand Up @@ -361,6 +361,7 @@ struct IncomingConnectionErrorLabels {
#[derive(EncodeLabelValue, Hash, Clone, Eq, PartialEq, Debug)]
enum PendingInboundConnectionError {
WrongPeerId,
LocalPeerId,
TransportErrorMultiaddrNotSupported,
TransportErrorOther,
Aborted,
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion protocols/kad/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1932,7 +1932,7 @@ where
match error {
DialError::Banned
| DialError::ConnectionLimit(_)
| DialError::LocalPeerId
| DialError::LocalPeerId { .. }
| DialError::InvalidPeerId { .. }
| DialError::WrongPeerId { .. }
| DialError::Aborted
Expand Down
3 changes: 3 additions & 0 deletions swarm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
12 changes: 11 additions & 1 deletion swarm/src/connection/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,14 @@ pub enum PendingConnectionError<TTransErr> {
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<T> PendingConnectionError<T> {
Expand All @@ -114,6 +117,9 @@ impl<T> PendingConnectionError<T> {
PendingConnectionError::WrongPeerId { obtained, endpoint } => {
PendingConnectionError::WrongPeerId { obtained, endpoint }
}
PendingConnectionError::LocalPeerId { endpoint } => {
PendingConnectionError::LocalPeerId { endpoint }
}
}
}
}
Expand All @@ -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:?}.")
}
}
}
}
Expand All @@ -152,6 +161,7 @@ where
match self {
PendingConnectionError::Transport(_) => None,
PendingConnectionError::WrongPeerId { .. } => None,
PendingConnectionError::LocalPeerId { .. } => None,
PendingConnectionError::Aborted => None,
PendingConnectionError::ConnectionLimit(..) => None,
}
Expand Down
3 changes: 1 addition & 2 deletions swarm/src/connection/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 9 additions & 5 deletions swarm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1611,6 +1611,7 @@ impl From<PendingOutboundConnectionError> for DialError {
PendingConnectionError::WrongPeerId { obtained, endpoint } => {
DialError::WrongPeerId { obtained, endpoint }
}
PendingConnectionError::LocalPeerId { endpoint } => DialError::LocalPeerId { endpoint },
PendingConnectionError::Transport(e) => DialError::Transport(e),
}
}
Expand All @@ -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.")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit 9f71459

Please sign in to comment.