Rate check fixes (#1196)

* Fix rate checks for governance objects

* Do not update last vote time when rate checks are disabled

* Bump governance serialization version

src/governance-object.cpp

@@ -150,6 +150,7 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom,
     }
     vote_instance_t& voteInstance = it2->second;
     int64_t nNow = GetTime();
+    int64_t nVoteTimeUpdate = voteInstance.nTime;
     if(governance.AreRateChecksEnabled()) {
         int64_t nTimeDelta = nNow - voteInstance.nTime;
         if(nTimeDelta < GOVERNANCE_UPDATE_MIN) {
@@ -160,6 +161,7 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom,
                  << ", time delta = " << nTimeDelta << "\n";
             LogPrint("gobject", ostr.str().c_str());
             exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_TEMPORARY_ERROR);
+            nVoteTimeUpdate = nNow;
             return false;
         }
     }
@@ -175,7 +177,7 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom,
         governance.AddInvalidVote(vote);
         return false;
     }
-    voteInstance = vote_instance_t(vote.GetOutcome(), nNow);
+    voteInstance = vote_instance_t(vote.GetOutcome(), nVoteTimeUpdate);
     fileVotes.AddVote(vote);
     fDirtyCache = true;
     return true;
@@ -459,15 +461,6 @@ bool CGovernanceObject::IsValidLocally(const CBlockIndex* pindex, std::string& s
                 return false;
             }
 
-            // Only perform rate check if we are synced because during syncing it is expected
-            // that objects will be seen in rapid succession
-            if(masternodeSync.IsSynced()) {
-                if(!governance.MasternodeRateCheck(vinMasternode, nObjectType)) {
-                    strError = "Masternode attempting to create too many objects: " + strOutpoint;
-                    return false;
-                }
-            }
-
             return true;
         }
 

src/governance.cpp

@@ -28,7 +28,7 @@ std::map<uint256, int64_t> mapAskedForGovernanceObject;
 
 int nSubmittedFinalBudget;
 
-const std::string CGovernanceManager::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-2";
+const std::string CGovernanceManager::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-3";
 
 CGovernanceManager::CGovernanceManager()
     : pCurrentBlockIndex(NULL),
@@ -41,7 +41,7 @@ CGovernanceManager::CGovernanceManager()
       mapVoteToObject(MAX_CACHE_SIZE),
       mapInvalidVotes(MAX_CACHE_SIZE),
       mapOrphanVotes(MAX_CACHE_SIZE),
-      mapLastMasternodeTrigger(),
+      mapLastMasternodeObject(),
       setRequestedObjects(),
       fRateChecksEnabled(true),
       cs()
@@ -171,6 +171,11 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C
             return;
         }
 
+        if(!MasternodeRateCheck(govobj, true)) {
+            LogPrintf("MNGOVERNANCEOBJECT -- masternode rate check failed - %s - (current block height %d) \n", strHash, nCachedBlockHeight);
+            return;
+        }
+
         std::string strError = "";
         // CHECK OBJECT AGAINST LOCAL BLOCKCHAIN
 
@@ -256,6 +261,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, CGovernance
     std::vector<vote_time_pair_t> vecVotePairs;
     mapOrphanVotes.GetAll(nHash, vecVotePairs);
 
+    fRateChecksEnabled = false;
     int64_t nNow = GetAdjustedTime();
     for(size_t i = 0; i < vecVotePairs.size(); ++i) {
         bool fRemove = false;
@@ -272,6 +278,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, CGovernance
             mapOrphanVotes.Erase(nHash, pairVote);
         }
     }
+    fRateChecksEnabled = true;
 }
 
 bool CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj)
@@ -307,13 +314,8 @@ bool CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj)
               << ", nObjectType = " << govobj.nObjectType
               << endl; );
 
-    if(govobj.GetObjectType() == GOVERNANCE_OBJECT_TRIGGER) {
-        mapLastMasternodeTrigger[govobj.GetMasternodeVin().prevout] = nCachedBlockHeight;
-    }
-
     switch(govobj.nObjectType) {
     case GOVERNANCE_OBJECT_TRIGGER:
-        mapLastMasternodeTrigger[govobj.vinMasternode.prevout] = nCachedBlockHeight;
         DBG( cout << "CGovernanceManager::AddGovernanceObject Before AddNewTrigger" << endl; );
         triggerman.AddNewTrigger(nHash);
         DBG( cout << "CGovernanceManager::AddGovernanceObject After AddNewTrigger" << endl; );
@@ -686,37 +688,72 @@ void CGovernanceManager::Sync(CNode* pfrom, uint256 nProp)
     LogPrintf("CGovernanceManager::Sync -- sent %d objects and %d votes to peer=%d\n", nObjCount, nVoteCount, pfrom->id);
 }
 
-bool CGovernanceManager::MasternodeRateCheck(const CTxIn& vin, int nObjectType)
+bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateLast)
 {
     LOCK(cs);
 
+    if(!masternodeSync.IsSynced()) {
+        return true;
+    }
+
     if(!fRateChecksEnabled) {
         return true;
     }
 
-    int mindiff = 0;
+    int nObjectType = govobj.GetObjectType();
+    if((nObjectType != GOVERNANCE_OBJECT_TRIGGER) && (nObjectType != GOVERNANCE_OBJECT_WATCHDOG)) {
+        return true;
+    }
+
+    const CTxIn& vin = govobj.GetMasternodeVin();
+
+    txout_m_it it  = mapLastMasternodeObject.find(vin.prevout);
+
+    if(it == mapLastMasternodeObject.end()) {
+        if(fUpdateLast) {
+            it = mapLastMasternodeObject.insert(txout_m_t::value_type(vin.prevout, last_object_rec(0, 0))).first;
             switch(nObjectType) {
             case GOVERNANCE_OBJECT_TRIGGER:
-        mindiff = Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockCycle / 10;
+                it->second.nLastTriggerBlockHeight = nCachedBlockHeight;
                 break;
             case GOVERNANCE_OBJECT_WATCHDOG:
-        mindiff = 1;
+                it->second.nLastWatchdogBlockHeight = nCachedBlockHeight;
+                break;
+            default:
+                break;
+            }
+        }
+        return true;
+    }
+
+    int nMinDiff = 0;
+    int nObjectBlock = 0;
+    switch(nObjectType) {
+    case GOVERNANCE_OBJECT_TRIGGER:
+        // Allow 1 trigger per mn per cycle, with a small fudge factor
+        nMinDiff = Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockCycle / 10;
+        nObjectBlock = it->second.nLastTriggerBlockHeight;
+        if(fUpdateLast) {
+            it->second.nLastTriggerBlockHeight = nCachedBlockHeight;
+        }
+        break;
+    case GOVERNANCE_OBJECT_WATCHDOG:
+        nMinDiff = 1;
+        nObjectBlock = it->second.nLastWatchdogBlockHeight;
+        if(fUpdateLast) {
+            it->second.nLastWatchdogBlockHeight = nCachedBlockHeight;
+        }
         break;
     default:
         break;
     }
 
-    txout_m_it it  = mapLastMasternodeTrigger.find(vin.prevout);
+    if((nCachedBlockHeight - nObjectBlock) > nMinDiff) {
-    if(it == mapLastMasternodeTrigger.end()) {
-        return true;
-    }
-    // Allow 1 trigger per mn per cycle, with a small fudge factor
-    if((nCachedBlockHeight - it->second) > mindiff) {
         return true;
     }
 
     LogPrintf("CGovernanceManager::MasternodeRateCheck -- Rate too high: vin = %s, current height = %d, last MN height = %d, minimum difference = %d\n",
-              vin.prevout.ToStringShort(), nCachedBlockHeight, it->second, mindiff);
+              vin.prevout.ToStringShort(), nCachedBlockHeight, nObjectBlock, nMinDiff);
     return false;
 }
 

src/governance.h

@@ -48,6 +48,26 @@ class CGovernanceManager
     friend class CGovernanceObject;
 
 public: // Types
+    struct last_object_rec {
+        last_object_rec(int nLastTriggerBlockHeightIn = 0, int nLastWatchdogBlockHeightIn = 0)
+            : nLastTriggerBlockHeight(nLastTriggerBlockHeightIn),
+              nLastWatchdogBlockHeight(nLastWatchdogBlockHeightIn)
+            {}
+
+        ADD_SERIALIZE_METHODS;
+
+        template <typename Stream, typename Operation>
+        inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion)
+        {
+            READWRITE(nLastTriggerBlockHeight);
+            READWRITE(nLastWatchdogBlockHeight);
+        }
+
+        int nLastTriggerBlockHeight;
+        int nLastWatchdogBlockHeight;
+    };
+
+
     typedef std::map<uint256, CGovernanceObject> object_m_t;
 
     typedef object_m_t::iterator object_m_it;
@@ -74,7 +94,7 @@ public: // Types
 
     typedef object_m_t::size_type size_type;
 
-    typedef std::map<COutPoint, int> txout_m_t;
+    typedef std::map<COutPoint, last_object_rec > txout_m_t;
 
     typedef txout_m_t::iterator txout_m_it;
 
@@ -124,7 +144,7 @@ private:
 
     vote_mcache_t mapOrphanVotes;
 
-    txout_m_t mapLastMasternodeTrigger;
+    txout_m_t mapLastMasternodeObject;
 
     hash_s_t setRequestedObjects;
 
@@ -186,7 +206,7 @@ public:
         mapVoteToObject.Clear();
         mapInvalidVotes.Clear();
         mapOrphanVotes.Clear();
-        mapLastMasternodeTrigger.clear();
+        mapLastMasternodeObject.clear();
     }
 
     std::string ToString() const;
@@ -209,7 +229,7 @@ public:
         READWRITE(mapOrphanVotes);
         READWRITE(mapObjects);
         READWRITE(mapWatchdogObjects);
-        READWRITE(mapLastMasternodeTrigger);
+        READWRITE(mapLastMasternodeObject);
         if(ser_action.ForRead() && (strVersion != SERIALIZATION_VERSION_STRING)) {
             Clear();
             return;
@@ -235,7 +255,7 @@ public:
 
     void AddSeenVote(uint256 nHash, int status);
 
-    bool MasternodeRateCheck(const CTxIn& vin, int nObjectType);
+    bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateLast = false);
 
     bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception) {
         bool fOK = ProcessVote(NULL, vote, exception);

src/rpcgovernance.cpp

@@ -217,6 +217,9 @@ UniValue gobject(const UniValue& params, bool fHelp)
         }
 
         // RELAY THIS OBJECT
+        if(!governance.MasternodeRateCheck(govobj)) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Object creation rate limit exceeded");
+        }
         governance.AddSeenGovernanceObject(govobj.GetHash(), SEEN_OBJECT_IS_VALID);
         govobj.Relay();
         governance.AddGovernanceObject(govobj);