Skip to content

Commit 967d413

Browse files
authored
fix: governance should not lock mempool mutex even call GetTransaction (#5175)
## Issue being fixed or feature implemented Previously noticed, that GetTransaction is called deep inside governance objects and it is true indeed. But so far it is used `nullptr` as a mempol object instead any real mempool in GetTransaction and no usage of a global mempool or any other mempool, this locks are useless. This changes are important for mempool globalization. ## What was done? Removed LOCK for ::mempool.cs in governance's code ## How Has This Been Tested? Run unit/functional tests. Also done deglobalization of mempool to validate that governance indeed doesn't use global mempool implicit in #5169 ## Breaking Changes No breaking changes ## Checklist: - [x] I have performed a self-review of my own code - [x] I have assigned this pull request to a milestone
1 parent aa3ecbf commit 967d413

File tree

2 files changed

+4
-9
lines changed

2 files changed

+4
-9
lines changed

src/governance/governance.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,7 @@ void CGovernanceManager::ProcessMessage(CNode& peer, std::string_view msg_type,
142142
return;
143143
}
144144

145-
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
146-
LOCK(cs);
145+
LOCK2(cs_main, cs);
147146

148147
if (mapObjects.count(nHash) || mapPostponedObjects.count(nHash) || mapErasedGovernanceObjects.count(nHash)) {
149148
// TODO - print error code? what if it's GOVOBJ_ERROR_IMMATURE?
@@ -262,8 +261,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman
262261

263262
govobj.UpdateSentinelVariables(); //this sets local vars in object
264263

265-
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
266-
LOCK(cs);
264+
LOCK2(cs_main, cs);
267265
std::string strError;
268266

269267
// MAKE SURE THIS OBJECT IS OK
@@ -320,8 +318,7 @@ void CGovernanceManager::UpdateCachesAndClean()
320318

321319
std::vector<uint256> vecDirtyHashes = mmetaman.GetAndClearDirtyGovernanceObjectHashes();
322320

323-
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
324-
LOCK(cs);
321+
LOCK2(cs_main, cs);
325322

326323
for (const uint256& nHash : vecDirtyHashes) {
327324
auto it = mapObjects.find(nHash);
@@ -835,8 +832,7 @@ void CGovernanceManager::CheckPostponedObjects(CConnman& connman)
835832
{
836833
if (!::masternodeSync->IsSynced()) return;
837834

838-
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
839-
LOCK(cs);
835+
LOCK2(cs_main, cs);
840836

841837
// Check postponed proposals
842838
for (auto it = mapPostponedObjects.begin(); it != mapPostponedObjects.end();) {

src/governance/object.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,6 @@ CAmount CGovernanceObject::GetMinCollateralFee(bool fork_active) const
531531
bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingConfirmations) const
532532
{
533533
AssertLockHeld(cs_main);
534-
AssertLockHeld(::mempool.cs); // because of GetTransaction
535534

536535
strError = "";
537536
fMissingConfirmations = false;

0 commit comments

Comments
 (0)