Selected Bitcoin v25 and onwards patches

src/index/base.cpp

@@ -151,9 +151,9 @@ void BaseIndex::ThreadSync()
                 const CBlockIndex* pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain);
                 if (!pindex_next) {
                     m_best_block_index = pindex;
-                    m_synced = true;
                     // No need to handle errors in Commit. See rationale above.
                     Commit();
+                    m_synced = true;
                     break;
                 }
                 if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) {

src/init.cpp

@@ -238,9 +238,9 @@ void Shutdown(NodeContext& node)
 
     // After everything has been shut down, but before things get flushed, stop the
     // CScheduler/checkqueue, scheduler and load block thread.
+    if (node.chainman && node.chainman->m_load_block.joinable()) node.chainman->m_load_block.join();
     if (node.scheduler) node.scheduler->stop();
     if (node.reverification_scheduler) node.reverification_scheduler->stop();
-    if (node.chainman && node.chainman->m_load_block.joinable()) node.chainman->m_load_block.join();
     StopScriptCheckWorkerThreads();
 
     // After the threads that potentially access these pointers have been stopped,

src/net.cpp

@@ -2221,7 +2221,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() const
     }
 
     for (const std::string& strAddNode : lAddresses) {
-        CService service(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode)));
+        CService service{MaybeFlipIPv6toCJDNS(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode)))};
         AddedNodeInfo addedNode{strAddNode, CService(), false, false};
         if (service.IsValid()) {
             // strAddNode is an IP:port

src/rpc/blockchain.cpp

@@ -279,12 +279,12 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIn
                 const CTxUndo* txundo = (have_undo && i > 0) ? &blockUndo.vtxundo.at(i - 1) : nullptr;
                 UniValue objTx(UniValue::VOBJ);
                 TxToUniv(*tx, uint256(), objTx, true, RPCSerializationFlags(), txundo, verbosity);
-                txs.push_back(objTx);
+                txs.push_back(std::move(objTx));
             }
             break;
     }
 
-    result.pushKV("tx", txs);
+    result.pushKV("tx", std::move(txs));
 
     return result;
 }

src/rpc/request.cpp

@@ -78,6 +78,8 @@ static fs::path GetAuthCookieFile(bool temp=false)
     return AbsPathForConfigVal(fs::PathFromString(arg));
 }
 
+static bool g_generated_cookie = false;
+
 bool GenerateAuthCookie(std::string *cookie_out)
 {
     const size_t COOKIE_SIZE = 32;
@@ -103,6 +105,7 @@ bool GenerateAuthCookie(std::string *cookie_out)
         LogPrintf("Unable to rename cookie authentication file %s to %s\n", fs::PathToString(filepath_tmp), fs::PathToString(filepath));
         return false;
     }
+    g_generated_cookie = true;
     LogPrintf("Generated RPC authentication cookie %s\n", fs::PathToString(filepath));
 
     if (cookie_out)
@@ -168,7 +171,10 @@ bool GetMainchainAuthCookie(std::string *cookie_out)
 void DeleteAuthCookie()
 {
     try {
-        fs::remove(GetAuthCookieFile());
+        if (g_generated_cookie) {
+            // Delete the cookie file if it was generated by this process
+            fs::remove(GetAuthCookieFile());
+        }
     } catch (const fs::filesystem_error& e) {
         LogPrintf("%s: Unable to remove random auth cookie file: %s\n", __func__, fsbridge::get_filesystem_error_message(e));
     }

src/test/net_peer_connection_tests.cpp

@@ -0,0 +1,163 @@
+// Copyright (c) 2023-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <chainparams.h>
+#include <compat/compat.h>
+#include <net.h>
+#include <net_processing.h>
+#include <netaddress.h>
+#include <netbase.h>
+#include <netgroup.h>
+#include <node/connection_types.h>
+#include <node/protocol_version.h>
+#include <protocol.h>
+#include <random.h>
+#include <test/util/logging.h>
+#include <test/util/net.h>
+#include <test/util/random.h>
+#include <test/util/setup_common.h>
+#include <tinyformat.h>
+#include <util/chaintype.h>
+
+#include <algorithm>
+#include <cstdint>
+#include <memory>
+#include <optional>
+#include <string>
+#include <vector>
+
+#include <boost/test/unit_test.hpp>
+
+struct LogIPsTestingSetup : public TestingSetup {
+    LogIPsTestingSetup()
+        : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-logips"}} {}
+};
+
+BOOST_FIXTURE_TEST_SUITE(net_peer_connection_tests, LogIPsTestingSetup)
+
+static CService ip(uint32_t i)
+{
+    struct in_addr s;
+    s.s_addr = i;
+    return CService{CNetAddr{s}, Params().GetDefaultPort()};
+}
+
+/** Create a peer and connect to it. If the optional `address` (IP/CJDNS only) isn't passed, a random address is created. */
+static void AddPeer(NodeId& id, std::vector<CNode*>& nodes, PeerManager& peerman, ConnmanTestMsg& connman, ConnectionType conn_type, bool onion_peer = false, std::optional<std::string> address = std::nullopt)
+{
+    CAddress addr{};
+
+    if (address.has_value()) {
+        addr = CAddress{MaybeFlipIPv6toCJDNS(LookupNumeric(address.value(), Params().GetDefaultPort())), NODE_NONE};
+    } else if (onion_peer) {
+        auto tor_addr{g_insecure_rand_ctx.randbytes(ADDR_TORV3_SIZE)};
+        BOOST_REQUIRE(addr.SetSpecial(OnionToString(tor_addr)));
+    }
+
+    while (!addr.IsLocal() && !addr.IsRoutable()) {
+        addr = CAddress{ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE};
+    }
+
+    BOOST_REQUIRE(addr.IsValid());
+
+    const bool inbound_onion{onion_peer && conn_type == ConnectionType::INBOUND};
+
+    nodes.emplace_back(new CNode{++id,
+                                 /*sock=*/nullptr,
+                                 addr,
+                                 /*nKeyedNetGroupIn=*/0,
+                                 /*nLocalHostNonceIn=*/0,
+                                 CAddress{},
+                                 /*addrNameIn=*/"",
+                                 conn_type,
+                                 /*inbound_onion=*/inbound_onion});
+    CNode& node = *nodes.back();
+    node.SetCommonVersion(PROTOCOL_VERSION);
+
+    peerman.InitializeNode(node, ServiceFlags(NODE_NETWORK | NODE_WITNESS));
+    node.fSuccessfullyConnected = true;
+
+    connman.AddTestNode(node);
+}
+
+BOOST_AUTO_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection)
+{
+    auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params());
+    auto peerman = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {});
+    NodeId id{0};
+    std::vector<CNode*> nodes;
+
+    // Connect a localhost peer.
+    {
+        ASSERT_DEBUG_LOG("Added connection to 127.0.0.1:8333 peer=1");
+        AddPeer(id, nodes, *peerman, *connman, ConnectionType::MANUAL, /*onion_peer=*/false, /*address=*/"127.0.0.1");
+        BOOST_REQUIRE(nodes.back() != nullptr);
+    }
+
+    // Call ConnectNode(), which is also called by RPC addnode onetry, for a localhost
+    // address that resolves to multiple IPs, including that of the connected peer.
+    // The connection attempt should consistently fail due to the check in ConnectNode().
+    for (int i = 0; i < 10; ++i) {
+        ASSERT_DEBUG_LOG("Not opening a connection to localhost, already connected to 127.0.0.1:8333");
+        BOOST_CHECK(!connman->ConnectNodePublic(*peerman, "localhost", ConnectionType::MANUAL));
+    }
+
+    // Add 3 more peer connections.
+    AddPeer(id, nodes, *peerman, *connman, ConnectionType::OUTBOUND_FULL_RELAY);
+    AddPeer(id, nodes, *peerman, *connman, ConnectionType::BLOCK_RELAY, /*onion_peer=*/true);
+    AddPeer(id, nodes, *peerman, *connman, ConnectionType::INBOUND);
+
+    // Add a CJDNS peer connection.
+    AddPeer(id, nodes, *peerman, *connman, ConnectionType::INBOUND, /*onion_peer=*/false,
+            /*address=*/"[fc00:3344:5566:7788:9900:aabb:ccdd:eeff]:1234");
+    BOOST_CHECK(nodes.back()->IsInboundConn());
+    BOOST_CHECK_EQUAL(nodes.back()->ConnectedThroughNetwork(), Network::NET_CJDNS);
+
+    BOOST_TEST_MESSAGE("Call AddNode() for all the peers");
+    for (auto node : connman->TestNodes()) {
+        BOOST_CHECK(connman->AddNode({/*m_added_node=*/node->addr.ToStringAddrPort(), /*m_use_v2transport=*/true}));
+        BOOST_TEST_MESSAGE(strprintf("peer id=%s addr=%s", node->GetId(), node->addr.ToStringAddrPort()));
+    }
+
+    BOOST_TEST_MESSAGE("\nCall AddNode() with 2 addrs resolving to existing localhost addnode entry; neither should be added");
+    BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.0.0.1", /*m_use_v2transport=*/true}));
+    // OpenBSD doesn't support the IPv4 shorthand notation with omitted zero-bytes.
+#if !defined(__OpenBSD__)
+    BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.1", /*m_use_v2transport=*/true}));
+#endif
+
+    BOOST_TEST_MESSAGE("\nExpect GetAddedNodeInfo to return expected number of peers with `include_connected` true/false");
+    BOOST_CHECK_EQUAL(connman->GetAddedNodeInfo(/*include_connected=*/true).size(), nodes.size());
+    BOOST_CHECK(connman->GetAddedNodeInfo(/*include_connected=*/false).empty());
+
+    // Test AddedNodesContain()
+    for (auto node : connman->TestNodes()) {
+        BOOST_CHECK(connman->AddedNodesContain(node->addr));
+    }
+    AddPeer(id, nodes, *peerman, *connman, ConnectionType::OUTBOUND_FULL_RELAY);
+    BOOST_CHECK(!connman->AddedNodesContain(nodes.back()->addr));
+
+    BOOST_TEST_MESSAGE("\nPrint GetAddedNodeInfo contents:");
+    for (const auto& info : connman->GetAddedNodeInfo(/*include_connected=*/true)) {
+        BOOST_TEST_MESSAGE(strprintf("\nadded node: %s", info.m_params.m_added_node));
+        BOOST_TEST_MESSAGE(strprintf("connected: %s", info.fConnected));
+        if (info.fConnected) {
+            BOOST_TEST_MESSAGE(strprintf("IP address: %s", info.resolvedAddress.ToStringAddrPort()));
+            BOOST_TEST_MESSAGE(strprintf("direction: %s", info.fInbound ? "inbound" : "outbound"));
+        }
+    }
+
+    BOOST_TEST_MESSAGE("\nCheck that all connected peers are correctly detected as connected");
+    for (auto node : connman->TestNodes()) {
+        BOOST_CHECK(connman->AlreadyConnectedPublic(node->addr));
+    }
+
+    // Clean up
+    for (auto node : connman->TestNodes()) {
+        peerman->FinalizeNode(*node);
+    }
+    connman->ClearTestNodes();
+}
+
+BOOST_AUTO_TEST_SUITE_END()

test/functional/feature_filelock.py

@@ -27,7 +27,8 @@ def run_test(self):
         self.log.info("Check that we can't start a second bitcoind instance using the same datadir")
         expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running."
         self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir}', '-noserver'], expected_msg=expected_msg)
-
+        cookie_file = datadir + "/.cookie"
+        assert os.path.isfile(cookie_file)  # should not be deleted during the second bitcoind instance shutdown
         if self.is_wallet_compiled():
             def check_wallet_filelock(descriptors):
                 wallet_name = ''.join([random.choice(string.ascii_lowercase) for _ in range(6)])