From 1c5537542c59a6532e8fb6e80f77d6a735bdefb2 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 12 Aug 2024 19:06:11 +0000 Subject: [PATCH] coinjoin: stop providing raw access to `m_wallet_manager_map` --- src/coinjoin/client.cpp | 56 ++++++++++++++++-------------- src/coinjoin/client.h | 29 ++++++++++++---- src/dsnotificationinterface.cpp | 5 ++- src/masternode/utils.cpp | 6 ++-- src/net_processing.cpp | 6 ++-- src/wallet/test/coinjoin_tests.cpp | 4 +-- 6 files changed, 62 insertions(+), 44 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 131a03d1b60b1c..096ce8b1d17731 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -96,11 +96,9 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS } // if the queue is ready, submit if we can - if (dsq.fReady && ranges::any_of(m_walletman.raw(), - [this, &dmn](const auto &pair) { - return pair.second->TrySubmitDenominate(dmn->pdmnState->addr, - this->connman); - })) { + if (dsq.fReady && m_walletman.ForAnyCJClientMan([this, &dmn](std::unique_ptr& clientman) { + return clientman->TrySubmitDenominate(dmn->pdmnState->addr, this->connman); + })) { LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue (%s) is ready on masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString()); return {}; @@ -121,8 +119,9 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue (%s) from masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString()); - ranges::any_of(m_walletman.raw(), - [&dsq](const auto &pair) { return pair.second->MarkAlreadyJoinedQueueAsTried(dsq); }); + m_walletman.ForAnyCJClientMan([&dsq](const std::unique_ptr& clientman) { + return clientman->MarkAlreadyJoinedQueueAsTried(dsq); + }); WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq)); } @@ -155,11 +154,14 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_cha } } -CCoinJoinClientSession::CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, - const CMasternodeSync& mn_sync, const std::unique_ptr& queueman, bool is_masternode) : +CCoinJoinClientSession::CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, + CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman, + CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync, + const std::unique_ptr& queueman, + bool is_masternode) : m_wallet(wallet), m_walletman(walletman), - m_manager(*Assert(walletman.Get(wallet.GetName()))), + m_clientman(clientman), m_dmnman(dmnman), m_mn_metaman(mn_metaman), m_mn_sync(mn_sync), @@ -684,7 +686,7 @@ void CCoinJoinClientSession::CompletedTransaction(PoolMessage nMessageID) if (m_is_masternode) return; if (nMessageID == MSG_SUCCESS) { - m_manager.UpdatedSuccessBlock(); + m_clientman.UpdatedSuccessBlock(); keyHolderStorage.KeepAll(); WalletCJLogPrint(m_wallet, "CompletedTransaction -- success\n"); } else { @@ -995,7 +997,8 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(CChainState& active_chainst AssertLockNotHeld(cs_deqsessions); LOCK(cs_deqsessions); if (int(deqSessions.size()) < CCoinJoinClientOptions::GetSessions()) { - deqSessions.emplace_back(m_wallet, m_walletman, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman, m_is_masternode); + deqSessions.emplace_back(m_wallet, m_walletman, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman, + m_is_masternode); } for (auto& session : deqSessions) { if (!CheckAutomaticBackup()) return false; @@ -1100,7 +1103,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, continue; } - m_manager.AddUsedMasternode(dsq.masternodeOutpoint); + m_clientman.AddUsedMasternode(dsq.masternodeOutpoint); if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->addr)) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- skipping masternode connection, addr=%s\n", dmn->pdmnState->addr.ToString()); @@ -1145,14 +1148,14 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon // otherwise, try one randomly while (nTries < 10) { - auto dmn = m_manager.GetRandomNotUsedMasternode(); + auto dmn = m_clientman.GetRandomNotUsedMasternode(); if (!dmn) { strAutoDenomResult = _("Can't find random Masternode."); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- %s\n", strAutoDenomResult.original); return false; } - m_manager.AddUsedMasternode(dmn->collateralOutpoint); + m_clientman.AddUsedMasternode(dmn->collateralOutpoint); // skip next mn payments winners if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) { @@ -1526,7 +1529,7 @@ bool CCoinJoinClientSession::MakeCollateralAmounts(const CompactTallyItem& tally return false; } - m_manager.UpdatedSuccessBlock(); + m_clientman.UpdatedSuccessBlock(); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- txid: %s\n", __func__, strResult.original); @@ -1803,7 +1806,7 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate, con } // use the same nCachedLastSuccessBlock as for DS mixing to prevent race - m_manager.UpdatedSuccessBlock(); + m_clientman.UpdatedSuccessBlock(); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- txid: %s\n", __func__, strResult.original); @@ -1894,21 +1897,22 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const obj.pushKV("sessions", arrSessions); } -void CoinJoinWalletManager::Add(CWallet& wallet) { +void CoinJoinWalletManager::Add(CWallet& wallet) +{ { LOCK(cs_wallet_manager_map); - m_wallet_manager_map.try_emplace( - wallet.GetName(), - std::make_unique(wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync, - m_queueman, m_is_masternode) - ); + m_wallet_manager_map.try_emplace(wallet.GetName(), + std::make_unique(wallet, *this, m_dmnman, m_mn_metaman, + m_mn_sync, m_queueman, m_is_masternode)); } g_wallet_init_interface.InitCoinJoinSettings(*this); } -void CoinJoinWalletManager::DoMaintenance() { - for (auto& [wallet_str, walletman] : raw()) { - walletman->DoMaintenance(m_chainstate, m_connman, m_mempool); +void CoinJoinWalletManager::DoMaintenance() +{ + LOCK(cs_wallet_manager_map); + for (auto& [_, clientman] : m_wallet_manager_map) { + clientman->DoMaintenance(m_chainstate, m_connman, m_mempool); } } diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index a49aa328c1e5ac..84b3ab930bc62d 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -94,11 +94,24 @@ class CoinJoinWalletManager { CCoinJoinClientManager* Get(const std::string& name) const; - const wallet_name_cjman_map& raw() const + template + void ForEachCJClientMan(Callable&& func) { LOCK(cs_wallet_manager_map); - return m_wallet_manager_map; - } + for (auto&& [_, clientman] : m_wallet_manager_map) { + func(clientman); + } + }; + + template + bool ForAnyCJClientMan(Callable&& func) + { + LOCK(cs_wallet_manager_map); + for (auto&& [_, clientman] : m_wallet_manager_map) { + if (func(clientman)) return true; + } + return false; + }; private: CChainState& m_chainstate; @@ -111,7 +124,7 @@ class CoinJoinWalletManager { const bool m_is_masternode; - mutable RecursiveMutex cs_wallet_manager_map; + mutable Mutex cs_wallet_manager_map; wallet_name_cjman_map m_wallet_manager_map GUARDED_BY(cs_wallet_manager_map); }; @@ -120,7 +133,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession private: CWallet& m_wallet; CoinJoinWalletManager& m_walletman; - CCoinJoinClientManager& m_manager; + CCoinJoinClientManager& m_clientman; CDeterministicMNManager& m_dmnman; CMasternodeMetaMan& m_mn_metaman; const CMasternodeSync& m_mn_sync; @@ -175,8 +188,10 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession void SetNull() override EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); public: - explicit CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, - const CMasternodeSync& mn_sync, const std::unique_ptr& queueman, bool is_masternode); + explicit CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, + CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman, + CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync, + const std::unique_ptr& queueman, bool is_masternode); void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv); diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index e5b7613650cf6b..ebf94562c4a271 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -89,9 +89,8 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con m_cj_ctx->dstxman->UpdatedBlockTip(pindexNew, *m_llmq_ctx->clhandler, m_mn_sync); #ifdef ENABLE_WALLET - for (auto& pair : m_cj_ctx->walletman->raw()) { - pair.second->UpdatedBlockTip(pindexNew); - } + m_cj_ctx->walletman->ForEachCJClientMan( + [&pindexNew](std::unique_ptr& clientman) { clientman->UpdatedBlockTip(pindexNew); }); #endif // ENABLE_WALLET m_llmq_ctx->isman->UpdatedBlockTip(pindexNew); diff --git a/src/masternode/utils.cpp b/src/masternode/utils.cpp index 90311288f92ab8..6bf48f2b5cbb6c 100644 --- a/src/masternode/utils.cpp +++ b/src/masternode/utils.cpp @@ -23,9 +23,9 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, CDeterministicMNManager& std::vector vecDmns; // will be empty when no wallet #ifdef ENABLE_WALLET - for (auto& pair : cj_ctx.walletman->raw()) { - pair.second->GetMixingMasternodesInfo(vecDmns); - } + cj_ctx.walletman->ForEachCJClientMan([&vecDmns](const std::unique_ptr& clientman) { + clientman->GetMixingMasternodesInfo(vecDmns); + }); #endif // ENABLE_WALLET // Don't disconnect masternode connections when we have less then the desired amount of outbound nodes diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c5a76acf10d0b4..db6d9c70ff53c1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5095,9 +5095,9 @@ void PeerManagerImpl::ProcessMessage( //probably one the extensions #ifdef ENABLE_WALLET ProcessPeerMsgRet(m_cj_ctx->queueman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); - for (auto& pair : m_cj_ctx->walletman->raw()) { - pair.second->ProcessMessage(pfrom, m_chainman.ActiveChainstate(), m_connman, m_mempool, msg_type, vRecv); - } + m_cj_ctx->walletman->ForEachCJClientMan([this, &pfrom, &msg_type, &vRecv](std::unique_ptr& clientman) { + clientman->ProcessMessage(pfrom, m_chainman.ActiveChainstate(), m_connman, m_mempool, msg_type, vRecv); + }); #endif // ENABLE_WALLET ProcessPeerMsgRet(m_cj_ctx->server->ProcessMessage(pfrom, msg_type, vRecv), pfrom); ProcessPeerMsgRet(m_sporkman.ProcessMessage(pfrom, m_connman, *this, msg_type, vRecv), pfrom); diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index c807888e42e937..bbb567ffce3e49 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -209,8 +209,8 @@ class CTransactionBuilderTestSetup : public TestChain100Setup BOOST_FIXTURE_TEST_CASE(coinjoin_manager_start_stop_tests, CTransactionBuilderTestSetup) { - BOOST_CHECK_EQUAL(m_node.cj_ctx->walletman->raw().size(), 1); - auto& cj_man = m_node.cj_ctx->walletman->raw().begin()->second; + CCoinJoinClientManager* cj_man = m_node.cj_ctx->walletman->Get(""); + BOOST_ASSERT(cj_man != nullptr); BOOST_CHECK_EQUAL(cj_man->IsMixing(), false); BOOST_CHECK_EQUAL(cj_man->StartMixing(), true); BOOST_CHECK_EQUAL(cj_man->IsMixing(), true);