Implement one-block payment in FillBlockPayee()

* CBudgetManager::FillBlockPayee is paying all budget in a single
  block
* CFinalizedBudget::GetBlockStart and CFinalizedBudget::GetBlockEnd
  both return the same block
* CFinalizedBudget::GetBudgetPayments is introduced
* GetBudgetPaymentByBlock and GetPayeeAndAmount are removed

src/masternode-budget.cpp

@@ -24,6 +24,16 @@ std::map<uint256, int64_t> askedForSourceProposalOrBudget;
 std::vector<CBudgetProposalBroadcast> vecImmatureBudgetProposals;
 std::vector<CFinalizedBudgetBroadcast> vecImmatureFinalizedBudgets;
 
+namespace
+{
+    CBitcoinAddress ScriptToAddress(const CScript& script)
+    {
+        CTxDestination destination;
+        ExtractDestination(script, destination);
+        return destination;
+    }
+}
+
 CAmount BlocksBeforeSuperblockToSubmitFinalBudget()
 {
     assert(GetBudgetPaymentCycleBlocks() > 10);
@@ -519,46 +529,42 @@ void CBudgetManager::FillBlockPayee(CMutableTransaction& txNew, CAmount nFees)
     LOCK(cs);
 
     CBlockIndex* pindexPrev = chainActive.Tip();
-    if(!pindexPrev) return;
+    if(!pindexPrev)
+        return;
 
-    int nHighestCount = 0;
-    CScript payee;
-    CAmount nAmount = 0;
+    // Pay the miner
 
-    // ------- Grab The Highest Count
+    txNew.vout[0].nValue = GetBlockValue(pindexPrev->nBits, pindexPrev->nHeight, nFees);
 
-    std::map<uint256, CFinalizedBudget>::iterator it = mapFinalizedBudgets.begin();
-    while(it != mapFinalizedBudgets.end())
-    {
-        CFinalizedBudget* pfinalizedBudget = &((*it).second);
-        if(pfinalizedBudget->GetVoteCount() > nHighestCount &&
-                pindexPrev->nHeight + 1 >= pfinalizedBudget->GetBlockStart() &&
-                pindexPrev->nHeight + 1 <= pfinalizedBudget->GetBlockEnd() &&
-                pfinalizedBudget->GetPayeeAndAmount(pindexPrev->nHeight + 1, payee, nAmount)){
-                    nHighestCount = pfinalizedBudget->GetVoteCount();
-        }
+    // Find finalized budgets with the most votes
 
-        ++it;
-    }
+    CFinalizedBudget* budgetToPay = NULL;
+    typedef std::map<uint256, CFinalizedBudget>::iterator FinalizedBudgetIterator;
+    for (FinalizedBudgetIterator i = mapFinalizedBudgets.begin(); i != mapFinalizedBudgets.end(); ++i)
+    {
+        CFinalizedBudget& fb = i->second;
+        if (pindexPrev->nHeight + 1 != fb.GetBlockStart())
+            continue;
 
-    CAmount blockValue = GetBlockValue(pindexPrev->nBits, pindexPrev->nHeight, nFees);
+        if (fb.GetVoteCount() == 0)
+            continue;
 
-    //miners get the full amount on these blocks
-    txNew.vout[0].nValue = blockValue;
+        if ((!budgetToPay || fb.GetVoteCount() > budgetToPay->GetVoteCount()))
+            budgetToPay = &fb;
+    }
 
-    if(nHighestCount > 0){
-        txNew.vout.resize(2);
+    if (budgetToPay == NULL)
+        return;
 
-        //these are super blocks, so their value can be much larger than normal
-        txNew.vout[1].scriptPubKey = payee;
-        txNew.vout[1].nValue = nAmount;
+    // Pay the proposals
 
-        CTxDestination address1;
-        ExtractDestination(payee, address1);
-        CBitcoinAddress address2(address1);
+    BOOST_FOREACH(const CTxBudgetPayment& payment, budgetToPay->GetBudgetPayments())
+    {
+        LogPrintf("CBudgetManager::FillBlockPayee - Budget payment to %s for %lld; proposal %s\n",
+            ScriptToAddress(payment.payee).ToString(), payment.nAmount, payment.nProposalHash.ToString());
 
-        LogPrintf("CBudgetManager::FillBlockPayee - Budget payment to %s for %lld\n", address2.ToString(), nAmount);
-   }
+        txNew.vout.push_back(CTxOut(payment.nAmount, payment.payee));
+    }
 }
 
 CFinalizedBudget *CBudgetManager::FindFinalizedBudget(uint256 nHash)
@@ -810,18 +816,17 @@ std::string CBudgetManager::GetRequiredPaymentsString(int nBlockHeight)
     std::map<uint256, CFinalizedBudget>::iterator it = mapFinalizedBudgets.begin();
     while(it != mapFinalizedBudgets.end())
     {
-        CFinalizedBudget* pfinalizedBudget = &((*it).second);
-        if(nBlockHeight >= pfinalizedBudget->GetBlockStart() && nBlockHeight <= pfinalizedBudget->GetBlockEnd()){
-            CTxBudgetPayment payment;
-            if(pfinalizedBudget->GetBudgetPaymentByBlock(nBlockHeight, payment)){
+        CFinalizedBudget& pfinalizedBudget = it->second;
+        if(nBlockHeight == pfinalizedBudget.GetBlockStart())
+        {
+            BOOST_FOREACH(const CTxBudgetPayment& payment, pfinalizedBudget.GetBudgetPayments())
+            {
                 if(ret == "unknown-budget"){
                     ret = payment.nProposalHash.ToString();
                 } else {
                     ret += ",";
                     ret += payment.nProposalHash.ToString();
                 }
-            } else {
-                LogPrintf("CBudgetManager::GetRequiredPaymentsString - Couldn't find budget payment for block %d\n", nBlockHeight);
             }
         }
 
@@ -1902,28 +1907,22 @@ std::string CFinalizedBudget::GetStatus() const
     std::string retBadHashes = "";
     std::string retBadPayeeOrAmount = "";
 
-    for(int nBlockHeight = GetBlockStart(); nBlockHeight <= GetBlockEnd(); nBlockHeight++)
+    BOOST_FOREACH(const CTxBudgetPayment& payment, GetBudgetPayments())
     {
-        CTxBudgetPayment budgetPayment;
-        if(!GetBudgetPaymentByBlock(nBlockHeight, budgetPayment)){
-            LogPrintf("CFinalizedBudget::GetStatus - Couldn't find budget payment for block %lld\n", nBlockHeight);
-            continue;
-        }
-
-        CBudgetProposal* pbudgetProposal =  budget.FindProposal(budgetPayment.nProposalHash);
+        CBudgetProposal* pbudgetProposal =  budget.FindProposal(payment.nProposalHash);
         if(!pbudgetProposal){
             if(retBadHashes == ""){
-                retBadHashes = "Unknown proposal hash! Check this proposal before voting" + budgetPayment.nProposalHash.ToString();
+                retBadHashes = "Unknown proposal hash! Check this proposal before voting" + payment.nProposalHash.ToString();
             } else {
-                retBadHashes += "," + budgetPayment.nProposalHash.ToString();
+                retBadHashes += "," + payment.nProposalHash.ToString();
             }
         } else {
-            if(pbudgetProposal->GetPayee() != budgetPayment.payee || pbudgetProposal->GetAmount() != budgetPayment.nAmount)
+            if(pbudgetProposal->GetPayee() != payment.payee || pbudgetProposal->GetAmount() != payment.nAmount)
             {
                 if(retBadPayeeOrAmount == ""){
-                    retBadPayeeOrAmount = "Budget payee/nAmount doesn't match our proposal! " + budgetPayment.nProposalHash.ToString();
+                    retBadPayeeOrAmount = "Budget payee/nAmount doesn't match our proposal! " + payment.nProposalHash.ToString();
                 } else {
-                    retBadPayeeOrAmount += "," + budgetPayment.nProposalHash.ToString();
+                    retBadPayeeOrAmount += "," + payment.nProposalHash.ToString();
                 }
             }
         }
@@ -2014,30 +2013,12 @@ bool CFinalizedBudget::IsTransactionValid(const CTransaction& txNew, int nBlockH
     
     return found;
 }
-bool CFinalizedBudget::GetBudgetPaymentByBlock(int64_t nBlockHeight, CTxBudgetPayment& payment) const
-{
-    LOCK(cs);
-    assert(boost::is_sorted(vecBudgetPayments, ComparePayments));
-
-    int i = nBlockHeight - GetBlockStart();
-    if(i < 0) return false;
-    if(i > (int)vecBudgetPayments.size() - 1) return false;
-    payment = vecBudgetPayments[i];
-    return true;
-}
 
-bool CFinalizedBudget::GetPayeeAndAmount(int64_t nBlockHeight, CScript& payee, CAmount& nAmount) const
+const std::vector<CTxBudgetPayment>& CFinalizedBudget::GetBudgetPayments() const
 {
-    LOCK(cs);
-    assert(boost::is_sorted(vecBudgetPayments, ComparePayments));
-
-    int i = nBlockHeight - GetBlockStart();
-    if(i < 0) return false;
-    if(i > (int)vecBudgetPayments.size() - 1) return false;
-    payee = vecBudgetPayments[i].payee;
-    nAmount = vecBudgetPayments[i].nAmount;
-    return true;
+    return vecBudgetPayments;
 }
+
 void CFinalizedBudget::SubmitVote()
 {
     CPubKey pubKeyMasternode;

src/masternode-budget.h

@@ -255,11 +255,10 @@ public:
     const std::map<uint256, CFinalizedBudgetVote>& GetVotes() const { return mapVotes; }
     std::string GetProposals() const;
     int GetBlockStart() const {return nBlockStart;}
-    int GetBlockEnd() const {return nBlockStart + (int)(vecBudgetPayments.size() - 1);}
+    int GetBlockEnd() const {return nBlockStart;} // Paid in single block
     int GetVoteCount() const {return (int)mapVotes.size();}
+    const std::vector<CTxBudgetPayment>& GetBudgetPayments() const;
     bool IsTransactionValid(const CTransaction& txNew, int nBlockHeight);
-    bool GetBudgetPaymentByBlock(int64_t nBlockHeight, CTxBudgetPayment& payment) const;
-    bool GetPayeeAndAmount(int64_t nBlockHeight, CScript& payee, CAmount& nAmount) const;
 
     //check to see if we should vote on this
     bool AutoCheck();

src/test/governance-test.cpp

@@ -29,6 +29,11 @@ bool operator == (const CTxBudgetPayment& a, const CTxBudgetPayment& b)
            a.payee == b.payee;
 }
 
+bool operator != (const CTxBudgetPayment& a, const CTxBudgetPayment& b)
+{
+    return !(a == b);
+}
+
 std::ostream& operator<<(std::ostream& os, const CTxBudgetPayment& value)
 {
     return os << "{" << value.nProposalHash.ToString() << ":" << value.nAmount << "@" << value.payee.ToString() << "}";
@@ -518,7 +523,7 @@ BOOST_FIXTURE_TEST_SUITE(FinalizedBudget, FinalizedBudgetFixture)
         BOOST_CHECK(!budget.IsTransactionValid(expected, blockStart + 2));
     }
 
-    BOOST_AUTO_TEST_CASE(GetBudgetPaymentByBlock_Block0)
+    BOOST_AUTO_TEST_CASE(GetBudgetPayments)
     {
         // Set Up
         std::vector<CTxBudgetPayment> txBudgetPayments;
@@ -526,47 +531,17 @@ BOOST_FIXTURE_TEST_SUITE(FinalizedBudget, FinalizedBudgetFixture)
         txBudgetPayments.push_back(GetPayment(proposalB));
 
         CFinalizedBudgetBroadcast budget(budgetName, blockStart, txBudgetPayments, ArithToUint256(42));
-        CTxBudgetPayment actual;
-
-        // Call & Check
-        bool result = budget.GetBudgetPaymentByBlock(blockStart, actual);
-
-        BOOST_CHECK_EQUAL(actual, GetPayment(proposalB));
-        BOOST_CHECK(result);
-    }
-
-    BOOST_AUTO_TEST_CASE(GetBudgetPaymentByBlock_Block1)
-    {
-        // Set Up
-        std::vector<CTxBudgetPayment> txBudgetPayments;
-        txBudgetPayments.push_back(GetPayment(proposalA));
-        txBudgetPayments.push_back(GetPayment(proposalB));
 
-        CFinalizedBudgetBroadcast budget(budgetName, blockStart, txBudgetPayments, ArithToUint256(42));
-        CTxBudgetPayment actual;
+        std::vector<CTxBudgetPayment> expected;
+        expected.push_back(GetPayment(proposalB));
+        expected.push_back(GetPayment(proposalA));
 
         // Call & Check
-        bool result = budget.GetBudgetPaymentByBlock(blockStart + 1, actual);
-
-        BOOST_CHECK_EQUAL(actual, GetPayment(proposalA));
-        BOOST_CHECK(result);
-    }
-
-    BOOST_AUTO_TEST_CASE(GetBudgetPaymentByBlock_Block2)
-    {
-        // Set Up
-        std::vector<CTxBudgetPayment> txBudgetPayments;
-        txBudgetPayments.push_back(GetPayment(proposalA));
-        txBudgetPayments.push_back(GetPayment(proposalB));
+        std::vector<CTxBudgetPayment> actual = budget.GetBudgetPayments();
 
-        CFinalizedBudgetBroadcast budget(budgetName, blockStart, txBudgetPayments, ArithToUint256(42));
-        CTxBudgetPayment dummy;
-
-        // Call & Check
-        BOOST_CHECK(!budget.GetBudgetPaymentByBlock(blockStart + 2, dummy));
+        BOOST_CHECK_EQUAL_COLLECTIONS(actual.begin(), actual.end(), expected.begin(), expected.end());
     }
 
-
 BOOST_AUTO_TEST_SUITE_END()
 
 namespace
@@ -1002,10 +977,10 @@ BOOST_FIXTURE_TEST_SUITE(SuperblockPayment, SuperblockPaymentFixture)
         // Call & Check
 
         BOOST_CHECK_EQUAL(fb.GetBlockStart(), blockHeight);
-        BOOST_CHECK_EQUAL(fb.GetBlockEnd(), blockHeight + 2);
+        BOOST_CHECK_EQUAL(fb.GetBlockEnd(), blockHeight);
     }
 
-    BOOST_AUTO_TEST_CASE(FirstProposalIsPaid)
+    BOOST_AUTO_TEST_CASE(AllProposalsArePaidInOneBlock)
     {
         // Set Up
 
@@ -1027,6 +1002,8 @@ BOOST_FIXTURE_TEST_SUITE(SuperblockPayment, SuperblockPaymentFixture)
         std::vector<CTxOut> expected;
         expected.push_back(CTxOut(9 * COIN, CScript()));
         expected.push_back(CTxOut(404, PayToPublicKey(keyPairB.GetPubKey()))); // Highest amount is #1
+        expected.push_back(CTxOut(111, PayToPublicKey(keyPairC.GetPubKey())));
+        expected.push_back(CTxOut(42, PayToPublicKey(keyPairA.GetPubKey()))); // Lowest amount is #31
 
         // Call & Check
 
@@ -1038,7 +1015,7 @@ BOOST_FIXTURE_TEST_SUITE(SuperblockPayment, SuperblockPaymentFixture)
         BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), actual.vout.begin(), actual.vout.end());
     }
 
-    BOOST_AUTO_TEST_CASE(SecondProposalIsPaid)
+    BOOST_AUTO_TEST_CASE(NoProposalsArePaidAfterSuperblockIsPaid)
     {
         // Set Up
 
@@ -1062,7 +1039,6 @@ BOOST_FIXTURE_TEST_SUITE(SuperblockPayment, SuperblockPaymentFixture)
         // Set expectations
         std::vector<CTxOut> expected;
         expected.push_back(CTxOut(9 * COIN, CScript()));
-        expected.push_back(CTxOut(111, PayToPublicKey(keyPairC.GetPubKey()))); // Lower amount is #2
 
         // Call & Check