backport: Merge bitcoin#31718, 28857, 28825, 29361, 26533, 27302, 29034, 28825 ,28450

ci/README.md

@@ -10,7 +10,7 @@ If the repository is not a fresh git clone, you might have to clean files from p
 The ci needs to perform various sysadmin tasks such as installing packages or writing to the user's home directory.
 While most of the actions are done inside a docker container, this is not possible for all. Thus, cache directories,
 such as the depends cache, previous release binaries, or ccache, are mounted as read-write into the docker container. While it should be fine to run
-the ci system locally on you development box, the ci scripts can generally be assumed to have received less review and
+the ci system locally on your development box, the ci scripts can generally be assumed to have received less review and
 testing compared to other parts of the codebase. If you want to keep the work tree clean, you might want to run the ci
 system in a virtual machine with a Linux operating system of your choice.
 

contrib/guix/README.md

@@ -247,7 +247,7 @@ details.
 * _**SDK_PATH**_
 
   Set the path where _extracted_ SDKs can be found. This is passed through to
-  the depends tree. Note that this is should be set to the _parent_ directory of
+  the depends tree. Note that this should be set to the _parent_ directory of
   the actual SDK (e.g. `SDK_PATH=$HOME/Downloads/macOS-SDKs` instead of
   `$HOME/Downloads/macOS-SDKs/Xcode-26.1.1-17B100-extracted-SDK-with-libcxx-headers`).
 

contrib/tracing/README.md

@@ -155,7 +155,7 @@ $ python3 contrib/tracing/log_raw_p2p_msgs.py ./src/dashd
 
 ```
 Logging raw P2P messages.
-Messages larger that about 32kb will be cut off!
+Messages larger than about 32kb will be cut off!
 Some messages might be lost!
  outbound msg 'inv' from peer 4 (outbound-full-relay, XX.XXX.XX.4:8333) with 253 bytes: 0705000000be2245c8f844c9f763748e1a7…
 …

contrib/tracing/log_raw_p2p_msgs.py

@@ -166,7 +166,7 @@ def handle_outbound(_, data, size):
     bpf["outbound_messages"].open_perf_buffer(handle_outbound)
 
     print("Logging raw P2P messages.")
-    print("Messages larger that about 32kb will be cut off!")
+    print("Messages larger than about 32kb will be cut off!")
     print("Some messages might be lost!")
     while True:
         try:

doc/release-notes-27302.md

@@ -0,0 +1,4 @@
+Configuration
+---
+
+- `bitcoind` and `bitcoin-qt` will now raise an error on startup if a datadir that is being used contains a bitcoin.conf file that will be ignored, which can happen when a datadir= line is used in a bitcoin.conf file. The error message is just a diagnostic intended to prevent accidental misconfiguration, and it can be disabled to restore the previous behavior of using the datadir while ignoring the bitcoin.conf contained in it.

src/init.cpp

@@ -581,6 +581,7 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (%d to %d, default: %d). In addition, unused mempool memory is shared for this cache (see -maxmempool).", nMinDbCache, nMaxDbCache, nDefaultDbCache), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-allowignoredconf", strprintf("For backwards compatibility, treat an unused %s file in the datadir as a warning, not an error.", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-loadblock=<file>", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-maxmempool=<n>", strprintf("Keep the transaction memory pool below <n> megabytes (default: %u)", DEFAULT_MAX_MEMPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-maxorphantxsize=<n>", strprintf("Maximum total size of all orphan transactions in megabytes (default: %u)", DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);

src/qt/test/test_main.cpp

@@ -72,6 +72,9 @@ int main(int argc, char* argv[])
     gArgs.ForceSetArg("-upnp", "0");
     gArgs.ForceSetArg("-natpmp", "0");
 
+    std::string error;
+    if (!gArgs.ReadConfigFiles(error, true)) QWARN(error.c_str());
+
     // Prefer the "minimal" platform for the test instead of the normal default
     // platform ("xcb", "windows", or "cocoa") so tests can't unintentionally
     // interfere with any background GUIs and don't require extra resources.

src/test/util/script.h

@@ -16,6 +16,18 @@ static const CScript P2SH_OP_TRUE{
     << ToByteVector(CScriptID{CScript{} << OP_TRUE})
     << OP_EQUAL};
 
+static const std::vector<uint8_t> EMPTY{};
+static const CScript P2WSH_EMPTY{
+    CScript{}
+    << OP_0
+    << ToByteVector([] {
+           uint256 hash;
+           CSHA256().Write(EMPTY.data(), EMPTY.size()).Finalize(hash.begin());
+           return hash;
+       }())};
+static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TRUE_STACK{{static_cast<uint8_t>(OP_TRUE)}, {}};
+static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TWO_STACK{{static_cast<uint8_t>(OP_2)}, {}};
+
 /** Flags that are not forbidden by an assert in script validation */
 bool IsValidFlagCombination(unsigned flags);
 

src/util/system.cpp

@@ -904,11 +904,6 @@ bool CheckDataDirOption()
     return datadir.empty() || fs::is_directory(fs::absolute(datadir));
 }
 
-fs::path GetConfigFile(const fs::path& configuration_file_path)
-{
-    return AbsPathForConfigVal(configuration_file_path, /*net_specific=*/false);
-}
-
 static bool GetConfigOptions(std::istream& stream, const std::string& filepath, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::list<SectionInfo>& sections)
 {
     std::string str, prefix;
@@ -999,7 +994,8 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file
 
 fs::path ArgsManager::GetConfigFilePath() const
 {
-    return GetConfigFile(GetPathArg("-conf", BITCOIN_CONF_FILENAME));
+    LOCK(cs_args);
+    return *Assert(m_config_path);
 }
 
 bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
@@ -1008,6 +1004,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
         LOCK(cs_args);
         m_settings.ro_config.clear();
         m_config_sections.clear();
+        m_config_path = AbsPathForConfigVal(*this, GetPathArg("-conf", BITCOIN_CONF_FILENAME), /*net_specific=*/false);
     }
 
     const auto conf_path{GetConfigFilePath()};

src/util/system.h

@@ -103,7 +103,6 @@ bool TryCreateDirectories(const fs::path& p);
 fs::path GetDefaultDataDir();
 // Return true if -datadir option points to a valid directory or is not specified.
 bool CheckDataDirOption();
-fs::path GetConfigFile(const fs::path& configuration_file_path);
 #ifdef WIN32
 fs::path GetSpecialFolderPath(int nFolder, bool fCreate = true);
 #endif
@@ -212,6 +211,7 @@ class ArgsManager
     std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
     bool m_accept_any_command GUARDED_BY(cs_args){true};
     std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
+    std::optional<fs::path> m_config_path GUARDED_BY(cs_args);
     mutable fs::path m_cached_blocks_path GUARDED_BY(cs_args);
     mutable fs::path m_cached_datadir_path GUARDED_BY(cs_args);
     mutable fs::path m_cached_network_datadir_path GUARDED_BY(cs_args);

test/functional/README.md

@@ -37,6 +37,10 @@ don't have test cases for.
   `set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of
   the subclass, then locally-defined helper methods, then the `run_test()` method.
 - Use `f'{x}'` for string formatting in preference to `'{}'.format(x)` or `'%s' % x`.
+- Use `platform.system()` for detecting the running operating system and `os.name` to
+  check whether it's a POSIX system (see also the `skip_if_platform_not_{linux,posix}`
+  methods in the `BitcoinTestFramework` class, which can be used to skip a whole test
+  depending on the platform).
 
 #### Naming guidelines
 

test/functional/feature_bind_extra.py

@@ -7,15 +7,12 @@
 that bind happens on the expected ports.
 """
 
-import sys
-
 from test_framework.netutil import (
     addr_to_hex,
     get_bind_addrs,
 )
 from test_framework.test_framework import (
     BitcoinTestFramework,
-    SkipTest,
 )
 from test_framework.util import (
     assert_equal,
@@ -32,12 +29,11 @@ def set_test_params(self):
         self.bind_to_localhost_only = False
         self.num_nodes = 2
 
-    def setup_network(self):
+    def skip_test_if_missing_module(self):
         # Due to OS-specific network stats queries, we only run on Linux.
-        self.log.info("Checking for Linux")
-        if not sys.platform.startswith('linux'):
-            raise SkipTest("This test can only be run on Linux.")
+        self.skip_if_platform_not_linux()
 
+    def setup_network(self):
         loopback_ipv4 = addr_to_hex("127.0.0.1")
 
         # Start custom ports by reusing unused p2p ports

test/functional/feature_config_args.py

@@ -5,9 +5,14 @@
 """Test various command line arguments and configuration file parameters."""
 
 import os
+import platform
+import pathlib
+import re
+import tempfile
 import time
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.test_node import ErrorMatch
 from test_framework import util
 
 
@@ -74,7 +79,7 @@ def test_config_file_parser(self):
         util.write_config(main_conf_file_path, n=0, chain='', extra_config=f'includeconf={inc_conf_file_path}\n')
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('acceptnonstdtxn=1\n')
-        self.nodes[0].assert_start_raises_init_error(extra_args=[f"-conf={main_conf_file_path}"], expected_msg='Error: acceptnonstdtxn is not currently supported for main chain')
+        self.nodes[0].assert_start_raises_init_error(extra_args=[f"-conf={main_conf_file_path}", "-allowignoredconf"], expected_msg='Error: acceptnonstdtxn is not currently supported for main chain')
 
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('nono\n')
@@ -108,6 +113,41 @@ def test_config_file_parser(self):
         with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
             conf.write('')  # clear
 
+    def test_config_file_log(self):
+        # Disable this test for windows currently because trying to override
+        # the default datadir through the environment does not seem to work.
+        if platform.system() == "Windows":
+            return
+
+        self.log.info('Test that correct configuration path is changed when configuration file changes the datadir')
+
+        # Create a temporary directory that will be treated as the default data
+        # directory by bitcoind.
+        env, default_datadir = util.get_temp_default_datadir(pathlib.Path(self.options.tmpdir, "test_config_file_log"))
+        default_datadir.mkdir(parents=True)
+
+        # Write a bitcoin.conf file in the default data directory containing a
+        # datadir= line pointing at the node datadir.
+        node = self.nodes[0]
+        conf_text = pathlib.Path(node.bitcoinconf).read_text()
+        conf_path = default_datadir / "dash.conf"
+        conf_path.write_text(f"datadir={node.datadir}\n{conf_text}")
+
+        # Drop the node -datadir= argument during this test, because if it is
+        # specified it would take precedence over the datadir setting in the
+        # config file.
+        node_args = node.args
+        node.args = [arg for arg in node.args if not arg.startswith("-datadir=")]
+
+        # Check that correct configuration file path is actually logged
+        # (conf_path, not node.bitcoinconf)
+        with self.nodes[0].assert_debug_log(expected_msgs=[f"Config file: {conf_path}"]):
+            self.start_node(0, ["-allowignoredconf"], env=env)
+            self.stop_node(0)
+
+        # Restore node arguments after the test
+        node.args = node_args
+
     def test_invalid_command_line_options(self):
         self.nodes[0].assert_start_raises_init_error(
             expected_msg='Error: Error parsing command line arguments: Can not set -proxy with no value. Please specify value with -proxy=value.',
@@ -278,6 +318,55 @@ def test_connect_with_seednode(self):
                     unexpected_msgs=seednode_ignored):
                 self.restart_node(0, extra_args=[connect_arg, '-seednode=fakeaddress2'])
 
+    def test_ignored_conf(self):
+        self.log.info('Test error is triggered when the datadir in use contains a bitcoin.conf file that would be ignored '
+                      'because a conflicting -conf file argument is passed.')
+        node = self.nodes[0]
+        with tempfile.NamedTemporaryFile(dir=self.options.tmpdir, mode="wt", delete=False) as temp_conf:
+            temp_conf.write(f"datadir={node.datadir}\n")
+        node.assert_start_raises_init_error([f"-conf={temp_conf.name}"], re.escape(
+            f'Error: Data directory "{node.datadir}" contains a "dash.conf" file which is ignored, because a '
+            f'different configuration file "{temp_conf.name}" from command line argument "-conf={temp_conf.name}" '
+            f'is being used instead.') + r"[\s\S]*", match=ErrorMatch.FULL_REGEX)
+
+        # Test that passing a redundant -conf command line argument pointing to
+        # the same bitcoin.conf that would be loaded anyway does not trigger an
+        # error.
+        self.start_node(0, [f'-conf={node.datadir}/bitcoin.conf'])
+        self.stop_node(0)
+
+    def test_ignored_default_conf(self):
+        # Disable this test for windows currently because trying to override
+        # the default datadir through the environment does not seem to work.
+        if platform.system() == "Windows":
+            return
+
+        self.log.info('Test error is triggered when dash.conf in the default data directory sets another datadir '
+                      'and it contains a different dash.conf file that would be ignored')
+
+        # Create a temporary directory that will be treated as the default data
+        # directory by bitcoind.
+        env, default_datadir = util.get_temp_default_datadir(pathlib.Path(self.options.tmpdir, "home"))
+        default_datadir.mkdir(parents=True)
+
+        # Write a dash.conf file in the default data directory containing a
+        # datadir= line pointing at the node datadir. This will trigger a
+        # startup error because the node datadir contains a different
+        # dash.conf that would be ignored.
+        node = self.nodes[0]
+        (default_datadir / "dash.conf").write_text(f"datadir={node.datadir}\n")
+
+        # Drop the node -datadir= argument during this test, because if it is
+        # specified it would take precedence over the datadir setting in the
+        # config file.
+        node_args = node.args
+        node.args = [arg for arg in node.args if not arg.startswith("-datadir=")]
+        node.assert_start_raises_init_error([], re.escape(
+            f'Error: Data directory "{node.datadir}" contains a "dash.conf" file which is ignored, because a '
+            f'different configuration file "{default_datadir}/dash.conf" from data directory "{default_datadir}" '
+            f'is being used instead.') + r"[\s\S]*", env=env, match=ErrorMatch.FULL_REGEX)
+        node.args = node_args
+
     def run_test(self):
         self.test_log_buffer()
         self.test_args_log()
@@ -287,7 +376,10 @@ def run_test(self):
 
 
         self.test_config_file_parser()
+        self.test_config_file_log()
         self.test_invalid_command_line_options()
+        self.test_ignored_conf()
+        self.test_ignored_default_conf()
 
         # Remove the -datadir argument so it doesn't override the config file
         self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")]

test/functional/feature_init.py

@@ -3,9 +3,9 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Stress tests related to node initialization."""
-import os
 from pathlib import Path
 from random import randint
+import platform
 import shutil
 
 from test_framework.test_framework import BitcoinTestFramework, SkipTest
@@ -37,7 +37,7 @@ def run_test(self):
         # and other approaches (like below) don't work:
         #
         #   os.kill(node.process.pid, signal.CTRL_C_EVENT)
-        if os.name == 'nt':
+        if platform.system() == 'Windows':
             raise SkipTest("can't SIGTERM on Windows")
 
         self.stop_node(0)

test/functional/feature_notifications.py

@@ -5,6 +5,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the -alertnotify, -blocknotify, -chainlocknotify, -instantsendnotify and -walletnotify options."""
 import os
+import platform
 
 from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE
 
@@ -16,13 +17,13 @@
 
 # Linux allow all characters other than \x00
 # Windows disallow control characters (0-31) and /\?%:|"<>
-FILE_CHAR_START = 32 if os.name == 'nt' else 1
+FILE_CHAR_START = 32 if platform.system() == 'Windows' else 1
 FILE_CHAR_END = 128
-FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if os.name == 'nt' else '/'
+FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if platform.system() == 'Windows' else '/'
 UNCONFIRMED_HASH_STRING = 'unconfirmed'
 
 def notify_outputname(walletname, txid):
-    return txid if os.name == 'nt' else f'{walletname}_{txid}'
+    return txid if platform.system() == 'Windows' else f'{walletname}_{txid}'
 
 
 class NotificationsTest(DashTestFramework):
@@ -169,7 +170,7 @@ def expect_wallet_notify(self, tx_details):
                 # Universal newline ensures '\n' on 'nt'
                 assert_equal(text[-1], '\n')
                 text = text[:-1]
-                if os.name == 'nt':
+                if platform.system() == 'Windows':
                     # On Windows, echo as above will append a whitespace
                     assert_equal(text[-1], ' ')
                     text = text[:-1]

test/functional/rpc_bind.py

@@ -4,8 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test running dashd with the -rpcbind and -rpcallowip options."""
 
-import sys
-
 from test_framework.netutil import all_interfaces, addr_to_hex, get_bind_addrs, test_ipv6_local
 from test_framework.test_framework import BitcoinTestFramework, SkipTest
 from test_framework.util import assert_equal, assert_raises_rpc_error, get_rpc_proxy, rpc_port, rpc_url
@@ -17,6 +15,10 @@ def set_test_params(self):
         self.num_nodes = 1
         self.supports_cli = False
 
+    def skip_test_if_missing_module(self):
+        # due to OS-specific network stats queries, this test works only on Linux
+        self.skip_if_platform_not_linux()
+
     def setup_network(self):
         self.add_nodes(self.num_nodes, None)
 
@@ -61,14 +63,9 @@ def run_allowip_test(self, allow_ips, rpchost, rpcport):
         self.stop_nodes()
 
     def run_test(self):
-        # due to OS-specific network stats queries, this test works only on Linux
         if sum([self.options.run_ipv4, self.options.run_ipv6, self.options.run_nonloopback]) > 1:
             raise AssertionError("Only one of --ipv4, --ipv6 and --nonloopback can be set")
 
-        self.log.info("Check for linux")
-        if not sys.platform.startswith('linux'):
-            raise SkipTest("This test can only be run on linux.")
-
         self.log.info("Check for ipv6")
         have_ipv6 = test_ipv6_local()
         if not have_ipv6 and not (self.options.run_ipv4 or self.options.run_nonloopback):