backport: bitcoin/bitcoin#26889: refactor: wallet, remove global 'ArgsManager' dependency

src/bench/coin_selection.cpp

@@ -44,7 +44,7 @@ static void CoinSelection(benchmark::Bench& bench)
 {
     NodeContext node;
     auto chain = interfaces::MakeChain(node);
-    CWallet wallet(chain.get(), /*coinjoin_loader=*/nullptr, "", gArgs, CreateDummyWalletDatabase());
+    CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
     std::vector<std::unique_ptr<CWalletTx>> wtxs;
     LOCK(wallet.cs_wallet);
 

src/bench/wallet_balance.cpp

@@ -31,7 +31,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);
-    CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", gArgs, CreateMockWalletDatabase()};
+    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()};
     {
         LOCK(wallet.cs_wallet);
         wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);

src/bench/wallet_create_tx.cpp

@@ -84,7 +84,7 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type
 {
     const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
 
-    CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", gArgs, CreateMockWalletDatabase()};
+    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()};
     {
         LOCK(wallet.cs_wallet);
         wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
@@ -137,7 +137,7 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type
 static void AvailableCoins(benchmark::Bench& bench, const std::vector<OutputType>& output_type)
 {
     const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
-    CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", gArgs, CreateMockWalletDatabase()};
+    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()};
     {
         LOCK(wallet.cs_wallet);
         wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);

src/dummywallet.cpp

@@ -96,8 +96,7 @@ std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<wallet::CWallet>& walle
     throw std::logic_error("Wallet function called in non-wallet build.");
 }
 
-std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args, node::NodeContext& node_context,
-                                               interfaces::CoinJoin::Loader& coinjoin_loader)
+std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args, node::NodeContext& node_context)
 {
     throw std::logic_error("Wallet function called in non-wallet build.");
 }

src/init/bitcoin-gui.cpp

@@ -32,9 +32,9 @@ class BitcoinGuiInit : public interfaces::Init
     {
         return interfaces::MakeCoinJoinLoader(m_node);
     }
-    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
+    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain) override
     {
-        return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
+        return MakeWalletLoader(chain, *Assert(m_node.args), m_node);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     interfaces::Ipc* ipc() override { return m_ipc.get(); }

src/init/bitcoin-node.cpp

@@ -35,9 +35,9 @@ class BitcoinNodeInit : public interfaces::Init
     {
         return interfaces::MakeCoinJoinLoader(m_node);
     }
-    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
+    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain) override
     {
-        return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
+        return MakeWalletLoader(chain, *Assert(m_node.args), m_node);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     interfaces::Ipc* ipc() override { return m_ipc.get(); }

src/init/bitcoin-qt.cpp

@@ -29,9 +29,9 @@ class BitcoinQtInit : public interfaces::Init
     {
         return interfaces::MakeCoinJoinLoader(m_node);
     }
-    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
+    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain) override
     {
-        return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
+        return MakeWalletLoader(chain, *Assert(m_node.args), m_node);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     node::NodeContext m_node;

src/init/bitcoind.cpp

@@ -32,9 +32,9 @@ class BitcoindInit : public interfaces::Init
     {
         return interfaces::MakeCoinJoinLoader(m_node);
     }
-    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
+    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain) override
     {
-        return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
+        return MakeWalletLoader(chain, *Assert(m_node.args), m_node);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     NodeContext& m_node;

src/interfaces/chain.h

@@ -48,6 +48,9 @@ namespace interfaces {
 
 class Wallet;
 class Handler;
+namespace CoinJoin {
+class Loader;
+} // namespace CoinJoin
 
 //! Hash/height pair to help track and identify blocks.
 struct BlockKey {
@@ -181,6 +184,10 @@ class Chain
     //! Return list of MN Collateral from outputs
     virtual std::vector<COutPoint> listMNCollaterials(const std::vector<std::pair<const CTransactionRef&, uint32_t>>& outputs) = 0;
 
+    //! Return the loader that registers wallets with the node's CoinJoin
+    //! client manager, or nullptr when the node runs without one.
+    virtual CoinJoin::Loader* coinJoinLoader() = 0;
+
     //! Return whether node has the block and optionally return block metadata
     //! or contents.
     virtual bool findBlock(const uint256& hash, const FoundBlock& block={}) = 0;

src/interfaces/init.h

@@ -34,7 +34,7 @@ class Init
     virtual std::unique_ptr<Node> makeNode() { return nullptr; }
     virtual std::unique_ptr<Chain> makeChain() { return nullptr; }
     virtual std::unique_ptr<CoinJoin::Loader> makeCoinJoinLoader() { return nullptr; }
-    virtual std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain&, CoinJoin::Loader&) { return nullptr; }
+    virtual std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain&) { return nullptr; }
     virtual std::unique_ptr<Echo> makeEcho() { return nullptr; }
     virtual Ipc* ipc() { return nullptr; }
 };

src/interfaces/wallet.h

@@ -64,9 +64,6 @@ struct WalletTx;
 struct WalletTxOut;
 struct WalletTxStatus;
 struct WalletMigrationResult;
-namespace CoinJoin {
-class Loader;
-}
 
 using WalletOrderForm = std::vector<std::pair<std::string, std::string>>;
 using WalletValueMap = std::map<std::string, std::string>;
@@ -589,8 +586,7 @@ std::unique_ptr<Wallet> MakeWallet(wallet::WalletContext& context, const std::sh
 
 //! Return implementation of ChainClient interface for a wallet loader. This
 //! function will be undefined in builds where ENABLE_WALLET is false.
-std::unique_ptr<WalletLoader> MakeWalletLoader(Chain& chain, ArgsManager& args, node::NodeContext& node_context,
-                                               CoinJoin::Loader& coinjoin_loader);
+std::unique_ptr<WalletLoader> MakeWalletLoader(Chain& chain, ArgsManager& args, node::NodeContext& node_context);
 
 } // namespace interfaces
 

src/node/interfaces.cpp

@@ -1463,6 +1463,7 @@ class ChainImpl : public Chain
         }
         return listRet;
     }
+    interfaces::CoinJoin::Loader* coinJoinLoader() override { return m_node.coinjoin_loader.get(); }
     bool findBlock(const uint256& hash, const FoundBlock& block) override
     {
         WAIT_LOCK(cs_main, lock);

src/qt/optionsmodel.cpp

@@ -18,8 +18,9 @@
 #include <net.h>
 #include <netbase.h>
 #include <txdb.h>       // for -dbcache defaults
-#include <validation.h> // For DEFAULT_SCRIPTCHECK_THREADS
 #include <util/string.h>
+#include <util/system.h>
+#include <validation.h> // For DEFAULT_SCRIPTCHECK_THREADS
 
 #ifdef ENABLE_WALLET
 #include <coinjoin/options.h>
@@ -1128,6 +1129,11 @@ bool OptionsModel::isRestartRequired() const
     return settings.value("fRestartRequired", false).toBool();
 }
 
+bool OptionsModel::hasSigner()
+{
+    return gArgs.GetArg("-signer", "") != "";
+}
+
 void OptionsModel::checkAndMigrate()
 {
     // Migration of default values

src/qt/optionsmodel.h

@@ -149,6 +149,9 @@ class OptionsModel : public QAbstractListModel
     const QString& getOverriddenByCommandLine() { return strOverriddenByCommandLine; }
     bool isOptionOverridden(const QString& option) const { return strOverriddenByCommandLine.contains(option); }
 
+    /** Whether -signer was set or not */
+    bool hasSigner();
+
     /* Explicit setters */
     void SetPruneTargetGB(int prune_target_gb);
 

src/qt/sendcoinsdialog.cpp

@@ -223,7 +223,7 @@ void SendCoinsDialog::setModel(WalletModel *_model)
         if (model->wallet().hasExternalSigner()) {
             //: "device" usually means a hardware wallet.
             ui->sendButton->setText(tr("Sign on device"));
-            if (gArgs.GetArg("-signer", "") != "") {
+            if (model->getOptionsModel()->hasSigner()) {
                 ui->sendButton->setEnabled(true);
                 ui->sendButton->setToolTip(tr("Connect your hardware wallet first."));
             } else {

src/qt/test/addressbooktests.cpp

@@ -72,7 +72,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
 {
     TestChain100Setup test;
     node.setContext(&test.m_node);
-    const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", gArgs, CreateMockWalletDatabase());
+    const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
     wallet->LoadWallet();
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
     {

src/qt/test/masternodetestutil.cpp

@@ -74,8 +74,7 @@ struct WalletOptions {
 std::shared_ptr<CWallet> MakeWallet(interfaces::Node& node, WalletContext& context, const std::string& name,
                                     const WalletOptions& options)
 {
-    auto wallet{std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), name,
-                                          gArgs, CreateMockWalletDatabase())};
+    auto wallet{std::make_shared<CWallet>(node.context()->chain.get(), name, CreateMockWalletDatabase())};
     wallet->LoadWallet();
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
     const CBlockIndex* const tip{options.setup == nullptr

src/qt/test/masternodewidgettests.cpp

@@ -407,8 +407,7 @@ void MasternodeWidgetTests::operationRunnerThreading()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     auto wallet_interface{interfaces::MakeWallet(context, wallet)};
     QVERIFY(wallet_interface != nullptr);
@@ -456,8 +455,7 @@ void MasternodeWidgetTests::operationRunnerExceptions()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     auto wallet_interface{interfaces::MakeWallet(context, wallet)};
     QVERIFY(wallet_interface != nullptr);
@@ -494,8 +492,7 @@ void MasternodeWidgetTests::operationRunnerShutdownDelivery()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     auto wallet_interface{interfaces::MakeWallet(context, wallet)};
     QVERIFY(wallet_interface != nullptr);
@@ -551,8 +548,7 @@ void MasternodeWidgetTests::feeSourcePickerEligibility()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     wallet->SetWalletFlag(wallet::WALLET_FLAG_DESCRIPTORS);
 
@@ -617,8 +613,7 @@ void MasternodeWidgetTests::registeredCollateralExclusion()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     wallet->SetWalletFlag(wallet::WALLET_FLAG_DESCRIPTORS);
 
@@ -921,8 +916,7 @@ void MasternodeWidgetTests::masternodeListRegistrationAvailability()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     MasternodeTestUtil::GuiModels models{m_node};
     QVERIFY2(models.ok, qPrintable(QString::fromStdString(models.error.translated)));
@@ -1005,8 +999,7 @@ void MasternodeWidgetTests::sharedMasternodeOwnedFilter()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     wallet->SetWalletFlag(wallet::WALLET_FLAG_DESCRIPTORS);
 
@@ -1043,8 +1036,7 @@ void MasternodeWidgetTests::sharedMasternodeContextMenu()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     wallet->SetWalletFlag(wallet::WALLET_FLAG_DESCRIPTORS);
 
@@ -1228,8 +1220,7 @@ void MasternodeWidgetTests::wizardInteractionLifecycle()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     MasternodeTestUtil::GuiModels models{m_node};
     QVERIFY2(models.ok, qPrintable(QString::fromStdString(models.error.translated)));
@@ -1312,8 +1303,7 @@ void MasternodeWidgetTests::registrationResultStates()
     TestChain100Setup test;
     m_node.setContext(&test.m_node);
     WalletContext& context{*m_node.walletLoader().context()};
-    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), m_node.context()->coinjoin_loader.get(),
-                                                "", gArgs, CreateMockWalletDatabase())};
+    const auto wallet{std::make_shared<CWallet>(m_node.context()->chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     MasternodeTestUtil::GuiModels models{m_node};
     QVERIFY2(models.ok, qPrintable(QString::fromStdString(models.error.translated)));

src/qt/test/providertransactiontests.cpp

@@ -220,8 +220,8 @@ void ProviderTransactionTests::providerTransactionHistory()
     m_node.setContext(&test.m_node);
 
     WalletContext& context{*m_node.walletLoader().context()};
-    std::shared_ptr<CWallet> wallet{std::make_shared<CWallet>(test.m_node.chain.get(), test.m_node.coinjoin_loader.get(),
-                                                              "", test.m_args, CreateMockWalletDatabase())};
+    std::shared_ptr<CWallet> wallet{
+        std::make_shared<CWallet>(test.m_node.chain.get(), "", CreateMockWalletDatabase())};
     wallet->LoadWallet();
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
     {

src/qt/test/wallettests.cpp

@@ -131,7 +131,7 @@ void TestGUI(interfaces::Node& node)
     }
     node.setContext(&test.m_node);
     WalletContext& context = *node.walletLoader().context();
-    const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", gArgs, CreateMockWalletDatabase());
+    const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
     AddWallet(context, wallet);
     wallet->LoadWallet();
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);

src/test/util/setup_common.cpp

@@ -395,7 +395,7 @@ TestingSetup::TestingSetup(
     // WalletTestingSetup
     m_node.coinjoin_loader = interfaces::MakeCoinJoinLoader(m_node);
 
-    auto wallet_loader = interfaces::MakeWalletLoader(*m_node.chain, *m_node.args, m_node, *m_node.coinjoin_loader);
+    auto wallet_loader = interfaces::MakeWalletLoader(*m_node.chain, *m_node.args, m_node);
     m_node.wallet_loader = wallet_loader.get();
     m_node.chain_clients.emplace_back(std::move(wallet_loader));
 #endif // ENABLE_WALLET

src/wallet/context.h

@@ -15,9 +15,6 @@
 class ArgsManager;
 namespace interfaces {
 class Chain;
-namespace CoinJoin {
-class Loader;
-} // namspace CoinJoin
 class Wallet;
 } // namespace interfaces
 namespace node {
@@ -48,7 +45,6 @@ struct WalletContext {
     std::vector<std::shared_ptr<CWallet>> wallets GUARDED_BY(wallets_mutex);
     std::list<LoadWalletFn> wallet_loading_fns GUARDED_BY(wallets_mutex);
     std::list<LoadWalletFn> wallet_load_fns GUARDED_BY(wallets_mutex);
-    interfaces::CoinJoin::Loader* coinjoin_loader{nullptr};
     // Some Dash RPCs rely on WalletContext yet access NodeContext members
     // even though wallet RPCs should refrain from accessing non-wallet
     // capabilities (even though it is a hard ask sometimes). We should get