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

6f39d88 Merge bitcoin#26889: refactor: wallet, remove global 'ArgsManager' dependency (Konstantin Akimov)
f7c864d refactor: drop coinjoin_loader from WalletContext and loader factories (Konstantin Akimov)
f899d9a refactor: drop coinjoin_loader from CWallet constructor (Konstantin Akimov)
66d6b7f refactor: expose CoinJoin loader through interfaces::Chain (Konstantin Akimov)
c25b960 refactor: register wallets with CoinJoin only in AddWallet (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  CWallet has extra argument coinjoin_loader in its ctor and it makes pain when doing backports due to more than 100 usages spread all over code bases and it causes conflicts when doing backport.

  ## What was done?
   - fixed double call of registration in CWallet::Create in the freshly loaded wallet: with the CoinJoin client manager  and AddWallet registered it again
   - expose CoinJoin loader through interfaces::Chain
   - drop coinjoin_loader from CWallet constructor
   - drop coinjoin_loader from WalletContext and loader factories
   - backport bitcoin#26889 - king of conflicts.

  Backport bitcoin#26889 is touching _every_ line what had caused conflicts due to extra argument in CWallet constructor, this PR has all important prerequisites to make its merge easier.
  NOTE: some chunks of 26889 is omitted because this code is not backported yet [including taproot's related], some other quite opposite added. Though, 26889 is not partial and done in full because compiler enforces missing changes to update: remove gArgs / m_args if they will be added later by other commits.

  ## How Has This Been Tested?
  Run unit & regressions tests.

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: 833aaf5c700e19345b172c7aa326d37d37ed12b1646fe274dba47eb9928ea481fd94afe8282e5a8d0e02e14d56a00bc07f0fcd0326990ea1135fe77244450264

Author: PastaPastaPasta

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; }
 };