Add addPeer proc and more comments

eth_p2p.nim

@@ -59,13 +59,10 @@ proc processIncoming(server: StreamServer,
   if not peerfut.failed:
     let peer = peerfut.read()
     if node.peerPool != nil:
-      if peer.remote notin node.peerPool.connectedNodes:
-        node.peerPool.connectedNodes[peer.remote] = peer
-        for o in node.peerPool.observers.values:
-          if not o.onPeerConnected.isNil:
-            o.onPeerConnected(peer)
-      else:
-        debug "Disconnecting already connected node"
+      if not node.peerPool.addPeer(peer):
+        # In case an outgoing connection was added in the meanwhile or a
+        # malicious peer opens multiple connections
+        debug "Disconnecting peer (incoming)", reason = AlreadyConnected
         await peer.disconnect(AlreadyConnected)
   else:
     remote.close()

eth_p2p/peer_pool.nim

@@ -102,18 +102,22 @@ proc lookupRandomNode(p: PeerPool) {.async.} =
 proc getRandomBootnode(p: PeerPool): Node =
   p.discovery.bootstrapNodes.rand()
 
+proc addPeer*(pool: PeerPool, peer: Peer): bool =
+  if peer.remote notin pool.connectedNodes:
+    pool.connectedNodes[peer.remote] = peer
+    for o in pool.observers.values:
+      if not o.onPeerConnected.isNil:
+        o.onPeerConnected(peer)
+    return true
+  else: return false
+
 proc connectToNode*(p: PeerPool, n: Node) {.async.} =
   let peer = await p.connect(n)
   if not peer.isNil:
     info "Connection established", peer
-    if peer.remote notin p.connectedNodes:
-      p.connectedNodes[peer.remote] = peer
-      for o in p.observers.values:
-        if not o.onPeerConnected.isNil:
-          o.onPeerConnected(peer)
-    else:
+    if not p.addPeer(peer):
       # In case an incoming connection was added in the meanwhile
-      debug "Disconnecting peer", reason = AlreadyConnected
+      debug "Disconnecting peer (outgoing)", reason = AlreadyConnected
       await peer.disconnect(AlreadyConnected)
 
 proc connectToNodes(p: PeerPool, nodes: seq[Node]) {.async.} =

eth_p2p/rlpx.nim

@@ -1108,6 +1108,10 @@ rlpxProtocol p2p(version = 0):
     discard
 
 proc removePeer(network: EthereumNode, peer: Peer) =
+  # It is necessary to check if peer.remote still exists. The connection might
+  # have been dropped already from the peers side.
+  # E.g. when receiving a p2p.disconnect message from a peer, a race will happen
+  # between which side disconnects first.
   if network.peerPool != nil and not peer.remote.isNil:
     network.peerPool.connectedNodes.del(peer.remote)