From ff4a38a32766942ce5c4be6d6510f800a9f8e0d9 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Sat, 19 Feb 2022 13:00:45 +0200
Subject: [PATCH 01/28] build: Fix configuring depends with cmake

---
 depends/hosts/mingw32.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/depends/hosts/mingw32.mk b/depends/hosts/mingw32.mk
index 2f370d2b873d..48020d71af05 100644
--- a/depends/hosts/mingw32.mk
+++ b/depends/hosts/mingw32.mk
@@ -13,4 +13,4 @@ mingw32_debug_CXXFLAGS=$(mingw32_debug_CFLAGS)
 
 mingw32_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC
 
-mingw_cmake_system=Windows
+mingw32_cmake_system=Windows

From 62e14285f95d4ecb9528813acca975640dd7c598 Mon Sep 17 00:00:00 2001
From: Martin Zumsande <mzumsande@gmail.com>
Date: Wed, 6 Apr 2022 17:05:32 +0200
Subject: [PATCH 02/28] doc: Add note that -reindex will rebuild optional
 indexes

---
 src/init.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/init.cpp b/src/init.cpp
index 86e6ec44513a..5ca5dd5401a0 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -423,7 +423,7 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex and -coinstatsindex. "
             "Warning: Reverting this setting requires re-downloading the entire blockchain. "
             "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >=%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk. This will also rebuild active optional indexes.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-settings=<file>", strprintf("Specify path to dynamic settings data file. Can be disabled with -nosettings. File is written at runtime and not meant to be edited by users (use %s instead for custom settings). Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME, BITCOIN_SETTINGS_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 #if HAVE_SYSTEM

From 9cc8e876e412056ed22d364538f0da3d5d71946d Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Sat, 11 Sep 2021 13:02:47 +0200
Subject: [PATCH 03/28] refactor: introduce single-separator split helper
 `SplitString`

This helper uses spanparsing::Split internally and enables to replace
all calls to boost::split where only a single separator is passed.

Co-authored-by: Martin Ankerl <Martin.Ankerl@gmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
---
 src/bitcoin-tx.cpp                             | 17 +++++------------
 src/chainparams.cpp                            |  7 ++-----
 src/rest.cpp                                   | 13 ++++---------
 src/rpc/server.cpp                             |  6 ++----
 src/rpc/util.cpp                               |  9 ++-------
 src/test/fuzz/script_assets_test_minimizer.cpp |  5 ++---
 src/test/transaction_tests.cpp                 |  5 +----
 src/torcontrol.cpp                             | 12 ++++++------
 src/util/spanparsing.cpp                       | 16 ----------------
 src/util/spanparsing.h                         | 17 ++++++++++++++++-
 src/util/string.h                              |  6 ++++++
 src/wallet/rpc/backup.cpp                      |  5 +----
 12 files changed, 47 insertions(+), 71 deletions(-)

diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
index 0b40626595fd..05f910e9cb3b 100644
--- a/src/bitcoin-tx.cpp
+++ b/src/bitcoin-tx.cpp
@@ -31,8 +31,6 @@
 #include <memory>
 #include <stdio.h>
 
-#include <boost/algorithm/string.hpp>
-
 static bool fCreateBlank;
 static std::map<std::string,UniValue> registers;
 static const int CONTINUE_EXECUTION=-1;
@@ -251,8 +249,7 @@ static T TrimAndParse(const std::string& int_str, const std::string& err)
 
 static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInput)
 {
-    std::vector<std::string> vStrInputParts;
-    boost::split(vStrInputParts, strInput, boost::is_any_of(":"));
+    std::vector<std::string> vStrInputParts = SplitString(strInput, ':');
 
     // separate TXID:VOUT in string
     if (vStrInputParts.size()<2)
@@ -287,8 +284,7 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu
 static void MutateTxAddOutAddr(CMutableTransaction& tx, const std::string& strInput)
 {
     // Separate into VALUE:ADDRESS
-    std::vector<std::string> vStrInputParts;
-    boost::split(vStrInputParts, strInput, boost::is_any_of(":"));
+    std::vector<std::string> vStrInputParts = SplitString(strInput, ':');
 
     if (vStrInputParts.size() != 2)
         throw std::runtime_error("TX output missing or too many separators");
@@ -312,8 +308,7 @@ static void MutateTxAddOutAddr(CMutableTransaction& tx, const std::string& strIn
 static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& strInput)
 {
     // Separate into VALUE:PUBKEY[:FLAGS]
-    std::vector<std::string> vStrInputParts;
-    boost::split(vStrInputParts, strInput, boost::is_any_of(":"));
+    std::vector<std::string> vStrInputParts = SplitString(strInput, ':');
 
     if (vStrInputParts.size() < 2 || vStrInputParts.size() > 3)
         throw std::runtime_error("TX output missing or too many separators");
@@ -356,8 +351,7 @@ static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& str
 static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& strInput)
 {
     // Separate into VALUE:REQUIRED:NUMKEYS:PUBKEY1:PUBKEY2:....[:FLAGS]
-    std::vector<std::string> vStrInputParts;
-    boost::split(vStrInputParts, strInput, boost::is_any_of(":"));
+    std::vector<std::string> vStrInputParts = SplitString(strInput, ':');
 
     // Check that there are enough parameters
     if (vStrInputParts.size()<3)
@@ -460,8 +454,7 @@ static void MutateTxAddOutData(CMutableTransaction& tx, const std::string& strIn
 static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& strInput)
 {
     // separate VALUE:SCRIPT[:FLAGS]
-    std::vector<std::string> vStrInputParts;
-    boost::split(vStrInputParts, strInput, boost::is_any_of(":"));
+    std::vector<std::string> vStrInputParts = SplitString(strInput, ':');
     if (vStrInputParts.size() < 2)
         throw std::runtime_error("TX output missing separator");
 
diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index c65a816a5f4d..7a7c72ea2570 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -10,13 +10,11 @@
 #include <deploymentinfo.h>
 #include <hash.h> // for signet block challenge hash
 #include <script/interpreter.h>
+#include <util/string.h>
 #include <util/system.h>
 
 #include <assert.h>
 
-#include <boost/algorithm/string/classification.hpp>
-#include <boost/algorithm/string/split.hpp>
-
 static CBlock CreateGenesisBlock(const char* pszTimestamp, const CScript& genesisOutputScript, uint32_t nTime, uint32_t nNonce, uint32_t nBits, int32_t nVersion, const CAmount& genesisReward)
 {
     CMutableTransaction txNew;
@@ -528,8 +526,7 @@ void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args)
     if (!args.IsArgSet("-vbparams")) return;
 
     for (const std::string& strDeployment : args.GetArgs("-vbparams")) {
-        std::vector<std::string> vDeploymentParams;
-        boost::split(vDeploymentParams, strDeployment, boost::is_any_of(":"));
+        std::vector<std::string> vDeploymentParams = SplitString(strDeployment, ':');
         if (vDeploymentParams.size() < 3 || 4 < vDeploymentParams.size()) {
             throw std::runtime_error("Version bits parameters malformed, expecting deployment:start:end[:min_activation_height]");
         }
diff --git a/src/rest.cpp b/src/rest.cpp
index a8eba05c3fd2..70703bca66a3 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -32,8 +32,6 @@
 #include <any>
 #include <string>
 
-#include <boost/algorithm/string.hpp>
-
 #include <univalue.h>
 
 using node::GetTransaction;
@@ -192,8 +190,7 @@ static bool rest_headers(const std::any& context,
         return false;
     std::string param;
     const RESTResponseFormat rf = ParseDataFormat(param, strURIPart);
-    std::vector<std::string> path;
-    boost::split(path, param, boost::is_any_of("/"));
+    std::vector<std::string> path = SplitString(param, '/');
 
     std::string raw_count;
     std::string hashStr;
@@ -363,8 +360,7 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
     std::string param;
     const RESTResponseFormat rf = ParseDataFormat(param, strURIPart);
 
-    std::vector<std::string> uri_parts;
-    boost::split(uri_parts, param, boost::is_any_of("/"));
+    std::vector<std::string> uri_parts = SplitString(param, '/');
     std::string raw_count;
     std::string raw_blockhash;
     if (uri_parts.size() == 3) {
@@ -484,8 +480,7 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s
     const RESTResponseFormat rf = ParseDataFormat(param, strURIPart);
 
     // request is sent over URI scheme /rest/blockfilter/filtertype/blockhash
-    std::vector<std::string> uri_parts;
-    boost::split(uri_parts, param, boost::is_any_of("/"));
+    std::vector<std::string> uri_parts = SplitString(param, '/');
     if (uri_parts.size() != 2) {
         return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilter/<filtertype>/<blockhash>");
     }
@@ -713,7 +708,7 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::
     if (param.length() > 1)
     {
         std::string strUriParams = param.substr(1);
-        boost::split(uriParts, strUriParams, boost::is_any_of("/"));
+        uriParts = SplitString(strUriParams, '/');
     }
 
     // throw exception in case of an empty request
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index 333ed6f5da15..6083c23d3bd3 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -9,10 +9,9 @@
 #include <shutdown.h>
 #include <sync.h>
 #include <util/strencodings.h>
+#include <util/string.h>
 #include <util/system.h>
 
-#include <boost/algorithm/string/classification.hpp>
-#include <boost/algorithm/string/split.hpp>
 #include <boost/signals2/signal.hpp>
 
 #include <cassert>
@@ -407,8 +406,7 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c
     // Process expected parameters.
     int hole = 0;
     for (const std::string &argNamePattern: argNames) {
-        std::vector<std::string> vargNames;
-        boost::algorithm::split(vargNames, argNamePattern, boost::algorithm::is_any_of("|"));
+        std::vector<std::string> vargNames = SplitString(argNamePattern, '|');
         auto fr = argsIn.end();
         for (const std::string & argName : vargNames) {
             fr = argsIn.find(argName);
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 01fae140cc3d..139bee1cf1bb 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -15,9 +15,6 @@
 
 #include <tuple>
 
-#include <boost/algorithm/string/classification.hpp>
-#include <boost/algorithm/string/split.hpp>
-
 const std::string UNIX_EPOCH_TIME = "UNIX epoch time";
 const std::string EXAMPLE_ADDRESS[2] = {"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl", "bc1q02ad21edsxd23d32dfgqqsz4vv4nmtfzuklhy3"};
 
@@ -513,8 +510,7 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector<RP
 {
     std::set<std::string> named_args;
     for (const auto& arg : m_args) {
-        std::vector<std::string> names;
-        boost::split(names, arg.m_names, boost::is_any_of("|"));
+        std::vector<std::string> names = SplitString(arg.m_names, '|');
         // Should have unique named arguments
         for (const std::string& name : names) {
             CHECK_NONFATAL(named_args.insert(name).second);
@@ -665,8 +661,7 @@ UniValue RPCHelpMan::GetArgMap() const
     UniValue arr{UniValue::VARR};
     for (int i{0}; i < int(m_args.size()); ++i) {
         const auto& arg = m_args.at(i);
-        std::vector<std::string> arg_names;
-        boost::split(arg_names, arg.m_names, boost::is_any_of("|"));
+        std::vector<std::string> arg_names = SplitString(arg.m_names, '|');
         for (const auto& arg_name : arg_names) {
             UniValue map{UniValue::VARR};
             map.push_back(m_name);
diff --git a/src/test/fuzz/script_assets_test_minimizer.cpp b/src/test/fuzz/script_assets_test_minimizer.cpp
index 00a3bed12f04..a18a0de47e19 100644
--- a/src/test/fuzz/script_assets_test_minimizer.cpp
+++ b/src/test/fuzz/script_assets_test_minimizer.cpp
@@ -11,8 +11,8 @@
 #include <streams.h>
 #include <univalue.h>
 #include <util/strencodings.h>
+#include <util/string.h>
 
-#include <boost/algorithm/string.hpp>
 #include <cstdint>
 #include <string>
 #include <vector>
@@ -130,8 +130,7 @@ unsigned int ParseScriptFlags(const std::string& str)
     if (str.empty()) return 0;
 
     unsigned int flags = 0;
-    std::vector<std::string> words;
-    boost::algorithm::split(words, str, boost::algorithm::is_any_of(","));
+    std::vector<std::string> words = SplitString(str, ',');
 
     for (const std::string& word : words) {
         auto it = FLAG_NAMES.find(word);
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 4fb7f9c4056e..df67841b6648 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -31,8 +31,6 @@
 #include <map>
 #include <string>
 
-#include <boost/algorithm/string/classification.hpp>
-#include <boost/algorithm/string/split.hpp>
 #include <boost/test/unit_test.hpp>
 
 #include <univalue.h>
@@ -70,8 +68,7 @@ unsigned int ParseScriptFlags(std::string strFlags)
 {
     if (strFlags.empty() || strFlags == "NONE") return 0;
     unsigned int flags = 0;
-    std::vector<std::string> words;
-    boost::algorithm::split(words, strFlags, boost::algorithm::is_any_of(","));
+    std::vector<std::string> words = SplitString(strFlags, ',');
 
     for (const std::string& word : words)
     {
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index a15094e5c877..6d7331f23aa3 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -24,9 +24,7 @@
 #include <set>
 #include <vector>
 
-#include <boost/algorithm/string/classification.hpp>
 #include <boost/algorithm/string/replace.hpp>
-#include <boost/algorithm/string/split.hpp>
 
 #include <event2/buffer.h>
 #include <event2/bufferevent.h>
@@ -347,8 +345,8 @@ void TorController::get_socks_cb(TorControlConnection& _conn, const TorControlRe
         for (const auto& line : reply.lines) {
             if (0 == line.compare(0, 20, "net/listeners/socks=")) {
                 const std::string port_list_str = line.substr(20);
-                std::vector<std::string> port_list;
-                boost::split(port_list, port_list_str, boost::is_any_of(" "));
+                std::vector<std::string> port_list = SplitString(port_list_str, ' ');
+
                 for (auto& portstr : port_list) {
                     if (portstr.empty()) continue;
                     if ((portstr[0] == '"' || portstr[0] == '\'') && portstr.size() >= 2 && (*portstr.rbegin() == portstr[0])) {
@@ -542,8 +540,10 @@ void TorController::protocolinfo_cb(TorControlConnection& _conn, const TorContro
             if (l.first == "AUTH") {
                 std::map<std::string,std::string> m = ParseTorReplyMapping(l.second);
                 std::map<std::string,std::string>::iterator i;
-                if ((i = m.find("METHODS")) != m.end())
-                    boost::split(methods, i->second, boost::is_any_of(","));
+                if ((i = m.find("METHODS")) != m.end()) {
+                    std::vector<std::string> m_vec = SplitString(i->second, ',');
+                    methods = std::set<std::string>(m_vec.begin(), m_vec.end());
+                }
                 if ((i = m.find("COOKIEFILE")) != m.end())
                     cookiefile = i->second;
             } else if (l.first == "VERSION") {
diff --git a/src/util/spanparsing.cpp b/src/util/spanparsing.cpp
index 50f6aee419c4..8614bd117695 100644
--- a/src/util/spanparsing.cpp
+++ b/src/util/spanparsing.cpp
@@ -48,20 +48,4 @@ Span<const char> Expr(Span<const char>& sp)
     return ret;
 }
 
-std::vector<Span<const char>> Split(const Span<const char>& sp, char sep)
-{
-    std::vector<Span<const char>> ret;
-    auto it = sp.begin();
-    auto start = it;
-    while (it != sp.end()) {
-        if (*it == sep) {
-            ret.emplace_back(start, it);
-            start = it + 1;
-        }
-        ++it;
-    }
-    ret.emplace_back(start, it);
-    return ret;
-}
-
 } // namespace spanparsing
diff --git a/src/util/spanparsing.h b/src/util/spanparsing.h
index fa2e698e6dc5..ebec8714a7bf 100644
--- a/src/util/spanparsing.h
+++ b/src/util/spanparsing.h
@@ -43,7 +43,22 @@ Span<const char> Expr(Span<const char>& sp);
  * Note that this function does not care about braces, so splitting
  * "foo(bar(1),2),3) on ',' will return {"foo(bar(1)", "2)", "3)"}.
  */
-std::vector<Span<const char>> Split(const Span<const char>& sp, char sep);
+template <typename T = Span<const char>>
+std::vector<T> Split(const Span<const char>& sp, char sep)
+{
+    std::vector<T> ret;
+    auto it = sp.begin();
+    auto start = it;
+    while (it != sp.end()) {
+        if (*it == sep) {
+            ret.emplace_back(start, it);
+            start = it + 1;
+        }
+        ++it;
+    }
+    ret.emplace_back(start, it);
+    return ret;
+}
 
 } // namespace spanparsing
 
diff --git a/src/util/string.h b/src/util/string.h
index a3b8df8d78ec..bcd6905fd585 100644
--- a/src/util/string.h
+++ b/src/util/string.h
@@ -6,6 +6,7 @@
 #define BITCOIN_UTIL_STRING_H
 
 #include <attributes.h>
+#include <util/spanparsing.h>
 
 #include <algorithm>
 #include <array>
@@ -15,6 +16,11 @@
 #include <string>
 #include <vector>
 
+[[nodiscard]] inline std::vector<std::string> SplitString(std::string_view str, char sep)
+{
+    return spanparsing::Split<std::string>(str, sep);
+}
+
 [[nodiscard]] inline std::string TrimString(const std::string& str, const std::string& pattern = " \f\n\r\t\v")
 {
     std::string::size_type front = str.find_first_not_of(pattern);
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index b048ddfc6eb3..2d57f20a041b 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -26,8 +26,6 @@
 #include <tuple>
 #include <string>
 
-#include <boost/algorithm/string.hpp>
-
 #include <univalue.h>
 
 
@@ -546,8 +544,7 @@ RPCHelpMan importwallet()
             if (line.empty() || line[0] == '#')
                 continue;
 
-            std::vector<std::string> vstr;
-            boost::split(vstr, line, boost::is_any_of(" "));
+            std::vector<std::string> vstr = SplitString(line, ' ');
             if (vstr.size() < 2)
                 continue;
             CKey key = DecodeSecret(vstr[0]);

From 4fad7e46d94a0fdee4ff917e81360d7ae6bd8110 Mon Sep 17 00:00:00 2001
From: Kiminuo <kiminuo@protonmail.com>
Date: Mon, 13 Sep 2021 12:17:35 +0200
Subject: [PATCH 04/28] test: add unit tests for `SplitString` helper

---
 src/test/util_tests.cpp | 49 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index b5d8411e1d7a..779ed2003267 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -2349,6 +2349,55 @@ BOOST_AUTO_TEST_CASE(test_spanparsing)
     BOOST_CHECK_EQUAL(SpanToStr(results[3]), "");
 }
 
+BOOST_AUTO_TEST_CASE(test_SplitString)
+{
+    // Empty string.
+    {
+        std::vector<std::string> result = SplitString("", '-');
+        BOOST_CHECK_EQUAL(result.size(), 1);
+        BOOST_CHECK_EQUAL(result[0], "");
+    }
+
+    // Empty items.
+    {
+        std::vector<std::string> result = SplitString("-", '-');
+        BOOST_CHECK_EQUAL(result.size(), 2);
+        BOOST_CHECK_EQUAL(result[0], "");
+        BOOST_CHECK_EQUAL(result[1], "");
+    }
+
+    // More empty items.
+    {
+        std::vector<std::string> result = SplitString("--", '-');
+        BOOST_CHECK_EQUAL(result.size(), 3);
+        BOOST_CHECK_EQUAL(result[0], "");
+        BOOST_CHECK_EQUAL(result[1], "");
+        BOOST_CHECK_EQUAL(result[2], "");
+    }
+
+    // Separator is not present.
+    {
+        std::vector<std::string> result = SplitString("abc", '-');
+        BOOST_CHECK_EQUAL(result.size(), 1);
+        BOOST_CHECK_EQUAL(result[0], "abc");
+    }
+
+    // Basic behavior.
+    {
+        std::vector<std::string> result = SplitString("a-b", '-');
+        BOOST_CHECK_EQUAL(result.size(), 2);
+        BOOST_CHECK_EQUAL(result[0], "a");
+        BOOST_CHECK_EQUAL(result[1], "b");
+    }
+
+    // Case-sensitivity of the separator.
+    {
+        std::vector<std::string> result = SplitString("AAA", 'a');
+        BOOST_CHECK_EQUAL(result.size(), 1);
+        BOOST_CHECK_EQUAL(result[0], "AAA");
+    }
+}
+
 BOOST_AUTO_TEST_CASE(test_LogEscapeMessage)
 {
     // ASCII and UTF-8 must pass through unaltered.

From a62e84438d27ee6213219fe2c233e58814fcbb5d Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Mon, 13 Sep 2021 13:52:07 +0200
Subject: [PATCH 05/28] fuzz: add `SplitString` fuzz target

---
 src/test/fuzz/string.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp
index ca57af25c49f..b4876d427fb3 100644
--- a/src/test/fuzz/string.cpp
+++ b/src/test/fuzz/string.cpp
@@ -224,6 +224,7 @@ FUZZ_TARGET(string)
         int64_t amount_out;
         (void)ParseFixedPoint(random_string_1, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 1024), &amount_out);
     }
+    (void)SplitString(random_string_1, fuzzed_data_provider.ConsumeIntegral<char>());
     {
         (void)Untranslated(random_string_1);
         const bilingual_str bs1{random_string_1, random_string_2};

From 912f1ed181161b0365776cd490b63137aaad708a Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Mon, 21 Mar 2022 14:19:10 -0400
Subject: [PATCH 06/28] wallet: track which coin selection algorithm produced a
 SelectionResult

---
 src/wallet/coinselection.cpp           | 19 ++++++++++++++++---
 src/wallet/coinselection.h             | 16 ++++++++++++++--
 src/wallet/spend.cpp                   |  4 ++--
 src/wallet/test/coinselector_tests.cpp |  2 +-
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 433759e086f1..0b236e2e484c 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -64,7 +64,7 @@ static const size_t TOTAL_TRIES = 100000;
 
 std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change)
 {
-    SelectionResult result(selection_target);
+    SelectionResult result(selection_target, SelectionAlgorithm::BNB);
     CAmount curr_value = 0;
     std::vector<size_t> curr_selection; // selected utxo indexes
 
@@ -167,7 +167,7 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo
 
 std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, FastRandomContext& rng)
 {
-    SelectionResult result(target_value);
+    SelectionResult result(target_value, SelectionAlgorithm::SRD);
 
     std::vector<size_t> indexes;
     indexes.resize(utxo_pool.size());
@@ -249,7 +249,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v
 std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue,
                                               CAmount change_target, FastRandomContext& rng)
 {
-    SelectionResult result(nTargetValue);
+    SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK);
 
     // List of values less than target
     std::optional<OutputGroup> lowest_larger;
@@ -460,4 +460,17 @@ std::string COutput::ToString() const
 {
     return strprintf("COutput(%s, %d, %d) [%s]", outpoint.hash.ToString(), outpoint.n, depth, FormatMoney(txout.nValue));
 }
+
+std::string GetAlgorithmName(const SelectionAlgorithm algo)
+{
+    switch (algo)
+    {
+    case SelectionAlgorithm::BNB: return "bnb";
+    case SelectionAlgorithm::KNAPSACK: return "knapsack";
+    case SelectionAlgorithm::SRD: return "srd";
+    case SelectionAlgorithm::MANUAL: return "manual";
+    // No default case to allow for compiler to warn
+    }
+    assert(false);
+}
 } // namespace wallet
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index c1484c0a5770..44eec9a41727 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -239,6 +239,16 @@ struct OutputGroup
  */
 [[nodiscard]] CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng);
 
+enum class SelectionAlgorithm : uint8_t
+{
+    BNB = 0,
+    KNAPSACK = 1,
+    SRD = 2,
+    MANUAL = 3,
+};
+
+std::string GetAlgorithmName(const SelectionAlgorithm algo);
+
 struct SelectionResult
 {
 private:
@@ -250,10 +260,12 @@ struct SelectionResult
     bool m_use_effective{false};
     /** The computed waste */
     std::optional<CAmount> m_waste;
+    /** The algorithm used to produce this result */
+    SelectionAlgorithm m_algo;
 
 public:
-    explicit SelectionResult(const CAmount target)
-        : m_target(target) {}
+    explicit SelectionResult(const CAmount target, SelectionAlgorithm algo)
+        : m_target(target), m_algo(algo) {}
 
     SelectionResult() = delete;
 
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 9e508f3a32f9..b4b726d4fd98 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -435,7 +435,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
              */
             preset_inputs.Insert(out, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false);
         }
-        SelectionResult result(nTargetValue);
+        SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL);
         result.AddInput(preset_inputs);
         if (result.GetSelectedValue() < nTargetValue) return std::nullopt;
         return result;
@@ -519,7 +519,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
     // permissive CoinEligibilityFilter.
     std::optional<SelectionResult> res = [&] {
         // Pre-selected inputs already cover the target amount.
-        if (value_to_select <= 0) return std::make_optional(SelectionResult(nTargetValue));
+        if (value_to_select <= 0) return std::make_optional(SelectionResult(nTargetValue, SelectionAlgorithm::MANUAL));
 
         // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six
         // confirmations on outputs received from other wallets and only spend confirmed change.
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 2a08c8ab5720..72e749477b17 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -168,7 +168,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     FastRandomContext rand{};
     // Setup
     std::vector<COutput> utxo_pool;
-    SelectionResult expected_result(CAmount(0));
+    SelectionResult expected_result(CAmount(0), SelectionAlgorithm::BNB);
 
     /////////////////////////
     // Known Outcome tests //

From 15b58383d0029c4ae7b487e03cd451e1580eb91d Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Wed, 23 Mar 2022 13:17:07 -0400
Subject: [PATCH 07/28] wallet: compute waste for SelectionResults of preset
 inputs

When we use only manually specified inputs, we should still calculate
the waste so that if anything later on calls GetWaste (in order to log
it), there won't be an error.
---
 src/wallet/spend.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index b4b726d4fd98..233478af41e7 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -438,6 +438,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
         SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL);
         result.AddInput(preset_inputs);
         if (result.GetSelectedValue() < nTargetValue) return std::nullopt;
+        result.ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
         return result;
     }
 
@@ -573,6 +574,9 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
 
     // Add preset inputs to result
     res->AddInput(preset_inputs);
+    if (res->m_algo == SelectionAlgorithm::MANUAL) {
+        res->ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
+    }
 
     return res;
 }

From 8e3f39e4fa2d8c63bc697c9ebd303965fcccea55 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Mon, 21 Mar 2022 14:33:29 -0400
Subject: [PATCH 08/28] wallet: Add some tracepoints for coin selection

---
 src/wallet/coinselection.h | 9 +++++----
 src/wallet/spend.cpp       | 5 +++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 44eec9a41727..25c672eda049 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -254,16 +254,17 @@ struct SelectionResult
 private:
     /** Set of inputs selected by the algorithm to use in the transaction */
     std::set<COutput> m_selected_inputs;
-    /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */
-    const CAmount m_target;
     /** Whether the input values for calculations should be the effective value (true) or normal value (false) */
     bool m_use_effective{false};
     /** The computed waste */
     std::optional<CAmount> m_waste;
-    /** The algorithm used to produce this result */
-    SelectionAlgorithm m_algo;
 
 public:
+    /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */
+    const CAmount m_target;
+    /** The algorithm used to produce this result */
+    const SelectionAlgorithm m_algo;
+
     explicit SelectionResult(const CAmount target, SelectionAlgorithm algo)
         : m_target(target), m_algo(algo) {}
 
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 233478af41e7..55c0a2cb7ff9 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -11,6 +11,7 @@
 #include <util/fees.h>
 #include <util/moneystr.h>
 #include <util/rbf.h>
+#include <util/trace.h>
 #include <util/translation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/fees.h>
@@ -792,6 +793,7 @@ static bool CreateTransactionInternal(
         error = _("Insufficient funds");
         return false;
     }
+    TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue());
 
     // Always make a change output
     // We will reduce the fee from this change output later, and remove the output if it is too small.
@@ -982,8 +984,10 @@ bool CreateTransaction(
     int nChangePosIn = nChangePosInOut;
     Assert(!tx); // tx is an out-param. TODO change the return type from bool to tx (or nullptr)
     bool res = CreateTransactionInternal(wallet, vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, fee_calc_out, sign);
+    TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res, nFeeRet, nChangePosInOut);
     // try with avoidpartialspends unless it's enabled already
     if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) {
+        TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str());
         CCoinControl tmp_cc = coin_control;
         tmp_cc.m_avoid_partial_spends = true;
         CAmount nFeeRet2;
@@ -994,6 +998,7 @@ bool CreateTransaction(
             // if fee of this alternative one is within the range of the max fee, we use this one
             const bool use_aps = nFeeRet2 <= nFeeRet + wallet.m_max_aps_fee;
             wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet, nFeeRet2, use_aps ? "grouped" : "non-grouped");
+            TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, res, nFeeRet2, nChangePosInOut2);
             if (use_aps) {
                 tx = tx2;
                 nFeeRet = nFeeRet2;

From a237a065cc2c6337e3797cc30a0f84c56c6d2f3b Mon Sep 17 00:00:00 2001
From: w0xlt <94266259+w0xlt@users.noreply.github.com>
Date: Tue, 25 Jan 2022 18:05:04 -0300
Subject: [PATCH 09/28] scripted-diff: rename cs_totalBytesSent ->
 m_total_bytes_sent_mutex

-BEGIN VERIFY SCRIPT-
sed -i 's/cs_totalBytesSent/m_total_bytes_sent_mutex/g' -- $(git grep --files-with-matches 'cs_totalBytesSent')
-END VERIFY SCRIPT-
---
 src/net.cpp | 12 ++++++------
 src/net.h   | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 602d56ab984f..77a1d1339bb5 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2913,7 +2913,7 @@ void CConnman::RecordBytesRecv(uint64_t bytes)
 
 void CConnman::RecordBytesSent(uint64_t bytes)
 {
-    LOCK(cs_totalBytesSent);
+    LOCK(m_total_bytes_sent_mutex);
     nTotalBytesSent += bytes;
 
     const auto now = GetTime<std::chrono::seconds>();
@@ -2929,7 +2929,7 @@ void CConnman::RecordBytesSent(uint64_t bytes)
 
 uint64_t CConnman::GetMaxOutboundTarget() const
 {
-    LOCK(cs_totalBytesSent);
+    LOCK(m_total_bytes_sent_mutex);
     return nMaxOutboundLimit;
 }
 
@@ -2940,7 +2940,7 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeframe() const
 
 std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const
 {
-    LOCK(cs_totalBytesSent);
+    LOCK(m_total_bytes_sent_mutex);
     if (nMaxOutboundLimit == 0)
         return 0s;
 
@@ -2954,7 +2954,7 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const
 
 bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const
 {
-    LOCK(cs_totalBytesSent);
+    LOCK(m_total_bytes_sent_mutex);
     if (nMaxOutboundLimit == 0)
         return false;
 
@@ -2974,7 +2974,7 @@ bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const
 
 uint64_t CConnman::GetOutboundTargetBytesLeft() const
 {
-    LOCK(cs_totalBytesSent);
+    LOCK(m_total_bytes_sent_mutex);
     if (nMaxOutboundLimit == 0)
         return 0;
 
@@ -2988,7 +2988,7 @@ uint64_t CConnman::GetTotalBytesRecv() const
 
 uint64_t CConnman::GetTotalBytesSent() const
 {
-    LOCK(cs_totalBytesSent);
+    LOCK(m_total_bytes_sent_mutex);
     return nTotalBytesSent;
 }
 
diff --git a/src/net.h b/src/net.h
index 1bc011c74891..e9737278d9fb 100644
--- a/src/net.h
+++ b/src/net.h
@@ -776,7 +776,7 @@ class CConnman
         nReceiveFloodSize = connOptions.nReceiveFloodSize;
         m_peer_connect_timeout = std::chrono::seconds{connOptions.m_peer_connect_timeout};
         {
-            LOCK(cs_totalBytesSent);
+            LOCK(m_total_bytes_sent_mutex);
             nMaxOutboundLimit = connOptions.nMaxOutboundLimit;
         }
         vWhitelistedRange = connOptions.vWhitelistedRange;
@@ -1062,14 +1062,14 @@ class CConnman
     static bool NodeFullyConnected(const CNode* pnode);
 
     // Network usage totals
-    mutable RecursiveMutex cs_totalBytesSent;
+    mutable RecursiveMutex m_total_bytes_sent_mutex;
     std::atomic<uint64_t> nTotalBytesRecv{0};
-    uint64_t nTotalBytesSent GUARDED_BY(cs_totalBytesSent) {0};
+    uint64_t nTotalBytesSent GUARDED_BY(m_total_bytes_sent_mutex) {0};
 
     // outbound limit & stats
-    uint64_t nMaxOutboundTotalBytesSentInCycle GUARDED_BY(cs_totalBytesSent) {0};
-    std::chrono::seconds nMaxOutboundCycleStartTime GUARDED_BY(cs_totalBytesSent) {0};
-    uint64_t nMaxOutboundLimit GUARDED_BY(cs_totalBytesSent);
+    uint64_t nMaxOutboundTotalBytesSentInCycle GUARDED_BY(m_total_bytes_sent_mutex) {0};
+    std::chrono::seconds nMaxOutboundCycleStartTime GUARDED_BY(m_total_bytes_sent_mutex) {0};
+    uint64_t nMaxOutboundLimit GUARDED_BY(m_total_bytes_sent_mutex);
 
     // P2P timeout in seconds
     std::chrono::seconds m_peer_connect_timeout;

From fa1970f075292d7312654730a994a68c2ca8bc06 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Tue, 19 Apr 2022 11:59:09 +0200
Subject: [PATCH 10/28] Make BlockManager::LoadBlockIndex private

---
 src/node/blockstorage.h | 15 +++++++--------
 src/validation.h        |  1 -
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index a051e9080895..7c6c379a470b 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -80,6 +80,13 @@ class BlockManager
     friend ChainstateManager;
 
 private:
+    /**
+     * Load the blocktree off disk and into memory. Populate certain metadata
+     * per index entry (nStatus, nChainWork, nTimeMax, etc.) as well as peripheral
+     * collections like m_dirty_blockindex.
+     */
+    bool LoadBlockIndex(const Consensus::Params& consensus_params)
+        EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     void FlushBlockFile(bool fFinalize = false, bool finalize_undo = false);
     void FlushUndoFile(int block_file, bool finalize = false);
     bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, CChain& active_chain, uint64_t nTime, bool fKnown);
@@ -136,14 +143,6 @@ class BlockManager
     bool WriteBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
     bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-    /**
-     * Load the blocktree off disk and into memory. Populate certain metadata
-     * per index entry (nStatus, nChainWork, nTimeMax, etc.) as well as peripheral
-     * collections like m_dirty_blockindex.
-     */
-    bool LoadBlockIndex(const Consensus::Params& consensus_params)
-        EXCLUSIVE_LOCKS_REQUIRED(cs_main);
-
     /** Clear all data members. */
     void Unload() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
diff --git a/src/validation.h b/src/validation.h
index 53ea2d4aea7e..f49f4f788876 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -837,7 +837,6 @@ class ChainstateManager
     bool m_snapshot_validated{false};
 
     CBlockIndex* m_best_invalid;
-    friend bool node::BlockManager::LoadBlockIndex(const Consensus::Params&);
 
     //! Internal helper for ActivateSnapshot().
     [[nodiscard]] bool PopulateAndValidateSnapshot(

From ca02b68e8a7147f80cbe84b0742908b0b0faa04d Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Thu, 14 Apr 2022 12:53:17 -0400
Subject: [PATCH 11/28] doc: document coin selection tracepoints

---
 doc/tracing.md | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/doc/tracing.md b/doc/tracing.md
index 6ec6a6c218ac..b6e3b9263a99 100644
--- a/doc/tracing.md
+++ b/doc/tracing.md
@@ -168,6 +168,49 @@ Arguments passed:
 4. Value of the coin as `int64`
 5. If the coin is a coinbase as `bool`
 
+### Context `coin_selection`
+
+#### Tracepoint `coin_selection:selected_coins`
+
+Is called when `SelectCoins` completes.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+2. Coin selection algorithm name as `pointer to C-style string`
+3. Selection target value as `int64`
+4. Calculated waste metric of the solution as `int64`
+5. Total value of the selected inputs as `int64`
+
+#### Tracepoint `coin_selection:normal_create_tx_internal`
+
+Is called when the first `CreateTransactionInternal` completes.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+2. Whether `CreateTransactionInternal` succeeded as `bool`
+3. The expected transaction fee as an `int64`
+4. The position of the change output as an `int32`
+
+#### Tracepoint `coin_selection:attempting_aps_create_tx`
+
+Is called when `CreateTransactionInternal` is called the second time for the optimistic
+Avoid Partial Spends selection attempt. This is used to determine whether the next
+tracepoints called are for the Avoid Partial Spends solution, or a different transaction.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+
+#### Tracepoint `coin_selection:aps_create_tx_internal`
+
+Is called when the second `CreateTransactionInternal` with Avoid Partial Spends enabled completes.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+2. Whether the Avoid Partial Spends solution will be used as `bool`
+3. Whether `CreateTransactionInternal` succeeded as` bool`
+4. The expected transaction fee as an `int64`
+5. The position of the change output as an `int32`
+
 ## Adding tracepoints to Bitcoin Core
 
 To add a new tracepoint, `#include <util/trace.h>` in the compilation unit where

From ab5af9ca7293239ffc24ea7e23159b8184543f94 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Thu, 14 Apr 2022 13:41:18 -0400
Subject: [PATCH 12/28] test: Add test for coinselection tracepoints

---
 .../interface_usdt_coinselection.py           | 208 ++++++++++++++++++
 test/functional/test_runner.py                |   1 +
 2 files changed, 209 insertions(+)
 create mode 100755 test/functional/interface_usdt_coinselection.py

diff --git a/test/functional/interface_usdt_coinselection.py b/test/functional/interface_usdt_coinselection.py
new file mode 100755
index 000000000000..ef32feda9916
--- /dev/null
+++ b/test/functional/interface_usdt_coinselection.py
@@ -0,0 +1,208 @@
+#!/usr/bin/env python3
+# Copyright (c) 2022 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+"""  Tests the coin_selection:* tracepoint API interface.
+     See https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#context-coin_selection
+"""
+
+# Test will be skipped if we don't have bcc installed
+try:
+    from bcc import BPF, USDT # type: ignore[import]
+except ImportError:
+    pass
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_greater_than,
+    assert_raises_rpc_error,
+)
+
+coinselection_tracepoints_program = """
+#include <uapi/linux/ptrace.h>
+
+#define WALLET_NAME_LENGTH 16
+#define ALGO_NAME_LENGTH 16
+
+struct event_data
+{
+    u8 type;
+    char wallet_name[WALLET_NAME_LENGTH];
+
+    // selected coins event
+    char algo[ALGO_NAME_LENGTH];
+    s64 target;
+    s64 waste;
+    s64 selected_value;
+
+    // create tx event
+    bool success;
+    s64 fee;
+    s32 change_pos;
+
+    // aps create tx event
+    bool use_aps;
+};
+
+BPF_QUEUE(coin_selection_events, struct event_data, 1024);
+
+int trace_selected_coins(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 1;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    bpf_usdt_readarg_p(2, ctx, &data.algo, ALGO_NAME_LENGTH);
+    bpf_usdt_readarg(3, ctx, &data.target);
+    bpf_usdt_readarg(4, ctx, &data.waste);
+    bpf_usdt_readarg(5, ctx, &data.selected_value);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+
+int trace_normal_create_tx(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 2;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    bpf_usdt_readarg(2, ctx, &data.success);
+    bpf_usdt_readarg(3, ctx, &data.fee);
+    bpf_usdt_readarg(4, ctx, &data.change_pos);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+
+int trace_attempt_aps(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 3;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+
+int trace_aps_create_tx(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 4;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    bpf_usdt_readarg(2, ctx, &data.use_aps);
+    bpf_usdt_readarg(3, ctx, &data.success);
+    bpf_usdt_readarg(4, ctx, &data.fee);
+    bpf_usdt_readarg(5, ctx, &data.change_pos);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+"""
+
+
+class CoinSelectionTracepointTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.setup_clean_chain = True
+
+    def skip_test_if_missing_module(self):
+        self.skip_if_platform_not_linux()
+        self.skip_if_no_bitcoind_tracepoints()
+        self.skip_if_no_python_bcc()
+        self.skip_if_no_bpf_permissions()
+        self.skip_if_no_wallet()
+
+    def get_tracepoints(self, expected_types):
+        events = []
+        try:
+            for i in range(0, len(expected_types) + 1):
+                event = self.bpf["coin_selection_events"].pop()
+                assert_equal(event.wallet_name.decode(), self.default_wallet_name)
+                assert_equal(event.type, expected_types[i])
+                events.append(event)
+            else:
+                # If the loop exits successfully instead of throwing a KeyError, then we have had
+                # more events than expected. There should be no more than len(expected_types) events.
+                assert False
+        except KeyError:
+            assert_equal(len(events), len(expected_types))
+            return events
+
+
+    def determine_selection_from_usdt(self, events):
+        success = None
+        use_aps = None
+        algo = None
+        waste = None
+        change_pos = None
+
+        is_aps = False
+        sc_events = []
+        for event in events:
+            if event.type == 1:
+                if not is_aps:
+                    algo = event.algo.decode()
+                    waste = event.waste
+                sc_events.append(event)
+            elif event.type == 2:
+                success = event.success
+                if not is_aps:
+                    change_pos = event.change_pos
+            elif event.type == 3:
+                is_aps = True
+            elif event.type == 4:
+                assert is_aps
+                if event.use_aps:
+                    use_aps = True
+                    assert_equal(len(sc_events), 2)
+                    algo = sc_events[1].algo.decode()
+                    waste = sc_events[1].waste
+                    change_pos = event.change_pos
+        return success, use_aps, algo, waste, change_pos
+
+    def run_test(self):
+        self.log.info("hook into the coin_selection tracepoints")
+        ctx = USDT(pid=self.nodes[0].process.pid)
+        ctx.enable_probe(probe="coin_selection:selected_coins", fn_name="trace_selected_coins")
+        ctx.enable_probe(probe="coin_selection:normal_create_tx_internal", fn_name="trace_normal_create_tx")
+        ctx.enable_probe(probe="coin_selection:attempting_aps_create_tx", fn_name="trace_attempt_aps")
+        ctx.enable_probe(probe="coin_selection:aps_create_tx_internal", fn_name="trace_aps_create_tx")
+        self.bpf = BPF(text=coinselection_tracepoints_program, usdt_contexts=[ctx], debug=0)
+
+        self.log.info("Prepare wallets")
+        self.generate(self.nodes[0], 101)
+        wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        self.log.info("Sending a transaction should result in all tracepoints")
+        # We should have 5 tracepoints in the order:
+        # 1. selected_coins (type 1)
+        # 2. normal_create_tx_internal (type 2)
+        # 3. attempting_aps_create_tx (type 3)
+        # 4. selected_coins (type 1)
+        # 5. aps_create_tx_internal (type 4)
+        wallet.sendtoaddress(wallet.getnewaddress(), 10)
+        events = self.get_tracepoints([1, 2, 3, 1, 4])
+        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        assert_equal(success, True)
+        assert_greater_than(change_pos, -1)
+
+        self.log.info("Failing to fund results in 1 tracepoint")
+        # We should have 1 tracepoints in the order
+        # 1. normal_create_tx_internal (type 2)
+        assert_raises_rpc_error(-6, "Insufficient funds", wallet.sendtoaddress, wallet.getnewaddress(), 102 * 50)
+        events = self.get_tracepoints([2])
+        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        assert_equal(success, False)
+
+        self.log.info("Explicitly enabling APS results in 2 tracepoints")
+        # We should have 2 tracepoints in the order
+        # 1. selected_coins (type 1)
+        # 2. normal_create_tx_internal (type 2)
+        wallet.setwalletflag("avoid_reuse")
+        wallet.sendtoaddress(address=wallet.getnewaddress(), amount=10, avoid_reuse=True)
+        events = self.get_tracepoints([1, 2])
+        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        assert_equal(success, True)
+        assert_equal(use_aps, None)
+
+        self.bpf.cleanup()
+
+
+if __name__ == '__main__':
+    CoinSelectionTracepointTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 1f0f806d919e..7a1d1a97765a 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -168,6 +168,7 @@
     'wallet_reorgsrestore.py',
     'interface_http.py',
     'interface_rpc.py',
+    'interface_usdt_coinselection.py',
     'interface_usdt_net.py',
     'interface_usdt_utxocache.py',
     'interface_usdt_validation.py',

From 8be75fd0f0039eeea5f9af7c1eb17c584ed9f507 Mon Sep 17 00:00:00 2001
From: w0xlt <94266259+w0xlt@users.noreply.github.com>
Date: Tue, 25 Jan 2022 18:18:52 -0300
Subject: [PATCH 13/28] p2p: add assertions and negative TS annotations for
 `m_total_bytes_sent_mutex`

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
---
 src/net.cpp | 24 +++++++++++++++++++++++-
 src/net.h   | 34 ++++++++++++++++++++--------------
 2 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 77a1d1339bb5..a2ba625cdbc0 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1563,6 +1563,8 @@ void CConnman::SocketEvents(const std::vector<CNode*>& nodes,
 
 void CConnman::SocketHandler()
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+
     std::set<SOCKET> recv_set;
     std::set<SOCKET> send_set;
     std::set<SOCKET> error_set;
@@ -1589,6 +1591,8 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
                                       const std::set<SOCKET>& send_set,
                                       const std::set<SOCKET>& error_set)
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+
     for (CNode* pnode : nodes) {
         if (interruptNet)
             return;
@@ -1690,6 +1694,8 @@ void CConnman::SocketHandlerListening(const std::set<SOCKET>& recv_set)
 
 void CConnman::ThreadSocketHandler()
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
+
     SetSyscallSandboxPolicy(SyscallSandboxPolicy::NET);
     while (!interruptNet)
     {
@@ -2561,6 +2567,7 @@ bool CConnman::InitBinds(const Options& options)
 
 bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     Init(connOptions);
 
     if (fListen && !InitBinds(connOptions)) {
@@ -2913,7 +2920,9 @@ void CConnman::RecordBytesRecv(uint64_t bytes)
 
 void CConnman::RecordBytesSent(uint64_t bytes)
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     LOCK(m_total_bytes_sent_mutex);
+
     nTotalBytesSent += bytes;
 
     const auto now = GetTime<std::chrono::seconds>();
@@ -2929,6 +2938,7 @@ void CConnman::RecordBytesSent(uint64_t bytes)
 
 uint64_t CConnman::GetMaxOutboundTarget() const
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     LOCK(m_total_bytes_sent_mutex);
     return nMaxOutboundLimit;
 }
@@ -2940,7 +2950,15 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeframe() const
 
 std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     LOCK(m_total_bytes_sent_mutex);
+    return GetMaxOutboundTimeLeftInCycle_();
+}
+
+std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle_() const
+{
+    AssertLockHeld(m_total_bytes_sent_mutex);
+
     if (nMaxOutboundLimit == 0)
         return 0s;
 
@@ -2954,6 +2972,7 @@ std::chrono::seconds CConnman::GetMaxOutboundTimeLeftInCycle() const
 
 bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     LOCK(m_total_bytes_sent_mutex);
     if (nMaxOutboundLimit == 0)
         return false;
@@ -2961,7 +2980,7 @@ bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const
     if (historicalBlockServingLimit)
     {
         // keep a large enough buffer to at least relay each block once
-        const std::chrono::seconds timeLeftInCycle = GetMaxOutboundTimeLeftInCycle();
+        const std::chrono::seconds timeLeftInCycle = GetMaxOutboundTimeLeftInCycle_();
         const uint64_t buffer = timeLeftInCycle / std::chrono::minutes{10} * MAX_BLOCK_SERIALIZED_SIZE;
         if (buffer >= nMaxOutboundLimit || nMaxOutboundTotalBytesSentInCycle >= nMaxOutboundLimit - buffer)
             return true;
@@ -2974,6 +2993,7 @@ bool CConnman::OutboundTargetReached(bool historicalBlockServingLimit) const
 
 uint64_t CConnman::GetOutboundTargetBytesLeft() const
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     LOCK(m_total_bytes_sent_mutex);
     if (nMaxOutboundLimit == 0)
         return 0;
@@ -2988,6 +3008,7 @@ uint64_t CConnman::GetTotalBytesRecv() const
 
 uint64_t CConnman::GetTotalBytesSent() const
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     LOCK(m_total_bytes_sent_mutex);
     return nTotalBytesSent;
 }
@@ -3035,6 +3056,7 @@ bool CConnman::NodeFullyConnected(const CNode* pnode)
 
 void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
 {
+    AssertLockNotHeld(m_total_bytes_sent_mutex);
     size_t nMessageSize = msg.data.size();
     LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n", msg.m_type, nMessageSize, pnode->GetId());
     if (gArgs.GetBoolArg("-capturemessages", false)) {
diff --git a/src/net.h b/src/net.h
index e9737278d9fb..8cd83e00b506 100644
--- a/src/net.h
+++ b/src/net.h
@@ -760,7 +760,10 @@ class CConnman
         bool m_i2p_accept_incoming;
     };
 
-    void Init(const Options& connOptions) {
+    void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex)
+    {
+        AssertLockNotHeld(m_total_bytes_sent_mutex);
+
         nLocalServices = connOptions.nLocalServices;
         nMaxConnections = connOptions.nMaxConnections;
         m_max_outbound_full_relay = std::min(connOptions.m_max_outbound_full_relay, connOptions.nMaxConnections);
@@ -789,7 +792,7 @@ class CConnman
 
     CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, bool network_active = true);
     ~CConnman();
-    bool Start(CScheduler& scheduler, const Options& options);
+    bool Start(CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     void StopThreads();
     void StopNodes();
@@ -808,7 +811,7 @@ class CConnman
 
     bool ForNode(NodeId id, std::function<bool(CNode* pnode)> func);
 
-    void PushMessage(CNode* pnode, CSerializedNetMsg&& msg);
+    void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     using NodeFn = std::function<void(CNode*)>;
     void ForEachNode(const NodeFn& func)
@@ -899,24 +902,22 @@ class CConnman
     //! that peer during `net_processing.cpp:PushNodeVersion()`.
     ServiceFlags GetLocalServices() const;
 
-    uint64_t GetMaxOutboundTarget() const;
+    uint64_t GetMaxOutboundTarget() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
     std::chrono::seconds GetMaxOutboundTimeframe() const;
 
     //! check if the outbound target is reached
     //! if param historicalBlockServingLimit is set true, the function will
     //! response true if the limit for serving historical blocks has been reached
-    bool OutboundTargetReached(bool historicalBlockServingLimit) const;
+    bool OutboundTargetReached(bool historicalBlockServingLimit) const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     //! response the bytes left in the current max outbound cycle
     //! in case of no limit, it will always response 0
-    uint64_t GetOutboundTargetBytesLeft() const;
+    uint64_t GetOutboundTargetBytesLeft() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
-    //! returns the time left in the current max outbound cycle
-    //! in case of no limit, it will always return 0
-    std::chrono::seconds GetMaxOutboundTimeLeftInCycle() const;
+    std::chrono::seconds GetMaxOutboundTimeLeftInCycle() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     uint64_t GetTotalBytesRecv() const;
-    uint64_t GetTotalBytesSent() const;
+    uint64_t GetTotalBytesSent() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     /** Get a unique deterministic randomizer. */
     CSipHasher GetDeterministicRandomizer(uint64_t id) const;
@@ -942,6 +943,10 @@ class CConnman
         NetPermissionFlags m_permissions;
     };
 
+    //! returns the time left in the current max outbound cycle
+    //! in case of no limit, it will always return 0
+    std::chrono::seconds GetMaxOutboundTimeLeftInCycle_() const EXCLUSIVE_LOCKS_REQUIRED(m_total_bytes_sent_mutex);
+
     bool BindListenPort(const CService& bindAddr, bilingual_str& strError, NetPermissionFlags permissions);
     bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions);
     bool InitBinds(const Options& options);
@@ -1001,7 +1006,7 @@ class CConnman
     /**
      * Check connected and listening sockets for IO readiness and process them accordingly.
      */
-    void SocketHandler();
+    void SocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     /**
      * Do the read/write for connected sockets that are ready for IO.
@@ -1014,7 +1019,8 @@ class CConnman
     void SocketHandlerConnected(const std::vector<CNode*>& nodes,
                                 const std::set<SOCKET>& recv_set,
                                 const std::set<SOCKET>& send_set,
-                                const std::set<SOCKET>& error_set);
+                                const std::set<SOCKET>& error_set)
+        EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     /**
      * Accept incoming connections, one from each read-ready listening socket.
@@ -1022,7 +1028,7 @@ class CConnman
      */
     void SocketHandlerListening(const std::set<SOCKET>& recv_set);
 
-    void ThreadSocketHandler();
+    void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
     void ThreadDNSAddressSeed();
 
     uint64_t CalculateKeyedNetGroup(const CAddress& ad) const;
@@ -1051,7 +1057,7 @@ class CConnman
 
     // Network stats
     void RecordBytesRecv(uint64_t bytes);
-    void RecordBytesSent(uint64_t bytes);
+    void RecordBytesSent(uint64_t bytes) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
     /**
      * Return vector of current BLOCK_RELAY peers.

From 709af67add93f6674fb80e3ae8e3f175653a62f0 Mon Sep 17 00:00:00 2001
From: w0xlt <94266259+w0xlt@users.noreply.github.com>
Date: Tue, 25 Jan 2022 18:20:13 -0300
Subject: [PATCH 14/28] p2p: replace RecursiveMutex `m_total_bytes_sent_mutex`
 with Mutex

---
 src/net.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/net.h b/src/net.h
index 8cd83e00b506..f77edb253b6e 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1068,7 +1068,7 @@ class CConnman
     static bool NodeFullyConnected(const CNode* pnode);
 
     // Network usage totals
-    mutable RecursiveMutex m_total_bytes_sent_mutex;
+    mutable Mutex m_total_bytes_sent_mutex;
     std::atomic<uint64_t> nTotalBytesRecv{0};
     uint64_t nTotalBytesSent GUARDED_BY(m_total_bytes_sent_mutex) {0};
 

From dac44fc06ff9938d070aa5221d106a7f31da9d81 Mon Sep 17 00:00:00 2001
From: Martin Zumsande <mzumsande@gmail.com>
Date: Wed, 6 Apr 2022 15:28:46 +0200
Subject: [PATCH 15/28] init: disallow reindex-chainstate with optional indexes

It currently leads to corruption (coinstatsindex) or
data duplication (blockfilterindex), so disable it.
---
 src/init.cpp                              | 15 ++++++++++++++-
 test/functional/feature_coinstatsindex.py | 14 ++++++++++++++
 test/functional/p2p_blockfilters.py       |  6 ++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/init.cpp b/src/init.cpp
index 5ca5dd5401a0..b2403d6f85cb 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -424,7 +424,7 @@ void SetupServerArgs(ArgsManager& argsman)
             "Warning: Reverting this setting requires re-downloading the entire blockchain. "
             "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >=%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk. This will also rebuild active optional indexes.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead. Deactivate all optional indexes before running this.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-settings=<file>", strprintf("Specify path to dynamic settings data file. Can be disabled with -nosettings. File is written at runtime and not meant to be edited by users (use %s instead for custom settings). Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME, BITCOIN_SETTINGS_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 #if HAVE_SYSTEM
     argsman.AddArg("-startupnotify=<cmd>", "Execute command on startup.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -1031,6 +1031,19 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
         return InitError(_("No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>."));
     }
 
+    if (args.GetBoolArg("-reindex-chainstate", false)) {
+        // indexes that must be deactivated to prevent index corruption, see #24630
+        if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) {
+            return InitError(_("-reindex-chainstate option is not compatible with -coinstatsindex. Please temporarily disable coinstatsindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes."));
+        }
+        if (g_enabled_filter_types.count(BlockFilterType::BASIC)) {
+            return InitError(_("-reindex-chainstate option is not compatible with -blockfilterindex. Please temporarily disable blockfilterindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes."));
+        }
+        if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
+            return InitError(_("-reindex-chainstate option is not compatible with -txindex. Please temporarily disable txindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes."));
+        }
+    }
+
 #if defined(USE_SYSCALL_SANDBOX)
     if (args.IsArgSet("-sandbox") && !args.IsArgNegated("-sandbox")) {
         const std::string sandbox_arg{args.GetArg("-sandbox", "")};
diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py
index f8656618948d..251aa2114bc5 100755
--- a/test/functional/feature_coinstatsindex.py
+++ b/test/functional/feature_coinstatsindex.py
@@ -223,6 +223,20 @@ def _test_coin_stats_index(self):
         res10 = index_node.gettxoutsetinfo('muhash')
         assert(res8['txouts'] < res10['txouts'])
 
+        self.log.info("Test that the index works with -reindex")
+
+        self.restart_node(1, extra_args=["-coinstatsindex", "-reindex"])
+        res11 = index_node.gettxoutsetinfo('muhash')
+        assert_equal(res11, res10)
+
+        self.log.info("Test that -reindex-chainstate is disallowed with coinstatsindex")
+
+        self.nodes[1].assert_start_raises_init_error(
+            expected_msg='Error: -reindex-chainstate option is not compatible with -coinstatsindex. '
+            'Please temporarily disable coinstatsindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.',
+            extra_args=['-coinstatsindex', '-reindex-chainstate'],
+        )
+
     def _test_use_index_option(self):
         self.log.info("Test use_index option for nodes running the index")
 
diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py
index e45cef65bdb6..2ffc825acd00 100755
--- a/test/functional/p2p_blockfilters.py
+++ b/test/functional/p2p_blockfilters.py
@@ -250,6 +250,12 @@ def run_test(self):
         msg = "Error: Cannot set -peerblockfilters without -blockfilterindex."
         self.nodes[0].assert_start_raises_init_error(expected_msg=msg)
 
+        self.log.info("Test -blockfilterindex with -reindex-chainstate raises an error")
+        self.nodes[0].assert_start_raises_init_error(
+            expected_msg='Error: -reindex-chainstate option is not compatible with -blockfilterindex. '
+            'Please temporarily disable blockfilterindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.',
+            extra_args=['-blockfilterindex', '-reindex-chainstate'],
+        )
 
 def compute_last_header(prev_header, hashes):
     """Compute the last filter header from a starting header and a sequence of filter hashes."""

From b8f17fbcb4a98187b16dd4737f4751e0eee94c49 Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Mon, 7 Feb 2022 10:18:00 +0000
Subject: [PATCH 16/28] [tests] Move TxOrphange tests to orphange_tests.cpp

---
 src/Makefile.test.include          |   1 +
 src/test/denialofservice_tests.cpp | 118 -------------------------
 src/test/orphanage_tests.cpp       | 137 +++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 118 deletions(-)
 create mode 100644 src/test/orphanage_tests.cpp

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 96a9a748028e..4a35f40112e8 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -110,6 +110,7 @@ BITCOIN_TESTS =\
   test/net_peer_eviction_tests.cpp \
   test/net_tests.cpp \
   test/netbase_tests.cpp \
+  test/orphanage_tests.cpp \
   test/pmt_tests.cpp \
   test/policy_fee_tests.cpp \
   test/policyestimator_tests.cpp \
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 69fcd7342935..7a917649a8c9 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -15,7 +15,6 @@
 #include <serialize.h>
 #include <test/util/net.h>
 #include <test/util/setup_common.h>
-#include <txorphanage.h>
 #include <util/string.h>
 #include <util/system.h>
 #include <util/time.h>
@@ -429,121 +428,4 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     peerLogic->FinalizeNode(dummyNode);
 }
 
-class TxOrphanageTest : public TxOrphanage
-{
-public:
-    inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
-    {
-        return m_orphans.size();
-    }
-
-    CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
-    {
-        std::map<uint256, OrphanTx>::iterator it;
-        it = m_orphans.lower_bound(InsecureRand256());
-        if (it == m_orphans.end())
-            it = m_orphans.begin();
-        return it->second.tx;
-    }
-};
-
-static void MakeNewKeyWithFastRandomContext(CKey& key)
-{
-    std::vector<unsigned char> keydata;
-    keydata = g_insecure_rand_ctx.randbytes(32);
-    key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn=*/true);
-    assert(key.IsValid());
-}
-
-BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
-{
-    // This test had non-deterministic coverage due to
-    // randomly selected seeds.
-    // This seed is chosen so that all branches of the function
-    // ecdsa_signature_parse_der_lax are executed during this test.
-    // Specifically branches that run only when an ECDSA
-    // signature's R and S values have leading zeros.
-    g_insecure_rand_ctx = FastRandomContext{uint256{33}};
-
-    TxOrphanageTest orphanage;
-    CKey key;
-    MakeNewKeyWithFastRandomContext(key);
-    FillableSigningProvider keystore;
-    BOOST_CHECK(keystore.AddKey(key));
-
-    LOCK(g_cs_orphans);
-
-    // 50 orphan transactions:
-    for (int i = 0; i < 50; i++)
-    {
-        CMutableTransaction tx;
-        tx.vin.resize(1);
-        tx.vin[0].prevout.n = 0;
-        tx.vin[0].prevout.hash = InsecureRand256();
-        tx.vin[0].scriptSig << OP_1;
-        tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
-        tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
-
-        orphanage.AddTx(MakeTransactionRef(tx), i);
-    }
-
-    // ... and 50 that depend on other orphans:
-    for (int i = 0; i < 50; i++)
-    {
-        CTransactionRef txPrev = orphanage.RandomOrphan();
-
-        CMutableTransaction tx;
-        tx.vin.resize(1);
-        tx.vin[0].prevout.n = 0;
-        tx.vin[0].prevout.hash = txPrev->GetHash();
-        tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
-        tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
-        BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL));
-
-        orphanage.AddTx(MakeTransactionRef(tx), i);
-    }
-
-    // This really-big orphan should be ignored:
-    for (int i = 0; i < 10; i++)
-    {
-        CTransactionRef txPrev = orphanage.RandomOrphan();
-
-        CMutableTransaction tx;
-        tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
-        tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
-        tx.vin.resize(2777);
-        for (unsigned int j = 0; j < tx.vin.size(); j++)
-        {
-            tx.vin[j].prevout.n = j;
-            tx.vin[j].prevout.hash = txPrev->GetHash();
-        }
-        BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL));
-        // Re-use same signature for other inputs
-        // (they don't have to be valid for this test)
-        for (unsigned int j = 1; j < tx.vin.size(); j++)
-            tx.vin[j].scriptSig = tx.vin[0].scriptSig;
-
-        BOOST_CHECK(!orphanage.AddTx(MakeTransactionRef(tx), i));
-    }
-
-    // Test EraseOrphansFor:
-    for (NodeId i = 0; i < 3; i++)
-    {
-        size_t sizeBefore = orphanage.CountOrphans();
-        orphanage.EraseForPeer(i);
-        BOOST_CHECK(orphanage.CountOrphans() < sizeBefore);
-    }
-
-    // Test LimitOrphanTxSize() function:
-    orphanage.LimitOrphans(40);
-    BOOST_CHECK(orphanage.CountOrphans() <= 40);
-    orphanage.LimitOrphans(10);
-    BOOST_CHECK(orphanage.CountOrphans() <= 10);
-    orphanage.LimitOrphans(0);
-    BOOST_CHECK(orphanage.CountOrphans() == 0);
-}
-
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
new file mode 100644
index 000000000000..842daa8bd4d4
--- /dev/null
+++ b/src/test/orphanage_tests.cpp
@@ -0,0 +1,137 @@
+// Copyright (c) 2011-2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <arith_uint256.h>
+#include <pubkey.h>
+#include <script/sign.h>
+#include <script/signingprovider.h>
+#include <script/standard.h>
+#include <test/util/setup_common.h>
+#include <txorphanage.h>
+
+#include <array>
+#include <cstdint>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(orphanage_tests, TestingSetup)
+
+class TxOrphanageTest : public TxOrphanage
+{
+public:
+    inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
+    {
+        return m_orphans.size();
+    }
+
+    CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
+    {
+        std::map<uint256, OrphanTx>::iterator it;
+        it = m_orphans.lower_bound(InsecureRand256());
+        if (it == m_orphans.end())
+            it = m_orphans.begin();
+        return it->second.tx;
+    }
+};
+
+static void MakeNewKeyWithFastRandomContext(CKey& key)
+{
+    std::vector<unsigned char> keydata;
+    keydata = g_insecure_rand_ctx.randbytes(32);
+    key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn=*/true);
+    assert(key.IsValid());
+}
+
+BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
+{
+    // This test had non-deterministic coverage due to
+    // randomly selected seeds.
+    // This seed is chosen so that all branches of the function
+    // ecdsa_signature_parse_der_lax are executed during this test.
+    // Specifically branches that run only when an ECDSA
+    // signature's R and S values have leading zeros.
+    g_insecure_rand_ctx = FastRandomContext{uint256{33}};
+
+    TxOrphanageTest orphanage;
+    CKey key;
+    MakeNewKeyWithFastRandomContext(key);
+    FillableSigningProvider keystore;
+    BOOST_CHECK(keystore.AddKey(key));
+
+    LOCK(g_cs_orphans);
+
+    // 50 orphan transactions:
+    for (int i = 0; i < 50; i++)
+    {
+        CMutableTransaction tx;
+        tx.vin.resize(1);
+        tx.vin[0].prevout.n = 0;
+        tx.vin[0].prevout.hash = InsecureRand256();
+        tx.vin[0].scriptSig << OP_1;
+        tx.vout.resize(1);
+        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
+
+        orphanage.AddTx(MakeTransactionRef(tx), i);
+    }
+
+    // ... and 50 that depend on other orphans:
+    for (int i = 0; i < 50; i++)
+    {
+        CTransactionRef txPrev = orphanage.RandomOrphan();
+
+        CMutableTransaction tx;
+        tx.vin.resize(1);
+        tx.vin[0].prevout.n = 0;
+        tx.vin[0].prevout.hash = txPrev->GetHash();
+        tx.vout.resize(1);
+        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
+        BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL));
+
+        orphanage.AddTx(MakeTransactionRef(tx), i);
+    }
+
+    // This really-big orphan should be ignored:
+    for (int i = 0; i < 10; i++)
+    {
+        CTransactionRef txPrev = orphanage.RandomOrphan();
+
+        CMutableTransaction tx;
+        tx.vout.resize(1);
+        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
+        tx.vin.resize(2777);
+        for (unsigned int j = 0; j < tx.vin.size(); j++)
+        {
+            tx.vin[j].prevout.n = j;
+            tx.vin[j].prevout.hash = txPrev->GetHash();
+        }
+        BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL));
+        // Re-use same signature for other inputs
+        // (they don't have to be valid for this test)
+        for (unsigned int j = 1; j < tx.vin.size(); j++)
+            tx.vin[j].scriptSig = tx.vin[0].scriptSig;
+
+        BOOST_CHECK(!orphanage.AddTx(MakeTransactionRef(tx), i));
+    }
+
+    // Test EraseOrphansFor:
+    for (NodeId i = 0; i < 3; i++)
+    {
+        size_t sizeBefore = orphanage.CountOrphans();
+        orphanage.EraseForPeer(i);
+        BOOST_CHECK(orphanage.CountOrphans() < sizeBefore);
+    }
+
+    // Test LimitOrphanTxSize() function:
+    orphanage.LimitOrphans(40);
+    BOOST_CHECK(orphanage.CountOrphans() <= 40);
+    orphanage.LimitOrphans(10);
+    BOOST_CHECK(orphanage.CountOrphans() <= 10);
+    orphanage.LimitOrphans(0);
+    BOOST_CHECK(orphanage.CountOrphans() == 0);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

From fa870e3d4ccd6dfd0a9a8f2c608721a7251114e2 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Sun, 24 Apr 2022 16:44:35 +0200
Subject: [PATCH 17/28] Remove not needed clang-format off comments

Can be reviewed with --word-diff-regex=. --ignore-all-space
---
 src/rpc/blockchain.cpp      |  68 ++++++++----------
 src/rpc/external_signer.cpp |  12 ++--
 src/rpc/mempool.cpp         |   2 -
 src/rpc/mining.cpp          |  41 +++++------
 src/rpc/misc.cpp            |  42 +++++------
 src/rpc/net.cpp             |  43 +++++------
 src/rpc/rawtransaction.cpp  |  38 +++++-----
 src/rpc/server.cpp          |  14 ++--
 src/rpc/txoutproof.cpp      |   2 -
 src/wallet/rpc/wallet.cpp   | 138 +++++++++++++++++-------------------
 src/zmq/zmqrpc.cpp          |   6 +-
 11 files changed, 180 insertions(+), 226 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index d6a6bd5f3158..95f90f946a67 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2371,44 +2371,36 @@ UniValue CreateUTXOSnapshot(
     return result;
 }
 
-
-void RegisterBlockchainRPCCommands(CRPCTable &t)
-{
-// clang-format off
-static const CRPCCommand commands[] =
-{ //  category              actor (function)
-  //  --------------------- ------------------------
-    { "blockchain",         &getblockchaininfo,                  },
-    { "blockchain",         &getchaintxstats,                    },
-    { "blockchain",         &getblockstats,                      },
-    { "blockchain",         &getbestblockhash,                   },
-    { "blockchain",         &getblockcount,                      },
-    { "blockchain",         &getblock,                           },
-    { "blockchain",         &getblockfrompeer,                   },
-    { "blockchain",         &getblockhash,                       },
-    { "blockchain",         &getblockheader,                     },
-    { "blockchain",         &getchaintips,                       },
-    { "blockchain",         &getdifficulty,                      },
-    { "blockchain",         &getdeploymentinfo,                  },
-    { "blockchain",         &gettxout,                           },
-    { "blockchain",         &gettxoutsetinfo,                    },
-    { "blockchain",         &pruneblockchain,                    },
-    { "blockchain",         &verifychain,                        },
-
-    { "blockchain",         &preciousblock,                      },
-    { "blockchain",         &scantxoutset,                       },
-    { "blockchain",         &getblockfilter,                     },
-
-    /* Not shown in help */
-    { "hidden",              &invalidateblock,                   },
-    { "hidden",              &reconsiderblock,                   },
-    { "hidden",              &waitfornewblock,                   },
-    { "hidden",              &waitforblock,                      },
-    { "hidden",              &waitforblockheight,                },
-    { "hidden",              &syncwithvalidationinterfacequeue,  },
-    { "hidden",              &dumptxoutset,                      },
-};
-// clang-format on
+void RegisterBlockchainRPCCommands(CRPCTable& t)
+{
+    static const CRPCCommand commands[]{
+        {"blockchain", &getblockchaininfo},
+        {"blockchain", &getchaintxstats},
+        {"blockchain", &getblockstats},
+        {"blockchain", &getbestblockhash},
+        {"blockchain", &getblockcount},
+        {"blockchain", &getblock},
+        {"blockchain", &getblockfrompeer},
+        {"blockchain", &getblockhash},
+        {"blockchain", &getblockheader},
+        {"blockchain", &getchaintips},
+        {"blockchain", &getdifficulty},
+        {"blockchain", &getdeploymentinfo},
+        {"blockchain", &gettxout},
+        {"blockchain", &gettxoutsetinfo},
+        {"blockchain", &pruneblockchain},
+        {"blockchain", &verifychain},
+        {"blockchain", &preciousblock},
+        {"blockchain", &scantxoutset},
+        {"blockchain", &getblockfilter},
+        {"hidden", &invalidateblock},
+        {"hidden", &reconsiderblock},
+        {"hidden", &waitfornewblock},
+        {"hidden", &waitforblock},
+        {"hidden", &waitforblockheight},
+        {"hidden", &syncwithvalidationinterfacequeue},
+        {"hidden", &dumptxoutset},
+    };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);
     }
diff --git a/src/rpc/external_signer.cpp b/src/rpc/external_signer.cpp
index 82aa6f9516fa..4de7fc4205bc 100644
--- a/src/rpc/external_signer.cpp
+++ b/src/rpc/external_signer.cpp
@@ -62,15 +62,11 @@ static RPCHelpMan enumeratesigners()
     };
 }
 
-void RegisterSignerRPCCommands(CRPCTable &t)
+void RegisterSignerRPCCommands(CRPCTable& t)
 {
-// clang-format off
-static const CRPCCommand commands[] =
-{ // category              actor (function)
-  // --------------------- ------------------------
-  { "signer",              &enumeratesigners,      },
-};
-// clang-format on
+    static const CRPCCommand commands[]{
+        {"signer", &enumeratesigners},
+    };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);
     }
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 1caf4ad96c3f..27080d3881ca 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -672,8 +672,6 @@ static RPCHelpMan savemempool()
 void RegisterMempoolRPCCommands(CRPCTable& t)
 {
     static const CRPCCommand commands[]{
-        // category     actor (function)
-        // --------     ----------------
         {"rawtransactions", &sendrawtransaction},
         {"rawtransactions", &testmempoolaccept},
         {"blockchain", &getmempoolancestors},
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 211026c8d972..e4b38efe0720 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -1249,30 +1249,25 @@ static RPCHelpMan estimaterawfee()
     };
 }
 
-void RegisterMiningRPCCommands(CRPCTable &t)
+void RegisterMiningRPCCommands(CRPCTable& t)
 {
-// clang-format off
-static const CRPCCommand commands[] =
-{ //  category               actor (function)
-  //  ---------------------  -----------------------
-    { "mining",              &getnetworkhashps,        },
-    { "mining",              &getmininginfo,           },
-    { "mining",              &prioritisetransaction,   },
-    { "mining",              &getblocktemplate,        },
-    { "mining",              &submitblock,             },
-    { "mining",              &submitheader,            },
-
-
-    { "hidden",              &generatetoaddress,       },
-    { "hidden",              &generatetodescriptor,    },
-    { "hidden",              &generateblock,           },
-
-    { "util",                &estimatesmartfee,        },
-
-    { "hidden",              &estimaterawfee,          },
-    { "hidden",              &generate,                },
-};
-// clang-format on
+    static const CRPCCommand commands[]{
+        {"mining", &getnetworkhashps},
+        {"mining", &getmininginfo},
+        {"mining", &prioritisetransaction},
+        {"mining", &getblocktemplate},
+        {"mining", &submitblock},
+        {"mining", &submitheader},
+
+        {"hidden", &generatetoaddress},
+        {"hidden", &generatetodescriptor},
+        {"hidden", &generateblock},
+
+        {"util", &estimatesmartfee},
+
+        {"hidden", &estimaterawfee},
+        {"hidden", &generate},
+    };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);
     }
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index d4bdf96affad..98d751e69d8c 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -790,33 +790,27 @@ static RPCHelpMan getindexinfo()
     };
 }
 
-void RegisterMiscRPCCommands(CRPCTable &t)
+void RegisterMiscRPCCommands(CRPCTable& t)
 {
-// clang-format off
-static const CRPCCommand commands[] =
-{ //  category              actor (function)
-  //  --------------------- ------------------------
-    { "control",            &getmemoryinfo,           },
-    { "control",            &logging,                 },
-    { "util",               &validateaddress,         },
-    { "util",               &createmultisig,          },
-    { "util",               &deriveaddresses,         },
-    { "util",               &getdescriptorinfo,       },
-    { "util",               &verifymessage,           },
-    { "util",               &signmessagewithprivkey,  },
-    { "util",               &getindexinfo,            },
-
-    /* Not shown in help */
-    { "hidden",             &setmocktime,             },
-    { "hidden",             &mockscheduler,           },
-    { "hidden",             &echo,                    },
-    { "hidden",             &echojson,                },
-    { "hidden",             &echoipc,                 },
+    static const CRPCCommand commands[]{
+        {"control", &getmemoryinfo},
+        {"control", &logging},
+        {"util", &validateaddress},
+        {"util", &createmultisig},
+        {"util", &deriveaddresses},
+        {"util", &getdescriptorinfo},
+        {"util", &verifymessage},
+        {"util", &signmessagewithprivkey},
+        {"util", &getindexinfo},
+        {"hidden", &setmocktime},
+        {"hidden", &mockscheduler},
+        {"hidden", &echo},
+        {"hidden", &echojson},
+        {"hidden", &echoipc},
 #if defined(USE_SYSCALL_SANDBOX)
-    { "hidden",             &invokedisallowedsyscall, },
+        {"hidden", &invokedisallowedsyscall},
 #endif // USE_SYSCALL_SANDBOX
-};
-// clang-format on
+    };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);
     }
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 225feabf144f..a9f9cb0153e2 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -959,30 +959,25 @@ static RPCHelpMan addpeeraddress()
     };
 }
 
-void RegisterNetRPCCommands(CRPCTable &t)
-{
-// clang-format off
-static const CRPCCommand commands[] =
-{ //  category              actor
-  //  --------------------- -----------------------
-    { "network",             &getconnectioncount,      },
-    { "network",             &ping,                    },
-    { "network",             &getpeerinfo,             },
-    { "network",             &addnode,                 },
-    { "network",             &disconnectnode,          },
-    { "network",             &getaddednodeinfo,        },
-    { "network",             &getnettotals,            },
-    { "network",             &getnetworkinfo,          },
-    { "network",             &setban,                  },
-    { "network",             &listbanned,              },
-    { "network",             &clearbanned,             },
-    { "network",             &setnetworkactive,        },
-    { "network",             &getnodeaddresses,        },
-
-    { "hidden",              &addconnection,           },
-    { "hidden",              &addpeeraddress,          },
-};
-// clang-format on
+void RegisterNetRPCCommands(CRPCTable& t)
+{
+    static const CRPCCommand commands[]{
+        {"network", &getconnectioncount},
+        {"network", &ping},
+        {"network", &getpeerinfo},
+        {"network", &addnode},
+        {"network", &disconnectnode},
+        {"network", &getaddednodeinfo},
+        {"network", &getnettotals},
+        {"network", &getnetworkinfo},
+        {"network", &setban},
+        {"network", &listbanned},
+        {"network", &clearbanned},
+        {"network", &setnetworkactive},
+        {"network", &getnodeaddresses},
+        {"hidden", &addconnection},
+        {"hidden", &addpeeraddress},
+    };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);
     }
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 806a65835539..ce8f16540f59 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1680,28 +1680,24 @@ static RPCHelpMan analyzepsbt()
     };
 }
 
-void RegisterRawTransactionRPCCommands(CRPCTable &t)
+void RegisterRawTransactionRPCCommands(CRPCTable& t)
 {
-// clang-format off
-static const CRPCCommand commands[] =
-{ //  category               actor (function)
-  //  ---------------------  -----------------------
-    { "rawtransactions",     &getrawtransaction,          },
-    { "rawtransactions",     &createrawtransaction,       },
-    { "rawtransactions",     &decoderawtransaction,       },
-    { "rawtransactions",     &decodescript,               },
-    { "rawtransactions",     &combinerawtransaction,      },
-    { "rawtransactions",     &signrawtransactionwithkey,  },
-    { "rawtransactions",     &decodepsbt,                 },
-    { "rawtransactions",     &combinepsbt,                },
-    { "rawtransactions",     &finalizepsbt,               },
-    { "rawtransactions",     &createpsbt,                 },
-    { "rawtransactions",     &converttopsbt,              },
-    { "rawtransactions",     &utxoupdatepsbt,             },
-    { "rawtransactions",     &joinpsbts,                  },
-    { "rawtransactions",     &analyzepsbt,                },
-};
-// clang-format on
+    static const CRPCCommand commands[]{
+        {"rawtransactions", &getrawtransaction},
+        {"rawtransactions", &createrawtransaction},
+        {"rawtransactions", &decoderawtransaction},
+        {"rawtransactions", &decodescript},
+        {"rawtransactions", &combinerawtransaction},
+        {"rawtransactions", &signrawtransactionwithkey},
+        {"rawtransactions", &decodepsbt},
+        {"rawtransactions", &combinepsbt},
+        {"rawtransactions", &finalizepsbt},
+        {"rawtransactions", &createpsbt},
+        {"rawtransactions", &converttopsbt},
+        {"rawtransactions", &utxoupdatepsbt},
+        {"rawtransactions", &joinpsbts},
+        {"rawtransactions", &analyzepsbt},
+    };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);
     }
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index 333ed6f5da15..95728c41a5fd 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -248,17 +248,13 @@ static RPCHelpMan getrpcinfo()
     };
 }
 
-// clang-format off
-static const CRPCCommand vRPCCommands[] =
-{ //  category               actor (function)
-  //  ---------------------  -----------------------
+static const CRPCCommand vRPCCommands[]{
     /* Overall control/query calls */
-    { "control",             &getrpcinfo,             },
-    { "control",             &help,                   },
-    { "control",             &stop,                   },
-    { "control",             &uptime,                 },
+    {"control", &getrpcinfo},
+    {"control", &help},
+    {"control", &stop},
+    {"control", &uptime},
 };
-// clang-format on
 
 CRPCTable::CRPCTable()
 {
diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp
index a5443b03290f..d16820baeb58 100644
--- a/src/rpc/txoutproof.cpp
+++ b/src/rpc/txoutproof.cpp
@@ -172,8 +172,6 @@ static RPCHelpMan verifytxoutproof()
 void RegisterTxoutProofRPCCommands(CRPCTable& t)
 {
     static const CRPCCommand commands[]{
-        // category     actor (function)
-        // --------     ----------------
         {"blockchain", &gettxoutproof},
         {"blockchain", &verifytxoutproof},
     };
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index 129166384718..efb4d8fc7eb7 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -664,79 +664,75 @@ RPCHelpMan abortrescan();
 
 Span<const CRPCCommand> GetWalletRPCCommands()
 {
-// clang-format off
-static const CRPCCommand commands[] =
-{ //  category              actor (function)
-  //  ------------------    ------------------------
-    { "rawtransactions",    &fundrawtransaction,             },
-    { "wallet",             &abandontransaction,             },
-    { "wallet",             &abortrescan,                    },
-    { "wallet",             &addmultisigaddress,             },
-    { "wallet",             &backupwallet,                   },
-    { "wallet",             &bumpfee,                        },
-    { "wallet",             &psbtbumpfee,                    },
-    { "wallet",             &createwallet,                   },
-    { "wallet",             &restorewallet,                  },
-    { "wallet",             &dumpprivkey,                    },
-    { "wallet",             &dumpwallet,                     },
-    { "wallet",             &encryptwallet,                  },
-    { "wallet",             &getaddressesbylabel,            },
-    { "wallet",             &getaddressinfo,                 },
-    { "wallet",             &getbalance,                     },
-    { "wallet",             &getnewaddress,                  },
-    { "wallet",             &getrawchangeaddress,            },
-    { "wallet",             &getreceivedbyaddress,           },
-    { "wallet",             &getreceivedbylabel,             },
-    { "wallet",             &gettransaction,                 },
-    { "wallet",             &getunconfirmedbalance,          },
-    { "wallet",             &getbalances,                    },
-    { "wallet",             &getwalletinfo,                  },
-    { "wallet",             &importaddress,                  },
-    { "wallet",             &importdescriptors,              },
-    { "wallet",             &importmulti,                    },
-    { "wallet",             &importprivkey,                  },
-    { "wallet",             &importprunedfunds,              },
-    { "wallet",             &importpubkey,                   },
-    { "wallet",             &importwallet,                   },
-    { "wallet",             &keypoolrefill,                  },
-    { "wallet",             &listaddressgroupings,           },
-    { "wallet",             &listdescriptors,                },
-    { "wallet",             &listlabels,                     },
-    { "wallet",             &listlockunspent,                },
-    { "wallet",             &listreceivedbyaddress,          },
-    { "wallet",             &listreceivedbylabel,            },
-    { "wallet",             &listsinceblock,                 },
-    { "wallet",             &listtransactions,               },
-    { "wallet",             &listunspent,                    },
-    { "wallet",             &listwalletdir,                  },
-    { "wallet",             &listwallets,                    },
-    { "wallet",             &loadwallet,                     },
-    { "wallet",             &lockunspent,                    },
-    { "wallet",             &newkeypool,                     },
-    { "wallet",             &removeprunedfunds,              },
-    { "wallet",             &rescanblockchain,               },
-    { "wallet",             &send,                           },
-    { "wallet",             &sendmany,                       },
-    { "wallet",             &sendtoaddress,                  },
-    { "wallet",             &sethdseed,                      },
-    { "wallet",             &setlabel,                       },
-    { "wallet",             &settxfee,                       },
-    { "wallet",             &setwalletflag,                  },
-    { "wallet",             &signmessage,                    },
-    { "wallet",             &signrawtransactionwithwallet,   },
-    { "wallet",             &sendall,                        },
-    { "wallet",             &unloadwallet,                   },
-    { "wallet",             &upgradewallet,                  },
-    { "wallet",             &walletcreatefundedpsbt,         },
+    static const CRPCCommand commands[]{
+        {"rawtransactions", &fundrawtransaction},
+        {"wallet", &abandontransaction},
+        {"wallet", &abortrescan},
+        {"wallet", &addmultisigaddress},
+        {"wallet", &backupwallet},
+        {"wallet", &bumpfee},
+        {"wallet", &psbtbumpfee},
+        {"wallet", &createwallet},
+        {"wallet", &restorewallet},
+        {"wallet", &dumpprivkey},
+        {"wallet", &dumpwallet},
+        {"wallet", &encryptwallet},
+        {"wallet", &getaddressesbylabel},
+        {"wallet", &getaddressinfo},
+        {"wallet", &getbalance},
+        {"wallet", &getnewaddress},
+        {"wallet", &getrawchangeaddress},
+        {"wallet", &getreceivedbyaddress},
+        {"wallet", &getreceivedbylabel},
+        {"wallet", &gettransaction},
+        {"wallet", &getunconfirmedbalance},
+        {"wallet", &getbalances},
+        {"wallet", &getwalletinfo},
+        {"wallet", &importaddress},
+        {"wallet", &importdescriptors},
+        {"wallet", &importmulti},
+        {"wallet", &importprivkey},
+        {"wallet", &importprunedfunds},
+        {"wallet", &importpubkey},
+        {"wallet", &importwallet},
+        {"wallet", &keypoolrefill},
+        {"wallet", &listaddressgroupings},
+        {"wallet", &listdescriptors},
+        {"wallet", &listlabels},
+        {"wallet", &listlockunspent},
+        {"wallet", &listreceivedbyaddress},
+        {"wallet", &listreceivedbylabel},
+        {"wallet", &listsinceblock},
+        {"wallet", &listtransactions},
+        {"wallet", &listunspent},
+        {"wallet", &listwalletdir},
+        {"wallet", &listwallets},
+        {"wallet", &loadwallet},
+        {"wallet", &lockunspent},
+        {"wallet", &newkeypool},
+        {"wallet", &removeprunedfunds},
+        {"wallet", &rescanblockchain},
+        {"wallet", &send},
+        {"wallet", &sendmany},
+        {"wallet", &sendtoaddress},
+        {"wallet", &sethdseed},
+        {"wallet", &setlabel},
+        {"wallet", &settxfee},
+        {"wallet", &setwalletflag},
+        {"wallet", &signmessage},
+        {"wallet", &signrawtransactionwithwallet},
+        {"wallet", &sendall},
+        {"wallet", &unloadwallet},
+        {"wallet", &upgradewallet},
+        {"wallet", &walletcreatefundedpsbt},
 #ifdef ENABLE_EXTERNAL_SIGNER
-    { "wallet",             &walletdisplayaddress,           },
+        {"wallet", &walletdisplayaddress},
 #endif // ENABLE_EXTERNAL_SIGNER
-    { "wallet",             &walletlock,                     },
-    { "wallet",             &walletpassphrase,               },
-    { "wallet",             &walletpassphrasechange,         },
-    { "wallet",             &walletprocesspsbt,              },
-};
-// clang-format on
+        {"wallet", &walletlock},
+        {"wallet", &walletpassphrase},
+        {"wallet", &walletpassphrasechange},
+        {"wallet", &walletprocesspsbt},
+    };
     return commands;
 }
 } // namespace wallet
diff --git a/src/zmq/zmqrpc.cpp b/src/zmq/zmqrpc.cpp
index f9f8b5a9dc01..ec6d1cbba34b 100644
--- a/src/zmq/zmqrpc.cpp
+++ b/src/zmq/zmqrpc.cpp
@@ -51,10 +51,8 @@ static RPCHelpMan getzmqnotifications()
     };
 }
 
-const CRPCCommand commands[] =
-{ //  category           actor (function)
-  //  -----------------  -----------------------
-    { "zmq",             &getzmqnotifications,    },
+const CRPCCommand commands[]{
+    {"zmq", &getzmqnotifications},
 };
 
 } // anonymous namespace

From 4637bbe448ae7370528f40092ce230c32602a6d6 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Mon, 25 Apr 2022 09:43:43 -0400
Subject: [PATCH 18/28] rpc: Explain active and internal in listdescriptors

The current help text for active and internal in listdescriptors is not
particularly helpful. They require the reader to already know what those
terms mean. This help text is updated to actually explain the
definitions of those words in context of a descriptor wallet.
---
 src/wallet/rpc/backup.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 74810728159e..34d3d17d1b62 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -1748,13 +1748,13 @@ RPCHelpMan listdescriptors()
                 {RPCResult::Type::OBJ, "", "", {
                     {RPCResult::Type::STR, "desc", "Descriptor string representation"},
                     {RPCResult::Type::NUM, "timestamp", "The creation time of the descriptor"},
-                    {RPCResult::Type::BOOL, "active", "Activeness flag"},
-                    {RPCResult::Type::BOOL, "internal", true, "Whether this is an internal or external descriptor; defined only for active descriptors"},
-                    {RPCResult::Type::ARR_FIXED, "range", true, "Defined only for ranged descriptors", {
+                    {RPCResult::Type::BOOL, "active", "Whether this descriptor is currently used to generate new addresses"},
+                    {RPCResult::Type::BOOL, "internal", /*optional=*/true, "True if this descriptor is used to generate change addresses. False if this descriptor is used to generate receiving addresses; defined only for active descriptors"},
+                    {RPCResult::Type::ARR_FIXED, "range", /*optional=*/true, "Defined only for ranged descriptors", {
                         {RPCResult::Type::NUM, "", "Range start inclusive"},
                         {RPCResult::Type::NUM, "", "Range end inclusive"},
                     }},
-                    {RPCResult::Type::NUM, "next", true, "The next index to generate addresses from; defined only for ranged descriptors"},
+                    {RPCResult::Type::NUM, "next", /*optional=*/true, "The next index to generate addresses from; defined only for ranged descriptors"},
                 }},
             }}
         }},

From df08c23f0164b3e4b0df7d43462924f90a245237 Mon Sep 17 00:00:00 2001
From: Gregory Sanders <gsanders87@gmail.com>
Date: Mon, 25 Apr 2022 11:58:42 -0400
Subject: [PATCH 19/28] Precomputed hashes are note #16 in BIP341

---
 src/script/interpreter.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/script/interpreter.h b/src/script/interpreter.h
index fbd904fb7b63..adf454aa1538 100644
--- a/src/script/interpreter.h
+++ b/src/script/interpreter.h
@@ -151,7 +151,7 @@ bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned i
 struct PrecomputedTransactionData
 {
     // BIP341 precomputed data.
-    // These are single-SHA256, see https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#cite_note-15.
+    // These are single-SHA256, see https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#cite_note-16.
     uint256 m_prevouts_single_hash;
     uint256 m_sequences_single_hash;
     uint256 m_outputs_single_hash;

From 231fc7b035481f748159968353c1cab81354e843 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sun, 18 Apr 2021 16:48:52 +0200
Subject: [PATCH 20/28] refactor: Introduce GetFirstStoredBlock helper function

---
 src/index/base.cpp        |  6 +-----
 src/node/blockstorage.cpp | 10 ++++++++++
 src/node/blockstorage.h   |  3 +++
 src/rpc/blockchain.cpp    | 13 ++++---------
 4 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/src/index/base.cpp b/src/index/base.cpp
index 8fe30f896010..9f60d331c620 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -75,11 +75,7 @@ bool BaseIndex::Init()
         if (!m_best_block_index) {
             // index is not built yet
             // make sure we have all block data back to the genesis
-            const CBlockIndex* block = active_chain.Tip();
-            while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
-                block = block->pprev;
-            }
-            prune_violation = block != active_chain.Genesis();
+            prune_violation = node::GetFirstStoredBlock(active_chain.Tip()) != active_chain.Genesis();
         }
         // in case the index has a best block set and is not fully synced
         // check if we have the required blocks to continue building the index
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 21cb0250d81c..c3f42fde2db1 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -397,6 +397,16 @@ bool BlockManager::IsBlockPruned(const CBlockIndex* pblockindex)
     return (m_have_pruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0);
 }
 
+const CBlockIndex* GetFirstStoredBlock(const CBlockIndex* start_block) {
+    AssertLockHeld(::cs_main);
+    assert(start_block);
+    const CBlockIndex* last_block = start_block;
+    while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) {
+        last_block = last_block->pprev;
+    }
+    return last_block;
+}
+
 // If we're using -prune with -reindex, then delete block files that will be ignored by the
 // reindex.  Since reindexing works by starting at block file 0 and looping until a blockfile
 // is missing, do the same here to delete any later block files after a gap.  Also delete all
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 11445aa22e23..229c099f1ffc 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -181,6 +181,9 @@ class BlockManager
     }
 };
 
+//! Find the first block that is not pruned
+const CBlockIndex* GetFirstStoredBlock(const CBlockIndex* start_block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
 void CleanupBlockRevFiles();
 
 /** Open a block file (blk?????.dat) */
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index d6a6bd5f3158..f1ad143e0fa0 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -787,10 +787,9 @@ static RPCHelpMan pruneblockchain()
 
     PruneBlockFilesManual(active_chainstate, height);
     const CBlockIndex* block = CHECK_NONFATAL(active_chain.Tip());
-    while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
-        block = block->pprev;
-    }
-    return uint64_t(block->nHeight);
+    const CBlockIndex* last_block = node::GetFirstStoredBlock(block);
+
+    return static_cast<uint64_t>(last_block->nHeight);
 },
     };
 }
@@ -1217,11 +1216,7 @@ RPCHelpMan getblockchaininfo()
     obj.pushKV("pruned", node::fPruneMode);
     if (node::fPruneMode) {
         const CBlockIndex* block = CHECK_NONFATAL(tip);
-        while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
-            block = block->pprev;
-        }
-
-        obj.pushKV("pruneheight",        block->nHeight);
+        obj.pushKV("pruneheight", node::GetFirstStoredBlock(block)->nHeight);
 
         // if 0, execution bypasses the whole if block.
         bool automatic_pruning{args.GetIntArg("-prune", 0) != 1};

From 2561823531c25e1510c107eb41de944b00444ce0 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Thu, 13 May 2021 19:13:08 +0200
Subject: [PATCH 21/28] blockstorage: Add prune locks to BlockManager

This change also introduces an aditional buffer of 10 blocks (PRUNE_LOCK_BUFFER) that will not be pruned before the best block.

Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 src/node/blockstorage.cpp |  7 +++++++
 src/node/blockstorage.h   | 16 ++++++++++++++++
 src/validation.cpp        | 40 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index c3f42fde2db1..8ed22bbbce8b 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -21,6 +21,8 @@
 #include <util/system.h>
 #include <validation.h>
 
+#include <unordered_map>
+
 namespace node {
 std::atomic_bool fImporting(false);
 std::atomic_bool fReindex(false);
@@ -230,6 +232,11 @@ void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPr
            nLastBlockWeCanPrune, count);
 }
 
+void BlockManager::UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) {
+    AssertLockHeld(::cs_main);
+    m_prune_locks[name] = lock_info;
+}
+
 CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash)
 {
     AssertLockHeld(cs_main);
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 229c099f1ffc..b33941dfa609 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -13,6 +13,7 @@
 
 #include <atomic>
 #include <cstdint>
+#include <unordered_map>
 #include <vector>
 
 extern RecursiveMutex cs_main;
@@ -65,6 +66,10 @@ struct CBlockIndexHeightOnlyComparator {
     bool operator()(const CBlockIndex* pa, const CBlockIndex* pb) const;
 };
 
+struct PruneLockInfo {
+    int height_first{std::numeric_limits<int>::max()}; //! Height of earliest block that should be kept and not pruned
+};
+
 /**
  * Maintains a tree of blocks (stored in `m_block_index`) which is consulted
  * to determine where the most-work tip is.
@@ -118,6 +123,14 @@ class BlockManager
     /** Dirty block file entries. */
     std::set<int> m_dirty_fileinfo;
 
+    /**
+     * Map from external index name to oldest block that must not be pruned.
+     *
+     * @note Internally, only blocks at height (height_first - PRUNE_LOCK_BUFFER - 1) and
+     * below will be pruned, but callers should avoid assuming any particular buffer size.
+     */
+    std::unordered_map<std::string, PruneLockInfo> m_prune_locks GUARDED_BY(::cs_main);
+
 public:
     BlockMap m_block_index GUARDED_BY(cs_main);
 
@@ -175,6 +188,9 @@ class BlockManager
     //! Check whether the block associated with this index entry is pruned or not.
     bool IsBlockPruned(const CBlockIndex* pblockindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    //! Create or update a prune lock identified by its name
+    void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
     ~BlockManager()
     {
         Unload();
diff --git a/src/validation.cpp b/src/validation.cpp
index 58686632f97e..c0c541a3966f 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -106,6 +106,12 @@ const std::vector<std::string> CHECKLEVEL_DOC {
     "level 4 tries to reconnect the blocks",
     "each level includes the checks of the previous levels",
 };
+/** The number of blocks to keep below the deepest prune lock.
+ *  There is nothing special about this number. It is higher than what we
+ *  expect to see in regular mainnet reorgs, but not so high that it would
+ *  noticeably interfere with the pruning mechanism.
+ * */
+static constexpr int PRUNE_LOCK_BUFFER{10};
 
 /**
  * Mutex to guard access to validation specific variables, such as reading
@@ -2338,13 +2344,29 @@ bool CChainState::FlushStateToDisk(
         CoinsCacheSizeState cache_state = GetCoinsCacheSizeState();
         LOCK(m_blockman.cs_LastBlockFile);
         if (fPruneMode && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && !fReindex) {
-            // make sure we don't prune above the blockfilterindexes bestblocks
+            // make sure we don't prune above any of the prune locks bestblocks
             // pruning is height-based
-            int last_prune = m_chain.Height(); // last height we can prune
+            int last_prune{m_chain.Height()}; // last height we can prune
+            std::optional<std::string> limiting_lock; // prune lock that actually was the limiting factor, only used for logging
+
             ForEachBlockFilterIndex([&](BlockFilterIndex& index) {
-               last_prune = std::max(1, std::min(last_prune, index.GetSummary().best_block_height));
+                last_prune = std::max(1, std::min(last_prune, index.GetSummary().best_block_height));
             });
 
+            for (const auto& prune_lock : m_blockman.m_prune_locks) {
+                if (prune_lock.second.height_first == std::numeric_limits<int>::max()) continue;
+                // Remove the buffer and one additional block here to get actual height that is outside of the buffer
+                const int lock_height{prune_lock.second.height_first - PRUNE_LOCK_BUFFER - 1};
+                last_prune = std::max(1, std::min(last_prune, lock_height));
+                if (last_prune == lock_height) {
+                    limiting_lock = prune_lock.first;
+                }
+            }
+
+            if (limiting_lock) {
+                LogPrint(BCLog::PRUNE, "%s limited pruning to height %d\n", limiting_lock.value(), last_prune);
+            }
+
             if (nManualPruneHeight > 0) {
                 LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCH);
 
@@ -2581,6 +2603,18 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr
         assert(flushed);
     }
     LogPrint(BCLog::BENCH, "- Disconnect block: %.2fms\n", (GetTimeMicros() - nStart) * MILLI);
+
+    {
+        // Prune locks that began at or after the tip should be moved backward so they get a chance to reorg
+        const int max_height_first{pindexDelete->nHeight - 1};
+        for (auto& prune_lock : m_blockman.m_prune_locks) {
+            if (prune_lock.second.height_first <= max_height_first) continue;
+
+            prune_lock.second.height_first = max_height_first;
+            LogPrint(BCLog::PRUNE, "%s prune lock moved back to %d\n", prune_lock.first, max_height_first);
+        }
+    }
+
     // Write the chain state to disk, if necessary.
     if (!FlushStateToDisk(state, FlushStateMode::IF_NEEDED)) {
         return false;

From f08c9fb0c6a799e3cb75ca5f763a746471625beb Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sun, 18 Apr 2021 23:06:18 +0200
Subject: [PATCH 22/28] Index: Use prune locks for blockfilterindex

Prior to this change blocks could be pruned up to the last block before the blockfilterindex current best block.
---
 src/index/base.cpp                      | 27 +++++++++++++++++--------
 src/index/base.h                        |  6 ++++++
 src/index/blockfilterindex.h            |  2 ++
 src/index/coinstatsindex.h              |  2 ++
 src/index/txindex.h                     |  2 ++
 src/validation.cpp                      |  5 -----
 test/lint/lint-circular-dependencies.py |  2 --
 7 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/src/index/base.cpp b/src/index/base.cpp
index 9f60d331c620..488a214ccf1e 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -65,9 +65,9 @@ bool BaseIndex::Init()
     LOCK(cs_main);
     CChain& active_chain = m_chainstate->m_chain;
     if (locator.IsNull()) {
-        m_best_block_index = nullptr;
+        SetBestBlockIndex(nullptr);
     } else {
-        m_best_block_index = m_chainstate->FindForkInGlobalIndex(locator);
+        SetBestBlockIndex(m_chainstate->FindForkInGlobalIndex(locator));
     }
     m_synced = m_best_block_index.load() == active_chain.Tip();
     if (!m_synced) {
@@ -134,7 +134,7 @@ void BaseIndex::ThreadSync()
         int64_t last_locator_write_time = 0;
         while (true) {
             if (m_interrupt) {
-                m_best_block_index = pindex;
+                SetBestBlockIndex(pindex);
                 // No need to handle errors in Commit. If it fails, the error will be already be
                 // logged. The best way to recover is to continue, as index cannot be corrupted by
                 // a missed commit to disk for an advanced index state.
@@ -146,7 +146,7 @@ void BaseIndex::ThreadSync()
                 LOCK(cs_main);
                 const CBlockIndex* pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain);
                 if (!pindex_next) {
-                    m_best_block_index = pindex;
+                    SetBestBlockIndex(pindex);
                     m_synced = true;
                     // No need to handle errors in Commit. See rationale above.
                     Commit();
@@ -168,7 +168,7 @@ void BaseIndex::ThreadSync()
             }
 
             if (last_locator_write_time + SYNC_LOCATOR_WRITE_INTERVAL < current_time) {
-                m_best_block_index = pindex;
+                SetBestBlockIndex(pindex);
                 last_locator_write_time = current_time;
                 // No need to handle errors in Commit. See rationale above.
                 Commit();
@@ -226,10 +226,10 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti
     // out of sync may be possible but a users fault.
     // In case we reorg beyond the pruned depth, ReadBlockFromDisk would
     // throw and lead to a graceful shutdown
-    m_best_block_index = new_tip;
+    SetBestBlockIndex(new_tip);
     if (!Commit()) {
         // If commit fails, revert the best block index to avoid corruption.
-        m_best_block_index = current_tip;
+        SetBestBlockIndex(current_tip);
         return false;
     }
 
@@ -270,7 +270,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const
     }
 
     if (WriteBlock(*block, pindex)) {
-        m_best_block_index = pindex;
+        SetBestBlockIndex(pindex);
     } else {
         FatalError("%s: Failed to write block %s to index",
                    __func__, pindex->GetBlockHash().ToString());
@@ -377,3 +377,14 @@ IndexSummary BaseIndex::GetSummary() const
     summary.best_block_height = m_best_block_index ? m_best_block_index.load()->nHeight : 0;
     return summary;
 }
+
+void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
+    assert(!node::fPruneMode || AllowPrune());
+
+    m_best_block_index = block;
+    if (AllowPrune() && block) {
+        node::PruneLockInfo prune_lock;
+        prune_lock.height_first = block->nHeight;
+        WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock));
+    }
+}
diff --git a/src/index/base.h b/src/index/base.h
index c4a8215bc4c0..a8f6a18c8de5 100644
--- a/src/index/base.h
+++ b/src/index/base.h
@@ -75,6 +75,9 @@ class BaseIndex : public CValidationInterface
     /// to a chain reorganization), the index must halt until Commit succeeds or else it could end up
     /// getting corrupted.
     bool Commit();
+
+    virtual bool AllowPrune() const = 0;
+
 protected:
     CChainState* m_chainstate{nullptr};
 
@@ -103,6 +106,9 @@ class BaseIndex : public CValidationInterface
     /// Get the name of the index for display in logs.
     virtual const char* GetName() const = 0;
 
+    /// Update the internal best block index as well as the prune lock.
+    void SetBestBlockIndex(const CBlockIndex* block);
+
 public:
     /// Destructor interrupts sync thread if running and blocks until it exits.
     virtual ~BaseIndex();
diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h
index a049019c020f..b1836fe12f0e 100644
--- a/src/index/blockfilterindex.h
+++ b/src/index/blockfilterindex.h
@@ -38,6 +38,8 @@ class BlockFilterIndex final : public BaseIndex
     /** cache of block hash to filter header, to avoid disk access when responding to getcfcheckpt. */
     std::unordered_map<uint256, uint256, FilterHeaderHasher> m_headers_cache GUARDED_BY(m_cs_headers_cache);
 
+    bool AllowPrune() const override { return true; }
+
 protected:
     bool Init() override;
 
diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h
index 24190ac13768..6f53bb74fbee 100644
--- a/src/index/coinstatsindex.h
+++ b/src/index/coinstatsindex.h
@@ -36,6 +36,8 @@ class CoinStatsIndex final : public BaseIndex
 
     bool ReverseBlock(const CBlock& block, const CBlockIndex* pindex);
 
+    bool AllowPrune() const override { return true; }
+
 protected:
     bool Init() override;
 
diff --git a/src/index/txindex.h b/src/index/txindex.h
index 2bbc60263137..ec339abaa187 100644
--- a/src/index/txindex.h
+++ b/src/index/txindex.h
@@ -20,6 +20,8 @@ class TxIndex final : public BaseIndex
 private:
     const std::unique_ptr<DB> m_db;
 
+    bool AllowPrune() const override { return false; }
+
 protected:
     bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
 
diff --git a/src/validation.cpp b/src/validation.cpp
index c0c541a3966f..3676316f76f0 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -19,7 +19,6 @@
 #include <deploymentstatus.h>
 #include <flatfile.h>
 #include <hash.h>
-#include <index/blockfilterindex.h>
 #include <logging.h>
 #include <logging/timer.h>
 #include <node/blockstorage.h>
@@ -2349,10 +2348,6 @@ bool CChainState::FlushStateToDisk(
             int last_prune{m_chain.Height()}; // last height we can prune
             std::optional<std::string> limiting_lock; // prune lock that actually was the limiting factor, only used for logging
 
-            ForEachBlockFilterIndex([&](BlockFilterIndex& index) {
-                last_prune = std::max(1, std::min(last_prune, index.GetSummary().best_block_height));
-            });
-
             for (const auto& prune_lock : m_blockman.m_prune_locks) {
                 if (prune_lock.second.height_first == std::numeric_limits<int>::max()) continue;
                 // Remove the buffer and one additional block here to get actual height that is outside of the buffer
diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py
index 24163ec7870f..e04909c0a593 100755
--- a/test/lint/lint-circular-dependencies.py
+++ b/test/lint/lint-circular-dependencies.py
@@ -15,8 +15,6 @@
 EXPECTED_CIRCULAR_DEPENDENCIES = (
     "chainparamsbase -> util/system -> chainparamsbase",
     "node/blockstorage -> validation -> node/blockstorage",
-    "index/blockfilterindex -> node/blockstorage -> validation -> index/blockfilterindex",
-    "index/base -> validation -> index/blockfilterindex -> index/base",
     "index/coinstatsindex -> node/coinstats -> index/coinstatsindex",
     "policy/fees -> txmempool -> policy/fees",
     "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel",

From 825d19839bf71245306d4c8edde040e5941caa46 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Thu, 13 May 2021 19:20:47 +0200
Subject: [PATCH 23/28] Index: Allow coinstatsindex with pruning enabled

---
 src/init.cpp                       | 4 +---
 test/functional/feature_pruning.py | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index aa1cff761e12..d193ed1da79a 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -421,7 +421,7 @@ void SetupServerArgs(ArgsManager& argsman)
         -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-persistmempool", strprintf("Whether to save the mempool on shutdown and load on restart (default: %u)", DEFAULT_PERSIST_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex and -coinstatsindex. "
+    argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex. "
             "Warning: Reverting this setting requires re-downloading the entire blockchain. "
             "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >=%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -858,8 +858,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
     if (args.GetIntArg("-prune", 0)) {
         if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX))
             return InitError(_("Prune mode is incompatible with -txindex."));
-        if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX))
-            return InitError(_("Prune mode is incompatible with -coinstatsindex."));
         if (args.GetBoolArg("-reindex-chainstate", false)) {
             return InitError(_("Prune mode is incompatible with -reindex-chainstate. Use full -reindex instead."));
         }
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index bf193842795e..4110526d1515 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -137,10 +137,6 @@ def test_invalid_command_line_options(self):
             expected_msg='Error: Prune mode is incompatible with -txindex.',
             extra_args=['-prune=550', '-txindex'],
         )
-        self.nodes[0].assert_start_raises_init_error(
-            expected_msg='Error: Prune mode is incompatible with -coinstatsindex.',
-            extra_args=['-prune=550', '-coinstatsindex'],
-        )
         self.nodes[0].assert_start_raises_init_error(
             expected_msg='Error: Prune mode is incompatible with -reindex-chainstate. Use full -reindex instead.',
             extra_args=['-prune=550', '-reindex-chainstate'],

From de08932efa953e9a237cbf879460488ad8947411 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Thu, 13 May 2021 19:32:09 +0200
Subject: [PATCH 24/28] test: Update test for indices on pruned nodes

---
 .../feature_blockfilterindex_prune.py         | 164 +++++++++++++-----
 1 file changed, 121 insertions(+), 43 deletions(-)

diff --git a/test/functional/feature_blockfilterindex_prune.py b/test/functional/feature_blockfilterindex_prune.py
index c983ceda6f7e..2bf57db9236b 100755
--- a/test/functional/feature_blockfilterindex_prune.py
+++ b/test/functional/feature_blockfilterindex_prune.py
@@ -2,77 +2,155 @@
 # Copyright (c) 2020-2021 The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Test blockfilterindex in conjunction with prune."""
+"""Test indices in conjunction with prune."""
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_greater_than,
     assert_raises_rpc_error,
+    p2p_port,
 )
 
 
-class FeatureBlockfilterindexPruneTest(BitcoinTestFramework):
+class FeatureIndexPruneTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.num_nodes = 1
-        self.extra_args = [["-fastprune", "-prune=1", "-blockfilterindex=1"]]
+        self.num_nodes = 4
+        self.extra_args = [
+            ["-fastprune", "-prune=1", "-blockfilterindex=1"],
+            ["-fastprune", "-prune=1", "-coinstatsindex=1"],
+            ["-fastprune", "-prune=1", "-blockfilterindex=1", "-coinstatsindex=1"],
+            []
+        ]
 
     def sync_index(self, height):
-        expected = {'basic block filter index': {'synced': True, 'best_block_height': height}}
-        self.wait_until(lambda: self.nodes[0].getindexinfo() == expected)
+        expected_filter = {
+            'basic block filter index': {'synced': True, 'best_block_height': height},
+        }
+        self.wait_until(lambda: self.nodes[0].getindexinfo() == expected_filter)
+
+        expected_stats = {
+            'coinstatsindex': {'synced': True, 'best_block_height': height}
+        }
+        self.wait_until(lambda: self.nodes[1].getindexinfo() == expected_stats)
+
+        expected = {**expected_filter, **expected_stats}
+        self.wait_until(lambda: self.nodes[2].getindexinfo() == expected)
+
+    def reconnect_nodes(self):
+        self.connect_nodes(0,1)
+        self.connect_nodes(0,2)
+        self.connect_nodes(0,3)
+
+    def mine_batches(self, blocks):
+        n = blocks // 250
+        for _ in range(n):
+            self.generate(self.nodes[0], 250)
+        self.generate(self.nodes[0], blocks % 250)
+        self.sync_blocks()
+
+    def restart_without_indices(self):
+        for i in range(3):
+            self.restart_node(i, extra_args=["-fastprune", "-prune=1"])
+        self.reconnect_nodes()
 
     def run_test(self):
-        self.log.info("check if we can access a blockfilter when pruning is enabled but no blocks are actually pruned")
+        filter_nodes = [self.nodes[0], self.nodes[2]]
+        stats_nodes = [self.nodes[1], self.nodes[2]]
+
+        self.log.info("check if we can access blockfilters and coinstats when pruning is enabled but no blocks are actually pruned")
         self.sync_index(height=200)
-        assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getbestblockhash())['filter']), 0)
-        self.generate(self.nodes[0], 500)
+        tip = self.nodes[0].getbestblockhash()
+        for node in filter_nodes:
+            assert_greater_than(len(node.getblockfilter(tip)['filter']), 0)
+        for node in stats_nodes:
+            assert(node.gettxoutsetinfo(hash_type="muhash", hash_or_height=tip)['muhash'])
+
+        self.mine_batches(500)
         self.sync_index(height=700)
 
         self.log.info("prune some blocks")
-        pruneheight = self.nodes[0].pruneblockchain(400)
-        # the prune heights used here and below are magic numbers that are determined by the
-        # thresholds at which block files wrap, so they depend on disk serialization and default block file size.
-        assert_equal(pruneheight, 249)
-
-        self.log.info("check if we can access the tips blockfilter when we have pruned some blocks")
-        assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getbestblockhash())['filter']), 0)
-
-        self.log.info("check if we can access the blockfilter of a pruned block")
-        assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getblockhash(2))['filter']), 0)
+        for node in self.nodes[:2]:
+            with node.assert_debug_log(['limited pruning to height 689']):
+                pruneheight_new = node.pruneblockchain(400)
+                # the prune heights used here and below are magic numbers that are determined by the
+                # thresholds at which block files wrap, so they depend on disk serialization and default block file size.
+                assert_equal(pruneheight_new, 249)
+
+        self.log.info("check if we can access the tips blockfilter and coinstats when we have pruned some blocks")
+        tip = self.nodes[0].getbestblockhash()
+        for node in filter_nodes:
+            assert_greater_than(len(node.getblockfilter(tip)['filter']), 0)
+        for node in stats_nodes:
+            assert(node.gettxoutsetinfo(hash_type="muhash", hash_or_height=tip)['muhash'])
+
+        self.log.info("check if we can access the blockfilter and coinstats of a pruned block")
+        height_hash = self.nodes[0].getblockhash(2)
+        for node in filter_nodes:
+            assert_greater_than(len(node.getblockfilter(height_hash)['filter']), 0)
+        for node in stats_nodes:
+            assert(node.gettxoutsetinfo(hash_type="muhash", hash_or_height=height_hash)['muhash'])
 
         # mine and sync index up to a height that will later be the pruneheight
         self.generate(self.nodes[0], 51)
         self.sync_index(height=751)
 
-        self.log.info("start node without blockfilterindex")
-        self.restart_node(0, extra_args=["-fastprune", "-prune=1"])
+        self.restart_without_indices()
+
+        self.log.info("make sure trying to access the indices throws errors")
+        for node in filter_nodes:
+            msg = "Index is not enabled for filtertype basic"
+            assert_raises_rpc_error(-1, msg, node.getblockfilter, height_hash)
+        for node in stats_nodes:
+            msg = "Querying specific block heights requires coinstatsindex"
+            assert_raises_rpc_error(-8, msg, node.gettxoutsetinfo, "muhash", height_hash)
 
-        self.log.info("make sure accessing the blockfilters throws an error")
-        assert_raises_rpc_error(-1, "Index is not enabled for filtertype basic", self.nodes[0].getblockfilter, self.nodes[0].getblockhash(2))
-        self.generate(self.nodes[0], 749)
+        self.mine_batches(749)
 
-        self.log.info("prune exactly up to the blockfilterindexes best block while blockfilters are disabled")
-        pruneheight_2 = self.nodes[0].pruneblockchain(1000)
-        assert_equal(pruneheight_2, 751)
-        self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1"])
-        self.log.info("make sure that we can continue with the partially synced index after having pruned up to the index height")
+        self.log.info("prune exactly up to the indices best blocks while the indices are disabled")
+        for i in range(3):
+            pruneheight_2 = self.nodes[i].pruneblockchain(1000)
+            assert_equal(pruneheight_2, 751)
+            # Restart the nodes again with the indices activated
+            self.restart_node(i, extra_args=self.extra_args[i])
+
+        self.log.info("make sure that we can continue with the partially synced indices after having pruned up to the index height")
         self.sync_index(height=1500)
 
-        self.log.info("prune below the blockfilterindexes best block while blockfilters are disabled")
-        self.restart_node(0, extra_args=["-fastprune", "-prune=1"])
-        self.generate(self.nodes[0], 1000)
-        pruneheight_3 = self.nodes[0].pruneblockchain(2000)
-        assert_greater_than(pruneheight_3, pruneheight_2)
-        self.stop_node(0)
+        self.log.info("prune further than the indices best blocks while the indices are disabled")
+        self.restart_without_indices()
+        self.mine_batches(1000)
+
+        for i in range(3):
+            pruneheight_3 = self.nodes[i].pruneblockchain(2000)
+            assert_greater_than(pruneheight_3, pruneheight_2)
+            self.stop_node(i)
+
+        self.log.info("make sure we get an init error when starting the nodes again with the indices")
+        filter_msg = "Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"
+        stats_msg = "Error: coinstatsindex best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"
+        for i, msg in enumerate([filter_msg, stats_msg, filter_msg]):
+            self.nodes[i].assert_start_raises_init_error(extra_args=self.extra_args[i], expected_msg=msg)
+
+        self.log.info("make sure the nodes start again with the indices and an additional -reindex arg")
+        ip_port = "127.0.0.1:" + str(p2p_port(3))
+        for i in range(3):
+            # The nodes need to be reconnected to the non-pruning node upon restart, otherwise they will be stuck
+            restart_args = self.extra_args[i]+["-reindex", f"-connect={ip_port}"]
+            self.restart_node(i, extra_args=restart_args)
+
+        self.sync_blocks(timeout=300)
 
-        self.log.info("make sure we get an init error when starting the node again with block filters")
-        self.nodes[0].assert_start_raises_init_error(
-            extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1"],
-            expected_msg="Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)",
-        )
+        for node in self.nodes[:2]:
+            with node.assert_debug_log(['limited pruning to height 2489']):
+                pruneheight_new = node.pruneblockchain(2500)
+                assert_equal(pruneheight_new, 2006)
 
-        self.log.info("make sure the node starts again with the -reindex arg")
-        self.start_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex", "-reindex"])
+        self.log.info("ensure that prune locks don't prevent indices from failing in a reorg scenario")
+        with self.nodes[0].assert_debug_log(['basic block filter index prune lock moved back to 2480']):
+            self.nodes[3].invalidateblock(self.nodes[0].getblockhash(2480))
+            self.generate(self.nodes[3], 30)
+            self.sync_blocks()
 
 
 if __name__ == '__main__':
-    FeatureBlockfilterindexPruneTest().main()
+    FeatureIndexPruneTest().main()

From 71c3f0356c01521a95c64fba1e7375aea6286bb0 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Thu, 13 May 2021 19:33:08 +0200
Subject: [PATCH 25/28] move-only: Rename index + pruning functional test

---
 ...feature_blockfilterindex_prune.py => feature_index_prune.py} | 0
 test/functional/test_runner.py                                  | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename test/functional/{feature_blockfilterindex_prune.py => feature_index_prune.py} (100%)

diff --git a/test/functional/feature_blockfilterindex_prune.py b/test/functional/feature_index_prune.py
similarity index 100%
rename from test/functional/feature_blockfilterindex_prune.py
rename to test/functional/feature_index_prune.py
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index a3c938ae261e..ee8053984d82 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -82,6 +82,7 @@
     # Longest test should go first, to favor running tests in parallel
     'feature_pruning.py',
     'feature_dbcrash.py',
+    'feature_index_prune.py',
 ]
 
 BASE_SCRIPTS = [
@@ -332,7 +333,6 @@
     'feature_help.py',
     'feature_shutdown.py',
     'p2p_ibd_txrelay.py',
-    'feature_blockfilterindex_prune.py'
     # Don't append tests at the end to avoid merge conflicts
     # Put them in a random line within the section that fits their approximate run-time
 ]

From bae4561938f66b31420ffc3f09c9a62932355b8c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= <joao.paulo.barbosa@gmail.com>
Date: Tue, 26 Apr 2022 09:41:00 +0100
Subject: [PATCH 26/28] scripted-diff: rename BytePtr to AsBytePtr

Building with iPhoneOS SDK fails because it also has `BytePtr` defined
in /usr/include/MacTypes.h.

-BEGIN VERIFY SCRIPT-
sed -i 's/BytePtr/AsBytePtr/' $(git grep -l "BytePtr" src)
-END VERIFY SCRIPT-
---
 src/serialize.h       | 8 ++++----
 src/span.h            | 8 ++++----
 src/wallet/bdb.cpp    | 6 +++---
 src/wallet/sqlite.cpp | 6 +++---
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/serialize.h b/src/serialize.h
index 44bb471f259c..a1cce78451c1 100644
--- a/src/serialize.h
+++ b/src/serialize.h
@@ -472,10 +472,10 @@ struct CustomUintFormatter
         if (v < 0 || v > MAX) throw std::ios_base::failure("CustomUintFormatter value out of range");
         if (BigEndian) {
             uint64_t raw = htobe64(v);
-            s.write({BytePtr(&raw) + 8 - Bytes, Bytes});
+            s.write({AsBytePtr(&raw) + 8 - Bytes, Bytes});
         } else {
             uint64_t raw = htole64(v);
-            s.write({BytePtr(&raw), Bytes});
+            s.write({AsBytePtr(&raw), Bytes});
         }
     }
 
@@ -485,10 +485,10 @@ struct CustomUintFormatter
         static_assert(std::numeric_limits<U>::max() >= MAX && std::numeric_limits<U>::min() <= 0, "Assigned type too small");
         uint64_t raw = 0;
         if (BigEndian) {
-            s.read({BytePtr(&raw) + 8 - Bytes, Bytes});
+            s.read({AsBytePtr(&raw) + 8 - Bytes, Bytes});
             v = static_cast<I>(be64toh(raw));
         } else {
-            s.read({BytePtr(&raw), Bytes});
+            s.read({AsBytePtr(&raw), Bytes});
             v = static_cast<I>(le64toh(raw));
         }
     }
diff --git a/src/span.h b/src/span.h
index b627b993c27b..444d63001f22 100644
--- a/src/span.h
+++ b/src/span.h
@@ -245,19 +245,19 @@ T& SpanPopBack(Span<T>& span)
 
 //! Convert a data pointer to a std::byte data pointer.
 //! Where possible, please use the safer AsBytes helpers.
-inline const std::byte* BytePtr(const void* data) { return reinterpret_cast<const std::byte*>(data); }
-inline std::byte* BytePtr(void* data) { return reinterpret_cast<std::byte*>(data); }
+inline const std::byte* AsBytePtr(const void* data) { return reinterpret_cast<const std::byte*>(data); }
+inline std::byte* AsBytePtr(void* data) { return reinterpret_cast<std::byte*>(data); }
 
 // From C++20 as_bytes and as_writeable_bytes
 template <typename T>
 Span<const std::byte> AsBytes(Span<T> s) noexcept
 {
-    return {BytePtr(s.data()), s.size_bytes()};
+    return {AsBytePtr(s.data()), s.size_bytes()};
 }
 template <typename T>
 Span<std::byte> AsWritableBytes(Span<T> s) noexcept
 {
-    return {BytePtr(s.data()), s.size_bytes()};
+    return {AsBytePtr(s.data()), s.size_bytes()};
 }
 
 template <typename V>
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 0d0456af4bd6..7f709ffa3ef0 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -683,10 +683,10 @@ bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool&
     // Convert to streams
     ssKey.SetType(SER_DISK);
     ssKey.clear();
-    ssKey.write({BytePtr(datKey.get_data()), datKey.get_size()});
+    ssKey.write({AsBytePtr(datKey.get_data()), datKey.get_size()});
     ssValue.SetType(SER_DISK);
     ssValue.clear();
-    ssValue.write({BytePtr(datValue.get_data()), datValue.get_size()});
+    ssValue.write({AsBytePtr(datValue.get_data()), datValue.get_size()});
     return true;
 }
 
@@ -758,7 +758,7 @@ bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value)
     SafeDbt datValue;
     int ret = pdb->get(activeTxn, datKey, datValue, 0);
     if (ret == 0 && datValue.get_data() != nullptr) {
-        value.write({BytePtr(datValue.get_data()), datValue.get_size()});
+        value.write({AsBytePtr(datValue.get_data()), datValue.get_size()});
         return true;
     }
     return false;
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index 3f860289f974..2515df317769 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -405,7 +405,7 @@ bool SQLiteBatch::ReadKey(CDataStream&& key, CDataStream& value)
         return false;
     }
     // Leftmost column in result is index 0
-    const std::byte* data{BytePtr(sqlite3_column_blob(m_read_stmt, 0))};
+    const std::byte* data{AsBytePtr(sqlite3_column_blob(m_read_stmt, 0))};
     size_t data_size(sqlite3_column_bytes(m_read_stmt, 0));
     value.write({data, data_size});
 
@@ -497,10 +497,10 @@ bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& compl
     }
 
     // Leftmost column in result is index 0
-    const std::byte* key_data{BytePtr(sqlite3_column_blob(m_cursor_stmt, 0))};
+    const std::byte* key_data{AsBytePtr(sqlite3_column_blob(m_cursor_stmt, 0))};
     size_t key_data_size(sqlite3_column_bytes(m_cursor_stmt, 0));
     key.write({key_data, key_data_size});
-    const std::byte* value_data{BytePtr(sqlite3_column_blob(m_cursor_stmt, 1))};
+    const std::byte* value_data{AsBytePtr(sqlite3_column_blob(m_cursor_stmt, 1))};
     size_t value_data_size(sqlite3_column_bytes(m_cursor_stmt, 1));
     value.write({value_data, value_data_size});
     return true;

From e53274868e0ec35156349826b0995bc444287690 Mon Sep 17 00:00:00 2001
From: practicalswift <practicalswift@users.noreply.github.com>
Date: Mon, 30 Jul 2018 12:37:09 +0200
Subject: [PATCH 27/28] Don't use zero as null pointer constant
 (-Wzero-as-null-pointer-constant)

---
 src/mapport.cpp                             | 4 ++--
 src/netbase.cpp                             | 2 +-
 src/qt/intro.cpp                            | 2 +-
 src/randomenv.cpp                           | 4 ++--
 src/support/allocators/secure.h             | 2 +-
 src/test/fuzz/crypto_diff_fuzz_chacha20.cpp | 2 +-
 src/torcontrol.cpp                          | 2 +-
 src/wallet/wallet.cpp                       | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/mapport.cpp b/src/mapport.cpp
index 42ca3660893b..235e6f904cec 100644
--- a/src/mapport.cpp
+++ b/src/mapport.cpp
@@ -193,7 +193,7 @@ static bool ProcessUpnp()
         std::string strDesc = PACKAGE_NAME " " + FormatFullVersion();
 
         do {
-            r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0, "0");
+            r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", nullptr, "0");
 
             if (r != UPNPCOMMAND_SUCCESS) {
                 ret = false;
@@ -206,7 +206,7 @@ static bool ProcessUpnp()
         } while (g_mapport_interrupt.sleep_for(PORT_MAPPING_REANNOUNCE_PERIOD));
         g_mapport_interrupt.reset();
 
-        r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", 0);
+        r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", nullptr);
         LogPrintf("UPNP_DeletePortMapping() returned: %d\n", r);
         freeUPNPDevlist(devlist); devlist = nullptr;
         FreeUPNPUrls(&urls);
diff --git a/src/netbase.cpp b/src/netbase.cpp
index 9557297df173..ce23f7e4add2 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -675,7 +675,7 @@ bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_
             return false;
         }
     } else {
-        if (!Socks5(strDest, port, 0, sock)) {
+        if (!Socks5(strDest, port, nullptr, sock)) {
             return false;
         }
     }
diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp
index 63b4055092bf..f4928951fe57 100644
--- a/src/qt/intro.cpp
+++ b/src/qt/intro.cpp
@@ -226,7 +226,7 @@ bool Intro::showIfNeeded(bool& did_show_intro, int64_t& prune_MiB)
         }
 
         /* If current default data directory does not exist, let the user choose one */
-        Intro intro(0, Params().AssumedBlockchainSize(), Params().AssumedChainStateSize());
+        Intro intro(nullptr, Params().AssumedBlockchainSize(), Params().AssumedChainStateSize());
         intro.setDataDirectory(dataDir);
         intro.setWindowIcon(QIcon(":icons/bitcoin"));
         did_show_intro = true;
diff --git a/src/randomenv.cpp b/src/randomenv.cpp
index 388841289a13..53ebeb31cdf8 100644
--- a/src/randomenv.cpp
+++ b/src/randomenv.cpp
@@ -363,10 +363,10 @@ void RandAddStaticEnv(CSHA512& hasher)
 
 #if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS
     // Network interfaces
-    struct ifaddrs *ifad = NULL;
+    struct ifaddrs *ifad = nullptr;
     getifaddrs(&ifad);
     struct ifaddrs *ifit = ifad;
-    while (ifit != NULL) {
+    while (ifit != nullptr) {
         hasher.Write((const unsigned char*)&ifit, sizeof(ifit));
         hasher.Write((const unsigned char*)ifit->ifa_name, strlen(ifit->ifa_name) + 1);
         hasher.Write((const unsigned char*)&ifit->ifa_flags, sizeof(ifit->ifa_flags));
diff --git a/src/support/allocators/secure.h b/src/support/allocators/secure.h
index a9aa928082e7..c6bd6851898d 100644
--- a/src/support/allocators/secure.h
+++ b/src/support/allocators/secure.h
@@ -37,7 +37,7 @@ struct secure_allocator : public std::allocator<T> {
         typedef secure_allocator<_Other> other;
     };
 
-    T* allocate(std::size_t n, const void* hint = 0)
+    T* allocate(std::size_t n, const void* hint = nullptr)
     {
         T* allocation = static_cast<T*>(LockedPoolManager::Instance().alloc(sizeof(T) * n));
         if (!allocation) {
diff --git a/src/test/fuzz/crypto_diff_fuzz_chacha20.cpp b/src/test/fuzz/crypto_diff_fuzz_chacha20.cpp
index fcc96c641883..1b89d557735e 100644
--- a/src/test/fuzz/crypto_diff_fuzz_chacha20.cpp
+++ b/src/test/fuzz/crypto_diff_fuzz_chacha20.cpp
@@ -128,7 +128,7 @@ void ECRYPT_encrypt_bytes(ECRYPT_ctx* x, const u8* m, u8* c, u32 bytes)
 {
     u32 x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15;
     u32 j0, j1, j2, j3, j4, j5, j6, j7, j8, j9, j10, j11, j12, j13, j14, j15;
-    u8* ctarget = NULL;
+    u8* ctarget = nullptr;
     u8 tmp[64];
     uint32_t i;
 
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index 287de9c79450..e2a7cb406621 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -306,7 +306,7 @@ std::map<std::string,std::string> ParseTorReplyMapping(const std::string &s)
 
 TorController::TorController(struct event_base* _base, const std::string& tor_control_center, const CService& target):
     base(_base),
-    m_tor_control_center(tor_control_center), conn(base), reconnect(true), reconnect_ev(0),
+    m_tor_control_center(tor_control_center), conn(base), reconnect(true), reconnect_ev(nullptr),
     reconnect_timeout(RECONNECT_TIMEOUT_START),
     m_target(target)
 {
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 0d7075810db7..489599e2a069 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2768,7 +2768,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
     } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) {
         // Make it impossible to disable private keys after creation
         error = strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile);
-        return NULL;
+        return nullptr;
     } else if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
         for (auto spk_man : walletInstance->GetActiveScriptPubKeyMans()) {
             if (spk_man->HavePrivateKeys()) {

From 9c96f1008b4997aea31f293fed31f98ed3becfcf Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Mon, 25 Apr 2022 10:33:32 +0100
Subject: [PATCH 28/28] tidy: enable modernize-use-nullptr

---
 ci/test/06_script_a.sh |  2 +-
 configure.ac           |  1 +
 src/.bear-tidy-config  | 15 +++++++++++++++
 src/.clang-tidy        |  4 ++--
 4 files changed, 19 insertions(+), 3 deletions(-)
 create mode 100644 src/.bear-tidy-config

diff --git a/ci/test/06_script_a.sh b/ci/test/06_script_a.sh
index 4742cb02ead3..6a6bde05a100 100755
--- a/ci/test/06_script_a.sh
+++ b/ci/test/06_script_a.sh
@@ -49,7 +49,7 @@ if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then
 fi
 
 if [[ "${RUN_TIDY}" == "true" ]]; then
-  MAYBE_BEAR="bear"
+  MAYBE_BEAR="bear --config src/.bear-tidy-config"
   MAYBE_TOKEN="--"
 fi
 
diff --git a/configure.ac b/configure.ac
index 4c80899e5f20..554083a44cee 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1935,6 +1935,7 @@ AC_CONFIG_LINKS([contrib/devtools/test-security-check.py:contrib/devtools/test-s
 AC_CONFIG_LINKS([contrib/devtools/test-symbol-check.py:contrib/devtools/test-symbol-check.py])
 AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py])
 AC_CONFIG_LINKS([contrib/macdeploy/background.tiff:contrib/macdeploy/background.tiff])
+AC_CONFIG_LINKS([src/.bear-tidy-config:src/.bear-tidy-config])
 AC_CONFIG_LINKS([src/.clang-tidy:src/.clang-tidy])
 AC_CONFIG_LINKS([test/functional/test_runner.py:test/functional/test_runner.py])
 AC_CONFIG_LINKS([test/fuzz/test_runner.py:test/fuzz/test_runner.py])
diff --git a/src/.bear-tidy-config b/src/.bear-tidy-config
new file mode 100644
index 000000000000..111ef6ee444a
--- /dev/null
+++ b/src/.bear-tidy-config
@@ -0,0 +1,15 @@
+{
+  "output": {
+    "content": {
+      "include_only_existing_source": true,
+      "paths_to_include": [],
+      "paths_to_exclude": [
+        "src/leveldb"
+      ]
+    },
+    "format": {
+      "command_as_array": true,
+      "drop_output_field": false
+    }
+  }
+}
diff --git a/src/.clang-tidy b/src/.clang-tidy
index 27616ad072fc..fda95967f54b 100644
--- a/src/.clang-tidy
+++ b/src/.clang-tidy
@@ -1,2 +1,2 @@
-Checks:          '-*,bugprone-argument-comment'
-WarningsAsErrors: bugprone-argument-comment
+Checks:          '-*,bugprone-argument-comment,modernize-use-nullptr'
+WarningsAsErrors: 'bugprone-argument-comment,modernize-use-nullptr'