Merge pull request #5310 from PastaPastaPasta/develop-trivial-2023-04-09

backport: trivial backports 2023 04 09

depends/Makefile

@@ -110,37 +110,31 @@ include builders/$(build_os).mk
 include builders/default.mk
 include packages/packages.mk
 
-full_env=$(shell printenv)
-
-build_id_string:=$(BUILD_ID_SALT)
-
-# GCC only prints COLLECT_LTO_WRAPPER when invoked with just "-v", but we want
-# the information from "-v -E -" as well, so just include both.
+# Previously, we directly invoked the well-known programs using $(shell ...)
+# to contruct build_id_string. However, that was problematic because:
 #
-# '3>&1 1>&2 2>&3 > /dev/null' is supposed to swap stdin and stdout and silence
-# stdin, since we only want the stderr output
-build_id_string+=$(shell $(build_CC) -v < /dev/null 3>&1 1>&2 2>&3 > /dev/null) $(shell $(build_CC) -v -E - < /dev/null 3>&1 1>&2 2>&3 > /dev/null)
-build_id_string+=$(shell $(build_AR) --version 2>/dev/null) $(filter AR_%,$(full_env)) ZERO_AR_DATE=$(ZERO_AR_DATE)
-build_id_string+=$(shell $(build_CXX) -v < /dev/null 3>&1 1>&2 2>&3 > /dev/null) $(shell $(build_CXX) -v -E - < /dev/null 3>&1 1>&2 2>&3 > /dev/null)
-build_id_string+=$(shell $(build_RANLIB) --version 2>/dev/null) $(filter RANLIB_%,$(full_env))
-build_id_string+=$(shell $(build_STRIP) --version 2>/dev/null) $(filter STRIP_%,$(full_env))
-
-$(host_arch)_$(host_os)_id_string:=$(HOST_ID_SALT)
-$(host_arch)_$(host_os)_id_string+=$(shell $(host_CC) -v < /dev/null 3>&1 1>&2 2>&3 > /dev/null) $(shell $(host_CC) -v -E - < /dev/null 3>&1 1>&2 2>&3 > /dev/null)
-$(host_arch)_$(host_os)_id_string+=$(shell $(host_AR) --version 2>/dev/null) $(filter AR_%,$(full_env)) ZERO_AR_DATE=$(ZERO_AR_DATE)
-$(host_arch)_$(host_os)_id_string+=$(shell $(host_CXX) -v < /dev/null 3>&1 1>&2 2>&3 > /dev/null) $(shell $(host_CXX) -v -E - < /dev/null 3>&1 1>&2 2>&3 > /dev/null)
-$(host_arch)_$(host_os)_id_string+=$(shell $(host_RANLIB) --version 2>/dev/null) $(filter RANLIB_%,$(full_env))
-$(host_arch)_$(host_os)_id_string+=$(shell $(host_STRIP) --version 2>/dev/null) $(filter STRIP_%,$(full_env))
-
-ifneq ($(strip $(FORCE_USE_SYSTEM_CLANG)),)
-# Make sure that cache is invalidated when switching between system and
-# depends-managed, pinned clang
-build_id_string+=system_clang
-$(host_arch)_$(host_os)_id_string+=system_clang
-endif
-
-build_id_string+=GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))
-$(host_arch)_$(host_os)_id_string+=GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))
+# 1. When invoking a shell, GNU Make special-cases exit code 127 (command not
+#    found) by not capturing the output but instead passing it through. This is
+#    not done for any other exit code.
+#
+# 2. Characters like '#' (from these programs' output) would end up in make
+#    variables like build_id_string, which would be wrongly interpreted by make
+#    when these variables were used.
+#
+# Therefore, we should avoid having arbitrary strings in make variables where
+# possible. The gen_id script used here hashes the output to construct a
+# "make-safe" id.
+#
+# Also note that these lines need to be:
+#
+#     1. After including {hosts,builders}/*.mk, since they rely on the tool
+#        variables (e.g. build_CC, host_STRIP, etc.) to be set.
+#
+#     2. Before including packages/*.mk (excluding packages/packages.mk), since
+#        they rely on the build_id variables
+#
+build_id:=$(shell env CC='$(build_CC)' CXX='$(build_CXX)' AR='$(build_AR)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))')
+$(host_arch)_$(host_os)_id:=$(shell env CC='$(host_CC)' CXX='$(host_CXX)' AR='$(host_AR)' RANLIB='$(host_RANLIB)' STRIP='$(host_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' ./gen_id '$(HOST_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))')
 
 qrencode_packages_$(NO_QR) = $(qrencode_$(host_os)_packages)
 

depends/funcs.mk

@@ -51,7 +51,7 @@ define int_get_build_id
 $(eval $(1)_dependencies += $($(1)_$(host_arch)_$(host_os)_dependencies) $($(1)_$(host_os)_dependencies))
 $(eval $(1)_all_dependencies:=$(call int_get_all_dependencies,$(1),$($($(1)_type)_native_toolchain) $($($(1)_type)_native_binutils) $($(1)_dependencies)))
 $(foreach dep,$($(1)_all_dependencies),$(eval $(1)_build_id_deps+=$(dep)-$($(dep)_version)-$($(dep)_recipe_hash)))
-$(eval $(1)_build_id_long:=$(1)-$($(1)_version)-$($(1)_recipe_hash)-$(release_type) $($(1)_build_id_deps) $($($(1)_type)_id_string))
+$(eval $(1)_build_id_long:=$(1)-$($(1)_version)-$($(1)_recipe_hash)-$(release_type) $($(1)_build_id_deps) $($($(1)_type)_id))
 $(eval $(1)_build_id:=$(shell echo -n "$($(1)_build_id_long)" | $(build_SHA256SUM) | cut -c-$(HASH_LENGTH)))
 final_build_id_long+=$($(package)_build_id_long)
 

depends/gen_id

@@ -0,0 +1,74 @@
+#!/usr/bin/env bash
+
+# Usage: env [ CC=... ] [ CXX=... ] [ AR=... ] [ RANLIB=... ] [ STRIP=... ] \
+#            [ DEBUG=... ] ./build-id [ID_SALT]...
+#
+# Prints to stdout a SHA256 hash representing the current toolset, used by
+# depends/Makefile as a build id for caching purposes (detecting when the
+# toolset has changed and the cache needs to be invalidated).
+#
+# If the DEBUG environment variable is non-empty and the system has `tee`
+# available in its $PATH, the pre-image to the SHA256 hash will be printed to
+# stderr. This is to help developers debug caching issues in depends.
+
+# This script explicitly does not `set -e` because id determination is mostly
+# opportunistic: it is fine that things fail, as long as they fail consistently.
+
+# Command variables (CC/CXX/AR) which can be blank are invoked with `bash -c`,
+# because the "command not found" error message printed by shells often include
+# the line number, like so:
+#
+#     ./depends/gen_id: line 43: --version: command not found
+#
+# By invoking with `bash -c`, we ensure that the line number is always 1
+
+(
+    # Redirect stderr to stdout
+    exec 2>&1
+
+    echo "BEGIN ALL"
+
+    # Include any ID salts supplied via command line
+    echo "BEGIN ID SALT"
+    echo "$@"
+    echo "END ID SALT"
+
+    # GCC only prints COLLECT_LTO_WRAPPER when invoked with just "-v", but we want
+    # the information from "-v -E -" as well, so just include both.
+    echo "BEGIN CC"
+    bash -c "${CC} -v"
+    bash -c "${CC} -v -E -xc -o /dev/null - < /dev/null"
+    bash -c "${CC} -v -E -xobjective-c -o /dev/null - < /dev/null"
+    echo "END CC"
+
+    echo "BEGIN CXX"
+    bash -c "${CXX} -v"
+    bash -c "${CXX} -v -E -xc++ -o /dev/null - < /dev/null"
+    bash -c "${CXX} -v -E -xobjective-c++ -o /dev/null - < /dev/null"
+    echo "END CXX"
+
+    echo "BEGIN AR"
+    bash -c "${AR} --version"
+    env | grep '^AR_'
+    echo "ZERO_AR_DATE=${ZERO_AR_DATE}"
+    echo "END AR"
+
+    echo "BEGIN RANLIB"
+    bash -c "${RANLIB} --version"
+    env | grep '^RANLIB_'
+    echo "END RANLIB"
+
+    echo "BEGIN STRIP"
+    bash -c "${STRIP} --version"
+    env | grep '^STRIP_'
+    echo "END STRIP"
+
+    echo "END ALL"
+) | if [ -n "$DEBUG" ] && command -v tee > /dev/null 2>&1; then
+        # When debugging and `tee` is available, output the preimage to stderr
+        # in addition to passing through stdin to stdout
+        tee >(cat 1>&2)
+    else
+        # Otherwise, passthrough stdin to stdout
+        cat
+    fi | ${SHA256SUM} - | cut -d' ' -f1

share/setup.nsi.in

@@ -3,6 +3,7 @@ Name "@PACKAGE_NAME@ (64-bit)"
 RequestExecutionLevel highest
 SetCompressor /SOLID lzma
 SetDateSave off
+Unicode true
 
 # Uncomment these lines when investigating reproducibility errors
 #SetCompress off

src/bitcoin-cli.cpp

@@ -409,6 +409,8 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co
         } else {
             throw std::runtime_error("Authorization failed: Incorrect rpcuser or rpcpassword");
         }
+    } else if (response.status == HTTP_SERVICE_UNAVAILABLE) {
+        throw std::runtime_error(strprintf("Server response: %s", response.body));
     } else if (response.status >= 400 && response.status != HTTP_BAD_REQUEST && response.status != HTTP_NOT_FOUND && response.status != HTTP_INTERNAL_SERVER_ERROR)
         throw std::runtime_error(strprintf("server returned HTTP error %d", response.status));
     else if (response.body.empty())

src/compressor.cpp

@@ -52,7 +52,7 @@ static bool IsToPubKey(const CScript& script, CPubKey &pubkey)
     return false;
 }
 
-bool CompressScript(const CScript& script, std::vector<unsigned char> &out)
+bool CompressScript(const CScript& script, CompressedScript& out)
 {
     CKeyID keyID;
     if (IsToKeyID(script, keyID)) {
@@ -92,7 +92,7 @@ unsigned int GetSpecialScriptSize(unsigned int nSize)
     return 0;
 }
 
-bool DecompressScript(CScript& script, unsigned int nSize, const std::vector<unsigned char> &in)
+bool DecompressScript(CScript& script, unsigned int nSize, const CompressedScript& in)
 {
     switch(nSize) {
     case 0x00:

src/compressor.h

@@ -6,14 +6,26 @@
 #ifndef BITCOIN_COMPRESSOR_H
 #define BITCOIN_COMPRESSOR_H
 
+#include <prevector.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
 #include <serialize.h>
 #include <span.h>
 
-bool CompressScript(const CScript& script, std::vector<unsigned char> &out);
+/**
+ * This saves us from making many heap allocations when serializing
+ * and deserializing compressed scripts.
+ *
+ * This prevector size is determined by the largest .resize() in the
+ * CompressScript function. The largest compressed script format is a
+ * compressed public key, which is 33 bytes.
+ */
+using CompressedScript = prevector<33, unsigned char>;
+
+
+bool CompressScript(const CScript& script, CompressedScript& out);
 unsigned int GetSpecialScriptSize(unsigned int nSize);
-bool DecompressScript(CScript& script, unsigned int nSize, const std::vector<unsigned char> &out);
+bool DecompressScript(CScript& script, unsigned int nSize, const CompressedScript& in);
 
 /**
  * Compress amount.
@@ -51,7 +63,7 @@ struct ScriptCompression
 
     template<typename Stream>
     void Ser(Stream &s, const CScript& script) {
-        std::vector<unsigned char> compr;
+        CompressedScript compr;
         if (CompressScript(script, compr)) {
             s << MakeSpan(compr);
             return;
@@ -66,7 +78,7 @@ struct ScriptCompression
         unsigned int nSize = 0;
         s >> VARINT(nSize);
         if (nSize < nSpecialScripts) {
-            std::vector<unsigned char> vch(GetSpecialScriptSize(nSize), 0x00);
+            CompressedScript vch(GetSpecialScriptSize(nSize), 0x00);
             s >> MakeSpan(vch);
             DecompressScript(script, nSize, vch);
             return;

src/httpserver.cpp

@@ -267,7 +267,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
             item.release(); /* if true, queue took ownership */
         } else {
             LogPrintf("WARNING: request rejected because http work queue depth exceeded, it can be increased with the -rpcworkqueue= setting\n");
-            item->req->WriteReply(HTTP_INTERNAL_SERVER_ERROR, "Work queue depth exceeded");
+            item->req->WriteReply(HTTP_SERVICE_UNAVAILABLE, "Work queue depth exceeded");
         }
     } else {
         hreq->WriteReply(HTTP_NOT_FOUND);

src/init.cpp

@@ -1640,7 +1640,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
         LogPrintf("Config file: %s\n", config_file_path.string());
     } else if (args.IsArgSet("-conf")) {
         // Warn if no conf file exists at path provided by user
-        InitWarning(strprintf(_("The specified config file %s does not exist\n"), config_file_path.string()));
+        InitWarning(strprintf(_("The specified config file %s does not exist"), config_file_path.string()));
     } else {
         // Not categorizing as "Warning" because it's the default behavior
         LogPrintf("Config file: %s (not found, skipping)\n", config_file_path.string());

src/protocol.cpp

@@ -209,7 +209,6 @@ CMessageHeader::CMessageHeader()
 {
     memset(pchMessageStart, 0, MESSAGE_START_SIZE);
     memset(pchCommand, 0, sizeof(pchCommand));
-    nMessageSize = -1;
     memset(pchChecksum, 0, CHECKSUM_SIZE);
 }
 

src/protocol.h

@@ -15,6 +15,7 @@
 #include <uint256.h>
 #include <version.h>
 
+#include <limits>
 #include <stdint.h>
 #include <string>
 
@@ -50,7 +51,7 @@ public:
 
     char pchMessageStart[MESSAGE_START_SIZE];
     char pchCommand[COMMAND_SIZE];
-    uint32_t nMessageSize;
+    uint32_t nMessageSize{std::numeric_limits<uint32_t>::max()};
     uint8_t pchChecksum[CHECKSUM_SIZE];
 };
 

src/test/compress_tests.cpp

@@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id)
     CScript script = CScript() << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
     BOOST_CHECK_EQUAL(script.size(), 25);
 
-    std::vector<unsigned char> out;
+    CompressedScript out;
     bool done = CompressScript(script, out);
     BOOST_CHECK_EQUAL(done, true);
 
@@ -89,7 +89,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id)
     script << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
     BOOST_CHECK_EQUAL(script.size(), 23);
 
-    std::vector<unsigned char> out;
+    CompressedScript out;
     bool done = CompressScript(script, out);
     BOOST_CHECK_EQUAL(done, true);
 
@@ -107,7 +107,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id)
     CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // COMPRESSED_PUBLIC_KEY_SIZE (33)
     BOOST_CHECK_EQUAL(script.size(), 35);
 
-    std::vector<unsigned char> out;
+    CompressedScript out;
     bool done = CompressScript(script, out);
     BOOST_CHECK_EQUAL(done, true);
 
@@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id)
     CScript script =  CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // PUBLIC_KEY_SIZE (65)
     BOOST_CHECK_EQUAL(script.size(), 67);                   // 1 char code + 65 char pubkey + OP_CHECKSIG
 
-    std::vector<unsigned char> out;
+    CompressedScript out;
     bool done = CompressScript(script, out);
     BOOST_CHECK_EQUAL(done, true);
 

src/test/fuzz/parse_numbers.cpp

@@ -20,6 +20,9 @@ FUZZ_TARGET(parse_numbers)
     uint8_t u8;
     (void)ParseUInt8(random_string, &u8);
 
+    uint16_t u16;
+    (void)ParseUInt16(random_string, &u16);
+
     int32_t i32;
     (void)ParseInt32(random_string, &i32);
     (void)LocaleIndependentAtoi<int>(random_string);

src/test/fuzz/script.cpp

@@ -43,7 +43,7 @@ FUZZ_TARGET_INIT(script, initialize_script)
     if (!script_opt) return;
     const CScript script{*script_opt};
 
-    std::vector<unsigned char> compressed;
+    CompressedScript compressed;
     if (CompressScript(script, compressed)) {
         const unsigned int size = compressed[0];
         assert(size <= 5);
@@ -93,10 +93,12 @@ FUZZ_TARGET_INIT(script, initialize_script)
 
     {
         const std::vector<uint8_t> bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider);
+        CompressedScript compressed_script;
+        compressed_script.assign(bytes.begin(), bytes.end());
         // DecompressScript(..., ..., bytes) is not guaranteed to be defined if the bytes vector is too short
-        if (bytes.size() >= 32) {
+        if (compressed_script.size() >= 32) {
             CScript decompressed_script;
-            DecompressScript(decompressed_script, fuzzed_data_provider.ConsumeIntegral<unsigned int>(), bytes);
+            DecompressScript(decompressed_script, fuzzed_data_provider.ConsumeIntegral<unsigned int>(), compressed_script);
         }
     }
 

src/test/fuzz/strprintf.cpp

@@ -48,67 +48,6 @@ FUZZ_TARGET(str_printf)
     //
     // Upstream bug report: https://github.com/c42f/tinyformat/issues/70
 
-    try {
-        (void)strprintf(format_string, (signed char*)nullptr);
-        (void)tinyformat::format(bilingual_string, (signed char*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (unsigned char*)nullptr);
-        (void)tinyformat::format(bilingual_string, (unsigned char*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (void*)nullptr);
-        (void)tinyformat::format(bilingual_string, (void*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (bool*)nullptr);
-        (void)tinyformat::format(bilingual_string, (bool*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (float*)nullptr);
-        (void)tinyformat::format(bilingual_string, (float*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (double*)nullptr);
-        (void)tinyformat::format(bilingual_string, (double*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (int16_t*)nullptr);
-        (void)tinyformat::format(bilingual_string, (int16_t*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (uint16_t*)nullptr);
-        (void)tinyformat::format(bilingual_string, (uint16_t*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (int32_t*)nullptr);
-        (void)tinyformat::format(bilingual_string, (int32_t*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (uint32_t*)nullptr);
-        (void)tinyformat::format(bilingual_string, (uint32_t*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (int64_t*)nullptr);
-        (void)tinyformat::format(bilingual_string, (int64_t*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-    try {
-        (void)strprintf(format_string, (uint64_t*)nullptr);
-        (void)tinyformat::format(bilingual_string, (uint64_t*)nullptr);
-    } catch (const tinyformat::format_error&) {
-    }
-
     try {
         CallOneOf(
             fuzzed_data_provider,

src/test/fuzz/timedata.cpp

@@ -15,10 +15,12 @@ FUZZ_TARGET(timedata)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const unsigned int max_size = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 1000);
+    // A max_size of 0 implies no limit, so cap the max number of insertions to avoid timeouts
+    auto max_to_insert = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 4000);
     // Divide by 2 to avoid signed integer overflow in .median()
     const int64_t initial_value = fuzzed_data_provider.ConsumeIntegral<int64_t>() / 2;
     CMedianFilter<int64_t> median_filter{max_size, initial_value};
-    while (fuzzed_data_provider.remaining_bytes() > 0) {
+    while (fuzzed_data_provider.remaining_bytes() > 0 && --max_to_insert >= 0) {
         (void)median_filter.median();
         assert(median_filter.size() > 0);
         assert(static_cast<size_t>(median_filter.size()) == median_filter.sorted().size());

src/test/fuzz/util.h

@@ -10,6 +10,7 @@
 #include <attributes.h>
 #include <chainparamsbase.h>
 #include <coins.h>
+#include <compat.h>
 #include <consensus/consensus.h>
 #include <merkleblock.h>
 #include <net.h>
@@ -518,14 +519,22 @@ public:
     {
     }
 
+    FuzzedSock& operator=(Sock&& other) override
+    {
+        assert(false && "Not implemented yet.");
+        return *this;
+    }
+
     SOCKET Get() const override
     {
         assert(false && "Not implemented yet.");
+        return INVALID_SOCKET;
     }
 
     SOCKET Release() override
     {
         assert(false && "Not implemented yet.");
+        return INVALID_SOCKET;
     }
 
     void Reset() override

src/test/util_tests.cpp

@@ -1306,7 +1306,6 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32)
     BOOST_CHECK(!ParseInt32("1a", &n));
     BOOST_CHECK(!ParseInt32("aap", &n));
     BOOST_CHECK(!ParseInt32("0x1", &n)); // no hex
-    BOOST_CHECK(!ParseInt32("0x1", &n)); // no hex
     BOOST_CHECK(!ParseInt32(STRING_WITH_EMBEDDED_NULL_CHAR, &n));
     // Overflow and underflow
     BOOST_CHECK(!ParseInt32("-2147483649", nullptr));
@@ -1515,7 +1514,6 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt8)
     BOOST_CHECK(!ParseUInt8("1a", &n));
     BOOST_CHECK(!ParseUInt8("aap", &n));
     BOOST_CHECK(!ParseUInt8("0x1", &n)); // no hex
-    BOOST_CHECK(!ParseUInt8("0x1", &n)); // no hex
     BOOST_CHECK(!ParseUInt8(STRING_WITH_EMBEDDED_NULL_CHAR, &n));
     // Overflow and underflow
     BOOST_CHECK(!ParseUInt8("-255", &n));
@@ -1525,6 +1523,41 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt8)
     BOOST_CHECK(!ParseUInt8("256", nullptr));
 }
 
+BOOST_AUTO_TEST_CASE(test_ParseUInt16)
+{
+    uint16_t n;
+    // Valid values
+    BOOST_CHECK(ParseUInt16("1234", nullptr));
+    BOOST_CHECK(ParseUInt16("0", &n) && n == 0);
+    BOOST_CHECK(ParseUInt16("1234", &n) && n == 1234);
+    BOOST_CHECK(ParseUInt16("01234", &n) && n == 1234); // no octal
+    BOOST_CHECK(ParseUInt16("65535", &n) && n == static_cast<uint16_t>(65535));
+    BOOST_CHECK(ParseUInt16("+65535", &n) && n == 65535);
+    BOOST_CHECK(ParseUInt16("00000000000000000012", &n) && n == 12);
+    BOOST_CHECK(ParseUInt16("00000000000000000000", &n) && n == 0);
+    // Invalid values
+    BOOST_CHECK(!ParseUInt16("-00000000000000000000", &n));
+    BOOST_CHECK(!ParseUInt16("", &n));
+    BOOST_CHECK(!ParseUInt16(" 1", &n)); // no padding inside
+    BOOST_CHECK(!ParseUInt16(" -1", &n));
+    BOOST_CHECK(!ParseUInt16("++1", &n));
+    BOOST_CHECK(!ParseUInt16("+-1", &n));
+    BOOST_CHECK(!ParseUInt16("-+1", &n));
+    BOOST_CHECK(!ParseUInt16("--1", &n));
+    BOOST_CHECK(!ParseUInt16("-1", &n));
+    BOOST_CHECK(!ParseUInt16("1 ", &n));
+    BOOST_CHECK(!ParseUInt16("1a", &n));
+    BOOST_CHECK(!ParseUInt16("aap", &n));
+    BOOST_CHECK(!ParseUInt16("0x1", &n)); // no hex
+    BOOST_CHECK(!ParseUInt16(STRING_WITH_EMBEDDED_NULL_CHAR, &n));
+    // Overflow and underflow
+    BOOST_CHECK(!ParseUInt16("-65535", &n));
+    BOOST_CHECK(!ParseUInt16("65536", &n));
+    BOOST_CHECK(!ParseUInt16("-123", &n));
+    BOOST_CHECK(!ParseUInt16("-123", nullptr));
+    BOOST_CHECK(!ParseUInt16("65536", nullptr));
+}
+
 BOOST_AUTO_TEST_CASE(test_ParseUInt32)
 {
     uint32_t n;
@@ -1553,7 +1586,6 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt32)
     BOOST_CHECK(!ParseUInt32("1a", &n));
     BOOST_CHECK(!ParseUInt32("aap", &n));
     BOOST_CHECK(!ParseUInt32("0x1", &n)); // no hex
-    BOOST_CHECK(!ParseUInt32("0x1", &n)); // no hex
     BOOST_CHECK(!ParseUInt32(STRING_WITH_EMBEDDED_NULL_CHAR, &n));
     // Overflow and underflow
     BOOST_CHECK(!ParseUInt32("-2147483648", &n));

src/txmempool.cpp

@@ -1387,7 +1387,7 @@ std::optional<CTxMemPool::txiter> CTxMemPool::GetIter(const uint256& txid) const
 {
     auto it = mapTx.find(txid);
     if (it != mapTx.end()) return it;
-    return {};
+    return std::nullopt;
 }
 
 CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>& hashes) const

src/validation.cpp

@@ -5604,7 +5604,7 @@ std::optional<uint256> ChainstateManager::SnapshotBlockhash() const {
         // If a snapshot chainstate exists, it will always be our active.
         return m_active_chainstate->m_from_snapshot_blockhash;
     }
-    return {};
+    return std::nullopt;
 }
 
 std::vector<CChainState*> ChainstateManager::GetAll()

src/wallet/db.cpp

@@ -18,7 +18,12 @@ std::vector<fs::path> ListDatabases(const fs::path& wallet_dir)
 
     for (auto it = fs::recursive_directory_iterator(wallet_dir, ec); it != fs::recursive_directory_iterator(); it.increment(ec)) {
         if (ec) {
+            if (fs::is_directory(*it)) {
+                it.no_push();
+                LogPrintf("%s: %s %s -- skipping.\n", __func__, ec.message(), it->path().string());
+            } else {
                 LogPrintf("%s: %s %s\n", __func__, ec.message(), it->path().string());
+            }
             continue;
         }
 

src/wallet/load.cpp

@@ -79,7 +79,7 @@ bool VerifyWallets(interfaces::Chain& chain)
         bilingual_str error_string;
         if (!MakeWalletDatabase(wallet_file, options, status, error_string)) {
             if (status == DatabaseStatus::FAILED_NOT_FOUND) {
-                chain.initWarning(Untranslated(strprintf("Skipping -wallet path that doesn't exist. %s\n", error_string.original)));
+                chain.initWarning(Untranslated(strprintf("Skipping -wallet path that doesn't exist. %s", error_string.original)));
             } else {
                 chain.initError(error_string);
                 return false;

src/wallet/wallet.cpp

@@ -257,7 +257,7 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string&
 {
     auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(name));
     if (!result.second) {
-        error = Untranslated("Wallet already being loading.");
+        error = Untranslated("Wallet already loading.");
         status = DatabaseStatus::FAILED_LOAD;
         return nullptr;
     }

test/functional/interface_rpc.py

@@ -8,6 +8,9 @@ import os
 from test_framework.authproxy import JSONRPCException
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal, assert_greater_than_or_equal
+from threading import Thread
+import subprocess
+
 
 def expect_http_status(expected_http_status, expected_rpc_code,
                        fcn, *args):
@@ -18,6 +21,16 @@ def expect_http_status(expected_http_status, expected_rpc_code,
         assert_equal(exc.error["code"], expected_rpc_code)
         assert_equal(exc.http_status, expected_http_status)
 
+
+def test_work_queue_getblock(node, got_exceeded_error):
+    while not got_exceeded_error:
+        try:
+            node.cli('getrpcinfo').send_cli()
+        except subprocess.CalledProcessError as e:
+            assert_equal(e.output, 'error: Server response: Work queue depth exceeded\n')
+            got_exceeded_error.append(True)
+
+
 class RPCInterfaceTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
@@ -67,10 +80,23 @@ class RPCInterfaceTest(BitcoinTestFramework):
         expect_http_status(404, -32601, self.nodes[0].invalidmethod)
         expect_http_status(500, -8, self.nodes[0].getblockhash, 42)
 
+    def test_work_queue_exceeded(self):
+        self.log.info("Testing work queue exceeded...")
+        self.restart_node(0, ['-rpcworkqueue=1', '-rpcthreads=1'])
+        got_exceeded_error = []
+        threads = []
+        for _ in range(3):
+            t = Thread(target=test_work_queue_getblock, args=(self.nodes[0], got_exceeded_error))
+            t.start()
+            threads.append(t)
+        for t in threads:
+            t.join()
+
     def run_test(self):
         self.test_getrpcinfo()
         self.test_batch_request()
         self.test_http_status_codes()
+        self.test_work_queue_exceeded()
 
 
 if __name__ == '__main__':

test/functional/wallet_multiwallet.py

@@ -34,7 +34,7 @@ def test_load_unload(node, name):
             node.loadwallet(name)
             node.unloadwallet(name)
         except JSONRPCException as e:
-            if e.error['code'] == -4 and 'Wallet already being loading' in e.error['message']:
+            if e.error['code'] == -4 and 'Wallet already loading' in e.error['message']:
                 got_loading_error = True
                 return
 

test/sanitizer_suppressions/ubsan

@@ -57,7 +57,6 @@ implicit-integer-sign-change:crypto/*
 implicit-integer-sign-change:key.cpp
 implicit-integer-sign-change:noui.cpp
 implicit-integer-sign-change:prevector.h
-implicit-integer-sign-change:protocol.cpp
 implicit-integer-sign-change:script/bitcoinconsensus.cpp
 implicit-integer-sign-change:script/interpreter.cpp
 implicit-integer-sign-change:serialize.h