Merge #6297: backport: merge bitcoin#23156, #23213, #23227, #23223, #23564, #23538, #23437, #23630, #23465, #23738, #17631, #22875 (auxiliary backports: part 18)

5aceee38fc merge bitcoin#22875: Fix Racy ParseOpCode function initialization (Kittywhiskers Van Gogh)
427d07f4db merge bitcoin#17631: Expose block filters over REST (Kittywhiskers Van Gogh)
d60f15ec33 merge bitcoin#23738: improve logging of ChainstateManager snapshot persistance (Kittywhiskers Van Gogh)
87257347c2 merge bitcoin#23465: Remove CTxMemPool params from ATMP (Kittywhiskers Van Gogh)
d2cbdc40d5 merge bitcoin#23630: Remove GetSpendHeight (Kittywhiskers Van Gogh)
8bdab4d4fe merge bitcoin#23437: AcceptToMemoryPool (Kittywhiskers Van Gogh)
1f4e8a0cf9 merge bitcoin#23538: Remove strtol in torcontrol (Kittywhiskers Van Gogh)
2318d9f996 merge bitcoin#23564: don't use deprecated brew package names (Kittywhiskers Van Gogh)
3b7a7394a9 merge bitcoin#23223: Disable lock contention logging in checkqueue_tests (Kittywhiskers Van Gogh)
b383609a72 merge bitcoin#23227: Avoid treating integer overflow as OP_0 (Kittywhiskers Van Gogh)
0188d32430 merge bitcoin#23213: Return error when header count is not integral (Kittywhiskers Van Gogh)
eb9e20890f merge bitcoin#23156: Remove unused ParsePrechecks and ParseDouble (Kittywhiskers Van Gogh)
18fff7e3d3 rpc: switch to taking an integer for `rate` in `quorum dkgsimerror` (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on https://github.com/dashpay/dash/pull/6288

  * Dependent on https://github.com/dashpay/dash/pull/6296

  ## Breaking changes

  - `quorum dkgsimerror` will no longer accept a decimal value between 0 and 1 for the `rate` argument, it will now expect an integer between 0 to 100.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 5aceee38fc
  UdjinM6:
    utACK 5aceee38fc
  knst:
    utACK 5aceee38fc

Tree-SHA512: 8fc34b05a74f2ddbe84b2a7a54772e49941042c89bc74d71d33711e658754a3d086af11fb2437d2bb72ede0c611adc57b82193783e7b6f10fbd4ebab2a7fa7cb

.cirrus.yml

@@ -164,7 +164,7 @@ task:
 task:
   name: 'macOS 11 native [gui] [no depends]'
   brew_install_script:
-    - brew install boost libevent berkeley-db4 qt@5 miniupnpc ccache zeromq qrencode sqlite libtool automake pkg-config gnu-getopt
+    - brew install boost libevent berkeley-db@4 qt@5 miniupnpc ccache zeromq qrencode sqlite libtool automake pkg-config gnu-getopt
   << : *GLOBAL_TASK_TEMPLATE
   macos_instance:
     # Use latest image, but hardcode version to avoid silent upgrades (and breaks)

configure.ac

@@ -746,8 +746,8 @@ case $host in
          dnl It's safe to add these paths even if the functionality is disabled by
          dnl the user (--without-wallet or --without-gui for example).
 
-         if test "x$use_bdb" != xno && $BREW list --versions berkeley-db4 >/dev/null && test "x$BDB_CFLAGS" = "x" && test "x$BDB_LIBS" = "x"; then
+         if test "x$use_bdb" != xno && $BREW list --versions berkeley-db@4 >/dev/null && test "x$BDB_CFLAGS" = "x" && test "x$BDB_LIBS" = "x"; then
-           bdb_prefix=$($BREW --prefix berkeley-db4 2>/dev/null)
+           bdb_prefix=$($BREW --prefix berkeley-db@4 2>/dev/null)
            dnl This must precede the call to BITCOIN_FIND_BDB48 below.
            BDB_CFLAGS="-I$bdb_prefix/include"
            BDB_LIBS="-L$bdb_prefix/lib -ldb_cxx-4.8"
@@ -757,8 +757,8 @@ case $host in
            export PKG_CONFIG_PATH="$($BREW --prefix sqlite3 2>/dev/null)/lib/pkgconfig:$PKG_CONFIG_PATH"
          fi
 
-         if $BREW list --versions qt5 >/dev/null; then
+         if $BREW list --versions qt@5 >/dev/null; then
-           export PKG_CONFIG_PATH="$($BREW --prefix qt5 2>/dev/null)/lib/pkgconfig:$PKG_CONFIG_PATH"
+           export PKG_CONFIG_PATH="$($BREW --prefix qt@5 2>/dev/null)/lib/pkgconfig:$PKG_CONFIG_PATH"
          fi
 
          gmp_prefix=$($BREW --prefix gmp 2>/dev/null)

doc/REST-interface.md

@@ -52,6 +52,20 @@ With the /notxdetails/ option JSON response will only contain the transaction ha
 Given a block hash: returns <COUNT> amount of blockheaders in upward direction.
 Returns empty if the block doesn't exist or it isn't in the active chain.
 
+#### Blockfilter Headers
+`GET /rest/blockfilterheaders/<FILTERTYPE>/<COUNT>/<BLOCK-HASH>.<bin|hex|json>`
+
+Given a block hash: returns <COUNT> amount of blockfilter headers in upward
+direction for the filter type <FILTERTYPE>.
+Returns empty if the block doesn't exist or it isn't in the active chain.
+
+#### Blockfilters
+`GET /rest/blockfilter/<FILTERTYPE>/<BLOCK-HASH>.<bin|hex|json>`
+
+Given a block hash: returns the block filter of the given block of type
+<FILTERTYPE>.
+Responds with 404 if the block doesn't exist.
+
 #### Blockhash by height
 `GET /rest/blockhashbyheight/<HEIGHT>.<bin|hex|json>`
 

doc/release-notes-6297.md

@@ -0,0 +1,5 @@
+RPC changes
+-----------
+
+- `quorum dkgsimerror` will no longer accept a decimal value between 0 and 1 for the `rate` argument, it will now
+  expect an integer between 0 to 100.

src/core_read.cpp

@@ -19,32 +19,43 @@
 #include <string>
 
 namespace {
-
+class OpCodeParser
-opcodetype ParseOpCode(const std::string& s)
 {
-    static std::map<std::string, opcodetype> mapOpNames;
+private:
+    std::map<std::string, opcodetype> mapOpNames;
 
-    if (mapOpNames.empty())
+public:
+    OpCodeParser()
     {
-        for (unsigned int op = 0; op <= MAX_OPCODE; op++)
+        for (unsigned int op = 0; op <= MAX_OPCODE; ++op) {
-        {
             // Allow OP_RESERVED to get into mapOpNames
-            if (op < OP_NOP && op != OP_RESERVED)
+            if (op < OP_NOP && op != OP_RESERVED) {
                 continue;
+            }
 
             std::string strName = GetOpName(static_cast<opcodetype>(op));
-            if (strName == "OP_UNKNOWN")
+            if (strName == "OP_UNKNOWN") {
                 continue;
+            }
             mapOpNames[strName] = static_cast<opcodetype>(op);
             // Convenience: OP_ADD and just ADD are both recognized:
-            if (strName.compare(0, 3, "OP_") == 0) {  // strName starts with "OP_"
+            if (strName.compare(0, 3, "OP_") == 0) { // strName starts with "OP_"
                 mapOpNames[strName.substr(3)] = static_cast<opcodetype>(op);
             }
         }
     }
-    auto it = mapOpNames.find(s);
+    opcodetype Parse(const std::string& s) const
-    if (it == mapOpNames.end()) throw std::runtime_error("script parse error: unknown opcode");
+    {
-    return it->second;
+        auto it = mapOpNames.find(s);
+        if (it == mapOpNames.end()) throw std::runtime_error("script parse error: unknown opcode");
+        return it->second;
+    }
+};
+
+opcodetype ParseOpCode(const std::string& s)
+{
+    static const OpCodeParser ocp;
+    return ocp.Parse(s);
 }
 
 } // namespace
@@ -56,44 +67,35 @@ CScript ParseScript(const std::string& s)
 
     std::vector<std::string> words = SplitString(s, " \t\n");
 
-    for (std::vector<std::string>::const_iterator w = words.begin(); w != words.end(); ++w)
+    for (const std::string& w : words) {
-    {
+        if (w.empty()) {
-        if (w->empty())
-        {
             // Empty string, ignore. (SplitString doesn't combine multiple separators)
-        }
+        } else if (std::all_of(w.begin(), w.end(), ::IsDigit) ||
-        else if (std::all_of(w->begin(), w->end(), ::IsDigit) ||
+                   (w.front() == '-' && w.size() > 1 && std::all_of(w.begin() + 1, w.end(), ::IsDigit)))
-            (w->front() == '-' && w->size() > 1 && std::all_of(w->begin()+1, w->end(), ::IsDigit)))
         {
             // Number
-            int64_t n = LocaleIndependentAtoi<int64_t>(*w);
+            const auto num{ToIntegral<int64_t>(w)};
 
-            //limit the range of numbers ParseScript accepts in decimal
+            // limit the range of numbers ParseScript accepts in decimal
-            //since numbers outside -0xFFFFFFFF...0xFFFFFFFF are illegal in scripts
+            // since numbers outside -0xFFFFFFFF...0xFFFFFFFF are illegal in scripts
-            if (n > int64_t{0xffffffff} || n < -1 * int64_t{0xffffffff}) {
+            if (!num.has_value() || num > int64_t{0xffffffff} || num < -1 * int64_t{0xffffffff}) {
                 throw std::runtime_error("script parse error: decimal numeric value only allowed in the "
                                          "range -0xFFFFFFFF...0xFFFFFFFF");
             }
 
-            result << n;
+            result << num.value();
-        }
+        } else if (w.substr(0, 2) == "0x" && w.size() > 2 && IsHex(std::string(w.begin() + 2, w.end()))) {
-        else if (w->substr(0,2) == "0x" && w->size() > 2 && IsHex(std::string(w->begin()+2, w->end())))
-        {
             // Raw hex data, inserted NOT pushed onto stack:
-            std::vector<unsigned char> raw = ParseHex(std::string(w->begin()+2, w->end()));
+            std::vector<unsigned char> raw = ParseHex(std::string(w.begin() + 2, w.end()));
             result.insert(result.end(), raw.begin(), raw.end());
-        }
+        } else if (w.size() >= 2 && w.front() == '\'' && w.back() == '\'') {
-        else if (w->size() >= 2 && w->front() == '\'' && w->back() == '\'')
-        {
             // Single-quoted string, pushed as data. NOTE: this is poor-man's
             // parsing, spaces/tabs/newlines in single-quoted strings won't work.
-            std::vector<unsigned char> value(w->begin()+1, w->end()-1);
+            std::vector<unsigned char> value(w.begin() + 1, w.end() - 1);
             result << value;
-        }
+        } else {
-        else
-        {
             // opcode, e.g. OP_ADD or ADD:
-            result << ParseOpCode(*w);
+            result << ParseOpCode(w);
         }
     }
 

src/node/blockstorage.h

@@ -185,13 +185,6 @@ public:
 
     //! Check whether the block associated with this index entry is pruned or not.
     bool IsBlockPruned(const CBlockIndex* pblockindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-
-    /**
-     * Return the spend height, which is one more than the inputs.GetBestBlock().
-     * While checking, GetBestBlock() refers to the parent block. (protected by cs_main)
-     * This is also true for mempool checks.
-     */
-    int GetSpendHeight(const CCoinsViewCache& inputs) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 };
 
 void CleanupBlockRevFiles();

src/rest.cpp

@@ -3,11 +3,13 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <blockfilter.h>
 #include <chain.h>
 #include <chainparams.h>
 #include <context.h>
 #include <core_io.h>
 #include <httpserver.h>
+#include <index/blockfilterindex.h>
 #include <index/txindex.h>
 #include <llmq/chainlocks.h>
 #include <llmq/context.h>
@@ -30,6 +32,7 @@
 #include <univalue.h>
 
 static const size_t MAX_GETUTXOS_OUTPOINTS = 15; //allow a max of 15 outpoints to be queried at once
+static constexpr unsigned int MAX_REST_HEADERS_RESULTS = 2000;
 
 enum class RetFormat {
     UNDEF,
@@ -188,9 +191,10 @@ static bool rest_headers(const CoreContext& context,
     if (path.size() != 2)
         return RESTERR(req, HTTP_BAD_REQUEST, "No header count specified. Use /rest/headers/<count>/<hash>.<ext>.");
 
-    long count = strtol(path[0].c_str(), nullptr, 10);
+    const auto parsed_count{ToIntegral<size_t>(path[0])};
-    if (count < 1 || count > 2000)
+    if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
-        return RESTERR(req, HTTP_BAD_REQUEST, "Header count out of range: " + path[0]);
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count out of acceptable range (1-%u): %s",  MAX_REST_HEADERS_RESULTS, path[0]));
+    }
 
     std::string hashStr = path[1];
     uint256 hash;
@@ -198,8 +202,8 @@ static bool rest_headers(const CoreContext& context,
         return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr);
 
     const CBlockIndex* tip = nullptr;
-    std::vector<const CBlockIndex *> headers;
+    std::vector<const CBlockIndex*> headers;
-    headers.reserve(count);
+    headers.reserve(*parsed_count);
     {
         ChainstateManager* maybe_chainman = GetChainman(context, req);
         if (!maybe_chainman) return false;
@@ -210,8 +214,9 @@ static bool rest_headers(const CoreContext& context,
         const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
         while (pindex != nullptr && active_chain.Contains(pindex)) {
             headers.push_back(pindex);
-            if (headers.size() == (unsigned long)count)
+            if (headers.size() == *parsed_count) {
                 break;
+            }
             pindex = active_chain.Next(pindex);
         }
     }
@@ -251,7 +256,7 @@ static bool rest_headers(const CoreContext& context,
         return true;
     }
     default: {
-        return RESTERR(req, HTTP_NOT_FOUND, "output format not found (available: .bin, .hex, .json)");
+        return RESTERR(req, HTTP_NOT_FOUND, "output format not found (available: " + AvailableDataFormatsString() + ")");
     }
     }
 }
@@ -334,6 +339,208 @@ static bool rest_block_notxdetails(const CoreContext& context, HTTPRequest* req,
     return rest_block(context, req, strURIPart, false);
 }
 
+
+static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart)
+{
+    if (!CheckWarmup(req))
+        return false;
+    std::string param;
+    const RetFormat rf = ParseDataFormat(param, strURIPart);
+
+    std::vector<std::string> uri_parts = SplitString(param, '/');
+    if (uri_parts.size() != 3) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilterheaders/<filtertype>/<count>/<blockhash>");
+    }
+
+    uint256 block_hash;
+    if (!ParseHashStr(uri_parts[2], block_hash)) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + uri_parts[2]);
+    }
+
+    BlockFilterType filtertype;
+    if (!BlockFilterTypeByName(uri_parts[0], filtertype)) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Unknown filtertype " + uri_parts[0]);
+    }
+
+    BlockFilterIndex* index = GetBlockFilterIndex(filtertype);
+    if (!index) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Index is not enabled for filtertype " + uri_parts[0]);
+    }
+
+    const auto parsed_count{ToIntegral<size_t>(uri_parts[1])};
+    if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count out of acceptable range (1-%u): %s",  MAX_REST_HEADERS_RESULTS, uri_parts[1]));
+    }
+
+    std::vector<const CBlockIndex *> headers;
+    headers.reserve(*parsed_count);
+    {
+        ChainstateManager* maybe_chainman = GetChainman(context, req);
+        if (!maybe_chainman) return false;
+        ChainstateManager& chainman = *maybe_chainman;
+        LOCK(cs_main);
+        CChain& active_chain = chainman.ActiveChain();
+        const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(block_hash);
+        while (pindex != nullptr && active_chain.Contains(pindex)) {
+            headers.push_back(pindex);
+            if (headers.size() == *parsed_count)
+                break;
+            pindex = active_chain.Next(pindex);
+        }
+    }
+
+    bool index_ready = index->BlockUntilSyncedToCurrentChain();
+
+    std::vector<uint256> filter_headers;
+    filter_headers.reserve(*parsed_count);
+    for (const CBlockIndex *pindex : headers) {
+        uint256 filter_header;
+        if (!index->LookupFilterHeader(pindex, filter_header)) {
+            std::string errmsg = "Filter not found.";
+
+            if (!index_ready) {
+                errmsg += " Block filters are still in the process of being indexed.";
+            } else {
+                errmsg += " This error is unexpected and indicates index corruption.";
+            }
+
+            return RESTERR(req, HTTP_NOT_FOUND, errmsg);
+        }
+        filter_headers.push_back(filter_header);
+    }
+
+    switch (rf) {
+    case RetFormat::BINARY: {
+        CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION);
+        for (const uint256& header : filter_headers) {
+            ssHeader << header;
+        }
+
+        std::string binaryHeader = ssHeader.str();
+        req->WriteHeader("Content-Type", "application/octet-stream");
+        req->WriteReply(HTTP_OK, binaryHeader);
+        return true;
+    }
+    case RetFormat::HEX: {
+        CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION);
+        for (const uint256& header : filter_headers) {
+            ssHeader << header;
+        }
+
+        std::string strHex = HexStr(ssHeader) + "\n";
+        req->WriteHeader("Content-Type", "text/plain");
+        req->WriteReply(HTTP_OK, strHex);
+        return true;
+    }
+    case RetFormat::JSON: {
+        UniValue jsonHeaders(UniValue::VARR);
+        for (const uint256& header : filter_headers) {
+            jsonHeaders.push_back(header.GetHex());
+        }
+
+        std::string strJSON = jsonHeaders.write() + "\n";
+        req->WriteHeader("Content-Type", "application/json");
+        req->WriteReply(HTTP_OK, strJSON);
+        return true;
+    }
+    default: {
+        return RESTERR(req, HTTP_NOT_FOUND, "output format not found (available: " + AvailableDataFormatsString() + ")");
+    }
+    }
+}
+
+static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart)
+{
+    if (!CheckWarmup(req))
+        return false;
+    std::string param;
+    const RetFormat rf = ParseDataFormat(param, strURIPart);
+
+    // request is sent over URI scheme /rest/blockfilter/filtertype/blockhash
+    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>");
+    }
+
+    uint256 block_hash;
+    if (!ParseHashStr(uri_parts[1], block_hash)) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + uri_parts[1]);
+    }
+
+    BlockFilterType filtertype;
+    if (!BlockFilterTypeByName(uri_parts[0], filtertype)) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Unknown filtertype " + uri_parts[0]);
+    }
+
+    BlockFilterIndex* index = GetBlockFilterIndex(filtertype);
+    if (!index) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Index is not enabled for filtertype " + uri_parts[0]);
+    }
+
+    const CBlockIndex* block_index;
+    bool block_was_connected;
+    {
+        ChainstateManager* maybe_chainman = GetChainman(context, req);
+        if (!maybe_chainman) return false;
+        ChainstateManager& chainman = *maybe_chainman;
+        LOCK(cs_main);
+        block_index = chainman.m_blockman.LookupBlockIndex(block_hash);
+        if (!block_index) {
+            return RESTERR(req, HTTP_NOT_FOUND, uri_parts[1] + " not found");
+        }
+        block_was_connected = block_index->IsValid(BLOCK_VALID_SCRIPTS);
+    }
+
+    bool index_ready = index->BlockUntilSyncedToCurrentChain();
+
+    BlockFilter filter;
+    if (!index->LookupFilter(block_index, filter)) {
+        std::string errmsg = "Filter not found.";
+
+        if (!block_was_connected) {
+            errmsg += " Block was not connected to active chain.";
+        } else if (!index_ready) {
+            errmsg += " Block filters are still in the process of being indexed.";
+        } else {
+            errmsg += " This error is unexpected and indicates index corruption.";
+        }
+
+        return RESTERR(req, HTTP_NOT_FOUND, errmsg);
+    }
+
+    switch (rf) {
+    case RetFormat::BINARY: {
+        CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION);
+        ssResp << filter;
+
+        std::string binaryResp = ssResp.str();
+        req->WriteHeader("Content-Type", "application/octet-stream");
+        req->WriteReply(HTTP_OK, binaryResp);
+        return true;
+    }
+    case RetFormat::HEX: {
+        CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION);
+        ssResp << filter;
+
+        std::string strHex = HexStr(ssResp) + "\n";
+        req->WriteHeader("Content-Type", "text/plain");
+        req->WriteReply(HTTP_OK, strHex);
+        return true;
+    }
+    case RetFormat::JSON: {
+        UniValue ret(UniValue::VOBJ);
+        ret.pushKV("filter", HexStr(filter.GetEncodedFilter()));
+        std::string strJSON = ret.write() + "\n";
+        req->WriteHeader("Content-Type", "application/json");
+        req->WriteReply(HTTP_OK, strJSON);
+        return true;
+    }
+    default: {
+        return RESTERR(req, HTTP_NOT_FOUND, "output format not found (available: " + AvailableDataFormatsString() + ")");
+    }
+    }
+}
+
 // A bit of a hack - dependency on a function defined in rpc/blockchain.cpp
 RPCHelpMan getblockchaininfo();
 
@@ -714,6 +921,8 @@ static const struct {
       {"/rest/tx/", rest_tx},
       {"/rest/block/notxdetails/", rest_block_notxdetails},
       {"/rest/block/", rest_block_extended},
+      {"/rest/blockfilter/", rest_block_filter},
+      {"/rest/blockfilterheaders/", rest_filter_header},
       {"/rest/chaininfo", rest_chaininfo},
       {"/rest/mempool/info", rest_mempool_info},
       {"/rest/mempool/contents", rest_mempool_contents},

src/rpc/quorums.cpp

@@ -750,24 +750,24 @@ static RPCHelpMan quorum_dkgsimerror()
         "as you will get yourself very likely PoSe banned for this.\n",
         {
             {"type", RPCArg::Type::STR, RPCArg::Optional::NO, "Error type."},
-            {"rate", RPCArg::Type::NUM, RPCArg::Optional::NO, "Rate at which to simulate this error type."},
+            {"rate", RPCArg::Type::NUM, RPCArg::Optional::NO, "Rate at which to simulate this error type (between 0 and 100)."},
         },
         RPCResults{},
         RPCExamples{""},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     std::string type_str = request.params[0].get_str();
-    double rate = ParseDoubleV(request.params[1], "rate");
+    int32_t rate = ParseInt32V(request.params[1], "rate");
 
-    if (rate < 0 || rate > 1) {
+    if (rate < 0 || rate > 100) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid rate. Must be between 0 and 1");
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid rate. Must be between 0 and 100");
     }
 
     if (const llmq::DKGError::type type = llmq::DKGError::from_string(type_str);
             type == llmq::DKGError::type::_COUNT) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid type. See DKGError class implementation");
     } else {
-        llmq::SetSimulatedDKGErrorRate(type, rate);
+        llmq::SetSimulatedDKGErrorRate(type, static_cast<double>(rate) / 100);
         return UniValue();
     }
 },

src/rpc/util.cpp

@@ -139,15 +139,6 @@ int64_t ParseInt64V(const UniValue& v, const std::string &strName)
     return num;
 }
 
-double ParseDoubleV(const UniValue& v, const std::string &strName)
-{
-    std::string strNum = v.getValStr();
-    double num;
-    if (!ParseDouble(strNum, &num))
-        throw JSONRPCError(RPC_INVALID_PARAMETER, strName+" must be a be number (not '"+strNum+"')");
-    return num;
-}
-
 bool ParseBoolV(const UniValue& v, const std::string &strName)
 {
     std::string strBool;

src/test/checkqueue_tests.cpp

@@ -17,7 +17,17 @@
 #include <utility>
 #include <vector>
 
-BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, TestingSetup)
+/**
+ * Identical to TestingSetup but excludes lock contention logging, as some of
+ * these tests are designed to be heavily contested to trigger race conditions
+ * or other issues.
+ */
+struct NoLockLoggingTestingSetup : public TestingSetup {
+    NoLockLoggingTestingSetup()
+        : TestingSetup{CBaseChainParams::MAIN, /*extra_args=*/{"-debugexclude=lock"}} {}
+};
+
+BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, NoLockLoggingTestingSetup)
 
 static const unsigned int QUEUE_BATCH_SIZE = 128;
 static const int SCRIPT_CHECK_THREADS = 3;

src/test/fuzz/parse_numbers.cpp

@@ -14,9 +14,6 @@ FUZZ_TARGET(parse_numbers)
 
     (void)ParseMoney(random_string);
 
-    double d;
-    (void)ParseDouble(random_string, &d);
-
     uint8_t u8;
     (void)ParseUInt8(random_string, &u8);
 

src/test/fuzz/tx_pool.cpp

@@ -28,6 +28,15 @@ struct MockedTxPool : public CTxMemPool {
     }
 };
 
+class DummyChainState final : public CChainState
+{
+public:
+    void SetMempool(CTxMemPool* mempool)
+    {
+        m_mempool = mempool;
+    }
+};
+
 void initialize_tx_pool()
 {
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
@@ -86,7 +95,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, con
         options.nBlockMaxSize = fuzzed_data_provider.ConsumeIntegralInRange(0U, MaxBlockSize(true));
         options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /* max */ COIN)};
 
-        auto assembler = BlockAssembler{chainstate, node, *static_cast<CTxMemPool*>(&tx_pool), ::Params(), options};
+        auto assembler = BlockAssembler{chainstate, node, *static_cast<CTxMemPool*>(&tx_pool), chainstate.m_params, options};
         auto block_template = assembler.CreateNewBlock(CScript{} << OP_TRUE);
         Assert(block_template->block.vtx.size() >= 1);
     }
@@ -114,7 +123,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const auto& node = g_setup->m_node;
-    auto& chainstate = node.chainman->ActiveChainstate();
+    auto& chainstate{static_cast<DummyChainState&>(node.chainman->ActiveChainstate())};
 
     MockTime(fuzzed_data_provider, chainstate);
     SetMempoolConstraints(*node.args, fuzzed_data_provider);
@@ -134,6 +143,8 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
     CTxMemPool tx_pool_{/* estimator */ nullptr, /* check_ratio */ 1};
     MockedTxPool& tx_pool = *static_cast<MockedTxPool*>(&tx_pool_);
 
+    chainstate.SetMempool(&tx_pool);
+
     // Helper to query an amount
     const CCoinsViewMemPool amount_view{WITH_LOCK(::cs_main, return &chainstate.CoinsTip()), tx_pool};
     const auto GetAmount = [&](const COutPoint& outpoint) {
@@ -222,13 +233,13 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
         // Make sure ProcessNewPackage on one transaction works and always fully validates the transaction.
         // The result is not guaranteed to be the same as what is returned by ATMP.
         const auto result_package = WITH_LOCK(::cs_main,
-                                    return ProcessNewPackage(node.chainman->ActiveChainstate(), tx_pool, {tx}, true));
+                                    return ProcessNewPackage(chainstate, tx_pool, {tx}, true));
         auto it = result_package.m_tx_results.find(tx->GetHash());
         Assert(it != result_package.m_tx_results.end());
         Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
                it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
 
-        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx_pool, tx, bypass_limits));
+        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         SyncWithValidationInterfaceQueue();
         UnregisterSharedValidationInterface(txr);
@@ -328,7 +339,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool)
         const auto tx = MakeTransactionRef(mut_tx);
         const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
         ::fRequireStandard = fuzzed_data_provider.ConsumeBool();
-        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(node.chainman->ActiveChainstate(), tx_pool, tx, bypass_limits));
+        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         if (accepted) {
             txids.push_back(tx->GetHash());

src/test/script_parse_tests.cpp

@@ -38,7 +38,6 @@ BOOST_AUTO_TEST_CASE(parse_script)
         {"'17'", "023137"},
         {"ELSE", "67"},
         {"NOP10", "b9"},
-        {"11111111111111111111", "00"},
     };
     std::string all_in;
     std::string all_out;
@@ -49,6 +48,7 @@ BOOST_AUTO_TEST_CASE(parse_script)
     }
     BOOST_CHECK_EQUAL(HexStr(ParseScript(all_in)), all_out);
 
+    BOOST_CHECK_EXCEPTION(ParseScript("11111111111111111111"), std::runtime_error, HasReason("script parse error: decimal numeric value only allowed in the range -0xFFFFFFFF...0xFFFFFFFF"));
     BOOST_CHECK_EXCEPTION(ParseScript("11111111111"), std::runtime_error, HasReason("script parse error: decimal numeric value only allowed in the range -0xFFFFFFFF...0xFFFFFFFF"));
     BOOST_CHECK_EXCEPTION(ParseScript("OP_CHECKSIGADD"), std::runtime_error, HasReason("script parse error: unknown opcode"));
 }

src/test/util_tests.cpp

@@ -1617,6 +1617,35 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32)
     BOOST_CHECK(!ParseInt32("32482348723847471234", nullptr));
 }
 
+template <typename T>
+static void RunToIntegralTests()
+{
+    BOOST_CHECK(!ToIntegral<T>(STRING_WITH_EMBEDDED_NULL_CHAR));
+    BOOST_CHECK(!ToIntegral<T>(" 1"));
+    BOOST_CHECK(!ToIntegral<T>("1 "));
+    BOOST_CHECK(!ToIntegral<T>("1a"));
+    BOOST_CHECK(!ToIntegral<T>("1.1"));
+    BOOST_CHECK(!ToIntegral<T>("1.9"));
+    BOOST_CHECK(!ToIntegral<T>("+01.9"));
+    BOOST_CHECK(!ToIntegral<T>("-"));
+    BOOST_CHECK(!ToIntegral<T>("+"));
+    BOOST_CHECK(!ToIntegral<T>(" -1"));
+    BOOST_CHECK(!ToIntegral<T>("-1 "));
+    BOOST_CHECK(!ToIntegral<T>(" -1 "));
+    BOOST_CHECK(!ToIntegral<T>("+1"));
+    BOOST_CHECK(!ToIntegral<T>(" +1"));
+    BOOST_CHECK(!ToIntegral<T>(" +1 "));
+    BOOST_CHECK(!ToIntegral<T>("+-1"));
+    BOOST_CHECK(!ToIntegral<T>("-+1"));
+    BOOST_CHECK(!ToIntegral<T>("++1"));
+    BOOST_CHECK(!ToIntegral<T>("--1"));
+    BOOST_CHECK(!ToIntegral<T>(""));
+    BOOST_CHECK(!ToIntegral<T>("aap"));
+    BOOST_CHECK(!ToIntegral<T>("0x1"));
+    BOOST_CHECK(!ToIntegral<T>("-32482348723847471234"));
+    BOOST_CHECK(!ToIntegral<T>("32482348723847471234"));
+}
+
 BOOST_AUTO_TEST_CASE(test_ToIntegral)
 {
     BOOST_CHECK_EQUAL(ToIntegral<int32_t>("1234").value(), 1'234);
@@ -1629,27 +1658,14 @@ BOOST_AUTO_TEST_CASE(test_ToIntegral)
     BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1234").value(), -1'234);
     BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1").value(), -1);
 
-    BOOST_CHECK(!ToIntegral<int32_t>(" 1"));
+    RunToIntegralTests<uint64_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>("1 "));
+    RunToIntegralTests<int64_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>("1a"));
+    RunToIntegralTests<uint32_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>("1.1"));
+    RunToIntegralTests<int32_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>("1.9"));
+    RunToIntegralTests<uint16_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>("+01.9"));
+    RunToIntegralTests<int16_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>(" -1"));
+    RunToIntegralTests<uint8_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>("-1 "));
+    RunToIntegralTests<int8_t>();
-    BOOST_CHECK(!ToIntegral<int32_t>(" -1 "));
-    BOOST_CHECK(!ToIntegral<int32_t>("+1"));
-    BOOST_CHECK(!ToIntegral<int32_t>(" +1"));
-    BOOST_CHECK(!ToIntegral<int32_t>(" +1 "));
-    BOOST_CHECK(!ToIntegral<int32_t>("+-1"));
-    BOOST_CHECK(!ToIntegral<int32_t>("-+1"));
-    BOOST_CHECK(!ToIntegral<int32_t>("++1"));
-    BOOST_CHECK(!ToIntegral<int32_t>("--1"));
-    BOOST_CHECK(!ToIntegral<int32_t>(""));
-    BOOST_CHECK(!ToIntegral<int32_t>("aap"));
-    BOOST_CHECK(!ToIntegral<int32_t>("0x1"));
-    BOOST_CHECK(!ToIntegral<int32_t>("-32482348723847471234"));
-    BOOST_CHECK(!ToIntegral<int32_t>("32482348723847471234"));
 
     BOOST_CHECK(!ToIntegral<int64_t>("-9223372036854775809"));
     BOOST_CHECK_EQUAL(ToIntegral<int64_t>("-9223372036854775808").value(), -9'223'372'036'854'775'807LL - 1LL);
@@ -1928,32 +1944,6 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt64)
     BOOST_CHECK(!ParseUInt64("-1234", &n));
 }
 
-BOOST_AUTO_TEST_CASE(test_ParseDouble)
-{
-    double n;
-    // Valid values
-    BOOST_CHECK(ParseDouble("1234", nullptr));
-    BOOST_CHECK(ParseDouble("0", &n) && n == 0.0);
-    BOOST_CHECK(ParseDouble("1234", &n) && n == 1234.0);
-    BOOST_CHECK(ParseDouble("01234", &n) && n == 1234.0); // no octal
-    BOOST_CHECK(ParseDouble("2147483647", &n) && n == 2147483647.0);
-    BOOST_CHECK(ParseDouble("-2147483648", &n) && n == -2147483648.0);
-    BOOST_CHECK(ParseDouble("-1234", &n) && n == -1234.0);
-    BOOST_CHECK(ParseDouble("1e6", &n) && n == 1e6);
-    BOOST_CHECK(ParseDouble("-1e6", &n) && n == -1e6);
-    // Invalid values
-    BOOST_CHECK(!ParseDouble("", &n));
-    BOOST_CHECK(!ParseDouble(" 1", &n)); // no padding inside
-    BOOST_CHECK(!ParseDouble("1 ", &n));
-    BOOST_CHECK(!ParseDouble("1a", &n));
-    BOOST_CHECK(!ParseDouble("aap", &n));
-    BOOST_CHECK(!ParseDouble("0x1", &n)); // no hex
-    BOOST_CHECK(!ParseDouble(STRING_WITH_EMBEDDED_NULL_CHAR, &n));
-    // Overflow and underflow
-    BOOST_CHECK(!ParseDouble("-1e10000", nullptr));
-    BOOST_CHECK(!ParseDouble("1e10000", nullptr));
-}
-
 BOOST_AUTO_TEST_CASE(test_FormatParagraph)
 {
     BOOST_CHECK_EQUAL(FormatParagraph("", 79, 0), "");

src/torcontrol.cpp

@@ -21,16 +21,13 @@
 #include <deque>
 #include <functional>
 #include <set>
-#include <stdlib.h>
 #include <vector>
 
-#include <boost/signals2/signal.hpp>
-
-#include <event2/bufferevent.h>
 #include <event2/buffer.h>
-#include <event2/util.h>
+#include <event2/bufferevent.h>
 #include <event2/event.h>
 #include <event2/thread.h>
+#include <event2/util.h>
 
 /** Default control port */
 const std::string DEFAULT_TOR_CONTROL = "127.0.0.1:9051";
@@ -273,9 +270,15 @@ std::map<std::string,std::string> ParseTorReplyMapping(const std::string &s)
                         if (j == 3 && value[i] > '3') {
                             j--;
                         }
-                        escaped_value.push_back(strtol(value.substr(i, j).c_str(), nullptr, 8));
+                        const auto end{i + j};
+                        uint8_t val{0};
+                        while (i < end) {
+                            val *= 8;
+                            val += value[i++] - '0';
+                        }
+                        escaped_value.push_back(char(val));
                         // Account for automatic incrementing at loop end
-                        i += j - 1;
+                        --i;
                     } else {
                         escaped_value.push_back(value[i]);
                     }

src/util/strencodings.cpp

@@ -260,16 +260,11 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid)
     return std::string((const char*)vchRet.data(), vchRet.size());
 }
 
-[[nodiscard]] static bool ParsePrechecks(const std::string&);
-
 namespace {
 template <typename T>
 bool ParseIntegral(const std::string& str, T* out)
 {
     static_assert(std::is_integral<T>::value);
-    if (!ParsePrechecks(str)) {
-        return false;
-    }
     // Replicate the exact behavior of strtol/strtoll/strtoul/strtoull when
     // handling leading +/- for backwards compatibility.
     if (str.length() >= 2 && str[0] == '+' && str[1] == '-') {
@@ -286,17 +281,6 @@ bool ParseIntegral(const std::string& str, T* out)
 }
 }; // namespace
 
-[[nodiscard]] static bool ParsePrechecks(const std::string& str)
-{
-    if (str.empty()) // No empty string allowed
-        return false;
-    if (str.size() >= 1 && (IsSpace(str[0]) || IsSpace(str[str.size()-1]))) // No padding allowed
-        return false;
-    if (!ValidAsCString(str)) // No embedded NUL characters allowed
-        return false;
-    return true;
-}
-
 bool ParseInt32(const std::string& str, int32_t* out)
 {
     return ParseIntegral<int32_t>(str, out);
@@ -327,20 +311,6 @@ bool ParseUInt64(const std::string& str, uint64_t* out)
     return ParseIntegral<uint64_t>(str, out);
 }
 
-bool ParseDouble(const std::string& str, double *out)
-{
-    if (!ParsePrechecks(str))
-        return false;
-    if (str.size() >= 2 && str[0] == '0' && str[1] == 'x') // No hexadecimal floats allowed
-        return false;
-    std::istringstream text(str);
-    text.imbue(std::locale::classic());
-    double result;
-    text >> result;
-    if(out) *out = result;
-    return text.eof() && !text.fail();
-}
-
 std::string FormatParagraph(const std::string& in, size_t width, size_t indent)
 {
     assert(width >= indent);

src/util/strencodings.h

@@ -90,7 +90,7 @@ void SplitHostPort(std::string in, uint16_t &portOut, std::string &hostOut);
 
 // LocaleIndependentAtoi is provided for backwards compatibility reasons.
 //
-// New code should use the ParseInt64/ParseUInt64/ParseInt32/ParseUInt32 functions
+// New code should use ToIntegral or the ParseInt* functions
 // which provide parse error feedback.
 //
 // The goal of LocaleIndependentAtoi is to replicate the exact defined behaviour
@@ -141,7 +141,9 @@ constexpr inline bool IsSpace(char c) noexcept {
 }
 
 /**
- * Convert string to integral type T.
+ * Convert string to integral type T. Leading whitespace, a leading +, or any
+ * trailing character fail the parsing. The required format expressed as regex
+ * is `-?[0-9]+`. The minus sign is only permitted for signed integer types.
  *
  * @returns std::nullopt if the entire string could not be parsed, or if the
  *   parsed value is not in the range representable by the type T.
@@ -155,7 +157,7 @@ std::optional<T> ToIntegral(const std::string& str)
     if (first_nonmatching != str.data() + str.size() || error_condition != std::errc{}) {
         return std::nullopt;
     }
-    return {result};
+    return result;
 }
 
 /**
@@ -200,13 +202,6 @@ std::optional<T> ToIntegral(const std::string& str)
  */
 [[nodiscard]] bool ParseUInt64(const std::string& str, uint64_t *out);
 
-/**
- * Convert string to double with strict parse error feedback.
- * @returns true if the entire string could be parsed as valid double,
- *   false if not the entire string could be parsed or when overflow or underflow occurred.
- */
-[[nodiscard]] bool ParseDouble(const std::string& str, double *out);
-
 /**
  * Convert a span of bytes to a lower-case hexadecimal string.
  */

src/validation.cpp

@@ -354,7 +354,9 @@ void CChainState::MaybeUpdateMempoolForReorg(
     while (it != disconnectpool.queuedTx.get<insertion_order>().rend()) {
         // ignore validation errors in resurrected transactions
         if (!fAddToMempool || (*it)->IsCoinBase() ||
-            AcceptToMemoryPool(*this, *m_mempool, *it, true /* bypass_limits */).m_result_type != MempoolAcceptResult::ResultType::VALID) {
+            AcceptToMemoryPool(*this, *it, GetTime(),
+                /*bypass_limits=*/true, /*test_accept=*/false).m_result_type !=
+                    MempoolAcceptResult::ResultType::VALID) {
             // If the transaction doesn't make it in to the mempool, remove any
             // transactions that depend on it (which would now be orphans).
             m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG);
@@ -691,6 +693,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     // to coins_to_uncache)
     m_view.SetBackend(m_dummy);
 
+    assert(m_active_chainstate.m_blockman.LookupBlockIndex(m_view.GetBestBlock()) == m_active_chainstate.m_chain.Tip());
+
     // Only accept BIP68 sequence locked transactions that can be mined in the next
     // block; we don't want our mempool filled up with transactions that can't
     // be mined yet.
@@ -699,7 +703,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     if (!CheckSequenceLocks(m_active_chainstate.m_chain.Tip(), m_view, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
         return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final");
 
-    if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_blockman.GetSpendHeight(m_view), ws.m_base_fees)) {
+    // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs
+    if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees)) {
         return false; // state filled in by CheckTxInputs
     }
 
@@ -978,16 +983,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
 
 } // anon namespace
 
-/** (try to) add transaction to memory pool with a specified acceptance time **/
+MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, const CTransactionRef& tx,
-static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CChainState& active_chainstate,
+                                       int64_t accept_time, bool bypass_limits, bool test_accept)
-                                                      const CTransactionRef &tx, int64_t nAcceptTime,
-                                                      bool bypass_limits, bool test_accept)
     EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
 {
-    AssertLockHeld(::cs_main);
+    const CChainParams& chainparams{active_chainstate.m_params};
-    std::vector<COutPoint> coins_to_uncache;
+    assert(active_chainstate.GetMempool() != nullptr);
-    MemPoolAccept::ATMPArgs args { chainparams, nAcceptTime, bypass_limits, coins_to_uncache, test_accept };
+    CTxMemPool& pool{*active_chainstate.GetMempool()};
 
+    std::vector<COutPoint> coins_to_uncache;
+    MemPoolAccept::ATMPArgs args { chainparams, accept_time, bypass_limits, coins_to_uncache, test_accept };
     const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args);
     if (result.m_result_type != MempoolAcceptResult::ResultType::VALID || test_accept) {
         if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
@@ -1008,11 +1013,6 @@ static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainp
     return result;
 }
 
-MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, const CTransactionRef &tx, bool bypass_limits, bool test_accept)
-{
-    return AcceptToMemoryPoolWithTime(Params(), pool, active_chainstate, tx, GetTime(), bypass_limits, test_accept);
-}
-
 PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTxMemPool& pool,
                                              const Package& package, bool test_accept)
 {
@@ -1232,12 +1232,12 @@ CChainState::CChainState(CTxMemPool* mempool,
                          const std::unique_ptr<llmq::CInstantSendManager>& isman,
                          std::optional<uint256> from_snapshot_blockhash)
     : m_mempool(mempool),
-      m_params(::Params()),
       m_chain_helper(chain_helper),
       m_clhandler(clhandler),
       m_isman(isman),
       m_evoDb(evoDb),
       m_blockman(blockman),
+      m_params(::Params()),
       m_chainman(chainman),
       m_from_snapshot_blockhash(from_snapshot_blockhash) {}
 
@@ -1407,14 +1407,6 @@ bool CScriptCheck::operator()() {
     return VerifyScript(scriptSig, m_tx_out.scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, txdata, cacheStore), &error);
 }
 
-int BlockManager::GetSpendHeight(const CCoinsViewCache& inputs)
-{
-    AssertLockHeld(cs_main);
-    CBlockIndex* pindexPrev = LookupBlockIndex(inputs.GetBestBlock());
-    return pindexPrev->nHeight + 1;
-}
-
-
 static CuckooCache::cache<uint256, SignatureCacheHasher> g_scriptExecutionCache;
 static CSHA256 g_scriptExecutionCacheHasher;
 
@@ -3996,13 +3988,13 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s
 MempoolAcceptResult ChainstateManager::ProcessTransaction(const CTransactionRef& tx, bool test_accept, bool bypass_limits)
 {
     CChainState& active_chainstate = ActiveChainstate();
-    if (!active_chainstate.m_mempool) {
+    if (!active_chainstate.GetMempool()) {
         TxValidationState state;
         state.Invalid(TxValidationResult::TX_NO_MEMPOOL, "no-mempool");
         return MempoolAcceptResult::Failure(state);
     }
-    auto result = AcceptToMemoryPool(active_chainstate, *active_chainstate.m_mempool, tx, bypass_limits, test_accept);
+    auto result = AcceptToMemoryPool(active_chainstate, tx, GetTime(), bypass_limits, test_accept);
-    active_chainstate.m_mempool->check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1);
+    active_chainstate.GetMempool()->check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1);
     return result;
 }
 
@@ -4966,7 +4958,6 @@ static const uint64_t MEMPOOL_DUMP_VERSION = 1;
 
 bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function)
 {
-    const CChainParams& chainparams = Params();
     int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60;
     FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat", "rb")};
     CAutoFile file(filestr, SER_DISK, CLIENT_VERSION);
@@ -5005,8 +4996,8 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka
             }
             if (nTime > nNow - nExpiryTimeout) {
                 LOCK(cs_main);
-                if (AcceptToMemoryPoolWithTime(chainparams, pool, active_chainstate, tx, nTime, false /* bypass_limits */,
+                const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false);
-                                               false /* test_accept */).m_result_type == MempoolAcceptResult::ResultType::VALID) {
+                if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) {
                     ++count;
                 } else {
                     // mempool may contain the transaction already, e.g. from
@@ -5285,6 +5276,17 @@ bool ChainstateManager::ActivateSnapshot(
     return true;
 }
 
+static void FlushSnapshotToDisk(CCoinsViewCache& coins_cache, bool snapshot_loaded)
+{
+    LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE(
+        strprintf("%s (%.2f MB)",
+                  snapshot_loaded ? "saving snapshot chainstate" : "flushing coins cache",
+                  coins_cache.DynamicMemoryUsage() / (1000 * 1000)),
+        BCLog::LogFlags::ALL);
+
+    coins_cache.Flush();
+}
+
 bool ChainstateManager::PopulateAndValidateSnapshot(
     CChainState& snapshot_chainstate,
     CAutoFile& coins_file,
@@ -5322,7 +5324,6 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
     uint64_t coins_left = metadata.m_coins_count;
 
     LogPrintf("[snapshot] loading coins from snapshot %s\n", base_blockhash.ToString());
-    int64_t flush_now{0};
     int64_t coins_processed{0};
 
     while (coins_left > 0) {
@@ -5366,19 +5367,14 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
             const auto snapshot_cache_state = WITH_LOCK(::cs_main,
                 return snapshot_chainstate.GetCoinsCacheSizeState());
 
-            if (snapshot_cache_state >=
+            if (snapshot_cache_state >= CoinsCacheSizeState::CRITICAL) {
-                    CoinsCacheSizeState::CRITICAL) {
-                LogPrintf("[snapshot] flushing coins cache (%.2f MB)... ", /* Continued */
-                    coins_cache.DynamicMemoryUsage() / (1000 * 1000));
-                flush_now = GetTimeMillis();
-
                 // This is a hack - we don't know what the actual best block is, but that
                 // doesn't matter for the purposes of flushing the cache here. We'll set this
                 // to its correct value (`base_blockhash`) below after the coins are loaded.
                 coins_cache.SetBestBlock(GetRandHash());
 
-                coins_cache.Flush();
+                // No need to acquire cs_main since this chainstate isn't being used yet.
-                LogPrintf("done (%.2fms)\n", GetTimeMillis() - flush_now);
+                FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/false);
             }
         }
     }
@@ -5408,9 +5404,8 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
         coins_cache.DynamicMemoryUsage() / (1000 * 1000),
         base_blockhash.ToString());
 
-    LogPrintf("[snapshot] flushing snapshot chainstate to disk\n");
     // No need to acquire cs_main since this chainstate isn't being used yet.
-    coins_cache.Flush(); // TODO: if #17487 is merged, add erase=false here for better performance.
+    FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/true);
 
     assert(coins_cache.GetBestBlock() == base_blockhash);
 

src/validation.h

@@ -240,19 +240,21 @@ struct PackageMempoolAcceptResult
 };
 
 /**
- * Try to add a transaction to the mempool. This is an internal function and is
+ * Try to add a transaction to the mempool. This is an internal function and is exposed only for testing.
- * exposed only for testing. Client code should use ChainstateManager::ProcessTransaction()
+ * Client code should use ChainstateManager::ProcessTransaction()
  *
  * @param[in]  active_chainstate  Reference to the active chainstate.
- * @param[in]  pool               Reference to the node's mempool.
  * @param[in]  tx                 The transaction to submit for mempool acceptance.
+ * @param[in]  accept_time        The timestamp for adding the transaction to the mempool.
+ *                                It is also used to determine when the entry expires.
  * @param[in]  bypass_limits      When true, don't enforce mempool fee and capacity limits.
  * @param[in]  test_accept        When true, run validation checks but don't submit to mempool.
  *
  * @returns a MempoolAcceptResult indicating whether the transaction was accepted/rejected with reason.
  */
-MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, const CTransactionRef& tx,
+MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, const CTransactionRef& tx,
-                                       bool bypass_limits, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+                                       int64_t accept_time, bool bypass_limits, bool test_accept)
+    EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 /**
 * Atomically test acceptance of a package. If the package only contains one tx, package rules still
@@ -486,8 +488,6 @@ protected:
     //! Only the active chainstate has a mempool.
     CTxMemPool* m_mempool;
 
-    const CChainParams& m_params;
-
     //! Manages the UTXO set, which is a reflection of the contents of `m_chain`.
     std::unique_ptr<CoinsViews> m_coins_views;
 
@@ -502,6 +502,9 @@ public:
     //! CChainState instances.
     BlockManager& m_blockman;
 
+    /** Chain parameters for this chainstate */
+    const CChainParams& m_params;
+
     //! The chainstate manager that owns this chainstate. The reference is
     //! necessary so that this instance can check whether it is the active
     //! chainstate within deeply nested method calls.
@@ -584,6 +587,12 @@ public:
         return m_coins_views->m_dbview;
     }
 
+    //! @returns A pointer to the mempool.
+    CTxMemPool* GetMempool()
+    {
+        return m_mempool;
+    }
+
     //! @returns A reference to a wrapped view of the in-memory UTXO set that
     //!     handles disk read errors gracefully.
     CCoinsViewErrorCatcher& CoinsErrorCatcher() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)

test/functional/feature_llmq_dkgerrors.py

@@ -25,18 +25,18 @@ class LLMQDKGErrors(DashTestFramework):
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)
 
         self.log.info("Lets omit the contribution")
-        self.mninfo[0].node.quorum('dkgsimerror', 'contribution-omit', '1')
+        self.mninfo[0].node.quorum('dkgsimerror', 'contribution-omit', '100')
         qh = self.mine_quorum(expected_contributions=2)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, False)
 
         self.log.info("Lets lie in the contribution but provide a correct justification")
         self.mninfo[0].node.quorum('dkgsimerror', 'contribution-omit', '0')
-        self.mninfo[0].node.quorum('dkgsimerror', 'contribution-lie', '1')
+        self.mninfo[0].node.quorum('dkgsimerror', 'contribution-lie', '100')
         qh = self.mine_quorum(expected_contributions=3, expected_complaints=2, expected_justifications=1)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)
 
         self.log.info("Lets lie in the contribution and then omit the justification")
-        self.mninfo[0].node.quorum('dkgsimerror', 'justify-omit', '1')
+        self.mninfo[0].node.quorum('dkgsimerror', 'justify-omit', '100')
         qh = self.mine_quorum(expected_contributions=3, expected_complaints=2)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, False)
 
@@ -45,26 +45,26 @@ class LLMQDKGErrors(DashTestFramework):
 
         self.log.info("Lets lie in the contribution and then also lie in the justification")
         self.mninfo[0].node.quorum('dkgsimerror', 'justify-omit', '0')
-        self.mninfo[0].node.quorum('dkgsimerror', 'justify-lie', '1')
+        self.mninfo[0].node.quorum('dkgsimerror', 'justify-lie', '100')
         qh = self.mine_quorum(expected_contributions=3, expected_complaints=2, expected_justifications=1)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, False)
 
         self.log.info("Lets lie about another MN")
         self.mninfo[0].node.quorum('dkgsimerror', 'contribution-lie', '0')
         self.mninfo[0].node.quorum('dkgsimerror', 'justify-lie', '0')
-        self.mninfo[0].node.quorum('dkgsimerror', 'complain-lie', '1')
+        self.mninfo[0].node.quorum('dkgsimerror', 'complain-lie', '100')
         qh = self.mine_quorum(expected_contributions=3, expected_complaints=1, expected_justifications=2)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)
 
         self.log.info("Lets omit 1 premature commitments")
         self.mninfo[0].node.quorum('dkgsimerror', 'complain-lie', '0')
-        self.mninfo[0].node.quorum('dkgsimerror', 'commit-omit', '1')
+        self.mninfo[0].node.quorum('dkgsimerror', 'commit-omit', '100')
         qh = self.mine_quorum(expected_contributions=3, expected_complaints=0, expected_justifications=0, expected_commitments=2)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)
 
         self.log.info("Lets lie in 1 premature commitments")
         self.mninfo[0].node.quorum('dkgsimerror', 'commit-omit', '0')
-        self.mninfo[0].node.quorum('dkgsimerror', 'commit-lie', '1')
+        self.mninfo[0].node.quorum('dkgsimerror', 'commit-lie', '100')
         qh = self.mine_quorum(expected_contributions=3, expected_complaints=0, expected_justifications=0, expected_commitments=2)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)
 

test/functional/interface_rest.py

@@ -45,7 +45,7 @@ class RESTTest (BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 2
-        self.extra_args = [["-rest"], []]
+        self.extra_args = [["-rest", "-blockfilterindex=1"], []]
         self.supports_cli = False
 
     def skip_test_if_missing_module(self):
@@ -282,6 +282,16 @@ class RESTTest (BitcoinTestFramework):
         self.generate(self.nodes[1], 5)
         json_obj = self.test_rest_request("/headers/5/{}".format(bb_hash))
         assert_equal(len(json_obj), 5)  # now we should have 5 header objects
+        json_obj = self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}")
+        assert_equal(len(json_obj), 5)  # now we should have 5 filter header objects
+        self.test_rest_request(f"/blockfilter/basic/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ)
+
+        # Test number parsing
+        for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']:
+            assert_equal(
+                bytes(f'Header count out of acceptable range (1-2000): {num}\r\n', 'ascii'),
+                self.test_rest_request(f"/headers/{num}/{bb_hash}", ret_type=RetType.BYTES, status=400),
+            )
 
         self.log.info("Test tx inclusion in the /mempool and /block URIs")
 

test/lint/lint-locale-dependence.sh

@@ -44,11 +44,9 @@ export LC_ALL=C
 KNOWN_VIOLATIONS=(
     "src/bitcoin-tx.cpp.*stoul"
     "src/dbwrapper.cpp:.*vsnprintf"
-    "src/rest.cpp:.*strtol"
     "src/test/dbwrapper_tests.cpp:.*snprintf"
     "src/test/fuzz/locale.cpp"
     "src/test/fuzz/string.cpp"
-    "src/torcontrol.cpp:.*strtol"
     "src/util/strencodings.cpp:.*strtoll"
     "src/util/system.cpp:.*fprintf"
 )

test/util/data/bitcoin-util-test.json

@@ -235,6 +235,12 @@
     "output_cmp": "txcreate2.json",
     "description": "Parses a transaction with no inputs and a single output script (output in json)"
   },
+  { "exec": "./dash-tx",
+    "args": ["-create", "outscript=0:999999999999999999999999999999"],
+    "return_code": 1,
+    "error_txt": "error: script parse error: decimal numeric value only allowed in the range -0xFFFFFFFF...0xFFFFFFFF",
+    "description": "Try to parse an output script with a decimal number above the allowed range"
+  },
   { "exec": "./dash-tx",
     "args": ["-create", "outscript=0:9999999999"],
     "return_code": 1,