Merge #6345: backport: trivial 2024 10 23 pr1

b70e091 Merge bitcoin#29667: fuzz: actually test garbage >64b in p2p transport test (fanquake)
6d7aa3d Merge bitcoin#29497: test: simplify test_runner.py (fanquake)
d0e15d5 Merge bitcoin#29606: refactor: Reserve memory for ToLower/ToUpper conversions (Ava Chow)
045fa5f Merge bitcoin#29514: tests: Provide more helpful assert_equal errors (Ava Chow)
bd607f0 Merge bitcoin#29393: i2p: log connection was refused due to arbitrary port (Ava Chow)
c961755 Merge bitcoin#29595: doc: Wrap flags with code in developer-notes.md (fanquake)
8d6e5e7 Merge bitcoin#29583: fuzz: Apply fuzz env (suppressions, etc.) when fetching harness list (fanquake)
4dce690 Merge bitcoin#29576: Update functional test runner to return error code when no tests are found to run (fanquake)
910a7d6 Merge bitcoin#29529: fuzz: restrict fopencookie usage to Linux & FreeBSD (fanquake)
fdac2b3 Merge bitcoin#29493: subtree: update crc32c subtree (fanquake)
a23b342 Merge bitcoin#29475: doc: Fix Broken Links (fanquake)
92bad90 Merge bitcoin#28178: fuzz: Generate with random libFuzzer settings (fanquake)
9b6a05d Merge bitcoin#29443: depends: fix BDB compilation on OpenBSD (fanquake)
9963e6b Merge bitcoin#29413: fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse` (fanquake)
3914745 Merge bitcoin#29425: test: fix intermittent failure in wallet_reorgrestore.py (fanquake)
b719883 Merge bitcoin#29399: test: Fix utxo set hash serialisation signedness (fanquake)
f096880 Merge bitcoin#29377: test: Add makefile target for running unit tests (Ava Chow)
03e0bd3 Merge bitcoin#27319: addrman, refactor: improve stochastic test in `AddSingle` (Ava Chow)

Pull request description:

  ## Issue being fixed or feature implemented
  Batch of trivial backports

  ## What was done?
  See commits

  ## How Has This Been Tested?
  built locally; large combined merge passed tests locally

  ## Breaking Changes
  Should be none

  ## 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
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK b70e091
  knst:
    utACK b70e091

Tree-SHA512: 659a931f812c8a92cf3854abb873d92885219a392d6aa8e49ac4b27fe41e8e163ef9a135050e7c2e1bd33cecd2f7dae215e05a9c29f62e837e0057d3c16746d6

depends/packages/bdb.mk

@@ -13,7 +13,6 @@ $(package)_cflags+=-Wno-error=implicit-function-declaration -Wno-error=format-se
 $(package)_cxxflags+=-std=c++17
 $(package)_cppflags_freebsd=-D_XOPEN_SOURCE=600 -D__BSD_VISIBLE=1
 $(package)_cppflags_netbsd=-D_XOPEN_SOURCE=600
-$(package)_cppflags_openbsd=-D_XOPEN_SOURCE=600
 $(package)_cppflags_mingw32=-DUNICODE -D_UNICODE
 endef
 

doc/developer-notes.md

@@ -326,7 +326,7 @@ produce better debugging builds.
 ### Show sources in debugging
 
 If you have ccache enabled, absolute paths are stripped from debug information
-with the -fdebug-prefix-map and -fmacro-prefix-map options (if supported by the
+with the `-fdebug-prefix-map` and `-fmacro-prefix-map` options (if supported by the
 compiler). This might break source file detection in case you move binaries
 after compilation, debug from the directory other than the project root or use
 an IDE that only supports absolute paths for debugging.

src/Makefile.test.include

@@ -367,7 +367,9 @@ dash_test_check: $(TEST_BINARY) FORCE
 dash_test_clean : FORCE
 	rm -f $(CLEAN_BITCOIN_TEST) $(test_test_dash_OBJECTS) $(TEST_BINARY)
 
-check-local: $(BITCOIN_TESTS:.cpp=.cpp.test)
+check-unit: $(BITCOIN_TESTS:.cpp=.cpp.test)
+
+check-local: check-unit
 if BUILD_BITCOIN_TX
 	@echo "Running test/util/bitcoin-util-test.py..."
 	$(PYTHON) $(top_builddir)/test/util/bitcoin-util-test.py

src/addrman.cpp

@@ -581,11 +581,10 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_
             return false;
 
         // stochastic test: previous nRefCount == N: 2^N times harder to increase it
-        int nFactor = 1;
-        for (int n = 0; n < pinfo->nRefCount; n++)
-            nFactor *= 2;
-        if (nFactor > 1 && (insecure_rand.randrange(nFactor) != 0))
-            return false;
+        if (pinfo->nRefCount > 0) {
+            const int nFactor{1 << pinfo->nRefCount};
+            if (insecure_rand.randrange(nFactor) != 0) return false;
+        }
     } else {
         pinfo = Create(addr, source, &nId);
         pinfo->nTime = std::max((int64_t)0, (int64_t)pinfo->nTime - nTimePenalty);

src/crc32c/src/crc32c_arm64.cc

@@ -12,6 +12,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <cstring>
 
 #include "./crc32c_internal.h"
 #ifdef CRC32C_HAVE_CONFIG_H
@@ -29,14 +30,14 @@
 // compute 8bytes for each segment parallelly
 #define CRC32C32BYTES(P, IND)                                             \
   do {                                                                    \
-    crc1 = __crc32cd(                                                     \
-        crc1, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 1 + (IND))); \
-    crc2 = __crc32cd(                                                     \
-        crc2, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 2 + (IND))); \
-    crc3 = __crc32cd(                                                     \
-        crc3, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 3 + (IND))); \
-    crc0 = __crc32cd(                                                     \
-        crc0, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 0 + (IND))); \
+    std::memcpy(&d64, (P) + SEGMENTBYTES * 1 + (IND) * 8, sizeof(d64));   \
+    crc1 = __crc32cd(crc1, d64);                                          \
+    std::memcpy(&d64, (P) + SEGMENTBYTES * 2 + (IND) * 8, sizeof(d64));   \
+    crc2 = __crc32cd(crc2, d64);                                          \
+    std::memcpy(&d64, (P) + SEGMENTBYTES * 3 + (IND) * 8, sizeof(d64));   \
+    crc3 = __crc32cd(crc3, d64);                                          \
+    std::memcpy(&d64, (P) + SEGMENTBYTES * 0 + (IND) * 8, sizeof(d64));   \
+    crc0 = __crc32cd(crc0, d64);                                          \
   } while (0);
 
 // compute 8*8 bytes for each segment parallelly
@@ -68,6 +69,9 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) {
   int64_t length = size;
   uint32_t crc0, crc1, crc2, crc3;
   uint64_t t0, t1, t2;
+  uint16_t d16;
+  uint32_t d32;
+  uint64_t d64;
 
   // k0=CRC(x^(3*SEGMENTBYTES*8)), k1=CRC(x^(2*SEGMENTBYTES*8)),
   // k2=CRC(x^(SEGMENTBYTES*8))
@@ -88,7 +92,8 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) {
     t2 = (uint64_t)vmull_p64(crc2, k2);
     t1 = (uint64_t)vmull_p64(crc1, k1);
     t0 = (uint64_t)vmull_p64(crc0, k0);
-    crc = __crc32cd(crc3, *(uint64_t *)data);
+    std::memcpy(&d64, data, sizeof(d64));
+    crc = __crc32cd(crc3, d64);
     data += sizeof(uint64_t);
     crc ^= __crc32cd(0, t2);
     crc ^= __crc32cd(0, t1);
@@ -98,18 +103,21 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) {
   }
 
   while (length >= 8) {
-    crc = __crc32cd(crc, *(uint64_t *)data);
+    std::memcpy(&d64, data, sizeof(d64));
+    crc = __crc32cd(crc, d64);
     data += 8;
     length -= 8;
   }
 
   if (length & 4) {
-    crc = __crc32cw(crc, *(uint32_t *)data);
+    std::memcpy(&d32, data, sizeof(d32));
+    crc = __crc32cw(crc, d32);
     data += 4;
   }
 
   if (length & 2) {
-    crc = __crc32ch(crc, *(uint16_t *)data);
+    std::memcpy(&d16, data, sizeof(d16));
+    crc = __crc32ch(crc, d16);
     data += 2;
   }
 

src/i2p.cpp

@@ -186,6 +186,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
     // Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy
     // when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT.
     if (to.GetPort() != I2P_SAM31_PORT) {
+        Log("Error connecting to %s, connection refused due to arbitrary port %s", to.ToStringAddrPort(), to.GetPort());
         proxy_error = false;
         return false;
     }

src/test/README.md

@@ -15,7 +15,8 @@ that runs all of the unit tests. The main source file for the test library is fo
 Unit tests will be automatically compiled if dependencies were met in `./configure`
 and tests weren't explicitly disabled.
 
-After configuring, they can be run with `make check`.
+After configuring, they can be run with `make check`, which includes unit tests from
+subtrees, or `make && make -C src check-unit` for just the unit tests.
 
 To run the unit tests manually, launch `src/test/test_dash`. To recompile
 after a test file was modified, run `make` and then run the test again. If you

src/test/fuzz/net_permissions.cpp

@@ -16,7 +16,7 @@
 FUZZ_TARGET(net_permissions)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
-    const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(32);
+    const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(1000);
     const NetPermissionFlags net_permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
 
     NetWhitebindPermissions net_whitebind_permissions;

src/test/fuzz/p2p_transport_serialization.cpp

@@ -349,6 +349,7 @@ std::unique_ptr<Transport> MakeV2Transport(NodeId nodeid, bool initiator, RNG& r
     } else {
         // If it's longer, generate it from the RNG. This avoids having large amounts of
         // (hopefully) irrelevant data needing to be stored in the fuzzer data.
+        garb.resize(garb_len);
         for (auto& v : garb) v = uint8_t(rng());
     }
     // Retrieve entropy

src/test/fuzz/util.cpp

@@ -438,7 +438,7 @@ FILE* FuzzedFileProvider::open()
         [&] {
             mode = "a+";
         });
-#if defined _GNU_SOURCE && !defined __ANDROID__
+#if defined _GNU_SOURCE && (defined(__linux__) || defined(__FreeBSD__))
     const cookie_io_functions_t io_hooks = {
         FuzzedFileProvider::read,
         FuzzedFileProvider::write,

src/util/strencodings.cpp

@@ -476,13 +476,15 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out)
 std::string ToLower(const std::string& str)
 {
     std::string r;
+    r.reserve(str.size());
     for (auto ch : str) r += ToLower(ch);
     return r;
 }
 
 std::string ToUpper(const std::string& str)
 {
     std::string r;
+    r.reserve(str.size());
     for (auto ch : str) r += ToUpper(ch);
     return r;
 }

test/functional/feature_utxo_set_hash.py

@@ -4,8 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test UTXO set hash value calculation in gettxoutsetinfo."""
 
-import struct
-
 from test_framework.messages import (
     CBlock,
     COutPoint,
@@ -58,7 +56,7 @@ def test_muhash_implementation(self):
                         continue
 
                     data = COutPoint(int(tx.rehash(), 16), n).serialize()
-                    data += struct.pack("<i", height * 2 + coinbase)
+                    data += (height * 2 + coinbase).to_bytes(4, "little")
                     data += tx_out.serialize()
 
                     muhash.insert(data)

test/functional/p2p_i2p_ports.py

@@ -6,36 +6,28 @@
 Test ports handling for I2P hosts
 """
 
-import re
 
 from test_framework.test_framework import BitcoinTestFramework
 
+PROXY = "127.0.0.1:60000"
 
 class I2PPorts(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
         # The test assumes that an I2P SAM proxy is not listening here.
-        self.extra_args = [["-i2psam=127.0.0.1:60000"]]
+        self.extra_args = [[f"-i2psam={PROXY}"]]
 
     def run_test(self):
         node = self.nodes[0]
 
         self.log.info("Ensure we don't try to connect if port!=0")
         addr = "zsxwyo6qcn3chqzwxnseusqgsnuw3maqnztkiypyfxtya4snkoka.b32.i2p:8333"
-        raised = False
-        try:
-            with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]):
-                node.addnode(node=addr, command="onetry")
-        except AssertionError as e:
-            raised = True
-            if not re.search(r"Expected messages .* does not partially match log", str(e)):
-                raise AssertionError(f"Assertion raised as expected, but with an unexpected message: {str(e)}")
-        if not raised:
-            raise AssertionError("Assertion should have been raised")
+        with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}, connection refused due to arbitrary port 8333"]):
+            node.addnode(node=addr, command="onetry")
 
         self.log.info("Ensure we try to connect if port=0 and get an error due to missing I2P proxy")
         addr = "h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p:0"
-        with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]):
+        with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}: Cannot connect to {PROXY}"]):
             node.addnode(node=addr, command="onetry")
 
 

test/functional/test_framework/netutil.py

@@ -4,7 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Linux network utilities.
 
-Roughly based on http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ by Ricardo Pascal
+Roughly based on https://web.archive.org/web/20190424172231/http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ by Ricardo Pascal
 """
 
 import sys

test/functional/test_framework/util.py

@@ -46,7 +46,24 @@ def assert_fee_amount(fee, tx_size, fee_per_kB):
         raise AssertionError("Fee of %s DASH too high! (Should be %s DASH)" % (str(fee), str(target_fee)))
 
 
+def summarise_dict_differences(thing1, thing2):
+    if not isinstance(thing1, dict) or not isinstance(thing2, dict):
+        return thing1, thing2
+    d1, d2 = {}, {}
+    for k in sorted(thing1.keys()):
+        if k not in thing2:
+            d1[k] = thing1[k]
+        elif thing1[k] != thing2[k]:
+            d1[k], d2[k] = summarise_dict_differences(thing1[k], thing2[k])
+    for k in sorted(thing2.keys()):
+        if k not in thing1:
+            d2[k] = thing2[k]
+    return d1, d2
+
 def assert_equal(thing1, thing2, *args):
+    if thing1 != thing2 and not args and isinstance(thing1, dict) and isinstance(thing2, dict):
+        d1,d2 = summarise_dict_differences(thing1, thing2)
+        raise AssertionError("not(%s == %s)\n  in particular not(%s == %s)" % (thing1, thing2, d1, d2))
     if thing1 != thing2 or any(thing1 != arg for arg in args):
         raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
 

test/functional/test_runner.py

@@ -453,7 +453,7 @@ def main():
     if not enable_bitcoind:
         print("No functional tests to run.")
         print("Rerun ./configure with --with-daemon and then make")
-        sys.exit(0)
+        sys.exit(1)
 
     # Build list of tests
     test_list = []
@@ -502,7 +502,7 @@ def main():
     if not test_list:
         print("No valid test scripts specified. Check that your test is in one "
               "of the test lists in test_runner.py, or run test_runner.py with no arguments to run all tests")
-        sys.exit(0)
+        sys.exit(1)
 
     if args.help:
         # Print help for test_runner.py, then print help of the first script (with args removed) and exit.
@@ -592,14 +592,12 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, attempts=1, enab
     max_len_name = len(max(test_list, key=len))
     test_count = len(test_list)
     all_passed = True
-    i = 0
-    while i < test_count:
+    while not job_queue.done():
         if failfast and not all_passed:
             break
         for test_result, testdir, stdout, stderr in job_queue.get_next():
             test_results.append(test_result)
-            i += 1
-            done_str = "{}/{} - {}{}{}".format(i, test_count, BOLD[1], test_result.name, BOLD[0])
+            done_str = f"{len(test_results)}/{test_count} - {BOLD[1]}{test_result.name}{BOLD[0]}"
             if test_result.status == "Passed":
                 logging.debug("%s passed, Duration: %s s" % (done_str, test_result.time))
             elif test_result.status == "Skipped":
@@ -684,15 +682,16 @@ def __init__(self, *, num_tests_parallel, tests_dir, tmpdir, test_list, flags, u
         self.tmpdir = tmpdir
         self.test_list = test_list
         self.flags = flags
-        self.num_running = 0
         self.jobs = []
         self.use_term_control = use_term_control
         self.attempts = attempts
 
+    def done(self):
+        return not (self.jobs or self.test_list)
+
     def get_next(self):
-        while self.num_running < self.num_jobs and self.test_list:
+        while len(self.jobs) < self.num_jobs and self.test_list:
             # Add tests
-            self.num_running += 1
             test = self.test_list.pop(0)
             portseed = len(self.test_list)
             portseed_arg = ["--portseed={}".format(portseed)]
@@ -764,7 +763,6 @@ def get_next(self):
                         continue
                     else:
                         status = "Failed"
-                    self.num_running -= 1
                     self.jobs.remove(job)
                     if self.use_term_control:
                         clearline = '\r' + (' ' * dot_count) + '\r'

test/functional/wallet_reorgsrestore.py

@@ -44,6 +44,7 @@ def run_test(self):
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10"))
         tx = self.nodes[0].gettransaction(txid)
         self.generate(self.nodes[0], 4, sync_fun=self.no_op)
+        self.sync_blocks([self.nodes[0], self.nodes[2]])
         tx_before_reorg = self.nodes[0].gettransaction(txid)
         assert_equal(tx_before_reorg["confirmations"], 4)
 

test/fuzz/test_runner.py

@@ -10,6 +10,7 @@
 import configparser
 import logging
 import os
+import random
 import subprocess
 import sys
 
@@ -93,7 +94,10 @@ def main():
         sys.exit(1)
 
     # Build list of tests
-    test_list_all = parse_test_list(fuzz_bin=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz'))
+    test_list_all = parse_test_list(
+        fuzz_bin=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz'),
+        source_dir=config['environment']['SRCDIR'],
+    )
 
     if not test_list_all:
         logging.error("No fuzz targets found")
@@ -207,9 +211,13 @@ def job(command, t):
     for target in targets:
         target_corpus_dir = os.path.join(corpus_dir, target)
         os.makedirs(target_corpus_dir, exist_ok=True)
+        use_value_profile = int(random.random() < .3)
         command = [
             os.path.join(build_dir, 'src', 'test', 'fuzz', 'fuzz'),
-            "-runs=100000",
+            "-rss_limit_mb=8000",
+            "-max_total_time=6000",
+            "-reload=0",
+            f"-use_value_profile={use_value_profile}",
             target_corpus_dir,
         ]
         futures.append(fuzz_pool.submit(job, command, target))
@@ -298,11 +306,12 @@ def job(t, args):
             sys.exit(1)
 
 
-def parse_test_list(*, fuzz_bin):
+def parse_test_list(*, fuzz_bin, source_dir):
     test_list_all = subprocess.run(
         fuzz_bin,
         env={
-            'PRINT_ALL_FUZZ_TARGETS_AND_ABORT': ''
+            'PRINT_ALL_FUZZ_TARGETS_AND_ABORT': '',
+            **get_fuzz_env(target="", source_dir=source_dir)
         },
         stdout=subprocess.PIPE,
         stderr=subprocess.DEVNULL,