[Consensus] Max payment count for budget proposals

src/budget/budgetmanager.cpp

@@ -749,47 +749,11 @@ std::string CBudgetManager::GetRequiredPaymentsString(int nBlockHeight)
 
 CAmount CBudgetManager::GetTotalBudget(int nHeight)
 {
-    if (Params().NetworkID() == CBaseChainParams::TESTNET) {
-        CAmount nSubsidy = 500 * COIN;
-        return ((nSubsidy / 100) * 10) * 146;
-    }
-
-    //get block value and calculate from that
-    CAmount nSubsidy = 0;
-    const Consensus::Params& consensus = Params().GetConsensus();
-    const bool isPoSActive = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_POS);
-    if (nHeight >= 151200 && !isPoSActive) {
-        nSubsidy = 50 * COIN;
-    } else if (isPoSActive && nHeight <= 302399) {
-        nSubsidy = 50 * COIN;
-    } else if (nHeight <= 345599 && nHeight >= 302400) {
-        nSubsidy = 45 * COIN;
-    } else if (nHeight <= 388799 && nHeight >= 345600) {
-        nSubsidy = 40 * COIN;
-    } else if (nHeight <= 431999 && nHeight >= 388800) {
-        nSubsidy = 35 * COIN;
-    } else if (nHeight <= 475199 && nHeight >= 432000) {
-        nSubsidy = 30 * COIN;
-    } else if (nHeight <= 518399 && nHeight >= 475200) {
-        nSubsidy = 25 * COIN;
-    } else if (nHeight <= 561599 && nHeight >= 518400) {
-        nSubsidy = 20 * COIN;
-    } else if (nHeight <= 604799 && nHeight >= 561600) {
-        nSubsidy = 15 * COIN;
-    } else if (nHeight <= 647999 && nHeight >= 604800) {
-        nSubsidy = 10 * COIN;
-    } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_ZC_V2)) {
-        nSubsidy = 10 * COIN;
-    } else {
-        nSubsidy = 5 * COIN;
-    }
+    // 20% of the block value
+    CAmount nSubsidy = GetBlockValue(nHeight) / 5;
 
-    // Amount of blocks in a months period of time (using 1 minutes per) = (60*24*30)
-    if (nHeight <= 172800) {
-        return 648000 * COIN;
-    } else {
-        return ((nSubsidy / 100) * 10) * 1440 * 30;
-    }
+    // multiplied by the number of blocks in a cycle (144 on testnet, 30*1440 on mainnet)
+    return nSubsidy * Params().GetConsensus().nBudgetCycleBlocks;
 }
 
 void CBudgetManager::AddSeenProposalVote(const CBudgetVote& vote)

src/budget/budgetproposal.cpp

@@ -8,6 +8,7 @@
 #include "masternodeman.h"
 
 CBudgetProposal::CBudgetProposal():
+        nAllotted(0),
         fValid(true),
         strInvalid(""),
         strProposalName("unknown"),
@@ -27,6 +28,7 @@ CBudgetProposal::CBudgetProposal(const std::string& name,
                                  const CAmount& amount,
                                  int blockstart,
                                  const uint256& nfeetxhash):
+        nAllotted(0),
         fValid(true),
         strInvalid(""),
         strProposalName(name),
@@ -38,14 +40,10 @@ CBudgetProposal::CBudgetProposal(const std::string& name,
         nTime(0)
 {
     const int nBlocksPerCycle = Params().GetConsensus().nBudgetCycleBlocks;
+    // !todo: remove this when v5 rules are enforced (nBlockStart is always = to nCycleStart)
     int nCycleStart = nBlockStart - nBlockStart % nBlocksPerCycle;
 
-    // Right now single payment proposals have nBlockEnd have a cycle too early!
-    // switch back if it break something else
-    // calculate the end of the cycle for this vote, add half a cycle (vote will be deleted after that block)
-    // nBlockEnd = nCycleStart + GetBudgetPaymentCycleBlocks() * nPaymentCount + GetBudgetPaymentCycleBlocks() / 2;
-
-    // Calculate the end of the cycle for this vote, vote will be deleted after next cycle
+    // calculate the expiration block
     nBlockEnd = nCycleStart + (nBlocksPerCycle + 1)  * paycount;
 }
 
@@ -90,8 +88,13 @@ bool CBudgetProposal::IsHeavilyDownvoted(bool fNewRules)
 
 bool CBudgetProposal::CheckStartEnd()
 {
-    if (nBlockStart < 0) {
-        strInvalid = "Invalid Proposal";
+    // !TODO: remove (and always use new rules) when all proposals submitted before v5 enforcement are expired.
+    bool fNewRules = Params().GetConsensus().NetworkUpgradeActive(nBlockStart, Consensus::UPGRADE_V5_0);
+
+    if (nBlockStart < 0 ||
+            // block start must be a superblock
+            (fNewRules && (nBlockStart % Params().GetConsensus().nBudgetCycleBlocks) != 0)) {
+        strInvalid = "Invalid nBlockStart";
         return false;
     }
 
@@ -100,6 +103,11 @@ bool CBudgetProposal::CheckStartEnd()
         return false;
     }
 
+    if (fNewRules && GetTotalPaymentCount() > Params().GetConsensus().nMaxProposalPayments) {
+        strInvalid = "Invalid payment count";
+        return false;
+    }
+
     return true;
 }
 

src/budget/budgetproposal.h

@@ -22,7 +22,7 @@ static const int64_t BUDGET_VOTE_UPDATE_MIN = 60 * 60;
 class CBudgetProposal
 {
 private:
-    CAmount nAlloted;
+    CAmount nAllotted;
     bool fValid;
     std::string strInvalid;
 
@@ -87,8 +87,8 @@ class CBudgetProposal
     int GetNays() const { return GetVoteCount(CBudgetVote::VOTE_NO); }
     int GetAbstains() const { return GetVoteCount(CBudgetVote::VOTE_ABSTAIN); };
     CAmount GetAmount() const { return nAmount; }
-    void SetAllotted(CAmount nAllotedIn) { nAlloted = nAllotedIn; }
-    CAmount GetAllotted() const { return nAlloted; }
+    void SetAllotted(CAmount nAllottedIn) { nAllotted = nAllottedIn; }
+    CAmount GetAllotted() const { return nAllotted; }
 
     void CleanAndRemove();
 

src/chainparams.cpp

@@ -147,6 +147,7 @@ class CMainParams : public CChainParams
         consensus.nTargetTimespanV2 = 30 * 60;
         consensus.nTargetSpacing = 1 * 60;
         consensus.nTimeSlotLength = 15;
+        consensus.nMaxProposalPayments = 6;
 
         // spork keys
         consensus.strSporkPubKey = "0410050aa740d280b134b40b40658781fc1116ba7700764e0ce27af3e1737586b3257d19232e0cb5084947f5107e44bcd577f126c9eb4a30ea2807b271d2145298";
@@ -281,6 +282,7 @@ class CTestNetParams : public CMainParams
         consensus.nTargetTimespanV2 = 30 * 60;
         consensus.nTargetSpacing = 1 * 60;
         consensus.nTimeSlotLength = 15;
+        consensus.nMaxProposalPayments = 20;
 
         // spork keys
         consensus.strSporkPubKey = "04677c34726c491117265f4b1c83cef085684f36c8df5a97a3a42fc499316d0c4e63959c9eca0dba239d9aaaf72011afffeb3ef9f51b9017811dec686e412eb504";

src/consensus/params.h

@@ -105,6 +105,7 @@ struct Params {
     int64_t nTargetTimespanV2;
     int64_t nTargetSpacing;
     int nTimeSlotLength;
+    int nMaxProposalPayments;
 
     // spork keys
     std::string strSporkPubKey;

src/rpc/budget.cpp

@@ -45,7 +45,7 @@ void budgetToJSON(const CBudgetProposal* pbudgetProposal, UniValue& bObj, int nC
     bObj.pushKV("IsValid", fValid);
     if (!fValid)
         bObj.pushKV("IsInvalidReason", pbudgetProposal->IsInvalidReason());
-    bObj.pushKV("Alloted", ValueFromAmount(pbudgetProposal->GetAllotted()));
+    bObj.pushKV("Allotted", ValueFromAmount(pbudgetProposal->GetAllotted()));
 }
 
 void checkBudgetInputs(const UniValue& params, std::string &strProposalName, std::string &strURL,
@@ -64,6 +64,12 @@ void checkBudgetInputs(const UniValue& params, std::string &strProposalName, std
     if (nPaymentCount < 1)
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid payment count, must be more than zero.");
 
+    int nMaxPayments = Params().GetConsensus().nMaxProposalPayments;
+    if (nPaymentCount > nMaxPayments) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER,
+                strprintf("Invalid payment count, must be <= %d", nMaxPayments));
+    }
+
     CBlockIndex* pindexPrev = GetChainTip();
     if (!pindexPrev)
         throw JSONRPCError(RPC_IN_WARMUP, "Try again after active chain is loaded");
@@ -538,8 +544,8 @@ UniValue getbudgetprojection(const JSONRPCRequest& request)
             "    \"IsEstablished\": true|false,  (boolean) Established (true) or (false)\n"
             "    \"IsValid\": true|false,        (boolean) Valid (true) or Invalid (false)\n"
             "    \"IsInvalidReason\": \"xxxx\",      (string) Error message, if any\n"
-            "    \"Alloted\": xxx.xxx,           (numeric) Amount alloted in current period\n"
-            "    \"TotalBudgetAlloted\": xxx.xxx (numeric) Total alloted\n"
+            "    \"Allotted\": xxx.xxx,           (numeric) Amount allotted in current period\n"
+            "    \"TotalBudgetAllotted\": xxx.xxx (numeric) Total allotted\n"
             "  }\n"
             "  ,...\n"
             "]\n"
@@ -556,7 +562,7 @@ UniValue getbudgetprojection(const JSONRPCRequest& request)
         UniValue bObj(UniValue::VOBJ);
         budgetToJSON(&p, bObj, g_budgetman.GetBestHeight());
         nTotalAllotted += p.GetAllotted();
-        bObj.pushKV("TotalBudgetAlloted", ValueFromAmount(nTotalAllotted));
+        bObj.pushKV("TotalBudgetAllotted", ValueFromAmount(nTotalAllotted));
         ret.push_back(bObj);
     }
 

src/test/budget_tests.cpp

@@ -23,7 +23,8 @@ BOOST_AUTO_TEST_CASE(budget_value)
 {
     SelectParams(CBaseChainParams::TESTNET);
     int nHeightTest = Params().GetConsensus().vUpgrades[Consensus::UPGRADE_ZC_V2].nActivationHeight + 1;
-    CheckBudgetValue(nHeightTest, "testnet", 7300*COIN);
+    CheckBudgetValue(nHeightTest-1, "testnet", 7200*COIN);
+    CheckBudgetValue(nHeightTest, "testnet", 144*COIN);
 
     SelectParams(CBaseChainParams::MAIN);
     nHeightTest = Params().GetConsensus().vUpgrades[Consensus::UPGRADE_ZC_V2].nActivationHeight + 1;

test/functional/rpc_budget.py

@@ -36,6 +36,10 @@ def run_test(self):
         assert_raises_rpc_error(-8, "Invalid payment count, must be more than zero.", self.nodes[0].preparebudget,
                                 name, scheme + url, 0, nextsuperblock, address, cycleamount)
 
+        self.log.info("Test with invalid (21) cycles")
+        assert_raises_rpc_error(-8, "Invalid payment count, must be <= 20", self.nodes[0].preparebudget,
+                                name, scheme + url, 21, nextsuperblock, address, cycleamount)
+
         self.log.info("Test with invalid block start")
         assert_raises_rpc_error(-8, "Invalid block start", self.nodes[0].preparebudget,
                                 name, scheme + url, numcycles, nextsuperblock - 12, address, cycleamount)
@@ -47,12 +51,14 @@ def run_test(self):
                                 name, scheme + url, numcycles, nextsuperblock, "DBREvBPNQguwuC4YMoCG5FoH1sA2YntvZm", cycleamount)
 
         self.log.info("Test with too low amount")
-        assert_raises_rpc_error(-8, "Invalid amount - Payment of 9.00 is less than minimum 10 PIV allowed", self.nodes[0].preparebudget,
-                                name, scheme + url, numcycles, nextsuperblock, address, 9)
+        invalid_amt = 9.99999999
+        assert_raises_rpc_error(-8, "Invalid amount - Payment of %.8f is less than minimum 10 PIV allowed" % invalid_amt, self.nodes[0].preparebudget,
+                                name, scheme + url, numcycles, nextsuperblock, address, invalid_amt)
 
         self.log.info("Test with too high amount")
-        assert_raises_rpc_error(-8, "Invalid amount - Payment of 648001.00 more than max of 648000.00", self.nodes[0].preparebudget,
-                                name, scheme + url, numcycles, nextsuperblock, address, 648001)
+        invalid_amt = 50 * 144 + 0.00000001
+        assert_raises_rpc_error(-8, "Invalid amount - Payment of %.8f more than max of 7200.00" % invalid_amt, self.nodes[0].preparebudget,
+                                name, scheme + url, numcycles, nextsuperblock, address, invalid_amt)
 
 
         self.log.info("Test without URL scheme")

test/functional/tiertwo_governance_sync_basic.py

@@ -7,6 +7,7 @@
 from test_framework.util import (
     assert_equal,
     assert_true,
+    Decimal,
 )
 
 import time
@@ -60,6 +61,39 @@ def check_vote_existence(self, proposalName, mnCollateralHash, voteType):
                     found = True
             assert_true(found, "Error checking vote existence in node " + str(i))
 
+    def get_proposal_obj(self, Name, URL, Hash, FeeHash, BlockStart, BlockEnd,
+                             TotalPaymentCount, RemainingPaymentCount, PaymentAddress,
+                             Ratio, Yeas, Nays, Abstains, TotalPayment, MonthlyPayment,
+                             IsEstablished, IsValid, Allotted, TotalBudgetAllotted, IsInvalidReason = ""):
+        obj = {}
+        obj["Name"] = Name
+        obj["URL"] = URL
+        obj["Hash"] = Hash
+        obj["FeeHash"] = FeeHash
+        obj["BlockStart"] = BlockStart
+        obj["BlockEnd"] = BlockEnd
+        obj["TotalPaymentCount"] = TotalPaymentCount
+        obj["RemainingPaymentCount"] = RemainingPaymentCount
+        obj["PaymentAddress"] = PaymentAddress
+        obj["Ratio"] = Ratio
+        obj["Yeas"] = Yeas
+        obj["Nays"] = Nays
+        obj["Abstains"] = Abstains
+        obj["TotalPayment"] = TotalPayment
+        obj["MonthlyPayment"] = MonthlyPayment
+        obj["IsEstablished"] = IsEstablished
+        obj["IsValid"] = IsValid
+        if IsInvalidReason != "":
+            obj["IsInvalidReason"] = IsInvalidReason
+        obj["Allotted"] = Allotted
+        obj["TotalBudgetAllotted"] = TotalBudgetAllotted
+        return obj
+
+    def check_budgetprojection(self, expected):
+        for i in range(self.num_nodes):
+            assert_equal(self.nodes[i].getbudgetprojection(), expected)
+            self.log.info("Budget projection valid for node %d" % i)
+
     def run_test(self):
         self.enable_mocktime()
         self.setup_2_masternodes_network()
@@ -106,7 +140,7 @@ def run_test(self):
         # let's wait a little bit and see if all nodes are sync
         time.sleep(1)
         self.check_proposal_existence(firstProposalName, proposalHash)
-        self.log.info("proposal broadcast succeed!")
+        self.log.info("proposal broadcast successful!")
 
         # Proposal is established after 5 minutes. Mine 7 blocks
         # Proposal needs to be on the chain > 5 min.
@@ -131,6 +165,20 @@ def run_test(self):
         self.check_vote_existence(firstProposalName, self.mnTwoTxHash, "YES")
         self.log.info("all good, MN2 vote accepted everywhere!")
 
+        # Now check the budget
+        blockStart = nextSuperBlockHeight
+        blockEnd = blockStart + firstProposalCycles * 145
+        TotalPayment = firstProposalAmountPerCycle * firstProposalCycles
+        Allotted = firstProposalAmountPerCycle
+        RemainingPaymentCount = firstProposalCycles
+        expected_budget = [
+            self.get_proposal_obj(firstProposalName, firstProposalLink, proposalHash, proposalFeeTxId, blockStart,
+                                  blockEnd, firstProposalCycles, RemainingPaymentCount, firstProposalAddress, 1,
+                                  2, 0, 0, Decimal(str(TotalPayment)), Decimal(str(firstProposalAmountPerCycle)),
+                                  True, True, Decimal(str(Allotted)), Decimal(str(Allotted)))
+                           ]
+        self.check_budgetprojection(expected_budget)
+
         # Quick block count check.
         assert_equal(self.ownerOne.getblockcount(), 276)
 
@@ -164,6 +212,10 @@ def run_test(self):
 
         self.log.info("budget proposal paid!, all good")
 
+        # Check that the proposal info returns updated payment count
+        expected_budget[0]["RemainingPaymentCount"] -= 1
+        self.check_budgetprojection(expected_budget)
+