Merge #6321: backport: trivial 2024 10 08

90744d0d65 Merge bitcoin/bitcoin#25115: scripted-diff: replace non-standard fixed width integer types (`u_int`... -> `uint`...) (fanquake)
e4f8b7097d Merge bitcoin/bitcoin#24852: util: optimize HexStr (laanwj)
1288494d4a Merge bitcoin/bitcoin#24976: netgroup: Follow-up for #22910 (fanquake)
656f525855 Merge bitcoin-core/gui#543: peers-tab: add connection duration column to tableview (Hennadii Stepanov)
33b9771ebc Merge bitcoin/bitcoin#24749: test: use MiniWallet for mempool_unbroadcast.py (MarcoFalke)
36e9b5fead Merge bitcoin/bitcoin#24381: test: Run symlink regression tests on Windows (laanwj)
a1691c7c2a Merge bitcoin/bitcoin#24102: mempool: Run coin.IsSpent only once in a row (MarcoFalke)
acbf718b57 Merge bitcoin/bitcoin#23976: document and clean up MaybeUpdateMempoolForReorg (MarcoFalke)
73e1861576 Merge bitcoin/bitcoin#23750: rpcwallet: mention labels are disabled for ranged descriptors (MarcoFalke)
c2fd4fe379 Merge bitcoin/bitcoin#23515: test: Return the largest utxo in MiniWallet.get_utxo (MarcoFalke)
7455b5557a Merge bitcoin-core/gui#454: Use only Qt translation primitives in GUI code (Hennadii Stepanov)
95aeb6a08d Merge bitcoin-core/gui#436: Include vout when copying transaction ID from coin selection (Hennadii Stepanov)
02b5fce942 Merge bitcoin-core/gui#318: Add `Copy address` Peers Tab Context Menu Action (Hennadii Stepanov)
e4774b9dad Merge bitcoin-core/gui#384: Add copy IP/Netmask action for banned peer (Hennadii Stepanov)

Pull request description:

  ## Issue being fixed or feature implemented
  batch of trivial backports

  ## What was done?

  ## How Has This Been Tested?

  ## 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:
  UdjinM6:
    utACK 90744d0d65
  kwvg:
    utACK 90744d0d65

Tree-SHA512: 64b562f559f0be9f04b033a642aea3f9a9b49c69a957fa2fd4a1dbc263c465ca26ef2db987b7200cf861ff3989a54376273eeb224f60a54308dfa19897b67724

src/Makefile.bench.include

@@ -45,6 +45,7 @@ bench_bench_dash_SOURCES = \
   bench/pool.cpp \
   bench/rpc_blockchain.cpp \
   bench/rpc_mempool.cpp \
+  bench/strencodings.cpp \
   bench/util_time.cpp \
   bench/base58.cpp \
   bench/bech32.cpp \

src/bench/strencodings.cpp

@@ -0,0 +1,18 @@
+// Copyright (c) 2022 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 <bench/bench.h>
+#include <bench/data.h>
+#include <util/strencodings.h>
+
+static void HexStrBench(benchmark::Bench& bench)
+{
+    auto const& data = benchmark::data::block813851;
+    bench.batch(data.size()).unit("byte").run([&] {
+        auto hex = HexStr(data);
+        ankerl::nanobench::doNotOptimizeAway(hex);
+    });
+}
+
+BENCHMARK(HexStrBench);

src/netgroup.cpp

@@ -71,7 +71,7 @@ std::vector<unsigned char> NetGroupManager::GetGroup(const CNetAddr& address) co
     // ...for the last byte, push nBits and for the rest of the byte push 1's
     if (nBits > 0) {
         assert(num_bytes < addr_bytes.size());
-        vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1));
+        vchRet.push_back(addr_bytes[num_bytes + nStartByte] | ((1 << (8 - nBits)) - 1));
     }
 
     return vchRet;

src/qt/bitcoin.cpp

@@ -156,10 +156,11 @@ static bool InitSettings()
 
     std::vector<std::string> errors;
     if (!gArgs.ReadSettingsFile(&errors)) {
-        bilingual_str error = _("Settings file could not be read");
-        InitError(Untranslated(strprintf("%s:\n%s\n", error.original, MakeUnorderedList(errors))));
+        std::string error = QT_TRANSLATE_NOOP("dash-core", "Settings file could not be read");
+        std::string error_translated = QCoreApplication::translate("dash-core", error.c_str()).toStdString();
+        InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors))));
 
-        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort);
+        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Reset | QMessageBox::Abort);
         /*: Explanatory text shown on startup when the settings file cannot be read.
             Prompts user to make a choice between resetting or aborting. */
         messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));
@@ -178,10 +179,11 @@ static bool InitSettings()
 
     errors.clear();
     if (!gArgs.WriteSettingsFile(&errors)) {
-        bilingual_str error = _("Settings file could not be written");
-        InitError(Untranslated(strprintf("%s:\n%s\n", error.original, MakeUnorderedList(errors))));
+        std::string error = QT_TRANSLATE_NOOP("dash-core", "Settings file could not be written");
+        std::string error_translated = QCoreApplication::translate("dash-core", error.c_str()).toStdString();
+        InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors))));
 
-        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Ok);
+        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Ok);
         /*: Explanatory text shown on startup when the settings file could not be written.
             Prompts user to check that we have the ability to write to the file.
             Explains that the user has the option of running without a settings file.*/

src/qt/coincontroldialog.cpp

@@ -67,7 +67,7 @@ CoinControlDialog::CoinControlDialog(CCoinControl& coin_control, WalletModel* _m
     contextMenu->addAction(tr("&Copy address"), this, &CoinControlDialog::copyAddress);
     contextMenu->addAction(tr("Copy &label"), this, &CoinControlDialog::copyLabel);
     contextMenu->addAction(tr("Copy &amount"), this, &CoinControlDialog::copyAmount);
-    copyTransactionHashAction = contextMenu->addAction(tr("Copy transaction &ID"), this, &CoinControlDialog::copyTransactionHash);
+    m_copy_transaction_outpoint_action = contextMenu->addAction(tr("Copy transaction &ID and output index"), this, &CoinControlDialog::copyTransactionOutpoint);
     contextMenu->addSeparator();
     lockAction = contextMenu->addAction(tr("L&ock unspent"), this, &CoinControlDialog::lockCoin);
     unlockAction = contextMenu->addAction(tr("&Unlock unspent"), this, &CoinControlDialog::unlockCoin);
@@ -235,7 +235,7 @@ void CoinControlDialog::showMenu(const QPoint &point)
         // disable some items (like Copy Transaction ID, lock, unlock) for tree roots in context menu
         if (item->data(COLUMN_ADDRESS, TxHashRole).toString().length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode)
         {
-            copyTransactionHashAction->setEnabled(true);
+            m_copy_transaction_outpoint_action->setEnabled(true);
             if (model->wallet().isLockedCoin(COutPoint(uint256S(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), item->data(COLUMN_ADDRESS, VOutRole).toUInt())))
             {
                 lockAction->setEnabled(false);
@@ -249,7 +249,7 @@ void CoinControlDialog::showMenu(const QPoint &point)
         }
         else // this means click on parent node in tree mode -> disable all
         {
-            copyTransactionHashAction->setEnabled(false);
+            m_copy_transaction_outpoint_action->setEnabled(false);
             lockAction->setEnabled(false);
             unlockAction->setEnabled(false);
         }
@@ -283,10 +283,14 @@ void CoinControlDialog::copyAddress()
         GUIUtil::setClipboard(contextMenuItem->text(COLUMN_ADDRESS));
 }
 
-// context menu action: copy transaction id
-void CoinControlDialog::copyTransactionHash()
+// context menu action: copy transaction id and vout index
+void CoinControlDialog::copyTransactionOutpoint()
 {
-    GUIUtil::setClipboard(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString());
+    const QString address = contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString();
+    const QString vout = contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toString();
+    const QString outpoint = QString("%1:%2").arg(address).arg(vout);
+
+    GUIUtil::setClipboard(outpoint);
 }
 
 // context menu action: lock coin

src/qt/coincontroldialog.h

@@ -59,7 +59,7 @@ private:
 
     QMenu *contextMenu;
     QTreeWidgetItem *contextMenuItem;
-    QAction *copyTransactionHashAction;
+    QAction* m_copy_transaction_outpoint_action;
     QAction *lockAction;
     QAction *unlockAction;
 
@@ -92,7 +92,7 @@ private Q_SLOTS:
     void copyAmount();
     void copyLabel();
     void copyAddress();
-    void copyTransactionHash();
+    void copyTransactionOutpoint();
     void lockCoin();
     void unlockCoin();
     void clipboardQuantity();

src/qt/guiutil.cpp

@@ -89,6 +89,8 @@
 void ForceActivation();
 #endif
 
+using namespace std::chrono_literals;
+
 namespace GUIUtil {
 
 static RecursiveMutex cs_css;
@@ -1729,6 +1731,16 @@ QString formatDurationStr(std::chrono::seconds dur)
     return str_list.join(" ");
 }
 
+QString FormatPeerAge(std::chrono::seconds time_connected)
+{
+    const auto time_now{GetTime<std::chrono::seconds>()};
+    const auto age{time_now - time_connected};
+    if (age >= 24h) return QObject::tr("%1 d").arg(age / 24h);
+    if (age >= 1h) return QObject::tr("%1 h").arg(age / 1h);
+    if (age >= 1min) return QObject::tr("%1 m").arg(age / 1min);
+    return QObject::tr("%1 s").arg(age / 1s);
+}
+
 QString formatServicesStr(quint64 mask)
 {
     QStringList strList;

src/qt/guiutil.h

@@ -420,6 +420,9 @@ namespace GUIUtil
     /** Convert seconds into a QString with days, hours, mins, secs */
     QString formatDurationStr(std::chrono::seconds dur);
 
+    /** Convert peer connection time to a QString denominated in the most relevant unit. */
+    QString FormatPeerAge(std::chrono::seconds time_connected);
+
     /** Format CNodeStats.nServices bitmask into a user-readable string */
     QString formatServicesStr(quint64 mask);
 

src/qt/peertablemodel.cpp

@@ -71,6 +71,8 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const
         switch (column) {
         case NetNodeId:
             return (qint64)rec->nodeStats.nodeid;
+        case Age:
+            return GUIUtil::FormatPeerAge(rec->nodeStats.m_connected);
         case Address:
             // prepend to peer address down-arrow symbol for inbound connection and up-arrow for outbound connection
             return QString::fromStdString((rec->nodeStats.fInbound ? "↓ " : "↑ ") + rec->nodeStats.m_addr_name);
@@ -91,6 +93,7 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const
     } else if (role == Qt::TextAlignmentRole) {
         switch (column) {
         case NetNodeId:
+        case Age:
             return QVariant(Qt::AlignRight | Qt::AlignVCenter);
         case Address:
             return {};

src/qt/peertablemodel.h

@@ -47,6 +47,7 @@ public:
 
     enum ColumnIndex {
         NetNodeId = 0,
+        Age,
         Address,
         ConnectionType,
         Network,
@@ -84,6 +85,9 @@ private:
         /*: Title of Peers Table column which contains a
             unique number used to identify a connection. */
         tr("Peer"),
+        /*: Title of Peers Table column which indicates the duration (length of time)
+            since the peer connection started. */
+        tr("Age"),
         /*: Title of Peers Table column which contains the
             IP/Onion/I2P address of the connected peer. */
         tr("Address"),

src/qt/peertablesortproxy.cpp

@@ -24,6 +24,8 @@ bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelInd
     switch (static_cast<PeerTableModel::ColumnIndex>(left_index.column())) {
     case PeerTableModel::NetNodeId:
         return left_stats.nodeid < right_stats.nodeid;
+    case PeerTableModel::Age:
+        return left_stats.m_connected > right_stats.m_connected;
     case PeerTableModel::Address:
         return left_stats.m_addr_name.compare(right_stats.m_addr_name) < 0;
     case PeerTableModel::ConnectionType:

src/qt/rpcconsole.cpp

@@ -713,11 +713,17 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
             ui->peerWidget->setColumnWidth(PeerTableModel::Subversion, SUBVERSION_COLUMN_WIDTH);
             ui->peerWidget->setColumnWidth(PeerTableModel::Ping, PING_COLUMN_WIDTH);
         }
+        ui->peerWidget->horizontalHeader()->setSectionResizeMode(PeerTableModel::Age, QHeaderView::ResizeToContents);
         ui->peerWidget->horizontalHeader()->setStretchLastSection(true);
         ui->peerWidget->setItemDelegateForColumn(PeerTableModel::NetNodeId, new PeerIdViewDelegate(this));
 
         // create peer table context menu
         peersTableContextMenu = new QMenu(this);
+        //: Context menu action to copy the address of a peer
+        peersTableContextMenu->addAction(tr("&Copy address"), [this] {
+            GUIUtil::copyEntryData(ui->peerWidget, PeerTableModel::Address, Qt::DisplayRole);
+        });
+        peersTableContextMenu->addSeparator();
         peersTableContextMenu->addAction(tr("&Disconnect"), this, &RPCConsole::disconnectSelectedNode);
         peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 &hour"), [this] { banSelectedNode(60 * 60); });
         peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 d&ay"), [this] { banSelectedNode(60 * 60 * 24); });
@@ -740,10 +746,18 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
             ui->banlistWidget->setColumnWidth(BanTableModel::Address, BANSUBNET_COLUMN_WIDTH);
             ui->banlistWidget->setColumnWidth(BanTableModel::Bantime, BANTIME_COLUMN_WIDTH);
         }
+        ui->banlistWidget->horizontalHeader()->setSectionResizeMode(BanTableModel::Address, QHeaderView::ResizeToContents);
         ui->banlistWidget->horizontalHeader()->setStretchLastSection(true);
 
         // create ban table context menu
         banTableContextMenu = new QMenu(this);
+        /*: Context menu action to copy the IP/Netmask of a banned peer.
+            IP/Netmask is the combination of a peer's IP address and its Netmask.
+            For IP address see: https://en.wikipedia.org/wiki/IP_address */
+        banTableContextMenu->addAction(tr("&Copy IP/Netmask"), [this] {
+            GUIUtil::copyEntryData(ui->banlistWidget, BanTableModel::Address, Qt::DisplayRole);
+        });
+        banTableContextMenu->addSeparator();
         banTableContextMenu->addAction(tr("&Unban"), this, &RPCConsole::unbanSelectedNode);
         connect(ui->banlistWidget, &QTableView::customContextMenuRequested, this, &RPCConsole::showBanTableContextMenu);
 

src/qt/splashscreen.cpp

@@ -189,8 +189,8 @@ static void InitMessage(SplashScreen *splash, const std::string &message)
 static void ShowProgress(SplashScreen *splash, const std::string &title, int nProgress, bool resume_possible)
 {
     InitMessage(splash, title + std::string("\n") +
-            (resume_possible ? _("(press q to shutdown and continue later)").translated
-                                : _("press q to shutdown").translated) +
+            (resume_possible ? SplashScreen::tr("(press q to shutdown and continue later)").toStdString()
+                                : SplashScreen::tr("press q to shutdown").toStdString()) +
             strprintf("\n%d", nProgress) + "%");
 }
 

src/test/fs_tests.cpp

@@ -152,7 +152,7 @@ BOOST_AUTO_TEST_CASE(rename)
     fs::remove(path2);
 }
 
-#ifndef WIN32
+#ifndef __MINGW64__ // no symlinks on mingw
 BOOST_AUTO_TEST_CASE(create_directories)
 {
     // Test fs::create_directories workaround.
@@ -174,7 +174,7 @@ BOOST_AUTO_TEST_CASE(create_directories)
     fs::remove(symlink);
     fs::remove(dir);
 }
-#endif // WIN32
+#endif // __MINGW64__
 
 BOOST_AUTO_TEST_SUITE_END()
 

src/test/util_tests.cpp

@@ -207,6 +207,24 @@ BOOST_AUTO_TEST_CASE(util_HexStr)
         BOOST_CHECK_EQUAL(HexStr(in_s), out_exp);
         BOOST_CHECK_EQUAL(HexStr(in_b), out_exp);
     }
+
+    {
+        auto input = std::string();
+        for (size_t i=0; i<256; ++i) {
+            input.push_back(static_cast<char>(i));
+        }
+
+        auto hex = HexStr(input);
+        BOOST_TEST_REQUIRE(hex.size() == 512);
+        static constexpr auto hexmap = std::string_view("0123456789abcdef");
+        for (size_t i = 0; i < 256; ++i) {
+            auto upper = hexmap.find(hex[i * 2]);
+            auto lower = hexmap.find(hex[i * 2 + 1]);
+            BOOST_TEST_REQUIRE(upper != std::string_view::npos);
+            BOOST_TEST_REQUIRE(lower != std::string_view::npos);
+            BOOST_TEST_REQUIRE(i == upper*16 + lower);
+        }
+    }
 }
 
 BOOST_AUTO_TEST_CASE(span_write_bytes)

src/txmempool.h

@@ -311,16 +311,6 @@ public:
     }
 };
 
-struct update_lock_points
-{
-    explicit update_lock_points(const LockPoints& _lp) : lp(_lp) { }
-
-    void operator() (CTxMemPoolEntry &e) { e.UpdateLockPoints(lp); }
-
-private:
-    const LockPoints& lp;
-};
-
 // Multi_index tag names
 struct descendant_score {};
 struct entry_time {};
@@ -631,10 +621,14 @@ public:
     bool removeSpentIndex(const uint256 txhash);
 
     void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
-    /** After reorg, check if mempool entries are now non-final, premature coinbase spends, or have
-     * invalid lockpoints. Update lockpoints and remove entries (and descendants of entries) that
-     * are no longer valid. */
-    void removeForReorg(CChain& chain, std::function<bool(txiter)> check_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
+    /** After reorg, filter the entries that would no longer be valid in the next block, and update
+     * the entries' cached LockPoints if needed.  The mempool does not have any knowledge of
+     * consensus rules. It just appplies the callable function and removes the ones for which it
+     * returns true.
+     * @param[in]   filter_final_and_mature   Predicate that checks the relevant validation rules
+     *                                        and updates an entry's LockPoints.
+     * */
+    void removeForReorg(CChain& chain, std::function<bool(txiter)> filter_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
     void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
     void removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId) EXCLUSIVE_LOCKS_REQUIRED(cs);
     void removeProTxPubKeyConflicts(const CTransaction &tx, const CBLSLazyPublicKey &pubKey) EXCLUSIVE_LOCKS_REQUIRED(cs);

src/util/strencodings.cpp

@@ -9,6 +9,7 @@
 #include <tinyformat.h>
 
 #include <algorithm>
+#include <array>
 #include <cstdlib>
 #include <cstring>
 #include <limits>
@@ -493,17 +494,37 @@ std::string Capitalize(std::string str)
     return str;
 }
 
+namespace {
+
+using ByteAsHex = std::array<char, 2>;
+
+constexpr std::array<ByteAsHex, 256> CreateByteToHexMap()
+{
+    constexpr char hexmap[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
+
+    std::array<ByteAsHex, 256> byte_to_hex{};
+    for (size_t i = 0; i < byte_to_hex.size(); ++i) {
+        byte_to_hex[i][0] = hexmap[i >> 4];
+        byte_to_hex[i][1] = hexmap[i & 15];
+    }
+    return byte_to_hex;
+}
+
+} // namespace
+
 std::string HexStr(const Span<const uint8_t> s)
 {
     std::string rv(s.size() * 2, '\0');
-    static constexpr char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7',
-                                         '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
-    auto it = rv.begin();
+    static constexpr auto byte_to_hex = CreateByteToHexMap();
+    static_assert(sizeof(byte_to_hex) == 512);
+
+    char* it = rv.data();
     for (uint8_t v : s) {
-        *it++ = hexmap[v >> 4];
-        *it++ = hexmap[v & 15];
+        std::memcpy(it, byte_to_hex[v].data(), 2);
+        it += 2;
     }
-    assert(it == rv.end());
+
+    assert(it == rv.data() + rv.size());
     return rv;
 }
 

src/validation.cpp

@@ -378,41 +378,55 @@ void CChainState::MaybeUpdateMempoolForReorg(
     // the disconnectpool that were added back and cleans up the mempool state.
     m_mempool->UpdateTransactionsFromBlock(vHashUpdate);
 
-    const auto check_final_and_mature = [this, flags=STANDARD_LOCKTIME_VERIFY_FLAGS](CTxMemPool::txiter it)
+    // Predicate to use for filtering transactions in removeForReorg.
+    // Checks whether the transaction is still final and, if it spends a coinbase output, mature.
+    // Also updates valid entries' cached LockPoints if needed.
+    // If false, the tx is still valid and its lockpoints are updated.
+    // If true, the tx would be invalid in the next block; remove this entry and all of its descendants.
+    const auto filter_final_and_mature = [this, flags=STANDARD_LOCKTIME_VERIFY_FLAGS](CTxMemPool::txiter it)
         EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs, ::cs_main) {
-        bool should_remove = false;
         AssertLockHeld(m_mempool->cs);
         AssertLockHeld(::cs_main);
         const CTransaction& tx = it->GetTx();
+
+        // The transaction must be final.
+        if (!CheckFinalTx(m_chain.Tip(), tx, flags)) return true;
         LockPoints lp = it->GetLockPoints();
         const bool validLP{TestLockPointValidity(m_chain, lp)};
         CCoinsViewMemPool view_mempool(&CoinsTip(), *m_mempool);
-        if (!CheckFinalTx(m_chain.Tip(), tx, flags)
-            || !CheckSequenceLocks(m_chain.Tip(), view_mempool, tx, flags, &lp, validLP)) {
-            // Note if CheckSequenceLocks fails the LockPoints may still be invalid
-            // So it's critical that we remove the tx and not depend on the LockPoints.
-            should_remove = true;
-        } else if (it->GetSpendsCoinbase()) {
+        // CheckSequenceLocks checks if the transaction will be final in the next block to be
+        // created on top of the new chain. We use useExistingLockPoints=false so that, instead of
+        // using the information in lp (which might now refer to a block that no longer exists in
+        // the chain), it will update lp to contain LockPoints relevant to the new chain.
+        if (!CheckSequenceLocks(m_chain.Tip(), view_mempool, tx, flags, &lp, validLP)) {
+            // If CheckSequenceLocks fails, remove the tx and don't depend on the LockPoints.
+            return true;
+        } else if (!validLP) {
+            // If CheckSequenceLocks succeeded, it also updated the LockPoints.
+            // Now update the mempool entry lockpoints as well.
+            m_mempool->mapTx.modify(it, [&lp](CTxMemPoolEntry& e) { e.UpdateLockPoints(lp); });
+        }
+
+        // If the transaction spends any coinbase outputs, it must be mature.
+        if (it->GetSpendsCoinbase()) {
             for (const CTxIn& txin : tx.vin) {
                 auto it2 = m_mempool->mapTx.find(txin.prevout.hash);
                 if (it2 != m_mempool->mapTx.end())
                     continue;
-                const Coin &coin = CoinsTip().AccessCoin(txin.prevout);
+                const Coin& coin{CoinsTip().AccessCoin(txin.prevout)};
                 assert(!coin.IsSpent());
                 const auto mempool_spend_height{m_chain.Tip()->nHeight + 1};
-                if (coin.IsSpent() || (coin.IsCoinBase() && mempool_spend_height - coin.nHeight < COINBASE_MATURITY)) {
-                    should_remove = true;
-                    break;
+                if (coin.IsCoinBase() && mempool_spend_height - coin.nHeight < COINBASE_MATURITY) {
+                    return true;
                 }
             }
         }
-        // CheckSequenceLocks updates lp. Update the mempool entry LockPoints.
-        if (!validLP) m_mempool->mapTx.modify(it, update_lock_points(lp));
-        return should_remove;
+        // Transaction is still valid and cached LockPoints are updated.
+        return false;
     };
 
     // We also need to remove any now-immature transactions
-    m_mempool->removeForReorg(m_chain, check_final_and_mature);
+    m_mempool->removeForReorg(m_chain, filter_final_and_mature);
     // Re-limit mempool size, in case we added any transactions
     LimitMempoolSize(
         *m_mempool,
@@ -2482,9 +2496,9 @@ bool CChainState::FlushStateToDisk(
             full_flush_completed = true;
             TRACE5(utxocache, flush,
                    (int64_t)(GetTimeMicros() - nNow.count()), // in microseconds (µs)
-                   (u_int32_t)mode,
-                   (u_int64_t)coins_count,
-                   (u_int64_t)coins_mem_usage,
+                   (uint32_t)mode,
+                   (uint64_t)coins_count,
+                   (uint64_t)coins_mem_usage,
                    (bool)fFlushForPrune);
         }
     }

src/wallet/bdb.cpp

@@ -98,7 +98,7 @@ void BerkeleyEnvironment::Close()
     if (ret != 0)
         LogPrintf("BerkeleyEnvironment::Close: Error %d closing database environment: %s\n", ret, DbEnv::strerror(ret));
     if (!fMockDb)
-        DbEnv((u_int32_t)0).remove(strPath.c_str(), 0);
+        DbEnv((uint32_t)0).remove(strPath.c_str(), 0);
 
     if (error_file) fclose(error_file);
 
@@ -246,7 +246,7 @@ const void* BerkeleyBatch::SafeDbt::get_data() const
     return m_dbt.get_data();
 }
 
-u_int32_t BerkeleyBatch::SafeDbt::get_size() const
+uint32_t BerkeleyBatch::SafeDbt::get_size() const
 {
     return m_dbt.get_size();
 }

src/wallet/bdb.h

@@ -34,7 +34,7 @@ static const unsigned int DEFAULT_WALLET_DBLOGSIZE = 100;
 static const bool DEFAULT_WALLET_PRIVDB = true;
 
 struct WalletDatabaseFileId {
-    u_int8_t value[DB_FILE_ID_LEN];
+    uint8_t value[DB_FILE_ID_LEN];
     bool operator==(const WalletDatabaseFileId& rhs) const;
 };
 
@@ -182,7 +182,7 @@ class BerkeleyBatch : public DatabaseBatch
 
         // delegate to Dbt
         const void* get_data() const;
-        u_int32_t get_size() const;
+        uint32_t get_size() const;
 
         // conversion operator to access the underlying Dbt
         operator Dbt*();

src/wallet/rpcdump.cpp

@@ -1802,7 +1802,7 @@ RPCHelpMan importdescriptors() {
                                         /* oneline_description */ "", {"timestamp | \"now\"", "integer / string"}
                                     },
                                     {"internal", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether matching outputs should be treated as not incoming payments (e.g. change)"},
-                                    {"label", RPCArg::Type::STR, RPCArg::Default{""}, "Label to assign to the address, only allowed with internal=false"},
+                                    {"label", RPCArg::Type::STR, RPCArg::Default{""}, "Label to assign to the address, only allowed with internal=false. Disabled for ranged descriptors"},
                                 },
                             },
                         },

test/functional/feature_dirsymlinks.py

@@ -6,9 +6,8 @@
 """
 
 import os
-import sys
 
-from test_framework.test_framework import BitcoinTestFramework, SkipTest
+from test_framework.test_framework import BitcoinTestFramework
 
 
 def rename_and_link(*, from_name, to_name):
@@ -16,24 +15,27 @@ def rename_and_link(*, from_name, to_name):
     os.symlink(to_name, from_name)
     assert os.path.islink(from_name) and os.path.isdir(from_name)
 
-class SymlinkTest(BitcoinTestFramework):
-    def skip_test_if_missing_module(self):
-        if sys.platform == 'win32':
-            raise SkipTest("Symlinks test skipped on Windows")
 
+class SymlinkTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
 
     def run_test(self):
+        dir_new_blocks = self.nodes[0].chain_path / "new_blocks"
+        dir_new_chainstate = self.nodes[0].chain_path / "new_chainstate"
         self.stop_node(0)
 
-        rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "blocks"),
-                        to_name=os.path.join(self.nodes[0].datadir, self.chain, "newblocks"))
-        rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "chainstate"),
-                        to_name=os.path.join(self.nodes[0].datadir, self.chain, "newchainstate"))
+        rename_and_link(
+            from_name=self.nodes[0].chain_path / "blocks",
+            to_name=dir_new_blocks,
+        )
+        rename_and_link(
+            from_name=self.nodes[0].chain_path / "chainstate",
+            to_name=dir_new_chainstate,
+        )
 
         self.start_node(0)
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     SymlinkTest().main()

test/functional/mempool_unbroadcast.py

@@ -9,21 +9,20 @@ import time
 
 from test_framework.p2p import P2PTxInvStore
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (
-    assert_equal,
-    create_confirmed_utxos,
-)
+from test_framework.util import assert_equal
+from test_framework.wallet import MiniWallet
 
 MAX_INITIAL_BROADCAST_DELAY = 15 * 60 # 15 minutes in seconds
 
 class MempoolUnbroadcastTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
-
-    def skip_test_if_missing_module(self):
-        self.skip_if_no_wallet()
+        if self.is_wallet_compiled():
+            self.requires_wallet = True
 
     def run_test(self):
+        self.wallet = MiniWallet(self.nodes[0])
+        self.wallet.rescan_utxos()
         self.test_broadcast()
         self.test_txn_removal()
 
@@ -31,30 +30,25 @@ class MempoolUnbroadcastTest(BitcoinTestFramework):
         self.log.info("Test that mempool reattempts delivery of locally submitted transaction")
         node = self.nodes[0]
 
-        min_relay_fee = node.getnetworkinfo()["relayfee"]
-        utxos = create_confirmed_utxos(self, min_relay_fee, node, 10)
-
         self.disconnect_nodes(0, 1)
 
         self.log.info("Generate transactions that only node 0 knows about")
 
-        # generate a wallet txn
-        addr = node.getnewaddress()
-        wallet_tx_hsh = node.sendtoaddress(addr, 0.0001)
+        if self.is_wallet_compiled():
+            # generate a wallet txn
+            addr = node.getnewaddress()
+            wallet_tx_hsh = node.sendtoaddress(addr, 0.0001)
 
         # generate a txn using sendrawtransaction
-        us0 = utxos.pop()
-        inputs = [{"txid": us0["txid"], "vout": us0["vout"]}]
-        outputs = {addr: 0.0001}
-        tx = node.createrawtransaction(inputs, outputs)
-        node.settxfee(min_relay_fee)
-        txF = node.fundrawtransaction(tx)
-        txFS = node.signrawtransactionwithwallet(txF["hex"])
+        txFS = self.wallet.create_self_transfer(from_node=node)
         rpc_tx_hsh = node.sendrawtransaction(txFS["hex"])
 
         # check transactions are in unbroadcast using rpc
         mempoolinfo = self.nodes[0].getmempoolinfo()
-        assert_equal(mempoolinfo['unbroadcastcount'], 2)
+        unbroadcast_count = 1
+        if self.is_wallet_compiled():
+            unbroadcast_count += 1
+        assert_equal(mempoolinfo['unbroadcastcount'], unbroadcast_count)
         mempool = self.nodes[0].getrawmempool(True)
         for tx in mempool:
             assert_equal(mempool[tx]['unbroadcast'], True)
@@ -62,7 +56,8 @@ class MempoolUnbroadcastTest(BitcoinTestFramework):
         # check that second node doesn't have these two txns
         mempool = self.nodes[1].getrawmempool()
         assert rpc_tx_hsh not in mempool
-        assert wallet_tx_hsh not in mempool
+        if self.is_wallet_compiled():
+            assert wallet_tx_hsh not in mempool
 
         # ensure that unbroadcast txs are persisted to mempool.dat
         self.restart_node(0)
@@ -75,7 +70,8 @@ class MempoolUnbroadcastTest(BitcoinTestFramework):
         self.sync_mempools(timeout=30)
         mempool = self.nodes[1].getrawmempool()
         assert rpc_tx_hsh in mempool
-        assert wallet_tx_hsh in mempool
+        if self.is_wallet_compiled():
+            assert wallet_tx_hsh in mempool
 
         # check that transactions are no longer in first node's unbroadcast set
         mempool = self.nodes[0].getrawmempool(True)
@@ -104,8 +100,7 @@ class MempoolUnbroadcastTest(BitcoinTestFramework):
 
         # since the node doesn't have any connections, it will not receive
         # any GETDATAs & thus the transaction will remain in the unbroadcast set.
-        addr = node.getnewaddress()
-        txhsh = node.sendtoaddress(addr, 0.0001)
+        txhsh = self.wallet.send_self_transfer(from_node=node)["txid"]
 
         # check transaction was removed from unbroadcast set due to presence in
         # a block

test/functional/rpc_txoutproof.py

@@ -54,7 +54,7 @@ class MerkleBlockTest(BitcoinTestFramework):
         assert_equal(self.nodes[0].verifytxoutproof(self.nodes[0].gettxoutproof([txid1, txid2])), txlist)
         assert_equal(self.nodes[0].verifytxoutproof(self.nodes[0].gettxoutproof([txid1, txid2], blockhash)), txlist)
 
-        txin_spent = miniwallet.get_utxo()  # Get the change from txid2
+        txin_spent = miniwallet.get_utxo(txid=txid2)  # Get the change from txid2
         tx3 = miniwallet.send_self_transfer(from_node=self.nodes[0], utxo_to_spend=txin_spent)
         txid3 = tx3['txid']
         self.generate(self.nodes[0], 1)

test/functional/test_framework/wallet.py

@@ -135,10 +135,9 @@ class MiniWallet:
 
         Args:
         txid: get the first utxo we find from a specific transaction
-
-        Note: Can be used to get the change output immediately after a send_self_transfer
         """
         index = -1  # by default the last utxo
+        self._utxos = sorted(self._utxos, key=lambda k: (k['value'], -k['height']))  # Put the largest utxo last
         if txid:
             utxo = next(filter(lambda utxo: txid == utxo['txid'], self._utxos))
             index = self._utxos.index(utxo)
@@ -155,8 +154,7 @@ class MiniWallet:
 
     def create_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node, utxo_to_spend=None, mempool_valid=True, locktime=0, sequence=0):
         """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed."""
-        self._utxos = sorted(self._utxos, key=lambda k: (k['value'], -k['height']))
-        utxo_to_spend = utxo_to_spend or self._utxos.pop()  # Pick the largest utxo (if none provided) and hope it covers the fee
+        utxo_to_spend = utxo_to_spend or self.get_utxo()
         if self._priv_key is None:
             vsize = Decimal(85)  # anyone-can-spend
         else: