Minor RC load/store/prune fixups

llarp/nodedb.cpp

@@ -109,7 +109,9 @@ namespace llarp
   {
     bootstraps.clear();  // this function really shouldn't be called more than once, but...
     for (const auto& rc : rcs)
+    {
       bootstraps.emplace(rc.router_id(), rc);
+    }
   }
 
   void
@@ -315,57 +317,56 @@ namespace llarp
     if (m_Root.empty())
       return;
 
-    router.loop()->call([this]() {
-      std::set<fs::path> purge;
+    std::set<fs::path> purge;
 
-      for (const char& ch : skiplist_subdirs)
-      {
-        if (!ch)
-          continue;
-        std::string p;
-        p += ch;
-        fs::path sub = m_Root / p;
-
-        llarp::util::IterDir(sub, [&](const fs::path& f) -> bool {
-          // skip files that are not suffixed with .signed
-          if (not(fs::is_regular_file(f) and f.extension() == RC_FILE_EXT))
-            return true;
-
-          RemoteRC rc{};
-
-          if (not rc.read(f))
-          {
-            // try loading it, purge it if it is junk
-            purge.emplace(f);
-            return true;
-          }
-
-          if (rc.is_expired(time_now_ms()))
-          {
-            // rc expired dont load it and purge it later
-            purge.emplace(f);
-            return true;
-          }
-
-          // validate signature and purge known_rcs with invalid signatures
-          // load ones with valid signatures
-          if (rc.verify())
-            known_rcs.emplace(rc.router_id(), rc);
-          else
-            purge.emplace(f);
+    const auto now = time_now_ms();
+
+    for (const char& ch : skiplist_subdirs)
+    {
+      if (!ch)
+        continue;
+      std::string p;
+      p += ch;
+      fs::path sub = m_Root / p;
 
+      llarp::util::IterDir(sub, [&](const fs::path& f) -> bool {
+        // skip files that are not suffixed with .signed
+        if (not(fs::is_regular_file(f) and f.extension() == RC_FILE_EXT))
           return true;
-        });
-      }
 
-      if (not purge.empty())
-      {
-        log::warning(logcat, "removing {} invalid RCs from disk", purge.size());
+        RemoteRC rc{};
 
-        for (const auto& fpath : purge)
-          fs::remove(fpath);
-      }
-    });
+        if (not rc.read(f))
+        {
+          // try loading it, purge it if it is junk
+          purge.emplace(f);
+          return true;
+        }
+
+        if (rc.is_expired(now))
+        {
+          // rc expired dont load it and purge it later
+          purge.emplace(f);
+          return true;
+        }
+
+        known_rcs.emplace(rc.router_id(), rc);
+        // TODO: the list of relays should be maintained and stored separately from
+        // the RCs, as we keep older RCs around in case we go offline and need to
+        // bootstrap, but they shouldn't be in the "good relays" list.
+        client_known_routers.insert(rc.router_id());
+
+        return true;
+      });
+    }
+
+    if (not purge.empty())
+    {
+      log::warning(logcat, "removing {} invalid RCs from disk", purge.size());
+
+      for (const auto& fpath : purge)
+        fs::remove(fpath);
+    }
   }
 
   void
@@ -407,11 +408,21 @@ namespace llarp
   }
 
   void
-  NodeDB::remove_stale_rcs(std::unordered_set<RouterID> keep, llarp_time_t cutoff)
+  NodeDB::remove_stale_rcs()
   {
-    (void)keep;
-    (void)cutoff;
-    // TODO: handling of "stale" is pending change, removing here for now.
+    auto cutoff_time =
+        std::chrono::time_point_cast<std::chrono::seconds>(std::chrono::system_clock::now());
+    cutoff_time -= router.is_service_node() ? RouterContact::OUTDATED_AGE : RouterContact::LIFETIME;
+    for (auto itr = known_rcs.begin(); itr != known_rcs.end();)
+    {
+      if (cutoff_time > itr->second.timestamp())
+      {
+        log::info(logcat, "Pruning RC for {}, as it is too old to keep.", itr->first);
+        known_rcs.erase(itr);
+        continue;
+      }
+      itr++;
+    }
   }
 
   bool

llarp/nodedb.hpp

@@ -271,9 +271,12 @@ namespace llarp
       });
     }
 
-    /// remove rcs that are not in keep and have been inserted before cutoff
+    /// remove rcs that are older than we want to keep.  For relays, this is when
+    /// they  become "outdated" (i.e. 12hrs).  Clients will hang on to them until
+    /// they are fully "expired" (i.e. 30 days), as the client may go offline for
+    /// some time and can still try to use those RCs to re-learn the network.
     void
-    remove_stale_rcs(std::unordered_set<RouterID> keep, llarp_time_t cutoff);
+    remove_stale_rcs();
 
     /// put (or replace) the RC if we consider it valid (want_rc).  returns true if put.
     bool

llarp/router_contact_remote.cpp

@@ -52,8 +52,8 @@ namespace llarp
       if (sig.size() != 64)
         throw std::runtime_error{"Invalid signature: not 64 bytes"};
 
-      if (is_expired(time_now_ms()) and reject_expired)
-        throw std::runtime_error{"Unable to verify expired RemoteRC!"};
+      if (reject_expired and is_expired(time_now_ms()))
+        throw std::runtime_error{"Rejecting expired RemoteRC!"};
 
       // TODO: revisit if this is needed; detail from previous implementation
       const auto* net = net::Platform::Default_ptr();
@@ -79,19 +79,19 @@ namespace llarp
     try
     {
       util::file_to_buffer(fname, buf.data(), MAX_RC_SIZE);
+
+      oxenc::bt_dict_consumer btdc{buf};
+      bt_load(btdc);
+      bt_verify(btdc);
+
+      _payload = buf;
     }
     catch (const std::exception& e)
     {
-      log::error(logcat, "Failed to read RC from {}: {}", fname, e.what());
+      log::error(logcat, "Failed to read or validate RC from {}: {}", fname, e.what());
       return false;
     }
 
-    oxenc::bt_dict_consumer btdc{buf};
-    bt_load(btdc);
-    bt_verify(btdc);
-
-    _payload = buf;
-
     return true;
   }