refactor: drop global coinJoinWalletManager

UdjinM6 · PastaPastaPasta · commit dc5152bd7417 · 2024-01-10T12:06:04.000-06:00
diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp
@@ -29,8 +29,6 @@
 #include <memory>
 #include <univalue.h>
 
-std::unique_ptr<CoinJoinWalletManager> coinJoinWalletManager;
-
 void CCoinJoinClientQueueManager::ProcessMessage(const CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv)
 {
     if (fMasternodeMode) return;
diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h
@@ -29,9 +29,6 @@ class UniValue;
 
 using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
 
-// The main object for accessing mixing
-extern std::unique_ptr<CoinJoinWalletManager> coinJoinWalletManager;
-
 class CPendingDsaRequest
 {
 private:
diff --git a/src/coinjoin/context.cpp b/src/coinjoin/context.cpp
@@ -5,8 +5,8 @@
 #include <coinjoin/context.h>
 
 #include <net.h>
-#include <policy/fees.h>
 #include <txmempool.h>
+#include <validation.h>
 
 #ifdef ENABLE_WALLET
 #include <coinjoin/client.h>
@@ -15,20 +15,10 @@
 
 CJContext::CJContext(CChainState& chainstate, CConnman& connman, CTxMemPool& mempool, const CMasternodeSync& mn_sync, bool relay_txes) :
 #ifdef ENABLE_WALLET
-    walletman {
-        [&]() -> CoinJoinWalletManager* const {
-            assert(::coinJoinWalletManager == nullptr);
-            ::coinJoinWalletManager = std::make_unique<CoinJoinWalletManager>(connman, mempool, mn_sync, queueman);
-            return ::coinJoinWalletManager.get();
-        }()
-    },
+    walletman{std::make_unique<CoinJoinWalletManager>(connman, mempool, mn_sync, queueman)},
     queueman {relay_txes ? std::make_unique<CCoinJoinClientQueueManager>(connman, *walletman, mn_sync) : nullptr},
 #endif // ENABLE_WALLET
     server{std::make_unique<CCoinJoinServer>(chainstate, connman, mempool, mn_sync)}
 {}
 
-CJContext::~CJContext() {
-#ifdef ENABLE_WALLET
-    ::coinJoinWalletManager.reset();
-#endif // ENABLE_WALLET
-}
+CJContext::~CJContext() {}
diff --git a/src/coinjoin/context.h b/src/coinjoin/context.h
@@ -30,7 +30,8 @@ struct CJContext {
     ~CJContext();
 
 #ifdef ENABLE_WALLET
-    CoinJoinWalletManager* const walletman;
+    // The main object for accessing mixing
+    const std::unique_ptr<CoinJoinWalletManager> walletman;
     const std::unique_ptr<CCoinJoinClientQueueManager> queueman;
 #endif // ENABLE_WALLET
     const std::unique_ptr<CCoinJoinServer> server;
diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp
@@ -81,18 +81,18 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader
     }
     std::unique_ptr<interfaces::CoinJoin::Client> GetClient(const std::string& name) override
     {
-        return interfaces::MakeCoinJoinClient(m_walletman, name);
+        auto clientman = m_walletman.Get(name);
+        return clientman ? std::make_unique<CoinJoinClientImpl>(*clientman) : nullptr;
+    }
+    CoinJoinWalletManager& walletman() override
+    {
+        return m_walletman;
     }
 };
 
 } // namespace
 } // namespace coinjoin
 
 namespace interfaces {
-std::unique_ptr<CoinJoin::Client> MakeCoinJoinClient(const CoinJoinWalletManager& walletman, const std::string& name)
-{
-    auto clientman = walletman.Get(name);
-    return clientman ? std::make_unique<coinjoin::CoinJoinClientImpl>(*clientman) : nullptr;
-}
 std::unique_ptr<CoinJoin::Loader> MakeCoinJoinLoader(CoinJoinWalletManager& walletman) { return std::make_unique<coinjoin::CoinJoinLoaderImpl>(walletman); }
 } // namespace interfaces
diff --git a/src/interfaces/coinjoin.h b/src/interfaces/coinjoin.h
@@ -38,10 +38,10 @@ class Loader
     virtual void RemoveWallet(const std::string&) = 0;
     virtual void FlushWallet(const std::string&) = 0;
     virtual std::unique_ptr<CoinJoin::Client> GetClient(const std::string&) = 0;
+    virtual CoinJoinWalletManager& walletman() = 0;
 };
 } // namespace CoinJoin
 
-std::unique_ptr<CoinJoin::Client> MakeCoinJoinClient(const CoinJoinWalletManager& walletman, const std::string& name);
 std::unique_ptr<CoinJoin::Loader> MakeCoinJoinLoader(CoinJoinWalletManager& walletman);
 
 } // namespace interfaces
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
@@ -303,7 +303,7 @@ class Node
     //! Return interface for accessing masternode related handler.
     virtual Masternode::Sync& masternodeSync() = 0;
 
-    //! Return interface for accessing coinjoin related handler.
+    //! Return interface for accessing coinjoin options related handler.
     virtual CoinJoin::Options& coinJoinOptions() = 0;
 
     //! Return interface for accessing coinjoin loader handler.
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
@@ -8,7 +8,6 @@
 #include <amount.h>                    // For CAmount
 #include <fs.h>                        // For fs::path
 #include <interfaces/chain.h>          // For ChainClient
-#include <interfaces/coinjoin.h>       // For CoinJoin::*
 #include <pubkey.h>                    // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation)
 #include <script/standard.h>           // For CTxDestination
 #include <support/allocators/secure.h> // For SecureString
@@ -48,6 +47,9 @@ struct WalletBalances;
 struct WalletTx;
 struct WalletTxOut;
 struct WalletTxStatus;
+namespace CoinJoin {
+class Loader;
+}
 
 using WalletOrderForm = std::vector<std::pair<std::string, std::string>>;
 using WalletValueMap = std::map<std::string, std::string>;
@@ -278,8 +280,6 @@ class Wallet
     // Return whether private keys enabled.
     virtual bool privateKeysDisabled() = 0;
 
-    virtual CoinJoin::Client& coinJoin() = 0;
-
     //! Get max tx fee.
     virtual CAmount getDefaultMaxTxFee() = 0;
 
diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp
@@ -14,6 +14,7 @@
 #ifdef ENABLE_WALLET
 #include <coinjoin/client.h>
 #include <coinjoin/options.h>
+#include <interfaces/coinjoin.h>
 #include <wallet/rpcwallet.h>
 #endif // ENABLE_WALLET
 
@@ -51,7 +52,8 @@ static UniValue coinjoin(const JSONRPCRequest& request)
         }
     }
 
-    auto cj_clientman = ::coinJoinWalletManager->Get(wallet->GetName());
+    const NodeContext& node = EnsureAnyNodeContext(request.context);
+    auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
     CHECK_NONFATAL(cj_clientman != nullptr);
 
     if (request.params[0].get_str() == "start") {
@@ -65,7 +67,6 @@ static UniValue coinjoin(const JSONRPCRequest& request)
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already.");
         }
 
-        const NodeContext& node = EnsureAnyNodeContext(request.context);
         CTxMemPool& mempool = EnsureMemPool(node);
         CBlockPolicyEstimator& fee_estimator = EnsureFeeEstimator(node);
         bool result = cj_clientman->DoAutomaticDenominating(*node.connman, fee_estimator, mempool);
@@ -163,7 +164,7 @@ static UniValue getcoinjoininfo(const JSONRPCRequest& request)
         return obj;
     }
 
-    auto manager = ::coinJoinWalletManager->Get(wallet->GetName());
+    auto manager = node.coinjoin_loader->walletman().Get(wallet->GetName());
     CHECK_NONFATAL(manager != nullptr);
     manager->GetJsonInfo(obj);
 
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -110,7 +110,7 @@ void DashTestSetup(NodeContext& node)
 
     node.cj_ctx = std::make_unique<CJContext>(chainstate, *node.connman, *node.mempool, *::masternodeSync, /* relay_txes */ true);
 #ifdef ENABLE_WALLET
-    node.coinjoin_loader = interfaces::MakeCoinJoinLoader(*::coinJoinWalletManager);
+    node.coinjoin_loader = interfaces::MakeCoinJoinLoader(*node.cj_ctx->walletman);
 #endif // ENABLE_WALLET
     ::deterministicMNManager = std::make_unique<CDeterministicMNManager>(chainstate, *node.connman, *node.evodb);
     node.llmq_ctx = std::make_unique<LLMQContext>(chainstate, *node.connman, *node.evodb, *sporkManager, *node.mempool, node.peerman, true, false);
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
@@ -124,7 +124,7 @@ WalletTxOut MakeWalletTxOut(const CWallet& wallet,
 class WalletImpl : public Wallet
 {
 public:
-    explicit WalletImpl(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& cjwalletman) : m_wallet(wallet), m_cjwalletman(cjwalletman) {}
+    explicit WalletImpl(const std::shared_ptr<CWallet>& wallet) : m_wallet(wallet) {}
 
     void markDirty() override
     {
@@ -504,11 +504,6 @@ class WalletImpl : public Wallet
     bool hdEnabled() override { return m_wallet->IsHDEnabled(); }
     bool canGetAddresses() override { return m_wallet->CanGetAddresses(); }
     bool privateKeysDisabled() override { return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); }
-    interfaces::CoinJoin::Client& coinJoin() override
-    {
-        if (m_coinjoin_client == nullptr) m_coinjoin_client = interfaces::MakeCoinJoinClient(m_cjwalletman, m_wallet->GetName());
-        return *m_coinjoin_client;
-    }
     CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; }
     void remove() override
     {
@@ -558,7 +553,6 @@ class WalletImpl : public Wallet
     CWallet* wallet() override { return m_wallet.get(); }
 
     std::shared_ptr<CWallet> m_wallet;
-    CoinJoinWalletManager& m_cjwalletman;
     std::unique_ptr<interfaces::CoinJoin::Client> m_coinjoin_client;
 };
 
@@ -644,7 +638,7 @@ class WalletLoaderImpl : public WalletLoader
 } // namespace wallet
 
 namespace interfaces {
-std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? std::make_unique<wallet::WalletImpl>(wallet, *::coinJoinWalletManager) : nullptr; }
+std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? std::make_unique<wallet::WalletImpl>(wallet) : nullptr; }
 std::unique_ptr<WalletLoader> MakeWalletLoader(Chain& chain, const std::unique_ptr<interfaces::CoinJoin::Loader>& coinjoin_loader, ArgsManager& args) {
     return std::make_unique<wallet::WalletLoaderImpl>(chain, coinjoin_loader, args);
 }
diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp
@@ -7,6 +7,7 @@
 #include <amount.h>
 #include <coinjoin/client.h>
 #include <coinjoin/coinjoin.h>
+#include <coinjoin/context.h>
 #include <coinjoin/options.h>
 #include <coinjoin/util.h>
 #include <node/context.h>
@@ -207,8 +208,8 @@ class CTransactionBuilderTestSetup : public TestChain100Setup
 
 BOOST_FIXTURE_TEST_CASE(coinjoin_manager_start_stop_tests, CTransactionBuilderTestSetup)
 {
-    BOOST_CHECK_EQUAL(::coinJoinWalletManager->raw().size(), 1);
-    auto& cj_man = ::coinJoinWalletManager->raw().begin()->second;
+    BOOST_CHECK_EQUAL(m_node.cj_ctx->walletman->raw().size(), 1);
+    auto& cj_man = m_node.cj_ctx->walletman->raw().begin()->second;
     BOOST_CHECK_EQUAL(cj_man->IsMixing(), false);
     BOOST_CHECK_EQUAL(cj_man->StartMixing(), true);
     BOOST_CHECK_EQUAL(cj_man->IsMixing(), true);
diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h
@@ -6,6 +6,7 @@
 #define BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H
 
 #include <interfaces/chain.h>
+#include <interfaces/coinjoin.h>
 #include <interfaces/wallet.h>
 #include <node/context.h>
 #include <test/util/setup_common.h>
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
@@ -11,7 +11,9 @@
 #include <vector>
 
 #include <coinjoin/client.h>
+#include <coinjoin/context.h>
 #include <interfaces/chain.h>
+#include <interfaces/coinjoin.h>
 #include <key_io.h>
 #include <node/context.h>
 #include <policy/policy.h>
@@ -1335,8 +1337,7 @@ static size_t CalculateNestedKeyhashInputSize(bool use_max_sig)
     node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
     node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get());
     auto chain = interfaces::MakeChain(node);
-    auto coinjoin_loader = interfaces::MakeCoinJoinLoader(*::coinJoinWalletManager);
-    CWallet wallet(chain.get(), coinjoin_loader, "", CreateDummyWalletDatabase());
+    CWallet wallet(chain.get(), /*coinjoin_loader=*/ nullptr, "", CreateDummyWalletDatabase());
     AddKey(wallet, key);
     auto spk_man = wallet.GetLegacyScriptPubKeyMan();
     spk_man->AddCScript(inner_script);

Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,6 @@`
`29`	`29`	`#include <memory>`
`30`	`30`	`#include <univalue.h>`
`31`	`31`
`32`		`-std::unique_ptr<CoinJoinWalletManager> coinJoinWalletManager;`
`33`		`-`
`34`	`32`	`void CCoinJoinClientQueueManager::ProcessMessage(const CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv)`
`35`	`33`	`{`
`36`	`34`	`if (fMasternodeMode) return;`
Original file line number	Diff line number	Diff line change
`@@ -29,9 +29,6 @@ class UniValue;`
`29`	`29`
`30`	`30`	`using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;`
`31`	`31`
`32`		`-// The main object for accessing mixing`
`33`		`-extern std::unique_ptr<CoinJoinWalletManager> coinJoinWalletManager;`
`34`		`-`
`35`	`32`	`class CPendingDsaRequest`
`36`	`33`	`{`
`37`	`34`	`private:`