refactor: polish CoinJoin interface, use WalletLoader in Dash wallet initialization code, introduce new RPC coinjoin status

doc/release-notes-6594.md

@@ -0,0 +1,7 @@
+Updated RPCs
+------------
+
+* `coinjoin status` is a new RPC that reports the status message of all running mix
+  sessions.
+* `coinjoin start` will no longer report errors from mix sessions, users are recommended
+  to query the status of mix sessions using `coinjoin status` instead.

src/Makefile.test_util.include

@@ -43,3 +43,6 @@ LIBTEST_UTIL += $(LIBBITCOIN_SERVER)
 LIBTEST_UTIL += $(LIBBITCOIN_COMMON)
 LIBTEST_UTIL += $(LIBBITCOIN_UTIL)
 LIBTEST_UTIL += $(LIBBITCOIN_CRYPTO_BASE)
+if ENABLE_WALLET
+LIBTEST_UTIL += $(LIBBITCOIN_WALLET)
+endif

src/coinjoin/client.cpp

@@ -350,17 +350,18 @@ bilingual_str CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const
     }
 }
 
-bilingual_str CCoinJoinClientManager::GetStatuses()
+std::vector<std::string> CCoinJoinClientManager::GetStatuses() const
 {
-    bilingual_str strStatus;
-    bool fWaitForBlock = WaitForAnotherBlock();
-
     AssertLockNotHeld(cs_deqsessions);
+
+    bool fWaitForBlock{WaitForAnotherBlock()};
+    std::vector<std::string> ret;
+
     LOCK(cs_deqsessions);
     for (const auto& session : deqSessions) {
-        strStatus = strStatus + session.GetStatus(fWaitForBlock) + Untranslated("; ");
+        ret.push_back(session.GetStatus(fWaitForBlock).original);
     }
-    return strStatus;
+    return ret;
 }
 
 std::string CCoinJoinClientManager::GetSessionDenoms()
@@ -1914,14 +1915,11 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const
 
 void CoinJoinWalletManager::Add(const std::shared_ptr<CWallet>& wallet)
 {
-    {
-        LOCK(cs_wallet_manager_map);
-        m_wallet_manager_map.try_emplace(wallet->GetName(),
-                                         std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman,
-                                                                                  m_mn_sync, m_isman, m_queueman,
-                                                                                  m_is_masternode));
-    }
-    g_wallet_init_interface.InitCoinJoinSettings(*this);
+    LOCK(cs_wallet_manager_map);
+    m_wallet_manager_map.try_emplace(wallet->GetName(),
+                                     std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman,
+                                                                              m_mn_sync, m_isman, m_queueman,
+                                                                              m_is_masternode));
 }
 
 void CoinJoinWalletManager::DoMaintenance(CConnman& connman)
@@ -1933,11 +1931,8 @@ void CoinJoinWalletManager::DoMaintenance(CConnman& connman)
 }
 
 void CoinJoinWalletManager::Remove(const std::string& name) {
-    {
-        LOCK(cs_wallet_manager_map);
-        m_wallet_manager_map.erase(name);
-    }
-    g_wallet_init_interface.InitCoinJoinSettings(*this);
+    LOCK(cs_wallet_manager_map);
+    m_wallet_manager_map.erase(name);
 }
 
 void CoinJoinWalletManager::Flush(const std::string& name)

src/coinjoin/client.h

@@ -327,7 +327,7 @@ class CCoinJoinClientManager
     bool IsMixing() const;
     void ResetPool() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
 
-    bilingual_str GetStatuses() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
+    std::vector<std::string> GetStatuses() const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
     std::string GetSessionDenoms() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
 
     bool GetMixingMasternodesInfo(std::vector<CDeterministicMNCPtr>& vecDmnsRet) const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);

src/coinjoin/interfaces.cpp

@@ -5,7 +5,14 @@
 #include <interfaces/coinjoin.h>
 
 #include <coinjoin/client.h>
+#include <coinjoin/context.h>
+#include <coinjoin/options.h>
+#include <node/context.h>
+#include <util/check.h>
 #include <wallet/wallet.h>
+#include <walletinitinterface.h>
+
+#include <univalue.h>
 
 #include <memory>
 #include <string>
@@ -37,10 +44,18 @@ class CoinJoinClientImpl : public interfaces::CoinJoin::Client
     {
         return m_clientman.nCachedNumBlocks;
     }
+    void getJsonInfo(UniValue& obj) override
+    {
+        return m_clientman.GetJsonInfo(obj);
+    }
     std::string getSessionDenoms() override
     {
         return m_clientman.GetSessionDenoms();
     }
+    std::vector<std::string> getSessionStatuses() override
+    {
+        return m_clientman.GetStatuses();
+    }
     void setCachedBlocks(int nCachedBlocks) override
     {
        m_clientman.nCachedNumBlocks = nCachedBlocks;
@@ -61,35 +76,51 @@ class CoinJoinClientImpl : public interfaces::CoinJoin::Client
 
 class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader
 {
-    CoinJoinWalletManager& m_walletman;
+private:
+    CoinJoinWalletManager& walletman()
+    {
+        return *Assert(Assert(m_node.cj_ctx)->walletman);
+    }
+
+    interfaces::WalletLoader& wallet_loader()
+    {
+        return *Assert(m_node.wallet_loader);
+    }
 
 public:
-    explicit CoinJoinLoaderImpl(CoinJoinWalletManager& walletman)
-        : m_walletman(walletman) {}
+    explicit CoinJoinLoaderImpl(NodeContext& node) :
+        m_node(node)
+    {
+        // Enablement will be re-evaluated when a wallet is added or removed
+        CCoinJoinClientOptions::SetEnabled(false);
+    }
 
-    void AddWallet(const std::shared_ptr<CWallet>& wallet) override { m_walletman.Add(wallet); }
+    void AddWallet(const std::shared_ptr<CWallet>& wallet) override
+    {
+        walletman().Add(wallet);
+        g_wallet_init_interface.InitCoinJoinSettings(*this, wallet_loader());
+    }
     void RemoveWallet(const std::string& name) override
     {
-        m_walletman.Remove(name);
+        walletman().Remove(name);
+        g_wallet_init_interface.InitCoinJoinSettings(*this, wallet_loader());
     }
     void FlushWallet(const std::string& name) override
     {
-        m_walletman.Flush(name);
+        walletman().Flush(name);
     }
     std::unique_ptr<interfaces::CoinJoin::Client> GetClient(const std::string& name) override
     {
-        auto clientman = m_walletman.Get(name);
+        auto clientman = walletman().Get(name);
         return clientman ? std::make_unique<CoinJoinClientImpl>(*clientman) : nullptr;
     }
-    CoinJoinWalletManager& walletman() override
-    {
-        return m_walletman;
-    }
+
+    NodeContext& m_node;
 };
 
 } // namespace
 } // namespace coinjoin
 
 namespace interfaces {
-std::unique_ptr<CoinJoin::Loader> MakeCoinJoinLoader(CoinJoinWalletManager& walletman) { return std::make_unique<coinjoin::CoinJoinLoaderImpl>(walletman); }
+std::unique_ptr<CoinJoin::Loader> MakeCoinJoinLoader(NodeContext& node) { return std::make_unique<coinjoin::CoinJoinLoaderImpl>(node); }
 } // namespace interfaces

src/dummywallet.cpp

@@ -14,6 +14,7 @@ class Chain;
 class Handler;
 class Wallet;
 class WalletClient;
+class WalletLoader;
 namespace CoinJoin {
 class Loader;
 } // namespcae CoinJoin
@@ -28,8 +29,8 @@ class DummyWalletInit : public WalletInitInterface {
     void Construct(NodeContext& node) const override {LogPrintf("No wallet support compiled in!\n");}
 
     // Dash Specific WalletInitInterface InitCoinJoinSettings
-    void AutoLockMasternodeCollaterals() const override {}
-    void InitCoinJoinSettings(const CoinJoinWalletManager& cjwalletman) const override {}
+    void AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_loader) const override {}
+    void InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loader, interfaces::WalletLoader& wallet_loader) const override {}
     bool InitAutoBackup() const override {return true;}
 };
 
@@ -80,12 +81,17 @@ const WalletInitInterface& g_wallet_init_interface = DummyWalletInit();
 
 namespace interfaces {
 
-std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet, const CoinJoinWalletManager& cjwalletman)
+std::unique_ptr<CoinJoin::Loader> MakeCoinJoinLoader(NodeContext& node)
 {
     throw std::logic_error("Wallet function called in non-wallet build.");
 }
 
-std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, const std::unique_ptr<interfaces::CoinJoin::Loader>& coinjoin_loader, ArgsManager& args)
+std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet)
+{
+    throw std::logic_error("Wallet function called in non-wallet build.");
+}
+
+std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args, interfaces::CoinJoin::Loader& coinjoin_loader)
 {
     throw std::logic_error("Wallet function called in non-wallet build.");
 }

src/init.cpp

@@ -2028,11 +2028,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                                               node.mn_activeman.get(), *node.mn_sync, *node.llmq_ctx->isman, node.peerman,
                                               !ignores_incoming_txs);
 
-#ifdef ENABLE_WALLET
-    node.coinjoin_loader = interfaces::MakeCoinJoinLoader(*node.cj_ctx->walletman);
-    g_wallet_init_interface.InitCoinJoinSettings(*node.cj_ctx->walletman);
-#endif // ENABLE_WALLET
-
     // ********************************************************* Step 7d: Setup other Dash services
 
     bool fLoadCacheFiles = !(fReindex || fReindexChainState) && (chainman.ActiveChain().Tip() != nullptr);
@@ -2207,6 +2202,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &args, &chainman, &node] {
         ThreadImport(chainman, *node.dmnman, *g_ds_notification_interface, vImportFiles, node.mn_activeman.get(), args);
     });
+#ifdef ENABLE_WALLET
+    if (!args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
+        g_wallet_init_interface.AutoLockMasternodeCollaterals(*node.wallet_loader);
+    }
+#endif // ENABLE_WALLET
 
     // Wait for genesis block to be processed
     {

src/init/bitcoin-node.cpp

@@ -3,12 +3,14 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <interfaces/chain.h>
+#include <interfaces/coinjoin.h>
 #include <interfaces/echo.h>
 #include <interfaces/init.h>
 #include <interfaces/ipc.h>
 #include <interfaces/node.h>
 #include <interfaces/wallet.h>
 #include <node/context.h>
+#include <util/check.h>
 #include <util/system.h>
 
 #include <memory>
@@ -29,9 +31,13 @@ class BitcoinNodeInit : public interfaces::Init
     }
     std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); }
     std::unique_ptr<interfaces::Chain> makeChain() override { return interfaces::MakeChain(m_node); }
-    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr<interfaces::CoinJoin::Loader>& loader) override
+    std::unique_ptr<interfaces::CoinJoin::Loader> makeCoinJoinLoader() override
     {
-        return MakeWalletLoader(chain, loader, *Assert(m_node.args));
+        return interfaces::MakeCoinJoinLoader(m_node);
+    }
+    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
+    {
+        return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     interfaces::Ipc* ipc() override { return m_ipc.get(); }

src/init/bitcoind.cpp

@@ -3,11 +3,13 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <interfaces/chain.h>
+#include <interfaces/coinjoin.h>
 #include <interfaces/echo.h>
 #include <interfaces/init.h>
 #include <interfaces/node.h>
 #include <interfaces/wallet.h>
 #include <node/context.h>
+#include <util/check.h>
 #include <util/system.h>
 
 #include <memory>
@@ -24,9 +26,13 @@ class BitcoindInit : public interfaces::Init
     }
     std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); }
     std::unique_ptr<interfaces::Chain> makeChain() override { return interfaces::MakeChain(m_node); }
-    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr<interfaces::CoinJoin::Loader>& loader) override
+    std::unique_ptr<interfaces::CoinJoin::Loader> makeCoinJoinLoader() override
     {
-        return MakeWalletLoader(chain, loader, *Assert(m_node.args));
+        return interfaces::MakeCoinJoinLoader(m_node);
+    }
+    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
+    {
+        return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     NodeContext& m_node;

src/interfaces/coinjoin.h

@@ -7,9 +7,12 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
-class CoinJoinWalletManager;
 class CWallet;
+struct NodeContext;
+
+class UniValue;
 
 namespace interfaces {
 namespace CoinJoin {
@@ -21,6 +24,8 @@ class Client
     virtual void resetCachedBlocks() = 0;
     virtual void resetPool() = 0;
     virtual int getCachedBlocks() = 0;
+    virtual void getJsonInfo(UniValue& obj) = 0;
+    virtual std::vector<std::string> getSessionStatuses() = 0;
     virtual std::string getSessionDenoms() = 0;
     virtual void setCachedBlocks(int nCachedBlocks) = 0;
     virtual void disableAutobackups() = 0;
@@ -38,11 +43,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::Loader> MakeCoinJoinLoader(CoinJoinWalletManager& walletman);
+std::unique_ptr<CoinJoin::Loader> MakeCoinJoinLoader(NodeContext& node);
 
 } // namespace interfaces
 

src/interfaces/init.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <interfaces/chain.h>
+#include <interfaces/coinjoin.h>
 #include <interfaces/echo.h>
 #include <interfaces/init.h>
 #include <interfaces/node.h>
@@ -11,7 +12,8 @@
 namespace interfaces {
 std::unique_ptr<Node> Init::makeNode() { return {}; }
 std::unique_ptr<Chain> Init::makeChain() { return {}; }
-std::unique_ptr<WalletLoader> Init::makeWalletLoader(Chain& chain, const std::unique_ptr<CoinJoin::Loader>&) { return {}; }
+std::unique_ptr<CoinJoin::Loader> Init::makeCoinJoinLoader() { return {}; }
+std::unique_ptr<WalletLoader> Init::makeWalletLoader(Chain& chain, CoinJoin::Loader& coinjoin_loader) { return {}; }
 std::unique_ptr<Echo> Init::makeEcho() { return {}; }
 Ipc* Init::ipc() { return nullptr; }
 } // namespace interfaces

src/interfaces/init.h

@@ -15,11 +15,9 @@ class Echo;
 class Ipc;
 class Node;
 class WalletLoader;
-
-namespace CoinJoin
-{
-    class Loader;
-}
+namespace CoinJoin {
+class Loader;
+} // namespace CoinJoin
 
 //! Initial interface created when a process is first started, and used to give
 //! and get access to other interfaces (Node, Chain, Wallet, etc).
@@ -34,7 +32,8 @@ class Init
     virtual ~Init() = default;
     virtual std::unique_ptr<Node> makeNode();
     virtual std::unique_ptr<Chain> makeChain();
-    virtual std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain&, const std::unique_ptr<CoinJoin::Loader>&);
+    virtual std::unique_ptr<CoinJoin::Loader> makeCoinJoinLoader();
+    virtual std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain&, CoinJoin::Loader&);
     virtual std::unique_ptr<Echo> makeEcho();
     virtual Ipc* ipc();
 };