diff --git a/src/Makefile.am b/src/Makefile.am index eae55f110ab2..806af0051955 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -502,7 +502,6 @@ libbitcoin_node_a_SOURCES = \ chainlock/signing.cpp \ coinjoin/coinjoin.cpp \ coinjoin/server.cpp \ - coinjoin/walletman.cpp \ consensus/tx_verify.cpp \ dbwrapper.cpp \ deploymentstatus.cpp \ @@ -659,6 +658,7 @@ libbitcoin_wallet_a_SOURCES = \ coinjoin/client.cpp \ coinjoin/interfaces.cpp \ coinjoin/util.cpp \ + coinjoin/walletman.cpp \ wallet/bip39.cpp \ wallet/coinjoin.cpp \ wallet/coincontrol.cpp \ diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 6474cbd708bb..087a7d586ff0 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -61,8 +61,8 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_cha if (!m_mn_sync.IsBlockchainSynced()) return; if (!CheckDiskSpace(gArgs.GetDataDirNet())) { - ResetPool(); - StopMixing(); + resetPool(); + stopMixing(); WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::ProcessMessage -- Not enough disk space, disabling CoinJoin.\n"); return; } @@ -137,16 +137,16 @@ void CCoinJoinClientSession::ProcessMessage(CNode& peer, CChainState& active_cha } } -bool CCoinJoinClientManager::StartMixing() { +bool CCoinJoinClientManager::startMixing() { bool expected{false}; return fMixing.compare_exchange_strong(expected, true); } -void CCoinJoinClientManager::StopMixing() { +void CCoinJoinClientManager::stopMixing() { fMixing = false; } -bool CCoinJoinClientManager::IsMixing() const +bool CCoinJoinClientManager::isMixing() const { return fMixing; } @@ -159,7 +159,7 @@ void CCoinJoinClientSession::ResetPool() WITH_LOCK(cs_coinjoin, SetNull()); } -void CCoinJoinClientManager::ResetPool() +void CCoinJoinClientManager::resetPool() { nCachedLastSuccessBlock = 0; AssertLockNotHeld(cs_deqsessions); @@ -241,7 +241,7 @@ bilingual_str CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const } } -std::vector CCoinJoinClientManager::GetStatuses() const +std::vector CCoinJoinClientManager::getSessionStatuses() const { AssertLockNotHeld(cs_deqsessions); @@ -255,7 +255,7 @@ std::vector CCoinJoinClientManager::GetStatuses() const return ret; } -std::string CCoinJoinClientManager::GetSessionDenoms() +std::string CCoinJoinClientManager::getSessionDenoms() const { std::string strSessionDenoms; @@ -328,7 +328,7 @@ void CCoinJoinClientManager::CheckTimeout() { AssertLockNotHeld(cs_deqsessions); - if (!CCoinJoinClientOptions::IsEnabled() || !IsMixing()) return; + if (!CCoinJoinClientOptions::IsEnabled() || !isMixing()) return; LOCK(cs_deqsessions); for (auto& session : deqSessions) { @@ -605,7 +605,7 @@ bool CCoinJoinClientManager::WaitForAnotherBlock() const bool CCoinJoinClientManager::CheckAutomaticBackup() { - if (!CCoinJoinClientOptions::IsEnabled() || !IsMixing()) return false; + if (!CCoinJoinClientOptions::IsEnabled() || !isMixing()) return false; // We don't need auto-backups for descriptor wallets if (!m_wallet->IsLegacy()) return true; @@ -614,7 +614,7 @@ bool CCoinJoinClientManager::CheckAutomaticBackup() case 0: strAutoDenomResult = _("Automatic backups disabled") + Untranslated(", ") + _("no mixing available."); WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original); - StopMixing(); + stopMixing(); m_wallet->nKeysLeftSinceAutoBackup = 0; // no backup, no "keys since last backup" return false; case -1: @@ -640,7 +640,7 @@ bool CCoinJoinClientManager::CheckAutomaticBackup() m_wallet->nKeysLeftSinceAutoBackup); WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original); // It's getting really dangerous, stop mixing - StopMixing(); + stopMixing(); return false; } else if (m_wallet->nKeysLeftSinceAutoBackup < COINJOIN_KEYS_THRESHOLD_WARNING) { // Low number of keys left, but it's still more or less safe to continue @@ -862,7 +862,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman, CConnman& connman, const CTxMemPool& mempool, bool fDryRun) { - if (!CCoinJoinClientOptions::IsEnabled() || !IsMixing()) return false; + if (!CCoinJoinClientOptions::IsEnabled() || !isMixing()) return false; if (!m_mn_sync.IsBlockchainSynced()) { strAutoDenomResult = _("Can't mix while sync in progress."); @@ -1765,10 +1765,10 @@ void CCoinJoinClientSession::GetJsonInfo(UniValue& obj) const obj.pushKV("entries_count", GetEntriesCount()); } -void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const +UniValue CCoinJoinClientManager::getJsonInfo() const { - assert(obj.isObject()); - obj.pushKV("running", IsMixing()); + UniValue obj(UniValue::VOBJ); + obj.pushKV("running", isMixing()); UniValue arrSessions(UniValue::VARR); AssertLockNotHeld(cs_deqsessions); @@ -1781,5 +1781,6 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const } } obj.pushKV("sessions", arrSessions); + return obj; } diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 3785d307099e..dc5c2cc9d187 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -154,7 +155,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession /** Used to keep track of current status of mixing pool */ -class CCoinJoinClientManager +class CCoinJoinClientManager : public interfaces::CoinJoin::Client { private: const std::shared_ptr m_wallet; @@ -178,15 +179,15 @@ class CCoinJoinClientManager // Keep track of current block height int nCachedBlockHeight{0}; + int nCachedNumBlocks{std::numeric_limits::max()}; // used for the overview screen + bool fCreateAutoBackups{true}; // builtin support for automatic backups + bool WaitForAnotherBlock() const; // Make sure we have enough keys since last backup bool CheckAutomaticBackup(); public: - int nCachedNumBlocks{std::numeric_limits::max()}; // used for the overview screen - bool fCreateAutoBackups{true}; // builtin support for automatic backups - CCoinJoinClientManager() = delete; CCoinJoinClientManager(const CCoinJoinClientManager&) = delete; CCoinJoinClientManager& operator=(const CCoinJoinClientManager&) = delete; @@ -197,14 +198,6 @@ class CCoinJoinClientManager void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); - bool StartMixing(); - void StopMixing(); - bool IsMixing() const; - void ResetPool() 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); /// Passively run mixing in the background according to the configuration in settings @@ -229,7 +222,18 @@ class CCoinJoinClientManager void DoMaintenance(ChainstateManager& chainman, CConnman& connman, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); - void GetJsonInfo(UniValue& obj) const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); + // interfaces::CoinJoin::Client overrides + void resetCachedBlocks() override { nCachedNumBlocks = std::numeric_limits::max(); } + int getCachedBlocks() const override { return nCachedNumBlocks; } + void setCachedBlocks(int nCachedBlocks) override { nCachedNumBlocks = nCachedBlocks; } + void disableAutobackups() override { fCreateAutoBackups = false; } + void resetPool() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); + UniValue getJsonInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); + std::vector getSessionStatuses() const override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); + std::string getSessionDenoms() const override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); + bool isMixing() const override; + bool startMixing() override; + void stopMixing() override; }; #endif // BITCOIN_COINJOIN_CLIENT_H diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index 8dd645025561..5ddb127f4217 100644 --- a/src/coinjoin/interfaces.cpp +++ b/src/coinjoin/interfaces.cpp @@ -20,60 +20,6 @@ using wallet::CWallet; namespace coinjoin { namespace { -class CoinJoinClientImpl : public interfaces::CoinJoin::Client -{ - CCoinJoinClientManager& m_clientman; - -public: - explicit CoinJoinClientImpl(CCoinJoinClientManager& clientman) - : m_clientman(clientman) {} - - void resetCachedBlocks() override - { - m_clientman.nCachedNumBlocks = std::numeric_limits::max(); - } - void resetPool() override - { - m_clientman.ResetPool(); - } - void disableAutobackups() override - { - m_clientman.fCreateAutoBackups = false; - } - int getCachedBlocks() override - { - 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; - } - bool isMixing() override - { - return m_clientman.IsMixing(); - } - bool startMixing() override - { - return m_clientman.StartMixing(); - } - void stopMixing() override - { - m_clientman.StopMixing(); - } -}; - class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader { private: @@ -82,37 +28,32 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader return *Assert(m_node.cj_walletman); } - interfaces::WalletLoader& wallet_loader() - { - return *Assert(m_node.wallet_loader); - } - public: explicit CoinJoinLoaderImpl(NodeContext& node) : m_node(node) { - // Enablement will be re-evaluated when a wallet is added or removed - CCoinJoinClientOptions::SetEnabled(false); + CCoinJoinClientOptions::SetEnabled(gArgs.GetBoolArg("-enablecoinjoin", true)); } void AddWallet(const std::shared_ptr& wallet) override { manager().addWallet(wallet); - g_wallet_init_interface.InitCoinJoinSettings(*this, wallet_loader()); + if (!CCoinJoinClientOptions::IsEnabled()) return; + manager().doForClient(wallet->GetName(), [](CCoinJoinClientManager& mgr) { + g_wallet_init_interface.InitCoinJoinSettings(mgr); + }); } void RemoveWallet(const std::string& name) override { manager().removeWallet(name); - g_wallet_init_interface.InitCoinJoinSettings(*this, wallet_loader()); } void FlushWallet(const std::string& name) override { manager().flushWallet(name); } - std::unique_ptr GetClient(const std::string& name) override + bool WithClient(const std::string& name, const std::function& func) override { - auto clientman = manager().getClient(name); - return clientman ? std::make_unique(*clientman) : nullptr; + return manager().doForClient(name, [&](CCoinJoinClientManager& mgr) { func(mgr); }); } NodeContext& m_node; diff --git a/src/coinjoin/walletman.cpp b/src/coinjoin/walletman.cpp index a54ac93795b4..3a89357f3ef2 100644 --- a/src/coinjoin/walletman.cpp +++ b/src/coinjoin/walletman.cpp @@ -39,7 +39,7 @@ class CJWalletManagerImpl final : public CJWalletManager public: bool hasQueue(const uint256& hash) const override; - CCoinJoinClientManager* getClient(const std::string& name) override EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); + bool doForClient(const std::string& name, const std::function& func) override EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); MessageProcessingResult processMessage(CNode& peer, CChainState& chainstate, CConnman& connman, CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) override EXCLUSIVE_LOCKS_REQUIRED(!cs_ProcessDSQueue, !cs_wallet_manager_map); std::optional getQueueFromHash(const uint256& hash) const override; @@ -140,11 +140,13 @@ bool CJWalletManagerImpl::hasQueue(const uint256& hash) const return false; } -CCoinJoinClientManager* CJWalletManagerImpl::getClient(const std::string& name) +bool CJWalletManagerImpl::doForClient(const std::string& name, const std::function& func) { LOCK(cs_wallet_manager_map); auto it = m_wallet_manager_map.find(name); - return (it != m_wallet_manager_map.end()) ? it->second.get() : nullptr; + if (it == m_wallet_manager_map.end()) return false; + func(*it->second); + return true; } std::optional CJWalletManagerImpl::getQueueFromHash(const uint256& hash) const @@ -180,9 +182,10 @@ void CJWalletManagerImpl::addWallet(const std::shared_ptr& wall void CJWalletManagerImpl::flushWallet(const std::string& name) { - auto* clientman = Assert(getClient(name)); - clientman->ResetPool(); - clientman->StopMixing(); + doForClient(name, [](CCoinJoinClientManager& clientman) { + clientman.resetPool(); + clientman.stopMixing(); + }); } void CJWalletManagerImpl::removeWallet(const std::string& name) @@ -303,17 +306,12 @@ MessageProcessingResult CJWalletManagerImpl::ProcessDSQueue(NodeId from, CConnma } // cs_ProcessDSQueue return ret; } -#endif // ENABLE_WALLET std::unique_ptr CJWalletManager::make(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool, const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, bool relay_txes) { -#ifdef ENABLE_WALLET return std::make_unique(chainman, dmnman, mn_metaman, mempool, mn_sync, isman, relay_txes); -#else - // Cannot be constructed if wallet support isn't built - return nullptr; -#endif // ENABLE_WALLET } +#endif // ENABLE_WALLET diff --git a/src/coinjoin/walletman.h b/src/coinjoin/walletman.h index 720a4bdf2f81..5979cdc048f9 100644 --- a/src/coinjoin/walletman.h +++ b/src/coinjoin/walletman.h @@ -10,6 +10,7 @@ #include +#include #include #include @@ -47,7 +48,9 @@ class CJWalletManager : public CValidationInterface public: virtual bool hasQueue(const uint256& hash) const = 0; - virtual CCoinJoinClientManager* getClient(const std::string& name) = 0; + //! Execute func under the wallet manager lock for the client identified by name. + //! Returns true if the client was found and func was called, false otherwise. + virtual bool doForClient(const std::string& name, const std::function& func) = 0; virtual MessageProcessingResult processMessage(CNode& peer, CChainState& chainstate, CConnman& connman, CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) = 0; virtual std::optional getQueueFromHash(const uint256& hash) const = 0; diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 14eb342d596e..dafac923a582 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -34,7 +34,7 @@ class DummyWalletInit : public WalletInitInterface { // Dash Specific WalletInitInterface InitCoinJoinSettings void AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_loader) const override {} - void InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loader, interfaces::WalletLoader& wallet_loader) const override {} + void InitCoinJoinSettings(CCoinJoinClientManager& mgr) const override {} void InitAutoBackup() const override {} }; diff --git a/src/init.cpp b/src/init.cpp index 703c50646cc3..f2094b33dd34 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2223,9 +2223,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.peerman->AddExtraHandler(std::move(cj_server)); } else { assert(!node.cj_walletman); - // Can return nullptr if built without wallet support, must check before use + // Only constructed in wallet-enabled builds; stays null otherwise, must check before use +#ifdef ENABLE_WALLET node.cj_walletman = CJWalletManager::make(chainman, *node.dmnman, *node.mn_metaman, *node.mempool, *node.mn_sync, *node.llmq_ctx->isman, !ignores_incoming_txs); +#endif } if (node.cj_walletman) { diff --git a/src/interfaces/coinjoin.h b/src/interfaces/coinjoin.h index c57de9ec3fe2..16037c086fa6 100644 --- a/src/interfaces/coinjoin.h +++ b/src/interfaces/coinjoin.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_INTERFACES_COINJOIN_H #define BITCOIN_INTERFACES_COINJOIN_H +#include #include #include #include @@ -27,13 +28,13 @@ class Client virtual ~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 int getCachedBlocks() const = 0; + virtual UniValue getJsonInfo() const = 0; + virtual std::vector getSessionStatuses() const = 0; + virtual std::string getSessionDenoms() const = 0; virtual void setCachedBlocks(int nCachedBlocks) = 0; virtual void disableAutobackups() = 0; - virtual bool isMixing() = 0; + virtual bool isMixing() const = 0; virtual bool startMixing() = 0; virtual void stopMixing() = 0; }; @@ -46,7 +47,9 @@ class Loader //! Remove wallet from CoinJoin client manager virtual void RemoveWallet(const std::string&) = 0; virtual void FlushWallet(const std::string&) = 0; - virtual std::unique_ptr GetClient(const std::string&) = 0; + //! Execute a callback with the CoinJoin client for the given wallet, under the wallet manager lock. + //! Returns false if the wallet was not found. + virtual bool WithClient(const std::string& name, const std::function& func) = 0; }; } // namespace CoinJoin diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 8f6800b482ec..096f29c5e8e3 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1608,7 +1608,7 @@ void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, const QStri #ifdef ENABLE_WALLET if (enableWallet) { for (const auto& wallet : m_node.walletLoader().getWallets()) { - disableAppNap |= m_node.coinJoinLoader()->GetClient(wallet->getWalletName())->isMixing(); + m_node.coinJoinLoader()->WithClient(wallet->getWalletName(), [&](auto& client) { disableAppNap |= client.isMixing(); }); } } #endif // ENABLE_WALLET diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 7be7120d9f70..b93c22cd6826 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -459,7 +459,7 @@ void OptionsDialog::on_okButton_clicked() #ifdef ENABLE_WALLET if (m_enable_wallet) { for (auto& wallet : model->node().walletLoader().getWallets()) { - model->node().coinJoinLoader()->GetClient(wallet->getWalletName())->resetCachedBlocks(); + model->node().coinJoinLoader()->WithClient(wallet->getWalletName(), [](auto& client) { client.resetCachedBlocks(); }); wallet->markDirty(); } } diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 7f11fde0f9a8..b657643d4101 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -337,7 +337,7 @@ void OverviewPage::setWalletModel(WalletModel *model) // Disable coinJoinClient builtin support for automatic backups while we are in GUI, // we'll handle automatic backups and user warnings in coinJoinStatus() - walletModel->coinJoin()->disableAutobackups(); + walletModel->withCoinJoin([](auto& client) { client.disableAutobackups(); }); connect(ui->toggleCoinJoin, &QPushButton::clicked, this, &OverviewPage::toggleCoinJoin); @@ -578,7 +578,11 @@ void OverviewPage::coinJoinStatus(bool fForce) int nBestHeight = clientModel->node().getNumBlocks(); // We are processing more than 1 block per second, we'll just leave - if (nBestHeight > walletModel->coinJoin()->getCachedBlocks() && GetTime() - nLastDSProgressBlockTime <= 1) return; + bool tooFast{false}; + walletModel->withCoinJoin([&](auto& client) { + tooFast = nBestHeight > client.getCachedBlocks() && GetTime() - nLastDSProgressBlockTime <= 1; + }); + if (tooFast) return; nLastDSProgressBlockTime = GetTime(); QString strKeysLeftText(tr("keys left: %1").arg(walletModel->getKeysLeftSinceAutoBackup())); @@ -592,12 +596,17 @@ void OverviewPage::coinJoinStatus(bool fForce) ui->labelCoinJoinEnabled->setToolTip(strKeysLeftText); QString strCoinJoinName = QString::fromStdString(gCoinJoinName); - if (!walletModel->coinJoin()->isMixing()) { - if (nBestHeight != walletModel->coinJoin()->getCachedBlocks()) { - walletModel->coinJoin()->setCachedBlocks(nBestHeight); - updateCoinJoinProgress(); + bool notMixing{false}; + bool refreshProgress{false}; + walletModel->withCoinJoin([&](auto& client) { + notMixing = !client.isMixing(); + if (notMixing && nBestHeight != client.getCachedBlocks()) { + client.setCachedBlocks(nBestHeight); + refreshProgress = true; } - + }); + if (refreshProgress) updateCoinJoinProgress(); + if (notMixing) { setWidgetsVisible(false); ui->toggleCoinJoin->setText(tr("Start %1").arg(strCoinJoinName)); @@ -655,7 +664,8 @@ void OverviewPage::coinJoinStatus(bool fForce) } } - QString strEnabled = walletModel->coinJoin()->isMixing() ? tr("Enabled") : tr("Disabled"); + QString strEnabled; + walletModel->withCoinJoin([&](auto& client) { strEnabled = client.isMixing() ? tr("Enabled") : tr("Disabled"); }); // Show how many keys left in advanced PS UI mode only if(fShowAdvancedCJUI && !strKeysLeftText.isEmpty()) strEnabled += ", " + strKeysLeftText; ui->labelCoinJoinEnabled->setText(strEnabled); @@ -679,15 +689,18 @@ void OverviewPage::coinJoinStatus(bool fForce) } // check coinjoin status and unlock if needed - if(nBestHeight != walletModel->coinJoin()->getCachedBlocks()) { - // Balance and number of transactions might have changed - walletModel->coinJoin()->setCachedBlocks(nBestHeight); - updateCoinJoinProgress(); - } + bool refreshProgressTail{false}; + walletModel->withCoinJoin([&](auto& client) { + if (nBestHeight != client.getCachedBlocks()) { + // Balance and number of transactions might have changed + client.setCachedBlocks(nBestHeight); + refreshProgressTail = true; + } + ui->labelSubmittedDenom->setText(m_privacy ? "####" : QString(client.getSessionDenoms().c_str())); + }); + if (refreshProgressTail) updateCoinJoinProgress(); setWidgetsVisible(true); - - ui->labelSubmittedDenom->setText(m_privacy ? "####" : QString(walletModel->coinJoin()->getSessionDenoms().c_str())); } void OverviewPage::toggleCoinJoin(){ @@ -702,7 +715,9 @@ void OverviewPage::toggleCoinJoin(){ settings.setValue("hasMixed", "hasMixed"); } - if (!walletModel->coinJoin()->isMixing()) { + bool mixing{false}; + walletModel->withCoinJoin([&](auto& client) { mixing = client.isMixing(); }); + if (!mixing) { auto& options = walletModel->node().coinJoinOptions(); const CAmount nMinAmount = options.getSmallestDenomination() + options.getMaxCollateralAmount(); if(m_balances.balance < nMinAmount) { @@ -720,7 +735,7 @@ void OverviewPage::toggleCoinJoin(){ if(!ctx.isValid()) { //unlock was cancelled - walletModel->coinJoin()->resetCachedBlocks(); + walletModel->withCoinJoin([](auto& client) { client.resetCachedBlocks(); }); QMessageBox::warning(this, strCoinJoinName, tr("Wallet is locked and user declined to unlock. Disabling %1.").arg(strCoinJoinName), QMessageBox::Ok, QMessageBox::Ok); @@ -731,16 +746,17 @@ void OverviewPage::toggleCoinJoin(){ } - walletModel->coinJoin()->resetCachedBlocks(); - - if (walletModel->coinJoin()->isMixing()) { - ui->toggleCoinJoin->setText(tr("Start %1").arg(strCoinJoinName)); - walletModel->coinJoin()->resetPool(); - walletModel->coinJoin()->stopMixing(); - } else { - ui->toggleCoinJoin->setText(tr("Stop %1").arg(strCoinJoinName)); - walletModel->coinJoin()->startMixing(); - } + walletModel->withCoinJoin([&](auto& client) { + client.resetCachedBlocks(); + if (client.isMixing()) { + ui->toggleCoinJoin->setText(tr("Start %1").arg(strCoinJoinName)); + client.resetPool(); + client.stopMixing(); + } else { + ui->toggleCoinJoin->setText(tr("Stop %1").arg(strCoinJoinName)); + client.startMixing(); + } + }); } void OverviewPage::SetupTransactionList(int nNumItems) @@ -779,5 +795,5 @@ void OverviewPage::DisableCoinJoinCompletely() if (nWalletBackups <= 0) { ui->labelCoinJoinEnabled->setText("(" + tr("Disabled") + ")"); } - walletModel->coinJoin()->stopMixing(); + walletModel->withCoinJoin([](auto& client) { client.stopMixing(); }); } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index f83760c862da..81f6677a8b3f 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -87,9 +87,9 @@ void WalletModel::setClientModel(ClientModel* client_model) if (!m_client_model) timer->stop(); } -std::unique_ptr WalletModel::coinJoin() const +bool WalletModel::withCoinJoin(const std::function& func) const { - return m_node.coinJoinLoader()->GetClient(m_wallet->getWalletName()); + return m_node.coinJoinLoader()->WithClient(m_wallet->getWalletName(), func); } void WalletModel::updateStatus() diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 9249282f13e7..1d5c9eb967e6 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -153,7 +154,7 @@ class WalletModel : public QObject interfaces::Node& node() const { return m_node; } interfaces::Wallet& wallet() const { return *m_wallet; } void setClientModel(ClientModel* client_model); - std::unique_ptr coinJoin() const; + bool withCoinJoin(const std::function& func) const; QString getWalletName() const; QString getDisplayName() const; diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 3a2cc40a6977..b5afdb84df12 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -93,8 +93,9 @@ static RPCHelpMan coinjoin_reset() ValidateCoinJoinArguments(); - auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->GetClient(wallet->GetName()); - CHECK_NONFATAL(cj_clientman)->resetPool(); + CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->WithClient(wallet->GetName(), [](auto& client) { + client.resetPool(); + })); return "Mixing was reset"; }, @@ -133,10 +134,11 @@ 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)->GetClient(wallet->GetName())); - if (!cj_clientman->startMixing()) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already."); - } + CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->WithClient(wallet->GetName(), [](auto& client) { + if (!client.startMixing()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already."); + } + })); return "Mixing requested"; }, @@ -168,15 +170,15 @@ static RPCHelpMan coinjoin_status() 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 (const auto& str_status : cj_clientman->getSessionStatuses()) { - ret.push_back(str_status); - } + CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->WithClient(wallet->GetName(), [&](auto& client) { + if (!client.isMixing()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "No ongoing mix session"); + } + for (const auto& str_status : client.getSessionStatuses()) { + ret.push_back(str_status); + } + })); return ret; }, }; @@ -207,14 +209,12 @@ static RPCHelpMan coinjoin_stop() ValidateCoinJoinArguments(); - CHECK_NONFATAL(node.coinjoin_loader); - auto cj_clientman = node.coinjoin_loader->GetClient(wallet->GetName()); - - CHECK_NONFATAL(cj_clientman); - if (!cj_clientman->isMixing()) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "No mix session to stop"); - } - cj_clientman->stopMixing(); + CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->WithClient(wallet->GetName(), [](auto& client) { + if (!client.isMixing()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "No mix session to stop"); + } + client.stopMixing(); + })); return "Mixing was stopped"; }, @@ -278,11 +278,12 @@ static RPCHelpMan coinjoinsalt_generate() const NodeContext& node = EnsureAnyNodeContext(request.context); if (node.coinjoin_loader != nullptr) { - 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)); - } + node.coinjoin_loader->WithClient(wallet->GetName(), [&](auto& client) { + if (client.isMixing()) { + throw JSONRPCError(RPC_WALLET_ERROR, + strprintf("Wallet \"%s\" is currently mixing, cannot change salt!", str_wallet)); + } + }); } const auto wallet_balance{GetBalance(*wallet)}; @@ -380,11 +381,12 @@ static RPCHelpMan coinjoinsalt_set() const NodeContext& node = EnsureAnyNodeContext(request.context); if (node.coinjoin_loader != nullptr) { - 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)); - } + node.coinjoin_loader->WithClient(wallet->GetName(), [&](auto& client) { + if (client.isMixing()) { + throw JSONRPCError(RPC_WALLET_ERROR, + strprintf("Wallet \"%s\" is currently mixing, cannot change salt!", str_wallet)); + } + }); } const auto wallet_balance{GetBalance(*wallet)}; @@ -484,8 +486,9 @@ static RPCHelpMan getcoinjoininfo() return obj; } - auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->GetClient(wallet->GetName()); - CHECK_NONFATAL(cj_clientman)->getJsonInfo(obj); + CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->WithClient(wallet->GetName(), [&](auto& client) { + obj.pushKVs(client.getJsonInfo()); + })); std::string warning_msg; if (wallet->IsLegacy()) { diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 7bb836c40083..f23068270c17 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -51,7 +51,7 @@ class WalletInit : public WalletInitInterface // Dash Specific Wallet Init void AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_loader) const override; - void InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loader, interfaces::WalletLoader& wallet_loader) const override; + void InitCoinJoinSettings(CCoinJoinClientManager& mgr) const override; void InitAutoBackup() const override; }; @@ -201,23 +201,11 @@ void WalletInit::AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_ } } -void WalletInit::InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loader, interfaces::WalletLoader& wallet_loader) const +void WalletInit::InitCoinJoinSettings(CCoinJoinClientManager& mgr) const { - 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& 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(); - LogPrintf("CoinJoin: Automatic mixing started for wallet \"%s\"\n", wallet->getWalletName()); - } + if (fAutoStart) { + mgr.startMixing(); } LogPrintf("CoinJoin: autostart=%d, multisession=%d," /* Continued */ "sessions=%d, rounds=%d, amount=%d, denoms_goal=%d, denoms_hardcap=%d\n", diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index 61f3c4d2ae0d..1695e1e574d9 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -10,7 +10,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -25,6 +27,9 @@ BOOST_FIXTURE_TEST_SUITE(coinjoin_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(coinjoin_options_tests) { + gArgs.ForceSetArg("-enablecoinjoin", "0"); + const auto loader{interfaces::MakeCoinJoinLoader(m_node)}; + BOOST_CHECK_EQUAL(CCoinJoinClientOptions::GetSessions(), DEFAULT_COINJOIN_SESSIONS); BOOST_CHECK_EQUAL(CCoinJoinClientOptions::GetRounds(), DEFAULT_COINJOIN_ROUNDS); BOOST_CHECK_EQUAL(CCoinJoinClientOptions::GetRandomRounds(), COINJOIN_RANDOM_ROUNDS); @@ -221,13 +226,14 @@ class CTransactionBuilderTestSetup : public TestChain100Setup BOOST_FIXTURE_TEST_CASE(coinjoin_manager_start_stop_tests, CTransactionBuilderTestSetup) { - auto& cj_man = *Assert(m_node.cj_walletman->getClient("")); - BOOST_CHECK_EQUAL(cj_man.IsMixing(), false); - BOOST_CHECK_EQUAL(cj_man.StartMixing(), true); - BOOST_CHECK_EQUAL(cj_man.IsMixing(), true); - BOOST_CHECK_EQUAL(cj_man.StartMixing(), false); - cj_man.StopMixing(); - BOOST_CHECK_EQUAL(cj_man.IsMixing(), false); + BOOST_CHECK(m_node.cj_walletman->doForClient("", [](auto& cj_man) { + BOOST_CHECK_EQUAL(cj_man.isMixing(), false); + BOOST_CHECK_EQUAL(cj_man.startMixing(), true); + BOOST_CHECK_EQUAL(cj_man.isMixing(), true); + BOOST_CHECK_EQUAL(cj_man.startMixing(), false); + cj_man.stopMixing(); + BOOST_CHECK_EQUAL(cj_man.isMixing(), false); + })); } BOOST_FIXTURE_TEST_CASE(CTransactionBuilderTest, CTransactionBuilderTestSetup) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d7be196ed58b..32b27cf42d7d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1670,10 +1670,10 @@ void CWallet::UnsetBlankWalletFlag(WalletBatch& batch) void CWallet::NewKeyPoolCallback() { - // Note: GetClient(*this) can return nullptr when this wallet is in the middle of its creation. + // Note: WithClient may not find this wallet when it is in the middle of its creation. // Skipping stopMixing() is fine in this case. - if (std::unique_ptr coinjoin_client = coinjoin_available() ? coinjoin_loader().GetClient(GetName()) : nullptr) { - coinjoin_client->stopMixing(); + if (coinjoin_available()) { + coinjoin_loader().WithClient(GetName(), [](auto& client) { client.stopMixing(); }); } nKeysLeftSinceAutoBackup = 0; } diff --git a/src/walletinitinterface.h b/src/walletinitinterface.h index 9222427a39b5..7fbdc4bcde98 100644 --- a/src/walletinitinterface.h +++ b/src/walletinitinterface.h @@ -6,11 +6,9 @@ #define BITCOIN_WALLETINITINTERFACE_H class ArgsManager; +class CCoinJoinClientManager; namespace interfaces { class WalletLoader; -namespace CoinJoin { -class Loader; -} // namespace CoinJoin } // namespace interfaces namespace node { struct NodeContext; @@ -29,7 +27,7 @@ class WalletInitInterface { // Dash Specific WalletInitInterface virtual void AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_loader) const = 0; - virtual void InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loader, interfaces::WalletLoader& wallet_loader) const = 0; + virtual void InitCoinJoinSettings(CCoinJoinClientManager& mgr) const = 0; virtual void InitAutoBackup() const = 0; virtual ~WalletInitInterface() {}