Merge bitcoin/bitcoin#22570: Ignore banlist.dat

fa1eddb1a3 Fix whitespace in touched files (MarcoFalke)
fa4e6afdae Remove unused CSubNet serialize code (MarcoFalke)
fa384fdd0b Ignore banlist.dat (MarcoFalke)

Pull request description:

  The code to read `banlist.dat` should be removed eventually. The major release (22.x) can be used to translate a `banlist.dat` into a `banlist.json`. Thus, it is now possible to remove the reading code.

ACKs for top commit:
  Zero-1729:
    re-ACK fa1eddb1a3
  laanwj:
    concept and code review ACK fa1eddb1a3
  vasild:
    ACK fa1eddb1a3
  jonatack:
    Light code review utACK fa1eddb1a3

Tree-SHA512: e136193b7c0ba1d6d2e79c7fb4106ba4af75fa229ed7214675ee64e98e59bb4808779e7a8a09eecce62f7a5d4bc6e16b8a5ad4596129357c8fc5e3b88f214249

doc/files.md

@@ -56,7 +56,6 @@ Subdirectory       | File(s)               | Description
 `indexes/coinstats/db/` | LevelDB database | Coinstats index; *optional*, used if `-coinstatsindex=1`
 `wallets/`         |                       | [Contains wallets](#multi-wallet-environment); can be specified by `-walletdir` option; if `wallets/` subdirectory does not exist, wallets reside in the [data directory](#data-directory-location)
 `./`               | `anchors.dat`         | Anchor IP address database, created on shutdown and deleted at startup. Anchors are last known outgoing block-relay-only peers that are tried to re-connect to on startup
-`./`               | `banlist.dat`         | Stores the addresses/subnets of banned nodes (deprecated). `bitcoind` or `bitcoin-qt` no longer save the banlist to this file, but read it on startup if `banlist.json` is not present.
 `./`               | `banlist.json`        | Stores the addresses/subnets of banned nodes.
 `./`               | `bitcoin.conf`        | User-defined [configuration settings](bitcoin-conf.md) for `bitcoind` or `bitcoin-qt`. File is not written to by the software and must be created manually. Path can be specified by `-conf` option
 `./`               | `bitcoind.pid`        | Stores the process ID (PID) of `bitcoind` or `bitcoin-qt` while running; created at start and deleted on shutdown; can be specified by `-pid` option
@@ -114,6 +113,7 @@ These subdirectories and files are no longer used by Bitcoin Core:
 
 Path           | Description | Repository notes
 ---------------|-------------|-----------------
+`banlist.dat`  | Stores the addresses/subnets of banned nodes; superseded by `banlist.json` in 22.0 and completely ignored in 23.0 | [PR #20966](https://github.com/bitcoin/bitcoin/pull/20966), [PR #22570](https://github.com/bitcoin/bitcoin/pull/22570)
 `blktree/`     | Blockchain index; replaced by `blocks/index/` in [0.8.0](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.8.0.md#improvements) | [PR #2231](https://github.com/bitcoin/bitcoin/pull/2231), [`8fdc94cc`](https://github.com/bitcoin/bitcoin/commit/8fdc94cc8f0341e96b1edb3a5b56811c0b20bd15)
 `coins/`       | Unspent transaction output database; replaced by `chainstate/` in 0.8.0 | [PR #2231](https://github.com/bitcoin/bitcoin/pull/2231), [`8fdc94cc`](https://github.com/bitcoin/bitcoin/commit/8fdc94cc8f0341e96b1edb3a5b56811c0b20bd15)
 `blkindex.dat` | Blockchain index BDB database; replaced by {`chainstate/`, `blocks/index/`, `blocks/revNNNNN.dat`<sup>[\[2\]](#note2)</sup>} in 0.8.0 | [PR #1677](https://github.com/bitcoin/bitcoin/pull/1677)

src/addrdb.cpp

@@ -197,16 +197,15 @@ bool CBanDB::Write(const banmap_t& banSet)
     return false;
 }
 
-bool CBanDB::Read(banmap_t& banSet, bool& dirty)
+bool CBanDB::Read(banmap_t& banSet)
 {
-    // If the JSON banlist does not exist, then try to read the non-upgraded banlist.dat.
-    if (!fs::exists(m_banlist_json)) {
-        // If this succeeds then we need to flush to disk in order to create the JSON banlist.
-        dirty = true;
-        return DeserializeFileDB(m_banlist_dat, banSet, CLIENT_VERSION);
+    if (fs::exists(m_banlist_dat)) {
+        LogPrintf("banlist.dat ignored because it can only be read by " PACKAGE_NAME " version 22.x. Remove %s to silence this warning.\n", m_banlist_dat);
+    }
+    // If the JSON banlist does not exist, then recreate it
+    if (!fs::exists(m_banlist_json)) {
+        return false;
     }
-
-    dirty = false;
 
     std::map<std::string, util::SettingsValue> settings;
     std::vector<std::string> errors;

src/addrdb.h

@@ -76,7 +76,7 @@ public:
     static bool Read(CAddrMan& addr, CDataStream& ssPeers);
 };
 
-/** Access to the banlist databases (banlist.json and banlist.dat) */
+/** Access to the banlist database (banlist.json) */
 class CBanDB
 {
 private:
@@ -95,11 +95,9 @@ public:
      * Read the banlist from disk.
      * @param[out] banSet The loaded list. Set if `true` is returned, otherwise it is left
      * in an undefined state.
-     * @param[out] dirty Indicates whether the loaded list needs flushing to disk. Set if
-     * `true` is returned, otherwise it is left in an undefined state.
      * @return true on success
      */
-    bool Read(banmap_t& banSet, bool& dirty);
+    bool Read(banmap_t& banSet);
 };
 
 /**

src/banman.cpp

@@ -18,7 +18,7 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t
     if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…").translated);
 
     int64_t n_start = GetTimeMillis();
-    if (m_ban_db.Read(m_banned, m_is_dirty)) {
+    if (m_ban_db.Read(m_banned)) {
         SweepBanned(); // sweep out unused entries
 
         LogPrint(BCLog::NET, "Loaded %d banned node addresses/subnets  %dms\n", m_banned.size(),

src/banman.h

@@ -88,7 +88,7 @@ private:
 
     RecursiveMutex m_cs_banned;
     banmap_t m_banned GUARDED_BY(m_cs_banned);
-    bool m_is_dirty GUARDED_BY(m_cs_banned);
+    bool m_is_dirty GUARDED_BY(m_cs_banned){false};
     CClientUIInterface* m_client_interface = nullptr;
     CBanDB m_ban_db;
     const int64_t m_default_ban_time;

src/netaddress.h

@@ -42,8 +42,7 @@ static constexpr int ADDRV2_FORMAT = 0x20000000;
  * over all enum values and also `GetExtNetwork()` "extends" this enum by
  * introducing standalone constants starting from `NET_MAX`.
  */
-enum Network
-{
+enum Network {
     /// Addresses from these networks are not publicly routable on the global Internet.
     NET_UNROUTABLE = 0,
 
@@ -73,16 +72,14 @@ enum Network
 /// Prefix of an IPv6 address when it contains an embedded IPv4 address.
 /// Used when (un)serializing addresses in ADDRv1 format (pre-BIP155).
 static const std::array<uint8_t, 12> IPV4_IN_IPV6_PREFIX{
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF
-};
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF};
 
 /// Prefix of an IPv6 address when it contains an embedded TORv2 address.
 /// Used when (un)serializing addresses in ADDRv1 format (pre-BIP155).
 /// Such dummy IPv6 addresses are guaranteed to not be publicly routable as they
 /// fall under RFC4193's fc00::/7 subnet allocated to unique-local addresses.
 static const std::array<uint8_t, 6> TORV2_IN_IPV6_PREFIX{
-    0xFD, 0x87, 0xD8, 0x7E, 0xEB, 0x43
-};
+    0xFD, 0x87, 0xD8, 0x7E, 0xEB, 0x43};
 
 /// Prefix of an IPv6 address when it contains an embedded "internal" address.
 /// Used when (un)serializing addresses in ADDRv1 format (pre-BIP155).
@@ -536,24 +533,6 @@ class CSubNet
     friend bool operator==(const CSubNet& a, const CSubNet& b);
     friend bool operator!=(const CSubNet& a, const CSubNet& b) { return !(a == b); }
     friend bool operator<(const CSubNet& a, const CSubNet& b);
-
-        SERIALIZE_METHODS(CSubNet, obj)
-        {
-            READWRITE(obj.network);
-            if (obj.network.IsIPv4()) {
-                // Before commit 102867c587f5f7954232fb8ed8e85cda78bb4d32, CSubNet used the last 4 bytes of netmask
-                // to store the relevant bytes for an IPv4 mask. For compatibility reasons, keep doing so in
-                // serialized form.
-                unsigned char dummy[12] = {0};
-                READWRITE(dummy);
-                READWRITE(MakeSpan(obj.netmask).first(4));
-            } else {
-                READWRITE(obj.netmask);
-            }
-            READWRITE(obj.valid);
-            // Mark invalid if the result doesn't pass sanity checking.
-            SER_READ(obj, if (obj.valid) obj.valid = obj.SanityCheck());
-        }
 };
 
 /** A combination of a network address (CNetAddr) and a (TCP) port */

src/test/fuzz/banman.cpp

@@ -52,8 +52,7 @@ FUZZ_TARGET_INIT(banman, initialize_banman)
     const bool start_with_corrupted_banlist{fuzzed_data_provider.ConsumeBool()};
     bool force_read_and_write_to_err{false};
     if (start_with_corrupted_banlist) {
-        const std::string sfx{fuzzed_data_provider.ConsumeBool() ? ".dat" : ".json"};
-        assert(WriteBinaryFile(banlist_file.string() + sfx,
+        assert(WriteBinaryFile(banlist_file.string() + ".json",
                                fuzzed_data_provider.ConsumeRandomLengthString()));
     } else {
         force_read_and_write_to_err = fuzzed_data_provider.ConsumeBool();
@@ -114,6 +113,5 @@ FUZZ_TARGET_INIT(banman, initialize_banman)
             (void)(banmap == banmap_read);
         }
     }
-    fs::remove(banlist_file.string() + ".dat");
     fs::remove(banlist_file.string() + ".json");
 }

src/test/fuzz/deserialize.cpp

@@ -142,17 +142,6 @@ FUZZ_TARGET_DESERIALIZE(script_deserialize, {
     CScript script;
     DeserializeFromFuzzingInput(buffer, script);
 })
-FUZZ_TARGET_DESERIALIZE(sub_net_deserialize, {
-        CSubNet sub_net_1;
-        DeserializeFromFuzzingInput(buffer, sub_net_1, INIT_PROTO_VERSION);
-        AssertEqualAfterSerializeDeserialize(sub_net_1, INIT_PROTO_VERSION);
-        CSubNet sub_net_2;
-        DeserializeFromFuzzingInput(buffer, sub_net_2, INIT_PROTO_VERSION | ADDRV2_FORMAT);
-        AssertEqualAfterSerializeDeserialize(sub_net_2, INIT_PROTO_VERSION | ADDRV2_FORMAT);
-        CSubNet sub_net_3;
-        DeserializeFromFuzzingInput(buffer, sub_net_3);
-        AssertEqualAfterSerializeDeserialize(sub_net_3, INIT_PROTO_VERSION | ADDRV2_FORMAT);
-})
 FUZZ_TARGET_DESERIALIZE(tx_in_deserialize, {
     CTxIn tx_in;
     DeserializeFromFuzzingInput(buffer, tx_in);