diff --git a/contrib/auto_gdb/simple_class_obj.py b/contrib/auto_gdb/simple_class_obj.py index 19752b72fafe..6fec8d9525cc 100644 --- a/contrib/auto_gdb/simple_class_obj.py +++ b/contrib/auto_gdb/simple_class_obj.py @@ -14,7 +14,7 @@ "CMasternodeBroadcast", "CMasternodePing", "CMasternodeMan", "CDarksendQueue", "CDarkSendEntry", "CTransaction", "CMutableTransaction", "CCoinJoinBaseSession", - "CCoinJoinBaseManager", "CCoinJoinClientSession", + "CoinJoinQueueManager", "CCoinJoinClientSession", "CCoinJoinClientManager", "CCoinJoinServer", "CMasternodePayments", "CMasternodePaymentVote", "CMasternodeBlockPayees", "CMasternodePayee", "CInstantSend", "CTxLockRequest", diff --git a/src/Makefile.am b/src/Makefile.am index a36f08be79c5..dd6fc88a0f45 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -501,7 +501,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 \ @@ -658,6 +657,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/Makefile.test.include b/src/Makefile.test.include index dd6dda7178c3..61d50f0a0b08 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -122,7 +122,6 @@ BITCOIN_TESTS =\ test/governance_validators_tests.cpp \ test/coinjoin_inouts_tests.cpp \ test/coinjoin_dstxmanager_tests.cpp \ - test/coinjoin_basemanager_tests.cpp \ test/coinjoin_queue_tests.cpp \ test/hash_tests.cpp \ test/httpserver_tests.cpp \ diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index a1bd10b14a4a..087a7d586ff0 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -40,122 +40,10 @@ using wallet::CoinType; using wallet::CWallet; using wallet::ReserveDestination; -CCoinJoinClientQueueManager::CCoinJoinClientQueueManager(CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman, - CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync) : - m_walletman{walletman}, - m_dmnman{dmnman}, - m_mn_metaman{mn_metaman}, - m_mn_sync{mn_sync} -{ -} - -CCoinJoinClientQueueManager::~CCoinJoinClientQueueManager() = default; - -MessageProcessingResult CCoinJoinClientQueueManager::ProcessMessage(NodeId from, CConnman& connman, - std::string_view msg_type, CDataStream& vRecv) -{ - if (msg_type != NetMsgType::DSQUEUE) { - return {}; - } - if (!m_mn_sync.IsBlockchainSynced()) return {}; - - assert(m_mn_metaman.IsValid()); - - CCoinJoinQueue dsq; - vRecv >> dsq; - - MessageProcessingResult ret{}; - ret.m_to_erase = CInv{MSG_DSQ, dsq.GetHash()}; - - if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) { - ret.m_error = MisbehavingError{100}; - return ret; - } - - const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - if (dsq.masternodeOutpoint.IsNull()) { - if (auto dmn = tip_mn_list.GetValidMN(dsq.m_protxHash)) { - dsq.masternodeOutpoint = dmn->collateralOutpoint; - } else { - ret.m_error = MisbehavingError{10}; - return ret; - } - } - - { - LOCK(cs_ProcessDSQueue); - - { - LOCK(cs_vecqueue); - // process every dsq only once - for (const auto &q: vecCoinJoinQueue) { - if (q == dsq) { - return ret; - } - if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) { - // no way the same mn can send another dsq with the same readiness this soon - LogPrint(BCLog::COINJOIN, /* Continued */ - "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", - from, dsq.masternodeOutpoint.ToStringShort()); - return ret; - } - } - } // cs_vecqueue - - LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString()); - - if (dsq.IsTimeOutOfBounds()) return ret; - - auto dmn = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); - if (!dmn) return ret; - - if (dsq.m_protxHash.IsNull()) { - dsq.m_protxHash = dmn->proTxHash; - } - - if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) { - ret.m_error = MisbehavingError{10}; - return ret; - } - - // if the queue is ready, submit if we can - if (dsq.fReady && - m_walletman.ForAnyCJClientMan([&connman, &dmn](std::unique_ptr& clientman) { - return clientman->TrySubmitDenominate(dmn->proTxHash, connman); - })) { - LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue is ready, masternode=%s, queue=%s\n", dmn->proTxHash.ToString(), dsq.ToString()); - return ret; - } else { - if (m_mn_metaman.IsMixingThresholdExceeded(dmn->proTxHash, tip_mn_list.GetCounts().enabled())) { - LogPrint(BCLog::COINJOIN, "DSQUEUE -- Masternode %s is sending too many dsq messages\n", - dmn->proTxHash.ToString()); - return ret; - } - m_mn_metaman.AllowMixing(dmn->proTxHash); - - LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue, masternode=%s, queue=%s\n", dmn->proTxHash.ToString(), dsq.ToString()); - - m_walletman.ForAnyCJClientMan([&dsq](const std::unique_ptr& clientman) { - return clientman->MarkAlreadyJoinedQueueAsTried(dsq); - }); - - WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq)); - } - } // cs_ProcessDSQueue - return ret; -} - -void CCoinJoinClientQueueManager::DoMaintenance() -{ - if (!m_mn_sync.IsBlockchainSynced() || ShutdownRequested()) return; - - CheckQueue(); -} - CCoinJoinClientManager::CCoinJoinClientManager(const std::shared_ptr& wallet, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, - const std::unique_ptr& queueman) : + CoinJoinQueueManager* queueman) : m_wallet{wallet}, m_dmnman{dmnman}, m_mn_metaman{mn_metaman}, @@ -173,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; } @@ -192,15 +80,13 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_cha CCoinJoinClientSession::CCoinJoinClientSession(const std::shared_ptr& wallet, CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, - const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, - const std::unique_ptr& queueman) : + const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman) : m_wallet(wallet), m_clientman(clientman), m_dmnman(dmnman), m_mn_metaman(mn_metaman), m_mn_sync(mn_sync), - m_isman{isman}, - m_queueman(queueman) + m_isman{isman} {} void CCoinJoinClientSession::ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) @@ -251,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; } @@ -273,7 +159,7 @@ void CCoinJoinClientSession::ResetPool() WITH_LOCK(cs_coinjoin, SetNull()); } -void CCoinJoinClientManager::ResetPool() +void CCoinJoinClientManager::resetPool() { nCachedLastSuccessBlock = 0; AssertLockNotHeld(cs_deqsessions); @@ -355,7 +241,7 @@ bilingual_str CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const } } -std::vector CCoinJoinClientManager::GetStatuses() const +std::vector CCoinJoinClientManager::getSessionStatuses() const { AssertLockNotHeld(cs_deqsessions); @@ -369,7 +255,7 @@ std::vector CCoinJoinClientManager::GetStatuses() const return ret; } -std::string CCoinJoinClientManager::GetSessionDenoms() +std::string CCoinJoinClientManager::getSessionDenoms() const { std::string strSessionDenoms; @@ -442,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) { @@ -719,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; @@ -728,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: @@ -754,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 @@ -976,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."); @@ -1007,7 +893,7 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman AssertLockNotHeld(cs_deqsessions); LOCK(cs_deqsessions); if (int(deqSessions.size()) < CCoinJoinClientOptions::GetSessions()) { - deqSessions.emplace_back(m_wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_isman, m_queueman); + deqSessions.emplace_back(m_wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_isman); } for (auto& session : deqSessions) { if (!CheckAutomaticBackup()) return false; @@ -1075,14 +961,13 @@ static int WinnersToSkip() bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman) { if (!CCoinJoinClientOptions::IsEnabled()) return false; - if (m_queueman == nullptr) return false; const auto mnList = m_dmnman.GetListAtChainTip(); const int nWeightedMnCount = mnList.GetCounts().m_valid_weighted; // Look through the queues and see if anything matches CCoinJoinQueue dsq; - while (m_queueman->GetQueueItemAndTry(dsq)) { + while (m_clientman.GetQueueItemAndTry(dsq)) { auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint); if (!dmn) { @@ -1288,6 +1173,11 @@ bool CCoinJoinClientManager::MarkAlreadyJoinedQueueAsTried(CCoinJoinQueue& dsq) return false; } +bool CCoinJoinClientManager::GetQueueItemAndTry(CCoinJoinQueue& dsq) const +{ + return m_queueman && m_queueman->GetQueueItemAndTry(dsq); +} + bool CCoinJoinClientSession::SubmitDenominate(CConnman& connman) { LOCK(m_wallet->cs_wallet); @@ -1875,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); @@ -1891,61 +1781,6 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const } } obj.pushKV("sessions", arrSessions); + return obj; } -CoinJoinWalletManager::CoinJoinWalletManager(ChainstateManager& chainman, CDeterministicMNManager& dmnman, - CMasternodeMetaMan& mn_metaman, const CTxMemPool& mempool, - const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, - const std::unique_ptr& queueman) : - m_chainman{chainman}, - m_dmnman{dmnman}, - m_mn_metaman{mn_metaman}, - m_mempool{mempool}, - m_mn_sync{mn_sync}, - m_isman{isman}, - m_queueman{queueman} -{ -} - -CoinJoinWalletManager::~CoinJoinWalletManager() -{ - LOCK(cs_wallet_manager_map); - for (auto& [wallet_name, cj_man] : m_wallet_manager_map) { - cj_man.reset(); - } -} - -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, m_dmnman, m_mn_metaman, m_mn_sync, - m_isman, m_queueman)); -} - -void CoinJoinWalletManager::DoMaintenance(CConnman& connman) -{ - LOCK(cs_wallet_manager_map); - for (auto& [_, clientman] : m_wallet_manager_map) { - clientman->DoMaintenance(m_chainman, connman, m_mempool); - } -} - -void CoinJoinWalletManager::Remove(const std::string& name) { - LOCK(cs_wallet_manager_map); - m_wallet_manager_map.erase(name); -} - -void CoinJoinWalletManager::Flush(const std::string& name) -{ - auto clientman = Assert(Get(name)); - clientman->ResetPool(); - clientman->StopMixing(); -} - -CCoinJoinClientManager* CoinJoinWalletManager::Get(const std::string& name) const -{ - LOCK(cs_wallet_manager_map); - auto it = m_wallet_manager_map.find(name); - return (it != m_wallet_manager_map.end()) ? it->second.get() : nullptr; -} diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 5ecf433010f1..c6a3ad47cc6d 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -8,10 +8,7 @@ #include #include #include -#include - -#include -#include +#include #include #include @@ -21,14 +18,12 @@ #include class CCoinJoinClientManager; -class CCoinJoinClientQueueManager; class CConnman; class CDeterministicMNManager; class ChainstateManager; class CMasternodeMetaMan; class CMasternodeSync; class CNode; -class CoinJoinWalletManager; class CTxMemPool; class UniValue; @@ -70,57 +65,6 @@ class CPendingDsaRequest } }; -class CoinJoinWalletManager { -public: - using wallet_name_cjman_map = std::map>; - -public: - CoinJoinWalletManager() = delete; - CoinJoinWalletManager(const CoinJoinWalletManager&) = delete; - CoinJoinWalletManager& operator=(const CoinJoinWalletManager&) = delete; - explicit CoinJoinWalletManager(ChainstateManager& chainman, CDeterministicMNManager& dmnman, - CMasternodeMetaMan& mn_metaman, const CTxMemPool& mempool, - const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, - const std::unique_ptr& queueman); - ~CoinJoinWalletManager(); - - void Add(const std::shared_ptr& wallet) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); - void DoMaintenance(CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); - - void Remove(const std::string& name) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); - void Flush(const std::string& name) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); - - CCoinJoinClientManager* Get(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); - - template - void ForEachCJClientMan(Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map) - { - LOCK(cs_wallet_manager_map); - for (auto&& [_, clientman] : m_wallet_manager_map) { - func(clientman); - } - }; - - template - bool ForAnyCJClientMan(Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map) - { - LOCK(cs_wallet_manager_map); - return std::ranges::any_of(m_wallet_manager_map, [&](auto& pair) { return func(pair.second); }); - }; - -private: - ChainstateManager& m_chainman; - CDeterministicMNManager& m_dmnman; - CMasternodeMetaMan& m_mn_metaman; - const CTxMemPool& m_mempool; - const CMasternodeSync& m_mn_sync; - const llmq::CInstantSendManager& m_isman; - const std::unique_ptr& m_queueman; - - mutable Mutex cs_wallet_manager_map; - wallet_name_cjman_map m_wallet_manager_map GUARDED_BY(cs_wallet_manager_map); -}; - class CCoinJoinClientSession : public CCoinJoinBaseSession { private: @@ -130,8 +74,6 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession CMasternodeMetaMan& m_mn_metaman; const CMasternodeSync& m_mn_sync; const llmq::CInstantSendManager& m_isman; - const std::unique_ptr& m_queueman; - std::vector vecOutPointLocked; bilingual_str strLastMessage; @@ -185,8 +127,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession public: explicit CCoinJoinClientSession(const std::shared_ptr& wallet, CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, - const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, - const std::unique_ptr& queueman); + const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman); void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv); @@ -212,35 +153,9 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession void GetJsonInfo(UniValue& obj) const; }; -/** Used to keep track of mixing queues - */ -class CCoinJoinClientQueueManager : public CCoinJoinBaseManager -{ -private: - CoinJoinWalletManager& m_walletman; - CDeterministicMNManager& m_dmnman; - CMasternodeMetaMan& m_mn_metaman; - const CMasternodeSync& m_mn_sync; - - mutable Mutex cs_ProcessDSQueue; - -public: - CCoinJoinClientQueueManager() = delete; - CCoinJoinClientQueueManager(const CCoinJoinClientQueueManager&) = delete; - CCoinJoinClientQueueManager& operator=(const CCoinJoinClientQueueManager&) = delete; - explicit CCoinJoinClientQueueManager(CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman, - CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync); - ~CCoinJoinClientQueueManager(); - - [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, CConnman& connman, std::string_view msg_type, - CDataStream& vRecv) - EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue, !cs_ProcessDSQueue); - void DoMaintenance(); -}; - /** 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; @@ -248,7 +163,8 @@ class CCoinJoinClientManager CMasternodeMetaMan& m_mn_metaman; const CMasternodeSync& m_mn_sync; const llmq::CInstantSendManager& m_isman; - const std::unique_ptr& m_queueman; + //! Non-owning pointer; null when relay_txes is disabled (no queue processing). + CoinJoinQueueManager* const m_queueman; mutable Mutex cs_deqsessions; // TODO: or map ?? @@ -277,20 +193,11 @@ class CCoinJoinClientManager CCoinJoinClientManager& operator=(const CCoinJoinClientManager&) = delete; explicit CCoinJoinClientManager(const std::shared_ptr& wallet, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync, - const llmq::CInstantSendManager& isman, - const std::unique_ptr& queueman); + const llmq::CInstantSendManager& isman, CoinJoinQueueManager* queueman); ~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 @@ -299,6 +206,7 @@ class CCoinJoinClientManager bool TrySubmitDenominate(const uint256& proTxHash, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); bool MarkAlreadyJoinedQueueAsTried(CCoinJoinQueue& dsq) const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); + bool GetQueueItemAndTry(CCoinJoinQueue& dsq) const; void CheckTimeout() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions); @@ -314,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/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 9b4d8953c48f..010a81c877b7 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -116,17 +116,13 @@ void CCoinJoinBaseSession::SetNull() nTimeLastSuccessfulStep = GetTime(); } -CCoinJoinBaseManager::CCoinJoinBaseManager() = default; - -CCoinJoinBaseManager::~CCoinJoinBaseManager() = default; - -void CCoinJoinBaseManager::SetNull() +void CoinJoinQueueManager::SetNull() { LOCK(cs_vecqueue); vecCoinJoinQueue.clear(); } -void CCoinJoinBaseManager::CheckQueue() +void CoinJoinQueueManager::CheckQueue() { TRY_LOCK(cs_vecqueue, lockDS); if (!lockDS) return; // it's ok to fail here, we run this quite frequently @@ -135,7 +131,7 @@ void CCoinJoinBaseManager::CheckQueue() auto it = vecCoinJoinQueue.begin(); while (it != vecCoinJoinQueue.end()) { if (it->IsTimeOutOfBounds()) { - LogPrint(BCLog::COINJOIN, "CCoinJoinBaseManager::%s -- Removing a queue (%s)\n", __func__, it->ToString()); + LogPrint(BCLog::COINJOIN, "CoinJoinQueueManager::%s -- Removing a queue (%s)\n", __func__, it->ToString()); it = vecCoinJoinQueue.erase(it); } else { ++it; @@ -143,7 +139,33 @@ void CCoinJoinBaseManager::CheckQueue() } } -bool CCoinJoinBaseManager::GetQueueItemAndTry(CCoinJoinQueue& dsqRet) +std::optional CoinJoinQueueManager::TryHasQueueFromMasternode(const COutPoint& outpoint) const +{ + TRY_LOCK(cs_vecqueue, lockDS); + if (!lockDS) return std::nullopt; + return std::ranges::any_of(vecCoinJoinQueue, [&outpoint](const auto& q) { return q.masternodeOutpoint == outpoint; }); +} + +std::optional CoinJoinQueueManager::TryCheckDuplicate(const CCoinJoinQueue& dsq) const +{ + TRY_LOCK(cs_vecqueue, lockDS); + if (!lockDS) return std::nullopt; + for (const auto& q : vecCoinJoinQueue) { + if (q == dsq) return true; + if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) return true; + } + return false; +} + +bool CoinJoinQueueManager::TryAddQueue(CCoinJoinQueue dsq) +{ + TRY_LOCK(cs_vecqueue, lockDS); + if (!lockDS) return false; + vecCoinJoinQueue.push_back(std::move(dsq)); + return true; +} + +bool CoinJoinQueueManager::GetQueueItemAndTry(CCoinJoinQueue& dsqRet) { TRY_LOCK(cs_vecqueue, lockDS); if (!lockDS) return false; // it's ok to fail here, we run this quite frequently diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 7cfd1c09efaf..ea073f14f79e 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -321,21 +321,19 @@ class CCoinJoinBaseSession int GetEntriesCountLocked() const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin) { return vecEntries.size(); } }; -// base class -class CCoinJoinBaseManager +class CoinJoinQueueManager { -protected: +private: mutable Mutex cs_vecqueue; // The current mixing sessions in progress on the network std::vector vecCoinJoinQueue GUARDED_BY(cs_vecqueue); +public: void SetNull() EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); - void CheckQueue() EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); -public: - CCoinJoinBaseManager(); - virtual ~CCoinJoinBaseManager(); + //! Remove timed-out queue entries. Call periodically (e.g. every second). + void CheckQueue() EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); int GetQueueSize() const EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue) { LOCK(cs_vecqueue); return vecCoinJoinQueue.size(); } bool GetQueueItemAndTry(CCoinJoinQueue& dsqRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); @@ -351,6 +349,30 @@ class CCoinJoinBaseManager LOCK(cs_vecqueue); return util::find_if_opt(vecCoinJoinQueue, [&queueHash](const auto& q) { return q.GetHash() == queueHash; }); } + + //! True if any queue entry matches the given masternode outpoint and readiness state. + //! Used to detect when a masternode is broadcasting queues too quickly. + bool HasQueueFromMasternode(const COutPoint& outpoint, bool fReady) const EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue) + { + LOCK(cs_vecqueue); + return std::any_of(vecCoinJoinQueue.begin(), vecCoinJoinQueue.end(), + [&](const auto& q) { return q.masternodeOutpoint == outpoint && q.fReady == fReady; }); + } + //! TRY_LOCK variant: returns nullopt if lock can't be acquired; true if any queue entry has this + //! outpoint (any readiness). + [[nodiscard]] std::optional TryHasQueueFromMasternode(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); + //! TRY_LOCK combined duplicate check: returns nullopt if lock can't be acquired; true if dsq is + //! an exact duplicate or the masternode is sending too many dsqs with the same readiness. + [[nodiscard]] std::optional TryCheckDuplicate(const CCoinJoinQueue& dsq) const EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); + + //! Append a queue entry (caller must have already checked for duplicates). + void AddQueue(CCoinJoinQueue dsq) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue) + { + LOCK(cs_vecqueue); + vecCoinJoinQueue.push_back(std::move(dsq)); + } + //! TRY_LOCK variant of AddQueue: returns false if the lock cannot be acquired. + bool TryAddQueue(CCoinJoinQueue dsq) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); }; // Various helpers and dstx manager implementation diff --git a/src/coinjoin/interfaces.cpp b/src/coinjoin/interfaces.cpp index 8dd645025561..c74e17d56b69 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,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) @@ -98,21 +39,23 @@ 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) { + if (!wallet->IsLocked()) { + 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, 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/server.cpp b/src/coinjoin/server.cpp index 4abeb79ba526..d968c1b0c567 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -87,13 +87,9 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) if (vecSessionCollaterals.empty()) { { - TRY_LOCK(cs_vecqueue, lockRecv); - if (!lockRecv) return; - - auto mnOutpoint = m_mn_activeman.GetOutPoint(); - - if (std::ranges::any_of(vecCoinJoinQueue, - [&mnOutpoint](const auto& q) { return q.masternodeOutpoint == mnOutpoint; })) { + const auto hasQueue = m_queueman.TryHasQueueFromMasternode(m_mn_activeman.GetOutPoint()); + if (!hasQueue.has_value()) return; + if (*hasQueue) { // refuse to create another queue this often LogPrint(BCLog::COINJOIN, "DSACCEPT -- last dsq is still in queue, refuse to mix\n"); PushStatus(peer, STATUS_REJECTED, ERR_RECENT); @@ -160,21 +156,13 @@ void CCoinJoinServer::ProcessDSQUEUE(NodeId from, CDataStream& vRecv) } { - TRY_LOCK(cs_vecqueue, lockRecv); - if (!lockRecv) return; - - // process every dsq only once - for (const auto& q : vecCoinJoinQueue) { - if (q == dsq) { - return; - } - if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) { - // no way the same mn can send another dsq with the same readiness this soon - LogPrint(BCLog::COINJOIN, "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", from, dsq.masternodeOutpoint.ToStringShort()); - return; - } + const auto isDup = m_queueman.TryCheckDuplicate(dsq); + if (!isDup.has_value()) return; + if (*isDup) { + LogPrint(BCLog::COINJOIN, "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", from, dsq.masternodeOutpoint.ToStringShort()); + return; } - } // cs_vecqueue + } LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString()); @@ -202,9 +190,7 @@ void CCoinJoinServer::ProcessDSQUEUE(NodeId from, CDataStream& vRecv) LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue, masternode=%s, queue=%s\n", dmn->proTxHash.ToString(), dsq.ToString()); - TRY_LOCK(cs_vecqueue, lockRecv); - if (!lockRecv) return; - vecCoinJoinQueue.push_back(dsq); + if (!m_queueman.TryAddQueue(dsq)) return; m_peer_manager->PeerRelayDSQ(dsq); } } @@ -267,7 +253,7 @@ void CCoinJoinServer::SetNull() vecSessionCollaterals.clear(); CCoinJoinBaseSession::SetNull(); - CCoinJoinBaseManager::SetNull(); + m_queueman.SetNull(); } // @@ -504,7 +490,7 @@ bool CCoinJoinServer::HasTimedOut() const // void CCoinJoinServer::CheckTimeout() { - CheckQueue(); + m_queueman.CheckQueue(); // Too early to do anything if (!CCoinJoinServer::HasTimedOut()) return; @@ -531,7 +517,7 @@ void CCoinJoinServer::CheckForCompleteQueue() "with %d participants\n", dsq.ToString(), vecSessionCollaterals.size()); dsq.vchSig = m_mn_activeman.SignBasic(dsq.GetSignatureHash()); m_peer_manager->PeerRelayDSQ(dsq); - WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq)); + m_queueman.AddQueue(std::move(dsq)); } } @@ -739,8 +725,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString()); dsq.vchSig = m_mn_activeman.SignBasic(dsq.GetSignatureHash()); m_peer_manager->PeerRelayDSQ(dsq); - LOCK(cs_vecqueue); - vecCoinJoinQueue.push_back(dsq); + m_queueman.AddQueue(std::move(dsq)); } vecSessionCollaterals.push_back(MakeTransactionRef(dsa.txCollateral)); @@ -916,7 +901,7 @@ void CCoinJoinServer::GetJsonInfo(UniValue& obj) const { obj.clear(); obj.setObject(); - obj.pushKV("queue_size", GetQueueSize()); + obj.pushKV("queue_size", m_queueman.GetQueueSize()); obj.pushKV("denomination", ValueFromAmount(CoinJoin::DenominationToAmount(nSessionDenom))); obj.pushKV("state", GetStateString()); obj.pushKV("entries_count", GetEntriesCount()); @@ -924,14 +909,14 @@ void CCoinJoinServer::GetJsonInfo(UniValue& obj) const bool CCoinJoinServer::AlreadyHave(const CInv& inv) { - return (inv.type == MSG_DSQ) ? HasQueue(inv.hash) : false; + return (inv.type == MSG_DSQ) ? m_queueman.HasQueue(inv.hash) : false; } bool CCoinJoinServer::ProcessGetData(CNode& pfrom, const CInv& inv, CConnman& connman, const CNetMsgMaker& msgMaker) { if (inv.type != MSG_DSQ) return false; - auto opt_dsq = GetQueueFromHash(inv.hash); + auto opt_dsq = m_queueman.GetQueueFromHash(inv.hash); if (!opt_dsq.has_value()) return false; connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::DSQUEUE, *opt_dsq)); diff --git a/src/coinjoin/server.h b/src/coinjoin/server.h index f02ce7dce305..4c900cacc08d 100644 --- a/src/coinjoin/server.h +++ b/src/coinjoin/server.h @@ -25,9 +25,11 @@ class UniValue; /** Used to keep track of current status of mixing pool */ -class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager, public NetHandler +class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler { private: + CoinJoinQueueManager m_queueman; + ChainstateManager& m_chainman; CConnman& connman; CDeterministicMNManager& m_dmnman; @@ -64,7 +66,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager /// Is this nDenom and txCollateral acceptable? bool IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) const; - bool CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); + bool CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet); bool AddUserToExistingSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet); /// Do we have enough users to take entries? bool IsSessionReady() const; @@ -83,8 +85,8 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager void RelayStatus(PoolStatusUpdate nStatusUpdate, PoolMessage nMessageID = MSG_NOERR) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); void RelayCompletedTransaction(PoolMessage nMessageID) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); - void ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); - void ProcessDSQUEUE(NodeId from, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue); + void ProcessDSACCEPT(CNode& peer, CDataStream& vRecv); + void ProcessDSQUEUE(NodeId from, CDataStream& vRecv); void ProcessDSVIN(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); void ProcessDSSIGNFINALTX(CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); diff --git a/src/coinjoin/walletman.cpp b/src/coinjoin/walletman.cpp index 50b552d766fb..ace7d3982259 100644 --- a/src/coinjoin/walletman.cpp +++ b/src/coinjoin/walletman.cpp @@ -7,18 +7,23 @@ #endif #include - +#include +#include +#include +#include +#include #include +#include #include +#include #include -#include - #ifdef ENABLE_WALLET #include #endif // ENABLE_WALLET #include +#include #ifdef ENABLE_WALLET class CJWalletManagerImpl final : public CJWalletManager @@ -27,32 +32,66 @@ class CJWalletManagerImpl final : public CJWalletManager CJWalletManagerImpl(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool, const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, bool relay_txes); - virtual ~CJWalletManagerImpl() = default; + ~CJWalletManagerImpl() override; public: void Schedule(CConnman& connman, CScheduler& scheduler) override; public: bool hasQueue(const uint256& hash) const override; - CCoinJoinClientManager* getClient(const std::string& name) override; + 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; + 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; std::optional getQueueSize() const override; - std::vector getMixingMasternodes() override; - void addWallet(const std::shared_ptr& wallet) override; - void removeWallet(const std::string& name) override; - void flushWallet(const std::string& name) override; + std::vector getMixingMasternodes() override EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); + void addWallet(const std::shared_ptr& wallet) override EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); + void removeWallet(const std::string& name) override EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); + void flushWallet(const std::string& name) override EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); protected: // CValidationInterface - void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override; + void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); private: const bool m_relay_txes; + ChainstateManager& m_chainman; + CDeterministicMNManager& m_dmnman; + CMasternodeMetaMan& m_mn_metaman; + CTxMemPool& m_mempool; + const CMasternodeSync& m_mn_sync; + const llmq::CInstantSendManager& m_isman; + + // m_queueman is declared before the wallet map so that it is destroyed + // after all CCoinJoinClientManager instances (which hold a raw pointer to it). + // Null when relay_txes is false (no queue processing). + const std::unique_ptr m_queueman; + + mutable Mutex cs_ProcessDSQueue; + + mutable Mutex cs_wallet_manager_map; + std::map> m_wallet_manager_map GUARDED_BY(cs_wallet_manager_map); + + void DoMaintenance(CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map); + + [[nodiscard]] MessageProcessingResult ProcessDSQueue(NodeId from, CConnman& connman, std::string_view msg_type, + CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_ProcessDSQueue, !cs_wallet_manager_map); - CoinJoinWalletManager walletman; - const std::unique_ptr queueman; + template + void ForEachCJClientMan(Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map) + { + LOCK(cs_wallet_manager_map); + for (auto& [_, clientman] : m_wallet_manager_map) { + func(*clientman); + } + } + + template + bool ForAnyCJClientMan(Callable&& func) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map) + { + LOCK(cs_wallet_manager_map); + return std::ranges::any_of(m_wallet_manager_map, [&](auto& pair) { return func(*pair.second); }); + } }; CJWalletManagerImpl::CJWalletManagerImpl(ChainstateManager& chainman, CDeterministicMNManager& dmnman, @@ -60,17 +99,28 @@ CJWalletManagerImpl::CJWalletManagerImpl(ChainstateManager& chainman, CDetermini const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, bool relay_txes) : m_relay_txes{relay_txes}, - walletman{chainman, dmnman, mn_metaman, mempool, mn_sync, isman, queueman}, - queueman{m_relay_txes ? std::make_unique(walletman, dmnman, mn_metaman, mn_sync) : nullptr} + m_chainman{chainman}, + m_dmnman{dmnman}, + m_mn_metaman{mn_metaman}, + m_mempool{mempool}, + m_mn_sync{mn_sync}, + m_isman{isman}, + m_queueman{m_relay_txes ? std::make_unique() : nullptr} +{ +} + +CJWalletManagerImpl::~CJWalletManagerImpl() { + LOCK(cs_wallet_manager_map); + for (auto& [_, clientman] : m_wallet_manager_map) { + clientman.reset(); + } } void CJWalletManagerImpl::Schedule(CConnman& connman, CScheduler& scheduler) { if (!m_relay_txes) return; - scheduler.scheduleEvery(std::bind(&CCoinJoinClientQueueManager::DoMaintenance, std::ref(*queueman)), - std::chrono::seconds{1}); - scheduler.scheduleEvery(std::bind(&CoinJoinWalletManager::DoMaintenance, std::ref(walletman), std::ref(connman)), + scheduler.scheduleEvery(std::bind(&CJWalletManagerImpl::DoMaintenance, this, std::ref(connman)), std::chrono::seconds{1}); } @@ -79,48 +129,38 @@ void CJWalletManagerImpl::UpdatedBlockTip(const CBlockIndex* pindexNew, const CB if (fInitialDownload || pindexNew == pindexFork) // In IBD or blocks were disconnected without any new ones return; - walletman.ForEachCJClientMan( - [&pindexNew](std::unique_ptr& clientman) { clientman->UpdatedBlockTip(pindexNew); }); + ForEachCJClientMan([&pindexNew](CCoinJoinClientManager& clientman) { clientman.UpdatedBlockTip(pindexNew); }); } bool CJWalletManagerImpl::hasQueue(const uint256& hash) const { - if (queueman) { - return queueman->HasQueue(hash); + if (m_queueman) { + return m_queueman->HasQueue(hash); } return false; } -CCoinJoinClientManager* CJWalletManagerImpl::getClient(const std::string& name) +bool CJWalletManagerImpl::doForClient(const std::string& name, const std::function& func) { - return walletman.Get(name); -} - -MessageProcessingResult CJWalletManagerImpl::processMessage(CNode& pfrom, CChainState& chainstate, CConnman& connman, - CTxMemPool& mempool, std::string_view msg_type, - CDataStream& vRecv) -{ - walletman.ForEachCJClientMan([&](std::unique_ptr& clientman) { - clientman->ProcessMessage(pfrom, chainstate, connman, mempool, msg_type, vRecv); - }); - if (queueman) { - return queueman->ProcessMessage(pfrom.GetId(), connman, msg_type, vRecv); - } - return {}; + LOCK(cs_wallet_manager_map); + auto it = m_wallet_manager_map.find(name); + if (it == m_wallet_manager_map.end()) return false; + func(*it->second); + return true; } std::optional CJWalletManagerImpl::getQueueFromHash(const uint256& hash) const { - if (queueman) { - return queueman->GetQueueFromHash(hash); + if (m_queueman) { + return m_queueman->GetQueueFromHash(hash); } return std::nullopt; } std::optional CJWalletManagerImpl::getQueueSize() const { - if (queueman) { - return queueman->GetQueueSize(); + if (m_queueman) { + return m_queueman->GetQueueSize(); } return std::nullopt; } @@ -128,24 +168,143 @@ std::optional CJWalletManagerImpl::getQueueSize() const std::vector CJWalletManagerImpl::getMixingMasternodes() { std::vector ret{}; - walletman.ForEachCJClientMan( - [&](const std::unique_ptr& clientman) { clientman->GetMixingMasternodesInfo(ret); }); + ForEachCJClientMan([&](const CCoinJoinClientManager& clientman) { clientman.GetMixingMasternodesInfo(ret); }); return ret; } void CJWalletManagerImpl::addWallet(const std::shared_ptr& wallet) { - walletman.Add(wallet); + LOCK(cs_wallet_manager_map); + m_wallet_manager_map.try_emplace(wallet->GetName(), + std::make_unique(wallet, m_dmnman, m_mn_metaman, m_mn_sync, + m_isman, m_queueman.get())); } void CJWalletManagerImpl::flushWallet(const std::string& name) { - walletman.Flush(name); + doForClient(name, [](CCoinJoinClientManager& clientman) { + clientman.resetPool(); + clientman.stopMixing(); + }); } void CJWalletManagerImpl::removeWallet(const std::string& name) { - walletman.Remove(name); + LOCK(cs_wallet_manager_map); + m_wallet_manager_map.erase(name); +} + +void CJWalletManagerImpl::DoMaintenance(CConnman& connman) +{ + if (m_queueman && m_mn_sync.IsBlockchainSynced() && !ShutdownRequested()) { + m_queueman->CheckQueue(); + } + LOCK(cs_wallet_manager_map); + for (auto& [_, clientman] : m_wallet_manager_map) { + clientman->DoMaintenance(m_chainman, connman, m_mempool); + } +} + +MessageProcessingResult CJWalletManagerImpl::processMessage(CNode& pfrom, CChainState& chainstate, CConnman& connman, + CTxMemPool& mempool, std::string_view msg_type, + CDataStream& vRecv) +{ + ForEachCJClientMan([&](CCoinJoinClientManager& clientman) { + clientman.ProcessMessage(pfrom, chainstate, connman, mempool, msg_type, vRecv); + }); + if (m_queueman) { + return ProcessDSQueue(pfrom.GetId(), connman, msg_type, vRecv); + } + return {}; +} + +MessageProcessingResult CJWalletManagerImpl::ProcessDSQueue(NodeId from, CConnman& connman, std::string_view msg_type, + CDataStream& vRecv) +{ + assert(m_queueman); + + if (msg_type != NetMsgType::DSQUEUE) { + return {}; + } + if (!m_mn_sync.IsBlockchainSynced()) return {}; + + assert(m_mn_metaman.IsValid()); + + CCoinJoinQueue dsq; + vRecv >> dsq; + + MessageProcessingResult ret{}; + ret.m_to_erase = CInv{MSG_DSQ, dsq.GetHash()}; + + if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) { + ret.m_error = MisbehavingError{100}; + return ret; + } + + const auto tip_mn_list = m_dmnman.GetListAtChainTip(); + if (dsq.masternodeOutpoint.IsNull()) { + if (auto dmn = tip_mn_list.GetValidMN(dsq.m_protxHash)) { + dsq.masternodeOutpoint = dmn->collateralOutpoint; + } else { + ret.m_error = MisbehavingError{10}; + return ret; + } + } + + { + LOCK(cs_ProcessDSQueue); + + if (m_queueman->HasQueue(dsq.GetHash())) return ret; + + if (m_queueman->HasQueueFromMasternode(dsq.masternodeOutpoint, dsq.fReady)) { + // no way the same mn can send another dsq with the same readiness this soon + LogPrint(BCLog::COINJOIN, /* Continued */ + "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", + from, dsq.masternodeOutpoint.ToStringShort()); + return ret; + } + + LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString()); + + if (dsq.IsTimeOutOfBounds()) return ret; + + auto dmn = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); + if (!dmn) return ret; + + if (dsq.m_protxHash.IsNull()) { + dsq.m_protxHash = dmn->proTxHash; + } + + if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) { + ret.m_error = MisbehavingError{10}; + return ret; + } + + // if the queue is ready, submit if we can + if (dsq.fReady && ForAnyCJClientMan([&connman, &dmn](CCoinJoinClientManager& clientman) { + return clientman.TrySubmitDenominate(dmn->proTxHash, connman); + })) { + LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue is ready, masternode=%s, queue=%s\n", + dmn->proTxHash.ToString(), dsq.ToString()); + return ret; + } else { + if (m_mn_metaman.IsMixingThresholdExceeded(dmn->proTxHash, tip_mn_list.GetCounts().enabled())) { + LogPrint(BCLog::COINJOIN, "DSQUEUE -- Masternode %s is sending too many dsq messages\n", + dmn->proTxHash.ToString()); + return ret; + } + m_mn_metaman.AllowMixing(dmn->proTxHash); + + LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue, masternode=%s, queue=%s\n", + dmn->proTxHash.ToString(), dsq.ToString()); + + ForAnyCJClientMan( + [&dsq](CCoinJoinClientManager& clientman) { return clientman.MarkAlreadyJoinedQueueAsTried(dsq); }); + + m_queueman->AddQueue(dsq); + } + } // cs_ProcessDSQueue + return ret; } #endif // ENABLE_WALLET @@ -158,6 +317,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/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/init.cpp b/src/init.cpp index 1a3656130f89..06253c098fda 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2219,8 +2219,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) { diff --git a/src/interfaces/coinjoin.h b/src/interfaces/coinjoin.h index c57de9ec3fe2..0d83999d38b5 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, 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 f83760c862da..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(); } -std::unique_ptr 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 9249282f13e7..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); - std::unique_ptr 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..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/test/coinjoin_basemanager_tests.cpp b/src/test/coinjoin_basemanager_tests.cpp deleted file mode 100644 index 5e4bd4b0e1a0..000000000000 --- a/src/test/coinjoin_basemanager_tests.cpp +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright (c) 2025 The Dash Core developers -// Distributed under the MIT/X11 software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include - -#include -#include -#include - -#include - -class TestBaseManager : public CCoinJoinBaseManager -{ -public: - void PushQueue(const CCoinJoinQueue& q) - { - LOCK(cs_vecqueue); - vecCoinJoinQueue.push_back(q); - } - size_t QueueSize() const - { - LOCK(cs_vecqueue); - return vecCoinJoinQueue.size(); - } - void CallCheckQueue() { CheckQueue(); } -}; - -BOOST_FIXTURE_TEST_SUITE(coinjoin_basemanager_tests, BasicTestingSetup) - -static CCoinJoinQueue MakeQueue(int denom, int64_t nTime, bool fReady, const COutPoint& outpoint) -{ - CCoinJoinQueue q; - q.nDenom = denom; - q.masternodeOutpoint = outpoint; - q.m_protxHash = uint256::ONE; - q.nTime = nTime; - q.fReady = fReady; - return q; -} - -BOOST_AUTO_TEST_CASE(checkqueue_removes_timeouts) -{ - TestBaseManager man; - const int denom = CoinJoin::AmountToDenomination(CoinJoin::GetSmallestDenomination()); - const int64_t now = GetAdjustedTime(); - // Non-expired - man.PushQueue(MakeQueue(denom, now, false, COutPoint(uint256S("11"), 0))); - // Expired (too old) - man.PushQueue(MakeQueue(denom, now - COINJOIN_QUEUE_TIMEOUT - 1, false, COutPoint(uint256S("12"), 0))); - - BOOST_CHECK_EQUAL(man.QueueSize(), 2U); - man.CallCheckQueue(); - // One should be removed - BOOST_CHECK_EQUAL(man.QueueSize(), 1U); -} - -BOOST_AUTO_TEST_CASE(getqueueitem_marks_tried_once) -{ - TestBaseManager man; - const int denom = CoinJoin::AmountToDenomination(CoinJoin::GetSmallestDenomination()); - const int64_t now = GetAdjustedTime(); - CCoinJoinQueue dsq = MakeQueue(denom, now, false, COutPoint(uint256S("21"), 0)); - man.PushQueue(dsq); - - CCoinJoinQueue picked; - // First retrieval should succeed - BOOST_CHECK(man.GetQueueItemAndTry(picked)); - // No other items left to try (picked is marked tried inside) - CCoinJoinQueue picked2; - BOOST_CHECK(!man.GetQueueItemAndTry(picked2)); -} - -BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/coinjoin_queue_tests.cpp b/src/test/coinjoin_queue_tests.cpp index a39ad552dbc2..a3e44281b230 100644 --- a/src/test/coinjoin_queue_tests.cpp +++ b/src/test/coinjoin_queue_tests.cpp @@ -145,4 +145,47 @@ BOOST_AUTO_TEST_CASE(calculate_amount_priority_guard) BOOST_CHECK_EQUAL(CoinJoin::CalculateAmountPriority(MAX_MONEY + 1), 0); } +static CCoinJoinQueue MakeQueue(int denom, int64_t nTime, bool fReady, const COutPoint& outpoint) +{ + CCoinJoinQueue q; + q.nDenom = denom; + q.masternodeOutpoint = outpoint; + q.m_protxHash = uint256::ONE; + q.nTime = nTime; + q.fReady = fReady; + return q; +} + +BOOST_AUTO_TEST_CASE(queuemanager_checkqueue_removes_timeouts) +{ + CoinJoinQueueManager man; + const int denom = CoinJoin::AmountToDenomination(CoinJoin::GetSmallestDenomination()); + const int64_t now = GetAdjustedTime(); + // Non-expired + man.AddQueue(MakeQueue(denom, now, false, COutPoint(uint256S("11"), 0))); + // Expired (too old) + man.AddQueue(MakeQueue(denom, now - COINJOIN_QUEUE_TIMEOUT - 1, false, COutPoint(uint256S("12"), 0))); + + BOOST_CHECK_EQUAL(man.GetQueueSize(), 2); + man.CheckQueue(); + // One should be removed + BOOST_CHECK_EQUAL(man.GetQueueSize(), 1); +} + +BOOST_AUTO_TEST_CASE(queuemanager_getqueueitem_marks_tried_once) +{ + CoinJoinQueueManager man; + const int denom = CoinJoin::AmountToDenomination(CoinJoin::GetSmallestDenomination()); + const int64_t now = GetAdjustedTime(); + CCoinJoinQueue dsq = MakeQueue(denom, now, false, COutPoint(uint256S("21"), 0)); + man.AddQueue(dsq); + + CCoinJoinQueue picked; + // First retrieval should succeed + BOOST_CHECK(man.GetQueueItemAndTry(picked)); + // No other items left to try (picked is marked tried inside) + CCoinJoinQueue picked2; + BOOST_CHECK(!man.GetQueueItemAndTry(picked2)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 7bb836c40083..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,23 +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) { - 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..a33f68be6091 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -221,13 +221,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 6ed8a693f074..67bce348cebc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1652,10 +1652,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() {}