banman: schedule sweep at ban expiry instead of polling

src/banman.cpp

@@ -9,10 +9,14 @@
 #include <logging.h>
 #include <netaddress.h>
 #include <node/interface_ui.h>
+#include <scheduler.h>
 #include <sync.h>
 #include <util/time.h>
 #include <util/translation.h>
 
+#include <algorithm>
+#include <limits>
+
 
 BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time)
     : m_client_interface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time)
@@ -26,6 +30,53 @@ BanMan::~BanMan()
     DumpBanlist();
 }
 
+void BanMan::SetScheduler(CScheduler& scheduler)
+{
+    LOCK(m_banned_mutex);
+    m_scheduler = &scheduler;
+}
+
+void BanMan::EnsureSweepScheduled()
+{
+    LOCK(m_banned_mutex);
+    if (!m_scheduler) return;
+    if (m_sweep_started) return;
+    m_sweep_started = true;
+    SweepBanned();
+    m_next_sweep_time = std::numeric_limits<int64_t>::max();
+    ScheduleNextSweep();
+}
+
+void BanMan::SweepBannedAndSchedule(uint64_t expected_seq)
+{
+    LOCK(m_banned_mutex);
+    if (expected_seq != m_sweep_seq) return;
+    SweepBanned();
+    m_next_sweep_time = std::numeric_limits<int64_t>::max();
+    ScheduleNextSweep();
+}
+
+void BanMan::ScheduleNextSweep()
+{
+    AssertLockHeld(m_banned_mutex);
+
+    if (!m_scheduler) return;
+    if (m_banned.empty()) return;
+
+    int64_t earliest = std::numeric_limits<int64_t>::max();
+    for (const auto& [subnet, entry] : m_banned) {
+        if (entry.nBanUntil < earliest) {
+            earliest = entry.nBanUntil;
+        }
+    }
+
+    m_next_sweep_time = earliest;
+    uint64_t seq = ++m_sweep_seq;
+    int64_t now = GetTime();
+    auto delay = std::chrono::seconds(std::max<int64_t>(0, earliest - now));
+    m_scheduler->scheduleFromNow([this, seq] { SweepBannedAndSchedule(seq); }, delay);
+}
+
 void BanMan::LoadBanlist()
 {
     LOCK(m_banned_mutex);
@@ -144,6 +195,13 @@ void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_uni
         if (m_banned[sub_net].nBanUntil < ban_entry.nBanUntil) {
             m_banned[sub_net] = ban_entry;
             m_is_dirty = true;
+            if (m_sweep_started && ban_entry.nBanUntil < m_next_sweep_time) {
+                m_next_sweep_time = ban_entry.nBanUntil;
+                uint64_t seq = ++m_sweep_seq;
+                int64_t now = GetTime();
+                auto delay = std::chrono::seconds(std::max<int64_t>(0, m_next_sweep_time - now));
+                m_scheduler->scheduleFromNow([this, seq] { SweepBannedAndSchedule(seq); }, delay);
+            }
         } else
             return;
     }
@@ -189,7 +247,7 @@ void BanMan::SweepBanned()
     while (it != m_banned.end()) {
         CSubNet sub_net = (*it).first;
         CBanEntry ban_entry = (*it).second;
-        if (!sub_net.IsValid() || now > ban_entry.nBanUntil) {
+        if (!sub_net.IsValid() || now >= ban_entry.nBanUntil) {
             m_banned.erase(it++);
             m_is_dirty = true;
             notify_ui = true;

src/banman.h

@@ -13,6 +13,7 @@
 
 #include <chrono>
 #include <cstdint>
+#include <limits>
 #include <memory>
 
 // NOTE: When adjusting this, update rpcnet:setban's help ("24h")
@@ -23,6 +24,7 @@ static constexpr std::chrono::minutes DUMP_BANS_INTERVAL{15};
 
 class CClientUIInterface;
 class CNetAddr;
+class CScheduler;
 class CSubNet;
 
 // Banman manages two related but distinct concepts:
@@ -62,6 +64,8 @@ class BanMan
     BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time);
     void Ban(const CNetAddr& net_addr, int64_t ban_time_offset = 0, bool since_unix_epoch = false) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
     void Ban(const CSubNet& sub_net, int64_t ban_time_offset = 0, bool since_unix_epoch = false) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
+    void SetScheduler(CScheduler& scheduler) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
+    void EnsureSweepScheduled() EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
     void Discourage(const CNetAddr& net_addr) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
     void ClearBanned() EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
 
@@ -83,8 +87,14 @@ class BanMan
     void LoadBanlist() EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
     //!clean unused entries (if bantime has expired)
     void SweepBanned() EXCLUSIVE_LOCKS_REQUIRED(m_banned_mutex);
+    void SweepBannedAndSchedule(uint64_t expected_seq) EXCLUSIVE_LOCKS_REQUIRED(!m_banned_mutex);
+    void ScheduleNextSweep() EXCLUSIVE_LOCKS_REQUIRED(m_banned_mutex);
 
     Mutex m_banned_mutex;
+    CScheduler* m_scheduler GUARDED_BY(m_banned_mutex){nullptr};
+    bool m_sweep_started GUARDED_BY(m_banned_mutex){false};
+    int64_t m_next_sweep_time GUARDED_BY(m_banned_mutex){std::numeric_limits<int64_t>::max()};
+    uint64_t m_sweep_seq GUARDED_BY(m_banned_mutex){0};
     banmap_t m_banned GUARDED_BY(m_banned_mutex);
     bool m_is_dirty GUARDED_BY(m_banned_mutex){false};
     CClientUIInterface* m_client_interface = nullptr;

src/init.cpp

@@ -2437,6 +2437,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         banman->DumpBanlist();
     }, DUMP_BANS_INTERVAL);
 
+    banman->SetScheduler(scheduler);
+
     if (node.peerman) node.peerman->StartScheduledTasks(scheduler);
 
 #if HAVE_SYSTEM

src/node/interfaces.cpp

@@ -415,6 +415,9 @@ class NodeImpl : public Node
     }
     std::unique_ptr<Handler> handleBannedListChanged(BannedListChangedFn fn) override
     {
+        if (m_context->banman) {
+            m_context->banman->EnsureSweepScheduled();
+        }
         return MakeSignalHandler(::uiInterface.BannedListChanged_connect(fn));
     }
     std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) override