From 81e763f1e5d878383487b5e4e695b3dca703e1d6 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 27 Nov 2025 14:34:21 +1000 Subject: [PATCH 001/502] wallet: Have GetBalance report used amount directly without two calls --- src/interfaces/wallet.h | 4 +++- src/wallet/interfaces.cpp | 1 + src/wallet/receive.cpp | 20 ++++++++++++++------ src/wallet/receive.h | 1 + src/wallet/rpc/coins.cpp | 8 +++----- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 2ccd514dbf0d..118f9a2e96de 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -369,11 +369,13 @@ struct WalletBalances CAmount balance = 0; CAmount unconfirmed_balance = 0; CAmount immature_balance = 0; + CAmount used_balance = 0; bool balanceChanged(const WalletBalances& prev) const { return balance != prev.balance || unconfirmed_balance != prev.unconfirmed_balance || - immature_balance != prev.immature_balance; + immature_balance != prev.immature_balance || + used_balance != prev.used_balance; } }; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 4dc873347ff1..7fa0d8a13b9c 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -388,6 +388,7 @@ class WalletImpl : public Wallet result.balance = bal.m_mine_trusted; result.unconfirmed_balance = bal.m_mine_untrusted_pending; result.immature_balance = bal.m_mine_immature; + result.used_balance = bal.m_mine_used; return result; } bool tryGetBalances(WalletBalances& balances, uint256& block_hash) override diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 13a25b36326e..8832ddb66cef 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -255,17 +255,25 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)}; const int tx_depth{wallet.GetTxDepthInMainChain(wtx)}; - if (!wallet.IsSpent(outpoint) && (allow_used_addresses || !wallet.IsSpentKey(txo.GetTxOut().scriptPubKey))) { - // Get the amounts for mine - CAmount credit_mine = txo.GetTxOut().nValue; + if (!wallet.IsSpent(outpoint)) { + CAmount* bucket = nullptr; // Set the amounts in the return object if (wallet.IsTxImmatureCoinBase(wtx) && wtx.isConfirmed()) { - ret.m_mine_immature += credit_mine; + bucket = &ret.m_mine_immature; } else if (is_trusted && tx_depth >= min_depth) { - ret.m_mine_trusted += credit_mine; + bucket = &ret.m_mine_trusted; } else if (!is_trusted && wtx.InMempool()) { - ret.m_mine_untrusted_pending += credit_mine; + bucket = &ret.m_mine_untrusted_pending; + } + if (bucket) { + // Get the amounts for mine + CAmount credit_mine = txo.GetTxOut().nValue; + + if (!allow_used_addresses && wallet.IsSpentKey(txo.GetTxOut().scriptPubKey)) { + bucket = &ret.m_mine_used; + } + *bucket += credit_mine; } } } diff --git a/src/wallet/receive.h b/src/wallet/receive.h index f6c55a45360b..5bc0545bebe2 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -47,6 +47,7 @@ struct Balance { CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending) CAmount m_mine_immature{0}; //!< Immature coinbases in the main chain + CAmount m_mine_used{0}; //!< Trusted/untrusted/immature funds in utxos that have already been spent from (only populated if AVOID REUSE wallet flag is set) }; Balance GetBalance(const CWallet& wallet, int min_depth = 0, bool avoid_reuse = true); diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 4aef0dda8865..c76d5e3c7fc0 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -433,7 +433,8 @@ RPCHelpMan getbalances() LOCK(wallet.cs_wallet); - const auto bal = GetBalance(wallet); + const auto bal = GetBalance(wallet, /*min_depth=*/0, /*avoid_reuse=*/true); + UniValue balances{UniValue::VOBJ}; { UniValue balances_mine{UniValue::VOBJ}; @@ -441,10 +442,7 @@ RPCHelpMan getbalances() balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending)); balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature)); if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { - // If the AVOID_REUSE flag is set, bal has been set to just the un-reused address balance. Get - // the total balance, and then subtract bal to get the reused address balance. - const auto full_bal = GetBalance(wallet, 0, false); - balances_mine.pushKV("used", ValueFromAmount(full_bal.m_mine_trusted + full_bal.m_mine_untrusted_pending - bal.m_mine_trusted - bal.m_mine_untrusted_pending)); + balances_mine.pushKV("used", ValueFromAmount(bal.m_mine_used)); } balances.pushKV("mine", std::move(balances_mine)); } From 938312d7a6dcf06cb401a16651e80320f36ee4db Mon Sep 17 00:00:00 2001 From: crStiv Date: Fri, 15 Aug 2025 15:54:42 +0200 Subject: [PATCH 002/502] docs: clarify RPC credentials security boundary --- doc/JSON-RPC-interface.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/JSON-RPC-interface.md b/doc/JSON-RPC-interface.md index e7f085a377ab..a9ea089d4bf7 100644 --- a/doc/JSON-RPC-interface.md +++ b/doc/JSON-RPC-interface.md @@ -124,6 +124,22 @@ RPC interface will be abused. security-sensitive operations on a computer whose other programs you trust. +- **RPC Credentials Security Boundary:** Any client with valid RPC credentials + should be treated as having significant control over both the Bitcoin Core node + and the filesystem resources accessible by the `bitcoind` process. RPC commands + can load wallet files from paths that the `bitcoind` process has permission to + access, specify file paths for operations, and potentially gain broader access + than intended. This means that someone with RPC access can potentially compromise + not only the Bitcoin Core node, but also the machine it is running on. Bitcoin Core + provides the `-rpcwhitelist` option to restrict which RPC commands specific users + can access, and `-rpcwhitelistdefault` to control the default behavior for users + without explicit whitelists. However, when using multiple wallets or sharing access + with different users, these should not be considered robust security boundaries, as + users with access to certain commands may still be able to exploit functionality in + unexpected ways. For security-sensitive operations, implement proper system-level + isolation (containers, virtualization, separate user accounts with restricted + permissions) rather than relying solely on RPC access controls. + - **Securing remote network access:** You may optionally allow other computers to remotely control Bitcoin Core by setting the `rpcallowip` and `rpcbind` configuration parameters. These settings are only meant From fd5e9d990431a6af08dd99b25d1e17c6c9818b4d Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 28 Jan 2026 20:25:34 +0000 Subject: [PATCH 003/502] signals: Use a lambda to avoid connecting a signal to another signal This is undocumented and unspecified Boost behavior that happens to work as intended for now, but could break at any point in the future. See the boost discussion here: https://groups.google.com/g/boost-list/c/So4i8JXneJ0 It also complicates a potential replacement of Boost::signals2. --- src/wallet/wallet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 64d7f191de76..bda104061b10 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3549,7 +3549,9 @@ bool CWallet::HaveCryptedKeys() const void CWallet::ConnectScriptPubKeyManNotifiers() { for (const auto& spk_man : GetActiveScriptPubKeyMans()) { - spk_man->NotifyCanGetAddressesChanged.connect(NotifyCanGetAddressesChanged); + spk_man->NotifyCanGetAddressesChanged.connect([this] { + NotifyCanGetAddressesChanged(); + }); spk_man->NotifyFirstKeyTimeChanged.connect([this](const ScriptPubKeyMan*, int64_t time) { MaybeUpdateBirthTime(time); }); From 2150153f372f99b789171b006626f66ff5d0299c Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 29 Jan 2026 16:09:08 +0000 Subject: [PATCH 004/502] signals: Temporarily add boost headers to bitcoind and bitcoin-node builds The current code forward-declares boost::signals2, which avoids the need for these includes. An upcoming commit will (temporarily) include boost headers directly instead. A follow-up commit will then replace boost with an internal signals implementation, which will allow this commit to be reverted. --- src/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cf1f26c9f24c..1eb61f138cf4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -307,6 +307,7 @@ if(BUILD_DAEMON) target_link_libraries(bitcoind core_interface bitcoin_node + Boost::headers $ ) install_binary_component(bitcoind HAS_MANPAGE) @@ -320,6 +321,7 @@ if(ENABLE_IPC AND BUILD_DAEMON) core_interface bitcoin_node bitcoin_ipc + Boost::headers $ ) install_binary_component(bitcoin-node INTERNAL) From 037e58b57b7304e4a7f013b8768b8a86a09c96e8 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Mon, 26 Sep 2022 20:16:57 +0000 Subject: [PATCH 005/502] signals: use forwarding header for boost signals For now, including btcsignals.h simply includes boost's signals. A follow-up commit will replace the implementation. --- src/btcsignals.h | 12 ++++++++++++ src/common/interfaces.cpp | 2 +- src/init.cpp | 3 +-- src/node/interface_ui.cpp | 4 +--- src/node/interfaces.cpp | 3 +-- src/noui.cpp | 4 +--- src/qt/bitcoin.cpp | 2 +- src/wallet/scriptpubkeyman.h | 3 +-- src/wallet/wallet.h | 3 +-- 9 files changed, 20 insertions(+), 16 deletions(-) create mode 100644 src/btcsignals.h diff --git a/src/btcsignals.h b/src/btcsignals.h new file mode 100644 index 000000000000..816d912571e3 --- /dev/null +++ b/src/btcsignals.h @@ -0,0 +1,12 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_BTCSIGNALS_H +#define BITCOIN_BTCSIGNALS_H + +#include +#include +#include + +#endif // BITCOIN_BTCSIGNALS_H diff --git a/src/common/interfaces.cpp b/src/common/interfaces.cpp index ffd85e6131fb..dc98fac6c635 100644 --- a/src/common/interfaces.cpp +++ b/src/common/interfaces.cpp @@ -2,10 +2,10 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include -#include #include #include diff --git a/src/init.cpp b/src/init.cpp index 841fdec5b2c9..8e29f70b17e5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -110,8 +111,6 @@ #include #endif -#include - #ifdef ENABLE_ZMQ #include #include diff --git a/src/node/interface_ui.cpp b/src/node/interface_ui.cpp index d96c5155ab50..525d341e7178 100644 --- a/src/node/interface_ui.cpp +++ b/src/node/interface_ui.cpp @@ -4,12 +4,10 @@ #include +#include #include #include -#include -#include - using util::MakeUnorderedList; CClientUIInterface uiInterface; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 37524176e201..f0897c8e422a 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -69,8 +70,6 @@ #include #include -#include - using interfaces::BlockRef; using interfaces::BlockTemplate; using interfaces::BlockTip; diff --git a/src/noui.cpp b/src/noui.cpp index 327e17f8dbf2..af04cb00823d 100644 --- a/src/noui.cpp +++ b/src/noui.cpp @@ -5,15 +5,13 @@ #include +#include #include #include #include #include -#include -#include - /** Store connections so we can disconnect them when suppressing output */ boost::signals2::connection noui_ThreadSafeMessageBoxConn; boost::signals2::connection noui_ThreadSafeQuestionConn; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 92c815fe70e5..cb236bd72092 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -44,7 +45,6 @@ #include #endif // ENABLE_WALLET -#include #include #include diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 00dd6eed4d1a..8647ae489435 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -6,6 +6,7 @@ #define BITCOIN_WALLET_SCRIPTPUBKEYMAN_H #include +#include #include #include #include @@ -23,8 +24,6 @@ #include #include -#include - #include #include #include diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4250acca69e9..a46a64555522 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -7,6 +7,7 @@ #define BITCOIN_WALLET_WALLET_H #include +#include #include #include #include @@ -51,8 +52,6 @@ #include #include -#include - class CKey; class CKeyID; class CPubKey; From 9ade3929aaa9b43577737f37404e28fd858b808d Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Mon, 26 Sep 2022 21:10:41 +0000 Subject: [PATCH 006/502] signals: remove forward-declare for signals This eases the transition to a replacement signals implementation --- src/interfaces/handler.h | 8 ++------ src/node/interface_ui.h | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/interfaces/handler.h b/src/interfaces/handler.h index b3aebf7e14da..5de131ee98ed 100644 --- a/src/interfaces/handler.h +++ b/src/interfaces/handler.h @@ -5,15 +5,11 @@ #ifndef BITCOIN_INTERFACES_HANDLER_H #define BITCOIN_INTERFACES_HANDLER_H +#include + #include #include -namespace boost { -namespace signals2 { -class connection; -} // namespace signals2 -} // namespace boost - namespace interfaces { //! Generic interface for managing an event handler or callback function diff --git a/src/node/interface_ui.h b/src/node/interface_ui.h index ce5171bb5181..8c90bdf4fef8 100644 --- a/src/node/interface_ui.h +++ b/src/node/interface_ui.h @@ -6,6 +6,8 @@ #ifndef BITCOIN_NODE_INTERFACE_UI_H #define BITCOIN_NODE_INTERFACE_UI_H +#include + #include #include #include @@ -15,12 +17,6 @@ class CBlockIndex; enum class SynchronizationState; struct bilingual_str; -namespace boost { -namespace signals2 { -class connection; -} -} // namespace boost - /** Signals for UI communication. */ class CClientUIInterface { From edc297805868da736f274d977137324a85548530 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Mon, 26 Sep 2022 21:13:15 +0000 Subject: [PATCH 007/502] signals: use an alias for the boost::signals2 namespace The next commit will add a real implementation in this namespace. --- src/btcsignals.h | 2 ++ src/common/interfaces.cpp | 6 +++--- src/interfaces/handler.h | 4 ++-- src/node/interface_ui.cpp | 24 ++++++++++++------------ src/node/interface_ui.h | 2 +- src/noui.cpp | 6 +++--- src/qt/bitcoin.cpp | 6 +++--- src/qt/test/wallettests.cpp | 2 +- src/wallet/scriptpubkeyman.h | 4 ++-- src/wallet/wallet.h | 12 ++++++------ 10 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/btcsignals.h b/src/btcsignals.h index 816d912571e3..415d8b8651f9 100644 --- a/src/btcsignals.h +++ b/src/btcsignals.h @@ -9,4 +9,6 @@ #include #include +namespace btcsignals = boost::signals2; + #endif // BITCOIN_BTCSIGNALS_H diff --git a/src/common/interfaces.cpp b/src/common/interfaces.cpp index dc98fac6c635..de028f9ca257 100644 --- a/src/common/interfaces.cpp +++ b/src/common/interfaces.cpp @@ -23,11 +23,11 @@ class CleanupHandler : public interfaces::Handler class SignalHandler : public interfaces::Handler { public: - explicit SignalHandler(boost::signals2::connection connection) : m_connection(std::move(connection)) {} + explicit SignalHandler(btcsignals::connection connection) : m_connection(std::move(connection)) {} void disconnect() override { m_connection.disconnect(); } - boost::signals2::scoped_connection m_connection; + btcsignals::scoped_connection m_connection; }; class EchoImpl : public interfaces::Echo @@ -44,7 +44,7 @@ std::unique_ptr MakeCleanupHandler(std::function cleanup) return std::make_unique(std::move(cleanup)); } -std::unique_ptr MakeSignalHandler(boost::signals2::connection connection) +std::unique_ptr MakeSignalHandler(btcsignals::connection connection) { return std::make_unique(std::move(connection)); } diff --git a/src/interfaces/handler.h b/src/interfaces/handler.h index 5de131ee98ed..09c2363891f9 100644 --- a/src/interfaces/handler.h +++ b/src/interfaces/handler.h @@ -24,8 +24,8 @@ class Handler virtual void disconnect() = 0; }; -//! Return handler wrapping a boost signal connection. -std::unique_ptr MakeSignalHandler(boost::signals2::connection connection); +//! Return handler wrapping a btcsignals connection. +std::unique_ptr MakeSignalHandler(btcsignals::connection connection); //! Return handler wrapping a cleanup function. std::unique_ptr MakeCleanupHandler(std::function cleanup); diff --git a/src/node/interface_ui.cpp b/src/node/interface_ui.cpp index 525d341e7178..6b1fe72fae46 100644 --- a/src/node/interface_ui.cpp +++ b/src/node/interface_ui.cpp @@ -13,22 +13,22 @@ using util::MakeUnorderedList; CClientUIInterface uiInterface; struct UISignals { - boost::signals2::signal> ThreadSafeMessageBox; - boost::signals2::signal> ThreadSafeQuestion; - boost::signals2::signal InitMessage; - boost::signals2::signal InitWallet; - boost::signals2::signal NotifyNumConnectionsChanged; - boost::signals2::signal NotifyNetworkActiveChanged; - boost::signals2::signal NotifyAlertChanged; - boost::signals2::signal ShowProgress; - boost::signals2::signal NotifyBlockTip; - boost::signals2::signal NotifyHeaderTip; - boost::signals2::signal BannedListChanged; + btcsignals::signal> ThreadSafeMessageBox; + btcsignals::signal> ThreadSafeQuestion; + btcsignals::signal InitMessage; + btcsignals::signal InitWallet; + btcsignals::signal NotifyNumConnectionsChanged; + btcsignals::signal NotifyNetworkActiveChanged; + btcsignals::signal NotifyAlertChanged; + btcsignals::signal ShowProgress; + btcsignals::signal NotifyBlockTip; + btcsignals::signal NotifyHeaderTip; + btcsignals::signal BannedListChanged; }; static UISignals g_ui_signals; #define ADD_SIGNALS_IMPL_WRAPPER(signal_name) \ - boost::signals2::connection CClientUIInterface::signal_name##_connect(std::function fn) \ + btcsignals::connection CClientUIInterface::signal_name##_connect(std::function fn) \ { \ return g_ui_signals.signal_name.connect(fn); \ } diff --git a/src/node/interface_ui.h b/src/node/interface_ui.h index 8c90bdf4fef8..c33df59a93a2 100644 --- a/src/node/interface_ui.h +++ b/src/node/interface_ui.h @@ -67,7 +67,7 @@ class CClientUIInterface #define ADD_SIGNALS_DECL_WRAPPER(signal_name, rtype, ...) \ rtype signal_name(__VA_ARGS__); \ using signal_name##Sig = rtype(__VA_ARGS__); \ - boost::signals2::connection signal_name##_connect(std::function fn) + btcsignals::connection signal_name##_connect(std::function fn) /** Show message box. */ ADD_SIGNALS_DECL_WRAPPER(ThreadSafeMessageBox, bool, const bilingual_str& message, unsigned int style); diff --git a/src/noui.cpp b/src/noui.cpp index af04cb00823d..6f33b22728d5 100644 --- a/src/noui.cpp +++ b/src/noui.cpp @@ -13,9 +13,9 @@ #include /** Store connections so we can disconnect them when suppressing output */ -boost::signals2::connection noui_ThreadSafeMessageBoxConn; -boost::signals2::connection noui_ThreadSafeQuestionConn; -boost::signals2::connection noui_InitMessageConn; +btcsignals::connection noui_ThreadSafeMessageBoxConn; +btcsignals::connection noui_ThreadSafeQuestionConn; +btcsignals::connection noui_InitMessageConn; bool noui_ThreadSafeMessageBox(const bilingual_str& message, unsigned int style) { diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index cb236bd72092..0b89c605b9ce 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -485,9 +485,9 @@ int GuiMain(int argc, char* argv[]) util::ThreadSetInternalName("main"); // Subscribe to global signals from core - boost::signals2::scoped_connection handler_message_box = ::uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox); - boost::signals2::scoped_connection handler_question = ::uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestion); - boost::signals2::scoped_connection handler_init_message = ::uiInterface.InitMessage_connect(noui_InitMessage); + btcsignals::scoped_connection handler_message_box = ::uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox); + btcsignals::scoped_connection handler_question = ::uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestion); + btcsignals::scoped_connection handler_init_message = ::uiInterface.InitMessage_connect(noui_InitMessage); // Do not refer to data directory yet, this can be overridden by Intro::pickDataDirectory diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index b62b2a3b82f4..4369a078be2a 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -87,7 +87,7 @@ Txid SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDesti ->findChild("optInRBF") ->setCheckState(rbf ? Qt::Checked : Qt::Unchecked); Txid txid; - boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](const Txid& hash, ChangeType status) { + btcsignals::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](const Txid& hash, ChangeType status) { if (status == CT_NEW) txid = hash; })); ConfirmSend(/*text=*/nullptr, confirm_type); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 8647ae489435..f6eb9cd3ed59 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -155,10 +155,10 @@ class ScriptPubKeyMan }; /** Keypool has new keys */ - boost::signals2::signal NotifyCanGetAddressesChanged; + btcsignals::signal NotifyCanGetAddressesChanged; /** Birth time changed */ - boost::signals2::signal NotifyFirstKeyTimeChanged; + btcsignals::signal NotifyFirstKeyTimeChanged; }; /** OutputTypes supported by the LegacyScriptPubKeyMan */ diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a46a64555522..7d14ecff25df 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -826,13 +826,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void Close(); /** Wallet is about to be unloaded */ - boost::signals2::signal NotifyUnload; + btcsignals::signal NotifyUnload; /** * Address book entry changed. * @note called without lock cs_wallet held. */ - boost::signals2::signal NotifyAddressBookChanged; @@ -841,19 +841,19 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * Wallet transaction added, removed or updated. * @note called with lock cs_wallet held. */ - boost::signals2::signal NotifyTransactionChanged; + btcsignals::signal NotifyTransactionChanged; /** Show progress e.g. for rescan */ - boost::signals2::signal ShowProgress; + btcsignals::signal ShowProgress; /** Keypool has new keys */ - boost::signals2::signal NotifyCanGetAddressesChanged; + btcsignals::signal NotifyCanGetAddressesChanged; /** * Wallet status (encrypted, locked) changed. * Note: Called without locks held. */ - boost::signals2::signal NotifyStatusChanged; + btcsignals::signal NotifyStatusChanged; /** Inquire whether this wallet broadcasts transactions. */ bool GetBroadcastTransactions() const { return fBroadcastTransactions; } From 1f309d1aa2f3386cc17531f4c6369e3bbd46ee1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 9 Dec 2024 13:25:25 +0100 Subject: [PATCH 008/502] coins: make `Coin::fCoinBase` a bool The coinbase flag is semantically boolean but was stored as an unsigned bitfield. Store it as a `bool : 1` bitfield to better reflect intent and avoid relying on implicit integral conversions at call sites. Update users to prefer `Coin::IsCoinBase()` and to pass explicit `true`/`false` values where appropriate. --- src/coins.h | 2 +- src/core_io.cpp | 2 +- src/rpc/blockchain.cpp | 2 +- src/test/coins_tests.cpp | 8 ++++---- src/test/fuzz/utxo_snapshot.cpp | 4 ++-- src/validation.cpp | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/coins.h b/src/coins.h index 4b39c0bacd28..e58586dd6cdf 100644 --- a/src/coins.h +++ b/src/coins.h @@ -37,7 +37,7 @@ class Coin CTxOut out; //! whether containing transaction was a coinbase - unsigned int fCoinBase : 1; + bool fCoinBase : 1; //! at which height this containing transaction was included in the active block chain uint32_t nHeight : 31; diff --git a/src/core_io.cpp b/src/core_io.cpp index 7492e9ca50fb..eeda115699f6 100644 --- a/src/core_io.cpp +++ b/src/core_io.cpp @@ -480,7 +480,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry ScriptToUniv(prev_txout.scriptPubKey, /*out=*/o_script_pub_key, /*include_hex=*/true, /*include_address=*/true); UniValue p(UniValue::VOBJ); - p.pushKV("generated", static_cast(prev_coin.fCoinBase)); + p.pushKV("generated", prev_coin.IsCoinBase()); p.pushKV("height", prev_coin.nHeight); p.pushKV("value", ValueFromAmount(prev_txout.nValue)); p.pushKV("scriptPubKey", std::move(o_script_pub_key)); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 262b6bab0e31..9838bda6722d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1222,7 +1222,7 @@ static RPCHelpMan gettxout() UniValue o(UniValue::VOBJ); ScriptToUniv(coin->out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); ret.pushKV("scriptPubKey", std::move(o)); - ret.pushKV("coinbase", static_cast(coin->fCoinBase)); + ret.pushKV("coinbase", coin->IsCoinBase()); return ret; }, diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 8c0756d8528b..f56e369cc5eb 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -519,7 +519,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) // Good example Coin cc1; SpanReader{"97f23c835800816115944e077fe7c803cfa57f29b36bf87c1d35"_hex} >> cc1; - BOOST_CHECK_EQUAL(cc1.fCoinBase, false); + BOOST_CHECK_EQUAL(cc1.IsCoinBase(), false); BOOST_CHECK_EQUAL(cc1.nHeight, 203998U); BOOST_CHECK_EQUAL(cc1.out.nValue, CAmount{60000000000}); BOOST_CHECK_EQUAL(HexStr(cc1.out.scriptPubKey), HexStr(GetScriptForDestination(PKHash(uint160("816115944e077fe7c803cfa57f29b36bf87c1d35"_hex_u8))))); @@ -527,7 +527,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) // Good example Coin cc2; SpanReader{"8ddf77bbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa4"_hex} >> cc2; - BOOST_CHECK_EQUAL(cc2.fCoinBase, true); + BOOST_CHECK_EQUAL(cc2.IsCoinBase(), true); BOOST_CHECK_EQUAL(cc2.nHeight, 120891U); BOOST_CHECK_EQUAL(cc2.out.nValue, 110397); BOOST_CHECK_EQUAL(HexStr(cc2.out.scriptPubKey), HexStr(GetScriptForDestination(PKHash(uint160("8c988f1a4a4de2161e0f50aac7f17e7f9555caa4"_hex_u8))))); @@ -535,7 +535,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) // Smallest possible example Coin cc3; SpanReader{"000006"_hex} >> cc3; - BOOST_CHECK_EQUAL(cc3.fCoinBase, false); + BOOST_CHECK_EQUAL(cc3.IsCoinBase(), false); BOOST_CHECK_EQUAL(cc3.nHeight, 0U); BOOST_CHECK_EQUAL(cc3.out.nValue, 0); BOOST_CHECK_EQUAL(cc3.out.scriptPubKey.size(), 0U); @@ -871,7 +871,7 @@ Coin MakeCoin() Coin coin; coin.out.nValue = m_rng.rand32(); coin.nHeight = m_rng.randrange(4096); - coin.fCoinBase = 0; + coin.fCoinBase = false; return coin; } diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 67290f7d306b..29089743a097 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -137,7 +137,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) outfile << coinbase->GetHash(); WriteCompactSize(outfile, 1); // number of coins for the hash WriteCompactSize(outfile, 0); // index of coin - outfile << Coin(coinbase->vout[0], height, /*fCoinBaseIn=*/1); + outfile << Coin(coinbase->vout[0], height, /*fCoinBaseIn=*/true); height++; } } @@ -149,7 +149,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) outfile << coinbase->GetHash(); WriteCompactSize(outfile, 1); // number of coins for the hash WriteCompactSize(outfile, 999); // index of coin - outfile << Coin{coinbase->vout[0], /*nHeightIn=*/999, /*fCoinBaseIn=*/0}; + outfile << Coin{coinbase->vout[0], /*nHeightIn=*/999, /*fCoinBaseIn=*/false}; } assert(outfile.fclose() == 0); } diff --git a/src/validation.cpp b/src/validation.cpp index c200c3d6cd84..014757364912 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2214,7 +2214,7 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn COutPoint out(hash, o); Coin coin; bool is_spent = view.SpendCoin(out, &coin); - if (!is_spent || tx.vout[o] != coin.out || pindex->nHeight != coin.nHeight || is_coinbase != coin.fCoinBase) { + if (!is_spent || tx.vout[o] != coin.out || pindex->nHeight != coin.nHeight || is_coinbase != coin.IsCoinBase()) { if (!is_bip30_exception) { fClean = false; // transaction output mismatch } From 76190489e6c89ad091ebba0781c585afc9a39f1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 9 Dec 2024 13:30:04 +0100 Subject: [PATCH 009/502] coins: pack `Coin` height/coinbase consistently Serialize `Coin` metadata using the canonical (height << 1) | coinbase packing across `Coin` serialization, undo records, and coinstats hashing. Cast the 31-bit `nHeight` bitfield to `uint32_t` before shifting to avoid signed promotion undefined behaviour. --- src/coins.h | 4 ++-- src/kernel/coinstats.cpp | 2 +- src/undo.h | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coins.h b/src/coins.h index e58586dd6cdf..bfd6c5b0d7eb 100644 --- a/src/coins.h +++ b/src/coins.h @@ -27,7 +27,7 @@ * A UTXO entry. * * Serialized format: - * - VARINT((coinbase ? 1 : 0) | (height << 1)) + * - VARINT((height << 1) | (coinbase ? 1 : 0)) * - the non-spent CTxOut (via TxOutCompression) */ class Coin @@ -62,7 +62,7 @@ class Coin template void Serialize(Stream &s) const { assert(!IsSpent()); - uint32_t code = nHeight * uint32_t{2} + fCoinBase; + uint32_t code{(uint32_t{nHeight} << 1) | uint32_t{fCoinBase}}; ::Serialize(s, VARINT(code)); ::Serialize(s, Using(out)); } diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp index d287ec4be6e3..53039e571019 100644 --- a/src/kernel/coinstats.cpp +++ b/src/kernel/coinstats.cpp @@ -47,7 +47,7 @@ template static void TxOutSer(T& ss, const COutPoint& outpoint, const Coin& coin) { ss << outpoint; - ss << static_cast((coin.nHeight << 1) + coin.fCoinBase); + ss << ((uint32_t{coin.nHeight} << 1) | uint32_t{coin.fCoinBase}); ss << coin.out; } diff --git a/src/undo.h b/src/undo.h index 5591fe6cc822..13b923495100 100644 --- a/src/undo.h +++ b/src/undo.h @@ -23,7 +23,8 @@ struct TxInUndoFormatter { template void Ser(Stream &s, const Coin& txout) { - ::Serialize(s, VARINT(txout.nHeight * uint32_t{2} + txout.fCoinBase )); + uint32_t nCode{(uint32_t{txout.nHeight} << 1) | uint32_t{txout.fCoinBase}}; + ::Serialize(s, VARINT(nCode)); if (txout.nHeight > 0) { // Required to maintain compatibility with older undo format. ::Serialize(s, (unsigned char)0); From 5f36e0ff1e7681988815664559fbc0e8c0b4b3dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 9 Dec 2024 14:02:49 +0100 Subject: [PATCH 010/502] rpc: fix getblockstats UTXO overhead accounting `Coin` packs height and the coinbase flag into a single 32-bit value, so `utxo_size_inc(_actual)` should not count an additional boolean. Update the calculation and adjust the `rpc_getblockstats` test expectations. --- src/rpc/blockchain.cpp | 4 ++-- test/functional/data/rpc_getblockstats.json | 12 ++++++------ test/functional/rpc_getblockstats.py | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9838bda6722d..eba10f0ddc7c 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1898,8 +1898,8 @@ static inline bool SetHasKeys(const std::set& set, const Tk& key, const Args& return (set.contains(key)) || SetHasKeys(set, args...); } -// outpoint (needed for the utxo index) + nHeight + fCoinBase -static constexpr size_t PER_UTXO_OVERHEAD = sizeof(COutPoint) + sizeof(uint32_t) + sizeof(bool); +// outpoint (needed for the utxo index) + nHeight|fCoinBase +static constexpr size_t PER_UTXO_OVERHEAD = sizeof(COutPoint) + sizeof(uint32_t); static RPCHelpMan getblockstats() { diff --git a/test/functional/data/rpc_getblockstats.json b/test/functional/data/rpc_getblockstats.json index 7d7460aacc8c..e0c0eb39fbc5 100644 --- a/test/functional/data/rpc_getblockstats.json +++ b/test/functional/data/rpc_getblockstats.json @@ -143,8 +143,8 @@ "txs": 1, "utxo_increase": 2, "utxo_increase_actual": 1, - "utxo_size_inc": 163, - "utxo_size_inc_actual": 75 + "utxo_size_inc": 161, + "utxo_size_inc_actual": 74 }, { "avgfee": 4440, @@ -182,8 +182,8 @@ "txs": 2, "utxo_increase": 3, "utxo_increase_actual": 2, - "utxo_size_inc": 235, - "utxo_size_inc_actual": 147 + "utxo_size_inc": 232, + "utxo_size_inc_actual": 145 }, { "avgfee": 21390, @@ -221,8 +221,8 @@ "txs": 5, "utxo_increase": 6, "utxo_increase_actual": 4, - "utxo_size_inc": 441, - "utxo_size_inc_actual": 300 + "utxo_size_inc": 435, + "utxo_size_inc_actual": 296 } ] } \ No newline at end of file diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py index 29d70a056e2c..75e096392972 100755 --- a/test/functional/rpc_getblockstats.py +++ b/test/functional/rpc_getblockstats.py @@ -171,16 +171,16 @@ def run_test(self): genesis_stats = self.nodes[0].getblockstats(0) assert_equal(genesis_stats["blockhash"], "0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206") assert_equal(genesis_stats["utxo_increase"], 1) - assert_equal(genesis_stats["utxo_size_inc"], 117) + assert_equal(genesis_stats["utxo_size_inc"], 116) assert_equal(genesis_stats["utxo_increase_actual"], 0) assert_equal(genesis_stats["utxo_size_inc_actual"], 0) self.log.info('Test tip including OP_RETURN') tip_stats = self.nodes[0].getblockstats(tip) assert_equal(tip_stats["utxo_increase"], 6) - assert_equal(tip_stats["utxo_size_inc"], 441) + assert_equal(tip_stats["utxo_size_inc"], 435) assert_equal(tip_stats["utxo_increase_actual"], 4) - assert_equal(tip_stats["utxo_size_inc_actual"], 300) + assert_equal(tip_stats["utxo_size_inc_actual"], 296) self.log.info("Test when only header is known") block = self.generateblock(self.nodes[0], output="raw(55)", transactions=[], submit=False) From b8827ce6190741250d6ee7b5a10c69f97663fb27 Mon Sep 17 00:00:00 2001 From: b-l-u-e Date: Mon, 5 Jan 2026 21:52:15 +0300 Subject: [PATCH 011/502] net: Fix Discover() not running when using -bind=0.0.0.0:port Signed-off-by: b-l-u-e --- src/init.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index e6cc2045b4a9..9c22c3c51b7d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2140,7 +2140,26 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) StartTorControl(onion_service_target); } - if (connOptions.bind_on_any) { + bool should_discover = connOptions.bind_on_any; + if (!should_discover) { + for (const auto& bind : connOptions.vBinds) { + if (bind.IsBindAny()) { + should_discover = true; + break; + } + } + } + + if (!should_discover) { + for (const auto& whitebind : connOptions.vWhiteBinds) { + if (whitebind.m_service.IsBindAny()) { + should_discover = true; + break; + } + } + } + + if (should_discover) { // Only add all IP addresses of the machine if we would be listening on // any address - 0.0.0.0 (IPv4) and :: (IPv6). Discover(); From 4f19508ae7e6b230da95eb695a8a0c17de3276a9 Mon Sep 17 00:00:00 2001 From: b-l-u-e Date: Sun, 15 Feb 2026 00:10:35 +0300 Subject: [PATCH 012/502] test: dont connect nodes in feature_bind_port_discover Signed-off-by: b-l-u-e --- test/functional/feature_bind_port_discover.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/functional/feature_bind_port_discover.py b/test/functional/feature_bind_port_discover.py index 338dbd02b3ab..f3fe34542330 100755 --- a/test/functional/feature_bind_port_discover.py +++ b/test/functional/feature_bind_port_discover.py @@ -38,6 +38,24 @@ def set_test_params(self): ] self.num_nodes = len(self.extra_args) + def setup_network(self): + """ + Override to avoid connecting nodes together. This test intentionally does not connect nodes + because each node is bound to a different address or interface, and connections are not needed. + """ + self.setup_nodes() + + def setup_nodes(self): + """ + Override to set has_explicit_bind=True for nodes with explicit bind arguments. + """ + self.add_nodes(self.num_nodes, self.extra_args) + # TestNode.start() will add -bind= to extra_args if has_explicit_bind is + # False. We do not want any -bind= thus set has_explicit_bind to True. + for node in self.nodes: + node.has_explicit_bind = True + self.start_nodes() + def add_options(self, parser): parser.add_argument( "--ihave1111and2222", action='store_true', dest="ihave1111and2222", From bb00fd2142288be2da169a3b6ed398a01626827d Mon Sep 17 00:00:00 2001 From: b-l-u-e Date: Sun, 15 Feb 2026 00:18:32 +0300 Subject: [PATCH 013/502] test: use dynamic ports and add coverage in feature_bind_port_discover Signed-off-by: b-l-u-e --- test/functional/feature_bind_port_discover.py | 74 ++++++++++++------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/test/functional/feature_bind_port_discover.py b/test/functional/feature_bind_port_discover.py index f3fe34542330..613b111b33f6 100755 --- a/test/functional/feature_bind_port_discover.py +++ b/test/functional/feature_bind_port_discover.py @@ -10,31 +10,46 @@ from test_framework.util import ( assert_equal, assert_not_equal, + p2p_port, + tor_port, ) -# We need to bind to a routable address for this test to exercise the relevant code -# and also must have another routable address on another interface which must not -# be named "lo" or "lo0". -# To set these routable addresses on the machine, use: +# We need to bind to routable addresses for this test. Both addresses must be on an +# interface that is UP and not a loopback interface (IFF_LOOPBACK). To set these +# routable addresses on the machine, use: # Linux: -# ifconfig lo:0 1.1.1.1/32 up && ifconfig lo:1 2.2.2.2/32 up # to set up -# ifconfig lo:0 down && ifconfig lo:1 down # to remove it, after the test +# First find your interfaces: ip addr show +# Then use your actual interface names (replace INTERFACE_NAME with yours): +# ip addr add 1.1.1.1/32 dev INTERFACE_NAME && ip addr add 2.2.2.2/32 dev INTERFACE_NAME # to set up +# ip addr del 1.1.1.1/32 dev INTERFACE_NAME && ip addr del 2.2.2.2/32 dev INTERFACE_NAME # to remove it +# +# macOS: +# ifconfig en0 alias 1.1.1.1 && ifconfig en0 alias 2.2.2.2 # to set up +# ifconfig en0 1.1.1.1 -alias && ifconfig en0 2.2.2.2 -alias # to remove it, after the test +# # FreeBSD: # ifconfig em0 1.1.1.1/32 alias && ifconfig wlan0 2.2.2.2/32 alias # to set up # ifconfig em0 1.1.1.1 -alias && ifconfig wlan0 2.2.2.2 -alias # to remove it, after the test ADDR1 = '1.1.1.1' ADDR2 = '2.2.2.2' -BIND_PORT = 31001 - class BindPortDiscoverTest(BitcoinTestFramework): def set_test_params(self): # Avoid any -bind= on the command line. Force the framework to avoid adding -bind=127.0.0.1. - self.setup_clean_chain = True self.bind_to_localhost_only = False + # Get dynamic ports for each node from the test framework + self.bind_ports = [ + p2p_port(0), + p2p_port(2), # node0 will use their port + 1 for onion listen, which is the same as p2p_port(1), so avoid collision + p2p_port(3), + p2p_port(4), + ] self.extra_args = [ - ['-discover', f'-port={BIND_PORT}'], # bind on any - ['-discover', f'-bind={ADDR1}:{BIND_PORT}'], + ['-discover', f'-port={self.bind_ports[0]}', '-listen=1'], # Without any -bind + ['-discover', f'-bind=0.0.0.0:{self.bind_ports[1]}'], # Explicit -bind=0.0.0.0 + # Explicit -whitebind=0.0.0.0, add onion bind to avoid port conflict + ['-discover', f'-whitebind=0.0.0.0:{self.bind_ports[2]}', f'-bind=127.0.0.1:{tor_port(3)}=onion'], + ['-discover', f'-bind={ADDR1}:{self.bind_ports[3]}'], # Explicit -bind=routable_addr ] self.num_nodes = len(self.extra_args) @@ -70,28 +85,35 @@ def skip_test_if_missing_module(self): def run_test(self): self.log.info( - "Test that if -bind= is not passed then all addresses are " + "Test that if -bind= is not passed or -bind=0.0.0.0 is used then all addresses are " "added to localaddresses") - found_addr1 = False - found_addr2 = False - for local in self.nodes[0].getnetworkinfo()['localaddresses']: - if local['address'] == ADDR1: - found_addr1 = True - assert_equal(local['port'], BIND_PORT) - if local['address'] == ADDR2: - found_addr2 = True - assert_equal(local['port'], BIND_PORT) - assert found_addr1 - assert found_addr2 + for i in [0, 1, 2]: + found_addr1 = False + found_addr2 = False + localaddresses = self.nodes[i].getnetworkinfo()['localaddresses'] + for local in localaddresses: + if local['address'] == ADDR1: + found_addr1 = True + assert_equal(local['port'], self.bind_ports[i]) + if local['address'] == ADDR2: + found_addr2 = True + assert_equal(local['port'], self.bind_ports[i]) + if not found_addr1: + self.log.error(f"Address {ADDR1} not found in node{i}'s local addresses: {localaddresses}") + assert False + if not found_addr2: + self.log.error(f"Address {ADDR2} not found in node{i}'s local addresses: {localaddresses}") + assert False self.log.info( - "Test that if -bind= is passed then only that address is " + "Test that if -bind=routable_addr is passed then only that address is " "added to localaddresses") found_addr1 = False - for local in self.nodes[1].getnetworkinfo()['localaddresses']: + i = 3 + for local in self.nodes[i].getnetworkinfo()['localaddresses']: if local['address'] == ADDR1: found_addr1 = True - assert_equal(local['port'], BIND_PORT) + assert_equal(local['port'], self.bind_ports[i]) assert_not_equal(local['address'], ADDR2) assert found_addr1 From 747da2536092804f9c488e34977ffbb054edabc8 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 24 Feb 2026 17:09:59 -0500 Subject: [PATCH 014/502] feefrac: drop comparison and operator{<<,>>} for sorted wrappers Instead of having an unintuitive but total implicit sort order on FeeFrac (first increasing feerate, then decreasing size), and separate overloaded operator<< and operator>> for a weak ordering that only looks at feerate, replace these with explicit wrapper classes which make the behavior more explicit. This allows for things like ByRatio{a} <= ByRatio{b}, instead of the earlier !(a >> b). It also supports usage inside std::max and std::greater, so one can use: * std::max>(a, b) * std::sort(v.begin(), v.end(), std::greater>{}) --- src/cluster_linearize.h | 20 ++-- src/node/miner.cpp | 2 +- src/node/mini_miner.cpp | 4 +- src/node/txorphanage.cpp | 20 ++-- src/policy/feerate.h | 6 +- src/test/feefrac_tests.cpp | 62 +++++------ src/test/fuzz/cluster_linearize.cpp | 20 ++-- src/test/fuzz/feefrac.cpp | 20 ++-- src/test/fuzz/feeratediagram.cpp | 6 +- src/test/fuzz/rbf.cpp | 4 +- src/test/fuzz/txgraph.cpp | 26 ++--- src/test/fuzz/txorphan.cpp | 6 +- src/txgraph.cpp | 19 ++-- src/util/feefrac.cpp | 6 +- src/util/feefrac.h | 154 +++++++++++++++++++--------- 15 files changed, 216 insertions(+), 159 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index 774bc61734f0..1bf3d475f10e 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -432,7 +432,7 @@ std::vector> ChunkLinearizationInfo(const DepGraph& de /** The new chunk to be added, initially a singleton. */ SetInfo new_chunk(depgraph, i); // As long as the new chunk has a higher feerate than the last chunk so far, absorb it. - while (!ret.empty() && new_chunk.feerate >> ret.back().feerate) { + while (!ret.empty() && ByRatio{new_chunk.feerate} > ByRatio{ret.back().feerate}) { new_chunk |= ret.back(); ret.pop_back(); } @@ -452,7 +452,7 @@ std::vector ChunkLinearization(const DepGraph& depgraph, std:: /** The new chunk to be added, initially a singleton. */ auto new_chunk = depgraph.FeeRate(i); // As long as the new chunk has a higher feerate than the last chunk so far, absorb it. - while (!ret.empty() && new_chunk >> ret.back()) { + while (!ret.empty() && ByRatio{new_chunk} > ByRatio{ret.back()}) { new_chunk += ret.back(); ret.pop_back(); } @@ -1027,8 +1027,8 @@ class SpanningForestState auto& reached_chunk_info = m_set_info[reached_chunk_idx]; todo -= reached_chunk_info.transactions; // See if it has an acceptable feerate. - auto cmp = DownWard ? FeeRateCompare(best_other_chunk_feerate, reached_chunk_info.feerate) - : FeeRateCompare(reached_chunk_info.feerate, best_other_chunk_feerate); + auto cmp = DownWard ? ByRatio{best_other_chunk_feerate} <=> ByRatio{reached_chunk_info.feerate} + : ByRatio{reached_chunk_info.feerate} <=> ByRatio{best_other_chunk_feerate}; if (cmp > 0) continue; uint64_t tiebreak = m_rng.rand64(); if (cmp < 0 || tiebreak >= best_other_chunk_tiebreak) { @@ -1150,7 +1150,7 @@ class SpanningForestState auto& dep_top_info = m_set_info[tx_data.dep_top_idx[child_idx]]; // Skip if this dependency is ineligible (the top chunk that would be created // does not have higher feerate than the chunk it is currently part of). - auto cmp = FeeRateCompare(dep_top_info.feerate, chunk_info.feerate); + auto cmp = ByRatio{dep_top_info.feerate} <=> ByRatio{chunk_info.feerate}; if (cmp <= 0) continue; // Generate a random tiebreak for this dependency, and reject it if its tiebreak // is worse than the best so far. This means that among all eligible @@ -1377,7 +1377,7 @@ class SpanningForestState // Skip if this dependency does not have equal top and bottom set feerates. Note // that the top cannot have higher feerate than the bottom, or OptimizeSteps would // have dealt with it. - if (dep_top_info.feerate << chunk_info.feerate) continue; + if (ByRatio{dep_top_info.feerate} < ByRatio{chunk_info.feerate}) continue; have_any = true; // Skip if this dependency does not have pivot in the right place. if (move_pivot_down == dep_top_info.transactions[pivot_idx]) continue; @@ -1506,7 +1506,7 @@ class SpanningForestState // First sort by increasing transaction feerate. auto& a_feerate = m_depgraph.FeeRate(a); auto& b_feerate = m_depgraph.FeeRate(b); - auto feerate_cmp = FeeRateCompare(a_feerate, b_feerate); + auto feerate_cmp = ByRatio{a_feerate} <=> ByRatio{b_feerate}; if (feerate_cmp != 0) return feerate_cmp < 0; // Then by decreasing transaction size. if (a_feerate.size != b_feerate.size) { @@ -1528,7 +1528,7 @@ class SpanningForestState // First sort by increasing chunk feerate. auto& chunk_feerate_a = m_set_info[a.first].feerate; auto& chunk_feerate_b = m_set_info[b.first].feerate; - auto feerate_cmp = FeeRateCompare(chunk_feerate_a, chunk_feerate_b); + auto feerate_cmp = ByRatio{chunk_feerate_a} <=> ByRatio{chunk_feerate_b}; if (feerate_cmp != 0) return feerate_cmp < 0; // Then by decreasing chunk size. if (chunk_feerate_a.size != chunk_feerate_b.size) { @@ -1618,7 +1618,7 @@ class SpanningForestState for (auto chunk_idx : m_chunk_idxs) { ret.push_back(m_set_info[chunk_idx].feerate); } - std::sort(ret.begin(), ret.end(), std::greater{}); + std::sort(ret.begin(), ret.end(), std::greater>{}); return ret; } @@ -1972,7 +1972,7 @@ void PostLinearize(const DepGraph& depgraph, std::span l DepGraphIndex next_group = SENTINEL; // We inserted at the end, so next group is sentinel. DepGraphIndex prev_group = entries[cur_group].prev_group; // Continue as long as the current group has higher feerate than the previous one. - while (entries[cur_group].feerate >> entries[prev_group].feerate) { + while (ByRatio{entries[cur_group].feerate} > ByRatio{entries[prev_group].feerate}) { // prev_group/cur_group/next_group refer to (the last transactions of) 3 // consecutive entries in groups list. Assume(cur_group == entries[next_group].prev_group); diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 7ea8d10b7306..836b828b5c42 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -295,7 +295,7 @@ void BlockAssembler::addChunks() while (selected_transactions.size() > 0) { // Check to see if min fee rate is still respected. - if (chunk_feerate_vsize << m_options.blockMinFeeRate.GetFeePerVSize()) { + if (ByRatio{chunk_feerate_vsize} < ByRatio{m_options.blockMinFeeRate.GetFeePerVSize()}) { // Everything else we might consider has a lower feerate return; } diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index 9d85db7ded0a..59ca1b556d6d 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -184,12 +184,12 @@ struct AncestorFeerateComparator auto min_feerate = [](const MiniMinerMempoolEntry& e) -> FeeFrac { FeeFrac self_feerate(e.GetModifiedFee(), e.GetTxSize()); FeeFrac ancestor_feerate(e.GetModFeesWithAncestors(), e.GetSizeWithAncestors()); - return std::min(ancestor_feerate, self_feerate); + return std::min>(ancestor_feerate, self_feerate); }; FeeFrac a_feerate{min_feerate(a->second)}; FeeFrac b_feerate{min_feerate(b->second)}; if (a_feerate != b_feerate) { - return a_feerate > b_feerate; + return ByRatioNegSize{a_feerate} > ByRatioNegSize{b_feerate}; } // Use txid as tiebreaker for stable sorting return a->first < b->first; diff --git a/src/node/txorphanage.cpp b/src/node/txorphanage.cpp index ca7eb20470a7..32608e057699 100644 --- a/src/node/txorphanage.cpp +++ b/src/node/txorphanage.cpp @@ -164,7 +164,7 @@ class TxOrphanageImpl final : public TxOrphanage { assert(max_peer_memory > 0); const FeeFrac latency_score(m_total_latency_score, max_peer_latency_score); const FeeFrac mem_score(m_total_usage, max_peer_memory); - return std::max(latency_score, mem_score); + return std::max>(latency_score, mem_score); } }; /** Store per-peer statistics. Used to determine each peer's DoS score. The size of this map is used to determine the @@ -457,12 +457,18 @@ void TxOrphanageImpl::LimitOrphans() for (const auto& [nodeid, entry] : m_peer_orphanage_info) { // Performance optimization: only consider peers with a DoS score > 1. const auto dos_score = entry.GetDosScore(max_lat, max_mem); - if (dos_score >> FeeFrac{1, 1}) { + if (ByRatio{dos_score} > ByRatio{FeeFrac{1, 1}}) { heap_peer_dos.emplace_back(nodeid, dos_score); } } static constexpr auto compare_score = [](const auto& left, const auto& right) { - if (left.second != right.second) return left.second < right.second; + if (left.second != right.second) { + // Note: if ratios are the same, this tiebreaks by denominator. In practice, since the + // latency denominator (number of announcements and inputs) is always lower, this means + // that a peer with only high latency scores will be targeted before a peer using a lot + // of memory, even if they have the same ratios. + return ByRatioNegSize{left.second} < ByRatioNegSize{right.second}; + } // Tiebreak by considering the more recent peer (higher NodeId) to be worse. return left.first < right.first; }; @@ -472,9 +478,6 @@ void TxOrphanageImpl::LimitOrphans() // This outer loop finds the peer with the highest DoS score, which is a fraction of memory and latency scores // over the respective allowances. We continue until the orphanage is within global limits. That means some peers // might still have a DoS score > 1 at the end. - // Note: if ratios are the same, FeeFrac tiebreaks by denominator. In practice, since the latency denominator (number of - // announcements and inputs) is always lower, this means that a peer with only high latency scores will be targeted - // before a peer using a lot of memory, even if they have the same ratios. do { Assume(!heap_peer_dos.empty()); // This is a max-heap, so the worst peer is at the front. pop_heap() @@ -484,7 +487,7 @@ void TxOrphanageImpl::LimitOrphans() heap_peer_dos.pop_back(); // If needs trim, then at least one peer has a DoS score higher than 1. - Assume(dos_score >> (FeeFrac{1, 1})); + Assume(ByRatio{dos_score} > ByRatio{FeeFrac(1, 1)}); auto it_worst_peer = m_peer_orphanage_info.find(worst_peer); @@ -506,7 +509,8 @@ void TxOrphanageImpl::LimitOrphans() // If we erased the last orphan from this peer, it_worst_peer will be invalidated. it_worst_peer = m_peer_orphanage_info.find(worst_peer); - if (it_worst_peer == m_peer_orphanage_info.end() || it_worst_peer->second.GetDosScore(max_lat, max_mem) <= dos_threshold) break; + if (it_worst_peer == m_peer_orphanage_info.end() || + ByRatioNegSize{it_worst_peer->second.GetDosScore(max_lat, max_mem)} <= ByRatioNegSize{dos_threshold}) break; } LogDebug(BCLog::TXPACKAGES, "peer=%d orphanage overflow, removed %u of %u announcements\n", worst_peer, num_erased_this_round, starting_num_ann); diff --git a/src/policy/feerate.h b/src/policy/feerate.h index f6b49a1465b3..8f13f8d0dca1 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -60,13 +60,13 @@ class CFeeRate * Return the fee in satoshis for a vsize of 1000 vbytes */ CAmount GetFeePerK() const { return CAmount(m_feerate.EvaluateFeeDown(1000)); } - friend std::weak_ordering operator<=>(const CFeeRate& a, const CFeeRate& b) noexcept + friend std::strong_ordering operator<=>(const CFeeRate& a, const CFeeRate& b) noexcept { - return FeeRateCompare(a.m_feerate, b.m_feerate); + return ByRatio{a.m_feerate} <=> ByRatio{b.m_feerate}; } friend bool operator==(const CFeeRate& a, const CFeeRate& b) noexcept { - return FeeRateCompare(a.m_feerate, b.m_feerate) == std::weak_ordering::equivalent; + return ByRatio{a.m_feerate} == ByRatio{b.m_feerate}; } CFeeRate& operator+=(const CFeeRate& a) { m_feerate = FeePerVSize(GetFeePerK() + a.GetFeePerK(), 1000); diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp index ff95fc9efb88..3d5fe0cb88cf 100644 --- a/src/test/feefrac_tests.cpp +++ b/src/test/feefrac_tests.cpp @@ -70,40 +70,40 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) BOOST_CHECK(p1 + p3 == p4); // Fee-rate comparison - BOOST_CHECK(p1 > p2); - BOOST_CHECK(p1 >= p2); - BOOST_CHECK(p1 >= p4-p3); - BOOST_CHECK(!(p1 >> p3)); // not strictly better - BOOST_CHECK(p1 >> p2); // strictly greater feerate - - BOOST_CHECK(p2 < p1); - BOOST_CHECK(p2 <= p1); - BOOST_CHECK(p1 <= p4-p3); - BOOST_CHECK(!(p3 << p1)); // not strictly worse - BOOST_CHECK(p2 << p1); // strictly lower feerate + BOOST_CHECK(ByRatioNegSize{p1} > ByRatioNegSize{p2}); + BOOST_CHECK(ByRatioNegSize{p1} >= ByRatioNegSize{p2}); + BOOST_CHECK(ByRatioNegSize{p1} >= ByRatioNegSize{p4-p3}); + BOOST_CHECK(!(ByRatio{p1} > ByRatio{p3})); // not strictly better + BOOST_CHECK(ByRatio{p1} > ByRatio{p2}); // strictly greater feerate + + BOOST_CHECK(ByRatioNegSize{p2} < ByRatioNegSize{p1}); + BOOST_CHECK(ByRatioNegSize{p2} <= ByRatioNegSize{p1}); + BOOST_CHECK(ByRatioNegSize{p1} <= ByRatioNegSize{p4-p3}); + BOOST_CHECK(!(ByRatio{p3} < ByRatio{p1})); // not strictly worse + BOOST_CHECK(ByRatio{p2} < ByRatio{p1}); // strictly lower feerate // "empty" comparisons - BOOST_CHECK(!(p1 >> empty)); // << will always result in false - BOOST_CHECK(!(p1 << empty)); - BOOST_CHECK(!(empty >> empty)); - BOOST_CHECK(!(empty << empty)); + BOOST_CHECK(!(ByRatio{p1} > ByRatio{empty})); // << will always result in false + BOOST_CHECK(!(ByRatio{p1} < ByRatio{empty})); + BOOST_CHECK(!(ByRatio{empty} > ByRatio{empty})); + BOOST_CHECK(!(ByRatio{empty} < ByRatio{empty})); // empty is always bigger than everything else - BOOST_CHECK(empty > p1); - BOOST_CHECK(empty > p2); - BOOST_CHECK(empty > p3); - BOOST_CHECK(empty >= p1); - BOOST_CHECK(empty >= p2); - BOOST_CHECK(empty >= p3); + BOOST_CHECK(ByRatioNegSize{empty} > ByRatioNegSize{p1}); + BOOST_CHECK(ByRatioNegSize{empty} > ByRatioNegSize{p2}); + BOOST_CHECK(ByRatioNegSize{empty} > ByRatioNegSize{p3}); + BOOST_CHECK(ByRatioNegSize{empty} >= ByRatioNegSize{p1}); + BOOST_CHECK(ByRatioNegSize{empty} >= ByRatioNegSize{p2}); + BOOST_CHECK(ByRatioNegSize{empty} >= ByRatioNegSize{p3}); // check "max" values for comparison FeeFrac oversized_1{4611686000000, 4000000}; FeeFrac oversized_2{184467440000000, 100000}; - BOOST_CHECK(oversized_1 < oversized_2); - BOOST_CHECK(oversized_1 <= oversized_2); - BOOST_CHECK(oversized_1 << oversized_2); - BOOST_CHECK(oversized_1 != oversized_2); + BOOST_CHECK(ByRatioNegSize{oversized_1} < ByRatioNegSize{oversized_2}); + BOOST_CHECK(ByRatioNegSize{oversized_1} <= ByRatioNegSize{oversized_2}); + BOOST_CHECK(ByRatio{oversized_1} < ByRatio{oversized_2}); + BOOST_CHECK(ByRatioNegSize{oversized_1} != ByRatioNegSize{oversized_2}); BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(0), 0); BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(1), 1152921); @@ -124,13 +124,13 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) // Tests paths that use double arithmetic FeeFrac busted{(static_cast(INT32_MAX)) + 1, INT32_MAX}; - BOOST_CHECK(!(busted < busted)); + BOOST_CHECK(!(ByRatioNegSize{busted} < ByRatioNegSize{busted})); FeeFrac max_fee{2100000000000000, INT32_MAX}; - BOOST_CHECK(!(max_fee < max_fee)); - BOOST_CHECK(!(max_fee > max_fee)); - BOOST_CHECK(max_fee <= max_fee); - BOOST_CHECK(max_fee >= max_fee); + BOOST_CHECK(!(ByRatioNegSize{max_fee} < ByRatioNegSize{max_fee})); + BOOST_CHECK(!(ByRatioNegSize{max_fee} > ByRatioNegSize{max_fee})); + BOOST_CHECK(ByRatioNegSize{max_fee} <= ByRatioNegSize{max_fee}); + BOOST_CHECK(ByRatioNegSize{max_fee} >= ByRatioNegSize{max_fee}); BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(0), 0); BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(1), 977888); @@ -146,7 +146,7 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(INT32_MAX), 2100000000000000); FeeFrac max_fee2{1, 1}; - BOOST_CHECK(max_fee >= max_fee2); + BOOST_CHECK(ByRatioNegSize{max_fee} >= ByRatioNegSize{max_fee2}); // Test for integer overflow issue (https://github.com/bitcoin/bitcoin/issues/32294) BOOST_CHECK_EQUAL((FeeFrac{0x7ffffffdfffffffb, 0x7ffffffd}.EvaluateFeeDown(0x7fffffff)), 0x7fffffffffffffff); diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index b735940100a9..e529078c65d6 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -126,7 +126,7 @@ class SimpleCandidateFinder // Add a queue entry with split excluded. queue.emplace_back(inc, und - m_depgraph.Descendants(split)); // Update statistics to account for the candidate new_inc. - if (new_inc.feerate > best.feerate) best = new_inc; + if (ByRatioNegSize{new_inc.feerate} > ByRatioNegSize{best.feerate}) best = new_inc; break; } } @@ -181,7 +181,7 @@ class ExhaustiveCandidateFinder x_shifted >>= 1; } SetInfo cur(m_depgraph, txn & m_todo); - if (cur.feerate > best.feerate) best = cur; + if (ByRatioNegSize{cur.feerate} > ByRatioNegSize{best.feerate}) best = cur; } return best; } @@ -737,7 +737,7 @@ FUZZ_TARGET(clusterlin_chunking) // Verify that chunk feerates are monotonically non-increasing. for (size_t i = 1; i < chunking.size(); ++i) { - assert(!(chunking[i] >> chunking[i - 1])); + assert(ByRatio{chunking[i]} <= ByRatio{chunking[i - 1]}); } // Naively recompute the chunks (each is the highest-feerate prefix of what remains). @@ -748,7 +748,7 @@ FUZZ_TARGET(clusterlin_chunking) for (DepGraphIndex idx : linearization) { if (todo[idx]) { accumulator.Set(depgraph, idx); - if (best.feerate.IsEmpty() || accumulator.feerate >> best.feerate) { + if (best.feerate.IsEmpty() || ByRatio{accumulator.feerate} > ByRatio{best.feerate}) { best = accumulator; } } @@ -825,7 +825,7 @@ FUZZ_TARGET(clusterlin_simple_finder) // Compare with a non-empty topological set read from the fuzz input (comparing with an // empty set is not interesting). auto read_topo = ReadTopologicalSet(depgraph, todo, reader, /*non_empty=*/true); - assert(found.feerate >= depgraph.FeeRate(read_topo)); + assert(ByRatioNegSize{found.feerate} >= ByRatioNegSize{depgraph.FeeRate(read_topo)}); } // Find a non-empty topologically valid subset of transactions to remove from the graph. @@ -1110,9 +1110,9 @@ FUZZ_TARGET(clusterlin_linearize) // Check whether tx2 only depends on transactions that precede tx1. if ((depgraph.Ancestors(tx2) - done).Count() == 1) { // tx2 could take position pos1. - // Verify that individual transaction feerate is decreasing (note that >= - // tie-breaks by size). - assert(depgraph.FeeRate(tx1) >= depgraph.FeeRate(tx2)); + // Verify that individual transaction feerate is decreasing (tie-breaking by + // size). + assert(ByRatioNegSize{depgraph.FeeRate(tx1)} >= ByRatioNegSize{depgraph.FeeRate(tx2)}); // If feerate and size are equal, compare by DepGraphIndex. if (depgraph.FeeRate(tx1) == depgraph.FeeRate(tx2)) { assert(tx1 < tx2); @@ -1139,8 +1139,8 @@ FUZZ_TARGET(clusterlin_linearize) // Check whether chunk2 only depends on transactions that precede chunk1. if ((chunk2_ancestors - done).IsSubsetOf(chunk2.transactions)) { // chunk2 could take position chunk_num1. - // Verify that chunk feerate is decreasing (note that >= tie-breaks by size). - assert(chunk1.feerate >= chunk2.feerate); + // Verify that chunk feerate is decreasing (tie-breaking by size). + assert(ByRatioNegSize{chunk1.feerate} >= ByRatioNegSize{chunk2.feerate}); // If feerate and size are equal, compare by maximum DepGraphIndex element. if (chunk1.feerate == chunk2.feerate) { assert(chunk1.transactions.Last() < chunk2.transactions.Last()); diff --git a/src/test/fuzz/feefrac.cpp b/src/test/fuzz/feefrac.cpp index d52a46582c35..6b6a348387fb 100644 --- a/src/test/fuzz/feefrac.cpp +++ b/src/test/fuzz/feefrac.cpp @@ -84,9 +84,9 @@ FUZZ_TARGET(feefrac) // Feerate comparisons auto cmp_feerate = MulCompare(f1, s2, f2, s1); - assert(FeeRateCompare(fr1, fr2) == cmp_feerate); - assert((fr1 << fr2) == std::is_lt(cmp_feerate)); - assert((fr1 >> fr2) == std::is_gt(cmp_feerate)); + assert((ByRatio{fr1} <=> ByRatio{fr2}) == cmp_feerate); + assert((ByRatio{fr1} < ByRatio{fr2}) == std::is_lt(cmp_feerate)); + assert((ByRatio{fr1} > ByRatio{fr2}) == std::is_gt(cmp_feerate)); // Compare with manual invocation of FeeFrac::Mul. auto cmp_mul = FeeFrac::Mul(f1, s2) <=> FeeFrac::Mul(f2, s1); @@ -98,13 +98,13 @@ FUZZ_TARGET(feefrac) // Total order comparisons auto cmp_total = std::is_eq(cmp_feerate) ? (s2 <=> s1) : cmp_feerate; - assert((fr1 <=> fr2) == cmp_total); - assert((fr1 < fr2) == std::is_lt(cmp_total)); - assert((fr1 > fr2) == std::is_gt(cmp_total)); - assert((fr1 <= fr2) == std::is_lteq(cmp_total)); - assert((fr1 >= fr2) == std::is_gteq(cmp_total)); - assert((fr1 == fr2) == std::is_eq(cmp_total)); - assert((fr1 != fr2) == std::is_neq(cmp_total)); + assert((ByRatioNegSize{fr1} <=> ByRatioNegSize{fr2}) == cmp_total); + assert((ByRatioNegSize{fr1} < ByRatioNegSize{fr2}) == std::is_lt(cmp_total)); + assert((ByRatioNegSize{fr1} > ByRatioNegSize{fr2}) == std::is_gt(cmp_total)); + assert((ByRatioNegSize{fr1} <= ByRatioNegSize{fr2}) == std::is_lteq(cmp_total)); + assert((ByRatioNegSize{fr1} >= ByRatioNegSize{fr2}) == std::is_gteq(cmp_total)); + assert((ByRatioNegSize{fr1} == ByRatioNegSize{fr2}) == std::is_eq(cmp_total)); + assert((ByRatioNegSize{fr1} != ByRatioNegSize{fr2}) == std::is_neq(cmp_total)); } FUZZ_TARGET(feefrac_div_fallback) diff --git a/src/test/fuzz/feeratediagram.cpp b/src/test/fuzz/feeratediagram.cpp index 6dcc63e91e2f..3a2d4e4ae266 100644 --- a/src/test/fuzz/feeratediagram.cpp +++ b/src/test/fuzz/feeratediagram.cpp @@ -63,9 +63,9 @@ FeeFrac EvaluateDiagram(int32_t size, std::span diagram) return {point_a.fee * dir_coef.size + dir_coef.fee * (size - point_a.size), dir_coef.size}; } -std::weak_ordering CompareFeeFracWithDiagram(const FeeFrac& ff, std::span diagram) +std::strong_ordering CompareFeeFracWithDiagram(const FeeFrac& ff, std::span diagram) { - return FeeRateCompare(FeeFrac{ff.fee, 1}, EvaluateDiagram(ff.size, diagram)); + return ByRatio{FeeFrac{ff.fee, 1}} <=> ByRatio{EvaluateDiagram(ff.size, diagram)}; } std::partial_ordering CompareDiagrams(std::span dia1, std::span dia2) @@ -126,7 +126,7 @@ FUZZ_TARGET(build_and_compare_feerate_diagram) int32_t size = fuzzed_data_provider.ConsumeIntegralInRange(0, diagram2.back().size); auto eval1 = EvaluateDiagram(size, diagram1); auto eval2 = EvaluateDiagram(size, diagram2); - auto cmp = FeeRateCompare(eval1, eval2); + auto cmp = ByRatio{eval1} <=> ByRatio{eval2}; if (std::is_lt(cmp)) assert(!std::is_gt(real)); if (std::is_gt(cmp)) assert(!std::is_lt(real)); } diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 023fe7a42b1d..bfa74bfb74b4 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -210,12 +210,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) FeeFrac first_sum; for (size_t i = 0; i < calc_results->first.size(); ++i) { first_sum += calc_results->first[i]; - if (i) assert(!(calc_results->first[i - 1] << calc_results->first[i])); + if (i) assert(ByRatio{calc_results->first[i - 1]} >= ByRatio{calc_results->first[i]}); } FeeFrac second_sum; for (size_t i = 0; i < calc_results->second.size(); ++i) { second_sum += calc_results->second[i]; - if (i) assert(!(calc_results->second[i - 1] << calc_results->second[i])); + if (i) assert(ByRatio{calc_results->second[i - 1]} >= ByRatio{calc_results->second[i]}); } FeeFrac replaced; diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index 0fc4e047814e..e3eb0dd18fe0 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -433,7 +433,7 @@ FUZZ_TARGET(txgraph) assert(num_tx == sim.GetTransactionCount()); // Sort by feerate only, since violating topological constraints within same-feerate // chunks won't affect diagram comparisons. - std::sort(chunk_feerates.begin(), chunk_feerates.end(), std::greater{}); + std::sort(chunk_feerates.begin(), chunk_feerates.end(), std::greater>{}); return chunk_feerates; }; @@ -806,10 +806,10 @@ FUZZ_TARGET(txgraph) assert(sim_gain == real_gain); // Check that the feerates in each diagram are monotonically decreasing. for (size_t i = 1; i < real_main_diagram.size(); ++i) { - assert(FeeRateCompare(real_main_diagram[i], real_main_diagram[i - 1]) <= 0); + assert(ByRatio{real_main_diagram[i]} <= ByRatio{real_main_diagram[i - 1]}); } for (size_t i = 1; i < real_staged_diagram.size(); ++i) { - assert(FeeRateCompare(real_staged_diagram[i], real_staged_diagram[i - 1]) <= 0); + assert(ByRatio{real_staged_diagram[i]} <= ByRatio{real_staged_diagram[i - 1]}); } break; } else if (block_builders.size() < 4 && !main_sim.IsOversized() && command-- == 0) { @@ -829,7 +829,7 @@ FUZZ_TARGET(txgraph) if (chunk) { // Chunk feerates must be monotonously decreasing. if (!builder_data.last_feerate.IsEmpty()) { - assert(!(chunk->second >> builder_data.last_feerate)); + assert(ByRatio{chunk->second} <= ByRatio{builder_data.last_feerate}); } builder_data.last_feerate = chunk->second; // Verify the contents of GetCurrentChunk. @@ -1118,7 +1118,7 @@ FUZZ_TARGET(txgraph) std::pair max_chunk_tiebreak{0, 0}; for (const auto& chunk : real_chunking) { // If this is the first chunk with a strictly lower feerate, reset. - if (chunk.feerate << last_chunk_feerate) { + if (ByRatio{chunk.feerate} < ByRatio{last_chunk_feerate}) { comp_prefix_sizes.clear(); max_chunk_tiebreak = {0, 0}; } @@ -1212,12 +1212,12 @@ FUZZ_TARGET(txgraph) if (pos > 0) { size_t before = rng.randrange(pos); auto before_feerate = real->GetMainChunkFeerate(*sims[0].GetRef(vec1[before])); - assert(FeeRateCompare(before_feerate, pos_feerate) >= 0); + assert(ByRatio{before_feerate} >= ByRatio{pos_feerate}); } if (pos + 1 < vec1.size()) { size_t after = pos + 1 + rng.randrange(vec1.size() - 1 - pos); auto after_feerate = real->GetMainChunkFeerate(*sims[0].GetRef(vec1[after])); - assert(FeeRateCompare(after_feerate, pos_feerate) <= 0); + assert(ByRatio{after_feerate} <= ByRatio{pos_feerate}); } } @@ -1265,17 +1265,17 @@ FUZZ_TARGET(txgraph) auto [main_cmp_diagram, stage_cmp_diagram] = real->GetMainStagingDiagrams(); // Check that the feerates in each diagram are monotonically decreasing. for (size_t i = 1; i < main_cmp_diagram.size(); ++i) { - assert(FeeRateCompare(main_cmp_diagram[i], main_cmp_diagram[i - 1]) <= 0); + assert(ByRatio{main_cmp_diagram[i]} <= ByRatio{main_cmp_diagram[i - 1]}); } for (size_t i = 1; i < stage_cmp_diagram.size(); ++i) { - assert(FeeRateCompare(stage_cmp_diagram[i], stage_cmp_diagram[i - 1]) <= 0); + assert(ByRatio{stage_cmp_diagram[i]} <= ByRatio{stage_cmp_diagram[i - 1]}); } // Treat the diagrams as sets of chunk feerates, and sort them in the same way so that // std::set_difference can be used on them below. The exact ordering does not matter // here, but it has to be consistent with the one used in main_real_diagram and // stage_real_diagram). - std::sort(main_cmp_diagram.begin(), main_cmp_diagram.end(), std::greater{}); - std::sort(stage_cmp_diagram.begin(), stage_cmp_diagram.end(), std::greater{}); + std::sort(main_cmp_diagram.begin(), main_cmp_diagram.end(), std::greater>{}); + std::sort(stage_cmp_diagram.begin(), stage_cmp_diagram.end(), std::greater>{}); // Find the chunks that appear in main_diagram but are missing from main_cmp_diagram. // This is allowed, because GetMainStagingDiagrams omits clusters in main unaffected // by staging. @@ -1283,7 +1283,7 @@ FUZZ_TARGET(txgraph) std::set_difference(main_real_diagram.begin(), main_real_diagram.end(), main_cmp_diagram.begin(), main_cmp_diagram.end(), std::inserter(missing_main_cmp, missing_main_cmp.end()), - std::greater{}); + std::greater>{}); assert(main_cmp_diagram.size() + missing_main_cmp.size() == main_real_diagram.size()); // Do the same for chunks in stage_diagram missing from stage_cmp_diagram. auto stage_real_diagram = get_diagram_fn(TxGraph::Level::TOP); @@ -1291,7 +1291,7 @@ FUZZ_TARGET(txgraph) std::set_difference(stage_real_diagram.begin(), stage_real_diagram.end(), stage_cmp_diagram.begin(), stage_cmp_diagram.end(), std::inserter(missing_stage_cmp, missing_stage_cmp.end()), - std::greater{}); + std::greater>{}); assert(stage_cmp_diagram.size() + missing_stage_cmp.size() == stage_real_diagram.size()); // The missing chunks must be equal across main & staging (otherwise they couldn't have // been omitted). diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index f6720ffb897a..9ba6c3a4fe9c 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -554,7 +554,7 @@ FUZZ_TARGET(txorphanage_sim) count += 1 + (txn[ann.tx]->vin.size() / 10); usage += GetTransactionWeight(*txn[ann.tx]); } - return std::max(FeeFrac{count, max_count}, FeeFrac{usage, max_usage}); + return std::max>(FeeFrac{count, max_count}, FeeFrac{usage, max_usage}); }; // @@ -706,13 +706,13 @@ FUZZ_TARGET(txorphanage_sim) auto dos_score = dos_score_fn(peer, max_ann, max_mem); // Use >= so that the more recent peer (higher NodeId) wins in case of // ties. - if (dos_score >= worst_dos_score) { + if (ByRatioNegSize{dos_score} >= ByRatioNegSize{worst_dos_score}) { worst_dos_score = dos_score; worst_peer = peer; } } assert(worst_peer != unsigned(-1)); - assert(worst_dos_score >> FeeFrac(1, 1)); + assert(ByRatio{worst_dos_score} > ByRatio{FeeFrac(1, 1)}); // Find oldest announcement from worst_peer, preferring non-reconsiderable ones. bool done{false}; for (int reconsider = 0; reconsider < 2; ++reconsider) { diff --git a/src/txgraph.cpp b/src/txgraph.cpp index ca12bca6f498..8993947a9fe7 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -496,9 +496,8 @@ class TxGraphImpl final : public TxGraph const auto& entry_a = m_entries[a]; const auto& entry_b = m_entries[b]; // Compare chunk feerates, and return result if it differs. - auto feerate_cmp = FeeRateCompare(entry_b.m_main_chunk_feerate, entry_a.m_main_chunk_feerate); - if (feerate_cmp < 0) return std::strong_ordering::less; - if (feerate_cmp > 0) return std::strong_ordering::greater; + auto feerate_cmp = ByRatio{entry_b.m_main_chunk_feerate} <=> ByRatio{entry_a.m_main_chunk_feerate}; + if (feerate_cmp != 0) return feerate_cmp; // Compare equal-feerate chunk prefix size for comparing equal chunk feerates. This does two // things: it distinguishes equal-feerate chunks within the same cluster (because later // ones will always have a higher prefix size), and it may distinguish equal-feerate chunks @@ -1100,7 +1099,7 @@ void GenericClusterImpl::Updated(TxGraphImpl& graph, int level, bool rename) noe Assume(chunk_count > 0); // Update equal_feerate_chunk_feerate to include this chunk, starting over when the // feerate changed. - if (chunk.feerate << equal_feerate_chunk_feerate) { + if (ByRatio{chunk.feerate} < ByRatio{equal_feerate_chunk_feerate}) { equal_feerate_chunk_feerate = chunk.feerate; } else { // Note that this is adding fees to fees, and sizes to sizes, so the overall @@ -2828,8 +2827,8 @@ std::pair, std::vector> TxGraphImpl::GetMainStagin } } // Sort both by decreasing feerate to obtain diagrams, and return them. - std::sort(main_feerates.begin(), main_feerates.end(), [](auto& a, auto& b) { return a > b; }); - std::sort(staging_feerates.begin(), staging_feerates.end(), [](auto& a, auto& b) { return a > b; }); + std::sort(main_feerates.begin(), main_feerates.end(), std::greater>{}); + std::sort(staging_feerates.begin(), staging_feerates.end(), std::greater>{}); return std::make_pair(std::move(main_feerates), std::move(staging_feerates)); } @@ -2875,10 +2874,10 @@ void GenericClusterImpl::SanityCheck(const TxGraphImpl& graph, int level) const ++chunk_num; assert(chunk_num < linchunking.size()); chunk_pos = 0; - if (linchunking[chunk_num].feerate << equal_feerate_prefix) { + if (ByRatio{linchunking[chunk_num].feerate} < ByRatio{equal_feerate_prefix}) { equal_feerate_prefix = linchunking[chunk_num].feerate; } else { - assert(!(linchunking[chunk_num].feerate >> equal_feerate_prefix)); + assert(ByRatio{linchunking[chunk_num].feerate} == ByRatio{equal_feerate_prefix}); equal_feerate_prefix += linchunking[chunk_num].feerate; } } @@ -3103,7 +3102,7 @@ void TxGraphImpl::SanityCheck() const actual_chunkindex.insert(idx); auto chunk_feerate = m_entries[idx].m_main_chunk_feerate; if (!last_chunk_feerate.IsEmpty()) { - assert(FeeRateCompare(last_chunk_feerate, chunk_feerate) >= 0); + assert(ByRatio{last_chunk_feerate} >= ByRatio{FeeFrac{chunk_feerate}}); } last_chunk_feerate = chunk_feerate; } @@ -3338,7 +3337,7 @@ std::vector TxGraphImpl::Trim() noexcept // We do not need to sort by cluster or within clusters, because due to the implicit // dependency between consecutive linearization elements, no two transactions from the // same Cluster will ever simultaneously be in the heap. - return a->m_chunk_feerate < b->m_chunk_feerate; + return ByRatioNegSize{a->m_chunk_feerate} < ByRatioNegSize{b->m_chunk_feerate}; }; /** Given a TrimTxData entry, find the representative of the partition it is in. */ diff --git a/src/util/feefrac.cpp b/src/util/feefrac.cpp index 68ba2b6665a8..6982626accb6 100644 --- a/src/util/feefrac.cpp +++ b/src/util/feefrac.cpp @@ -42,17 +42,17 @@ std::partial_ordering CompareChunks(std::span chunks0, std::span< const auto slope_ap = point_p - point_a; Assume(slope_ap.size > 0); - std::weak_ordering cmp = std::weak_ordering::equivalent; + auto cmp = std::strong_ordering::equivalent; if (done_0 || done_1) { // If a single side has no points left, act as if AB has slope tail_feerate(of 0). Assume(!(done_0 && done_1)); - cmp = FeeRateCompare(slope_ap, FeeFrac(0, 1)); + cmp = ByRatio{slope_ap} <=> ByRatio{FeeFrac(0, 1)}; } else { // If both sides have points left, compute B, and the slope of AB explicitly. const FeeFrac& point_b = next_point(!unproc_side); const auto slope_ab = point_b - point_a; Assume(slope_ab.size >= slope_ap.size); - cmp = FeeRateCompare(slope_ap, slope_ab); + cmp = ByRatio{slope_ap} <=> ByRatio{slope_ab}; // If B and P have the same size, B can be marked as processed (in addition to P, see // below), as we've already performed a comparison at this size. diff --git a/src/util/feefrac.h b/src/util/feefrac.h index 7577107e8c27..7dac2f1366bb 100644 --- a/src/util/feefrac.h +++ b/src/util/feefrac.h @@ -12,29 +12,9 @@ #include #include -/** Data structure storing a fee and size, ordered by increasing fee/size. +/** Data structure storing a fee and size. * * The size of a FeeFrac cannot be zero unless the fee is also zero. - * - * FeeFracs have a total ordering, first by increasing feerate (ratio of fee over size), and then - * by decreasing size. The empty FeeFrac (fee and size both 0) sorts last. So for example, the - * following FeeFracs are in sorted order: - * - * - fee=0 size=1 (feerate 0) - * - fee=1 size=2 (feerate 0.5) - * - fee=2 size=3 (feerate 0.667...) - * - fee=2 size=2 (feerate 1) - * - fee=1 size=1 (feerate 1) - * - fee=3 size=2 (feerate 1.5) - * - fee=2 size=1 (feerate 2) - * - fee=0 size=0 (undefined feerate) - * - * A FeeFrac is considered "better" if it sorts after another, by this ordering. All standard - * comparison operators (<=>, ==, !=, >, <, >=, <=) respect this ordering. - * - * The FeeRateCompare, and >> and << operators only compare feerate and treat equal feerate but - * different size as equivalent. The empty FeeFrac is neither lower or higher in feerate than any - * other. */ struct FeeFrac { @@ -153,35 +133,6 @@ struct FeeFrac return a.fee == b.fee && a.size == b.size; } - /** Compare two FeeFracs just by feerate. */ - friend inline std::weak_ordering FeeRateCompare(const FeeFrac& a, const FeeFrac& b) noexcept - { - auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size); - return cross_a <=> cross_b; - } - - /** Check if a FeeFrac object has strictly lower feerate than another. */ - friend inline bool operator<<(const FeeFrac& a, const FeeFrac& b) noexcept - { - auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size); - return cross_a < cross_b; - } - - /** Check if a FeeFrac object has strictly higher feerate than another. */ - friend inline bool operator>>(const FeeFrac& a, const FeeFrac& b) noexcept - { - auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size); - return cross_a > cross_b; - } - - /** Compare two FeeFracs. <, >, <=, and >= are auto-generated from this. */ - friend inline std::strong_ordering operator<=>(const FeeFrac& a, const FeeFrac& b) noexcept - { - auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size); - if (cross_a == cross_b) return b.size <=> a.size; - return cross_a <=> cross_b; - } - /** Swap two FeeFracs. */ friend inline void swap(FeeFrac& a, FeeFrac& b) noexcept { @@ -255,4 +206,107 @@ using FeePerVSize = FeePerUnit; struct WeightTag {}; using FeePerWeight = FeePerUnit; +/** Wrapper around FeeFrac & derived types, which adds a feerate-based ordering which treats + * equal-feerate but distinct-size FeeFracs as equals. + * + * This is not included inside FeeFrac itself, because it is not a total ordering (as would be + * expected for built-in comparison operators). + */ +template T> +class ByRatio +{ + const T& m_feefrac; + +public: + constexpr ByRatio(const T& feefrac) noexcept : m_feefrac{feefrac} {} + + friend bool operator==(const ByRatio& a, const ByRatio& b) noexcept + { + auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size); + auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size); + return cross_a == cross_b; + } + + // Note that we can use std::strong_ordering here, because even though FeeFrac{1,2} and + // FeeFrac{2,4} are distinct as FeeFracs, they are indistinguishable from ByRatio's perspective + // (operator== also treats them as equal). + friend std::strong_ordering operator<=>(const ByRatio& a, const ByRatio& b) noexcept + { + auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size); + auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size); + return cross_a <=> cross_b; + } + + // Specialized versions for efficiency. GCC 15+ and Clang 11+ produce operator<=>-derived + // versions that are equally efficient as this at -O2, but earlier versions do not. + friend bool operator<(const ByRatio& a, const ByRatio& b) noexcept + { + auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size); + auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size); + return cross_a < cross_b; + } + friend bool operator>(const ByRatio& a, const ByRatio& b) noexcept + { + auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size); + auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size); + return cross_a > cross_b; + } + friend bool operator<=(const ByRatio& a, const ByRatio& b) noexcept + { + auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size); + auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size); + return cross_a <= cross_b; + } + friend bool operator>=(const ByRatio& a, const ByRatio& b) noexcept + { + auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size); + auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size); + return cross_a >= cross_b; + } +}; + +/** Wrapper around FeeFrac & derived types, which adds a total ordering which first sorts by feerate + * and then by reversed size (i.e., larger sizes come first). + * + * This is not included inside FeeFrac itself, because it is not the most natural behavior, so it + * is better to make code using it invoke this explicitly. + * + * The empty FeeFrac (fee and size both 0) sorts last. So for example, the following FeeFracs are + * in sorted order: + * + * - fee=0 size=1 (feerate 0) + * - fee=1 size=2 (feerate 0.5) + * - fee=2 size=3 (feerate 0.667...) + * - fee=2 size=2 (feerate 1) + * - fee=1 size=1 (feerate 1) + * - fee=3 size=2 (feerate 1.5) + * - fee=2 size=1 (feerate 2) + * - fee=0 size=0 (undefined feerate) + */ +template T> +class ByRatioNegSize +{ + const T& m_feefrac; + +public: + constexpr ByRatioNegSize(const T& feefrac) noexcept : m_feefrac{feefrac} {} + + friend bool operator==(const ByRatioNegSize& a, const ByRatioNegSize& b) noexcept + { + return a.m_feefrac == b.m_feefrac; + } + + friend std::strong_ordering operator<=>(const ByRatioNegSize& a, const ByRatioNegSize& b) noexcept + { + auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size); + auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size); + auto cmp = cross_a <=> cross_b; + if (cmp != 0) return cmp; + return b.m_feefrac.size <=> a.m_feefrac.size; + } + + // Support conversion back to underlying FeeFrac, which allows using std::max(). + operator const T&() const noexcept { return m_feefrac; } +}; + #endif // BITCOIN_UTIL_FEEFRAC_H From 1aa78cdab6bc8ecd4448b1b75ee3181ee8f3f519 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 25 Feb 2026 13:11:01 -0500 Subject: [PATCH 015/502] clusterlin: adopt STL ranges algorithms (refactor) --- src/cluster_linearize.h | 9 ++++---- src/test/fuzz/txgraph.cpp | 15 +++++++------- src/txgraph.cpp | 43 ++++++++++++++++++++------------------- 3 files changed, 35 insertions(+), 32 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index 1bf3d475f10e..4da4703fe866 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -316,7 +317,7 @@ class DepGraph { DepGraphIndex old_len = list.size(); for (auto i : select) list.push_back(i); - std::sort(list.begin() + old_len, list.end(), [&](DepGraphIndex a, DepGraphIndex b) noexcept { + std::ranges::sort(std::span{list}.subspan(old_len), [&](DepGraphIndex a, DepGraphIndex b) noexcept { const auto a_anc_count = entries[a].ancestors.Count(); const auto b_anc_count = entries[b].ancestors.Count(); if (a_anc_count != b_anc_count) return a_anc_count < b_anc_count; @@ -1618,7 +1619,7 @@ class SpanningForestState for (auto chunk_idx : m_chunk_idxs) { ret.push_back(m_set_info[chunk_idx].feerate); } - std::sort(ret.begin(), ret.end(), std::greater>{}); + std::ranges::sort(ret, std::greater>{}); return ret; } @@ -1647,8 +1648,8 @@ class SpanningForestState } } } - std::sort(expected_dependencies.begin(), expected_dependencies.end()); - std::sort(all_dependencies.begin(), all_dependencies.end()); + std::ranges::sort(expected_dependencies); + std::ranges::sort(all_dependencies); assert(expected_dependencies == all_dependencies); // diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index e3eb0dd18fe0..88091fb00baa 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -433,7 +434,7 @@ FUZZ_TARGET(txgraph) assert(num_tx == sim.GetTransactionCount()); // Sort by feerate only, since violating topological constraints within same-feerate // chunks won't affect diagram comparisons. - std::sort(chunk_feerates.begin(), chunk_feerates.end(), std::greater>{}); + std::ranges::sort(chunk_feerates, std::greater>{}); return chunk_feerates; }; @@ -1006,7 +1007,7 @@ FUZZ_TARGET(txgraph) for (auto i : cluster) sizes.push_back(top_sim.graph.FeeRate(i).size); auto sum_sizes = std::accumulate(sizes.begin(), sizes.end(), uint64_t{0}); // Sort from large to small. - std::sort(sizes.begin(), sizes.end(), std::greater{}); + std::ranges::sort(sizes, std::greater{}); // In the worst case, only the smallest transactions are removed. while (sizes.size() > max_cluster_count || sum_sizes > max_cluster_size) { sum_sizes -= sizes.back(); @@ -1075,8 +1076,8 @@ FUZZ_TARGET(txgraph) auto cmp = [&](SimTxGraph::Pos a, SimTxGraph::Pos b) noexcept { return real->CompareMainOrder(*sims[0].GetRef(a), *sims[0].GetRef(b)) < 0; }; - std::sort(vec1.begin(), vec1.end(), cmp); - std::sort(vec2.begin(), vec2.end(), cmp); + std::ranges::sort(vec1, cmp); + std::ranges::sort(vec2, cmp); // Verify the resulting orderings are identical. This could only fail if the ordering was // not total. @@ -1199,7 +1200,7 @@ FUZZ_TARGET(txgraph) auto cmp_redo = [&](SimTxGraph::Pos a, SimTxGraph::Pos b) noexcept { return real_redo->CompareMainOrder(*txobjects_redo[a], *txobjects_redo[b]) < 0; }; - std::sort(vec_redo.begin(), vec_redo.end(), cmp_redo); + std::ranges::sort(vec_redo, cmp_redo); // Compare with the ordering we got from real. assert(vec1 == vec_redo); } @@ -1274,8 +1275,8 @@ FUZZ_TARGET(txgraph) // std::set_difference can be used on them below. The exact ordering does not matter // here, but it has to be consistent with the one used in main_real_diagram and // stage_real_diagram). - std::sort(main_cmp_diagram.begin(), main_cmp_diagram.end(), std::greater>{}); - std::sort(stage_cmp_diagram.begin(), stage_cmp_diagram.end(), std::greater>{}); + std::ranges::sort(main_cmp_diagram, std::greater>{}); + std::ranges::sort(stage_cmp_diagram, std::greater>{}); // Find the chunks that appear in main_diagram but are missing from main_cmp_diagram. // This is allowed, because GetMainStagingDiagrams omits clusters in main unaffected // by staging. diff --git a/src/txgraph.cpp b/src/txgraph.cpp index 8993947a9fe7..39a8a8814abe 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -1212,8 +1213,8 @@ std::vector TxGraphImpl::GetConflicts() const noexcept } } // Deduplicate the result (the same Cluster may appear multiple times). - std::sort(ret.begin(), ret.end(), [](Cluster* a, Cluster* b) noexcept { return CompareClusters(a, b) < 0; }); - ret.erase(std::unique(ret.begin(), ret.end()), ret.end()); + std::ranges::sort(ret, [](Cluster* a, Cluster* b) noexcept { return CompareClusters(a, b) < 0; }); + ret.erase(std::ranges::unique(ret).begin(), ret.end()); return ret; } @@ -1558,7 +1559,7 @@ void SingletonClusterImpl::Merge(TxGraphImpl&, int, Cluster&) noexcept void GenericClusterImpl::ApplyDependencies(TxGraphImpl& graph, int level, std::span> to_apply) noexcept { // Sort the list of dependencies to apply by child, so those can be applied in batch. - std::sort(to_apply.begin(), to_apply.end(), [](auto& a, auto& b) { return a.second < b.second; }); + std::ranges::sort(to_apply, [](auto& a, auto& b) { return a.second < b.second; }); // Iterate over groups of to-be-added dependencies with the same child. auto it = to_apply.begin(); while (it != to_apply.end()) { @@ -1720,7 +1721,7 @@ void TxGraphImpl::ApplyRemovals(int up_to_level) noexcept } } // Group the set of to-be-removed entries by Cluster::m_sequence. - std::sort(to_remove.begin(), to_remove.end(), [&](GraphIndex a, GraphIndex b) noexcept { + std::ranges::sort(to_remove, [&](GraphIndex a, GraphIndex b) noexcept { Cluster* cluster_a = m_entries[a].m_locator[level].cluster; Cluster* cluster_b = m_entries[b].m_locator[level].cluster; return CompareClusters(cluster_a, cluster_b) < 0; @@ -1791,7 +1792,7 @@ void TxGraphImpl::Compact() noexcept // ones get processed first. This means earlier-processed GraphIndexes will not cause moving of // later-processed ones during the "swap with end of m_entries" step below (which might // invalidate them). - std::sort(m_unlinked.begin(), m_unlinked.end(), std::greater{}); + std::ranges::sort(m_unlinked, std::greater{}); std::vector affected_main; auto last = GraphIndex(-1); @@ -1817,7 +1818,7 @@ void TxGraphImpl::Compact() noexcept // Update the affected clusters, to fixup Entry::m_main_max_chunk_fallback values which may // have become outdated due to the compaction above. - std::sort(affected_main.begin(), affected_main.end()); + std::ranges::sort(affected_main); affected_main.erase(std::unique(affected_main.begin(), affected_main.end()), affected_main.end()); for (Cluster* cluster : affected_main) { cluster->Updated(*this, /*level=*/0, /*rename=*/true); @@ -1901,10 +1902,10 @@ void TxGraphImpl::GroupClusters(int level) noexcept } // Sort and deduplicate an_clusters, so we end up with a sorted list of all involved Clusters // to which dependencies apply, or which are oversized. - std::sort(an_clusters.begin(), an_clusters.end(), [](auto& a, auto& b) noexcept { return a.second < b.second; }); - an_clusters.erase(std::unique(an_clusters.begin(), an_clusters.end()), an_clusters.end()); + std::ranges::sort(an_clusters, [](auto& a, auto& b) noexcept { return a.second < b.second; }); + an_clusters.erase(std::ranges::unique(an_clusters).begin(), an_clusters.end()); // Sort an_deps by applying the same order to the involved child cluster. - std::sort(an_deps.begin(), an_deps.end(), [&](auto& a, auto& b) noexcept { return a.second < b.second; }); + std::ranges::sort(an_deps, [&](auto& a, auto& b) noexcept { return a.second < b.second; }); // Run the union-find algorithm to find partitions of the input Clusters which need to be // grouped together. See https://en.wikipedia.org/wiki/Disjoint-set_data_structure. @@ -2017,8 +2018,8 @@ void TxGraphImpl::GroupClusters(int level) noexcept // Sort both an_clusters and an_deps by sequence number of the representative of the // partition they are in, grouping all those applying to the same partition together. - std::sort(an_deps.begin(), an_deps.end(), [](auto& a, auto& b) noexcept { return a.second < b.second; }); - std::sort(an_clusters.begin(), an_clusters.end(), [](auto& a, auto& b) noexcept { return a.second < b.second; }); + std::ranges::sort(an_deps, [](auto& a, auto& b) noexcept { return a.second < b.second; }); + std::ranges::sort(an_clusters, [](auto& a, auto& b) noexcept { return a.second < b.second; }); // Translate the resulting cluster groups to the m_group_data structure, and the dependencies // back to m_deps_to_add. @@ -2489,7 +2490,7 @@ std::vector TxGraphImpl::GetAncestorsUnion(std::span ret; @@ -2522,7 +2523,7 @@ std::vector TxGraphImpl::GetDescendantsUnion(std::span ret; @@ -2796,7 +2797,7 @@ TxGraph::GraphIndex TxGraphImpl::CountDistinctClusters(std::span, std::vector> TxGraphImpl::GetMainStagin } } // Sort both by decreasing feerate to obtain diagrams, and return them. - std::sort(main_feerates.begin(), main_feerates.end(), std::greater>{}); - std::sort(staging_feerates.begin(), staging_feerates.end(), std::greater>{}); + std::ranges::sort(main_feerates, std::greater>{}); + std::ranges::sort(staging_feerates, std::greater>{}); return std::make_pair(std::move(main_feerates), std::move(staging_feerates)); } @@ -3401,7 +3402,7 @@ std::vector TxGraphImpl::Trim() noexcept // Sort the trim data by GraphIndex. In what follows, we will treat this sorted vector as // a map from GraphIndex to TrimTxData via locate_fn, and its ordering will not change // anymore. - std::sort(trim_data.begin(), trim_data.end(), [](auto& a, auto& b) noexcept { return a.m_index < b.m_index; }); + std::ranges::sort(trim_data, [](auto& a, auto& b) noexcept { return a.m_index < b.m_index; }); // Add the explicitly added dependencies to deps_by_child. deps_by_child.insert(deps_by_child.end(), @@ -3410,7 +3411,7 @@ std::vector TxGraphImpl::Trim() noexcept // Sort deps_by_child by child transaction GraphIndex. The order will not be changed // anymore after this. - std::sort(deps_by_child.begin(), deps_by_child.end(), [](auto& a, auto& b) noexcept { return a.second < b.second; }); + std::ranges::sort(deps_by_child, [](auto& a, auto& b) noexcept { return a.second < b.second; }); // Fill m_parents_count and m_parents_offset in trim_data, as well as m_deps_left, and // initially populate trim_heap. Because of the sort above, all dependencies involving the // same child are grouped together, so a single linear scan suffices. @@ -3434,7 +3435,7 @@ std::vector TxGraphImpl::Trim() noexcept // Construct deps_by_parent, sorted by parent transaction GraphIndex. The order will not be // changed anymore after this. deps_by_parent = deps_by_child; - std::sort(deps_by_parent.begin(), deps_by_parent.end(), [](auto& a, auto& b) noexcept { return a.first < b.first; }); + std::ranges::sort(deps_by_parent, [](auto& a, auto& b) noexcept { return a.first < b.first; }); // Fill m_children_offset and m_children_count in trim_data. Because of the sort above, all // dependencies involving the same parent are grouped together, so a single linear scan // suffices. @@ -3482,8 +3483,8 @@ std::vector TxGraphImpl::Trim() noexcept Assume(chl == entry.m_index); current_deps.push_back(find_fn(&*locate_fn(par))); } - std::sort(current_deps.begin(), current_deps.end()); - current_deps.erase(std::unique(current_deps.begin(), current_deps.end()), current_deps.end()); + std::ranges::sort(current_deps); + current_deps.erase(std::ranges::unique(current_deps).begin(), current_deps.end()); // Compute resource counts. uint32_t new_count = 1; From 55d37546faa6b928d361e8b7ce9a90b49057d09a Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Fri, 6 Mar 2026 23:40:27 +0000 Subject: [PATCH 016/502] Bugfix: GUI/Intro: Handle errors from SelectParams the same as if during InitConfig Without this, invalid vbparams just silently exit with no message --- src/qt/intro.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp index f86b16707645..f966d2f8883d 100644 --- a/src/qt/intro.cpp +++ b/src/qt/intro.cpp @@ -18,7 +18,9 @@ #include #include +#include #include +#include #include #include @@ -26,6 +28,7 @@ #include #include +#include /* Check free space asynchronously to prevent hanging the UI thread. @@ -221,8 +224,10 @@ bool Intro::showIfNeeded(bool& did_show_intro, int64_t& prune_MiB) /* Use selectParams here to guarantee Params() can be used by node interface */ try { SelectParams(gArgs.GetChainType()); - } catch (const std::exception&) { - return false; + } catch (const std::exception& e) { + InitError(Untranslated(e.what())); + QMessageBox::critical(nullptr, CLIENT_NAME, QObject::tr("Error: %1").arg(QString(e.what()))); + std::exit(EXIT_FAILURE); } /* If current default data directory does not exist, let the user choose one */ From 25e063d950a1a77045f330d7a68e646612d228ab Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 27 Nov 2025 14:34:41 +1000 Subject: [PATCH 017/502] wallet: Add separate balance info for non-mempool wallet txs --- src/interfaces/wallet.h | 3 ++- src/wallet/interfaces.cpp | 1 + src/wallet/receive.cpp | 17 +++++++++++++++-- src/wallet/receive.h | 3 ++- src/wallet/rpc/coins.cpp | 4 +++- src/wallet/wallet.cpp | 23 +++++++++++++++++++++++ src/wallet/wallet.h | 7 +++++++ test/functional/wallet_abandonconflict.py | 5 ++--- test/functional/wallet_balance.py | 6 ++++-- test/functional/wallet_conflicts.py | 5 +++-- test/functional/wallet_migration.py | 15 +++++++++++---- test/functional/wallet_orphanedreward.py | 1 + test/functional/wallet_v3_txs.py | 18 +++++++++--------- 13 files changed, 83 insertions(+), 25 deletions(-) diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 118f9a2e96de..33264805ea12 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -370,12 +370,13 @@ struct WalletBalances CAmount unconfirmed_balance = 0; CAmount immature_balance = 0; CAmount used_balance = 0; + CAmount nonmempool_balance = 0; bool balanceChanged(const WalletBalances& prev) const { return balance != prev.balance || unconfirmed_balance != prev.unconfirmed_balance || immature_balance != prev.immature_balance || - used_balance != prev.used_balance; + used_balance != prev.used_balance || nonmempool_balance != prev.nonmempool_balance; } }; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 7fa0d8a13b9c..055e1840c8c4 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -389,6 +389,7 @@ class WalletImpl : public Wallet result.unconfirmed_balance = bal.m_mine_untrusted_pending; result.immature_balance = bal.m_mine_immature; result.used_balance = bal.m_mine_used; + result.nonmempool_balance = bal.m_mine_nonmempool; return result; } bool tryGetBalances(WalletBalances& balances, uint256& block_hash) override diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 8832ddb66cef..44fea391bb90 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -242,7 +242,7 @@ bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx) return CachedTxIsTrusted(wallet, wtx, trusted_parents); } -Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) +Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse, bool include_nonmempool) { Balance ret; bool allow_used_addresses = !avoid_reuse || !wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); @@ -255,7 +255,17 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)}; const int tx_depth{wallet.GetTxDepthInMainChain(wtx)}; - if (!wallet.IsSpent(outpoint)) { + bool nonmempool_spent = false; + switch (wallet.HowSpent(outpoint)) { + case CWallet::SpendType::CONFIRMED: + case CWallet::SpendType::MEMPOOL: + // treat as spent; ignore + break; + case CWallet::SpendType::NONMEMPOOL: + if (!include_nonmempool) break; + nonmempool_spent = true; + [[fallthrough]]; + case CWallet::SpendType::UNSPENT: CAmount* bucket = nullptr; // Set the amounts in the return object @@ -274,6 +284,9 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) bucket = &ret.m_mine_used; } *bucket += credit_mine; + if (nonmempool_spent) { + ret.m_mine_nonmempool -= credit_mine; + } } } } diff --git a/src/wallet/receive.h b/src/wallet/receive.h index 5bc0545bebe2..14f0a9bdf27b 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -48,8 +48,9 @@ struct Balance { CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending) CAmount m_mine_immature{0}; //!< Immature coinbases in the main chain CAmount m_mine_used{0}; //!< Trusted/untrusted/immature funds in utxos that have already been spent from (only populated if AVOID REUSE wallet flag is set) + CAmount m_mine_nonmempool{0}; //!< Coins spent by wallet txs that are not in the mempool }; -Balance GetBalance(const CWallet& wallet, int min_depth = 0, bool avoid_reuse = true); +Balance GetBalance(const CWallet& wallet, int min_depth = 0, bool avoid_reuse = true, bool include_nonmempool = false); std::map GetAddressBalances(const CWallet& wallet); std::set> GetAddressGroupings(const CWallet& wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index c76d5e3c7fc0..63e400bfc6d9 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -413,6 +413,7 @@ RPCHelpMan getbalances() {RPCResult::Type::STR_AMOUNT, "trusted", "trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", "untrusted pending balance (outputs created by others that are in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "immature", "balance from immature coinbase outputs"}, + {RPCResult::Type::STR_AMOUNT, "nonmempool", "sum of coins that are spent by transactions not in the mempool (usually an over-estimate due to not accounting for change or spends that conflict with each other)"}, {RPCResult::Type::STR_AMOUNT, "used", /*optional=*/true, "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, }}, RESULT_LAST_PROCESSED_BLOCK, @@ -433,7 +434,7 @@ RPCHelpMan getbalances() LOCK(wallet.cs_wallet); - const auto bal = GetBalance(wallet, /*min_depth=*/0, /*avoid_reuse=*/true); + const auto bal = GetBalance(wallet, /*min_depth=*/0, /*avoid_reuse=*/true, /*include_nonmempool=*/true); UniValue balances{UniValue::VOBJ}; { @@ -441,6 +442,7 @@ RPCHelpMan getbalances() balances_mine.pushKV("trusted", ValueFromAmount(bal.m_mine_trusted)); balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending)); balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature)); + balances_mine.pushKV("nonmempool", ValueFromAmount(bal.m_mine_nonmempool)); if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { balances_mine.pushKV("used", ValueFromAmount(bal.m_mine_used)); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d08d6782c1be..37e9ef13ca20 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -738,6 +738,29 @@ bool CWallet::IsSpent(const COutPoint& outpoint) const return false; } +CWallet::SpendType CWallet::HowSpent(const COutPoint& outpoint) const +{ + SpendType st{SpendType::UNSPENT}; + + std::pair range; + range = mapTxSpends.equal_range(outpoint); + + for (TxSpends::const_iterator it = range.first; it != range.second; ++it) { + const Txid& txid = it->second; + const auto mit = mapWallet.find(txid); + if (mit != mapWallet.end()) { + const auto& wtx = mit->second; + if (wtx.isConfirmed()) return SpendType::CONFIRMED; + if (wtx.InMempool()) { + st = SpendType::MEMPOOL; + } else if (!wtx.isAbandoned() && !wtx.isBlockConflicted() && !wtx.isMempoolConflicted()) { + if (st == SpendType::UNSPENT) st = SpendType::NONMEMPOOL; + } + } + } + return st; +} + void CWallet::AddToSpends(const COutPoint& outpoint, const Txid& txid) { mapTxSpends.insert(std::make_pair(outpoint, txid)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b341ac6da2d4..e114f01f2e43 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -556,6 +556,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati int GetTxBlocksToMaturity(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsTxImmatureCoinBase(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + enum class SpendType { + UNSPENT, + CONFIRMED, + MEMPOOL, + NONMEMPOOL, + }; + SpendType HowSpent(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsSpent(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); // Whether this or any known scriptPubKey with the same single key has been spent. diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index f5650b87f01e..2aa79c95bf7c 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -115,10 +115,9 @@ def run_test(self): # inputs are still spent, but change not received newbalance = alice.getbalance() assert_equal(newbalance, balance - signed3_change) - # Unconfirmed received funds that are not in mempool, also shouldn't show - # up in unconfirmed balance + # Unconfirmed received funds that are not in mempool balances = alice.getbalances()['mine'] - assert_equal(balances['untrusted_pending'] + balances['trusted'], newbalance) + assert_equal(balances['untrusted_pending'] + balances['trusted'] + balances['nonmempool'], newbalance) # Also shouldn't show up in listunspent assert not txABC2 in [utxo["txid"] for utxo in alice.listunspent(0)] balance = newbalance diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 8c83f42ecfd8..62f900e13483 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -145,10 +145,12 @@ def test_balances(*, fee_node_1=0): # getbalances expected_balances_0 = {'mine': {'immature': Decimal('0E-8'), 'trusted': Decimal('9.99'), # change from node 0's send - 'untrusted_pending': Decimal('60.0')}} + 'untrusted_pending': Decimal('60.0'), + 'nonmempool': Decimal('0.0')}} expected_balances_1 = {'mine': {'immature': Decimal('0E-8'), 'trusted': Decimal('0E-8'), # node 1's send had an unsafe input - 'untrusted_pending': Decimal('30.0') - fee_node_1}} # Doesn't include output of node 0's send since it was spent + 'untrusted_pending': Decimal('30.0') - fee_node_1, # Doesn't include output of node 0's send since it was spent + 'nonmempool': Decimal('0.0')}} balances_0 = self.nodes[0].getbalances() balances_1 = self.nodes[1].getbalances() # remove lastprocessedblock keys (they will be tested later) diff --git a/test/functional/wallet_conflicts.py b/test/functional/wallet_conflicts.py index b16a2f83d2ac..a6562be9360f 100755 --- a/test/functional/wallet_conflicts.py +++ b/test/functional/wallet_conflicts.py @@ -304,8 +304,9 @@ def test_mempool_and_block_conflicts(self): bob.sendrawtransaction(tx1_conflict_conflict) # kick tx1_conflict out of the mempool bob.sendrawtransaction(raw_tx1) #re-broadcast tx1 because it is no longer conflicted - # Now bob has no pending funds because tx1 and tx2 are spent by tx3, which hasn't been re-broadcast yet - assert_equal(bob.getbalances()["mine"]["untrusted_pending"], 0) + # Now bob has pending funds because tx1 and tx2 are spent by tx3, which hasn't been re-broadcast yet + bob_bal = bob.getbalances()["mine"] + assert_equal(bob_bal["untrusted_pending"], -bob_bal["nonmempool"]) bob.sendrawtransaction(raw_tx3) assert_equal(len(bob.getrawmempool()), 4) # The mempool contains: tx1, tx2, tx1_conflict_conflict, tx3 diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 89989fbf81b0..a55f4943a0a6 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -8,6 +8,7 @@ import shutil import struct import time +from decimal import Decimal from test_framework.address import ( key_to_p2pkh, @@ -522,6 +523,7 @@ def test_pk_coinbases(self): self.generatetodescriptor(self.master_node, 1, desc) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') _, wallet = self.migrate_and_get_rpc("pkcb") @@ -537,6 +539,7 @@ def test_encrypted(self): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') # Use self.migrate_and_get_rpc to test this error to get everything copied over to the master node assert_raises_rpc_error(-4, "Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect", self.migrate_and_get_rpc, "encrypted") @@ -577,6 +580,7 @@ def test_unloaded_by_path(self): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') wallet.unloadwallet() @@ -616,6 +620,7 @@ def test_wallet_with_relative_path(self): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') migrate_res, wallet = self.migrate_and_get_rpc(relative_name) @@ -636,6 +641,7 @@ def test_wallet_with_relative_path(self): self.old_node.restorewallet("relative_restored", migrate_res['backup_path']) wallet = self.old_node.get_wallet_rpc("relative_restored") assert wallet.gettransaction(txid) + del bals["mine"]["nonmempool"] assert_equal(bals, wallet.getbalances()) info = wallet.getwalletinfo() @@ -652,6 +658,7 @@ def test_wallet_with_path(self, wallet_path): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') _, wallet = self.migrate_and_get_rpc(wallet_path) @@ -1423,14 +1430,14 @@ def test_miniscript(self): _, wallet = self.migrate_and_get_rpc("miniscript") # The miniscript with all keys should be in the migrated wallet - assert_equal(wallet.getbalances()["mine"], {"trusted": 0.75, "untrusted_pending": 0, "immature": 0}) + assert_equal(wallet.getbalances()["mine"], {"trusted": 0.75, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(wallet.getaddressinfo(all_keys_addr)["ismine"], True) assert_equal(wallet.getaddressinfo(some_keys_addr)["ismine"], False) # The miniscript with some keys should be in the watchonly wallet assert "miniscript_watchonly" in self.master_node.listwallets() watchonly = self.master_node.get_wallet_rpc("miniscript_watchonly") - assert_equal(watchonly.getbalances()["mine"], {"trusted": 1, "untrusted_pending": 0, "immature": 0}) + assert_equal(watchonly.getbalances()["mine"], {"trusted": 1, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(watchonly.getaddressinfo(some_keys_addr)["ismine"], True) assert_equal(watchonly.getaddressinfo(all_keys_addr)["ismine"], False) @@ -1479,7 +1486,7 @@ def test_taproot(self): res, wallet = self.migrate_and_get_rpc("taproot") # The rawtr should be migrated - assert_equal(wallet.getbalances()["mine"], {"trusted": 0.5, "untrusted_pending": 0, "immature": 0}) + assert_equal(wallet.getbalances()["mine"], {"trusted": 0.5, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(wallet.getaddressinfo(rawtr_addr)["ismine"], True) assert_equal(wallet.getaddressinfo(tr_addr)["ismine"], False) assert_equal(wallet.getaddressinfo(tr_script_addr)["ismine"], False) @@ -1487,7 +1494,7 @@ def test_taproot(self): # The tr() with some keys should be in the watchonly wallet assert "taproot_watchonly" in self.master_node.listwallets() watchonly = self.master_node.get_wallet_rpc("taproot_watchonly") - assert_equal(watchonly.getbalances()["mine"], {"trusted": 5, "untrusted_pending": 0, "immature": 0}) + assert_equal(watchonly.getbalances()["mine"], {"trusted": 5, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(watchonly.getaddressinfo(rawtr_addr)["ismine"], False) assert_equal(watchonly.getaddressinfo(tr_addr)["ismine"], True) assert_equal(watchonly.getaddressinfo(tr_script_addr)["ismine"], True) diff --git a/test/functional/wallet_orphanedreward.py b/test/functional/wallet_orphanedreward.py index f13b5a8c1b8a..bd02010fa7cc 100755 --- a/test/functional/wallet_orphanedreward.py +++ b/test/functional/wallet_orphanedreward.py @@ -46,6 +46,7 @@ def run_test(self): "trusted": 10, "untrusted_pending": 0, "immature": 0, + "nonmempool": 0, }) # And the unconfirmed tx to be abandoned assert_equal(self.nodes[1].gettransaction(txid)["details"][0]["abandoned"], True) diff --git a/test/functional/wallet_v3_txs.py b/test/functional/wallet_v3_txs.py index db9f1483aba2..79d8ad66738e 100755 --- a/test/functional/wallet_v3_txs.py +++ b/test/functional/wallet_v3_txs.py @@ -39,19 +39,19 @@ def wrapper(self, *args): func(self, *args) finally: self.generate(self.nodes[0], 1) - try: - self.alice.sendall([self.charlie.getnewaddress()]) - except JSONRPCException as e: - assert "Total value of UTXO pool too low to pay for transaction" in e.error['message'] - try: - self.bob.sendall([self.charlie.getnewaddress()]) - except JSONRPCException as e: - assert "Total value of UTXO pool too low to pay for transaction" in e.error['message'] + for wallet in [self.alice, self.bob]: + txs = set(tx["txid"] for tx in wallet.listtransactions("*", 1000) if tx["confirmations"] == 0 and not tx["abandoned"]) + for tx in txs: + wallet.abandontransaction(tx) + try: + wallet.sendall([self.charlie.getnewaddress()]) + except JSONRPCException as e: + assert "Total value of UTXO pool too low to pay for transaction" in e.error['message'] self.generate(self.nodes[0], 1) for wallet in [self.alice, self.bob]: balance = wallet.getbalances()["mine"] - for balance_type in ["untrusted_pending", "trusted", "immature"]: + for balance_type in ["untrusted_pending", "trusted", "immature", "nonmempool"]: assert_equal(balance[balance_type], 0) assert_equal(self.alice.getrawmempool(), []) From 32325d17777fcb58f36842fc8e679fbedf6b2b02 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 19 Nov 2025 11:17:27 +1000 Subject: [PATCH 018/502] tests: Add test for mempool-invalid wallet tx Uses send rpc to create a tx with oversized OP_RETURN output, verifies that it doesn't enter the mempool, and that getbalance rpc returns a nonmempool value. --- test/functional/wallet_send.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 1316aecbf7a7..b5119dc8bb5a 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -32,8 +32,8 @@ def set_test_params(self): self.noban_tx_relay = True self.supports_cli = False self.extra_args = [ - ["-walletrbf=1"], - ["-walletrbf=1"] + ["-walletrbf=1", "-datacarriersize=16"], + ["-walletrbf=1", "-datacarriersize=16"] ] getcontext().prec = 8 # Satoshi precision for Decimal @@ -45,7 +45,7 @@ def test_send(self, from_wallet, to_wallet=None, amount=None, data=None, conf_target=None, estimate_mode=None, fee_rate=None, add_to_wallet=None, psbt=None, inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None, change_type=None, locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None, - expect_error=None, solving_data=None, minconf=None): + expect_error=None, solving_data=None, minconf=None, nonmempool=False): assert_not_equal((amount is None), (data is None)) from_balance_before = from_wallet.getbalances()["mine"]["trusted"] @@ -171,16 +171,20 @@ def test_send(self, from_wallet, to_wallet=None, amount=None, data=None, tx = from_wallet.gettransaction(res["txid"]) assert tx assert_equal(tx["bip125-replaceable"], "yes" if replaceable else "no") - # Ensure transaction exists in the mempool: - tx = from_wallet.getrawtransaction(res["txid"], True) - assert tx - if amount: - if subtract_fee_from_outputs: - assert_equal(from_balance_before - from_balance, amount) - else: - assert_greater_than(from_balance_before - from_balance, amount) + if nonmempool: + assert_raises_rpc_error(-5, "No such mempool transaction", from_wallet.getrawtransaction, res["txid"]) + assert from_wallet.getbalances()["mine"]["nonmempool"] < 0 else: - assert next((out for out in tx["vout"] if out["scriptPubKey"]["asm"] == "OP_RETURN 35"), None) + # Ensure transaction exists in the mempool: + tx = from_wallet.getrawtransaction(res["txid"], True) + assert tx + if amount: + if subtract_fee_from_outputs: + assert_equal(from_balance_before - from_balance, amount) + else: + assert_greater_than(from_balance_before - from_balance, amount) + else: + assert next((out for out in tx["vout"] if out["scriptPubKey"]["asm"] == "OP_RETURN 35"), None) else: assert_equal(from_balance_before, from_balance) @@ -280,6 +284,9 @@ def run_test(self): res = w2.walletprocesspsbt(res["psbt"]) assert res["complete"] + self.log.info("Create mempool-invalid tx (due to large OP_RETURN)...") + self.test_send(from_wallet=w0, data=b"The quick brown fox jumps over the lazy dog".hex(), nonmempool=True) + self.log.info("Test setting explicit fee rate") res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate="1", add_to_wallet=False) res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate="1", add_to_wallet=False) From 9f273f1c1c51d44db455dc7a655825c0692ad65d Mon Sep 17 00:00:00 2001 From: Pol Espinasa Date: Wed, 18 Feb 2026 17:45:13 +0100 Subject: [PATCH 019/502] build: Add path to doc recommended versions for CLANG, GCC and MSVC --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index f992a8d6af4e..0dace60925c2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -75,6 +75,10 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND NOT CMAKE_HOST_APPLE) set(CMAKE_PLATFORM_HAS_INSTALLNAME FALSE) endif() enable_language(CXX) +set(MIN_CLANG_DOCS "doc/dependencies.md#compiler") +set(MIN_GCC_DOCS "doc/dependencies.md#compiler") +set(MIN_MSVC_DOCS "doc/build-windows-msvc.md") + set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) From ac1ccc5bd912bd8ffa9816513b3129fa3d802aad Mon Sep 17 00:00:00 2001 From: Pol Espinasa Date: Wed, 18 Feb 2026 17:42:15 +0100 Subject: [PATCH 020/502] build: Add CTAD feature check --- CMakeLists.txt | 8 +++++ cmake/module/CheckCXXFeatures.cmake | 45 +++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 cmake/module/CheckCXXFeatures.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 0dace60925c2..42a640955e9a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -189,6 +189,14 @@ string(APPEND CMAKE_CXX_COMPILE_OBJECT " ${APPEND_CPPFLAGS} ${APPEND_CXXFLAGS}") string(APPEND CMAKE_CXX_CREATE_SHARED_LIBRARY " ${APPEND_LDFLAGS}") string(APPEND CMAKE_CXX_LINK_EXECUTABLE " ${APPEND_LDFLAGS}") +#============================= +# C++ Feature Detection +#============================= +# In case the compiler is not GCC, Clang or MSVC this provides extra checks +# which verify that some features are available in the standard library. +include(CheckCXXFeatures) +check_cxx_features() + set(configure_warnings) include(CheckLinkerSupportsPIE) diff --git a/cmake/module/CheckCXXFeatures.cmake b/cmake/module/CheckCXXFeatures.cmake new file mode 100644 index 000000000000..b3760a7ee46b --- /dev/null +++ b/cmake/module/CheckCXXFeatures.cmake @@ -0,0 +1,45 @@ +# Copyright (c) 2026-present The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://opensource.org/license/mit/. + +include_guard(GLOBAL) + +#Checks for C++ features required to compile Bitcoin Core. + +include(CheckCXXSourceCompiles) + +function(check_cxx_features) + set(CMAKE_REQUIRED_QUIET TRUE) + + message(STATUS "Checking for required C++ features") + + # Checks for Class Template Argument Deduction for aggregate types - used in src/util/overloaded.h + check_cxx_source_compiles(" + #include + + template struct Overloaded : Ts... { using Ts::operator()...; }; + + int main() { + std::variant v = 42; + return std::visit(Overloaded{ + [](int) { return 0; }, + [](double) { return 1; } + }, v); + } + " HAVE_CTAD_FOR_AGGREGATES) + + if(NOT HAVE_CTAD_FOR_AGGREGATES) + message(FATAL_ERROR + "Compiler lacks Class Template Argument Deduction (CTAD) for aggregates.\n" + "This C++ feature is required for src/util/overloaded.h.\n" + "You are probably using an old compiler version\n" + "The recommended compiler versions can be checked in:\n" + " - GCC -> ${MIN_GCC_DOCS}\n" + " - Clang -> ${MIN_CLANG_DOCS}\n" + " - MSVC -> ${MIN_MSVC_DOCS}\n" + ) + endif() + + message(STATUS "Checking for required C++ features - done") + +endfunction() From 43b09b993d0a5a8178d70a0149a5301aed82bb8d Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Thu, 11 Dec 2025 10:28:06 +0000 Subject: [PATCH 021/502] fuzz: Improve oracle for existing CCoinControl tests --- src/wallet/test/fuzz/coincontrol.cpp | 34 +++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/wallet/test/fuzz/coincontrol.cpp b/src/wallet/test/fuzz/coincontrol.cpp index 6774a21d01bf..e46a469f9ea1 100644 --- a/src/wallet/test/fuzz/coincontrol.cpp +++ b/src/wallet/test/fuzz/coincontrol.cpp @@ -57,26 +57,48 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol) }, [&] { (void)coin_control.Select(out_point); + assert(coin_control.IsSelected(out_point)); }, [&] { const CTxOut tx_out{ConsumeMoney(fuzzed_data_provider), ConsumeScript(fuzzed_data_provider)}; - (void)coin_control.Select(out_point).SetTxOut(tx_out); + auto& input = coin_control.Select(out_point); + const auto set_tx_out{fuzzed_data_provider.ConsumeBool()}; + if (set_tx_out) { + input.SetTxOut(tx_out); + } + auto has_tx_out{input.HasTxOut()}; + auto is_external_selected{coin_control.IsExternalSelected(out_point)}; + if (set_tx_out) { + assert(has_tx_out); + assert(input.GetTxOut() == tx_out); + assert(is_external_selected); + } else if (!has_tx_out) { + assert(!is_external_selected); + } }, [&] { - (void)coin_control.UnSelect(out_point); + coin_control.UnSelect(out_point); + assert(!coin_control.IsSelected(out_point)); }, [&] { - (void)coin_control.UnSelectAll(); + coin_control.UnSelectAll(); + assert(!coin_control.HasSelected()); }, [&] { - (void)coin_control.ListSelected(); + const std::vector selected = coin_control.ListSelected(); + for (const auto& out : selected) { + assert(coin_control.IsSelected(out)); + } }, [&] { int64_t weight{fuzzed_data_provider.ConsumeIntegral()}; - (void)coin_control.SetInputWeight(out_point, weight); + coin_control.SetInputWeight(out_point, weight); + assert(coin_control.GetInputWeight(out_point) == weight); }, [&] { - (void)coin_control.GetInputWeight(out_point); + const bool is_selected = coin_control.IsSelected(out_point); + assert(!coin_control.GetInputWeight(out_point) || is_selected); + assert(!coin_control.GetSequence(out_point) || is_selected); }); } } From 2104282ddde8e7a86965a149f82cd3c3ac35c014 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Sun, 7 Dec 2025 08:42:54 +0000 Subject: [PATCH 022/502] fuzz: Add tests for CCoinControl methods The `ccoincontrol` fuzzer misses tests for a number of CCoinControl operations. Add them. --- src/wallet/test/fuzz/coincontrol.cpp | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/wallet/test/fuzz/coincontrol.cpp b/src/wallet/test/fuzz/coincontrol.cpp index e46a469f9ea1..870eab89490e 100644 --- a/src/wallet/test/fuzz/coincontrol.cpp +++ b/src/wallet/test/fuzz/coincontrol.cpp @@ -99,6 +99,46 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol) const bool is_selected = coin_control.IsSelected(out_point); assert(!coin_control.GetInputWeight(out_point) || is_selected); assert(!coin_control.GetSequence(out_point) || is_selected); + }, + [&] { + const auto scripts = coin_control.GetScripts(out_point); + assert(coin_control.IsSelected(out_point) || (!scripts.first && !scripts.second)); + }, + [&] { + assert(coin_control.HasSelectedOrder() || !coin_control.GetSelectionPos(out_point)); + }, + [&] { + assert(!coin_control.GetSelectionPos(out_point) || coin_control.IsSelected(out_point)); + }, + [&] { + auto& input = coin_control.Select(out_point); + uint32_t sequence{fuzzed_data_provider.ConsumeIntegral()}; + input.SetSequence(sequence); + assert(input.GetSequence() == sequence); + assert(coin_control.GetSequence(out_point) == sequence); + }, + [&] { + auto& input = coin_control.Select(out_point); + const CScript script{ConsumeScript(fuzzed_data_provider)}; + input.SetScriptSig(script); + assert(input.HasScripts()); + assert(input.GetScripts().first == script); + assert(coin_control.GetScripts(out_point).first == script); + }, + [&] { + auto& input = coin_control.Select(out_point); + const CScriptWitness script_wit{ConsumeScriptWitness(fuzzed_data_provider)}; + input.SetScriptWitness(script_wit); + assert(input.HasScripts()); + assert(input.GetScripts().second->stack == script_wit.stack); + assert(coin_control.GetScripts(out_point).second->stack == script_wit.stack); + }, + [&] { + auto& input = coin_control.Select(out_point); + unsigned int pos{fuzzed_data_provider.ConsumeIntegral()}; + input.SetPosition(pos); + assert(input.GetPosition() == pos); + assert(coin_control.GetSelectionPos(out_point) == pos); }); } } From fa9f434df89745ab5a0fd1d6c07800cbe802de03 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 7 May 2025 00:57:44 +0200 Subject: [PATCH 023/502] test: Allow time_point in boost checks This is required in the next commit. --- src/test/util/common.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/test/util/common.h b/src/test/util/common.h index 591f651fbaf4..2fa4cb3cfe7e 100644 --- a/src/test/util/common.h +++ b/src/test/util/common.h @@ -5,8 +5,9 @@ #ifndef BITCOIN_TEST_UTIL_COMMON_H #define BITCOIN_TEST_UTIL_COMMON_H -#include +#include #include +#include #include /** @@ -27,6 +28,12 @@ class HasReason // Make types usable in BOOST_CHECK_* @{ namespace std { +template +inline std::ostream& operator<<(std::ostream& os, const std::chrono::time_point& tp) +{ + return os << tp.time_since_epoch().count(); +} + template requires std::is_enum_v inline std::ostream& operator<<(std::ostream& os, const T& e) { From fa8fe0941edfe515cb491e970371f8f84d2d8cdc Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 20 Mar 2026 10:37:35 +0100 Subject: [PATCH 024/502] fuzz: Use NodeClockContext This refactor is a follow-up to commit eeeeb2a0b902ed69b5cd5523833d3ab5d963c81f and does not change any behavior. However, it is nice to know that no global mocktime leaks from the fuzz init step to the first fuzz input, or from one fuzz input execution to the next. With the clock context, the global is re-set at the end of the context. --- src/wallet/test/fuzz/scriptpubkeyman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index a627c7705413..fa8bc250d07e 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -205,7 +205,7 @@ FUZZ_TARGET(spkm_migration, .init = initialize_spkm_migration) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; const auto& node{g_setup->m_node}; Chainstate& chainstate{node.chainman->ActiveChainstate()}; From faad08e59c4419e09eb75054bf468ca98a837ca8 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 7 May 2025 02:04:48 +0200 Subject: [PATCH 025/502] test: Use NodeClockContext in more tests --- src/bench/util_time.cpp | 5 ++-- src/bench/wallet_balance.cpp | 3 ++- src/bench/wallet_create_tx.cpp | 5 ++-- src/test/addrman_tests.cpp | 4 ++- src/test/banman_tests.cpp | 4 +-- src/test/chainstate_write_tests.cpp | 6 +++-- src/test/denialofservice_tests.cpp | 39 +++++++++++++++-------------- src/test/mempool_tests.cpp | 16 ++++++------ src/test/orphanage_tests.cpp | 5 ++-- src/test/testnet4_miner_tests.cpp | 21 ++++++---------- src/test/util_tests.cpp | 4 +-- 11 files changed, 57 insertions(+), 55 deletions(-) diff --git a/src/bench/util_time.cpp b/src/bench/util_time.cpp index bc3e3892f1fe..19ffd0e04985 100644 --- a/src/bench/util_time.cpp +++ b/src/bench/util_time.cpp @@ -3,7 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include - +#include #include static void BenchTimeDeprecated(benchmark::Bench& bench) @@ -15,11 +15,10 @@ static void BenchTimeDeprecated(benchmark::Bench& bench) static void BenchTimeMock(benchmark::Bench& bench) { - SetMockTime(111); + NodeClockContext clock_ctx{111s}; bench.run([&] { (void)GetTime(); }); - SetMockTime(0); } static void BenchTimeMillis(benchmark::Bench& bench) diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 03774ef54acd..45f23494280a 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -32,7 +33,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b // Set clock to genesis block, so the descriptors/keys creation time don't interfere with the blocks scanning process. // The reason is 'generatetoaddress', which creates a chain with deterministic timestamps in the past. - SetMockTime(test_setup->m_node.chainman->GetParams().GenesisBlock().nTime); + NodeClockContext clock_ctx{test_setup->m_node.chainman->GetParams().GenesisBlock().Time()}; CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()}; { LOCK(wallet.cs_wallet); diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp index 8ff1e39a2f83..11125e26fed0 100644 --- a/src/bench/wallet_create_tx.cpp +++ b/src/bench/wallet_create_tx.cpp @@ -20,6 +20,7 @@ #include