From e23b07bfa43aaec3b8133e8c79b319fdf96c7ddf Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 13 Dec 2023 01:57:41 +0700 Subject: [PATCH] refactor: create a new global object dstxManager Its class CDSTXManager has a lot of ex-static functions and data that are actually meant to be an object --- src/coinjoin/coinjoin.cpp | 27 +++++++++++++-------------- src/coinjoin/coinjoin.h | 20 ++++++++++++++++++-- src/coinjoin/server.cpp | 4 ++-- src/dsnotificationinterface.cpp | 10 +++++----- src/init.cpp | 5 +++++ src/net.cpp | 2 +- src/net_processing.cpp | 14 +++++++------- src/test/util/setup_common.cpp | 2 ++ 8 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 2b2d9ea663..9bb932da05 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -307,10 +307,6 @@ bool CCoinJoinBaseSession::IsValidInOuts(const CTxMemPool& mempool, const std::v return true; } -// Definitions for static data members -Mutex CoinJoin::cs_mapdstx; -std::map CoinJoin::mapDSTX GUARDED_BY(CoinJoin::cs_mapdstx); - // check to make sure the collateral provided by the client is valid bool CoinJoin::IsCollateralValid(CTxMemPool& mempool, const CTransaction& txCollateral) { @@ -418,14 +414,17 @@ bilingual_str CoinJoin::GetMessageByID(PoolMessage nMessageID) } } -void CoinJoin::AddDSTX(const CCoinJoinBroadcastTx& dstx) +// Definitions for static data members +std::unique_ptr dstxManager; + +void CDSTXManager::AddDSTX(const CCoinJoinBroadcastTx& dstx) { AssertLockNotHeld(cs_mapdstx); LOCK(cs_mapdstx); mapDSTX.insert(std::make_pair(dstx.tx->GetHash(), dstx)); } -CCoinJoinBroadcastTx CoinJoin::GetDSTX(const uint256& hash) +CCoinJoinBroadcastTx CDSTXManager::GetDSTX(const uint256& hash) { AssertLockNotHeld(cs_mapdstx); LOCK(cs_mapdstx); @@ -433,7 +432,7 @@ CCoinJoinBroadcastTx CoinJoin::GetDSTX(const uint256& hash) return (it == mapDSTX.end()) ? CCoinJoinBroadcastTx() : it->second; } -void CoinJoin::CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) +void CDSTXManager::CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) { AssertLockNotHeld(cs_mapdstx); LOCK(cs_mapdstx); @@ -448,21 +447,21 @@ void CoinJoin::CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHan LogPrint(BCLog::COINJOIN, "CoinJoin::CheckDSTXes -- mapDSTX.size()=%llu\n", mapDSTX.size()); } -void CoinJoin::UpdatedBlockTip(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync) +void CDSTXManager::UpdatedBlockTip(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync) { if (pindex && mn_sync.IsBlockchainSynced()) { CheckDSTXes(pindex, clhandler); } } -void CoinJoin::NotifyChainLock(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync) +void CDSTXManager::NotifyChainLock(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync) { if (pindex && mn_sync.IsBlockchainSynced()) { CheckDSTXes(pindex, clhandler); } } -void CoinJoin::UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optional nHeight) +void CDSTXManager::UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optional nHeight) { AssertLockHeld(cs_mapdstx); @@ -472,17 +471,17 @@ void CoinJoin::UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optiona } it->second.SetConfirmedHeight(nHeight); - LogPrint(BCLog::COINJOIN, "CoinJoin::%s -- txid=%s, nHeight=%d\n", __func__, tx->GetHash().ToString(), nHeight.value_or(-1)); + LogPrint(BCLog::COINJOIN, "CDSTXManager::%s -- txid=%s, nHeight=%d\n", __func__, tx->GetHash().ToString(), nHeight.value_or(-1)); } -void CoinJoin::TransactionAddedToMempool(const CTransactionRef& tx) +void CDSTXManager::TransactionAddedToMempool(const CTransactionRef& tx) { AssertLockNotHeld(cs_mapdstx); LOCK(cs_mapdstx); UpdateDSTXConfirmedHeight(tx, std::nullopt); } -void CoinJoin::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) +void CDSTXManager::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) { AssertLockNotHeld(cs_mapdstx); LOCK(cs_mapdstx); @@ -492,7 +491,7 @@ void CoinJoin::BlockConnected(const std::shared_ptr& pblock, const } } -void CoinJoin::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex*) +void CDSTXManager::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex*) { AssertLockNotHeld(cs_mapdstx); LOCK(cs_mapdstx); diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 967198a44f..c010fac9e6 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -340,8 +341,6 @@ public: // Various helpers and dstx manager implementation namespace CoinJoin { - extern Mutex cs_mapdstx; - bilingual_str GetMessageByID(PoolMessage nMessageID); /// Get the minimum/maximum number of participants for the pool @@ -353,6 +352,15 @@ namespace CoinJoin /// If the collateral is valid given by a client bool IsCollateralValid(CTxMemPool& mempool, const CTransaction& txCollateral); +} + +class CDSTXManager +{ + Mutex cs_mapdstx; + std::map mapDSTX GUARDED_BY(cs_mapdstx); + +public: + CDSTXManager() = default; void AddDSTX(const CCoinJoinBroadcastTx& dstx) LOCKS_EXCLUDED(cs_mapdstx); CCoinJoinBroadcastTx GetDSTX(const uint256& hash) LOCKS_EXCLUDED(cs_mapdstx); @@ -362,6 +370,14 @@ namespace CoinJoin void TransactionAddedToMempool(const CTransactionRef& tx) LOCKS_EXCLUDED(cs_mapdstx); void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) LOCKS_EXCLUDED(cs_mapdstx); void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex*) LOCKS_EXCLUDED(cs_mapdstx); + +private: + void CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler); + void UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optional nHeight); + }; + +extern std::unique_ptr dstxManager; + #endif // BITCOIN_COINJOIN_COINJOIN_H diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index ed36dfdb90..c81abb5359 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -333,13 +333,13 @@ void CCoinJoinServer::CommitFinalTransaction() LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- CREATING DSTX\n"); // create and sign masternode dstx transaction - if (!CoinJoin::GetDSTX(hashTx)) { + if (!::dstxManager->GetDSTX(hashTx)) { CCoinJoinBroadcastTx dstxNew(finalTransaction, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash), GetAdjustedTime()); dstxNew.Sign(); - CoinJoin::AddDSTX(dstxNew); + ::dstxManager->AddDSTX(dstxNew); } LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- TRANSMITTING DSTX\n"); diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index faee9999dc..ca28e605a7 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -67,7 +67,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con if (fInitialDownload) return; - CoinJoin::UpdatedBlockTip(pindexNew, *llmq_ctx->clhandler, m_mn_sync); + ::dstxManager->UpdatedBlockTip(pindexNew, *llmq_ctx->clhandler, m_mn_sync); #ifdef ENABLE_WALLET for (auto& pair : cj_ctx->clientman->raw()) { pair.second->UpdatedBlockTip(pindexNew); @@ -88,7 +88,7 @@ void CDSNotificationInterface::TransactionAddedToMempool(const CTransactionRef& { llmq_ctx->isman->TransactionAddedToMempool(ptx); llmq_ctx->clhandler->TransactionAddedToMempool(ptx, nAcceptTime); - CoinJoin::TransactionAddedToMempool(ptx); + ::dstxManager->TransactionAddedToMempool(ptx); } void CDSNotificationInterface::TransactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason) @@ -100,14 +100,14 @@ void CDSNotificationInterface::BlockConnected(const std::shared_ptrisman->BlockConnected(pblock, pindex); llmq_ctx->clhandler->BlockConnected(pblock, pindex); - CoinJoin::BlockConnected(pblock, pindex); + ::dstxManager->BlockConnected(pblock, pindex); } void CDSNotificationInterface::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) { llmq_ctx->isman->BlockDisconnected(pblock, pindexDisconnected); llmq_ctx->clhandler->BlockDisconnected(pblock, pindexDisconnected); - CoinJoin::BlockDisconnected(pblock, pindexDisconnected); + ::dstxManager->BlockDisconnected(pblock, pindexDisconnected); } void CDSNotificationInterface::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff) @@ -119,5 +119,5 @@ void CDSNotificationInterface::NotifyMasternodeListChanged(bool undo, const CDet void CDSNotificationInterface::NotifyChainLock(const CBlockIndex* pindex, const std::shared_ptr& clsig) { llmq_ctx->isman->NotifyChainLock(pindex); - CoinJoin::NotifyChainLock(pindex, *llmq_ctx->clhandler, m_mn_sync); + ::dstxManager->NotifyChainLock(pindex, *llmq_ctx->clhandler, m_mn_sync); } diff --git a/src/init.cpp b/src/init.cpp index ec21e2ee36..1a6fd61fcc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -68,6 +68,7 @@ #include #include +#include #include #ifdef ENABLE_WALLET #include @@ -306,6 +307,7 @@ void PrepareShutdown(NodeContext& node) ::masternodeSync.reset(); ::netfulfilledman.reset(); ::mmetaman.reset(); + ::dstxManager.reset(); // Stop and delete all indexes only after flushing background callbacks. if (g_txindex) { @@ -2216,6 +2218,9 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc } } + assert(!::dstxManager); + ::dstxManager = std::make_unique(); + assert(!::mmetaman); ::mmetaman = std::make_unique(fLoadCacheFiles); if (!::mmetaman->IsValid()) { diff --git a/src/net.cpp b/src/net.cpp index 2deec3d9ba..2777d67abc 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3758,7 +3758,7 @@ void CConnman::RelayTransaction(const CTransaction& tx) { uint256 hash = tx.GetHash(); int nInv = MSG_TX; - if (CoinJoin::GetDSTX(hash)) { + if (::dstxManager->GetDSTX(hash)) { nInv = MSG_DSTX; } CInv inv(nInv, hash); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 797ab8e15c..cabf064eec 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1834,7 +1834,7 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv) m_llmq_ctx->isman->IsLocked(inv.hash); return (!fIgnoreRecentRejects && m_recent_rejects.contains(inv.hash)) || - (inv.type == MSG_DSTX && static_cast(CoinJoin::GetDSTX(inv.hash))) || + (inv.type == MSG_DSTX && static_cast(::dstxManager->GetDSTX(inv.hash))) || m_mempool.exists(inv.hash) || (g_txindex != nullptr && g_txindex->HasTx(inv.hash)); } @@ -1882,7 +1882,7 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv) void PeerManagerImpl::RelayTransaction(const uint256& txid) { - CInv inv(CoinJoin::GetDSTX(txid) ? MSG_DSTX : MSG_TX, txid); + CInv inv(::dstxManager->GetDSTX(txid) ? MSG_DSTX : MSG_TX, txid); m_connman.ForEachNode([&inv](CNode* pnode) { pnode->PushInventory(inv); @@ -2149,7 +2149,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic if (tx) { CCoinJoinBroadcastTx dstx; if (inv.type == MSG_DSTX) { - dstx = CoinJoin::GetDSTX(inv.hash); + dstx = ::dstxManager->GetDSTX(inv.hash); } if (dstx) { m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::DSTX, dstx)); @@ -2731,7 +2731,7 @@ std::pair static ValidateDSTX(CTxMemPool& memp LogPrint(BCLog::COINJOIN, "DSTX -- Invalid DSTX structure: %s\n", hashTx.ToString()); return {false, true}; } - if (CoinJoin::GetDSTX(hashTx)) { + if (::dstxManager->GetDSTX(hashTx)) { LogPrint(BCLog::COINJOIN, "DSTX -- Already have %s, skipping...\n", hashTx.ToString()); return {true, true}; // not an error } @@ -3589,7 +3589,7 @@ void PeerManagerImpl::ProcessMessage( if (nInvType == MSG_DSTX) { LogPrint(BCLog::COINJOIN, "DSTX -- Masternode transaction accepted, txid=%s, peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); - CoinJoin::AddDSTX(dstx); + ::dstxManager->AddDSTX(dstx); } m_mempool.check(m_chainman.ActiveChainstate()); @@ -4992,7 +4992,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) pto->m_tx_relay->setInventoryTxToSend.erase(hash); if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; - int nInvType = CoinJoin::GetDSTX(hash) ? MSG_DSTX : MSG_TX; + int nInvType = ::dstxManager->GetDSTX(hash) ? MSG_DSTX : MSG_TX; queueAndMaybePushInv(CInv(nInvType, hash)); const auto islock = m_llmq_ctx->isman->GetInstantSendLockByTxid(hash); @@ -5061,7 +5061,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) vRelayExpiration.push_back(std::make_pair(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret.first)); } } - int nInvType = CoinJoin::GetDSTX(hash) ? MSG_DSTX : MSG_TX; + int nInvType = ::dstxManager->GetDSTX(hash) ? MSG_DSTX : MSG_TX; queueAndMaybePushInv(CInv(nInvType, hash)); } } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 29bfccd987..821b52f574 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -214,6 +214,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve ::sporkManager = std::make_unique(); ::governance = std::make_unique(); ::masternodeSync = std::make_unique(*m_node.connman, *::governance); + ::dstxManager = std::make_unique(); ::mmetaman = std::make_unique(/* load_cache */ false); ::netfulfilledman = std::make_unique(/* load_cache */ false); @@ -237,6 +238,7 @@ ChainTestingSetup::~ChainTestingSetup() GetMainSignals().UnregisterBackgroundSignalScheduler(); ::netfulfilledman.reset(); ::mmetaman.reset(); + ::dstxManager.reset(); ::masternodeSync.reset(); ::governance.reset(); ::sporkManager.reset();