From 6079b860e39dcef6f840b0f5ca9a0d4ab1174e34 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 6 Jun 2018 18:56:33 +0300 Subject: [PATCH] Drop trigger objects when triggers are deleted or failed to be created (#2098) * Mark an object for deletion when the corresponding trigger is deleted * Mark objects for deletion if creation of corresponding triggers failed * NULL -> nullptr * Make sure pObj is not null --- src/governance-classes.cpp | 40 ++++++++++++++++++------------------ src/governance.cpp | 42 +++++++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/src/governance-classes.cpp b/src/governance-classes.cpp index 9249d5a997..5f6553ec18 100644 --- a/src/governance-classes.cpp +++ b/src/governance-classes.cpp @@ -156,35 +156,27 @@ void CGovernanceTriggerManager::CleanAndRemove() DBG( std::cout << "CGovernanceTriggerManager::CleanAndRemove: Start" << std::endl; ); AssertLockHeld(governance.cs); - // LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS - for(trigger_m_it it = mapTrigger.begin(); it != mapTrigger.end(); ++it) { - //int nNewStatus = -1; - CGovernanceObject* pObj = governance.FindGovernanceObject((*it).first); - if(!pObj) { - continue; - } - CSuperblock_sptr& pSuperblock = it->second; - if(!pSuperblock) { - continue; - } - // IF THIS ISN'T A TRIGGER, WHY ARE WE HERE? - if(pObj->GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) { - pSuperblock->SetStatus(SEEN_OBJECT_ERROR_INVALID); - } - } - // Remove triggers that are invalid or expired DBG( std::cout << "CGovernanceTriggerManager::CleanAndRemove: mapTrigger.size() = " << mapTrigger.size() << std::endl; ); LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- mapTrigger.size() = %d\n", mapTrigger.size()); + trigger_m_it it = mapTrigger.begin(); while(it != mapTrigger.end()) { bool remove = false; + CGovernanceObject* pObj = nullptr; CSuperblock_sptr& pSuperblock = it->second; if(!pSuperblock) { DBG( std::cout << "CGovernanceTriggerManager::CleanAndRemove: NULL superblock marked for removal" << std::endl; ); LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- NULL superblock marked for removal\n"); remove = true; } else { + pObj = governance.FindGovernanceObject(it->first); + if(!pObj || pObj->GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) { + DBG( std::cout << "CGovernanceTriggerManager::CleanAndRemove: Unknown or non-trigger superblock" << std::endl; ); + LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- Unknown or non-trigger superblock\n"); + pSuperblock->SetStatus(SEEN_OBJECT_ERROR_INVALID); + } + DBG( std::cout << "CGovernanceTriggerManager::CleanAndRemove: superblock status = " << pSuperblock->GetStatus() << std::endl; ); LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- superblock status = %d\n", pSuperblock->GetStatus()); switch(pSuperblock->GetStatus()) { @@ -201,19 +193,27 @@ void CGovernanceTriggerManager::CleanAndRemove() break; } } + LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- %smarked for removal\n", remove ? "" : "NOT "); if(remove) { DBG( std::string strDataAsPlainString = "NULL"; - CGovernanceObject* pgovobj = pSuperblock->GetGovernanceObject(); - if(pgovobj) { - strDataAsPlainString = pgovobj->GetDataAsPlainString(); + if(pObj) { + strDataAsPlainString = pObj->GetDataAsPlainString(); } std::cout << "CGovernanceTriggerManager::CleanAndRemove: Removing object: " << strDataAsPlainString << std::endl; ); LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- Removing trigger object\n"); + // mark corresponding object for deletion + if (pObj) { + pObj->fCachedDelete = true; + if (pObj->nDeletionTime == 0) { + pObj->nDeletionTime = GetAdjustedTime(); + } + } + // delete the trigger mapTrigger.erase(it++); } else { diff --git a/src/governance.cpp b/src/governance.cpp index 31fc138309..df62ca2768 100644 --- a/src/governance.cpp +++ b/src/governance.cpp @@ -316,17 +316,16 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman return; } - // IF WE HAVE THIS OBJECT ALREADY, WE DON'T WANT ANOTHER COPY - - if(mapObjects.count(nHash)) { - LogPrintf("CGovernanceManager::AddGovernanceObject -- already have governance object %s\n", nHash.ToString()); - return; - } - LogPrint("gobject", "CGovernanceManager::AddGovernanceObject -- Adding object: hash = %s, type = %d\n", nHash.ToString(), govobj.GetObjectType()); // INSERT INTO OUR GOVERNANCE OBJECT MEMORY - mapObjects.insert(std::make_pair(nHash, govobj)); + // IF WE HAVE THIS OBJECT ALREADY, WE DON'T WANT ANOTHER COPY + auto objpair = mapObjects.emplace(nHash, govobj); + + if(!objpair.second) { + LogPrintf("CGovernanceManager::AddGovernanceObject -- already have governance object %s\n", nHash.ToString()); + return; + } // SHOULD WE ADD THIS OBJECT TO ANY OTHER MANANGERS? @@ -337,7 +336,15 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman if (govobj.nObjectType == GOVERNANCE_OBJECT_TRIGGER) { DBG( std::cout << "CGovernanceManager::AddGovernanceObject Before AddNewTrigger" << std::endl; ); - triggerman.AddNewTrigger(nHash); + if (!triggerman.AddNewTrigger(nHash)) { + LogPrint("gobject", "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString()); + CGovernanceObject& objref = objpair.first->second; + objref.fCachedDelete = true; + if (objref.nDeletionTime == 0) { + objref.nDeletionTime = GetAdjustedTime(); + } + return; + } DBG( std::cout << "CGovernanceManager::AddGovernanceObject After AddNewTrigger" << std::endl; ); } @@ -376,13 +383,10 @@ void CGovernanceManager::UpdateCachesAndClean() ScopedLockBool guard(cs, fRateChecksEnabled, false); - // UPDATE CACHE FOR EACH OBJECT THAT IS FLAGGED DIRTYCACHE=TRUE - - object_m_it it = mapObjects.begin(); - // Clean up any expired or invalid triggers triggerman.CleanAndRemove(); + object_m_it it = mapObjects.begin(); int64_t nNow = GetAdjustedTime(); while(it != mapObjects.end()) @@ -445,6 +449,7 @@ void CGovernanceManager::UpdateCachesAndClean() mapErasedGovernanceObjects.insert(std::make_pair(nHash, nTimeExpired)); mapObjects.erase(it++); } else { + // NOTE: triggers are handled via triggerman // DO NOT USE THIS UNTIL MAY, 2018 on mainnet if ((GetAdjustedTime() >= 1526423380 || Params().NetworkIDString() != CBaseChainParams::MAIN) && pObj->GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { CProposalValidator validator(pObj->GetDataAsHexString()); @@ -1197,14 +1202,19 @@ void CGovernanceManager::AddCachedTriggers() { LOCK(cs); - for(object_m_it it = mapObjects.begin(); it != mapObjects.end(); ++it) { - CGovernanceObject& govobj = it->second; + for (auto& objpair : mapObjects) { + CGovernanceObject& govobj = objpair.second; if(govobj.nObjectType != GOVERNANCE_OBJECT_TRIGGER) { continue; } - triggerman.AddNewTrigger(govobj.GetHash()); + if (!triggerman.AddNewTrigger(govobj.GetHash())) { + govobj.fCachedDelete = true; + if (govobj.nDeletionTime == 0) { + govobj.nDeletionTime = GetAdjustedTime(); + } + } } }