From fae5696d820ee35562a7de043625fb59817c311b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 29 Mar 2024 21:49:28 +0000 Subject: [PATCH 1/7] init: move CActiveMasternodeManager construction before PeerManager --- src/init.cpp | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 64ee07832e1b..cd43100fcdc5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1713,6 +1713,22 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc ::masternodeSync = std::make_unique(*node.connman, *node.netfulfilledman, *node.govman); node.mn_sync = ::masternodeSync.get(); + fMasternodeMode = false; + std::string strMasterNodeBLSPrivKey = args.GetArg("-masternodeblsprivkey", ""); + if (!strMasterNodeBLSPrivKey.empty()) { + CBLSSecretKey keyOperator(ParseHex(strMasterNodeBLSPrivKey)); + if (!keyOperator.IsValid()) { + return InitError(_("Invalid masternodeblsprivkey. Please see documentation.")); + } + fMasternodeMode = true; + { + // Create and register activeMasternodeManager, will init later in ThreadImport + ::activeMasternodeManager = std::make_unique(keyOperator, *node.connman, ::deterministicMNManager); + node.mn_activeman = ::activeMasternodeManager.get(); + RegisterValidationInterface(node.mn_activeman); + } + } + assert(!node.peerman); node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(), *node.scheduler, chainman, *node.mempool, *node.govman, *node.sporkman, @@ -1844,22 +1860,6 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc ); RegisterValidationInterface(pdsNotificationInterface); - fMasternodeMode = false; - std::string strMasterNodeBLSPrivKey = args.GetArg("-masternodeblsprivkey", ""); - if (!strMasterNodeBLSPrivKey.empty()) { - CBLSSecretKey keyOperator(ParseHex(strMasterNodeBLSPrivKey)); - if (!keyOperator.IsValid()) { - return InitError(_("Invalid masternodeblsprivkey. Please see documentation.")); - } - fMasternodeMode = true; - { - // Create and register activeMasternodeManager, will init later in ThreadImport - ::activeMasternodeManager = std::make_unique(keyOperator, *node.connman, ::deterministicMNManager); - node.mn_activeman = ::activeMasternodeManager.get(); - RegisterValidationInterface(node.mn_activeman); - } - } - // ********************************************************* Step 7a: Load sporks if (!node.sporkman->LoadCache()) { From cf940e8d8568b94ea6303a7f32d4565117297934 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 9 Apr 2024 20:45:28 +0000 Subject: [PATCH 2/7] init: decouple CMasternodeMetaMan construction from initialization --- src/coinjoin/client.cpp | 4 ++++ src/coinjoin/server.cpp | 3 +++ src/evo/mnauth.cpp | 2 ++ src/evo/mnauth.h | 5 +++++ src/governance/governance.cpp | 3 +++ src/governance/object.cpp | 2 ++ src/init.cpp | 13 +++++++------ src/llmq/dkgsession.cpp | 2 ++ src/llmq/utils.cpp | 2 ++ src/masternode/meta.cpp | 17 +++++++++-------- src/masternode/meta.h | 6 ++++-- src/test/util/setup_common.cpp | 8 ++++---- 12 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index ef83ad6bf059..007bbe55f200 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -42,6 +42,8 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessMessage(const CNode& peer, std::s PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataStream& vRecv) { + assert(::mmetaman->IsValid()); + CCoinJoinQueue dsq; vRecv >> dsq; @@ -1117,6 +1119,8 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman) { + assert(::mmetaman->IsValid()); + if (!CCoinJoinClientOptions::IsEnabled()) return false; if (nBalanceNeedsAnonymized <= 0) return false; diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 148fa6b309a3..d1d279a57097 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -44,6 +44,7 @@ PeerMsgRet CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_typ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) { assert(m_mn_activeman); + assert(::mmetaman->IsValid()); if (IsSessionReady()) { // too many users in this session already, reject new ones @@ -110,6 +111,8 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv) { + assert(::mmetaman->IsValid()); + CCoinJoinQueue dsq; vRecv >> dsq; diff --git a/src/evo/mnauth.cpp b/src/evo/mnauth.cpp index e87ee5616351..032492ebc6ad 100644 --- a/src/evo/mnauth.cpp +++ b/src/evo/mnauth.cpp @@ -63,6 +63,8 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip) PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, const CDeterministicMNList& tip_mn_list, std::string_view msg_type, CDataStream& vRecv) { + assert(::mmetaman->IsValid()); + if (msg_type != NetMsgType::MNAUTH || !::masternodeSync->IsBlockchainSynced()) { // we can't verify MNAUTH messages when we don't have the latest MN list return {}; diff --git a/src/evo/mnauth.h b/src/evo/mnauth.h index 21b1c0d62c28..e4311e3c0eff 100644 --- a/src/evo/mnauth.h +++ b/src/evo/mnauth.h @@ -49,6 +49,11 @@ class CMNAuth } static void PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip); + + /** + * @pre CMasternodeMetaMan's database must be successfully loaded before + * attempting to call this function regardless of sync state + */ static PeerMsgRet ProcessMessage(CNode& peer, CConnman& connman, const CDeterministicMNList& tip_mn_list, std::string_view msg_type, CDataStream& vRecv); static void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff, CConnman& connman); }; diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index b7569017f7d0..883bd030346c 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -344,6 +344,9 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman void CGovernanceManager::UpdateCachesAndClean() { + // Return if meta manager hasn't had a chance to load its database yet + if (!::mmetaman->IsValid()) return; + // Return on initial sync, spammed the debug.log and provided no use if (::masternodeSync == nullptr || !::masternodeSync->IsBlockchainSynced()) return; diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 117af0846684..eed3301ccd73 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -82,6 +82,8 @@ CGovernanceObject::CGovernanceObject(const CGovernanceObject& other) : bool CGovernanceObject::ProcessVote(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, const CGovernanceVote& vote, CGovernanceException& exception) { + assert(::mmetaman->IsValid()); + LOCK(cs); // do not process already known valid votes twice diff --git a/src/init.cpp b/src/init.cpp index cd43100fcdc5..87e9d00fc2ca 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -305,13 +305,13 @@ void PrepareShutdown(NodeContext& node) // After all scheduled tasks have been flushed, destroy pointers // and reset all to nullptr. - node.mn_metaman = nullptr; - ::mmetaman.reset(); node.mn_sync = nullptr; ::masternodeSync.reset(); node.sporkman.reset(); node.govman.reset(); node.netfulfilledman.reset(); + node.mn_metaman = nullptr; + ::mmetaman.reset(); // Stop and delete all indexes only after flushing background callbacks. if (g_txindex) { @@ -1676,6 +1676,10 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc node.chainman = &g_chainman; ChainstateManager& chainman = *Assert(node.chainman); + assert(!::mmetaman); + ::mmetaman = std::make_unique(); + node.mn_metaman = ::mmetaman.get(); + assert(!node.netfulfilledman); node.netfulfilledman = std::make_unique(); @@ -2221,10 +2225,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc } } - assert(!::mmetaman); - ::mmetaman = std::make_unique(fLoadCacheFiles); - node.mn_metaman = ::mmetaman.get(); - if (!node.mn_metaman->IsValid()) { + if (!node.mn_metaman->LoadCache(fLoadCacheFiles)) { auto file_path = (GetDataDir() / "mncache.dat").string(); if (fLoadCacheFiles) { return InitError(strprintf(_("Failed to load masternode cache from %s"), file_path)); diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index 8b4345940c4a..cb46b184ccc2 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -450,6 +450,8 @@ void CDKGSession::VerifyAndComplain(CDKGPendingMessages& pendingMessages) void CDKGSession::VerifyConnectionAndMinProtoVersions() const { + assert(::mmetaman->IsValid()); + if (!IsQuorumPoseEnabled(params.type, m_sporkman)) { return; } diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 1db6df816ad4..2f2ed213c45b 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -812,6 +812,8 @@ void AddQuorumProbeConnections(const Consensus::LLMQParams& llmqParams, CConnman const CDeterministicMNList& tip_mn_list, gsl::not_null pQuorumBaseBlockIndex, const uint256 &myProTxHash) { + assert(::mmetaman->IsValid()); + if (!IsQuorumPoseEnabled(llmqParams.type, sporkman)) { return; } diff --git a/src/masternode/meta.cpp b/src/masternode/meta.cpp index 2d01e47bab35..7c7861640c98 100644 --- a/src/masternode/meta.cpp +++ b/src/masternode/meta.cpp @@ -13,15 +13,16 @@ std::unique_ptr mmetaman; const std::string MasternodeMetaStore::SERIALIZATION_VERSION_STRING = "CMasternodeMetaMan-Version-3"; -CMasternodeMetaMan::CMasternodeMetaMan(bool load_cache) : - m_db{std::make_unique("mncache.dat", "magicMasternodeCache")}, - is_valid{ - [&]() -> bool { - assert(m_db != nullptr); - return load_cache ? m_db->Load(*this) : m_db->Store(*this); - }() - } +CMasternodeMetaMan::CMasternodeMetaMan() : + m_db{std::make_unique("mncache.dat", "magicMasternodeCache")} +{ +} + +bool CMasternodeMetaMan::LoadCache(bool load_cache) { + assert(m_db != nullptr); + is_valid = load_cache ? m_db->Load(*this) : m_db->Store(*this); + return is_valid; } CMasternodeMetaMan::~CMasternodeMetaMan() diff --git a/src/masternode/meta.h b/src/masternode/meta.h index 68550cbe49b5..b2c4dfc3718c 100644 --- a/src/masternode/meta.h +++ b/src/masternode/meta.h @@ -150,14 +150,16 @@ class CMasternodeMetaMan : public MasternodeMetaStore private: const std::unique_ptr m_db; - const bool is_valid{false}; + bool is_valid{false}; std::vector vecDirtyGovernanceObjectHashes GUARDED_BY(cs); public: - explicit CMasternodeMetaMan(bool load_cache); + explicit CMasternodeMetaMan(); ~CMasternodeMetaMan(); + bool LoadCache(bool load_cache); + bool IsValid() const { return is_valid; } CMasternodeMetaInfoPtr GetMetaInfo(const uint256& proTxHash, bool fCreate = true); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index d40b0ecb7a66..9bbaad0bb7cf 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -225,13 +225,13 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_node.connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests. + ::mmetaman = std::make_unique(); + m_node.mn_metaman = ::mmetaman.get(); m_node.netfulfilledman = std::make_unique(); m_node.sporkman = std::make_unique(); m_node.govman = std::make_unique(*m_node.netfulfilledman, ::deterministicMNManager); ::masternodeSync = std::make_unique(*m_node.connman, *m_node.netfulfilledman, *m_node.govman); m_node.mn_sync = ::masternodeSync.get(); - ::mmetaman = std::make_unique(/* load_cache */ false); - m_node.mn_metaman = ::mmetaman.get(); // Start script-checking threads. Set g_parallel_script_checks to true so they are used. constexpr int script_check_threads = 2; @@ -245,13 +245,13 @@ ChainTestingSetup::~ChainTestingSetup() StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); - m_node.mn_metaman = nullptr; - ::mmetaman.reset(); m_node.mn_sync = nullptr; ::masternodeSync.reset(); m_node.govman.reset(); m_node.sporkman.reset(); m_node.netfulfilledman.reset(); + m_node.mn_metaman = nullptr; + ::mmetaman.reset(); m_node.connman.reset(); m_node.addrman.reset(); m_node.args = nullptr; From ff825acaacaf96e42665e5ecb25a87d83ef87ef1 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 9 Apr 2024 20:38:27 +0000 Subject: [PATCH 3/7] init: load databases of governance dependencies before loading its own The order in which they're loaded is dictated by the order in which they are initialized. This also lets us get rid of that fast-fail because governance db was always going to load before mncache db and replace it with an assert. Also, rename UpdateCachesAndClean to CheckAndRemove in CGovernanceManager as CheckAndRemove is already an existing alias to UpdateCachesAndClean and CheckAndRemove is associated with similar functionality in C{NetFulfilledRequest, Spork}Manager --- src/dsnotificationinterface.cpp | 4 +++- src/governance/governance.cpp | 7 +++---- src/governance/governance.h | 4 +--- src/governance/object.cpp | 2 +- src/init.cpp | 24 ++++++++++++------------ 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index bfe452653cad..4bb1010a03b4 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -134,7 +134,9 @@ void CDSNotificationInterface::BlockDisconnected(const std::shared_ptr& clsig) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 883bd030346c..fab40235983f 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -342,10 +342,9 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman GetMainSignals().NotifyGovernanceObject(std::make_shared(govobj.Object())); } -void CGovernanceManager::UpdateCachesAndClean() +void CGovernanceManager::CheckAndRemove() { - // Return if meta manager hasn't had a chance to load its database yet - if (!::mmetaman->IsValid()) return; + assert(::mmetaman->IsValid()); // Return on initial sync, spammed the debug.log and provided no use if (::masternodeSync == nullptr || !::masternodeSync->IsBlockchainSynced()) return; @@ -797,7 +796,7 @@ void CGovernanceManager::DoMaintenance(CConnman& connman) RequestOrphanObjects(connman); // CHECK AND REMOVE - REPROCESS GOVERNANCE OBJECTS - UpdateCachesAndClean(); + CheckAndRemove(); } bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) diff --git a/src/governance/governance.h b/src/governance/governance.h index 152f13fbfed8..94a1cbccb68e 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -304,9 +304,7 @@ class CGovernanceManager : public GovernanceStore void AddGovernanceObject(CGovernanceObject& govobj, CConnman& connman, const CNode* pfrom = nullptr); - void UpdateCachesAndClean(); - - void CheckAndRemove() { UpdateCachesAndClean(); } + void CheckAndRemove(); UniValue ToJson() const; diff --git a/src/governance/object.cpp b/src/governance/object.cpp index eed3301ccd73..3553ccafb4ce 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -426,7 +426,7 @@ bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, case GovernanceObject::PROPOSAL: { CProposalValidator validator(GetDataAsHexString()); // Note: It's ok to have expired proposals - // they are going to be cleared by CGovernanceManager::UpdateCachesAndClean() + // they are going to be cleared by CGovernanceManager::CheckAndRemove() // TODO: should they be tagged as "expired" to skip vote downloading? if (!validator.Validate(false)) { strError = strprintf("Invalid proposal data, error messages: %s", validator.GetErrorMessages()); diff --git a/src/init.cpp b/src/init.cpp index 87e9d00fc2ca..d3646f40152b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2215,14 +2215,12 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc bool fLoadCacheFiles = !(fReindex || fReindexChainState) && (::ChainActive().Tip() != nullptr); - if (!fDisableGovernance) { - if (!node.govman->LoadCache(fLoadCacheFiles)) { - auto file_path = (GetDataDir() / "governance.dat").string(); - if (fLoadCacheFiles && !fDisableGovernance) { - return InitError(strprintf(_("Failed to load governance cache from %s"), file_path)); - } - return InitError(strprintf(_("Failed to clear governance cache at %s"), file_path)); + if (!node.netfulfilledman->LoadCache(fLoadCacheFiles)) { + auto file_path = (GetDataDir() / "netfulfilled.dat").string(); + if (fLoadCacheFiles) { + return InitError(strprintf(_("Failed to load fulfilled requests cache from %s"), file_path)); } + return InitError(strprintf(_("Failed to clear fulfilled requests cache at %s"), file_path)); } if (!node.mn_metaman->LoadCache(fLoadCacheFiles)) { @@ -2233,12 +2231,14 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc return InitError(strprintf(_("Failed to clear masternode cache at %s"), file_path)); } - if (!node.netfulfilledman->LoadCache(fLoadCacheFiles)) { - auto file_path = (GetDataDir() / "netfulfilled.dat").string(); - if (fLoadCacheFiles) { - return InitError(strprintf(_("Failed to load fulfilled requests cache from %s"), file_path)); + if (!fDisableGovernance) { + if (!node.govman->LoadCache(fLoadCacheFiles)) { + auto file_path = (GetDataDir() / "governance.dat").string(); + if (fLoadCacheFiles && !fDisableGovernance) { + return InitError(strprintf(_("Failed to load governance cache from %s"), file_path)); + } + return InitError(strprintf(_("Failed to clear governance cache at %s"), file_path)); } - return InitError(strprintf(_("Failed to clear fulfilled requests cache at %s"), file_path)); } // ********************************************************* Step 8: start indexers From 2a4fdbff1ad1a7af458b71597438212829c448ed Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 30 Mar 2024 16:23:45 +0000 Subject: [PATCH 4/7] refactor: trim globals use in net threads and functions --- src/init.cpp | 4 +- src/masternode/utils.cpp | 6 ++- src/masternode/utils.h | 4 +- src/net.cpp | 78 ++++++++++++++++++++------------------- src/net.h | 29 +++++++++------ src/net_processing.cpp | 5 ++- src/node/interfaces.cpp | 2 +- src/rpc/net.cpp | 2 +- src/test/fuzz/connman.cpp | 2 +- 9 files changed, 73 insertions(+), 59 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d3646f40152b..d6b36ff28374 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2306,7 +2306,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc node.scheduler->scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(*node.netfulfilledman)), std::chrono::minutes{1}); node.scheduler->scheduleEvery(std::bind(&CMasternodeSync::DoMaintenance, std::ref(*node.mn_sync)), std::chrono::seconds{1}); - node.scheduler->scheduleEvery(std::bind(&CMasternodeUtils::DoMaintenance, std::ref(*node.connman), std::ref(*node.mn_sync), std::ref(*node.cj_ctx)), std::chrono::minutes{1}); + node.scheduler->scheduleEvery(std::bind(&CMasternodeUtils::DoMaintenance, std::ref(*node.connman), std::ref(*node.dmnman), std::ref(*node.mn_sync), std::ref(*node.cj_ctx)), std::chrono::minutes{1}); node.scheduler->scheduleEvery(std::bind(&CDeterministicMNManager::DoMaintenance, std::ref(*node.dmnman)), std::chrono::seconds{10}); if (!fDisableGovernance) { @@ -2534,7 +2534,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc connOptions.m_i2p_accept_incoming = args.GetBoolArg("-i2pacceptincoming", true); - if (!node.connman->Start(*node.scheduler, connOptions)) { + if (!node.connman->Start(*node.dmnman, *node.mn_metaman, *node.mn_sync, *node.scheduler, connOptions)) { return false; } diff --git a/src/masternode/utils.cpp b/src/masternode/utils.cpp index 29469139558e..3d20a7b4b794 100644 --- a/src/masternode/utils.cpp +++ b/src/masternode/utils.cpp @@ -15,7 +15,8 @@ #include #include -void CMasternodeUtils::DoMaintenance(CConnman& connman, const CMasternodeSync& mn_sync, const CJContext& cj_ctx) +void CMasternodeUtils::DoMaintenance(CConnman& connman, CDeterministicMNManager& dmnman, + const CMasternodeSync& mn_sync, const CJContext& cj_ctx) { if (!mn_sync.IsBlockchainSynced()) return; if (ShutdownRequested()) return; @@ -53,8 +54,9 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, const CMasternodeSync& m // we're only disconnecting m_masternode_connection connections if (!pnode->m_masternode_connection) return; if (!pnode->GetVerifiedProRegTxHash().IsNull()) { + const auto tip_mn_list = dmnman.GetListAtChainTip(); // keep _verified_ LLMQ connections - if (connman.IsMasternodeQuorumNode(pnode)) { + if (connman.IsMasternodeQuorumNode(pnode, tip_mn_list)) { return; } // keep _verified_ LLMQ relay connections diff --git a/src/masternode/utils.h b/src/masternode/utils.h index 9ff6b0ed3ad9..3bb56edca942 100644 --- a/src/masternode/utils.h +++ b/src/masternode/utils.h @@ -6,13 +6,15 @@ #define BITCOIN_MASTERNODE_UTILS_H class CConnman; +class CDeterministicMNManager; class CMasternodeSync; struct CJContext; class CMasternodeUtils { public: - static void DoMaintenance(CConnman &connman, const CMasternodeSync& mn_sync, const CJContext& cj_ctx); + static void DoMaintenance(CConnman &connman, CDeterministicMNManager& dmnman, + const CMasternodeSync& mn_sync, const CJContext& cj_ctx); }; #endif // BITCOIN_MASTERNODE_UTILS_H diff --git a/src/net.cpp b/src/net.cpp index 59e15169e70f..de7cd85dfcf0 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1119,7 +1119,7 @@ bool CConnman::AttemptToEvictConnection() return false; } -void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { +void CConnman::AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync) { struct sockaddr_storage sockaddr; socklen_t len = sizeof(sockaddr); SOCKET hSocket = accept(hListenSocket.socket, (struct sockaddr*)&sockaddr, &len); @@ -1141,13 +1141,14 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { NetPermissionFlags permissionFlags = NetPermissionFlags::None; hListenSocket.AddSocketPermissionFlags(permissionFlags); - CreateNodeFromAcceptedSocket(hSocket, permissionFlags, addr_bind, addr); + CreateNodeFromAcceptedSocket(hSocket, permissionFlags, addr_bind, addr, mn_sync); } void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, NetPermissionFlags permissionFlags, const CAddress& addr_bind, - const CAddress& addr) + const CAddress& addr, + CMasternodeSync& mn_sync) { int nInbound = 0; int nVerifiedInboundMasternodes = 0; @@ -1236,7 +1237,7 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, } // don't accept incoming connections until blockchain is synced - if(fMasternodeMode && !::masternodeSync->IsBlockchainSynced()) { + if (fMasternodeMode && !mn_sync.IsBlockchainSynced()) { LogPrint(BCLog::NET, "AcceptConnection -- blockchain is not synced yet, skipping inbound connection attempt\n"); CloseSocket(hSocket); return; @@ -1401,7 +1402,7 @@ void CConnman::DisconnectNodes() } } -void CConnman::NotifyNumConnectionsChanged() +void CConnman::NotifyNumConnectionsChanged(CMasternodeSync& mn_sync) { size_t vNodesSize; { @@ -1412,7 +1413,7 @@ void CConnman::NotifyNumConnectionsChanged() // If we had zero connections before and new connections now or if we just dropped // to zero connections reset the sync process if its outdated. if ((vNodesSize > 0 && nPrevNodeCount == 0) || (vNodesSize == 0 && nPrevNodeCount > 0)) { - ::masternodeSync->Reset(); + mn_sync.Reset(); } if(vNodesSize != nPrevNodeCount) { @@ -1779,7 +1780,7 @@ void CConnman::SocketEvents(std::set &recv_set, std::set &send_s } } -void CConnman::SocketHandler() +void CConnman::SocketHandler(CMasternodeSync& mn_sync) { bool fOnlyPoll = [this]() { // check if we have work to do and thus should avoid waiting for events @@ -1826,7 +1827,7 @@ void CConnman::SocketHandler() { if (recv_set.count(hListenSocket.socket) > 0) { - AcceptConnection(hListenSocket); + AcceptConnection(hListenSocket, mn_sync); } } @@ -2041,7 +2042,7 @@ size_t CConnman::SocketRecvData(CNode *pnode) return (size_t)nBytes; } -void CConnman::ThreadSocketHandler() +void CConnman::ThreadSocketHandler(CMasternodeSync& mn_sync) { int64_t nLastCleanupNodes = 0; @@ -2049,7 +2050,7 @@ void CConnman::ThreadSocketHandler() { // Handle sockets before we do the next round of disconnects. This allows us to flush send buffers one last time // before actually closing sockets. Receiving is however skipped in case a peer is pending to be disconnected - SocketHandler(); + SocketHandler(mn_sync); if (GetTimeMillis() - nLastCleanupNodes > 1000) { ForEachNode(AllNodes, [&](CNode* pnode) { if (InactivityCheck(*pnode)) pnode->fDisconnect = true; @@ -2057,7 +2058,7 @@ void CConnman::ThreadSocketHandler() nLastCleanupNodes = GetTimeMillis(); } DisconnectNodes(); - NotifyNumConnectionsChanged(); + NotifyNumConnectionsChanged(mn_sync); } } @@ -2273,7 +2274,7 @@ int CConnman::GetExtraBlockRelayCount() return std::max(block_relay_peers - m_max_outbound_block_relay, 0); } -void CConnman::ThreadOpenConnections(const std::vector connect) +void CConnman::ThreadOpenConnections(const std::vector connect, CDeterministicMNManager& dmnman) { FastRandomContext rng; // Connect to specific addresses @@ -2455,7 +2456,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) addrman.ResolveCollisions(); - auto mnList = deterministicMNManager->GetListAtChainTip(); + auto mnList = dmnman.GetListAtChainTip(); int64_t nANow = GetAdjustedTime(); int nTries = 0; @@ -2668,14 +2669,13 @@ void CConnman::ThreadOpenAddedConnections() } } -void CConnman::ThreadOpenMasternodeConnections() +void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, + CMasternodeSync& mn_sync) { // Connecting to specific addresses, no masternode connections available if (gArgs.IsArgSet("-connect") && gArgs.GetArgs("-connect").size() > 0) return; - assert(::mmetaman != nullptr); - auto& chainParams = Params(); bool didConnect = false; @@ -2690,7 +2690,7 @@ void CConnman::ThreadOpenMasternodeConnections() didConnect = false; - if (!fNetworkActive || !::masternodeSync->IsBlockchainSynced()) + if (!fNetworkActive || !mn_sync.IsBlockchainSynced()) continue; std::set connectedNodes; @@ -2703,7 +2703,7 @@ void CConnman::ThreadOpenMasternodeConnections() } }); - auto mnList = deterministicMNManager->GetListAtChainTip(); + auto mnList = dmnman.GetListAtChainTip(); if (interruptNet) return; @@ -2742,7 +2742,7 @@ void CConnman::ThreadOpenMasternodeConnections() continue; } if (!connectedNodes.count(addr2) && !IsMasternodeOrDisconnectRequested(addr2) && !connectedProRegTxHashes.count(proRegTxHash)) { - int64_t lastAttempt = mmetaman->GetMetaInfo(dmn->proTxHash)->GetLastOutboundAttempt(); + int64_t lastAttempt = mn_metaman.GetMetaInfo(dmn->proTxHash)->GetLastOutboundAttempt(); // back off trying connecting to an address if we already tried recently if (nANow - lastAttempt < chainParams.LLMQConnectionRetryTimeout()) { continue; @@ -2766,14 +2766,14 @@ void CConnman::ThreadOpenMasternodeConnections() bool connectedAndOutbound = connectedProRegTxHashes.count(dmn->proTxHash) && !connectedProRegTxHashes[dmn->proTxHash]; if (connectedAndOutbound) { // we already have an outbound connection to this MN so there is no theed to probe it again - mmetaman->GetMetaInfo(dmn->proTxHash)->SetLastOutboundSuccess(nANow); + mn_metaman.GetMetaInfo(dmn->proTxHash)->SetLastOutboundSuccess(nANow); it = masternodePendingProbes.erase(it); continue; } ++it; - int64_t lastAttempt = mmetaman->GetMetaInfo(dmn->proTxHash)->GetLastOutboundAttempt(); + int64_t lastAttempt = mn_metaman.GetMetaInfo(dmn->proTxHash)->GetLastOutboundAttempt(); // back off trying connecting to an address if we already tried recently if (nANow - lastAttempt < chainParams.LLMQConnectionRetryTimeout()) { continue; @@ -2824,7 +2824,7 @@ void CConnman::ThreadOpenMasternodeConnections() didConnect = true; - mmetaman->GetMetaInfo(connectToDmn->proTxHash)->SetLastOutboundAttempt(nANow); + mn_metaman.GetMetaInfo(connectToDmn->proTxHash)->SetLastOutboundAttempt(nANow); OpenMasternodeConnection(CAddress(connectToDmn->pdmnState->addr, NODE_NETWORK), isProbe); // should be in the list now if connection was opened @@ -2837,7 +2837,7 @@ void CConnman::ThreadOpenMasternodeConnections() if (!connected) { LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- connection failed for masternode %s, service=%s\n", __func__, connectToDmn->proTxHash.ToString(), connectToDmn->pdmnState->addr.ToString(false)); // Will take a few consequent failed attempts to PoSe-punish a MN. - if (mmetaman->GetMetaInfo(connectToDmn->proTxHash)->OutboundFailedTooManyTimes()) { + if (mn_metaman.GetMetaInfo(connectToDmn->proTxHash)->OutboundFailedTooManyTimes()) { LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- failed to connect to masternode %s too many times\n", __func__, connectToDmn->proTxHash.ToString()); } } @@ -2976,7 +2976,7 @@ void CConnman::ThreadMessageHandler() } } -void CConnman::ThreadI2PAcceptIncoming() +void CConnman::ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) { static constexpr auto err_wait_begin = 1s; static constexpr auto err_wait_cap = 5min; @@ -3011,7 +3011,7 @@ void CConnman::ThreadI2PAcceptIncoming() } CreateNodeFromAcceptedSocket(conn.sock->Release(), NetPermissionFlags::None, - CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE}); + CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE}, mn_sync); } } @@ -3153,7 +3153,7 @@ void Discover() #endif } -void CConnman::SetNetworkActive(bool active) +void CConnman::SetNetworkActive(bool active, CMasternodeSync* const mn_sync) { LogPrintf("%s: %s\n", __func__, active); @@ -3163,11 +3163,11 @@ void CConnman::SetNetworkActive(bool active) fNetworkActive = active; - // masternodeSync is nullptr during app initialization with `-networkactive=` - if (::masternodeSync) { + // mn_sync is nullptr during app initialization with `-networkactive=` + if (mn_sync) { // Always call the Reset() if the network gets enabled/disabled to make sure the sync process // gets a reset if its outdated.. - ::masternodeSync->Reset(); + mn_sync->Reset(); } uiInterface.NotifyNetworkActiveChanged(fNetworkActive); @@ -3180,7 +3180,7 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, CAddrMan& addrman_in, b Options connOptions; Init(connOptions); - SetNetworkActive(network_active); + SetNetworkActive(network_active, /* mn_sync = */ nullptr); } NodeId CConnman::GetNewNodeId() @@ -3235,7 +3235,8 @@ bool CConnman::InitBinds( return fBound; } -bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) +bool CConnman::Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, + CScheduler& scheduler, const Options& connOptions) { Init(connOptions); @@ -3371,7 +3372,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) #endif // Send and receive from sockets, accept connections - threadSocketHandler = std::thread(&util::TraceThread, "net", [this] { ThreadSocketHandler(); }); + threadSocketHandler = std::thread(&util::TraceThread, "net", [this, &mn_sync] { ThreadSocketHandler(mn_sync); }); if (!gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED)) LogPrintf("DNS seeding disabled\n"); @@ -3392,18 +3393,20 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) if (connOptions.m_use_addrman_outgoing || !connOptions.m_specified_outgoing.empty()) { threadOpenConnections = std::thread( &util::TraceThread, "opencon", - [this, connect = connOptions.m_specified_outgoing] { ThreadOpenConnections(connect); }); + [this, connect = connOptions.m_specified_outgoing, &dmnman] { ThreadOpenConnections(connect, dmnman); }); } // Initiate masternode connections - threadOpenMasternodeConnections = std::thread(&util::TraceThread, "mncon", [this] { ThreadOpenMasternodeConnections(); }); + threadOpenMasternodeConnections = std::thread(&util::TraceThread, "mncon", [this, &dmnman, &mn_metaman, &mn_sync] { + ThreadOpenMasternodeConnections(dmnman, mn_metaman, mn_sync); + }); // Process messages threadMessageHandler = std::thread(&util::TraceThread, "msghand", [this] { ThreadMessageHandler(); }); if (connOptions.m_i2p_accept_incoming && m_i2p_sam_session.get() != nullptr) { threadI2PAcceptIncoming = - std::thread(&util::TraceThread, "i2paccept", [this] { ThreadI2PAcceptIncoming(); }); + std::thread(&util::TraceThread, "i2paccept", [this, &mn_sync] { ThreadI2PAcceptIncoming(mn_sync); }); } // Dump network addresses @@ -3754,14 +3757,13 @@ void CConnman::RemoveMasternodeQuorumNodes(Consensus::LLMQType llmqType, const u masternodeQuorumRelayMembers.erase(std::make_pair(llmqType, quorumHash)); } -bool CConnman::IsMasternodeQuorumNode(const CNode* pnode) +bool CConnman::IsMasternodeQuorumNode(const CNode* pnode, const CDeterministicMNList& tip_mn_list) { // Let's see if this is an outgoing connection to an address that is known to be a masternode // We however only need to know this if the node did not authenticate itself as a MN yet uint256 assumedProTxHash; if (pnode->GetVerifiedProRegTxHash().IsNull() && !pnode->IsInboundConn()) { - auto mnList = deterministicMNManager->GetListAtChainTip(); - auto dmn = mnList.GetMNByService(pnode->addr); + auto dmn = tip_mn_list.GetMNByService(pnode->addr); if (dmn == nullptr) { // This is definitely not a masternode return false; diff --git a/src/net.h b/src/net.h index dde7a0e7277a..da4e8ced77d4 100644 --- a/src/net.h +++ b/src/net.h @@ -48,6 +48,10 @@ #endif class CConnman; +class CDeterministicMNList; +class CDeterministicMNManager; +class CMasternodeMetaMan; +class CMasternodeSync; class CScheduler; class CNode; class BanMan; @@ -938,7 +942,8 @@ friend class CNode; CConnman(uint64_t seed0, uint64_t seed1, CAddrMan& addrman, bool network_active = true); ~CConnman(); - bool Start(CScheduler& scheduler, const Options& options); + bool Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, + CScheduler& scheduler, const Options& options); void StopThreads(); void StopNodes(); @@ -951,7 +956,7 @@ friend class CNode; void Interrupt(); bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; - void SetNetworkActive(bool active); + void SetNetworkActive(bool active, CMasternodeSync* const mn_sync); SocketEventsMode GetSocketEventsMode() const { return socketEventsMode; } enum class MasternodeConn { @@ -1180,7 +1185,7 @@ friend class CNode; // also returns QWATCH nodes std::set GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const; void RemoveMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash); - bool IsMasternodeQuorumNode(const CNode* pnode); + bool IsMasternodeQuorumNode(const CNode* pnode, const CDeterministicMNList& tip_mn_list); bool IsMasternodeQuorumRelayMember(const uint256& protxHash); void AddPendingProbeConnections(const std::set& proTxHashes); @@ -1258,10 +1263,10 @@ friend class CNode; void ThreadOpenAddedConnections(); void AddAddrFetch(const std::string& strDest); void ProcessAddrFetch(); - void ThreadOpenConnections(std::vector connect); + void ThreadOpenConnections(const std::vector connect, CDeterministicMNManager& dmnman); void ThreadMessageHandler(); - void ThreadI2PAcceptIncoming(); - void AcceptConnection(const ListenSocket& hListenSocket); + void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync); + void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync); /** * Create a `CNode` object from a socket that has just been accepted and add the node to @@ -1274,10 +1279,11 @@ friend class CNode; void CreateNodeFromAcceptedSocket(SOCKET hSocket, NetPermissionFlags permissionFlags, const CAddress& addr_bind, - const CAddress& addr); + const CAddress& addr, + CMasternodeSync& mn_sync); void DisconnectNodes(); - void NotifyNumConnectionsChanged(); + void NotifyNumConnectionsChanged(CMasternodeSync& mn_sync); void CalculateNumConnectionsChangedStats(); /** Return true if the peer is inactive and should be disconnected. */ bool InactivityCheck(const CNode& node) const; @@ -1293,10 +1299,11 @@ friend class CNode; #endif void SocketEventsSelect(std::set &recv_set, std::set &send_set, std::set &error_set, bool fOnlyPoll); void SocketEvents(std::set &recv_set, std::set &send_set, std::set &error_set, bool fOnlyPoll); - void SocketHandler(); - void ThreadSocketHandler(); + void SocketHandler(CMasternodeSync& mn_sync); + void ThreadSocketHandler(CMasternodeSync& mn_sync); void ThreadDNSAddressSeed(); - void ThreadOpenMasternodeConnections(); + void ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, + CMasternodeSync& mn_sync); uint64_t CalculateKeyedNetGroup(const CAddress& ad) const; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 23cd76cc7e93..384b22be57f4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3309,7 +3309,8 @@ void PeerManagerImpl::ProcessMessage( // Tell our peer that he should send us CoinJoin queue messages m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDDSQUEUE, true)); // Tell our peer that he should send us intra-quorum messages - if (llmq::IsWatchQuorumsEnabled() && m_connman.IsMasternodeQuorumNode(&pfrom)) { + const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); + if (llmq::IsWatchQuorumsEnabled() && m_connman.IsMasternodeQuorumNode(&pfrom, tip_mn_list)) { m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QWATCH)); } } @@ -3821,7 +3822,7 @@ void PeerManagerImpl::ProcessMessage( if (nInvType == MSG_DSTX) { // Validate DSTX and return bRet if we need to return from here uint256 hashTx = tx.GetHash(); - const auto& [bRet, bDoReturn] = ValidateDSTX(*Assert(m_dmnman), *(m_cj_ctx->dstxman), m_chainman, m_mempool, dstx, hashTx); + const auto& [bRet, bDoReturn] = ValidateDSTX(*m_dmnman, *(m_cj_ctx->dstxman), m_chainman, m_mempool, dstx, hashTx); if (bDoReturn) { return; } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 406932ef55f3..6937cbdf2cdf 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -482,7 +482,7 @@ class NodeImpl : public Node void setNetworkActive(bool active) override { if (m_context->connman) { - m_context->connman->SetNetworkActive(active); + m_context->connman->SetNetworkActive(active, m_context->mn_sync); } } bool getNetworkActive() override { return m_context->connman && m_context->connman->GetNetworkActive(); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e906e4759af4..daf1aa4eecc0 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -891,7 +891,7 @@ static RPCHelpMan setnetworkactive() throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); } - node.connman->SetNetworkActive(request.params[0].get_bool()); + node.connman->SetNetworkActive(request.params[0].get_bool(), node.mn_sync); return node.connman->GetNetworkActive(); }, diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 5923b06afe23..e9493f87b8d2 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -113,7 +113,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman) } }, [&] { - connman.SetNetworkActive(fuzzed_data_provider.ConsumeBool()); + connman.SetNetworkActive(fuzzed_data_provider.ConsumeBool(), /* mn_sync = */ nullptr); }, [&] { connman.SetTryNewOutboundPeer(fuzzed_data_provider.ConsumeBool()); From 1d9b7fad0fa88c52fa57f72e6b77f0efdf796ecd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 2 Apr 2024 12:24:02 +0000 Subject: [PATCH 5/7] refactor: trim globals use in net processing functions --- src/init.cpp | 5 +++-- src/net_processing.cpp | 33 +++++++++++++++++++----------- src/net_processing.h | 5 ++++- src/test/denialofservice_tests.cpp | 20 ++++++++++-------- src/test/util/setup_common.cpp | 5 +++-- 5 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d6b36ff28374..d51bf21e4b7c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1735,8 +1735,9 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc assert(!node.peerman); node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(), - *node.scheduler, chainman, *node.mempool, *node.govman, *node.sporkman, - ::deterministicMNManager, node.cj_ctx, node.llmq_ctx, ignores_incoming_txs); + *node.scheduler, chainman, *node.mempool, *node.mn_metaman, *node.mn_sync, + *node.govman, *node.sporkman, ::deterministicMNManager, + node.cj_ctx, node.llmq_ctx, ignores_incoming_txs); RegisterValidationInterface(node.peerman.get()); // sanitize comments per BIP-0014, format user agent and check total size diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 384b22be57f4..962e66579785 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -293,9 +293,10 @@ using PeerRef = std::shared_ptr; class PeerManagerImpl final : public PeerManager { public: - PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, - BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman, - CTxMemPool& pool, CGovernanceManager& govman, CSporkManager& sporkman, + PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, BanMan* banman, + CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool, + CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, + CGovernanceManager& govman, CSporkManager& sporkman, const std::unique_ptr& dmnman, const std::unique_ptr& cj_ctx, const std::unique_ptr& llmq_ctx, @@ -418,6 +419,8 @@ class PeerManagerImpl final : public PeerManager const std::unique_ptr& m_dmnman; const std::unique_ptr& m_cj_ctx; const std::unique_ptr& m_llmq_ctx; + CMasternodeMetaMan& m_mn_metaman; + CMasternodeSync& m_mn_sync; CGovernanceManager& m_govman; CSporkManager& m_sporkman; @@ -1730,18 +1733,22 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) std::unique_ptr PeerManager::make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool, + CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, CGovernanceManager& govman, CSporkManager& sporkman, const std::unique_ptr& dmnman, const std::unique_ptr& cj_ctx, const std::unique_ptr& llmq_ctx, bool ignore_incoming_txs) { - return std::make_unique(chainparams, connman, addrman, banman, scheduler, chainman, pool, govman, sporkman, dmnman, cj_ctx, llmq_ctx, ignore_incoming_txs); + return std::make_unique(chainparams, connman, addrman, banman, scheduler, chainman, pool, mn_metaman, mn_sync, govman, sporkman, dmnman, cj_ctx, llmq_ctx, ignore_incoming_txs); } PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool, - CGovernanceManager& govman, CSporkManager& sporkman, const std::unique_ptr& dmnman, - const std::unique_ptr& cj_ctx, const std::unique_ptr& llmq_ctx, + CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, + CGovernanceManager& govman, CSporkManager& sporkman, + const std::unique_ptr& dmnman, + const std::unique_ptr& cj_ctx, + const std::unique_ptr& llmq_ctx, bool ignore_incoming_txs) : m_chainparams(chainparams), m_connman(connman), @@ -1752,6 +1759,8 @@ PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& conn m_dmnman(dmnman), m_cj_ctx(cj_ctx), m_llmq_ctx(llmq_ctx), + m_mn_metaman(mn_metaman), + m_mn_sync(mn_sync), m_govman(govman), m_sporkman(sporkman), m_ignore_incoming_txs(ignore_incoming_txs) @@ -2950,9 +2959,9 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) } std::pair static ValidateDSTX(CDeterministicMNManager& dmnman, CDSTXManager& dstxman, ChainstateManager& chainman, - CTxMemPool& mempool, CCoinJoinBroadcastTx& dstx, uint256 hashTx) + CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool, CCoinJoinBroadcastTx& dstx, uint256 hashTx) { - assert(::mmetaman != nullptr); + assert(mn_metaman.IsValid()); if (!dstx.IsValidStructure()) { LogPrint(BCLog::COINJOIN, "DSTX -- Invalid DSTX structure: %s\n", hashTx.ToString()); @@ -2996,7 +3005,7 @@ std::pair static ValidateDSTX(CDeterministicMN return {false, true}; } - if (!mmetaman->GetMetaInfo(dmn->proTxHash)->IsValidForMixingTxes()) { + if (!mn_metaman.GetMetaInfo(dmn->proTxHash)->IsValidForMixingTxes()) { LogPrint(BCLog::COINJOIN, "DSTX -- Masternode %s is sending too many transactions %s\n", dstx.masternodeOutpoint.ToStringShort(), hashTx.ToString()); return {true, true}; // TODO: Not an error? Could it be that someone is relaying old DSTXes @@ -3010,7 +3019,7 @@ std::pair static ValidateDSTX(CDeterministicMN LogPrint(BCLog::COINJOIN, "DSTX -- Got Masternode transaction %s\n", hashTx.ToString()); mempool.PrioritiseTransaction(hashTx, 0.1*COIN); - mmetaman->DisallowMixing(dmn->proTxHash); + mn_metaman.DisallowMixing(dmn->proTxHash); return {true, false}; } @@ -3822,7 +3831,7 @@ void PeerManagerImpl::ProcessMessage( if (nInvType == MSG_DSTX) { // Validate DSTX and return bRet if we need to return from here uint256 hashTx = tx.GetHash(); - const auto& [bRet, bDoReturn] = ValidateDSTX(*m_dmnman, *(m_cj_ctx->dstxman), m_chainman, m_mempool, dstx, hashTx); + const auto& [bRet, bDoReturn] = ValidateDSTX(*m_dmnman, *(m_cj_ctx->dstxman), m_chainman, m_mn_metaman, m_mempool, dstx, hashTx); if (bDoReturn) { return; } @@ -4614,7 +4623,7 @@ void PeerManagerImpl::ProcessMessage( #endif // ENABLE_WALLET ProcessPeerMsgRet(m_cj_ctx->server->ProcessMessage(pfrom, msg_type, vRecv), pfrom); ProcessPeerMsgRet(m_sporkman.ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom); - ::masternodeSync->ProcessMessage(pfrom, msg_type, vRecv); + m_mn_sync.ProcessMessage(pfrom, msg_type, vRecv); ProcessPeerMsgRet(m_govman.ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom); ProcessPeerMsgRet(CMNAuth::ProcessMessage(pfrom, m_connman, m_dmnman->GetListAtChainTip(), msg_type, vRecv), pfrom); ProcessPeerMsgRet(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom); diff --git a/src/net_processing.h b/src/net_processing.h index 680dd2cae56b..23b7a9595f3f 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -15,6 +15,8 @@ class CAddrMan; class CTxMemPool; class CDeterministicMNManager; +class CMasternodeMetaMan; +class CMasternodeSync; class ChainstateManager; class CCoinJoinServer; class CGovernanceManager; @@ -48,7 +50,8 @@ class PeerManager : public CValidationInterface, public NetEventsInterface public: static std::unique_ptr make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman, - CTxMemPool& pool, CGovernanceManager& govman, CSporkManager& sporkman, + CTxMemPool& pool, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, + CGovernanceManager& govman, CSporkManager& sporkman, const std::unique_ptr& dmnman, const std::unique_ptr& cj_ctx, const std::unique_ptr& llmq_ctx, bool ignore_incoming_txs); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 0ae1ad5c316c..5c0910da3c9c 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -65,8 +65,9 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) const CChainParams& chainparams = Params(); auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler, - *m_node.chainman, *m_node.mempool, *m_node.govman, *m_node.sporkman, - ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false); + *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, + *m_node.govman, *m_node.sporkman, ::deterministicMNManager, + m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -136,8 +137,9 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) const CChainParams& chainparams = Params(); auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler, - *m_node.chainman, *m_node.mempool, *m_node.govman, *m_node.sporkman, - ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false); + *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, + *m_node.govman, *m_node.sporkman, ::deterministicMNManager, + m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false); constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS; CConnman::Options options; @@ -210,8 +212,9 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) auto banman = std::make_unique(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler, - *m_node.chainman, *m_node.mempool, *m_node.govman, *m_node.sporkman, - ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false); + *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, + *m_node.govman, *m_node.sporkman, ::deterministicMNManager, + m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false); banman->ClearBanned(); CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -257,8 +260,9 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) auto banman = std::make_unique(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler, - *m_node.chainman, *m_node.mempool, *m_node.govman, *m_node.sporkman, - ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false); + *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, + *m_node.govman, *m_node.sporkman, ::deterministicMNManager, + m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false); banman->ClearBanned(); int64_t nStartTime = GetTime(); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 9bbaad0bb7cf..884d4659fed9 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -283,8 +283,9 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, m_node.banman.get(), - *m_node.scheduler, *m_node.chainman, *m_node.mempool, *m_node.govman, - *m_node.sporkman, ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false); + *m_node.scheduler, *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, + *m_node.govman, *m_node.sporkman, ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, + /* ignore_incoming_txs = */ false); { CConnman::Options options; options.m_msgproc = m_node.peerman.get(); From 91f4588f71dd78c55d9a0e376d43d0d6e2f9a4fe Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 30 Mar 2024 00:18:33 +0000 Subject: [PATCH 6/7] refactor: const the pointer of type `const CActiveMasternodeManager` --- src/coinjoin/context.cpp | 2 +- src/coinjoin/context.h | 2 +- src/coinjoin/server.h | 4 ++-- src/llmq/context.cpp | 2 +- src/llmq/context.h | 2 +- src/llmq/dkgsession.h | 4 ++-- src/llmq/dkgsessionhandler.cpp | 2 +- src/llmq/dkgsessionhandler.h | 4 ++-- src/llmq/dkgsessionmgr.cpp | 2 +- src/llmq/dkgsessionmgr.h | 2 +- src/llmq/quorums.cpp | 3 +-- src/llmq/quorums.h | 4 ++-- src/llmq/signing.cpp | 2 +- src/llmq/signing.h | 4 ++-- src/llmq/signing_shares.h | 4 ++-- src/rpc/quorums.cpp | 2 +- 16 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/coinjoin/context.cpp b/src/coinjoin/context.cpp index 2328ac3ceafd..286a5928cadf 100644 --- a/src/coinjoin/context.cpp +++ b/src/coinjoin/context.cpp @@ -14,7 +14,7 @@ #include CJContext::CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool, - const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes) : + const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes) : dstxman{std::make_unique()}, #ifdef ENABLE_WALLET walletman{std::make_unique(connman, dmnman, mempool, mn_sync, queueman)}, diff --git a/src/coinjoin/context.h b/src/coinjoin/context.h index 856a41f5deea..c3257447c606 100644 --- a/src/coinjoin/context.h +++ b/src/coinjoin/context.h @@ -30,7 +30,7 @@ struct CJContext { CJContext() = delete; CJContext(const CJContext&) = delete; CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool, - const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes); + const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes); ~CJContext(); const std::unique_ptr dstxman; diff --git a/src/coinjoin/server.h b/src/coinjoin/server.h index 27d132961fe4..3f0df1454351 100644 --- a/src/coinjoin/server.h +++ b/src/coinjoin/server.h @@ -30,7 +30,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager CDeterministicMNManager& m_dmnman; CDSTXManager& m_dstxman; CTxMemPool& mempool; - const CActiveMasternodeManager* m_mn_activeman; + const CActiveMasternodeManager* const m_mn_activeman; const CMasternodeSync& m_mn_sync; // Mixing uses collateral transactions to trust parties entering the pool @@ -87,7 +87,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager public: explicit CCoinJoinServer(CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDSTXManager& dstxman, - CTxMemPool& mempool, const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync) : + CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync) : m_chainstate(chainstate), connman(_connman), m_dmnman(dmnman), diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 0b88365d562b..6963ce5cb675 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -20,7 +20,7 @@ #include LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CEvoDB& evo_db, - CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool, const CActiveMasternodeManager* mn_activeman, + CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const std::unique_ptr& peerman, bool unit_tests, bool wipe) : bls_worker{std::make_shared()}, dkg_debugman{std::make_unique()}, diff --git a/src/llmq/context.h b/src/llmq/context.h index d338a9e8b9c1..a5a3dd6de0f0 100644 --- a/src/llmq/context.h +++ b/src/llmq/context.h @@ -35,7 +35,7 @@ struct LLMQContext { LLMQContext() = delete; LLMQContext(const LLMQContext&) = delete; LLMQContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CEvoDB& evo_db, - CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool, const CActiveMasternodeManager* mn_activeman, + CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const std::unique_ptr& peerman, bool unit_tests, bool wipe); ~LLMQContext(); diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index 35e54b9ac766..16da61894bbd 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -275,7 +275,7 @@ class CDKGSession CDeterministicMNManager& m_dmnman; CDKGSessionManager& dkgManager; CDKGDebugManager& dkgDebugManager; - const CActiveMasternodeManager* m_mn_activeman; + const CActiveMasternodeManager* const m_mn_activeman; const CSporkManager& m_sporkman; const CBlockIndex* m_quorum_base_block_index{nullptr}; @@ -319,7 +319,7 @@ class CDKGSession public: CDKGSession(const Consensus::LLMQParams& _params, CBLSWorker& _blsWorker, CDeterministicMNManager& dmnman, CDKGSessionManager& _dkgManager, CDKGDebugManager& _dkgDebugManager, CConnman& _connman, - const CActiveMasternodeManager* mn_activeman, const CSporkManager& sporkman) : + const CActiveMasternodeManager* const mn_activeman, const CSporkManager& sporkman) : params(_params), blsWorker(_blsWorker), cache(_blsWorker), m_dmnman(dmnman), dkgManager(_dkgManager), dkgDebugManager(_dkgDebugManager), m_mn_activeman(mn_activeman), m_sporkman(sporkman), connman(_connman) {} diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 247410072742..429b2b2c475f 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -26,7 +26,7 @@ namespace llmq CDKGSessionHandler::CDKGSessionHandler(CBLSWorker& _blsWorker, CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDKGDebugManager& _dkgDebugManager, CDKGSessionManager& _dkgManager, CQuorumBlockProcessor& _quorumBlockProcessor, - const CActiveMasternodeManager* mn_activeman, const CSporkManager& sporkman, const Consensus::LLMQParams& _params, + const CActiveMasternodeManager* const mn_activeman, const CSporkManager& sporkman, const Consensus::LLMQParams& _params, int _quorumIndex) : blsWorker(_blsWorker), m_chainstate(chainstate), diff --git a/src/llmq/dkgsessionhandler.h b/src/llmq/dkgsessionhandler.h index 543bcbd3389a..c8121edcf15c 100644 --- a/src/llmq/dkgsessionhandler.h +++ b/src/llmq/dkgsessionhandler.h @@ -126,7 +126,7 @@ class CDKGSessionHandler CDKGDebugManager& dkgDebugManager; CDKGSessionManager& dkgManager; CQuorumBlockProcessor& quorumBlockProcessor; - const CActiveMasternodeManager* m_mn_activeman; + const CActiveMasternodeManager* const m_mn_activeman; const CSporkManager& m_sporkman; const Consensus::LLMQParams params; const int quorumIndex; @@ -148,7 +148,7 @@ class CDKGSessionHandler public: CDKGSessionHandler(CBLSWorker& _blsWorker, CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDKGDebugManager& _dkgDebugManager, CDKGSessionManager& _dkgManager, CQuorumBlockProcessor& _quorumBlockProcessor, - const CActiveMasternodeManager* mn_activeman, const CSporkManager& sporkman, const Consensus::LLMQParams& _params, + const CActiveMasternodeManager* const mn_activeman, const CSporkManager& sporkman, const Consensus::LLMQParams& _params, int _quorumIndex); ~CDKGSessionHandler() = default; diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index 935a4d7df770..9715368f6ce1 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -25,7 +25,7 @@ static const std::string DB_SKCONTRIB = "qdkg_S"; static const std::string DB_ENC_CONTRIB = "qdkg_E"; CDKGSessionManager::CDKGSessionManager(CBLSWorker& _blsWorker, CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, - CDKGDebugManager& _dkgDebugManager, CQuorumBlockProcessor& _quorumBlockProcessor, const CActiveMasternodeManager* mn_activeman, + CDKGDebugManager& _dkgDebugManager, CQuorumBlockProcessor& _quorumBlockProcessor, const CActiveMasternodeManager* const mn_activeman, const CSporkManager& sporkman, bool unitTests, bool fWipe) : db(std::make_unique(unitTests ? "" : (GetDataDir() / "llmq/dkgdb"), 1 << 20, unitTests, fWipe)), blsWorker(_blsWorker), diff --git a/src/llmq/dkgsessionmgr.h b/src/llmq/dkgsessionmgr.h index c493a65940d9..d237f77eaab1 100644 --- a/src/llmq/dkgsessionmgr.h +++ b/src/llmq/dkgsessionmgr.h @@ -67,7 +67,7 @@ class CDKGSessionManager public: CDKGSessionManager(CBLSWorker& _blsWorker, CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, - CDKGDebugManager& _dkgDebugManager, CQuorumBlockProcessor& _quorumBlockProcessor, const CActiveMasternodeManager* mn_activeman, + CDKGDebugManager& _dkgDebugManager, CQuorumBlockProcessor& _quorumBlockProcessor, const CActiveMasternodeManager* const mn_activeman, const CSporkManager& sporkman, bool unitTests, bool fWipe); ~CDKGSessionManager() = default; diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 1aa34122b9d1..eb4fc0c605a3 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -207,8 +207,7 @@ bool CQuorum::ReadContributions(CEvoDB& evoDb) CQuorumManager::CQuorumManager(CBLSWorker& _blsWorker, CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDKGSessionManager& _dkgManager, CEvoDB& _evoDb, CQuorumBlockProcessor& _quorumBlockProcessor, - const CActiveMasternodeManager* mn_activeman, const CSporkManager& sporkman, - const std::unique_ptr& mn_sync) : + const CActiveMasternodeManager* const mn_activeman, const CSporkManager& sporkman, const std::unique_ptr& mn_sync) : blsWorker(_blsWorker), m_chainstate(chainstate), connman(_connman), diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 9f3c139c9e9d..6d9515dffa70 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -224,7 +224,7 @@ class CQuorumManager CDKGSessionManager& dkgManager; CEvoDB& m_evoDb; CQuorumBlockProcessor& quorumBlockProcessor; - const CActiveMasternodeManager* m_mn_activeman; + const CActiveMasternodeManager* const m_mn_activeman; const CSporkManager& m_sporkman; const std::unique_ptr& m_mn_sync; @@ -241,7 +241,7 @@ class CQuorumManager public: CQuorumManager(CBLSWorker& _blsWorker, CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDKGSessionManager& _dkgManager, CEvoDB& _evoDb, CQuorumBlockProcessor& _quorumBlockProcessor, - const CActiveMasternodeManager* mn_activeman, const CSporkManager& sporkman, const std::unique_ptr& mn_sync); + const CActiveMasternodeManager* const mn_activeman, const CSporkManager& sporkman, const std::unique_ptr& mn_sync); ~CQuorumManager() { Stop(); }; void Start(); diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index afd2c52cf3e2..98bf59d96d3a 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -539,7 +539,7 @@ void CRecoveredSigsDb::CleanupOldVotes(int64_t maxAge) ////////////////// -CSigningManager::CSigningManager(CConnman& _connman, const CActiveMasternodeManager* mn_activeman, const CQuorumManager& _qman, +CSigningManager::CSigningManager(CConnman& _connman, const CActiveMasternodeManager* const mn_activeman, const CQuorumManager& _qman, bool fMemory, bool fWipe) : db(fMemory, fWipe), connman(_connman), m_mn_activeman(mn_activeman), qman(_qman) { diff --git a/src/llmq/signing.h b/src/llmq/signing.h index 4a00bd248c4d..92102dbe855b 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -163,7 +163,7 @@ class CSigningManager CRecoveredSigsDb db; CConnman& connman; - const CActiveMasternodeManager* m_mn_activeman; + const CActiveMasternodeManager* const m_mn_activeman; const CQuorumManager& qman; std::atomic m_peerman{nullptr}; @@ -179,7 +179,7 @@ class CSigningManager std::vector recoveredSigsListeners GUARDED_BY(cs); public: - CSigningManager(CConnman& _connman, const CActiveMasternodeManager* mn_activeman, const CQuorumManager& _qman, bool fMemory, bool fWipe); + CSigningManager(CConnman& _connman, const CActiveMasternodeManager* const mn_activeman, const CQuorumManager& _qman, bool fMemory, bool fWipe); bool AlreadyHave(const CInv& inv) const; bool GetRecoveredSigForGetData(const uint256& hash, CRecoveredSig& ret) const; diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index d678e99d8015..3725d295be80 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -401,7 +401,7 @@ class CSigSharesManager : public CRecoveredSigsListener CConnman& connman; CSigningManager& sigman; - const CActiveMasternodeManager* m_mn_activeman; + const CActiveMasternodeManager* const m_mn_activeman; const CQuorumManager& qman; const CSporkManager& m_sporkman; @@ -411,7 +411,7 @@ class CSigSharesManager : public CRecoveredSigsListener std::atomic recoveredSigsCounter{0}; public: - explicit CSigSharesManager(CConnman& _connman, CSigningManager& _sigman, const CActiveMasternodeManager* mn_activeman, + explicit CSigSharesManager(CConnman& _connman, CSigningManager& _sigman, const CActiveMasternodeManager* const mn_activeman, const CQuorumManager& _qman, const CSporkManager& sporkman, const std::unique_ptr& peerman) : connman(_connman), sigman(_sigman), m_mn_activeman(mn_activeman), qman(_qman), m_sporkman(sporkman), m_peerman(peerman) { diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 96926556751c..78551db8e69b 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -275,7 +275,7 @@ static void quorum_dkgstatus_help(const JSONRPCRequest& request) }.Check(request); } -static UniValue quorum_dkgstatus(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const CActiveMasternodeManager* mn_activeman, +static UniValue quorum_dkgstatus(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const CActiveMasternodeManager* const mn_activeman, const ChainstateManager& chainman, const CSporkManager& sporkman, const LLMQContext& llmq_ctx) { quorum_dkgstatus_help(request); From 191b3de9b48212dc5880fdca69ea949977becd57 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 4 Apr 2024 10:51:39 +0000 Subject: [PATCH 7/7] refactor: move CheckSpecialTx into CSpecialTxProcessor --- src/evo/specialtxman.cpp | 4 +-- src/evo/specialtxman.h | 5 ++- src/rpc/evo.cpp | 68 +++++++++++++++++++++------------------- src/validation.cpp | 10 ++++-- src/validation.h | 6 ++++ 5 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 0bcd52dda176..8e71a0e68aaf 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -72,10 +72,10 @@ static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, const CTransact return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-tx-type-check"); } -bool CheckSpecialTx(CDeterministicMNManager& dmnman, const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state) +bool CSpecialTxProcessor::CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state) { AssertLockHeld(cs_main); - return CheckSpecialTxInner(dmnman, tx, pindexPrev, view, std::nullopt, check_sigs, state); + return CheckSpecialTxInner(m_dmnman, tx, pindexPrev, view, std::nullopt, check_sigs, state); } [[nodiscard]] bool CSpecialTxProcessor::ProcessSpecialTx(const CTransaction& tx, const CBlockIndex* pindex, TxValidationState& state) diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 31d712e928e5..f8464fc64e57 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -29,9 +29,6 @@ class CChainLocksHandler; extern RecursiveMutex cs_main; -bool CheckSpecialTx(CDeterministicMNManager& dmnman, const CTransaction& tx, const CBlockIndex* pindexPrev, - const CCoinsViewCache& view, bool check_sigs, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - class CSpecialTxProcessor { private: @@ -51,6 +48,8 @@ class CSpecialTxProcessor const Consensus::Params& consensus_params, const llmq::CChainLocksHandler& clhandler) : m_cpoolman(cpoolman), m_dmnman{dmnman}, m_mnhfman{mnhfman}, m_qblockman{qblockman}, m_consensus_params{consensus_params}, m_clhandler{clhandler} {} + bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state) + EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerkleRoots, BlockValidationState& state, std::optional& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 45668884c271..541438e0fa13 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -324,13 +325,13 @@ static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxP payload.sig = key.Sign(hash); } -static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit = true) +static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit = true) { { LOCK(cs_main); TxValidationState state; - if (!CheckSpecialTx(dmnman, CTransaction(tx), chainman.ActiveChain().Tip(), chainman.ActiveChainstate().CoinsTip(), true, state)) { + if (!chain_helper.special_tx->CheckSpecialTx(CTransaction(tx), chainman.ActiveChain().Tip(), chainman.ActiveChainstate().CoinsTip(), true, state)) { throw std::runtime_error(state.ToString()); } } // cs_main @@ -579,7 +580,7 @@ static void protx_register_prepare_evo_help(const JSONRPCRequest& request) } static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, - CDeterministicMNManager& dmnman, + CChainstateHelper& chain_helper, const ChainstateManager& chainman, const bool specific_legacy_bls_scheme, const bool isExternalRegister, @@ -739,7 +740,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, ptx.collateralOutpoint.n = collateralIndex; SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, dmnman, chainman, tx, fSubmit); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit); } else { // referencing external collateral @@ -787,7 +788,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, } SignSpecialTxPayloadByString(tx, ptx, key); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, dmnman, chainman, tx, fSubmit); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit); } } catch (...) { if (unlockOnError) { @@ -798,7 +799,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, } } -static UniValue protx_register_evo(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) +static UniValue protx_register_evo(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman) { bool isExternalRegister = request.strMethod == "protxregister_evo"; bool isFundRegister = request.strMethod == "protxregister_fund_evo"; @@ -811,26 +812,26 @@ static UniValue protx_register_evo(const JSONRPCRequest& request, CDeterministic isFundRegister = request.strMethod == "protxregister_fund_hpmn"; isPrepareRegister = request.strMethod == "protxregister_prepare_hpmn"; } - return protx_register_common_wrapper(request, dmnman, chainman, false, isExternalRegister, isFundRegister, isPrepareRegister, MnType::Evo); + return protx_register_common_wrapper(request, chain_helper, chainman, false, isExternalRegister, isFundRegister, isPrepareRegister, MnType::Evo); } -static UniValue protx_register(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) +static UniValue protx_register(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman) { bool isExternalRegister = request.strMethod == "protxregister"; bool isFundRegister = request.strMethod == "protxregister_fund"; bool isPrepareRegister = request.strMethod == "protxregister_prepare"; - return protx_register_common_wrapper(request, dmnman, chainman, false, isExternalRegister, isFundRegister, isPrepareRegister, MnType::Regular); + return protx_register_common_wrapper(request, chain_helper, chainman, false, isExternalRegister, isFundRegister, isPrepareRegister, MnType::Regular); } -static UniValue protx_register_legacy(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) +static UniValue protx_register_legacy(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman) { bool isExternalRegister = request.strMethod == "protxregister_legacy"; bool isFundRegister = request.strMethod == "protxregister_fund_legacy"; bool isPrepareRegister = request.strMethod == "protxregister_prepare_legacy"; - return protx_register_common_wrapper(request, dmnman, chainman, true, isExternalRegister, isFundRegister, isPrepareRegister, MnType::Regular); + return protx_register_common_wrapper(request, chain_helper, chainman, true, isExternalRegister, isFundRegister, isPrepareRegister, MnType::Regular); } -static UniValue protx_register_submit(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) +static UniValue protx_register_submit(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman) { protx_register_submit_help(request); @@ -863,7 +864,7 @@ static UniValue protx_register_submit(const JSONRPCRequest& request, CDeterminis } SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, dmnman, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx); } static void protx_update_service_help(const JSONRPCRequest& request) @@ -914,7 +915,7 @@ static void protx_update_service_evo_help(const JSONRPCRequest& request) }.Check(request); } -static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const MnType mnType) +static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& request, CChainstateHelper& chain_helper, CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const MnType mnType) { if (request.strMethod.find("_hpmn") != std::string::npos) { if (!IsDeprecatedRPCEnabled("hpmn")) { @@ -1026,7 +1027,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques SignSpecialTxPayloadByHash(tx, ptx, keyOperator); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, dmnman, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx); } static void protx_update_registrar_help(const JSONRPCRequest& request, bool legacy) @@ -1056,7 +1057,7 @@ static void protx_update_registrar_help(const JSONRPCRequest& request, bool lega }.Check(request); } -static UniValue protx_update_registrar_wrapper(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const bool specific_legacy_bls_scheme) +static UniValue protx_update_registrar_wrapper(const JSONRPCRequest& request, CChainstateHelper& chain_helper, CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const bool specific_legacy_bls_scheme) { protx_update_registrar_help(request, specific_legacy_bls_scheme); @@ -1131,17 +1132,17 @@ static UniValue protx_update_registrar_wrapper(const JSONRPCRequest& request, CD SignSpecialTxPayloadByHash(tx, ptx, keyOwner); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, dmnman, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx); } -static UniValue protx_update_registrar(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) +static UniValue protx_update_registrar(const JSONRPCRequest& request, CChainstateHelper& chain_helper, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) { - return protx_update_registrar_wrapper(request, dmnman, chainman, false); + return protx_update_registrar_wrapper(request, chain_helper, dmnman, chainman, false); } -static UniValue protx_update_registrar_legacy(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) +static UniValue protx_update_registrar_legacy(const JSONRPCRequest& request, CChainstateHelper& chain_helper, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) { - return protx_update_registrar_wrapper(request, dmnman, chainman, true); + return protx_update_registrar_wrapper(request, chain_helper, dmnman, chainman, true); } static void protx_revoke_help(const JSONRPCRequest& request) @@ -1167,7 +1168,7 @@ static void protx_revoke_help(const JSONRPCRequest& request) }.Check(request); } -static UniValue protx_revoke(const JSONRPCRequest& request, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) +static UniValue protx_revoke(const JSONRPCRequest& request, CChainstateHelper& chain_helper, CDeterministicMNManager& dmnman, const ChainstateManager& chainman) { protx_revoke_help(request); @@ -1227,7 +1228,7 @@ static UniValue protx_revoke(const JSONRPCRequest& request, CDeterministicMNMana SignSpecialTxPayloadByHash(tx, ptx, keyOperator); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, dmnman, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx); } #endif//ENABLE_WALLET @@ -1676,24 +1677,27 @@ static UniValue protx(const JSONRPCRequest& request) CMasternodeMetaMan& mn_metaman = *node.mn_metaman; #ifdef ENABLE_WALLET + CHECK_NONFATAL(node.chain_helper); + CChainstateHelper& chain_helper = *node.chain_helper; + if (command == "protxregister" || command == "protxregister_fund" || command == "protxregister_prepare") { - return protx_register(new_request, dmnman, chainman); + return protx_register(new_request, chain_helper, chainman); } else if (command == "protxregister_evo" || command == "protxregister_fund_evo" || command == "protxregister_prepare_evo" || command == "protxregister_hpmn" || command == "protxregister_fund_hpmn" || command == "protxregister_prepare_hpmn") { - return protx_register_evo(new_request, dmnman, chainman); + return protx_register_evo(new_request, chain_helper, chainman); } else if (command == "protxregister_legacy" || command == "protxregister_fund_legacy" || command == "protxregister_prepare_legacy") { - return protx_register_legacy(new_request, dmnman, chainman); + return protx_register_legacy(new_request, chain_helper, chainman); } else if (command == "protxregister_submit") { - return protx_register_submit(new_request, dmnman, chainman); + return protx_register_submit(new_request, chain_helper, chainman); } else if (command == "protxupdate_service") { - return protx_update_service_common_wrapper(new_request, dmnman, chainman, MnType::Regular); + return protx_update_service_common_wrapper(new_request, chain_helper, dmnman, chainman, MnType::Regular); } else if (command == "protxupdate_service_evo" || command == "protxupdate_service_hpmn") { - return protx_update_service_common_wrapper(new_request, dmnman, chainman, MnType::Evo); + return protx_update_service_common_wrapper(new_request, chain_helper, dmnman, chainman, MnType::Evo); } else if (command == "protxupdate_registrar") { - return protx_update_registrar(new_request, dmnman, chainman); + return protx_update_registrar(new_request, chain_helper, dmnman, chainman); } else if (command == "protxupdate_registrar_legacy") { - return protx_update_registrar_legacy(new_request, dmnman, chainman); + return protx_update_registrar_legacy(new_request, chain_helper, dmnman, chainman); } else if (command == "protxrevoke") { - return protx_revoke(new_request, dmnman, chainman); + return protx_revoke(new_request, chain_helper, dmnman, chainman); } else #endif if (command == "protxlist") { diff --git a/src/validation.cpp b/src/validation.cpp index df51a87ce8fd..3dc6580de347 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -529,7 +529,12 @@ namespace { class MemPoolAccept { public: - explicit MemPoolAccept(CTxMemPool& mempool, CChainState& active_chainstate) : m_pool(mempool), m_view(&m_dummy), m_viewmempool(&active_chainstate.CoinsTip(), m_pool), m_active_chainstate(active_chainstate), + explicit MemPoolAccept(CTxMemPool& mempool, CChainState& active_chainstate) : + m_pool(mempool), + m_view(&m_dummy), + m_viewmempool(&active_chainstate.CoinsTip(), m_pool), + m_active_chainstate(active_chainstate), + m_chain_helper(active_chainstate.ChainHelper()), m_limit_ancestors(gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT)), m_limit_ancestor_size(gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000), m_limit_descendants(gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT)), @@ -621,6 +626,7 @@ class MemPoolAccept CCoinsViewMemPool m_viewmempool; CCoinsView m_dummy; CChainState& m_active_chainstate; + CChainstateHelper& m_chain_helper; // The package limits in effect at the time of invocation. const size_t m_limit_ancestors; @@ -830,7 +836,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // DoS scoring a node for non-critical errors, e.g. duplicate keys because a TX is received that was already // mined // NOTE: we use UTXO here and do NOT allow mempool txes as masternode collaterals - if (!CheckSpecialTx(*::deterministicMNManager, tx, m_active_chainstate.m_chain.Tip(), m_active_chainstate.CoinsTip(), true, state)) + if (!m_chain_helper.special_tx->CheckSpecialTx(tx, m_active_chainstate.m_chain.Tip(), m_active_chainstate.CoinsTip(), true, state)) return false; if (m_pool.existsProviderTxConflict(tx)) { diff --git a/src/validation.h b/src/validation.h index 1b6b1bb28c36..5e5f03109125 100644 --- a/src/validation.h +++ b/src/validation.h @@ -718,6 +718,12 @@ class CChainState */ std::set setBlockIndexCandidates; + CChainstateHelper& ChainHelper() + { + assert(m_chain_helper); + return *m_chain_helper; + } + //! @returns A reference to the in-memory cache of the UTXO set. CCoinsViewCache& CoinsTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main) {