Merge #6158: backport: trivial 2024 07 25

0dd997c4e5 Merge bitcoin/bitcoin#26835: contrib: add PE Canary check to security-check (fanquake)
b6bde7322c Merge bitcoin/bitcoin#26827: doc: use "std lib clock" over "C++11 clock" (MarcoFalke)
93c4652a05 Merge bitcoin/bitcoin#26821: refactor: Make `ThreadHTTP` return void (Andrew Chow)
07f4c39c44 Merge bitcoin/bitcoin#26730: test: add coverage for `purpose` arg in `listlabels` (MarcoFalke)
6fe46fc02a Merge bitcoin/bitcoin#26864: doc: net: fix link to onion address encoding scheme [ONIONADDRESS] (MarcoFalke)
d1b93c78b7 Merge bitcoin/bitcoin#26818: test: Fix feature_startupnotify intermittent issue (MarcoFalke)
864d02e4a9 Merge bitcoin/bitcoin#26809: compat: use STDIN_FILENO over 0 (Andrew Chow)
092ddc3a3e Merge bitcoin/bitcoin#26603: doc: CalculateSequenceLocks: prevHeights entries are set to 0, not removed (glozow)
7f2b934089 Merge bitcoin/bitcoin#26772: contrib: fix sha256 check in install_db4.sh for FreeBSD (fanquake)
df2f533aaf Merge bitcoin/bitcoin#26759: test: Drop no longer needed `race:epoll_ctl` TSan suppression (MarcoFalke)
9590929900 Merge bitcoin/bitcoin#24944: rpc: add getblockfrompeer RPCTypeCheck and invalid input test coverage (MacroFake)

Pull request description:

  ## Issue being fixed or feature implemented
  Trivial backports; need to see ci pass

  ## What was done?

  ## How Has This Been Tested?
  built

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

ACKs for top commit:
  knst:
    utACK 0dd997c4e5
  UdjinM6:
    utACK 0dd997c4e5

Tree-SHA512: e23207508dfc88a961e0eed031b1c6d195469d61343f0d1c66be853a6fcd882a84d8d40670c263264fe96c9a8a98d83fc0f0a05c263eadc64056edb563c588db

.github/ISSUE_TEMPLATE/good_first_issue.md

@@ -15,7 +15,7 @@ assignees: ''
 
 #### Useful skills:
 
-<!-- (For example, “C++11 std::thread”, “Qt5 GUI and async GUI design” or “basic understanding of Bitcoin mining and the Bitcoin Core RPC interface”.) -->
+<!-- (For example, “std::thread”, “Qt5 GUI and async GUI design” or “basic understanding of Bitcoin mining and the Bitcoin Core RPC interface”.) -->
 
 #### Want to work on this issue?
 

contrib/devtools/security-check.py

@@ -146,6 +146,12 @@ def check_PE_control_flow(binary) -> bool:
         return True
     return False
 
+def check_PE_Canary(binary) -> bool:
+    '''
+    Check for use of stack canary
+    '''
+    return binary.has_symbol('__stack_chk_fail')
+
 def check_MACHO_NOUNDEFS(binary) -> bool:
     '''
     Check for no undefined references.
@@ -203,6 +209,7 @@ BASE_PE = [
     ('NX', check_NX),
     ('RELOC_SECTION', check_PE_RELOC_SECTION),
     ('CONTROL_FLOW', check_PE_control_flow),
+    ('Canary', check_PE_Canary),
 ]
 
 BASE_MACHO = [

contrib/devtools/test-security-check.py

@@ -94,19 +94,19 @@ class TestSecurityChecks(unittest.TestCase):
         cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc')
         write_testcode(source)
 
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']),
-            (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']),
+        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']),
+            (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW Canary'))
+        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']),
+        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE']),
+        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW'))  # -pie -fPIE does nothing unless --dynamicbase is also supplied
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE']),
+        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed HIGH_ENTROPY_VA CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE']),
+        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full']),
+        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']),
             (0, ''))
 
         clean_files(source, executable)

contrib/install_db4.sh

@@ -32,14 +32,13 @@ check_exists() {
 sha256_check() {
   # Args: <sha256_hash> <filename>
   #
-  if check_exists sha256sum; then
+  if [ "$(uname)" = "FreeBSD" ]; then
+    # sha256sum exists on FreeBSD, but takes different arguments than the GNU version
+    sha256 -c "${1}" "${2}"
+  elif check_exists sha256sum; then
     echo "${1} ${2}" | sha256sum -c
   elif check_exists sha256; then
-    if [ "$(uname)" = "FreeBSD" ]; then
-      sha256 -c "${1}" "${2}"
-    else
     echo "${1} ${2}" | sha256 -c
-    fi
   else
     echo "${1} ${2}" | shasum -a 256 -c
   fi

src/compat/stdin.cpp

@@ -62,7 +62,7 @@ bool StdinReady()
     return false;
 #else
     struct pollfd fds;
-    fds.fd = 0; /* this is STDIN */
+    fds.fd = STDIN_FILENO;
     fds.events = POLLIN;
     return poll(&fds, 1, 0) == 1;
 #endif

src/consensus/tx_verify.h

@@ -63,8 +63,8 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime);
 /**
  * Calculates the block height and previous block's median time past at
  * which the transaction will be considered final in the context of BIP 68.
- * Also removes from the vector of input heights any entries which did not
- * correspond to sequence locked inputs as they do not affect the calculation.
+ * For each input that is not sequence locked, the corresponding entries in
+ * prevHeights are set to 0 as they do not affect the calculation.
  */
 std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>& prevHeights, const CBlockIndex& block);
 

src/httpserver.cpp

@@ -315,14 +315,13 @@ static void http_reject_request_cb(struct evhttp_request* req, void*)
 }
 
 /** Event dispatcher thread */
-static bool ThreadHTTP(struct event_base* base)
+static void ThreadHTTP(struct event_base* base)
 {
     util::ThreadRename("http");
     LogPrint(BCLog::HTTP, "Entering http event loop\n");
     event_base_dispatch(base);
     // Event loop will be interrupted by InterruptHTTPServer()
     LogPrint(BCLog::HTTP, "Exited http event loop\n");
-    return event_base_got_break(base) == 0;
 }
 
 /** Bind HTTP server to specified addresses */

src/netaddress.cpp

@@ -184,7 +184,7 @@ bool CNetAddr::SetInternal(const std::string &name)
 }
 
 namespace torv3 {
-// https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt#n2135
+// https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt?id=7116c9cdaba248aae07a3f1d0e15d9dd102f62c5#n2175
 static constexpr size_t CHECKSUM_LEN = 2;
 static const unsigned char VERSION[] = {3};
 static constexpr size_t TOTAL_LEN = ADDR_TORV3_SIZE + CHECKSUM_LEN + sizeof(VERSION);

src/random.cpp

@@ -64,7 +64,7 @@ static inline int64_t GetPerformanceCounter() noexcept
     __asm__ volatile ("rdtsc" : "=a"(r1), "=d"(r2)); // Constrain r1 to rax and r2 to rdx.
     return (r2 << 32) | r1;
 #else
-    // Fall back to using C++11 clock (usually microsecond or nanosecond precision)
+    // Fall back to using standard library clock (usually microsecond or nanosecond precision)
     return std::chrono::high_resolution_clock::now().time_since_epoch().count();
 #endif
 }
@@ -444,7 +444,7 @@ public:
 
 RNGState& GetRNGState() noexcept
 {
-    // This C++11 idiom relies on the guarantee that static variable are initialized
+    // This idiom relies on the guarantee that static variable are initialized
     // on first call, even when multiple parallel calls are permitted.
     static std::vector<RNGState, secure_allocator<RNGState>> g_rng(1);
     return g_rng[0];

src/random.h

@@ -232,7 +232,7 @@ public:
                                    /* interval [0..0] */ Dur{0};
     };
 
-    // Compatibility with the C++11 UniformRandomBitGenerator concept
+    // Compatibility with the UniformRandomBitGenerator concept
     typedef uint64_t result_type;
     static constexpr uint64_t min() { return 0; }
     static constexpr uint64_t max() { return std::numeric_limits<uint64_t>::max(); }

src/randomenv.cpp

@@ -251,7 +251,7 @@ void RandAddDynamicEnv(CSHA512& hasher)
     gettimeofday(&tv, nullptr);
     hasher << tv;
 #endif
-    // Probably redundant, but also use all the clocks C++11 provides:
+    // Probably redundant, but also use all the standard library clocks:
     hasher << std::chrono::system_clock::now().time_since_epoch().count();
     hasher << std::chrono::steady_clock::now().time_since_epoch().count();
     hasher << std::chrono::high_resolution_clock::now().time_since_epoch().count();

src/reverse_iterator.h

@@ -4,7 +4,7 @@
 #define BITCOIN_REVERSE_ITERATOR_H
 
 /**
- * Template used for reverse iteration in C++11 range-based for loops.
+ * Template used for reverse iteration in range-based for loops.
  *
  *   std::vector<int> v = {1, 2, 3, 4, 5};
  *   for (auto x : reverse_iterate(v))

src/rpc/blockchain.cpp

@@ -790,6 +790,11 @@ static RPCHelpMan getblockfrompeer()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
+    RPCTypeCheck(request.params, {
+        UniValue::VSTR, // blockhash
+        UniValue::VNUM, // peer_id
+    });
+
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     ChainstateManager& chainman = EnsureChainman(node);
     PeerManager& peerman = EnsurePeerman(node);

src/test/random_tests.cpp

@@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(fastrandom_randbits)
     }
 }
 
-/** Does-it-compile test for compatibility with standard C++11 RNG interface. */
+/** Does-it-compile test for compatibility with standard library RNG interface. */
 BOOST_AUTO_TEST_CASE(stdrandom_test)
 {
     FastRandomContext ctx;

test/functional/feature_startupnotify.py

@@ -29,9 +29,14 @@ class StartupNotifyTest(BitcoinTestFramework):
         self.wait_until(lambda: os.path.exists(tmpdir_file))
 
         self.log.info("Test -startupnotify is executed once")
+
+        def get_count():
             with open(tmpdir_file, "r", encoding="utf8") as f:
                 file_content = f.read()
-            assert_equal(file_content.count(FILE_NAME), 1)
+                return file_content.count(FILE_NAME)
+
+        self.wait_until(lambda: get_count() > 0)
+        assert_equal(get_count(), 1)
 
         self.log.info("Test node is fully started")
         assert_equal(self.nodes[0].getblockcount(), 200)

test/functional/rpc_getblockfrompeer.py

@@ -49,14 +49,17 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
         assert_equal(len(peers), 1)
         peer_0_peer_1_id = peers[0]["id"]
 
-        self.log.info("Arguments must be sensible")
-        assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", 0)
+        self.log.info("Arguments must be valid")
+        assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", peer_0_peer_1_id)
+        assert_raises_rpc_error(-3, "Expected type string, got number", self.nodes[0].getblockfrompeer, 1234, peer_0_peer_1_id)
+        assert_raises_rpc_error(-3, "Expected type number, got string", self.nodes[0].getblockfrompeer, short_tip, "0")
 
         self.log.info("We must already have the header")
         assert_raises_rpc_error(-1, "Block header missing", self.nodes[0].getblockfrompeer, "00" * 32, 0)
 
         self.log.info("Non-existent peer generates error")
-        assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id + 1)
+        for peer_id in [-1, peer_0_peer_1_id + 1]:
+            assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_id)
 
         self.log.info("Successful fetch")
         result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id)

test/functional/wallet_labels.py

@@ -20,7 +20,7 @@ from test_framework.wallet_util import test_address
 class WalletLabelsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
-        self.num_nodes = 1
+        self.num_nodes = 2
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -82,8 +82,14 @@ class WalletLabelsTest(BitcoinTestFramework):
             label.add_receive_address(address)
             label.verify(node)
 
+        # Check listlabels when passing 'purpose'
+        node2_addr = self.nodes[1].getnewaddress()
+        node.setlabel(node2_addr, "node2_addr")
+        assert_equal(node.listlabels(purpose="send"), ["node2_addr"])
+        assert_equal(node.listlabels(purpose="receive"), sorted(['coinbase'] + [label.name for label in labels]))
+
         # Check all labels are returned by listlabels.
-        assert_equal(node.listlabels(), sorted(['coinbase'] + [label.name for label in labels]))
+        assert_equal(node.listlabels(), sorted(['coinbase'] + [label.name for label in labels] + ["node2_addr"]))
 
         # Send a transaction to each label.
         for label in labels:

test/sanitizer_suppressions/tsan

@@ -44,8 +44,5 @@ race:libzmq
 # https://github.com/bitcoin/bitcoin/issues/20618
 race:CZMQAbstractPublishNotifier::SendZmqMessage
 
-# https://github.com/bitcoin/bitcoin/pull/20218, https://github.com/bitcoin/bitcoin/pull/20745
-race:epoll_ctl
-
 # https://github.com/bitcoin/bitcoin/issues/23366
 race:std::__1::ios_base::*