banman: pass the banfile path in

There's no need to hard-code the path here. Passing it in means that there are
no ordering concerns wrt establishing the datadir.

src/addrdb.cpp

@@ -105,19 +105,18 @@ bool DeserializeFileDB(const fs::path& path, Data& data)
 
 }
 
-CBanDB::CBanDB()
+CBanDB::CBanDB(fs::path ban_list_path) : m_ban_list_path(std::move(ban_list_path))
 {
-    pathBanlist = GetDataDir() / "banlist.dat";
 }
 
 bool CBanDB::Write(const banmap_t& banSet)
 {
-    return SerializeFileDB("banlist", pathBanlist, banSet);
+    return SerializeFileDB("banlist", m_ban_list_path, banSet);
 }
 
 bool CBanDB::Read(banmap_t& banSet)
 {
-    return DeserializeFileDB(pathBanlist, banSet);
+    return DeserializeFileDB(m_ban_list_path, banSet);
 }
 
 CAddrDB::CAddrDB()

src/addrdb.h

@@ -92,9 +92,9 @@ class CAddrDB
 class CBanDB
 {
 private:
-    fs::path pathBanlist;
+    const fs::path m_ban_list_path;
 public:
-    CBanDB();
+    explicit CBanDB(fs::path ban_list_path);
     bool Write(const banmap_t& banSet);
     bool Read(banmap_t& banSet);
 };

src/init.cpp

@@ -1296,7 +1296,7 @@ bool AppInitMain(InitInterfaces& interfaces)
     // need to reindex later.
 
     assert(!g_banman);
-    g_banman = MakeUnique<BanMan>(&uiInterface);
+    g_banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", &uiInterface);
     assert(!g_connman);
     g_connman = std::unique_ptr<CConnman>(new CConnman(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max())));
 

src/net.cpp

@@ -466,10 +466,9 @@ void BanMan::DumpBanlist()
 
     int64_t nStart = GetTimeMillis();
 
-    CBanDB bandb;
     banmap_t banmap;
     GetBanned(banmap);
-    if (bandb.Write(banmap)) {
+    if (m_ban_db.Write(banmap)) {
         SetBannedSetDirty(false);
     }
 
@@ -2431,16 +2430,14 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
     return true;
 }
 
-BanMan::BanMan(CClientUIInterface* client_interface) : clientInterface(client_interface)
+BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface) : clientInterface(client_interface), m_ban_db(std::move(ban_file))
 {
     if (clientInterface) clientInterface->InitMessage(_("Loading banlist..."));
-    // Load addresses from banlist.dat
 
     int64_t nStart = GetTimeMillis();
     setBannedIsDirty = false;
-    CBanDB bandb;
     banmap_t banmap;
-    if (bandb.Read(banmap)) {
+    if (m_ban_db.Read(banmap)) {
         SetBanned(banmap); // thread save setter
         SetBannedSetDirty(false); // no need to write down, just read data
         SweepBanned(); // sweep out unused entries

src/net.h

@@ -133,7 +133,7 @@ class BanMan
     // between nodes running old code and nodes running
     // new code.
     ~BanMan();
-    BanMan(CClientUIInterface* client_interface);
+    BanMan(fs::path ban_file, CClientUIInterface* client_interface);
     void Ban(const CNetAddr& netAddr, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
     void Ban(const CSubNet& subNet, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
     void ClearBanned(); // needed for unit testing
@@ -156,6 +156,7 @@ class BanMan
     CCriticalSection cs_setBanned;
     bool setBannedIsDirty;
     CClientUIInterface* clientInterface = nullptr;
+    CBanDB m_ban_db;
 };
 
 class NetEventsInterface;

src/test/denialofservice_tests.cpp

@@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 
 BOOST_AUTO_TEST_CASE(DoS_banning)
 {
-    auto banman = MakeUnique<BanMan>(nullptr);
+    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr);
     auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
     auto peerLogic = MakeUnique<PeerLogicValidation>(connman.get(), banman.get(), scheduler, false);
 
@@ -271,7 +271,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
 
 BOOST_AUTO_TEST_CASE(DoS_banscore)
 {
-    auto banman = MakeUnique<BanMan>(nullptr);
+    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr);
     auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
     auto peerLogic = MakeUnique<PeerLogicValidation>(connman.get(), banman.get(), scheduler, false);
 
@@ -318,7 +318,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
 
 BOOST_AUTO_TEST_CASE(DoS_bantime)
 {
-    auto banman = MakeUnique<BanMan>(nullptr);
+    auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr);
     auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
     auto peerLogic = MakeUnique<PeerLogicValidation>(connman.get(), banman.get(), scheduler, false);
 

src/test/test_bitcoin.cpp

@@ -94,7 +94,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
         for (int i=0; i < nScriptCheckThreads-1; i++)
             threadGroup.create_thread(&ThreadScriptCheck);
 
-        g_banman = MakeUnique<BanMan>(nullptr);
+        g_banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr);
         g_connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests.
 }