Merge #5909: backport: trivial 2024 02 28

19db5875f0 Merge bitcoin/bitcoin#25959: doc: Fix link to MurmurHash3.cpp (moved from Google Code to Github) (MacroFake)
676eb1b8d7 Merge bitcoin/bitcoin#25967: refactor: add LIFETIMEBOUND to blockfilter where needed (MacroFake)
82116f2c07 Merge bitcoin/bitcoin#25883: doc: Security config warning (MacroFake)
1c678a1b62 Merge bitcoin/bitcoin#25888: refactor: use `strprintf` for creating unknown-service-flag string (MacroFake)
1526885baf Merge bitcoin/bitcoin#25836: subtree: update crc32c subtree (MacroFake)
c9d3695688 Merge bitcoin/bitcoin#25798: build: fix cleanup of test logs (Andrew Chow)
afc1a9f4ac Merge bitcoin/bitcoin#25811: doc: test: suggest multi-line imports in functional test style guide (MacroFake)
3e693ddfb5 Merge bitcoin/bitcoin#25788: guix: patch NSIS to remove .reloc sections from installer stubs (Andrew Chow)
f8719ec4a4 Merge bitcoin/bitcoin#22176: test: Correct outstanding -Werror=sign-compare errors (fanquake)
8fcd549956 Merge bitcoin/bitcoin#22180: fuzz: Increase branch coverage of the float fuzz target (MarcoFalke)

Pull request description:

  ## What was done?
  Trivial Backports

  ## How Has This Been Tested?
  Built locally; did not run tests or review

  ## Breaking Changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: 457a69ced1684f550fe80a2456ae828337d236a47f2b6e8af2f395698877a1f7daaaadb32fdfc9d338009a90568c7bddbc4f7d172e238840555c9613fe5fb18f

contrib/guix/manifest.scm

@@ -200,7 +200,8 @@ chain for " target " development."))
 
 (define (make-nsis-for-gcc-10 base-nsis)
   (package-with-extra-patches base-nsis
-    (search-our-patches "nsis-gcc-10-memmove.patch")))
+    (search-our-patches "nsis-gcc-10-memmove.patch"
+                        "nsis-disable-installer-reloc.patch")))
 
 (define osslsigncode
   (package

contrib/guix/patches/nsis-disable-installer-reloc.patch

@@ -0,0 +1,30 @@
+Patch NSIS so that it's installer stubs, produced at NSIS build time,
+do not contain .reloc sections, which will exist by default when using
+binutils/ld 2.36+.
+
+This ultimately fixes an issue when running the installer with the
+"Force randomization for images (Mandatory ASLR)" setting active.
+
+This patch has not yet been sent upstream, because it's not clear if this
+is the best fix, for the underlying issue, which seems to be that makensis
+doesn't account for .reloc sections when it builds installers.
+
+The existence of a reloc section shouldn't be a problem, and, if anything,
+is actually a requirement for working ASLR. All other Windows binaries we
+produce contain them, and function correctly when under the same
+"Force randomization for images (Mandatory ASLR)" setting.
+
+See:
+https://github.com/bitcoin/bitcoin/issues/25726
+https://sourceforge.net/p/nsis/bugs/1131/
+
+--- a/SCons/Config/gnu
++++ b/SCons/Config/gnu
+@@ -102,6 +102,7 @@ stub_env.Append(LINKFLAGS = ['-mwindows'])          # build windows executables
+ stub_env.Append(LINKFLAGS = ['$NODEFLIBS_FLAG'])    # no standard libraries
+ stub_env.Append(LINKFLAGS = ['$ALIGN_FLAG'])        # 512 bytes align
+ stub_env.Append(LINKFLAGS = ['$MAP_FLAG'])          # generate map file
++stub_env.Append(LINKFLAGS = ['-Wl,--disable-reloc-section'])
+ 
+ conf = FlagsConfigure(stub_env)
+ conf.CheckCompileFlag('-fno-tree-loop-distribute-patterns')  # GCC 10: Don't generate msvcrt!memmove calls (bug #1248)

doc/dash-conf.md

@@ -6,6 +6,8 @@ All command-line options (except for `-?`, `-help`, `-version` and `-conf`) may
 
 Changes to the configuration file while `dashd` or `dash-qt` is running only take effect after restarting.
 
+Users should never make any configuration changes which they do not understand. Furthermore, users should always be wary of accepting any configuration changes provided to them by another source (even if they believe that they do understand them).
+
 ## Configuration File Format
 
 The configuration file is a plain text file and consists of `option=value` entries, one per line. Leading and trailing whitespaces are removed.

src/Makefile.test.include

@@ -337,7 +337,7 @@ nodist_test_test_dash_SOURCES = $(GENERATED_TEST_FILES)
 
 $(BITCOIN_TESTS): $(GENERATED_TEST_FILES)
 
-CLEAN_BITCOIN_TEST = test/*.gcda test/*.gcno test/fuzz/*.gcda test/fuzz/*.gcno test/util/*.gcda test/util/*.gcno $(GENERATED_TEST_FILES) $(BITCOIN_TESTS:=.log)
+CLEAN_BITCOIN_TEST = test/*.gcda test/*.gcno test/fuzz/*.gcda test/fuzz/*.gcno test/util/*.gcda test/util/*.gcno $(GENERATED_TEST_FILES) $(addsuffix .log,$(basename $(BITCOIN_TESTS)))
 
 CLEANFILES += $(CLEAN_BITCOIN_TEST)
 

src/blockfilter.h

@@ -11,6 +11,7 @@
 #include <unordered_set>
 #include <vector>
 
+#include <attributes.h>
 #include <primitives/block.h>
 #include <serialize.h>
 #include <uint256.h>
@@ -65,8 +66,8 @@ public:
     GCSFilter(const Params& params, const ElementSet& elements);
 
     uint32_t GetN() const { return m_N; }
-    const Params& GetParams() const { return m_params; }
-    const std::vector<unsigned char>& GetEncoded() const { return m_encoded; }
+    const Params& GetParams() const LIFETIMEBOUND { return m_params; }
+    const std::vector<unsigned char>& GetEncoded() const LIFETIMEBOUND { return m_encoded; }
 
     /**
      * Checks if the element may be in the set. False positives are possible
@@ -128,10 +129,10 @@ public:
     BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo);
 
     BlockFilterType GetFilterType() const { return m_filter_type; }
-    const uint256& GetBlockHash() const { return m_block_hash; }
-    const GCSFilter& GetFilter() const { return m_filter; }
+    const uint256& GetBlockHash() const LIFETIMEBOUND { return m_block_hash; }
+    const GCSFilter& GetFilter() const LIFETIMEBOUND { return m_filter; }
 
-    const std::vector<unsigned char>& GetEncodedFilter() const
+    const std::vector<unsigned char>& GetEncodedFilter() const LIFETIMEBOUND
     {
         return m_filter.GetEncoded();
     }

src/crc32c/CMakeLists.txt

@@ -296,9 +296,10 @@ target_include_directories(crc32c
     $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
 )
 
-target_compile_definitions(crc32c
-PRIVATE
-  CRC32C_HAVE_CONFIG_H=1
+set_property(
+  TARGET crc32c_arm64 crc32c_sse42 crc32c
+  APPEND
+  PROPERTY COMPILE_DEFINITIONS CRC32C_HAVE_CONFIG_H
 )
 
 set_target_properties(crc32c

src/hash.cpp

@@ -15,7 +15,7 @@ inline uint32_t ROTL32(uint32_t x, int8_t r)
 
 unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vDataToHash)
 {
-    // The following is MurmurHash3 (x86_32), see https://code.google.com/p/smhasher/source/browse/trunk/MurmurHash3.cpp
+    // The following is MurmurHash3 (x86_32), see https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp
     uint32_t h1 = nHashSeed;
     const uint32_t c1 = 0xcc9e2d51;
     const uint32_t c2 = 0x1b873593;

src/index/blockfilterindex.h

@@ -5,6 +5,7 @@
 #ifndef BITCOIN_INDEX_BLOCKFILTERINDEX_H
 #define BITCOIN_INDEX_BLOCKFILTERINDEX_H
 
+#include <attributes.h>
 #include <blockfilter.h>
 #include <chain.h>
 #include <flatfile.h>
@@ -47,9 +48,9 @@ protected:
 
     bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) override;
 
-    BaseIndex::DB& GetDB() const override { return *m_db; }
+    BaseIndex::DB& GetDB() const LIFETIMEBOUND override { return *m_db; }
 
-    const char* GetName() const override { return m_name.c_str(); }
+    const char* GetName() const LIFETIMEBOUND override { return m_name.c_str(); }
 
 public:
     /** Constructs the index, which becomes available to be queried. */

src/protocol.cpp

@@ -349,12 +349,7 @@ static std::string serviceFlagToStr(size_t bit)
     // Not using default, so we get warned when a case is missing
     }
 
-    std::ostringstream stream;
-    stream.imbue(std::locale::classic());
-    stream << "UNKNOWN[";
-    stream << "2^" << bit;
-    stream << "]";
-    return stream.str();
+    return strprintf("UNKNOWN[2^%u]", bit);
 }
 
 std::vector<std::string> serviceFlagsToStr(uint64_t flags)

src/test/fuzz/float.cpp

@@ -5,6 +5,7 @@
 #include <memusage.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
 #include <util/serfloat.h>
 #include <version.h>
 
@@ -17,7 +18,33 @@ FUZZ_TARGET(float)
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
     {
-        const double d = fuzzed_data_provider.ConsumeFloatingPoint<double>();
+        const double d{[&] {
+            double tmp;
+            CallOneOf(
+                fuzzed_data_provider,
+                // an actual number
+                [&] { tmp = fuzzed_data_provider.ConsumeFloatingPoint<double>(); },
+                // special numbers and NANs
+                [&] { tmp = fuzzed_data_provider.PickValueInArray({
+                          std::numeric_limits<double>::infinity(),
+                          -std::numeric_limits<double>::infinity(),
+                          std::numeric_limits<double>::min(),
+                          -std::numeric_limits<double>::min(),
+                          std::numeric_limits<double>::max(),
+                          -std::numeric_limits<double>::max(),
+                          std::numeric_limits<double>::lowest(),
+                          -std::numeric_limits<double>::lowest(),
+                          std::numeric_limits<double>::quiet_NaN(),
+                          -std::numeric_limits<double>::quiet_NaN(),
+                          std::numeric_limits<double>::signaling_NaN(),
+                          -std::numeric_limits<double>::signaling_NaN(),
+                          std::numeric_limits<double>::denorm_min(),
+                          -std::numeric_limits<double>::denorm_min(),
+                      }); },
+                // Anything from raw memory (also checks that DecodeDouble doesn't crash on any input)
+                [&] { tmp = DecodeDouble(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); });
+            return tmp;
+        }()};
         (void)memusage::DynamicUsage(d);
 
         uint64_t encoded = EncodeDouble(d);

src/test/fuzz/util.h

@@ -47,7 +47,7 @@ void CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callables)
     const size_t call_index{fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, call_size - 1)};
 
     size_t i{0};
-    return ((i++ == call_index ? callables() : void()), ...);
+    ((i++ == call_index ? callables() : void()), ...);
 }
 
 template <typename Collection>

src/test/serfloat_tests.cpp

@@ -36,9 +36,9 @@ uint64_t TestDouble(double f) {
 } // namespace
 
 BOOST_AUTO_TEST_CASE(double_serfloat_tests) {
-    BOOST_CHECK_EQUAL(TestDouble(0.0), 0);
+    BOOST_CHECK_EQUAL(TestDouble(0.0), 0U);
     BOOST_CHECK_EQUAL(TestDouble(-0.0), 0x8000000000000000);
-    BOOST_CHECK_EQUAL(TestDouble(std::numeric_limits<double>::infinity()), 0x7ff0000000000000);
+    BOOST_CHECK_EQUAL(TestDouble(std::numeric_limits<double>::infinity()), 0x7ff0000000000000U);
     BOOST_CHECK_EQUAL(TestDouble(-std::numeric_limits<double>::infinity()), 0xfff0000000000000);
     BOOST_CHECK_EQUAL(TestDouble(0.5), 0x3fe0000000000000ULL);
     BOOST_CHECK_EQUAL(TestDouble(1.0), 0x3ff0000000000000ULL);
@@ -48,8 +48,8 @@ BOOST_AUTO_TEST_CASE(double_serfloat_tests) {
 
     // Roundtrip test on IEC559-compatible systems
     if (std::numeric_limits<double>::is_iec559) {
-        BOOST_CHECK_EQUAL(sizeof(double), 8);
-        BOOST_CHECK_EQUAL(sizeof(uint64_t), 8);
+        BOOST_CHECK_EQUAL(sizeof(double), 8U);
+        BOOST_CHECK_EQUAL(sizeof(uint64_t), 8U);
         // Test extreme values
         TestDouble(std::numeric_limits<double>::min());
         TestDouble(-std::numeric_limits<double>::min());

src/test/streams_tests.cpp

@@ -121,7 +121,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader_rvalue)
     uint32_t varint = 0;
     // Deserialize into r-value
     reader >> VARINT(varint);
-    BOOST_CHECK_EQUAL(varint, 54321);
+    BOOST_CHECK_EQUAL(varint, 54321U);
     BOOST_CHECK(reader.empty());
 }
 

test/functional/README.md

@@ -28,7 +28,9 @@ don't have test cases for.
   could lead to bugs and issues in the test code.
 - Use [type hints](https://docs.python.org/3/library/typing.html) in your code to improve code readability
   and to detect possible bugs earlier.
-- Avoid wildcard imports
+- Avoid wildcard imports.
+- If more than one name from a module is needed, use lexicographically sorted multi-line imports
+  in order to reduce the possibility of potential merge conflicts.
 - Use a module-level docstring to describe what the test is testing, and how it
   is testing it.
 - When subclassing the BitcoinTestFramework, place overrides for the

test/functional/example_test.py

@@ -14,8 +14,15 @@ is testing and *how* it's being tested
 from collections import defaultdict
 
 # Avoid wildcard * imports
-from test_framework.blocktools import (create_block, create_coinbase)
-from test_framework.messages import CInv, MSG_BLOCK
+# Use lexicographically sorted multi-line imports
+from test_framework.blocktools import (
+    create_block,
+    create_coinbase,
+)
+from test_framework.messages import (
+    CInv,
+    MSG_BLOCK,
+)
 from test_framework.p2p import (
     P2PInterface,
     msg_block,