Budget finalization, compare budget payments with existent proposals.

PIVX-Project · Jun 22, 2021 · 29e11d1 · 29e11d1
1 parent eceec6b
commit 29e11d1
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 16 deletions.
diff --git a/src/budget/budgetmanager.cpp b/src/budget/budgetmanager.cpp
@@ -185,7 +185,7 @@ std::string CBudgetManager::GetFinalizedBudgetStatus(const uint256& nHash) const
     return retBadHashes + " -- " + retBadPayeeOrAmount;
 }
 
-bool CBudgetManager::AddFinalizedBudget(CFinalizedBudget& finalizedBudget)
+bool CBudgetManager::AddFinalizedBudget(CFinalizedBudget& finalizedBudget, CNode* pfrom)
 {
     AssertLockNotHeld(cs_budgets);    // need to lock cs_main here (CheckCollateral)
     const uint256& nHash = finalizedBudget.GetHash();
@@ -216,6 +216,25 @@ bool CBudgetManager::AddFinalizedBudget(CFinalizedBudget& finalizedBudget)
         return false;
     }
 
+    // Compare budget payments with existent proposals, don't care on the order, just verify proposals existence.
+    std::vector<CBudgetProposal> vBudget = GetBudget();
+    std::map<uint256, CBudgetProposal> mapWinningProposals;
+    for (const CBudgetProposal& p: vBudget) { mapWinningProposals.emplace(p.GetHash(), p); }
+    if (!finalizedBudget.CheckProposals(mapWinningProposals)) {
+        finalizedBudget.SetStrInvalid("Invalid proposals");
+        LogPrint(BCLog::MNBUDGET,"%s: Budget finalization does not match with winning proposals\n", __func__);
+        // just for now (until v6), request proposals sync in case we are missing one of them.
+        if (pfrom) {
+            for (const auto& propId : finalizedBudget.GetProposalsHashes()) {
+                if (!g_budgetman.HaveProposal(propId)) {
+                    pfrom->PushInventory(CInv(MSG_BUDGET_PROPOSAL, propId));
+                }
+            }
+        }
+        return false;
+    }
+
+    // Add budget finalization.
     SetBudgetProposalsStr(finalizedBudget);
     {
         LOCK(cs_budgets);
@@ -979,14 +998,14 @@ int CBudgetManager::ProcessProposalVote(CBudgetVote& vote, CNode* pfrom)
     return 0;
 }
 
-int CBudgetManager::ProcessFinalizedBudget(CFinalizedBudget& finalbudget)
+int CBudgetManager::ProcessFinalizedBudget(CFinalizedBudget& finalbudget, CNode* pfrom)
 {
     const uint256& nHash = finalbudget.GetHash();
     if (HaveFinalizedBudget(nHash)) {
         masternodeSync.AddedBudgetItem(nHash);
         return 0;
     }
-    if (!AddFinalizedBudget(finalbudget)) {
+    if (!AddFinalizedBudget(finalbudget, pfrom)) {
         return 0;
     }
     finalbudget.Relay();
@@ -1080,7 +1099,7 @@ int CBudgetManager::ProcessMessageInner(CNode* pfrom, std::string& strCommand, C
         if (!finalbudget.ParseBroadcast(vRecv)) {
             return 20;
         }
-        return ProcessFinalizedBudget(finalbudget);
+        return ProcessFinalizedBudget(finalbudget, pfrom);
     }
 
     if (strCommand == NetMsgType::FINALBUDGETVOTE) {

diff --git a/src/budget/budgetmanager.h b/src/budget/budgetmanager.h
@@ -98,7 +98,7 @@ class CBudgetManager
     int ProcessBudgetVoteSync(const uint256& nProp, CNode* pfrom);
     int ProcessProposal(CBudgetProposal& proposal);
     int ProcessProposalVote(CBudgetVote& proposal, CNode* pfrom);
-    int ProcessFinalizedBudget(CFinalizedBudget& finalbudget);
+    int ProcessFinalizedBudget(CFinalizedBudget& finalbudget, CNode* pfrom);
     int ProcessFinalizedBudgetVote(CFinalizedBudgetVote& vote, CNode* pfrom);
 
     // functions returning a pointer in the map. Need cs_proposals/cs_budgets locked from the caller
@@ -118,7 +118,7 @@ class CBudgetManager
     bool IsBudgetPaymentBlock(int nBlockHeight) const;
     bool IsBudgetPaymentBlock(int nBlockHeight, int& nCountThreshold) const;
     bool AddProposal(CBudgetProposal& budgetProposal);
-    bool AddFinalizedBudget(CFinalizedBudget& finalizedBudget);
+    bool AddFinalizedBudget(CFinalizedBudget& finalizedBudget, CNode* pfrom = nullptr);
     uint256 SubmitFinalBudget();
 
     bool UpdateProposal(const CBudgetVote& vote, CNode* pfrom, std::string& strError);

diff --git a/test/functional/tiertwo_governance_invalid_budget.py b/test/functional/tiertwo_governance_invalid_budget.py
@@ -84,17 +84,9 @@ def run_test(self):
         time.sleep(1)
         self.stake_and_ping(self.minerAPos, 4, [self.mn1, self.mn2])
         res = self.minerA.createrawmnfinalbudget(budgetname, blockstart, proposals, feeBudgetId)
-        assert(res["result"] == "fin_budget_sent")
-        budgetFinHash = res["id"]
-        assert (budgetFinHash != "")
-        time.sleep(1)
+        assert(res["result"] == "error") # not accepted
 
-        self.log.info("Voting for invalid budget finalization...")
-        self.minerA.mnfinalbudget("vote-many", budgetFinHash)
-        self.stake_and_ping(self.minerAPos, 2, [self.mn1, self.mn2])
-        budFin = self.minerB.mnfinalbudget("show")
-        budget = budFin[next(iter(budFin))]
-        assert_equal(budget["VoteCount"], 2)
+        self.log.info("Good, invalid budget not accepted.")
 
         # Stake up until the block before the superblock.
         skip_blocks = next_superblock - self.minerA.getblockcount() - 1