Merge #454

454: Fix connecting to peers r=koivunej a=koivunej

The correct way to connect to a peer is in my understanding to DialPeer and then proceed to give the address of that said peer from `NetworkBehaviour::addresses_of_peer`. While doing this, the previous workaround for missing events from banning a peer (the way we force a disconnect) was also necessary to fix.

Co-authored-by: Joonas Koivunen <joonas.koivunen@gmail.com>

Author: bors[bot]

CHANGELOG.md

@@ -7,6 +7,7 @@
 * feat: `sled` pinstore [#439], [#442], [#444]
 * chore: update a lot of dependencies including libp2p, tokio, warp [#446]
 * fix: rename spans (part of [#453])
+* fix: connect using DialPeer instead of DialAddress [#454]
 
 [#429]: https://github.com/rs-ipfs/rust-ipfs/pull/429
 [#428]: https://github.com/rs-ipfs/rust-ipfs/pull/428
@@ -17,6 +18,7 @@
 [#444]: https://github.com/rs-ipfs/rust-ipfs/pull/444
 [#446]: https://github.com/rs-ipfs/rust-ipfs/pull/446
 [#453]: https://github.com/rs-ipfs/rust-ipfs/pull/453
+[#454]: https://github.com/rs-ipfs/rust-ipfs/pull/454
 
 # 0.2.1
 

src/lib.rs

@@ -656,7 +656,8 @@ impl<Types: IpfsTypes> Ipfs<Types> {
     /// Accepts only multiaddresses with the PeerId to authenticate the connection.
     ///
     /// Returns a future which will complete when the connection has been successfully made or
-    /// failed for whatever reason.
+    /// failed for whatever reason. It is possible for this method to return an error, while ending
+    /// up being connected to the peer by the means of another connection.
     pub async fn connect(&self, target: MultiaddrWithPeerId) -> Result<(), Error> {
         async move {
             let (tx, rx) = oneshot_channel();
@@ -716,6 +717,8 @@ impl<Types: IpfsTypes> Ipfs<Types> {
     ///
     /// At the moment the peer is disconnected by temporarily banning the peer and unbanning it
     /// right after. This should always disconnect all connections to the peer.
+    ///
+    /// Note: this is rarely needed in pratice as connections will time out if left unused.
     pub async fn disconnect(&self, target: MultiaddrWithPeerId) -> Result<(), Error> {
         async move {
             let (tx, rx) = oneshot_channel();
@@ -1510,7 +1513,7 @@ impl<TRepoTypes: RepoTypes> Future for IpfsFuture<TRepoTypes> {
                         let _ = ret.send(peers);
                     }
                     IpfsEvent::FindPeer(peer_id, local_only, ret) => {
-                        let swarm_addrs = self.swarm.swarm.addresses_of_peer(&peer_id);
+                        let swarm_addrs = self.swarm.swarm.connections_to(&peer_id);
                         let locally_known_addrs = if !swarm_addrs.is_empty() {
                             swarm_addrs
                         } else {

src/p2p/addr.rs

@@ -34,6 +34,12 @@ impl std::error::Error for MultiaddrWrapperError {}
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct MultiaddrWithoutPeerId(Multiaddr);
 
+impl fmt::Display for MultiaddrWithoutPeerId {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Display::fmt(&self.0, fmt)
+    }
+}
+
 impl TryFrom<Multiaddr> for MultiaddrWithoutPeerId {
     type Error = MultiaddrWrapperError;
 
@@ -77,6 +83,20 @@ impl FromStr for MultiaddrWithoutPeerId {
     }
 }
 
+impl PartialEq<Multiaddr> for MultiaddrWithoutPeerId {
+    fn eq(&self, other: &Multiaddr) -> bool {
+        &self.0 == other
+    }
+}
+
+impl MultiaddrWithoutPeerId {
+    /// Adds the peer_id information to this address without peer_id, turning it into
+    /// [`MultiaddrWithPeerId`].
+    pub fn with(self, peer_id: PeerId) -> MultiaddrWithPeerId {
+        (self, peer_id).into()
+    }
+}
+
 /// A `Multiaddr` paired with a discrete `PeerId`. The `Multiaddr` can contain a
 /// `Protocol::P2p`, but it's not as easy to work with, and some functionalities
 /// don't support it being contained within the `Multiaddr`.
@@ -147,7 +167,7 @@ impl FromStr for MultiaddrWithPeerId {
 
 impl fmt::Display for MultiaddrWithPeerId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}/p2p/{}", self.multiaddr.as_ref(), self.peer_id)
+        write!(f, "{}/p2p/{}", self.multiaddr, self.peer_id)
     }
 }