Skip to content

Commit 05a4b16

Browse files
authored
protocols/kad/behaviour: Remove false assert on connected_peers (#2120)
Given the following scenario: 1. Remote peer X connects and is added to `connected_peers`. 2. Remote peer X opens a Kademlia substream and thus confirms that it supports the Kademlia protocol. 3. Remote peer X is added to the routing table as `Connected`. 4. Remote peer X disconnects and is thus marked as `Disconnected` in the routing table. 5. Remote peer Y connects and is added to `connected_peers`. 6. Remote peer X re-connects and is added to `connected_peers`. 7. Remote peer Y opens a Kademlia substream and thus confirms that it supports the Kademlia protocol. 8. Remote peer Y is added to the routing table. Given that the bucket is already full the call to `entry.insert` returns `kbucket::InsertResult::Pending { disconnected }` where disconnected is peer X. While peer X is in `connected_peers` it has not yet (re-) confirmed that it supports the Kademlia routing protocol and thus is still tracked as `Disconnected` in the routing table. The `debug_assert` removed in this pull request does not capture this scenario.
1 parent a414afd commit 05a4b16

File tree

2 files changed

+16
-5
lines changed

2 files changed

+16
-5
lines changed

protocols/kad/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@
1111

1212
- Expose kbucket range on `KademliaEvent::RoutingUpdated` (see [PR 2087]).
1313

14+
- Remove false `debug_assert` on `connected_peers` (see [PR 2120]).
15+
1416
[PR 2087]: https://github.com/libp2p/rust-libp2p/pull/2087
17+
[PR 2120]: https://github.com/libp2p/rust-libp2p/pull/2120
1518

1619
# 0.30.0 [2021-04-13]
1720

protocols/kad/src/behaviour.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,15 +1042,23 @@ where
10421042
));
10431043
},
10441044
kbucket::InsertResult::Pending { disconnected } => {
1045-
debug_assert!(!self.connected_peers.contains(disconnected.preimage()));
10461045
let address = addresses.first().clone();
10471046
self.queued_events.push_back(NetworkBehaviourAction::GenerateEvent(
10481047
KademliaEvent::PendingRoutablePeer { peer, address }
10491048
));
1050-
self.queued_events.push_back(NetworkBehaviourAction::DialPeer {
1051-
peer_id: disconnected.into_preimage(),
1052-
condition: DialPeerCondition::Disconnected
1053-
})
1049+
1050+
// `disconnected` might already be in the process of re-connecting.
1051+
// In other words `disconnected` might have already re-connected but
1052+
// is not yet confirmed to support the Kademlia protocol via
1053+
// [`KademliaHandlerEvent::ProtocolConfirmed`].
1054+
//
1055+
// Only try dialing peer if not currently connected.
1056+
if !self.connected_peers.contains(disconnected.preimage()) {
1057+
self.queued_events.push_back(NetworkBehaviourAction::DialPeer {
1058+
peer_id: disconnected.into_preimage(),
1059+
condition: DialPeerCondition::Disconnected
1060+
})
1061+
}
10541062
},
10551063
}
10561064
}

0 commit comments

Comments
 (0)