From 82c95a2486c360b5835bf72bd2a140d457a256e4 Mon Sep 17 00:00:00 2001
From: Thomas Winget <tewinget@gmail.com>
Date: Wed, 28 Sep 2022 10:15:45 -0400
Subject: [PATCH] don't ping core if active with too few peers

If running as a service node, we ping core on a regular interval to
inform it we're running and in a good state.  If we're an active
(not decommissioned or deregistered) service node and have too few
peers and thus we're not actually connected to lokinet, we should skip
that ping so core doesn't think we're ok.
---
 llarp/router/abstractrouter.hpp |  9 +++++++++
 llarp/router/router.cpp         | 31 ++++++++++++++++++++++++-------
 llarp/router/router.hpp         | 10 ++++++++++
 llarp/rpc/lokid_rpc_client.cpp  | 27 +++++++++++++++++----------
 4 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/llarp/router/abstractrouter.hpp b/llarp/router/abstractrouter.hpp
index 4826947a6..022e6767a 100644
--- a/llarp/router/abstractrouter.hpp
+++ b/llarp/router/abstractrouter.hpp
@@ -199,6 +199,15 @@ namespace llarp
     virtual bool
     IsServiceNode() const = 0;
 
+    virtual bool
+    IsActiveServiceNode() const = 0;
+
+    /// If we are running as a service node and appear active, i.e. registered and not
+    /// decommissioned, we should *not* ping core if we know of too few peers, to indicate to core
+    /// we are not in a good state.
+    virtual bool
+    ShouldPingOxen() const = 0;
+
     virtual bool
     StartRpcServer() = 0;
 
diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp
index b1747dfdc..8d207c38e 100644
--- a/llarp/router/router.cpp
+++ b/llarp/router/router.cpp
@@ -464,6 +464,25 @@ namespace llarp
     return m_isServiceNode;
   }
 
+  bool
+  Router::TooFewPeers() const
+  {
+    constexpr int KnownPeerWarningThreshold = 5;
+    return nodedb()->NumLoaded() < KnownPeerWarningThreshold;
+  }
+
+  bool
+  Router::IsActiveServiceNode() const
+  {
+    return IsServiceNode() and not(LooksDeregistered() or LooksDecommissioned());
+  }
+
+  bool
+  Router::ShouldPingOxen() const
+  {
+    return IsActiveServiceNode() and not TooFewPeers();
+  }
+
   void
   Router::Close()
   {
@@ -1057,14 +1076,12 @@ namespace llarp
             dereg ? "deregistered" : "decommissioned");
         m_NextDecommissionWarn = now + DecommissionWarnInterval;
       }
-      else if (isSvcNode)
+      else if (isSvcNode and TooFewPeers())
       {
-        constexpr int KnownPeerWarningThreshold = 5;
-        if (nodedb()->NumLoaded() < KnownPeerWarningThreshold)
-          log::error(
-              logcat,
-              "We appear to be an active service node, but have fewer than {} known peers.",
-              KnownPeerWarningThreshold);
+        log::error(
+            logcat,
+            "We appear to be an active service node, but have only {} known peers.",
+            nodedb()->NumLoaded());
         m_NextDecommissionWarn = now + DecommissionWarnInterval;
       }
     }
diff --git a/llarp/router/router.hpp b/llarp/router/router.hpp
index 8f56024ea..8eb751410 100644
--- a/llarp/router/router.hpp
+++ b/llarp/router/router.hpp
@@ -378,6 +378,13 @@ namespace llarp
     bool
     IsServiceNode() const override;
 
+    /// return true if service node *and* not deregistered or decommissioned
+    bool
+    IsActiveServiceNode() const override;
+
+    bool
+    ShouldPingOxen() const override;
+
     void
     Close();
 
@@ -573,6 +580,9 @@ namespace llarp
     void
     MessageSent(const RouterID& remote, SendStatus status);
 
+    bool
+    TooFewPeers() const;
+
    protected:
     virtual void
     HandleRouterEvent(tooling::RouterEventPtr event) const override;
diff --git a/llarp/rpc/lokid_rpc_client.cpp b/llarp/rpc/lokid_rpc_client.cpp
index e525d7427..7048188fd 100644
--- a/llarp/rpc/lokid_rpc_client.cpp
+++ b/llarp/rpc/lokid_rpc_client.cpp
@@ -152,18 +152,25 @@ namespace llarp
       auto makePingRequest = [self = shared_from_this()]() {
         // send a ping
         PubKey pk{};
+        bool should_ping = false;
         if (auto r = self->m_Router.lock())
+        {
           pk = r->pubkey();
-        nlohmann::json payload = {
-            {"pubkey_ed25519", oxenc::to_hex(pk.begin(), pk.end())},
-            {"version", {VERSION[0], VERSION[1], VERSION[2]}}};
-        self->Request(
-            "admin.lokinet_ping",
-            [](bool success, std::vector<std::string> data) {
-              (void)data;
-              LogDebug("Received response for ping. Successful: ", success);
-            },
-            payload.dump());
+          should_ping = r->ShouldPingOxen();
+        }
+        if (should_ping)
+        {
+          nlohmann::json payload = {
+              {"pubkey_ed25519", oxenc::to_hex(pk.begin(), pk.end())},
+              {"version", {VERSION[0], VERSION[1], VERSION[2]}}};
+          self->Request(
+              "admin.lokinet_ping",
+              [](bool success, std::vector<std::string> data) {
+                (void)data;
+                LogDebug("Received response for ping. Successful: ", success);
+              },
+              payload.dump());
+        }
         // subscribe to block updates
         self->Request("sub.block", [](bool success, std::vector<std::string> data) {
           if (data.empty() or not success)