Pull request feedback

libp2p/host/routed_host.py

@@ -30,7 +30,7 @@ class RoutedHost(BasicHost):
             found_peer_info = await self._router.find_peer(peer_info.peer_id)
             if not found_peer_info:
                 raise ConnectionFailure("Unable to find Peer address")
-            peer_info.addrs = found_peer_info.addrs
+            self.peerstore.add_addrs(peer_info.peer_id, found_peer_info.addrs, 10)
         self.peerstore.add_addrs(peer_info.peer_id, peer_info.addrs, 10)
 
         # there is already a connection to this peer

libp2p/peer/peerinfo.py

@@ -1,4 +1,3 @@
-import json
 from typing import Any, List, Sequence
 
 import multiaddr
@@ -14,11 +13,6 @@ class PeerInfo:
         self.peer_id = peer_id
         self.addrs = list(addrs)
 
-    def to_string(self) -> str:
-        return json.dumps(
-            [self.peer_id.to_string(), list(map(lambda a: str(a), self.addrs))]
-        )
-
     def __eq__(self, other: Any) -> bool:
         return (
             isinstance(other, PeerInfo)
@@ -26,14 +20,6 @@ class PeerInfo:
             and self.addrs == other.addrs
         )
 
-    @classmethod
-    def info_from_string(cls, info: str) -> "PeerInfo":
-        peer_id, raw_addrs = json.loads(info)
-        return PeerInfo(
-            ID.from_base58(peer_id),
-            list(map(lambda a: multiaddr.Multiaddr(a), raw_addrs)),
-        )
-
 
 def info_from_p2p_addr(addr: multiaddr.Multiaddr) -> PeerInfo:
     if not addr:

libp2p/routing/kademlia/kademlia_peer_router.py

@@ -1,3 +1,7 @@
+import json
+
+import multiaddr
+
 from libp2p.kademlia.network import KademliaServer
 from libp2p.peer.id import ID
 from libp2p.peer.peerinfo import PeerInfo
@@ -21,5 +25,18 @@ class KadmeliaPeerRouter(IPeerRouting):
         # ignore type for kad
         value = await self.server.get(xor_id)  # type: ignore
         return (
-            PeerInfo.info_from_string(value) if value else None
+            peer_info_from_str(value) if value else None
         )  # TODO: should raise error if None?
+
+
+def peer_info_to_str(peer_info: PeerInfo) -> str:
+    return json.dumps(
+        [peer_info.peer_id.to_string(), list(map(lambda a: str(a), peer_info.addrs))]
+    )
+
+
+def peer_info_from_str(string: str) -> PeerInfo:
+    peer_id, raw_addrs = json.loads(string)
+    return PeerInfo(
+        ID.from_base58(peer_id), list(map(lambda a: multiaddr.Multiaddr(a), raw_addrs))
+    )

tests/host/test_routed_host.py

@@ -4,6 +4,7 @@ import pytest
 
 from libp2p.host.exceptions import ConnectionFailure
 from libp2p.peer.peerinfo import PeerInfo
+from libp2p.routing.kademlia.kademlia_peer_router import peer_info_to_str
 from tests.utils import (
     set_up_nodes_by_transport_and_disc_opt,
     set_up_nodes_by_transport_opt,
@@ -23,11 +24,11 @@ async def test_host_routing_success():
     # Set routing info
     await routers[0].server.set(
         host_a.get_id().xor_id,
-        PeerInfo(host_a.get_id(), host_a.get_addrs()).to_string(),
+        peer_info_to_str(PeerInfo(host_a.get_id(), host_a.get_addrs())),
     )
     await routers[1].server.set(
         host_b.get_id().xor_id,
-        PeerInfo(host_b.get_id(), host_b.get_addrs()).to_string(),
+        peer_info_to_str(PeerInfo(host_b.get_id(), host_b.get_addrs())),
     )
 
     # forces to use routing as no addrs are provided
@@ -54,11 +55,11 @@ async def test_host_routing_fail():
     # Set routing info
     await routers[0].server.set(
         host_a.get_id().xor_id,
-        PeerInfo(host_a.get_id(), host_a.get_addrs()).to_string(),
+        peer_info_to_str(PeerInfo(host_a.get_id(), host_a.get_addrs())),
     )
     await routers[1].server.set(
         host_b.get_id().xor_id,
-        PeerInfo(host_b.get_id(), host_b.get_addrs()).to_string(),
+        peer_info_to_str(PeerInfo(host_b.get_id(), host_b.get_addrs())),
     )
 
     # routing fails because host_c does not use routing

tests/routing/test_kad_peer_router.py

@@ -2,7 +2,10 @@ import pytest
 
 from libp2p.kademlia.network import KademliaServer
 from libp2p.peer.id import ID
-from libp2p.routing.kademlia.kademlia_peer_router import KadmeliaPeerRouter
+from libp2p.routing.kademlia.kademlia_peer_router import (
+    KadmeliaPeerRouter,
+    peer_info_to_str,
+)
 
 
 @pytest.mark.asyncio
@@ -15,7 +18,7 @@ async def test_simple_two_nodes():
 
     node_a_value = await node_b.bootstrap([("127.0.0.1", 5678)])
     node_a_kad_peerinfo = node_a_value[0]
-    await node_a.set(node_a_kad_peerinfo.xor_id, node_a_kad_peerinfo.to_string())
+    await node_a.set(node_a_kad_peerinfo.xor_id, peer_info_to_str(node_a_kad_peerinfo))
 
     router = KadmeliaPeerRouter(node_b)
     returned_info = await router.find_peer(ID(node_a_kad_peerinfo.peer_id_bytes))
@@ -37,7 +40,7 @@ async def test_simple_three_nodes():
     node_a_kad_peerinfo = node_a_value[0]
 
     await node_c.bootstrap([("127.0.0.1", 5702)])
-    await node_a.set(node_a_kad_peerinfo.xor_id, node_a_kad_peerinfo.to_string())
+    await node_a.set(node_a_kad_peerinfo.xor_id, peer_info_to_str(node_a_kad_peerinfo))
 
     router = KadmeliaPeerRouter(node_c)
     returned_info = await router.find_peer(ID(node_a_kad_peerinfo.peer_id_bytes))
@@ -65,7 +68,7 @@ async def test_simple_four_nodes():
 
     await node_d.bootstrap([("127.0.0.1", 5803)])
 
-    await node_b.set(node_a_kad_peerinfo.xor_id, node_a_kad_peerinfo.to_string())
+    await node_b.set(node_a_kad_peerinfo.xor_id, peer_info_to_str(node_a_kad_peerinfo))
 
     router = KadmeliaPeerRouter(node_d)
     returned_info = await router.find_peer(ID(node_a_kad_peerinfo.peer_id_bytes))