net: notice a peer that has gone quiet, and stay audible to peers that check (#55)

A connection whose path dies silently -- NAT drops an idle mapping, a relay
restarts, a route changes -- never delivers FIN. Nothing becomes readable,
no error is raised, and the node simply stops hearing from that peer while
the socket looks perfectly healthy. #51 bounded this for the rendezvous
registration; the connection that comes out of the rendezvous had the same
hole, and so did every ordinary peer.

Stamp nLastRecv and nLastSend on the socket path and act on them:

  - nothing at all within 60s of connecting, disconnect. In testing this
    fired twice per run against dead .btf descriptors handed out by the
    relay, which is where the ghosts have been coming from.
  - queued to send for 10 minutes with nothing going out, disconnect.
  - nothing received for 30 minutes, disconnect. A healthy peer relays a
    block inventory about every two minutes, so this is fifteen intervals
    of silence. Bitcoin used ninety minutes against ten-minute blocks.

Disconnecting is the whole cure: reconnection already works, and a peer we
cannot hear is worth what no peer is worth.

A node with nothing to relay would now look dead to a peer applying the
same rule, so send "ping" after ten minutes of having said nothing. It
carries no payload and expects no answer -- arriving is the entire content,
because the receive path has already stamped nLastRecv by the time it is
parsed. Requiring an answer would have read every node predating this
change as dead, since those ignore unknown commands rather than replying.

Verified on the live network: synced 0 to 3790 with five peers and no
spurious disconnects, then again with the interval cut to 20s, which put 54
pings on the wire with no peer dropping us and no exceptions.

3 files changed, 97 insertions, 1 deletions

diff --git a/src/main.cpp b/src/main.cpp
index d4fc6d0..f4e6ce2 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -2190,6 +2190,18 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
     }
 
 
+    else if (strCommand == "ping")
+    {
+        // Nothing to do and nothing to answer. Arriving at all is the entire
+        // content of the message: the receive path has already stamped
+        // nLastRecv, which is what the sender wanted us to notice.
+        //
+        // Deliberately no reply. A node old enough not to know "ping" ignores
+        // it as an unknown command and would never answer one, so a scheme
+        // that required an answer would read every such peer as dead.
+    }
+
+
     else
     {
         // Ignore unknown commands for extensibility
@@ -2222,6 +2234,17 @@ bool SendMessages(CNode* pto)
 
 
         //
+        // Message: keep-alive ping
+        //
+        // Only when we have nothing else queued -- any real message serves the
+        // same purpose. A peer with nothing to relay is otherwise silent, and
+        // silence is exactly what the inactivity check disconnects on.
+        //
+        if (pto->nLastSend && GetTime() - pto->nLastSend > BTF_PING_INTERVAL_SECS && pto->vSend.empty())
+            pto->PushMessage("ping");
+
+
+        //
         // Message: inventory
         //
         vector<CInv> vInventoryToSend;
diff --git a/src/net.cpp b/src/net.cpp
index 9ee056f..81932d7 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1092,7 +1092,11 @@ void ThreadSocketHandler2(void* parg)
                     vRecv.resize(nPos + nBufSize);
                     int nBytes = recv(hSocket, &vRecv[nPos], nBufSize, 0);
                     vRecv.resize(nPos + max(nBytes, 0));
-                    if (nBytes == 0)
+                    if (nBytes > 0)
+                    {
+                        pnode->nLastRecv = GetTime();
+                    }
+                    else if (nBytes == 0)
                     {
                         // socket closed gracefully
                         if (!pnode->fDisconnect)
@@ -1127,6 +1131,7 @@ void ThreadSocketHandler2(void* parg)
                         if (nBytes > 0)
                         {
                             vSend.erase(vSend.begin(), vSend.begin() + nBytes);
+                            pnode->nLastSend = GetTime();
                         }
                         else if (nBytes == 0)
                         {
@@ -1140,6 +1145,42 @@ void ThreadSocketHandler2(void* parg)
                                 pnode->vSend.clear();
                         }
                     }
+                    if (vSend.empty())
+                        pnode->nLastSendEmpty = GetTime();
+                }
+            }
+
+            //
+            // Inactivity
+            //
+            // Dropping the connection is the whole cure: everything above this
+            // layer already knows how to reconnect, and a peer we cannot hear
+            // is worth exactly as much as no peer at all. Give a new
+            // connection a grace period first, or we would cut off peers that
+            // are still completing the rendezvous handshake.
+            //
+            if (GetTime() - pnode->nTimeConnected > BTF_HANDSHAKE_GRACE_SECS)
+            {
+                if (pnode->nLastRecv == 0 || pnode->nLastSend == 0)
+                {
+                    LogPrint("net", "socket no message in first %d seconds, recv=%d send=%d\n",
+                             BTF_HANDSHAKE_GRACE_SECS, pnode->nLastRecv != 0, pnode->nLastSend != 0);
+                    pnode->fDisconnect = true;
+                }
+                else if (GetTime() - pnode->nLastSend > BTF_SEND_STALL_SECS &&
+                         GetTime() - pnode->nLastSendEmpty > BTF_SEND_STALL_SECS)
+                {
+                    // We have had something queued to send for this long and
+                    // none of it has gone out: the socket accepts no more.
+                    LogPrint("net", "socket not sending\n");
+                    pnode->fDisconnect = true;
+                }
+                else if (GetTime() - pnode->nLastRecv > BTF_RECV_TIMEOUT_SECS)
+                {
+                    // The deaf case. Nothing has arrived for many block
+                    // intervals while the socket still looks perfectly fine.
+                    LogPrint("net", "socket inactivity timeout\n");
+                    pnode->fDisconnect = true;
                 }
             }
         }
diff --git a/src/net.h b/src/net.h
index 11c023e..449cff5 100644
--- a/src/net.h
+++ b/src/net.h
@@ -51,6 +51,24 @@ static const int64        PEX_MIN_INTERVAL         = 60;
 // traffic is nothing: one per relay per five minutes.
 static const int          BTF_RENDEZVOUS_WAIT_SECS = 300;
 
+// Liveness of an established peer connection. BTF_RENDEZVOUS_WAIT_SECS bounds
+// the wait for a dial to arrive; these bound the connection that comes out of
+// it, which has the same failure mode once it goes quiet.
+//
+// A healthy peer relays a block inventory roughly every nTargetSpacing (two
+// minutes), so silence measured in block intervals is the honest signal.
+// Bitcoin used ninety minutes against ten-minute blocks -- nine intervals --
+// and the same nine intervals here is eighteen minutes; thirty leaves margin
+// for a slow stretch without letting a dead path last an hour.
+static const int          BTF_RECV_TIMEOUT_SECS    = 30 * 60;
+static const int          BTF_SEND_STALL_SECS      = 10 * 60;
+static const int          BTF_HANDSHAKE_GRACE_SECS = 60;
+
+// Send a ping after this long with nothing to say, so a peer running the
+// inactivity check above does not mistake a quiet node for a dead one. Must
+// stay well under BTF_RECV_TIMEOUT_SECS.
+static const int          BTF_PING_INTERVAL_SECS   = 10 * 60;
+
 // Descriptors to hand a peer: ours first, then peers that answered us.
 void BtfPexCollect(std::vector<std::string>& vDescOut);
 // Verify descriptors a peer sent and remember the good ones. Returns how many.
@@ -500,6 +518,16 @@ public:
     // Last "btfpeers" we accepted from this node, to rate-limit the exchange.
     int64 nLastPexRecv;
 
+    // Liveness. A peer whose network path dies silently -- NAT drops an idle
+    // mapping, a relay restarts, a route changes -- never sends FIN, so the
+    // socket stays readable-never and the node simply stops hearing from it
+    // with no error anywhere. These stamps are the only way to tell that
+    // apart from a peer that merely has nothing to say.
+    int64 nTimeConnected;
+    int64 nLastSend;
+    int64 nLastRecv;
+    int64 nLastSendEmpty;
+
 
     CNode(SOCKET hSocketIn, CAddress addrIn, bool fInboundIn=false)
     {
@@ -517,6 +545,10 @@ public:
         nRefCount = 0;
         nReleaseTime = 0;
         nLastPexRecv = 0;
+        nTimeConnected = GetTime();
+        nLastSend = 0;
+        nLastRecv = 0;
+        nLastSendEmpty = GetTime();
         vfSubscribe.assign(256, false);
 
         // Push a version message