From 9452854900f3b3e30567e7d4d9cd3f070aa717b4 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 29 Mar 2026 20:55:20 +0700 Subject: [PATCH 1/8] fix: linkage error for non-wallet builds for further changes /usr/bin/ld: libbitcoin_node.a(libbitcoin_node_a-walletman.o): in function `CJWalletManagerImpl::UpdatedBlockTip(CBlockIndex const*, CBlockIndex const*, bool)::{lambda(CCoinJoinClientManager&)#1}::operator()(CCoinJoinClientManager&) const': DASH/src/coinjoin/walletman.cpp:132:(.text+0x51c): undefined reference to `CCoinJoinClientManager::UpdatedBlockTip(CBlockIndex const*)' --- src/Makefile.am | 2 +- src/coinjoin/walletman.cpp | 2 +- src/init.cpp | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) 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/walletman.cpp b/src/coinjoin/walletman.cpp index a54ac93795b4..9f582c3a77db 100644 --- a/src/coinjoin/walletman.cpp +++ b/src/coinjoin/walletman.cpp @@ -314,6 +314,6 @@ std::unique_ptr CJWalletManager::make(ChainstateManager& chainm 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; + assert(false); #endif // ENABLE_WALLET } diff --git a/src/init.cpp b/src/init.cpp index 703c50646cc3..f70f9b867ef4 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2224,8 +2224,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } else { assert(!node.cj_walletman); // Can return nullptr if built without wallet support, 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) { From f61af767ff309a2bc19d83de12a6178d187a5267 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 17 Apr 2026 02:10:12 +0700 Subject: [PATCH 2/8] fix: regression test coinjoin_options_tests by enforcing call of `interfaces::MakeCoinJoinLoader` --- src/wallet/test/coinjoin_tests.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index 61f3c4d2ae0d..377b987b29e9 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); From 66c8443d67b09129a31a7e0bd78509cc67ac2abd Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 28 Mar 2026 02:32:24 +0700 Subject: [PATCH 3/8] refactor: remove CoinJoinClientImpl wrapper and return bare pointer to interfaces::CoinJoin::Client --- src/coinjoin/client.h | 16 +++++++++- src/coinjoin/interfaces.cpp | 59 ++----------------------------------- src/interfaces/coinjoin.h | 2 +- src/qt/walletmodel.cpp | 2 +- src/qt/walletmodel.h | 2 +- src/wallet/wallet.cpp | 2 +- 6 files changed, 21 insertions(+), 62 deletions(-) diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 3785d307099e..4454a29d4b3d 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; @@ -230,6 +231,19 @@ class CCoinJoinClientManager 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(); } + void resetPool() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { ResetPool(); } + int getCachedBlocks() override { return nCachedNumBlocks; } + void getJsonInfo(UniValue& obj) override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { GetJsonInfo(obj); } + std::vector getSessionStatuses() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { return GetStatuses(); } + std::string getSessionDenoms() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { return GetSessionDenoms(); } + void setCachedBlocks(int nCachedBlocks) override { nCachedNumBlocks = nCachedBlocks; } + void disableAutobackups() override { fCreateAutoBackups = false; } + bool isMixing() override { return IsMixing(); } + bool startMixing() override { return StartMixing(); } + void stopMixing() override { StopMixing(); } }; #endif // BITCOIN_COINJOIN_CLIENT_H diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index 8dd645025561..a8137dc57b3d 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: @@ -109,10 +55,9 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader { manager().flushWallet(name); } - std::unique_ptr GetClient(const std::string& name) override + interfaces::CoinJoin::Client* GetClient(const std::string& name) override { - auto clientman = manager().getClient(name); - return clientman ? std::make_unique(*clientman) : nullptr; + return manager().getClient(name); } NodeContext& m_node; diff --git a/src/interfaces/coinjoin.h b/src/interfaces/coinjoin.h index c57de9ec3fe2..0929e06674f6 100644 --- a/src/interfaces/coinjoin.h +++ b/src/interfaces/coinjoin.h @@ -46,7 +46,7 @@ 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; + virtual Client* GetClient(const std::string&) = 0; }; } // namespace CoinJoin diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index f83760c862da..d92161a62605 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -87,7 +87,7 @@ void WalletModel::setClientModel(ClientModel* client_model) if (!m_client_model) timer->stop(); } -std::unique_ptr WalletModel::coinJoin() const +interfaces::CoinJoin::Client* WalletModel::coinJoin() const { return m_node.coinJoinLoader()->GetClient(m_wallet->getWalletName()); } diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 9249282f13e7..081acc07e1c9 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -153,7 +153,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; + interfaces::CoinJoin::Client* coinJoin() const; QString getWalletName() const; QString getDisplayName() const; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d7be196ed58b..9f2ef98c80c1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1672,7 +1672,7 @@ void CWallet::NewKeyPoolCallback() { // Note: GetClient(*this) can return nullptr when this wallet 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) { + if (auto* coinjoin_client = coinjoin_available() ? coinjoin_loader().GetClient(GetName()) : nullptr) { coinjoin_client->stopMixing(); } nKeysLeftSinceAutoBackup = 0; From 721233de12694a0182d4c0a7046e251cfcda14e0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 29 Mar 2026 18:26:17 +0700 Subject: [PATCH 4/8] fix: avoid dangling pointers to CJ clients --- src/coinjoin/interfaces.cpp | 4 +- src/coinjoin/walletman.cpp | 15 ++++--- src/coinjoin/walletman.h | 5 ++- src/interfaces/coinjoin.h | 5 ++- src/qt/bitcoingui.cpp | 2 +- src/qt/optionsdialog.cpp | 2 +- src/qt/overviewpage.cpp | 66 +++++++++++++++------------ src/qt/walletmodel.cpp | 4 +- src/qt/walletmodel.h | 3 +- src/rpc/coinjoin.cpp | 71 ++++++++++++++++-------------- src/wallet/init.cpp | 17 +++---- src/wallet/test/coinjoin_tests.cpp | 15 ++++--- src/wallet/wallet.cpp | 6 +-- 13 files changed, 121 insertions(+), 94 deletions(-) diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index a8137dc57b3d..98e067b82bde 100644 --- a/src/coinjoin/interfaces.cpp +++ b/src/coinjoin/interfaces.cpp @@ -55,9 +55,9 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader { manager().flushWallet(name); } - interfaces::CoinJoin::Client* GetClient(const std::string& name) override + bool WithClient(const std::string& name, std::function func) override { - return manager().getClient(name); + return manager().doForClient(name, [&](CCoinJoinClientManager& mgr) { func(mgr); }); } NodeContext& m_node; diff --git a/src/coinjoin/walletman.cpp b/src/coinjoin/walletman.cpp index 9f582c3a77db..69f8a924b0a7 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) 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/interfaces/coinjoin.h b/src/interfaces/coinjoin.h index 0929e06674f6..f459b1ea41b6 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 @@ -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 Client* 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, 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..ae5172b0252c 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,15 @@ 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); + bool notMixing{false}; + walletModel->withCoinJoin([&](auto& client) { + notMixing = !client.isMixing(); + if (notMixing && nBestHeight != client.getCachedBlocks()) { + client.setCachedBlocks(nBestHeight); updateCoinJoinProgress(); } - + }); + if (notMixing) { setWidgetsVisible(false); ui->toggleCoinJoin->setText(tr("Start %1").arg(strCoinJoinName)); @@ -655,7 +662,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 +687,16 @@ 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(); - } + walletModel->withCoinJoin([&](auto& client) { + if (nBestHeight != client.getCachedBlocks()) { + // Balance and number of transactions might have changed + client.setCachedBlocks(nBestHeight); + updateCoinJoinProgress(); + } + ui->labelSubmittedDenom->setText(m_privacy ? "####" : QString(client.getSessionDenoms().c_str())); + }); setWidgetsVisible(true); - - ui->labelSubmittedDenom->setText(m_privacy ? "####" : QString(walletModel->coinJoin()->getSessionDenoms().c_str())); } void OverviewPage::toggleCoinJoin(){ @@ -702,7 +711,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 +731,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 +742,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 +791,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 d92161a62605..84260faa786b 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(); } -interfaces::CoinJoin::Client* WalletModel::coinJoin() const +bool WalletModel::withCoinJoin(std::function func) const { - return m_node.coinJoinLoader()->GetClient(m_wallet->getWalletName()); + return m_node.coinJoinLoader()->WithClient(m_wallet->getWalletName(), std::move(func)); } void WalletModel::updateStatus() diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 081acc07e1c9..3c7445f31325 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); - interfaces::CoinJoin::Client* coinJoin() const; + bool withCoinJoin(std::function func) const; QString getWalletName() const; QString getDisplayName() const; diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 3a2cc40a6977..9509effb2e20 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) { + client.getJsonInfo(obj); + })); std::string warning_msg; if (wallet->IsLegacy()) { diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 7bb836c40083..57a2f3bd6ed0 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -210,14 +210,15 @@ void WalletInit::InitCoinJoinSettings(interfaces::CoinJoin::Loader& coinjoin_loa } 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()); - } + coinjoin_loader.WithClient(wallet->getWalletName(), [&](auto& client) { + if (wallet->isLocked(/*fForMixing=*/false)) { + client.stopMixing(); + LogPrintf("CoinJoin: Mixing stopped for locked wallet \"%s\"\n", wallet->getWalletName()); + } else if (fAutoStart) { + client.startMixing(); + LogPrintf("CoinJoin: Automatic mixing started for wallet \"%s\"\n", wallet->getWalletName()); + } + }); } 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 377b987b29e9..8730444b2f6c 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -226,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 9f2ef98c80c1..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 (auto* 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; } From bfc3c02b70d273b87623577a7a1c522363f87a32 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 29 Mar 2026 20:46:57 +0700 Subject: [PATCH 5/8] refactor: remove duplicated methods from CJWalletManagerImpl to prefer interface's --- src/coinjoin/client.cpp | 33 +++++++++++++++--------------- src/coinjoin/client.h | 26 ++++++++--------------- src/coinjoin/walletman.cpp | 4 ++-- src/interfaces/coinjoin.h | 10 ++++----- src/rpc/coinjoin.cpp | 2 +- src/wallet/test/coinjoin_tests.cpp | 12 +++++------ 6 files changed, 39 insertions(+), 48 deletions(-) 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 4454a29d4b3d..c6a3ad47cc6d 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -198,14 +198,6 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client 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 @@ -230,20 +222,18 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client 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(); } - void resetPool() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { ResetPool(); } - int getCachedBlocks() override { return nCachedNumBlocks; } - void getJsonInfo(UniValue& obj) override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { GetJsonInfo(obj); } - std::vector getSessionStatuses() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { return GetStatuses(); } - std::string getSessionDenoms() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions) { return GetSessionDenoms(); } + int getCachedBlocks() const override { return nCachedNumBlocks; } void setCachedBlocks(int nCachedBlocks) override { nCachedNumBlocks = nCachedBlocks; } void disableAutobackups() override { fCreateAutoBackups = false; } - bool isMixing() override { return IsMixing(); } - bool startMixing() override { return StartMixing(); } - void stopMixing() override { StopMixing(); } + 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/walletman.cpp b/src/coinjoin/walletman.cpp index 69f8a924b0a7..ace7d3982259 100644 --- a/src/coinjoin/walletman.cpp +++ b/src/coinjoin/walletman.cpp @@ -183,8 +183,8 @@ void CJWalletManagerImpl::addWallet(const std::shared_ptr& wall void CJWalletManagerImpl::flushWallet(const std::string& name) { doForClient(name, [](CCoinJoinClientManager& clientman) { - clientman.ResetPool(); - clientman.StopMixing(); + clientman.resetPool(); + clientman.stopMixing(); }); } diff --git a/src/interfaces/coinjoin.h b/src/interfaces/coinjoin.h index f459b1ea41b6..0d83999d38b5 100644 --- a/src/interfaces/coinjoin.h +++ b/src/interfaces/coinjoin.h @@ -28,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; }; diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 9509effb2e20..b5afdb84df12 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -487,7 +487,7 @@ static RPCHelpMan getcoinjoininfo() } CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->WithClient(wallet->GetName(), [&](auto& client) { - client.getJsonInfo(obj); + obj.pushKVs(client.getJsonInfo()); })); std::string warning_msg; diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index 8730444b2f6c..1695e1e574d9 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -227,12 +227,12 @@ class CTransactionBuilderTestSetup : public TestChain100Setup BOOST_FIXTURE_TEST_CASE(coinjoin_manager_start_stop_tests, CTransactionBuilderTestSetup) { 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_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); })); } From d9a3c7422c65d717c8cde3d76bb897af41e01893 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 30 Mar 2026 16:08:28 +0700 Subject: [PATCH 6/8] fix: call InitCoinJoin only wallet is added --- src/coinjoin/interfaces.cpp | 10 +++------- src/dummywallet.cpp | 2 +- src/wallet/init.cpp | 19 +++++-------------- src/walletinitinterface.h | 6 ++---- 4 files changed, 11 insertions(+), 26 deletions(-) diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index 98e067b82bde..ddde39d36146 100644 --- a/src/coinjoin/interfaces.cpp +++ b/src/coinjoin/interfaces.cpp @@ -28,11 +28,6 @@ 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) @@ -44,12 +39,13 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader void AddWallet(const std::shared_ptr& wallet) override { manager().addWallet(wallet); - g_wallet_init_interface.InitCoinJoinSettings(*this, wallet_loader()); + 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 { 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/wallet/init.cpp b/src/wallet/init.cpp index 57a2f3bd6ed0..8764cd540965 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,24 +201,15 @@ 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); + CCoinJoinClientOptions::SetEnabled(gArgs.GetBoolArg("-enablecoinjoin", true)); if (!CCoinJoinClientOptions::IsEnabled()) { return; } bool fAutoStart = gArgs.GetBoolArg("-coinjoinautostart", DEFAULT_COINJOIN_AUTOSTART); - for (auto& wallet : wallets) { - coinjoin_loader.WithClient(wallet->getWalletName(), [&](auto& client) { - if (wallet->isLocked(/*fForMixing=*/false)) { - client.stopMixing(); - LogPrintf("CoinJoin: Mixing stopped for locked wallet \"%s\"\n", wallet->getWalletName()); - } else if (fAutoStart) { - client.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/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() {} From 646af4927baed8d830c8fb1cffe728ed381156b8 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 13 Apr 2026 15:15:24 +0700 Subject: [PATCH 7/8] fix: keep flag `coinjoin` enabled consistently with user's intention over UI or command line --- src/coinjoin/interfaces.cpp | 4 ++-- src/wallet/init.cpp | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index ddde39d36146..e615a679d84b 100644 --- a/src/coinjoin/interfaces.cpp +++ b/src/coinjoin/interfaces.cpp @@ -32,13 +32,13 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader 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); + if (!CCoinJoinClientOptions::IsEnabled()) return; manager().doForClient(wallet->GetName(), [](CCoinJoinClientManager& mgr) { g_wallet_init_interface.InitCoinJoinSettings(mgr); }); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 8764cd540965..f23068270c17 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -203,10 +203,6 @@ void WalletInit::AutoLockMasternodeCollaterals(interfaces::WalletLoader& wallet_ void WalletInit::InitCoinJoinSettings(CCoinJoinClientManager& mgr) const { - CCoinJoinClientOptions::SetEnabled(gArgs.GetBoolArg("-enablecoinjoin", true)); - if (!CCoinJoinClientOptions::IsEnabled()) { - return; - } bool fAutoStart = gArgs.GetBoolArg("-coinjoinautostart", DEFAULT_COINJOIN_AUTOSTART); if (fAutoStart) { mgr.startMixing(); From fbf9d098b2793909cd3f3e0b4b0a00806366007b Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 20 Apr 2026 00:27:25 +0300 Subject: [PATCH 8/8] refactor: cleanups for CoinJoin client interface - Move nCachedNumBlocks and fCreateAutoBackups from public to private in CCoinJoinClientManager; all external access now goes through the interface accessors (resetCachedBlocks/getCachedBlocks/setCachedBlocks/ disableAutobackups). - Take the WithClient/withCoinJoin std::function callback by const reference instead of by value, matching CJWalletManager::doForClient and avoiding an unnecessary copy. - Hoist updateCoinJoinProgress() out of the withCoinJoin lambdas in OverviewPage::coinJoinStatus so cs_wallet_manager_map is no longer held across UI/balance work. - Remove dead `#else assert(false)` branch in CJWalletManager::make by extending the existing `#ifdef ENABLE_WALLET` block; walletman.cpp is only compiled into libbitcoin_wallet_a. - Refresh stale comment in init.cpp about CJWalletManager::make's nullptr return. Co-Authored-By: Claude Opus 4.7 --- src/coinjoin/client.h | 6 +++--- src/coinjoin/interfaces.cpp | 2 +- src/coinjoin/walletman.cpp | 7 +------ src/init.cpp | 2 +- src/interfaces/coinjoin.h | 2 +- src/qt/overviewpage.cpp | 8 ++++++-- src/qt/walletmodel.cpp | 4 ++-- src/qt/walletmodel.h | 2 +- 8 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index c6a3ad47cc6d..dc5c2cc9d187 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -179,15 +179,15 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client // 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; diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index e615a679d84b..5ddb127f4217 100644 --- a/src/coinjoin/interfaces.cpp +++ b/src/coinjoin/interfaces.cpp @@ -51,7 +51,7 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader { manager().flushWallet(name); } - bool WithClient(const std::string& name, std::function func) override + bool WithClient(const std::string& name, const std::function& func) override { return manager().doForClient(name, [&](CCoinJoinClientManager& mgr) { func(mgr); }); } diff --git a/src/coinjoin/walletman.cpp b/src/coinjoin/walletman.cpp index ace7d3982259..3a89357f3ef2 100644 --- a/src/coinjoin/walletman.cpp +++ b/src/coinjoin/walletman.cpp @@ -306,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 - assert(false); -#endif // ENABLE_WALLET } +#endif // ENABLE_WALLET diff --git a/src/init.cpp b/src/init.cpp index f70f9b867ef4..f2094b33dd34 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2223,7 +2223,7 @@ 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); diff --git a/src/interfaces/coinjoin.h b/src/interfaces/coinjoin.h index 0d83999d38b5..16037c086fa6 100644 --- a/src/interfaces/coinjoin.h +++ b/src/interfaces/coinjoin.h @@ -49,7 +49,7 @@ class Loader virtual void FlushWallet(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, std::function func) = 0; + virtual bool WithClient(const std::string& name, const std::function& func) = 0; }; } // namespace CoinJoin diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index ae5172b0252c..b657643d4101 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -597,13 +597,15 @@ void OverviewPage::coinJoinStatus(bool fForce) QString strCoinJoinName = QString::fromStdString(gCoinJoinName); bool notMixing{false}; + bool refreshProgress{false}; walletModel->withCoinJoin([&](auto& client) { notMixing = !client.isMixing(); if (notMixing && nBestHeight != client.getCachedBlocks()) { client.setCachedBlocks(nBestHeight); - updateCoinJoinProgress(); + refreshProgress = true; } }); + if (refreshProgress) updateCoinJoinProgress(); if (notMixing) { setWidgetsVisible(false); ui->toggleCoinJoin->setText(tr("Start %1").arg(strCoinJoinName)); @@ -687,14 +689,16 @@ void OverviewPage::coinJoinStatus(bool fForce) } // check coinjoin status and unlock if needed + bool refreshProgressTail{false}; walletModel->withCoinJoin([&](auto& client) { if (nBestHeight != client.getCachedBlocks()) { // Balance and number of transactions might have changed client.setCachedBlocks(nBestHeight); - updateCoinJoinProgress(); + refreshProgressTail = true; } ui->labelSubmittedDenom->setText(m_privacy ? "####" : QString(client.getSessionDenoms().c_str())); }); + if (refreshProgressTail) updateCoinJoinProgress(); setWidgetsVisible(true); } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 84260faa786b..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(); } -bool WalletModel::withCoinJoin(std::function func) const +bool WalletModel::withCoinJoin(const std::function& func) const { - return m_node.coinJoinLoader()->WithClient(m_wallet->getWalletName(), std::move(func)); + 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 3c7445f31325..1d5c9eb967e6 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -154,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); - bool withCoinJoin(std::function func) const; + bool withCoinJoin(const std::function& func) const; QString getWalletName() const; QString getDisplayName() const;