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 @@ class CMessageHeader
 
     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);
         }
     }