Drop watchdogs, replace them with sentinel pings (#1949)

* Drop watchdogs, replace them with sentinel pings

* Address review comments

* revert `()`

Author: UdjinM6

src/activemasternode.cpp

@@ -98,7 +98,7 @@ bool CActiveMasternode::SendMasternodePing(CConnman& connman)
     CMasternodePing mnp(outpoint);
     mnp.nSentinelVersion = nSentinelVersion;
     mnp.fSentinelIsCurrent =
-            (abs(GetAdjustedTime() - nSentinelPingTime) < MASTERNODE_WATCHDOG_MAX_SECONDS);
+            (abs(GetAdjustedTime() - nSentinelPingTime) < MASTERNODE_SENTINEL_PING_MAX_SECONDS);
     if(!mnp.Sign(keyMasternode, pubKeyMasternode)) {
         LogPrintf("CActiveMasternode::SendMasternodePing -- ERROR: Couldn't sign Masternode Ping\n");
         return false;

src/governance-object.cpp

@@ -447,9 +447,11 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingMast
     }
 
     switch(nObjectType) {
+        case GOVERNANCE_OBJECT_WATCHDOG:
+            // watchdogs are deprecated
+            return false;
         case GOVERNANCE_OBJECT_PROPOSAL:
         case GOVERNANCE_OBJECT_TRIGGER:
-        case GOVERNANCE_OBJECT_WATCHDOG:
             if (vchData.size() > MAX_GOVERNANCE_OBJECT_DATA_SIZE) {
                 strError = strprintf("Invalid object size %d", vchData.size());
                 return false;
@@ -465,7 +467,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingMast
     // CHECK COLLATERAL IF REQUIRED (HIGH CPU USAGE)
 
     if(fCheckCollateral) { 
-        if((nObjectType == GOVERNANCE_OBJECT_TRIGGER) || (nObjectType == GOVERNANCE_OBJECT_WATCHDOG)) {
+        if(nObjectType == GOVERNANCE_OBJECT_TRIGGER) {
             std::string strOutpoint = masternodeOutpoint.ToStringShort();
             masternode_info_t infoMn;
             if(!mnodeman.GetMasternodeInfo(masternodeOutpoint, infoMn)) {

src/governance-object.h

@@ -42,7 +42,6 @@ static const int64_t GOVERNANCE_MIN_RELAY_FEE_CONFIRMATIONS = 1;
 static const int64_t GOVERNANCE_UPDATE_MIN = 60*60;
 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;
 
 // FOR SEEN MAP ARRAYS - GOVERNANCE OBJECTS AND VOTES
 static const int SEEN_OBJECT_IS_VALID = 0;

src/governance.cpp

@@ -29,9 +29,6 @@ CGovernanceManager::CGovernanceManager()
       mapObjects(),
       mapErasedGovernanceObjects(),
       mapMasternodeOrphanObjects(),
-      mapWatchdogObjects(),
-      nHashWatchdogCurrent(),
-      nTimeWatchdogCurrent(0),
       cmapVoteToObject(MAX_CACHE_SIZE),
       cmapInvalidVotes(MAX_CACHE_SIZE),
       cmmapOrphanVotes(MAX_CACHE_SIZE),
@@ -321,26 +318,6 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman
 
     LogPrint("gobject", "CGovernanceManager::AddGovernanceObject -- Adding object: hash = %s, type = %d\n", nHash.ToString(), govobj.GetObjectType());
 
-    if(govobj.nObjectType == GOVERNANCE_OBJECT_WATCHDOG) {
-        // If it's a watchdog, make sure it fits required time bounds
-        if((govobj.GetCreationTime() < GetAdjustedTime() - GOVERNANCE_WATCHDOG_EXPIRATION_TIME ||
-            govobj.GetCreationTime() > GetAdjustedTime() + GOVERNANCE_WATCHDOG_EXPIRATION_TIME)
-            ) {
-            // drop it
-            LogPrint("gobject", "CGovernanceManager::AddGovernanceObject -- CreationTime is out of bounds: hash = %s\n", nHash.ToString());
-            return;
-        }
-
-        if(!UpdateCurrentWatchdog(govobj)) {
-            // Allow wd's which are not current to be reprocessed
-            if(pfrom && (nHashWatchdogCurrent != uint256())) {
-                pfrom->PushInventory(CInv(MSG_GOVERNANCE_OBJECT, nHashWatchdogCurrent));
-            }
-            LogPrint("gobject", "CGovernanceManager::AddGovernanceObject -- Watchdog not better than current: hash = %s\n", nHash.ToString());
-            return;
-        }
-    }
-
     // INSERT INTO OUR GOVERNANCE OBJECT MEMORY
     mapObjects.insert(std::make_pair(nHash, govobj));
 
@@ -351,18 +328,10 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman
               << ", nObjectType = " << govobj.nObjectType
               << std::endl; );
 
-    switch(govobj.nObjectType) {
-    case GOVERNANCE_OBJECT_TRIGGER:
+    if (govobj.nObjectType == GOVERNANCE_OBJECT_TRIGGER) {
         DBG( std::cout << "CGovernanceManager::AddGovernanceObject Before AddNewTrigger" << std::endl; );
         triggerman.AddNewTrigger(nHash);
         DBG( std::cout << "CGovernanceManager::AddGovernanceObject After AddNewTrigger" << std::endl; );
-        break;
-    case GOVERNANCE_OBJECT_WATCHDOG:
-        mapWatchdogObjects[nHash] = govobj.GetCreationTime() + GOVERNANCE_WATCHDOG_EXPIRATION_TIME;
-        LogPrint("gobject", "CGovernanceManager::AddGovernanceObject -- Added watchdog to map: hash = %s\n", nHash.ToString());
-        break;
-    default:
-        break;
     }
 
     LogPrintf("AddGovernanceObject -- %s new, received form %s\n", strHash, pfrom? pfrom->GetAddrName() : "NULL");
@@ -381,40 +350,6 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman
     DBG( std::cout << "CGovernanceManager::AddGovernanceObject END" << std::endl; );
 }
 
-bool CGovernanceManager::UpdateCurrentWatchdog(CGovernanceObject& watchdogNew)
-{
-    bool fAccept = false;
-
-    arith_uint256 nHashNew = UintToArith256(watchdogNew.GetHash());
-    arith_uint256 nHashCurrent = UintToArith256(nHashWatchdogCurrent);
-
-    int64_t nExpirationDelay = GOVERNANCE_WATCHDOG_EXPIRATION_TIME / 2;
-    int64_t nNow = GetAdjustedTime();
-
-    if(nHashWatchdogCurrent == uint256() ||                                             // no known current OR
-       ((nNow - watchdogNew.GetCreationTime() < nExpirationDelay) &&                    // (new one is NOT expired AND
-        ((nNow - nTimeWatchdogCurrent > nExpirationDelay) || (nHashNew > nHashCurrent)))//  (current is expired OR
-                                                                                        //   its hash is lower))
-    ) {
-        LOCK(cs);
-        object_m_it it = mapObjects.find(nHashWatchdogCurrent);
-        if(it != mapObjects.end()) {
-            LogPrint("gobject", "CGovernanceManager::UpdateCurrentWatchdog -- Expiring previous current watchdog, hash = %s\n", nHashWatchdogCurrent.ToString());
-            it->second.fExpired = true;
-            if(it->second.nDeletionTime == 0) {
-                it->second.nDeletionTime = nNow;
-            }
-        }
-        nHashWatchdogCurrent = watchdogNew.GetHash();
-        nTimeWatchdogCurrent = watchdogNew.GetCreationTime();
-        fAccept = true;
-        LogPrint("gobject", "CGovernanceManager::UpdateCurrentWatchdog -- Current watchdog updated to: hash = %s\n",
-                 ArithToUint256(nHashNew).ToString());
-    }
-
-    return fAccept;
-}
-
 void CGovernanceManager::UpdateCachesAndClean()
 {
     LogPrint("gobject", "CGovernanceManager::UpdateCachesAndClean\n");
@@ -423,34 +358,6 @@ void CGovernanceManager::UpdateCachesAndClean()
 
     LOCK2(cs_main, cs);
 
-    // Flag expired watchdogs for removal
-    int64_t nNow = GetAdjustedTime();
-    LogPrint("gobject", "CGovernanceManager::UpdateCachesAndClean -- Number watchdogs in map: %d, current time = %d\n", mapWatchdogObjects.size(), nNow);
-    if(mapWatchdogObjects.size() > 1) {
-        hash_time_m_it it = mapWatchdogObjects.begin();
-        while(it != mapWatchdogObjects.end()) {
-            LogPrint("gobject", "CGovernanceManager::UpdateCachesAndClean -- Checking watchdog: %s, expiration time = %d\n", it->first.ToString(), it->second);
-            if(it->second < nNow) {
-                LogPrint("gobject", "CGovernanceManager::UpdateCachesAndClean -- Attempting to expire watchdog: %s, expiration time = %d\n", it->first.ToString(), it->second);
-                object_m_it it2 = mapObjects.find(it->first);
-                if(it2 != mapObjects.end()) {
-                    LogPrint("gobject", "CGovernanceManager::UpdateCachesAndClean -- Expiring watchdog: %s, expiration time = %d\n", it->first.ToString(), it->second);
-                    it2->second.fExpired = true;
-                    if(it2->second.nDeletionTime == 0) {
-                        it2->second.nDeletionTime = nNow;
-                    }
-                }
-                if(it->first == nHashWatchdogCurrent) {
-                    nHashWatchdogCurrent = uint256();
-                }
-                mapWatchdogObjects.erase(it++);
-            }
-            else {
-                ++it;
-            }
-        }
-    }
-
     for(size_t i = 0; i < vecDirtyHashes.size(); ++i) {
         object_m_it it = mapObjects.find(vecDirtyHashes[i]);
         if(it == mapObjects.end()) {
@@ -469,6 +376,8 @@ void CGovernanceManager::UpdateCachesAndClean()
     // Clean up any expired or invalid triggers
     triggerman.CleanAndRemove();
 
+    int64_t nNow = GetAdjustedTime();
+
     while(it != mapObjects.end())
     {
         CGovernanceObject* pObj = &((*it).second);
@@ -490,13 +399,9 @@ void CGovernanceManager::UpdateCachesAndClean()
             pObj->UpdateSentinelVariables();
         }
 
-        if(pObj->IsSetCachedDelete() && (nHash == nHashWatchdogCurrent)) {
-            nHashWatchdogCurrent = uint256();
-        }
-
         // IF DELETE=TRUE, THEN CLEAN THE MESS UP!
 
-        int64_t nTimeSinceDeletion = GetAdjustedTime() - pObj->GetDeletionTime();
+        int64_t nTimeSinceDeletion = nNow - pObj->GetDeletionTime();
 
         LogPrint("gobject", "CGovernanceManager::UpdateCachesAndClean -- Checking object for deletion: %s, deletion time = %d, time since deletion = %d, delete flag = %d, expired flag = %d\n",
                  strHash, pObj->GetDeletionTime(), nTimeSinceDeletion, pObj->IsSetCachedDelete(), pObj->IsSetExpired());
@@ -520,14 +425,14 @@ void CGovernanceManager::UpdateCachesAndClean()
                 }
             }
 
-            int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing;
-            int64_t nTimeExpired = pObj->GetCreationTime() + 2 * nSuperblockCycleSeconds + GOVERNANCE_DELETION_DELAY;
+            int64_t nTimeExpired{0};
 
-            if(pObj->GetObjectType() == GOVERNANCE_OBJECT_WATCHDOG) {
-                mapWatchdogObjects.erase(nHash);
-            } else if(pObj->GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) {
+            if(pObj->GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) {
                 // keep hashes of deleted proposals forever
                 nTimeExpired = std::numeric_limits<int64_t>::max();
+            } else {
+                int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing;
+                nTimeExpired = pObj->GetCreationTime() + 2 * nSuperblockCycleSeconds + GOVERNANCE_DELETION_DELAY;
             }
 
             mapErasedGovernanceObjects.insert(std::make_pair(nHash, nTimeExpired));
@@ -824,8 +729,7 @@ void CGovernanceManager::SyncAll(CNode* pnode, CConnman& connman)
 
 void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj)
 {
-    int nObjectType = govobj.GetObjectType();
-    if((nObjectType != GOVERNANCE_OBJECT_TRIGGER) && (nObjectType != GOVERNANCE_OBJECT_WATCHDOG))
+    if(govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER)
         return;
 
     const COutPoint& masternodeOutpoint = govobj.GetMasternodeOutpoint();
@@ -835,10 +739,7 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj)
         it = mapLastMasternodeObject.insert(txout_m_t::value_type(masternodeOutpoint, last_object_rec(true))).first;
 
     int64_t nTimestamp = govobj.GetCreationTime();
-    if (GOVERNANCE_OBJECT_TRIGGER == nObjectType)
-        it->second.triggerBuffer.AddTimestamp(nTimestamp);
-    else if (GOVERNANCE_OBJECT_WATCHDOG == nObjectType)
-        it->second.watchdogBuffer.AddTimestamp(nTimestamp);
+    it->second.triggerBuffer.AddTimestamp(nTimestamp);
 
     if (nTimestamp > GetTime() + MAX_TIME_FUTURE_DEVIATION - RELIABLE_PROPAGATION_TIME) {
         // schedule additional relay for the object
@@ -868,8 +769,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo
         return true;
     }
 
-    int nObjectType = govobj.GetObjectType();
-    if((nObjectType != GOVERNANCE_OBJECT_TRIGGER) && (nObjectType != GOVERNANCE_OBJECT_WATCHDOG)) {
+    if(govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) {
         return true;
     }
 
@@ -901,38 +801,26 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo
         return true;
     }
 
-    double dMaxRate = 1.1 / nSuperblockCycleSeconds;
-    double dRate = 0.0;
-    CRateCheckBuffer buffer;
-    switch(nObjectType) {
-    case GOVERNANCE_OBJECT_TRIGGER:
-        // Allow 1 trigger per mn per cycle, with a small fudge factor
-        buffer = it->second.triggerBuffer;
-        dMaxRate = 2 * 1.1 / double(nSuperblockCycleSeconds);
-        break;
-    case GOVERNANCE_OBJECT_WATCHDOG:
-        buffer = it->second.watchdogBuffer;
-        dMaxRate = 2 * 1.1 / 3600.;
-        break;
-    default:
-        break;
-    }
+    // Allow 1 trigger per mn per cycle, with a small fudge factor
+    double dMaxRate = 2 * 1.1 / double(nSuperblockCycleSeconds);
+
+    // Temporary copy to check rate after new timestamp is added
+    CRateCheckBuffer buffer = it->second.triggerBuffer;
 
     buffer.AddTimestamp(nTimestamp);
-    dRate = buffer.GetRate();
+    double dRate = buffer.GetRate();
 
-    bool fRateOK = ( dRate < dMaxRate );
+    if(dRate < dMaxRate) {
+        return true;
+    }
 
-    if(!fRateOK)
-    {
-        LogPrintf("CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = %s, masternode = %s, object timestamp = %d, rate = %f, max rate = %f\n",
-                  strHash, masternodeOutpoint.ToStringShort(), nTimestamp, dRate, dMaxRate);
+    LogPrintf("CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = %s, masternode = %s, object timestamp = %d, rate = %f, max rate = %f\n",
+              strHash, masternodeOutpoint.ToStringShort(), nTimestamp, dRate, dMaxRate);
 
-        if (fUpdateFailStatus)
-            it->second.fStatusOK = false;
-    }
+    if (fUpdateFailStatus)
+        it->second.fStatusOK = false;
 
-    return fRateOK;
+    return false;
 }
 
 bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman)
@@ -985,10 +873,6 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote,
     }
 
     bool fOk = govobj.ProcessVote(pfrom, vote, exception, connman) && cmapVoteToObject.Insert(nHashVote, &govobj);
-    if(fOk && govobj.GetObjectType() == GOVERNANCE_OBJECT_WATCHDOG) {
-        mnodeman.UpdateWatchdogVoteTime(vote.GetMasternodeOutpoint());
-        LogPrint("gobject", "CGovernanceObject::ProcessVote -- GOVERNANCE_OBJECT_WATCHDOG vote %s for %s\n", nHashVote.ToString(), nHashGovobj.ToString());
-    }
     LEAVE_CRITICAL_SECTION(cs);
     return fOk;
 }
@@ -1051,8 +935,7 @@ void CGovernanceManager::CheckPostponedObjects(CConnman& connman)
         const uint256& nHash = it->first;
         CGovernanceObject& govobj = it->second;
 
-        assert(govobj.GetObjectType() != GOVERNANCE_OBJECT_WATCHDOG &&
-               govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER);
+        assert(govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER);
 
         std::string strError;
         bool fMissingConfirmations;
@@ -1074,7 +957,7 @@ void CGovernanceManager::CheckPostponedObjects(CConnman& connman)
     }
 
 
-    // Perform additional relays for triggers/watchdogs
+    // Perform additional relays for triggers
     int64_t nNow = GetAdjustedTime();
     int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing;
 
@@ -1323,7 +1206,6 @@ std::string CGovernanceManager::ToString() const
 
     int nProposalCount = 0;
     int nTriggerCount = 0;
-    int nWatchdogCount = 0;
     int nOtherCount = 0;
 
     object_m_cit it = mapObjects.begin();
@@ -1336,19 +1218,16 @@ std::string CGovernanceManager::ToString() const
             case GOVERNANCE_OBJECT_TRIGGER:
                 nTriggerCount++;
                 break;
-            case GOVERNANCE_OBJECT_WATCHDOG:
-                nWatchdogCount++;
-                break;
             default:
                 nOtherCount++;
                 break;
         }
         ++it;
     }
 
-    return strprintf("Governance Objects: %d (Proposals: %d, Triggers: %d, Watchdogs: %d/%d, Other: %d; Erased: %d), Votes: %d",
+    return strprintf("Governance Objects: %d (Proposals: %d, Triggers: %d, Other: %d; Erased: %d), Votes: %d",
                     (int)mapObjects.size(),
-                    nProposalCount, nTriggerCount, nWatchdogCount, mapWatchdogObjects.size(), nOtherCount, (int)mapErasedGovernanceObjects.size(),
+                    nProposalCount, nTriggerCount, nOtherCount, (int)mapErasedGovernanceObjects.size(),
                     (int)cmapVoteToObject.GetSize());
 }