From 3919059deb60d6ead9defc9d213a3f0c2ab72e90 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Fri, 20 May 2022 14:54:33 +0200
Subject: [PATCH 1/3] refactor: Move code from ctor into private
 `BanMan::LoadBanlist()`

Co-authored-by: Anthony Towns <aj@erisian.com.au>
---
 src/banman.cpp | 20 +++++++++++++-------
 src/banman.h   |  1 +
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/banman.cpp b/src/banman.cpp
index b28e3f7f7c..901d91809e 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -16,6 +16,19 @@
 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)
 {
+    LoadBanlist();
+    DumpBanlist();
+}
+
+BanMan::~BanMan()
+{
+    DumpBanlist();
+}
+
+void BanMan::LoadBanlist()
+{
+    LOCK(m_cs_banned);
+
     if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…").translated);
 
     int64_t n_start = GetTimeMillis();
@@ -29,13 +42,6 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t
         m_banned = {};
         m_is_dirty = true;
     }
-
-    DumpBanlist();
-}
-
-BanMan::~BanMan()
-{
-    DumpBanlist();
 }
 
 void BanMan::DumpBanlist()
diff --git a/src/banman.h b/src/banman.h
index f268fffa5a..b037b8869b 100644
--- a/src/banman.h
+++ b/src/banman.h
@@ -80,6 +80,7 @@ public:
     void DumpBanlist();
 
 private:
+    void LoadBanlist() EXCLUSIVE_LOCKS_REQUIRED(!m_cs_banned);
     bool BannedSetIsDirty();
     //!set the "dirty" flag for the banlist
     void SetBannedSetDirty(bool dirty = true);

From 52c0b3e859089c0ac98e7261ded6391b4f8eeeaf Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Fri, 20 May 2022 15:17:00 +0200
Subject: [PATCH 2/3] refactor: Add thread safety annotation to
 `BanMan::SweepBanned()`

---
 src/banman.cpp | 3 ++-
 src/banman.h   | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/banman.cpp b/src/banman.cpp
index 901d91809e..02c70be584 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -179,10 +179,11 @@ void BanMan::GetBanned(banmap_t& banmap)
 
 void BanMan::SweepBanned()
 {
+    AssertLockHeld(m_cs_banned);
+
     int64_t now = GetTime();
     bool notify_ui = false;
     {
-        LOCK(m_cs_banned);
         banmap_t::iterator it = m_banned.begin();
         while (it != m_banned.end()) {
             CSubNet sub_net = (*it).first;
diff --git a/src/banman.h b/src/banman.h
index b037b8869b..77b043f081 100644
--- a/src/banman.h
+++ b/src/banman.h
@@ -85,7 +85,7 @@ private:
     //!set the "dirty" flag for the banlist
     void SetBannedSetDirty(bool dirty = true);
     //!clean unused entries (if bantime has expired)
-    void SweepBanned();
+    void SweepBanned() EXCLUSIVE_LOCKS_REQUIRED(m_cs_banned);
 
     RecursiveMutex m_cs_banned;
     banmap_t m_banned GUARDED_BY(m_cs_banned);

From ab7538832043a6c15e45a178fb6bb6298a00108f Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Fri, 20 May 2022 15:20:42 +0200
Subject: [PATCH 3/3] refactor: Remove redundant scope in
 `BanMan::SweepBanned()`

---
 src/banman.cpp | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/banman.cpp b/src/banman.cpp
index 02c70be584..2a6e0e010f 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -183,20 +183,20 @@ void BanMan::SweepBanned()
 
     int64_t now = GetTime();
     bool notify_ui = false;
-    {
-        banmap_t::iterator it = m_banned.begin();
-        while (it != m_banned.end()) {
-            CSubNet sub_net = (*it).first;
-            CBanEntry ban_entry = (*it).second;
-            if (!sub_net.IsValid() || now > ban_entry.nBanUntil) {
-                m_banned.erase(it++);
-                m_is_dirty = true;
-                notify_ui = true;
-                LogPrint(BCLog::NET, "Removed banned node address/subnet: %s\n", sub_net.ToString());
-            } else
-                ++it;
+    banmap_t::iterator it = m_banned.begin();
+    while (it != m_banned.end()) {
+        CSubNet sub_net = (*it).first;
+        CBanEntry ban_entry = (*it).second;
+        if (!sub_net.IsValid() || now > ban_entry.nBanUntil) {
+            m_banned.erase(it++);
+            m_is_dirty = true;
+            notify_ui = true;
+            LogPrint(BCLog::NET, "Removed banned node address/subnet: %s\n", sub_net.ToString());
+        } else {
+            ++it;
         }
     }
+
     // update UI
     if (notify_ui && m_client_interface) {
         m_client_interface->BannedListChanged();