Add Peer::uselessPeer / log error when no peer found

Rather than use a magic number to indicate a useless peer (numeric_limits<unsigned>::max()), add a function to Peer (Peer::uselessPeer) which can be called to detect this. Also update Peer::fallbackSeconds() to return a number which is large enough to prevent us from realistically ever connecting to a useless peer but which is small enough to not overflow when added to Peer::m_lastAttempted.

Also, log a message with error verbosity in Host::handshakeFailed() if a peer is not found rather than asserting, since while we never expect users to hit this case, the EIP8 handshake tests can.

Author: halfalicious

libp2p/Host.cpp

@@ -211,7 +211,11 @@ std::shared_ptr<Peer> Host::peer(NodeID const& _n) const
 void Host::handshakeFailed(NodeID const& _n, HandshakeFailureReason _r)
 {
     std::shared_ptr<Peer> p = peer(_n);
-    assert(p);
+    if (!p)
+    {
+        cerror << "Peer " << _n << " not found";
+        return;
+    }
     p->m_lastHandshakeFailure = _r;
 }
 
@@ -812,7 +816,7 @@ void Host::run(boost::system::error_code const& _ec)
                     reqConn++;
                 else if (!haveSession)
                 {
-                    if (p->second->fallbackSeconds() == numeric_limits<unsigned>::max())
+                    if (p->second->uselessPeer())
                         p = m_peers.erase(p);
                     else if (p->second->shouldReconnect() && (!m_netConfig.pin || required))
                         toConnect.push_back(p->second);

libp2p/Peer.cpp

@@ -49,17 +49,45 @@ bool Peer::shouldReconnect() const
            fallbackSeconds() != numeric_limits<unsigned>::max() &&
            chrono::system_clock::now() > m_lastAttempted + chrono::seconds(fallbackSeconds());
 }
-    
+
+bool Peer::uselessPeer() const
+{
+    switch (m_lastHandshakeFailure)
+    {
+    case FrameDecryptionFailure:
+    case ProtocolError:
+        return true;
+    default:
+        break;
+    }
+
+    switch (m_lastDisconnect)
+    {
+    case BadProtocol:
+    case UselessPeer:
+    case IncompatibleProtocol:
+    case UnexpectedIdentity:
+    case UserReason:
+        return true;
+    default:
+        break;
+    }
+    return false;
+}
+
+
 unsigned Peer::fallbackSeconds() const
 {
+    constexpr unsigned oneYearInSeconds{60 * 60 * 24 * 360};
+
     if (peerType == PeerType::Required)
         return 5;
 
     switch (m_lastHandshakeFailure)
     {
         case FrameDecryptionFailure:
         case ProtocolError:
-            return numeric_limits<unsigned>::max();
+            return oneYearInSeconds;
         default:
             break;
     }
@@ -71,7 +99,7 @@ unsigned Peer::fallbackSeconds() const
     case IncompatibleProtocol:
     case UnexpectedIdentity:
     case UserReason:
-        return numeric_limits<unsigned>::max();
+        return oneYearInSeconds;
     case TooManyPeers:
         return 25 * (m_failedAttempts + 1);
     case ClientQuit:

libp2p/Peer.h

@@ -70,6 +70,10 @@ class Peer: public Node
     /// Return true if connection attempt should be made to this peer or false if
     bool shouldReconnect() const;
 
+    /// A peer which should never be reconnected to - e.g. it's running on a different network, we
+    /// don't have any common capabilities
+    bool uselessPeer() const;
+
     /// Number of times connection has been attempted to peer.
     int failedAttempts() const { return m_failedAttempts; }