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
This commit is contained in:
Konstantin Akimov 2023-02-01 00:08:25 +07:00 committed by GitHub
parent aa3ecbf6af
commit 967d413db3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 4 additions and 9 deletions

View File

@ -142,8 +142,7 @@ void CGovernanceManager::ProcessMessage(CNode& peer, std::string_view msg_type,
return;
}
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
LOCK(cs);
LOCK2(cs_main, cs);
if (mapObjects.count(nHash) || mapPostponedObjects.count(nHash) || mapErasedGovernanceObjects.count(nHash)) {
// TODO - print error code? what if it's GOVOBJ_ERROR_IMMATURE?
@ -262,8 +261,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman
govobj.UpdateSentinelVariables(); //this sets local vars in object
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
LOCK(cs);
LOCK2(cs_main, cs);
std::string strError;
// MAKE SURE THIS OBJECT IS OK
@ -320,8 +318,7 @@ void CGovernanceManager::UpdateCachesAndClean()
std::vector<uint256> vecDirtyHashes = mmetaman.GetAndClearDirtyGovernanceObjectHashes();
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
LOCK(cs);
LOCK2(cs_main, cs);
for (const uint256& nHash : vecDirtyHashes) {
auto it = mapObjects.find(nHash);
@ -835,8 +832,7 @@ void CGovernanceManager::CheckPostponedObjects(CConnman& connman)
{
if (!::masternodeSync->IsSynced()) return;
LOCK2(cs_main, ::mempool.cs); // Lock mempool because of GetTransaction deep inside
LOCK(cs);
LOCK2(cs_main, cs);
// Check postponed proposals
for (auto it = mapPostponedObjects.begin(); it != mapPostponedObjects.end();) {

View File

@ -531,7 +531,6 @@ CAmount CGovernanceObject::GetMinCollateralFee(bool fork_active) const
bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingConfirmations) const
{
AssertLockHeld(cs_main);
AssertLockHeld(::mempool.cs); // because of GetTransaction
strError = "";
fMissingConfirmations = false;