Fix some issues found in code review

Thanks to Mariano Sorgente for review comments.

src/full_node.py

@@ -115,12 +115,16 @@ class FullNode:
         async for msg in self._send_tips_to_farmers(Delivery.RESPOND):
             yield msg
 
-    def _need_more_peers(self):
-        conns = self.server.global_connections
-        return len(conns.get_full_node_connections()) < \
-               self.config['target_peer_count']
+    def _num_needed_peers(self):
+        diff = self.config['target_peer_count'] - \
+               len(self.server.global_connections.get_full_node_connections())
+        return diff if diff >= 0 else 0
 
     def _start_bg_tasks(self):
+        """
+        Start a background task connecting periodically to the introducer and
+        requesting the peer list.
+        """
         introducer = self.config['introducer_peer']
         introducer_peerinfo = PeerInfo(introducer['host'], introducer['port'])
         async def introducer_client():
@@ -130,7 +134,7 @@ class FullNode:
                                       Delivery.RESPOND)
 
             while True:
-                if self._need_more_peers():
+                if self._num_needed_peers():
                     if not await self.server.start_client(introducer_peerinfo,
                                                           on_connect):
                         continue
@@ -138,6 +142,11 @@ class FullNode:
 
         self._bg_tasks.add(asyncio.create_task(introducer_client()))
 
+    def _shutdown(self):
+        for task in self._bg_tasks:
+            task.cancel()
+        self._bg_tasks = set()
+
     async def _sync(self):
         """
         Performs a full sync of the blockchain.
@@ -691,9 +700,13 @@ class FullNode:
         yield OutboundMessage(NodeType.INTRODUCER, Message('', None),
                               Delivery.CLOSE)
 
-        tasks = []
         unconnected = conns.get_unconnected_peers()
-        log.info(f"Trying to connect to peers: {unconnected}")
-        for peer in unconnected:
+        to_connect = unconnected[:self._num_needed_peers()]
+        if not len(to_connect):
+            return
+
+        log.info(f"Trying to connect to peers: {to_connect}")
+        tasks = []
+        for peer in to_connect:
             tasks.append(asyncio.create_task(self.server.start_client(peer)))
         await asyncio.gather(*tasks)

src/scripts/regenerate_keys.py

@@ -30,9 +30,9 @@ def main():
     )
     parser.add_argument("-f", "--farmer", type=str2bool, nargs='?', const=True, default=True,
                         help="Regenerate farmer key")
-    parser.add_argument("-p", "--harvester", type=str2bool, nargs='?', const=True, default=True,
+    parser.add_argument("-a", "--harvester", type=str2bool, nargs='?', const=True, default=True,
                         help="Regenerate plot key seed")
-    parser.add_argument("-l", "--pool", type=str2bool, nargs='?', const=True, default=True,
+    parser.add_argument("-p", "--pool", type=str2bool, nargs='?', const=True, default=True,
                         help="Regenerate pool keys")
     args = parser.parse_args()
 

src/scripts/simulate_farming.sh

@@ -5,6 +5,6 @@
 
 _run_bg_cmd python -m src.server.start_harvester
 _run_bg_cmd python -m src.server.start_farmer
-_run_bg_cmd python -m src.server.start_full_node "127.0.0.1" 8444 -f -t -u 8222
+_run_bg_cmd python -m src.server.start_full_node "127.0.0.1" 8444 -f -u 8222
 
 wait

src/server/connection.py

@@ -129,6 +129,10 @@ class PeerConnections:
 
 
 class Peers:
+    """
+    Has the list of known full node peers that are already connected or may be
+    connected to.
+    """
     def __init__(self):
         self._peers: List[PeerInfo] = []
 
@@ -147,19 +151,10 @@ class Peers:
         except ValueError:
             return False
 
-    def randomize(self, limit: int):
-        """
-        Randomize list of peers up to the limit using Fisher-Yates shuffle.
-        """
-        last_idx = len(self._peers) - 1
-        for i in range(limit):
-            j = random.randint(i, last_idx)
-            self._peers[i], self._peers[j] = self._peers[j], self._peers[i]
-
     def get_peers(self, max_peers: int = 0, randomize: bool = False) \
             -> List[PeerInfo]:
         if not max_peers or max_peers > len(self._peers):
             max_peers = len(self._peers)
         if randomize:
-            self.randomize(max_peers)
+            random.shuffle(self._peers)
         return self._peers[:max_peers]

src/server/start_full_node.py

@@ -38,6 +38,7 @@ async def main():
     def master_close_cb():
         # Called by the UI, when node is closed, or when a signal is sent
         log.info("Closing all connections, and server...")
+        full_node._shutdown()
         server.close_all()
         global server_closed
         server_closed = True

src/types/peer_info.py

@@ -6,5 +6,6 @@ from dataclasses import dataclass
 @dataclass(frozen=True)
 @streamable
 class PeerInfo(Streamable):
+    # TODO: Change `host` type to bytes16
     host: str
     port: uint16

src/util/streamable.py

@@ -83,7 +83,7 @@ class Streamable:
             return f_type.from_bytes(f.read(size_hints[f_type.__name__]))
         if f_type is str:
             str_size: uint32 = uint32(int.from_bytes(f.read(4), "big"))
-            return bytes.decode(f.read(str_size))
+            return bytes.decode(f.read(str_size), 'utf-8')
         else:
             raise RuntimeError(f"Type {f_type} does not have parse")
 
@@ -116,7 +116,7 @@ class Streamable:
             f.write(bytes(item))
         elif f_type is str:
             f.write(uint32(len(item)).to_bytes(4, "big"))
-            f.write(item.encode())
+            f.write(item.encode('utf-8'))
         else:
             raise NotImplementedError(f"can't stream {item}, {f_type}")