diff --git a/doc/release-notes-6594.md b/doc/release-notes-6594.md new file mode 100644 index 0000000000000..2f28971807229 --- /dev/null +++ b/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. diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index 5e17e398b1be9..ec3b0a27a6dc2 100644 --- a/src/Makefile.test_util.include +++ b/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 diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 9becb2b9eea2a..a7e76fd10c100 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -350,17 +350,18 @@ bilingual_str CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const } } -bilingual_str CCoinJoinClientManager::GetStatuses() +std::vector CCoinJoinClientManager::GetStatuses() const { - bilingual_str strStatus; - bool fWaitForBlock = WaitForAnotherBlock(); - AssertLockNotHeld(cs_deqsessions); + + bool fWaitForBlock{WaitForAnotherBlock()}; + std::vector 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& wallet) { - { - LOCK(cs_wallet_manager_map); - m_wallet_manager_map.try_emplace(wallet->GetName(), - std::make_unique(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(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) diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index dad2fd7638228..6c7e3205ee3b0 100644 --- a/src/coinjoin/client.h +++ b/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 GetStatuses() const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); std::string GetSessionDenoms() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); bool GetMixingMasternodesInfo(std::vector& vecDmnsRet) const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index 6c40a3b6d3e07..acb9565974e02 100644 --- a/src/coinjoin/interfaces.cpp +++ b/src/coinjoin/interfaces.cpp @@ -5,7 +5,14 @@ #include #include +#include +#include +#include +#include #include +#include + +#include #include #include @@ -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 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& wallet) override { m_walletman.Add(wallet); } + void AddWallet(const std::shared_ptr& 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 GetClient(const std::string& name) override { - auto clientman = m_walletman.Get(name); + auto clientman = walletman().Get(name); return clientman ? std::make_unique(*clientman) : nullptr; } - CoinJoinWalletManager& walletman() override - { - return m_walletman; - } + + NodeContext& m_node; }; } // namespace } // namespace coinjoin namespace interfaces { -std::unique_ptr MakeCoinJoinLoader(CoinJoinWalletManager& walletman) { return std::make_unique(walletman); } +std::unique_ptr MakeCoinJoinLoader(NodeContext& node) { return std::make_unique(node); } } // namespace interfaces diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 10cd516a00f30..cc8e3c5af24d8 100644 --- a/src/dummywallet.cpp +++ b/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 MakeWallet(const std::shared_ptr& wallet, const CoinJoinWalletManager& cjwalletman) +std::unique_ptr MakeCoinJoinLoader(NodeContext& node) { throw std::logic_error("Wallet function called in non-wallet build."); } -std::unique_ptr MakeWalletLoader(Chain& chain, const std::unique_ptr& coinjoin_loader, ArgsManager& args) +std::unique_ptr MakeWallet(const std::shared_ptr& wallet) +{ + throw std::logic_error("Wallet function called in non-wallet build."); +} + +std::unique_ptr MakeWalletLoader(Chain& chain, ArgsManager& args, interfaces::CoinJoin::Loader& coinjoin_loader) { throw std::logic_error("Wallet function called in non-wallet build."); } diff --git a/src/init.cpp b/src/init.cpp index 7be800f83fb2f..690c5ffe43776 100644 --- a/src/init.cpp +++ b/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 { diff --git a/src/init/bitcoin-node.cpp b/src/init/bitcoin-node.cpp index 7300b36731fce..33a87b355fe41 100644 --- a/src/init/bitcoin-node.cpp +++ b/src/init/bitcoin-node.cpp @@ -3,12 +3,14 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include #include #include #include +#include #include #include @@ -29,9 +31,13 @@ class BitcoinNodeInit : public interfaces::Init } std::unique_ptr makeNode() override { return interfaces::MakeNode(m_node); } std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } - std::unique_ptr makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr& loader) override + std::unique_ptr makeCoinJoinLoader() override { - return MakeWalletLoader(chain, loader, *Assert(m_node.args)); + return interfaces::MakeCoinJoinLoader(m_node); + } + std::unique_ptr makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override + { + return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader); } std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } interfaces::Ipc* ipc() override { return m_ipc.get(); } diff --git a/src/init/bitcoind.cpp b/src/init/bitcoind.cpp index 1a084227ec1fc..f04f89338c633 100644 --- a/src/init/bitcoind.cpp +++ b/src/init/bitcoind.cpp @@ -3,11 +3,13 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include #include #include +#include #include #include @@ -24,9 +26,13 @@ class BitcoindInit : public interfaces::Init } std::unique_ptr makeNode() override { return interfaces::MakeNode(m_node); } std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } - std::unique_ptr makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr& loader) override + std::unique_ptr makeCoinJoinLoader() override { - return MakeWalletLoader(chain, loader, *Assert(m_node.args)); + return interfaces::MakeCoinJoinLoader(m_node); + } + std::unique_ptr makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override + { + return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader); } std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } NodeContext& m_node; diff --git a/src/interfaces/coinjoin.h b/src/interfaces/coinjoin.h index 7f881bf07f5c3..a96c524080e71 100644 --- a/src/interfaces/coinjoin.h +++ b/src/interfaces/coinjoin.h @@ -7,9 +7,12 @@ #include #include +#include -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 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 GetClient(const std::string&) = 0; - virtual CoinJoinWalletManager& walletman() = 0; }; } // namespace CoinJoin -std::unique_ptr MakeCoinJoinLoader(CoinJoinWalletManager& walletman); +std::unique_ptr MakeCoinJoinLoader(NodeContext& node); } // namespace interfaces diff --git a/src/interfaces/init.cpp b/src/interfaces/init.cpp index 8c51948fe8b2d..797049d2438dc 100644 --- a/src/interfaces/init.cpp +++ b/src/interfaces/init.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -11,7 +12,8 @@ namespace interfaces { std::unique_ptr Init::makeNode() { return {}; } std::unique_ptr Init::makeChain() { return {}; } -std::unique_ptr Init::makeWalletLoader(Chain& chain, const std::unique_ptr&) { return {}; } +std::unique_ptr Init::makeCoinJoinLoader() { return {}; } +std::unique_ptr Init::makeWalletLoader(Chain& chain, CoinJoin::Loader& coinjoin_loader) { return {}; } std::unique_ptr Init::makeEcho() { return {}; } Ipc* Init::ipc() { return nullptr; } } // namespace interfaces diff --git a/src/interfaces/init.h b/src/interfaces/init.h index 75779e6e1b1db..c1c6edab81481 100644 --- a/src/interfaces/init.h +++ b/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 makeNode(); virtual std::unique_ptr makeChain(); - virtual std::unique_ptr makeWalletLoader(interfaces::Chain&, const std::unique_ptr&); + virtual std::unique_ptr makeCoinJoinLoader(); + virtual std::unique_ptr makeWalletLoader(interfaces::Chain&, CoinJoin::Loader&); virtual std::unique_ptr makeEcho(); virtual Ipc* ipc(); }; diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index d6a0cf413a680..cfb904e870af9 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -86,6 +86,9 @@ class Wallet //! Abort a rescan. virtual void abortRescan() = 0; + //! Lock masternode collaterals + virtual void autoLockMasternodeCollaterals() = 0; + //! Back up wallet. virtual bool backupWallet(const std::string& filename) = 0; @@ -444,7 +447,7 @@ std::unique_ptr MakeWallet(const std::shared_ptr& wallet); //! Return implementation of ChainClient interface for a wallet loader. This //! function will be undefined in builds where ENABLE_WALLET is false. -std::unique_ptr MakeWalletLoader(Chain& chain, const std::unique_ptr& coinjoin_loader, ArgsManager& args); +std::unique_ptr MakeWalletLoader(Chain& chain, ArgsManager& args, CoinJoin::Loader& coinjoin_loader); } // namespace interfaces diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 0f00280734981..2034846086043 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -926,7 +926,5 @@ void ThreadImport(ChainstateManager& chainman, CDeterministicMNManager& dmnman, mn_activeman->Init(chainman.ActiveTip()); } - g_wallet_init_interface.AutoLockMasternodeCollaterals(); - chainman.ActiveChainstate().LoadMempool(args); } diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index ede8755afffcc..c4a47847aa1c7 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -63,8 +63,6 @@ void EditAddressAndSubmit( void TestAddAddressesToSendBook(interfaces::Node& node) { TestChain100Setup test; - auto wallet_loader = interfaces::MakeWalletLoader(*test.m_node.chain, test.m_node.coinjoin_loader, *Assert(test.m_node.args)); - test.m_node.wallet_loader = wallet_loader.get(); node.setContext(&test.m_node); const std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 4425b91b11118..effb354fd18e1 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -108,8 +108,6 @@ void TestGUI(interfaces::Node& node) for (int i = 0; i < 5; ++i) { test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } - auto wallet_loader = interfaces::MakeWalletLoader(*test.m_node.chain, test.m_node.coinjoin_loader, *Assert(test.m_node.args)); - test.m_node.wallet_loader = wallet_loader.get(); node.setContext(&test.m_node); const std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); AddWallet(wallet); diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 7047b928a8ceb..bd7fe17a012ea 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -45,6 +45,7 @@ static RPCHelpMan coinjoin() return RPCHelpMan{"coinjoin", "\nAvailable commands:\n" " start - Start mixing\n" + " status - Get mixing status\n" " stop - Stop mixing\n" " reset - Reset mixing", { @@ -84,8 +85,8 @@ static RPCHelpMan coinjoin_reset() ValidateCoinJoinArguments(); - auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->walletman().Get(wallet->GetName()); - CHECK_NONFATAL(cj_clientman)->ResetPool(); + auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->GetClient(wallet->GetName()); + CHECK_NONFATAL(cj_clientman)->resetPool(); return "Mixing was reset"; }, @@ -124,16 +125,51 @@ static RPCHelpMan coinjoin_start() throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please unlock wallet for mixing with walletpassphrase first."); } - auto cj_clientman = CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->walletman().Get(wallet->GetName())); - if (!cj_clientman->StartMixing()) { + auto cj_clientman = CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->GetClient(wallet->GetName())); + if (!cj_clientman->startMixing()) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already."); } - ChainstateManager& chainman = EnsureChainman(node); - CTxMemPool& mempool = EnsureMemPool(node); - CConnman& connman = EnsureConnman(node); - bool result = cj_clientman->DoAutomaticDenominating(chainman, connman, mempool); - return "Mixing " + (result ? "started successfully" : ("start failed: " + cj_clientman->GetStatuses().original + ", will retry")); + return "Mixing requested"; +}, + }; +} + +static RPCHelpMan coinjoin_status() +{ + return RPCHelpMan{"coinjoin status", + "\nGet status on CoinJoin mixing sessions\n", + {}, + RPCResult{ + RPCResult::Type::ARR, "", "", + {{RPCResult::Type::STR, "", "Status of mixing session"}}}, + RPCExamples{ + HelpExampleCli("coinjoin status", "") + + HelpExampleRpc("coinjoin status", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + const std::shared_ptr wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + + const NodeContext& node = EnsureAnyNodeContext(request.context); + + if (node.mn_activeman) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Client-side mixing is not supported on masternodes"); + } + + ValidateCoinJoinArguments(); + + auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->GetClient(wallet->GetName()); + if (!CHECK_NONFATAL(cj_clientman)->isMixing()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "No ongoing mix session"); + } + + UniValue ret(UniValue::VARR); + for (auto str_status : cj_clientman->getSessionStatuses()) { + ret.push_back(str_status); + } + return ret; }, }; } @@ -164,13 +200,13 @@ static RPCHelpMan coinjoin_stop() ValidateCoinJoinArguments(); CHECK_NONFATAL(node.coinjoin_loader); - auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName()); + auto cj_clientman = node.coinjoin_loader->GetClient(wallet->GetName()); CHECK_NONFATAL(cj_clientman); - if (!cj_clientman->IsMixing()) { + if (!cj_clientman->isMixing()) { throw JSONRPCError(RPC_INTERNAL_ERROR, "No mix session to stop"); } - cj_clientman->StopMixing(); + cj_clientman->stopMixing(); return "Mixing was stopped"; }, @@ -234,8 +270,8 @@ static RPCHelpMan coinjoinsalt_generate() const NodeContext& node = EnsureAnyNodeContext(request.context); if (node.coinjoin_loader != nullptr) { - auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName()); - if (cj_clientman != nullptr && cj_clientman->IsMixing()) { + auto cj_clientman = node.coinjoin_loader->GetClient(wallet->GetName()); + if (cj_clientman != nullptr && cj_clientman->isMixing()) { throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Wallet \"%s\" is currently mixing, cannot change salt!", str_wallet)); } @@ -336,8 +372,8 @@ static RPCHelpMan coinjoinsalt_set() const NodeContext& node = EnsureAnyNodeContext(request.context); if (node.coinjoin_loader != nullptr) { - auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName()); - if (cj_clientman != nullptr && cj_clientman->IsMixing()) { + auto cj_clientman = node.coinjoin_loader->GetClient(wallet->GetName()); + if (cj_clientman != nullptr && cj_clientman->isMixing()) { throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Wallet \"%s\" is currently mixing, cannot change salt!", str_wallet)); } @@ -431,8 +467,8 @@ static RPCHelpMan getcoinjoininfo() return obj; } - auto* manager = CHECK_NONFATAL(node.coinjoin_loader->walletman().Get(wallet->GetName())); - manager->GetJsonInfo(obj); + auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->GetClient(wallet->GetName()); + CHECK_NONFATAL(cj_clientman)->getJsonInfo(obj); std::string warning_msg{""}; if (wallet->IsLegacy()) { @@ -460,6 +496,7 @@ static const CRPCCommand commands[] = { "dash", &coinjoin, }, { "dash", &coinjoin_reset, }, { "dash", &coinjoin_start, }, + { "dash", &coinjoin_status, }, { "dash", &coinjoin_stop, }, { "dash", &coinjoinsalt, }, { "dash", &coinjoinsalt_generate, }, diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 4014ad6847fed..83f3b4f164cbb 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -63,6 +63,7 @@ #ifdef ENABLE_WALLET #include +#include #endif // ENABLE_WALLET #include @@ -118,24 +119,6 @@ void DashChainstateSetupClose(NodeContext& node) Assert(node.mempool.get())); } -void DashPostChainstateSetup(NodeContext& node) -{ - node.cj_ctx = std::make_unique(*node.chainman, *node.connman, *node.dmnman, *node.mn_metaman, *node.mempool, - /*mn_activeman=*/nullptr, *node.mn_sync, *node.llmq_ctx->isman, node.peerman, - /*relay_txes=*/true); -#ifdef ENABLE_WALLET - node.coinjoin_loader = interfaces::MakeCoinJoinLoader(*node.cj_ctx->walletman); -#endif // ENABLE_WALLET -} - -void DashPostChainstateSetupClose(NodeContext& node) -{ -#ifdef ENABLE_WALLET - node.coinjoin_loader.reset(); -#endif // ENABLE_WALLET - node.cj_ctx.reset(); -} - BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::vector& extra_args) : m_path_root{fs::temp_directory_path() / "test_common_" PACKAGE_NAME / g_insecure_rand_ctx_temp_path.rand256().ToString()}, m_args{} @@ -330,7 +313,20 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vectorInit(options); } - DashPostChainstateSetup(m_node); + m_node.cj_ctx = std::make_unique(*m_node.chainman, *m_node.connman, *m_node.dmnman, *m_node.mn_metaman, *m_node.mempool, + /*mn_activeman=*/nullptr, *m_node.mn_sync, *m_node.llmq_ctx->isman, m_node.peerman, + /*relay_txes=*/true); + +#ifdef ENABLE_WALLET + // WalletInit::Construct()-like logic needed for wallet tests that run on + // TestingSetup and its children (e.g. TestChain100Setup) instead of + // WalletTestingSetup + m_node.coinjoin_loader = interfaces::MakeCoinJoinLoader(m_node); + + auto wallet_loader = interfaces::MakeWalletLoader(*m_node.chain, *m_node.args, *m_node.coinjoin_loader); + m_node.wallet_loader = wallet_loader.get(); + m_node.chain_clients.emplace_back(std::move(wallet_loader)); +#endif // ENABLE_WALLET BlockValidationState state; if (!m_node.chainman->ActiveChainstate().ActivateBestChain(state)) { @@ -340,7 +336,15 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vectorInterrupt(); m_node.llmq_ctx->Stop(); @@ -85,7 +83,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) chainstates.push_back(&c2); DashChainstateSetup(manager, m_node, /*fReset=*/false, /*fReindexChainState=*/false, consensus_params); - DashPostChainstateSetup(m_node); BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash); @@ -120,7 +117,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Let scheduler events finish running to avoid accessing memory that is going to be unloaded SyncWithValidationInterfaceQueue(); - DashPostChainstateSetupClose(m_node); if (m_node.llmq_ctx) { m_node.llmq_ctx->Interrupt(); m_node.llmq_ctx->Stop(); diff --git a/src/wallet/context.cpp b/src/wallet/context.cpp index d0cd59c8e5183..09b2f30467099 100644 --- a/src/wallet/context.cpp +++ b/src/wallet/context.cpp @@ -4,7 +4,5 @@ #include -WalletContext::WalletContext(const std::unique_ptr& coinjoin_loader) : - m_coinjoin_loader(coinjoin_loader) -{} +WalletContext::WalletContext() {} WalletContext::~WalletContext() {} diff --git a/src/wallet/context.h b/src/wallet/context.h index 059b7f1062395..d3ceba7cfd079 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -28,14 +28,12 @@ class Loader; struct WalletContext { interfaces::Chain* chain{nullptr}; ArgsManager* args{nullptr}; - // TODO: replace this unique_ptr to a pointer - // probably possible to do after bitcoin/bitcoin#22219 - const std::unique_ptr& m_coinjoin_loader; + interfaces::CoinJoin::Loader* coinjoin_loader{nullptr}; //! Declare default constructor and destructor that are not inline, so code //! instantiating the WalletContext struct doesn't need to #include class //! definitions for smart pointer and container members. - WalletContext(const std::unique_ptr& coinjoin_loader); + WalletContext(); ~WalletContext(); }; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index a141317d9f2fc..2cd6b24633f22 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -46,8 +47,8 @@ class WalletInit : public WalletInitInterface void Construct(NodeContext& node) const override; // Dash Specific Wallet Init - 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; }; @@ -188,34 +189,37 @@ void WalletInit::Construct(NodeContext& node) const LogPrintf("Wallet disabled!\n"); return; } - auto wallet_loader = node.init->makeWalletLoader(*node.chain, node.coinjoin_loader); + node.coinjoin_loader = node.init->makeCoinJoinLoader(); + auto wallet_loader = node.init->makeWalletLoader(*node.chain, *node.coinjoin_loader); node.wallet_loader = wallet_loader.get(); node.chain_clients.emplace_back(std::move(wallet_loader)); } -void WalletInit::AutoLockMasternodeCollaterals() const +void WalletInit::AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_loader) const { // we can't do this before DIP3 is fully initialized - for (const auto& pwallet : GetWallets()) { - pwallet->AutoLockMasternodeCollaterals(); + for (const auto& wallet : wallet_loader.getWallets()) { + wallet->autoLockMasternodeCollaterals(); } } -void WalletInit::InitCoinJoinSettings(const CoinJoinWalletManager& cjwalletman) const +void WalletInit::InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loader, interfaces::WalletLoader& wallet_loader) const { - CCoinJoinClientOptions::SetEnabled(!GetWallets().empty() ? gArgs.GetBoolArg("-enablecoinjoin", true) : false); + const auto& wallets{wallet_loader.getWallets()}; + CCoinJoinClientOptions::SetEnabled(!wallets.empty() ? gArgs.GetBoolArg("-enablecoinjoin", true) : false); if (!CCoinJoinClientOptions::IsEnabled()) { return; } bool fAutoStart = gArgs.GetBoolArg("-coinjoinautostart", DEFAULT_COINJOIN_AUTOSTART); - for (auto& pwallet : GetWallets()) { - auto manager = cjwalletman.Get(pwallet->GetName()); - assert(manager != nullptr); - if (pwallet->IsLocked()) { - manager->StopMixing(); + for (auto& wallet : wallets) { + auto manager = Assert(coinjoin_loader.GetClient(wallet->getWalletName())); + if (wallet->isLocked(/*fForMixing=*/false)) { + manager->stopMixing(); + LogPrintf("CoinJoin: Mixing stopped for locked wallet \"%s\"\n", wallet->getWalletName()); } else if (fAutoStart) { - manager->StartMixing(); + manager->startMixing(); + LogPrintf("CoinJoin: Automatic mixing started for wallet \"%s\"\n", wallet->getWalletName()); } } LogPrintf("CoinJoin: autostart=%d, multisession=%d," /* Continued */ diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 67a2e5467a5af..c6450bcd619e2 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -147,6 +147,7 @@ class WalletImpl : public Wallet return m_wallet->ChangeWalletPassphrase(old_wallet_passphrase, new_wallet_passphrase); } void abortRescan() override { m_wallet->AbortRescan(); } + void autoLockMasternodeCollaterals() override { m_wallet->AutoLockMasternodeCollaterals(); } bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); } bool autoBackupWallet(const fs::path& wallet_path, bilingual_str& error_string, std::vector& warnings) override { @@ -553,11 +554,11 @@ class WalletImpl : public Wallet class WalletLoaderImpl : public WalletLoader { public: - WalletLoaderImpl(Chain& chain, const std::unique_ptr& coinjoin_loader, ArgsManager& args) : - m_context(coinjoin_loader) + WalletLoaderImpl(Chain& chain, ArgsManager& args, interfaces::CoinJoin::Loader& coinjoin_loader) { m_context.chain = &chain; m_context.args = &args; + m_context.coinjoin_loader = &coinjoin_loader; } ~WalletLoaderImpl() override { UnloadWallets(); } @@ -574,7 +575,7 @@ class WalletLoaderImpl : public WalletLoader } } bool verify() override { return VerifyWallets(*m_context.chain); } - bool load() override { assert(m_context.m_coinjoin_loader); return LoadWallets(*m_context.chain, *m_context.m_coinjoin_loader); } + bool load() override { return LoadWallets(*m_context.chain, *m_context.coinjoin_loader); } void start(CScheduler& scheduler) override { return StartWallets(scheduler, *Assert(m_context.args)); } void flush() override { return FlushWallets(); } void stop() override { return StopWallets(); } @@ -589,22 +590,19 @@ class WalletLoaderImpl : public WalletLoader options.require_create = true; options.create_flags = wallet_creation_flags; options.create_passphrase = passphrase; - assert(m_context.m_coinjoin_loader); - return MakeWallet(CreateWallet(*m_context.chain, *m_context.m_coinjoin_loader, name, true /* load_on_start */, options, status, error, warnings)); + return MakeWallet(CreateWallet(*m_context.chain, *m_context.coinjoin_loader, name, true /* load_on_start */, options, status, error, warnings)); } std::unique_ptr loadWallet(const std::string& name, bilingual_str& error, std::vector& warnings) override { DatabaseOptions options; DatabaseStatus status; options.require_existing = true; - assert(m_context.m_coinjoin_loader); - return MakeWallet(LoadWallet(*m_context.chain, *m_context.m_coinjoin_loader, name, true /* load_on_start */, options, status, error, warnings)); + return MakeWallet(LoadWallet(*m_context.chain, *m_context.coinjoin_loader, name, true /* load_on_start */, options, status, error, warnings)); } std::unique_ptr restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector& warnings) override { DatabaseStatus status; - assert(m_context.m_coinjoin_loader); - return MakeWallet(RestoreWallet(*m_context.chain, *m_context.m_coinjoin_loader, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings)); + return MakeWallet(RestoreWallet(*m_context.chain, *m_context.coinjoin_loader, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings)); } std::string getWalletDir() override { @@ -641,7 +639,7 @@ class WalletLoaderImpl : public WalletLoader namespace interfaces { std::unique_ptr MakeWallet(const std::shared_ptr& wallet) { return wallet ? std::make_unique(wallet) : nullptr; } -std::unique_ptr MakeWalletLoader(Chain& chain, const std::unique_ptr& coinjoin_loader, ArgsManager& args) { - return std::make_unique(chain, coinjoin_loader, args); +std::unique_ptr MakeWalletLoader(Chain& chain, ArgsManager& args, interfaces::CoinJoin::Loader& coinjoin_loader) { + return std::make_unique(chain, args, coinjoin_loader); } } // namespace interfaces diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5aa51e8ca6dbd..1ceac9fd32b13 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2747,7 +2747,7 @@ static RPCHelpMan loadwallet() bilingual_str error; std::vector warnings; std::optional load_on_start = request.params[1].isNull() ? std::nullopt : std::optional(request.params[1].get_bool()); - std::shared_ptr const wallet = LoadWallet(*context.chain, *context.m_coinjoin_loader, name, load_on_start, options, status, error, warnings); + std::shared_ptr const wallet = LoadWallet(*context.chain, *context.coinjoin_loader, name, load_on_start, options, status, error, warnings); HandleWalletError(wallet, status, error); @@ -2903,7 +2903,7 @@ static RPCHelpMan createwallet() options.create_passphrase = passphrase; bilingual_str error; std::optional load_on_start = request.params[6].isNull() ? std::nullopt : std::optional(request.params[6].get_bool()); - const std::shared_ptr wallet = CreateWallet(*context.chain, *context.m_coinjoin_loader, request.params[0].get_str(), load_on_start, options, status, error, warnings); + const std::shared_ptr wallet = CreateWallet(*context.chain, *context.coinjoin_loader, request.params[0].get_str(), load_on_start, options, status, error, warnings); if (!wallet) { RPCErrorCode code = status == DatabaseStatus::FAILED_ENCRYPT ? RPC_WALLET_ENCRYPTION_FAILED : RPC_WALLET_ERROR; throw JSONRPCError(code, error.original); @@ -2957,7 +2957,7 @@ static RPCHelpMan restorewallet() bilingual_str error; std::vector warnings; - const std::shared_ptr wallet = RestoreWallet(*context.chain, *context.m_coinjoin_loader, backup_file, wallet_name, load_on_start, status, error, warnings); + const std::shared_ptr wallet = RestoreWallet(*context.chain, *context.coinjoin_loader, backup_file, wallet_name, load_on_start, status, error, warnings); HandleWalletError(wallet, status, error); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 55d3e0b5d8028..43ccd5729ae12 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -14,7 +14,8 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - m_wallet_loader = MakeWalletLoader(*m_node.chain, m_node.coinjoin_loader, *Assert(m_node.args)); + m_coinjoin_loader = interfaces::MakeCoinJoinLoader(m_node); + m_wallet_loader = MakeWalletLoader(*m_node.chain, *Assert(m_node.args), *Assert(m_coinjoin_loader)); std::string sep; sep += fs::path::preferred_separator; diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 93f635da79a9c..534853200e643 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -8,8 +8,9 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), - m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, m_node.coinjoin_loader, *Assert(m_node.args))}, - m_wallet(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()) + m_coinjoin_loader{interfaces::MakeCoinJoinLoader(m_node)}, + m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args), *Assert(m_coinjoin_loader))}, + m_wallet(m_node.chain.get(), m_coinjoin_loader.get(), "", CreateMockWalletDatabase()) { m_wallet.LoadWallet(); m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 752c8940888ac..c72693d1a7741 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -21,6 +22,7 @@ struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); + std::unique_ptr m_coinjoin_loader; std::unique_ptr m_wallet_loader; CWallet m_wallet; std::unique_ptr m_chain_notifications_handler; diff --git a/src/walletinitinterface.h b/src/walletinitinterface.h index cb70b76bc649e..60a424d1248b2 100644 --- a/src/walletinitinterface.h +++ b/src/walletinitinterface.h @@ -6,9 +6,13 @@ #define BITCOIN_WALLETINITINTERFACE_H class ArgsManager; -class CoinJoinWalletManager; - struct NodeContext; +namespace interfaces { +class WalletLoader; +namespace CoinJoin { +class Loader; +} // namespace CoinJoin +} // namespace interfaces class WalletInitInterface { public: @@ -22,8 +26,8 @@ class WalletInitInterface { virtual void Construct(NodeContext& node) const = 0; // Dash Specific WalletInitInterface - virtual void AutoLockMasternodeCollaterals() const = 0; - virtual void InitCoinJoinSettings(const CoinJoinWalletManager& cjwalletman) const = 0; + virtual void AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_loader) const = 0; + virtual void InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loader, interfaces::WalletLoader& wallet_loader) const = 0; virtual bool InitAutoBackup() const = 0; virtual ~WalletInitInterface() {} diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 35ee13d5f5cd6..00c5ac8cd99e1 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -66,6 +66,8 @@ def test_coinjoin_start_stop(self, node): assert_equal(cj_info['running'], True) # Repeated start should yield error assert_raises_rpc_error(-32603, 'Mixing has been started already.', node.coinjoin, 'start') + # Requesting status shouldn't complain + node.coinjoin('status') # Stop mix session and ensure it's reported node.coinjoin('stop') @@ -74,6 +76,8 @@ def test_coinjoin_start_stop(self, node): assert_equal(cj_info['running'], False) # Repeated stop should yield error assert_raises_rpc_error(-32603, 'No mix session to stop', node.coinjoin, 'stop') + # Requesting status should tell us off + assert_raises_rpc_error(-32603, 'No ongoing mix session', node.coinjoin, 'status') # Reset mix session assert_equal(node.coinjoin('reset'), "Mixing was reset")