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

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/chain.h

@@ -10,14 +10,19 @@
 #include <consensus/params.h>
 #include <flatfile.h>
 #include <primitives/block.h>
+#include <serialize.h>
 #include <sync.h>
 #include <uint256.h>
 
+#include <algorithm>
+#include <cassert>
+#include <cstdint>
+#include <string>
 #include <vector>
 
 /**
  * Maximum amount of time that a block timestamp is allowed to exceed the
- * current network-adjusted time before the block will be accepted.
+ * current time before the block will be accepted.
  */
 static constexpr int64_t MAX_FUTURE_BLOCK_TIME = 2 * 60 * 60;
 

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/net.cpp

@@ -692,10 +692,8 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete)
 }
 
 V1Transport::V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noexcept :
-    m_node_id(node_id), hdrbuf(nTypeIn, nVersionIn), vRecv(nTypeIn, nVersionIn)
+    m_magic_bytes{Params().MessageStart()}, m_node_id(node_id), hdrbuf(nTypeIn, nVersionIn), vRecv(nTypeIn, nVersionIn)
 {
-    assert(std::size(Params().MessageStart()) == std::size(m_magic_bytes));
-    std::copy(std::begin(Params().MessageStart()), std::end(Params().MessageStart()), m_magic_bytes);
     LOCK(m_recv_mutex);
     Reset();
 }

src/net.h

@@ -407,7 +407,7 @@ class Transport {
 class V1Transport final : public Transport
 {
 private:
-    CMessageHeader::MessageStartChars m_magic_bytes;
+    const CMessageHeader::MessageStartChars m_magic_bytes;
     const NodeId m_node_id; // Only for logging
     mutable Mutex m_recv_mutex; //!< Lock for receive state
     mutable CHash256 hasher GUARDED_BY(m_recv_mutex);

src/net_processing.cpp

@@ -37,6 +37,7 @@
 #include <util/std23.h>
 #include <util/strencodings.h>
 #include <util/system.h>
+#include <util/time.h>
 #include <util/trace.h>
 #include <validation.h>
 
@@ -65,7 +66,6 @@
 
 #include <algorithm>
 #include <atomic>
-#include <chrono>
 #include <future>
 #include <list>
 #include <memory>

src/node/blockstorage.cpp

@@ -400,6 +400,23 @@ bool BlockManager::LoadBlockIndexDB()
     return true;
 }
 
+void BlockManager::ScanAndUnlinkAlreadyPrunedFiles()
+{
+    AssertLockHeld(::cs_main);
+    if (!m_have_pruned) {
+        return;
+    }
+
+    std::set<int> block_files_to_prune;
+    for (int file_number = 0; file_number < m_last_blockfile; file_number++) {
+        if (m_blockfile_info[file_number].nSize == 0) {
+            block_files_to_prune.insert(file_number);
+        }
+    }
+
+    UnlinkPrunedFiles(block_files_to_prune);
+}
+
 const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data)
 {
     const MapCheckpoints& checkpoints = data.mapCheckpoints;
@@ -575,11 +592,14 @@ uint64_t BlockManager::CalculateCurrentUsage()
 
 void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune)
 {
+    std::error_code ec;
     for (std::set<int>::iterator it = setFilesToPrune.begin(); it != setFilesToPrune.end(); ++it) {
         FlatFilePos pos(*it, 0);
-        fs::remove(BlockFileSeq().FileName(pos));
-        fs::remove(UndoFileSeq().FileName(pos));
-        LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it);
+        const bool removed_blockfile{fs::remove(BlockFileSeq().FileName(pos), ec)};
+        const bool removed_undofile{fs::remove(UndoFileSeq().FileName(pos), ec)};
+        if (removed_blockfile || removed_undofile) {
+            LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it);
+        }
     }
 }
 

src/node/blockstorage.h

@@ -169,6 +169,13 @@ class BlockManager
     bool WriteBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
     bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    /**
+     * Remove any pruned block & undo files that are still on disk.
+     * This could happen on some systems if the file was still being read while unlinked,
+     * or if we crash before unlinking.
+     */
+    void ScanAndUnlinkAlreadyPrunedFiles() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
     CBlockIndex* AddToBlockIndex(const CBlockHeader& block, const uint256& hash, CBlockIndex*& best_header,
                                  enum BlockStatus nStatus = BLOCK_VALID_TREE)
         EXCLUSIVE_LOCKS_REQUIRED(cs_main);

src/node/miner.cpp

@@ -19,9 +19,9 @@
 #include <policy/policy.h>
 #include <pow.h>
 #include <primitives/transaction.h>
-#include <timedata.h>
 #include <util/moneystr.h>
 #include <util/system.h>
+#include <util/time.h>
 #include <validation.h>
 
 #include <chainlock/chainlock.h>

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/rpc/mining.cpp

@@ -39,6 +39,7 @@
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/system.h>
+#include <util/time.h>
 #include <util/translation.h>
 #include <validation.h>
 #include <validationinterface.h>

src/test/blockmanager_tests.cpp

@@ -12,6 +12,8 @@
 
 using node::BlockManager;
 using node::BLOCK_SERIALIZATION_HEADER_SIZE;
+using node::MAX_BLOCKFILE_SIZE;
+using node::OpenBlockFile;
 
 // use BasicTestingSetup here for the data directory configuration, setup, and cleanup
 BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup)
@@ -42,4 +44,45 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos)
     BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(params->GenesisBlock(), CLIENT_VERSION) + BLOCK_SERIALIZATION_HEADER_SIZE);
 }
 
+BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain100Setup)
+{
+    // Cap last block file size, and mine new block in a new block file.
+    const auto& chainman = Assert(m_node.chainman);
+    auto& blockman = chainman->m_blockman;
+    const CBlockIndex* old_tip{WITH_LOCK(chainman->GetMutex(), return chainman->ActiveChain().Tip())};
+    WITH_LOCK(chainman->GetMutex(), blockman.GetBlockFileInfo(old_tip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE);
+    CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
+
+    // Prune the older block file, but don't unlink it
+    int file_number;
+    {
+        LOCK(chainman->GetMutex());
+        file_number = old_tip->GetBlockPos().nFile;
+        blockman.PruneOneBlockFile(file_number);
+    }
+
+    const FlatFilePos pos(file_number, 0);
+
+    // Check that the file is not unlinked after ScanAndUnlinkAlreadyPrunedFiles
+    // if m_have_pruned is not yet set
+    WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles());
+    BOOST_CHECK(!AutoFile(OpenBlockFile(pos, true)).IsNull());
+
+    // Check that the file is unlinked after ScanAndUnlinkAlreadyPrunedFiles
+    // once m_have_pruned is set
+    blockman.m_have_pruned = true;
+    WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles());
+    BOOST_CHECK(AutoFile(OpenBlockFile(pos, true)).IsNull());
+
+    // Check that calling with already pruned files doesn't cause an error
+    WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles());
+
+    // Check that the new tip file has not been removed
+    const CBlockIndex* new_tip{WITH_LOCK(chainman->GetMutex(), return chainman->ActiveChain().Tip())};
+    BOOST_CHECK_NE(old_tip, new_tip);
+    const int new_file_number{WITH_LOCK(chainman->GetMutex(), return new_tip->GetBlockPos().nFile)};
+    const FlatFilePos new_pos(new_file_number, 0);
+    BOOST_CHECK(!AutoFile(OpenBlockFile(new_pos, true)).IsNull());
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/serialize_tests.cpp

@@ -84,6 +84,8 @@ BOOST_AUTO_TEST_CASE(sizes)
     BOOST_CHECK_EQUAL(GetSerializeSize(int64_t(0), 0), 8U);
     BOOST_CHECK_EQUAL(GetSerializeSize(uint64_t(0), 0), 8U);
     BOOST_CHECK_EQUAL(GetSerializeSize(bool(0), 0), 1U);
+    BOOST_CHECK_EQUAL(GetSerializeSize(std::array<uint8_t, 1>{0}, 0), 1U);
+    BOOST_CHECK_EQUAL(GetSerializeSize(std::array<uint8_t, 2>{0, 0}, 0), 2U);
 }
 
 BOOST_AUTO_TEST_CASE(varints)
@@ -178,6 +180,16 @@ BOOST_AUTO_TEST_CASE(vector_bool)
     BOOST_CHECK(SerializeHash(vec1) == SerializeHash(vec2));
 }
 
+BOOST_AUTO_TEST_CASE(array)
+{
+    std::array<uint8_t, 32> array1{1, 0, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1, 0, 1, 0, 0, 1};
+    DataStream ds;
+    ds << array1;
+    std::array<uint8_t, 32> array2;
+    ds >> array2;
+    BOOST_CHECK(array1 == array2);
+}
+
 BOOST_AUTO_TEST_CASE(noncanonical)
 {
     // Write some non-canonical CompactSize encodings, and

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/timedata.h

@@ -5,11 +5,8 @@
 #ifndef BITCOIN_TIMEDATA_H
 #define BITCOIN_TIMEDATA_H
 
-#include <util/time.h>
-
 #include <algorithm>
 #include <cassert>
-#include <chrono>
 #include <cstdint>
 #include <vector>
 

src/validation.cpp

@@ -4693,6 +4693,8 @@ bool ChainstateManager::LoadBlockIndex()
         bool ret{m_blockman.LoadBlockIndexDB()};
         if (!ret) return false;
 
+        m_blockman.ScanAndUnlinkAlreadyPrunedFiles();
+
         std::vector<CBlockIndex*> vSortedByHeight{m_blockman.GetAllBlockIndices()};
         std::sort(vSortedByHeight.begin(), vSortedByHeight.end(),
                   CBlockIndexHeightOnlyComparator());

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