Fix trigger execution and expiration (#1920)

* Move trigger expiration logic into CSuperblock and handle executed/non-executed triggers differently

* Actually execute triggers

src/governance-classes.cpp

@@ -173,7 +173,7 @@ void CGovernanceTriggerManager::CleanAndRemove()
         }
     }
 
-    // Remove triggers that are invalid or already executed
+    // Remove triggers that are invalid or expired
     DBG( cout << "CGovernanceTriggerManager::CleanAndRemove: mapTrigger.size() = " << mapTrigger.size() << endl; );
     LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- mapTrigger.size() = %d\n", mapTrigger.size());
     trigger_m_it it = mapTrigger.begin();
@@ -195,22 +195,7 @@ void CGovernanceTriggerManager::CleanAndRemove()
                 break;
             case SEEN_OBJECT_IS_VALID:
             case SEEN_OBJECT_EXECUTED:
-                {
+                remove = pSuperblock->IsExpired();
-                    int nTriggerBlock = pSuperblock->GetBlockHeight();
-                    // Rough approximation: a cycle of superblock ++
-                    int nExpirationBlock = nTriggerBlock + GOVERNANCE_TRIGGER_EXPIRATION_BLOCKS;
-                    LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- nTriggerBlock = %d, nExpirationBlock = %d\n", nTriggerBlock, nExpirationBlock);
-                    if(governance.GetCachedBlockHeight() > nExpirationBlock) {
-                        LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- Outdated trigger found\n");
-                        remove = true;
-                        CGovernanceObject* pgovobj = pSuperblock->GetGovernanceObject();
-                        if(pgovobj) {
-                            LogPrint("gobject", "CGovernanceTriggerManager::CleanAndRemove -- Expiring outdated object: %s\n", pgovobj->GetHash().ToString());
-                            pgovobj->fExpired = true;
-                            pgovobj->nDeletionTime = GetAdjustedTime();
-                        }
-                    }
-                }
                 break;
             default:
                 break;
@@ -460,6 +445,18 @@ bool CSuperblockManager::IsValid(const CTransaction& txNew, int nBlockHeight, CA
     return false;
 }
 
+void CSuperblockManager::ExecuteBestSuperblock(int nBlockHeight)
+{
+    LOCK(governance.cs);
+
+    CSuperblock_sptr pSuperblock;
+    if(GetBestSuperblock(pSuperblock, nBlockHeight)) {
+        // All checks are done in CSuperblock::IsValid via IsBlockValueValid and IsBlockPayeeValid,
+        // tip wouldn't be updated if anything was wrong. Mark this trigger as executed.
+        pSuperblock->SetExecuted();
+    }
+}
+
 CSuperblock::
 CSuperblock()
     : nGovObjHash(),
@@ -742,6 +739,42 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int nBlockHeight, CAmount b
     return true;
 }
 
+bool CSuperblock::IsExpired()
+{
+    bool fExpired{false};
+    int nExpirationBlocks{0};
+    // Executed triggers are kept for another superblock cycle (approximately 1 month),
+    // other valid triggers are kept for ~1 day only, everything else is pruned after ~1h.
+    switch (nStatus) {
+        case SEEN_OBJECT_EXECUTED:
+            nExpirationBlocks = Params().GetConsensus().nSuperblockCycle;
+            break;
+        case SEEN_OBJECT_IS_VALID:
+            nExpirationBlocks = 576;
+            break;
+        default:
+            nExpirationBlocks = 24;
+            break;
+    }
+
+    int nExpirationBlock = nBlockHeight + nExpirationBlocks;
+
+    LogPrint("gobject", "CSuperblock::IsExpired -- nBlockHeight = %d, nExpirationBlock = %d\n", nBlockHeight, nExpirationBlock);
+
+    if(governance.GetCachedBlockHeight() > nExpirationBlock) {
+        LogPrint("gobject", "CSuperblock::IsExpired -- Outdated trigger found\n");
+        fExpired = true;
+        CGovernanceObject* pgovobj = GetGovernanceObject();
+        if(pgovobj) {
+            LogPrint("gobject", "CSuperblock::IsExpired -- Expiring outdated object: %s\n", pgovobj->GetHash().ToString());
+            pgovobj->fExpired = true;
+            pgovobj->nDeletionTime = GetAdjustedTime();
+        }
+    }
+
+    return fExpired;
+}
+
 /**
 *   Get Required Payment String
 *

src/governance-classes.h

@@ -70,6 +70,7 @@ public:
     static bool IsSuperblockTriggered(int nBlockHeight);
 
     static void CreateSuperblock(CMutableTransaction& txNewRet, int nBlockHeight, std::vector<CTxOut>& voutSuperblockRet);
+    static void ExecuteBestSuperblock(int nBlockHeight);
 
     static std::string GetRequiredPaymentsString(int nBlockHeight);
     static bool IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
@@ -185,6 +186,7 @@ public:
     CAmount GetPaymentsTotalAmount();
 
     bool IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
+    bool IsExpired();
 };
 
 #endif

src/governance-object.h

@@ -43,8 +43,6 @@ static const int64_t GOVERNANCE_DELETION_DELAY = 10*60;
 static const int64_t GOVERNANCE_ORPHAN_EXPIRATION_TIME = 10*60;
 static const int64_t GOVERNANCE_WATCHDOG_EXPIRATION_TIME = 2*60*60;
 
-static const int GOVERNANCE_TRIGGER_EXPIRATION_BLOCKS = 576;
-
 // FOR SEEN MAP ARRAYS - GOVERNANCE OBJECTS AND VOTES
 static const int SEEN_OBJECT_IS_VALID = 0;
 static const int SEEN_OBJECT_ERROR_INVALID = 1;
@@ -112,8 +110,8 @@ struct vote_rec_t {
 class CGovernanceObject
 {
     friend class CGovernanceManager;
-
     friend class CGovernanceTriggerManager;
+    friend class CSuperblock;
 
 public: // Types
     typedef std::map<COutPoint, vote_rec_t> vote_m_t;

src/governance.cpp

@@ -1369,6 +1369,8 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex *pindex, CConnman& co
     LogPrint("gobject", "CGovernanceManager::UpdatedBlockTip -- nCachedBlockHeight: %d\n", nCachedBlockHeight);
 
     CheckPostponedObjects(connman);
+
+    CSuperblockManager::ExecuteBestSuperblock(pindex->nHeight);
 }
 
 void CGovernanceManager::RequestOrphanObjects(CConnman& connman)