More for governance block checks, p1 (non-compilable):

- add GetPaymentsLimit() and GetPaymentsTotalAmount()
- IsValidBlockHeight() should check nSuperblockStartBlock
- CSuperblock::IsValid should check payment limit and miner payout
- no cs_main
- slightly refactored related things

src/governance-classes.cpp

@@ -299,9 +299,8 @@ std::vector<CSuperblock_sptr> CGovernanceTriggerManager::GetActiveTriggers()
 
 bool CSuperblockManager::IsSuperblockTriggered(int nBlockHeight)
 {
-    if (!IsValidSuperblockHeight(nBlockHeight)) {
+    if (!CSuperblock::IsValidBlockHeight(nBlockHeight))
         return false;
-    }
 
     LOCK(governance.cs);
     // GET ALL ACTIVE TRIGGERS
@@ -392,17 +391,11 @@ bool CSuperblockManager::GetBestSuperblock(CSuperblock_sptr& pBlock, int nBlockH
 *   - Create the correct payment structure for a given superblock
 */
 
-void CSuperblockManager::CreateSuperblock(CMutableTransaction& txNew, CAmount nFees, int nBlockHeight)
+void CSuperblockManager::CreateSuperblock(CMutableTransaction& txNew, int nBlockHeight)
 {
     DBG( cout << "CSuperblockManager::CreateSuperblock Start" << endl; );
 
     LOCK(governance.cs);
-    AssertLockHeld(cs_main);
-
-    if(!chainActive.Tip()) {
-        DBG( cout << "CSuperblockManager::CreateSuperblock No active tip, returning" << endl; );
-        return;
-    }
 
     // GET THE BEST SUPERBLOCK FOR THIS BLOCK HEIGHT
 
@@ -419,6 +412,10 @@ void CSuperblockManager::CreateSuperblock(CMutableTransaction& txNew, CAmount nF
 
     DBG( cout << "CSuperblockManager::CreateSuperblock Number payments: " << pBlock->CountPayments() << endl; );
 
+    // TODO: How many payments can we add before things blow up?
+    //       Consider at least following limits:
+    //          - max coinbase tx size
+    //          - max "budget" available
     for(int i = 0; i < pBlock->CountPayments(); i++)  {
         CGovernancePayment payment;
         DBG( cout << "CSuperblockManager::CreateSuperblock i = " << i << endl; );
@@ -449,14 +446,14 @@ void CSuperblockManager::CreateSuperblock(CMutableTransaction& txNew, CAmount nF
     DBG( cout << "CSuperblockManager::CreateSuperblock End" << endl; );
 }
 
-bool CSuperblockManager::IsValid(const CTransaction& txNew, int nBlockHeight)
+bool CSuperblockManager::IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward)
 {
     // GET BEST SUPERBLOCK, SHOULD MATCH
     LOCK(governance.cs);
 
-    CSuperblock_sptr pBlock;
+    CSuperblock_sptr pSuperblock;
-    if(CSuperblockManager::GetBestSuperblock(pBlock, nBlockHeight)) {
+    if(CSuperblockManager::GetBestSuperblock(pSuperblock, nBlockHeight)) {
-        return pBlock->IsValid(txNew);
+        return pSuperblock->IsValid(txNew, nBlockHeight, blockReward);
     }
 
     return false;
@@ -512,9 +509,31 @@ CSuperblock(uint256& nHash)
     DBG( cout << "CSuperblock Constructor End" << endl; );
 }
 
-void 
+bool CSuperblock::IsValidBlockHeight(int nBlockHeight)
-CSuperblock::
+{
-ParsePaymentSchedule(std::string& strPaymentAddresses, std::string& strPaymentAmounts)
+    // SUPERBLOCKS CAN HAPPEN ONLY after hardfork and only ONCE PER CYCLE
+    return nBlockHeight >= Params().GetConsensus().nSuperblockStartBlock &&
+            ((nBlockHeight % Params().GetConsensus().nSuperblockCycle) == 0);
+}
+
+CAmount CSuperblock::GetPaymentsLimit(int nBlockHeight)
+{
+    const Consensus::Params& consensusParams = Params().GetConsensus();
+
+    if(!IsValidBlockHeight(nBlockHeight))
+        return 0;
+
+    // min subsidy for high diff networks and vice versa
+    int nBits = consensusParams.fPowAllowMinDifficultyBlocks ? UintToArith256(consensusParams.powLimit).GetCompact() : 1;
+    CAmount nBlockSubsidy = GetBlockSubsidy(nBits, nBlockHeight, consensusParams);
+    // 10% of all blocks issued during the cycle goes to superblock
+    CAmount nPaymentsLimit = nBlockSubsidy * consensusParams.nSuperblockCycle / 10;
+    LogPrint("gobject", "CSuperblock::GetPaymentsLimit -- Valid superblock height %d, payments max %lld\n", nBlockHeight, nPaymentsLimit);
+
+    return nPaymentsLimit;
+}
+
+void CSuperblock::ParsePaymentSchedule(std::string& strPaymentAddresses, std::string& strPaymentAmounts)
 {
     // SPLIT UP ADDR/AMOUNT STRINGS AND PUT IN VECTORS
 
@@ -571,6 +590,27 @@ ParsePaymentSchedule(std::string& strPaymentAddresses, std::string& strPaymentAm
     }
 }
 
+bool CSuperblock::GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet)
+{
+    if((nPaymentIndex<0) || (nPaymentIndex >= (int)vecPayments.size())) {
+        return false;
+    }
+
+    paymentRet = vecPayments[nPaymentIndex];
+    return true;
+}
+
+CAmount CSuperblock::GetPaymentsTotalAmount()
+{
+    CAmount nPaymentsTotalAmount = 0;
+    int nPayments = CountPayments();
+
+    for(int i = 0; i < nPayments; i++) {
+        nPaymentsTotalAmount += vecPayments[i].nAmount;
+    }
+
+    return nPaymentsTotalAmount;
+}
 
 /**
 *   Is Transaction Valid
@@ -578,7 +618,7 @@ ParsePaymentSchedule(std::string& strPaymentAddresses, std::string& strPaymentAm
 *   - Does this transaction match the superblock?
 */
 
-bool CSuperblock::IsValid(const CTransaction& txNew)
+bool CSuperblock::IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward)
 {
     // TODO : LOCK(cs);
     // No reason for a lock here now since this method only accesses data
@@ -598,18 +638,34 @@ bool CSuperblock::IsValid(const CTransaction& txNew)
     // superblock payments and the payments actually in the block, after
     // skipping any initial miner payments.
 
-    if(nMinerPayments<0)  {
+    if(nMinerPayments < 0) {
         // This means the block cannot have all the superblock payments
         // so it is not valid.
-        LogPrintf("CSuperblock::IsValid WARNING: Block invalid: Too few superblock payments");
+        // TODO: could that be that we just hit coinbase size limit?
+        LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid, too few superblock payments\n");
+        return false;
+    }
+
+    // payments should not exceed limit
+    CAmount nPaymentsTotalAmount = GetPaymentsTotalAmount();
+    CAmount nPaymentsLimit = GetPaymentsLimit(nBlockHeight);
+    if(nPaymentsTotalAmount > nPaymentsLimit) {
+        LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid, payments limit exceeded: payments %lld, limit %lld\n", nPaymentsTotalAmount, nPaymentsLimit);
+        return false;
+    }
+
+    // miner should not get more than he would usually get
+    CAmount nBlockValue = txNew.GetValueOut();
+    if(nBlockValue > blockReward + nPaymentsTotalAmount) {
+        LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid, block value limit exceeded: block %lld, limit %lld\n", nBlockValue, blockReward + nPaymentsTotalAmount);
         return false;
     }
 
     for(int i = 0; i < nPayments; i++) {
         CGovernancePayment payment;
-        if(!GetPayment(i, payment))  {
+        if(!GetPayment(i, payment)) {
             // This shouldn't happen so log a warning
-            LogPrintf("CSuperblock::IsValid WARNING: Failed to find payment: %d of %d total payments", i, nPayments);
+            LogPrintf("CSuperblock::IsValid -- WARNING: Failed to find payment: %d of %d total payments\n", i, nPayments);
             continue;
         }
 
@@ -618,13 +674,13 @@ bool CSuperblock::IsValid(const CTransaction& txNew)
         bool fPaymentMatch = ((payment.script == txNew.vout[nVoutIndex].scriptPubKey) &&
                               (payment.nAmount == txNew.vout[nVoutIndex].nValue));
 
-        if(!fPaymentMatch)  {
+        if(!fPaymentMatch) {
             // MISMATCHED SUPERBLOCK OUTPUT!
 
             CTxDestination address1;
             ExtractDestination(payment.script, address1);
             CBitcoinAddress address2(address1);
-            LogPrintf("CSuperblock::IsValid WARNING: Block invalid: output n %d payment %d to %s\n", nVoutIndex, payment.nAmount, address2.ToString());
+            LogPrintf("CSuperblock::IsValid -- WARNING: Block invalid: output n %d payment %d to %s\n", nVoutIndex, payment.nAmount, address2.ToString());
 
             return false;
         }

src/governance-classes.h

@@ -83,6 +83,9 @@ private:
 
 class CSuperblockManager
 {
+private:
+    static bool GetBestSuperblock(CSuperblock_sptr& pBlock, int nBlockHeight);
+
 public:
 
     /**
@@ -90,22 +93,12 @@ public:
      *
      *   - See if this block can be a superblock
      */
-    static bool IsValidSuperblockHeight(int nBlockHeight)
-    {
-        // SUPERBLOCKS CAN HAPPEN ONCE PER DAY
-        return ((nBlockHeight % Params().GetConsensus().nSuperblockCycle) == 1);
-    }
-
     static bool IsSuperblockTriggered(int nBlockHeight);
-    static void CreateSuperblock(CMutableTransaction& txNew, CAmount nFees, int nBlockHeight);
 
+    static void CreateSuperblock(CMutableTransaction& txNew, int nBlockHeight);
 
     static std::string GetRequiredPaymentsString(int nBlockHeight);
-    static bool IsValid(const CTransaction& txNew, int nBlockHeight);
+    static bool IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
-
-private:
-    static bool GetBestSuperblock(CSuperblock_sptr& pBlock, int nBlockHeight);
-
 };
 
 /**
@@ -181,23 +174,26 @@ private:
     int nStatus;
     std::vector<CGovernancePayment> vecPayments;
 
+    void ParsePaymentSchedule(std::string& strPaymentAddresses, std::string& strPaymentAmounts);
+
 public:
 
     CSuperblock();
-
     CSuperblock(uint256& nHash);
 
-    int GetStatus()
+    static bool IsValidBlockHeight(int nBlockHeight);
-    {
+    static CAmount GetPaymentsLimit(int nBlockHeight);
-        return nStatus;
-    }
 
-    void SetStatus(int nStatus_)
+    int GetStatus() { return nStatus; }
-    {
+    void SetStatus(int nStatusIn) { nStatus = nStatusIn; }
-        nStatus = nStatus_;
-    }
 
-    CGovernanceObject* GetGovernanceObject()  {
+    // IS THIS TRIGGER ALREADY EXECUTED?
+    bool IsExecuted() { return nStatus == SEEN_OBJECT_EXECUTED; }
+    // TELL THE ENGINE WE EXECUTED THIS EVENT
+    void SetExecuted() { nStatus = SEEN_OBJECT_EXECUTED; }
+
+    CGovernanceObject* GetGovernanceObject()
+    {
         AssertLockHeld(governance.cs);
         CGovernanceObject* pObj = governance.FindGovernanceObject(nGovObjHash);
         return pObj;
@@ -225,37 +221,11 @@ public:
         return nEpochStart;
     }
 
-    // IS THIS TRIGGER ALREADY EXECUTED?
+    int CountPayments() { return (int)vecPayments.size(); }
-    bool IsExecuted()
+    bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet);
-    {
+    CAmount GetPaymentsTotalAmount();
-        return (nStatus == SEEN_OBJECT_EXECUTED);
-    }
 
-    // TELL THE ENGINE WE EXECUTED THIS EVENT
+    bool IsValid(const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
-    void SetExecuted()
-    {
-        nStatus = SEEN_OBJECT_EXECUTED;
-    }
-
-    int CountPayments()
-    {
-        return (int)vecPayments.size();
-    }
-
-    bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentOut) 
-    {
-        if((nPaymentIndex<0) || (nPaymentIndex >= (int)vecPayments.size())) {
-            return false;
-        }
-
-        paymentOut = vecPayments[nPaymentIndex];
-        return true;
-    }
-
-    bool IsValid(const CTransaction& txNew);
-
-private:
-    void ParsePaymentSchedule(std::string& strPaymentAddresses, std::string& strPaymentAmounts);
 };
 
 #endif