Merge bitcoin/bitcoin#29910: refactor: Rename `subprocess.hpp` to follow our header name conventions

08f756bd37 Replace locale-dependent `std::strerror` with `SysErrorString` (Hennadii Stepanov)
d8e4ba4d05 refactor: Rename `subprocess.hpp` to follow our header name conventions (Hennadii Stepanov)

Pull request description:

  This PR renames the header `*.hpp` --> `*.h` and adjusts the header guard name, which makes it available for processing by linters.

  Fixed the following linter warning:
  ```
  The locale dependent function strerror(...) appears to be used:
  src/util/subprocess.h:    std::runtime_error( err_msg + ": " + std::strerror(err_code) )

  Unnecessary locale dependence can cause bugs that are very tricky to isolate and fix. Please avoid using locale-dependent functions if possible.

  Advice not applicable in this specific case? Add an exception by updating the ignore list in /bitcoin/test/lint/lint-locale-dependence.py
  ^---- failure generated from lint-locale-dependence.py
  ```

ACKs for top commit:
  TheCharlatan:
    ACK 08f756bd37

Tree-SHA512: 57a2f01c20eb9552481e428a4969bd59e9ada9f784fe1a45cb62aa9c9152c8e950d336854f45af0e2e5dc7c7b2a1fb216c8f832e3d6ccfb457ad71b6e423231e

ci/test/00_setup_env_native_fuzz_with_msan.sh

@@ -17,7 +17,8 @@ export PACKAGES="ninja-build"
 # BDB generates false-positives and will be removed in future
 export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
 export GOAL="install"
-export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'"
+# _FORTIFY_SOURCE is not compatible with MSAN.
+export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -U_FORTIFY_SOURCE'"
 export USE_MEMORY_SANITIZER="true"
 export RUN_UNIT_TESTS="false"
 export RUN_FUNCTIONAL_TESTS="false"

ci/test/00_setup_env_native_msan.sh

@@ -17,7 +17,8 @@ export PACKAGES="ninja-build"
 # BDB generates false-positives and will be removed in future
 export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
 export GOAL="install"
-export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening"
+# _FORTIFY_SOURCE is not compatible with MSAN.
+export BITCOIN_CONFIG="--with-sanitizers=memory CPPFLAGS='-U_FORTIFY_SOURCE'"
 export USE_MEMORY_SANITIZER="true"
 export RUN_FUNCTIONAL_TESTS="false"
 export CCACHE_MAXSIZE=250M

ci/test/00_setup_env_native_tidy.sh

@@ -16,5 +16,5 @@ export RUN_FUNCTIONAL_TESTS=false
 export RUN_FUZZ_TESTS=false
 export RUN_TIDY=true
 export GOAL="install"
-export BITCOIN_CONFIG="CC=clang-${TIDY_LLVM_V} CXX=clang++-${TIDY_LLVM_V} --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -I/usr/lib/llvm-${TIDY_LLVM_V}/lib/clang/${TIDY_LLVM_V}/include'"
+export BITCOIN_CONFIG="CC=clang-${TIDY_LLVM_V} CXX=clang++-${TIDY_LLVM_V} --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'"
 export CCACHE_MAXSIZE=200M

contrib/devtools/gen-manpages.py

@@ -62,6 +62,10 @@ with tempfile.NamedTemporaryFile('w', suffix='.h2m') as footer:
     # Copyright is the same for all binaries, so just use the first.
     footer.write('[COPYRIGHT]\n')
     footer.write('\n'.join(versions[0][2]).strip())
+    # Create SEE ALSO section
+    footer.write('\n[SEE ALSO]\n')
+    footer.write(', '.join(s.rpartition('/')[2] + '(1)' for s in BINARIES))
+    footer.write('\n')
     footer.flush()
 
     # Call the binaries through help2man to produce a manual page for each of them.

doc/build-freebsd.md

@@ -1,6 +1,6 @@
 # FreeBSD Build Guide
 
-**Updated for FreeBSD [12.3](https://www.freebsd.org/releases/12.3R/announce/)**
+**Updated for FreeBSD [14.0](https://www.freebsd.org/releases/14.0R/announce/)**
 
 This guide describes how to build bitcoind, command-line utilities, and GUI on FreeBSD.
 
@@ -64,10 +64,11 @@ sh/bash: export BDB_PREFIX=[path displayed above]
 #### GUI Dependencies
 ###### Qt5
 
-Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install `qt5`. Skip if you don't intend to use the GUI.
+Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install the necessary parts of Qt. Skip if you don't intend to use the GUI.
 ```bash
-pkg install qt5
+pkg install qt5-buildtools qt5-core qt5-gui qt5-linguisttools qt5-testlib qt5-widgets
 ```
+
 ###### libqrencode
 
 The GUI can encode addresses in a QR Code. To build in QR support for the GUI, install `libqrencode`. Skip if not using the GUI or don't want QR code functionality.

doc/build-openbsd.md

@@ -1,6 +1,6 @@
 # OpenBSD Build Guide
 
-**Updated for OpenBSD [7.4](https://www.openbsd.org/74.html)**
+**Updated for OpenBSD [7.5](https://www.openbsd.org/75.html)**
 
 This guide describes how to build bitcoind, command-line utilities, and GUI on OpenBSD.
 
@@ -63,7 +63,7 @@ export BDB_PREFIX="/path/to/bitcoin/depends/x86_64-unknown-openbsd"
 Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, Qt 5 is required.
 
 ```bash
-pkg_add qt5
+pkg_add qtbase qttools
 ```
 
 ## Building Bitcoin Core

src/Makefile.am

@@ -320,7 +320,7 @@ BITCOIN_CORE_H = \
   util/sock.h \
   util/spanparsing.h \
   util/string.h \
-  util/subprocess.hpp \
+  util/subprocess.h \
   util/syserror.h \
   util/task_runner.h \
   util/thread.h \

src/common/run_command.cpp

@@ -12,7 +12,7 @@
 #include <univalue.h>
 
 #ifdef ENABLE_EXTERNAL_SIGNER
-#include <util/subprocess.hpp>
+#include <util/subprocess.h>
 #endif // ENABLE_EXTERNAL_SIGNER
 
 UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in)

src/core_read.cpp

@@ -256,6 +256,6 @@ util::Result<int> SighashFromStr(const std::string& sighash)
     if (it != map_sighash_values.end()) {
         return it->second;
     } else {
-        return util::Error{Untranslated(sighash + " is not a valid sighash parameter.")};
+        return util::Error{Untranslated("'" + sighash + "' is not a valid sighash parameter.")};
     }
 }

src/netbase.cpp

@@ -632,10 +632,7 @@ std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connecti
 
 std::unique_ptr<Sock> Proxy::Connect() const
 {
-    if (!IsValid()) {
-        LogPrintf("Cannot connect to invalid Proxy\n");
-        return {};
-    }
+    if (!IsValid()) return {};
 
     if (!m_is_unix_socket) return ConnectDirectly(proxy, /*manual_connection=*/true);
 
@@ -656,7 +653,6 @@ std::unique_ptr<Sock> Proxy::Connect() const
     socklen_t len = sizeof(addrun);
 
     if(!ConnectToSocket(*sock, (struct sockaddr*)&addrun, len, path, /*manual_connection=*/true)) {
-        LogPrintf("Cannot connect to socket for %s\n", path);
         return {};
     }
 

src/script/sign.cpp

@@ -295,7 +295,7 @@ struct TapSatisfier: Satisfier<XOnlyPubKey> {
     //! Conversion from a raw xonly public key.
     template <typename I>
     std::optional<XOnlyPubKey> FromPKBytes(I first, I last) const {
-        CHECK_NONFATAL(last - first == 32);
+        if (last - first != 32) return {};
         XOnlyPubKey pubkey;
         std::copy(first, last, pubkey.begin());
         return pubkey;

src/test/script_tests.cpp

@@ -1254,6 +1254,30 @@ BOOST_AUTO_TEST_CASE(script_combineSigs)
     BOOST_CHECK(combined.scriptSig == partial3c);
 }
 
+/**
+ * Reproduction of an exception incorrectly raised when parsing a public key inside a TapMiniscript.
+ */
+BOOST_AUTO_TEST_CASE(sign_invalid_miniscript)
+{
+    FillableSigningProvider keystore;
+    SignatureData sig_data;
+    CMutableTransaction prev, curr;
+
+    // Create a Taproot output which contains a leaf in which a non-32 bytes push is used where a public key is expected
+    // by the Miniscript parser. This offending Script was found by the RPC fuzzer.
+    const auto invalid_pubkey{ParseHex("173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292")};
+    TaprootBuilder builder;
+    builder.Add(0, {invalid_pubkey}, 0xc0);
+    XOnlyPubKey nums{ParseHex("50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0")};
+    builder.Finalize(nums);
+    prev.vout.emplace_back(0, GetScriptForDestination(builder.GetOutput()));
+    curr.vin.emplace_back(COutPoint{prev.GetHash(), 0});
+    sig_data.tr_spenddata = builder.GetSpendData();
+
+    // SignSignature can fail but it shouldn't raise an exception (nor crash).
+    BOOST_CHECK(!SignSignature(keystore, CTransaction(prev), curr, 0, SIGHASH_ALL, sig_data));
+}
+
 BOOST_AUTO_TEST_CASE(script_standard_push)
 {
     ScriptError err;

src/test/system_tests.cpp

@@ -11,7 +11,7 @@
 #include <univalue.h>
 
 #ifdef ENABLE_EXTERNAL_SIGNER
-#include <util/subprocess.hpp>
+#include <util/subprocess.h>
 #endif // ENABLE_EXTERNAL_SIGNER
 
 #include <boost/test/unit_test.hpp>

src/util/subprocess.h

@@ -33,8 +33,10 @@ Documentation for C++ subprocessing library.
 @version 1.0.0
 */
 
-#ifndef SUBPROCESS_HPP
-#define SUBPROCESS_HPP
+#ifndef BITCOIN_UTIL_SUBPROCESS_H
+#define BITCOIN_UTIL_SUBPROCESS_H
+
+#include <util/syserror.h>
 
 #include <algorithm>
 #include <cassert>
@@ -150,7 +152,7 @@ class OSError: public std::runtime_error
 {
 public:
   OSError(const std::string& err_msg, int err_code):
-    std::runtime_error( err_msg + ": " + std::strerror(err_code) )
+    std::runtime_error(err_msg + ": " + SysErrorString(err_code))
   {}
 };
 
@@ -1609,4 +1611,4 @@ namespace detail {
 
 }
 
-#endif // SUBPROCESS_HPP
+#endif // BITCOIN_UTIL_SUBPROCESS_H

test/functional/rpc_psbt.py

@@ -877,7 +877,7 @@ class PSBTTest(BitcoinTestFramework):
             assert_equal(comb_psbt, psbt)
 
         self.log.info("Test walletprocesspsbt raises if an invalid sighashtype is passed")
-        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all")
+        assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all")
 
         self.log.info("Test decoding PSBT with per-input preimage types")
         # note that the decodepsbt RPC doesn't check whether preimages and hashes match
@@ -983,7 +983,7 @@ class PSBTTest(BitcoinTestFramework):
         self.nodes[2].sendrawtransaction(processed_psbt['hex'])
 
         self.log.info("Test descriptorprocesspsbt raises if an invalid sighashtype is passed")
-        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all")
+        assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all")
 
 
 if __name__ == '__main__':

test/functional/rpc_signrawtransactionwithkey.py

@@ -124,7 +124,7 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework):
         self.log.info("Test signing transaction with invalid sighashtype")
         tx = self.nodes[0].createrawtransaction(INPUTS, OUTPUTS)
         privkeys = [self.nodes[0].get_deterministic_priv_key().key]
-        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all")
+        assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all")
 
     def run_test(self):
         self.successful_signing_test()

test/functional/wallet_signrawtransactionwithwallet.py

@@ -55,7 +55,7 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework):
 
     def test_with_invalid_sighashtype(self):
         self.log.info("Test signrawtransactionwithwallet raises if an invalid sighashtype is passed")
-        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all")
+        assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all")
 
     def script_verification_error_test(self):
         """Create and sign a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script.