[TierTwo] P2SH Proposal Payouts

src/budget/budgetmanager.cpp

@@ -324,19 +324,18 @@ bool CBudgetManager::AddProposal(CBudgetProposal& budgetProposal)
 {
     AssertLockNotHeld(cs_proposals);    // need to lock cs_main here (CheckCollateral)
     const uint256& nHash = budgetProposal.GetHash();
-
+    int nCurrentHeight = GetBestHeight();
     if (WITH_LOCK(cs_proposals, return mapProposals.count(nHash))) {
         LogPrint(BCLog::MNBUDGET,"%s: proposal %s already added\n", __func__, nHash.ToString());
         return false;
     }
 
-    if (!budgetProposal.IsWellFormed(GetTotalBudget(budgetProposal.GetBlockStart()))) {
+    if (!budgetProposal.IsWellFormed(GetTotalBudget(budgetProposal.GetBlockStart()), nCurrentHeight)) {
         LogPrint(BCLog::MNBUDGET,"%s: Invalid budget proposal %s %s\n", __func__, nHash.ToString(), budgetProposal.IsInvalidLogStr());
         return false;
     }
 
     std::string strError;
-    int nCurrentHeight = GetBestHeight();
     const uint256& feeTxId = budgetProposal.GetFeeTXHash();
     if (!CheckCollateral(feeTxId, nHash, strError, budgetProposal.nTime, nCurrentHeight, false)) {
         LogPrint(BCLog::MNBUDGET,"%s: invalid budget proposal (%s) collateral id=%s - %s\n",

src/budget/budgetproposal.cpp

@@ -5,6 +5,7 @@
 
 #include "budget/budgetproposal.h"
 #include "chainparams.h"
+#include "logging.h"
 #include "script/standard.h"
 #include "utilstrencodings.h"
 
@@ -123,11 +124,11 @@ bool CBudgetProposal::CheckAmount(const CAmount& nTotalBudget)
     return true;
 }
 
-bool CBudgetProposal::CheckAddress()
+bool CBudgetProposal::CheckAddress(const int nCurrentHeight)
 {
-    // !TODO: There might be an issue with multisig in the coinbase on mainnet
-    // we will add support for it in a future release.
-    if (address.IsPayToScriptHash()) {
+    // Multisig is supported only after v6
+    bool isV6Enforced = Params().GetConsensus().NetworkUpgradeActive(nCurrentHeight, Consensus::UPGRADE_V6_0);
+    if (!isV6Enforced && address.IsPayToScriptHash()) {
         strInvalid = "Multisig is not currently supported.";
         return false;
     }
@@ -146,9 +147,6 @@ bool CBudgetProposal::CheckAddress()
     return true;
 }
 
-/* TODO: Add this to IsWellFormed() for the next hard-fork
- * This will networkly reject malformed proposal names and URLs
- */
 bool CBudgetProposal::CheckStrings()
 {
     if (strProposalName != SanitizeString(strProposalName)) {
@@ -157,12 +155,18 @@ bool CBudgetProposal::CheckStrings()
     }
     if (strURL != SanitizeString(strURL)) {
         strInvalid = "Proposal URL contains illegal characters.";
+        return false;
     }
+    return true;
 }
 
-bool CBudgetProposal::IsWellFormed(const CAmount& nTotalBudget)
+bool CBudgetProposal::IsWellFormed(const CAmount& nTotalBudget, const int nCurrentHeight)
 {
-    return CheckStartEnd() && CheckAmount(nTotalBudget) && CheckAddress();
+    bool isV6Enforced = Params().GetConsensus().NetworkUpgradeActive(nCurrentHeight, Consensus::UPGRADE_V6_0);
+    if (isV6Enforced && !CheckStrings()) {
+        return false;
+    }
+    return CheckStartEnd() && CheckAmount(nTotalBudget) && CheckAddress(nCurrentHeight);
 }
 
 bool CBudgetProposal::updateExpired(int nCurrentHeight)

src/budget/budgetproposal.h

@@ -41,7 +41,7 @@ class CBudgetProposal
     bool updateExpired(int nCurrentHeight);
     bool CheckStartEnd();
     bool CheckAmount(const CAmount& nTotalBudget);
-    bool CheckAddress();
+    bool CheckAddress(const int nCurrentHeight);
     bool CheckStrings();
 
 protected:
@@ -71,7 +71,7 @@ class CBudgetProposal
     // sets fValid and strInvalid, returns fValid
     bool UpdateValid(int nHeight, int mnCount);
     // Static checks that should be done only once - sets strInvalid
-    bool IsWellFormed(const CAmount& nTotalBudget);
+    bool IsWellFormed(const CAmount& nTotalBudget, const int nCurrentHeight);
     bool IsValid() const  { return fValid; }
     void SetStrInvalid(const std::string& _strInvalid) { strInvalid = _strInvalid; }
     std::string IsInvalidReason() const { return strInvalid; }

src/qt/pivx/governancemodel.cpp

@@ -245,7 +245,7 @@ OperationResult GovernanceModel::createProposal(const std::string& strProposalNa
 
     // Validate proposal
     CBudgetProposal proposal(strProposalName, strURL, nPaymentCount, scriptPubKey, nAmount, nBlockStart, UINT256_ZERO);
-    if (!proposal.IsWellFormed(g_budgetman.GetTotalBudget(proposal.GetBlockStart()))) {
+    if (!proposal.IsWellFormed(g_budgetman.GetTotalBudget(proposal.GetBlockStart()), clientModel->getNumBlocks())) {
         return {false, strprintf(_("Proposal is not valid %s"), proposal.IsInvalidReason())};
     }
 

src/rpc/budget.cpp

@@ -147,7 +147,7 @@ UniValue preparebudget(const JSONRPCRequest& request)
     // create transaction 15 minutes into the future, to allow for confirmation time
     CBudgetProposal proposal(strProposalName, strURL, nPaymentCount, scriptPubKey, nAmount, nBlockStart, UINT256_ZERO);
     const uint256& nHash = proposal.GetHash();
-    if (!proposal.IsWellFormed(g_budgetman.GetTotalBudget(proposal.GetBlockStart())))
+    if (!proposal.IsWellFormed(g_budgetman.GetTotalBudget(proposal.GetBlockStart()), GetChainTip()->nHeight))
         throw std::runtime_error("Proposal is not valid " + proposal.IsInvalidReason());
 
     CTransactionRef wtx;

test/functional/tiertwo_governance_sync_basic.py

@@ -59,7 +59,7 @@ def check_budget_finalization_sync(self, votesCount, status):
         for i in range(0, len(self.nodes)):
             node = self.nodes[i]
             budFin = node.mnfinalbudget("show")
-            assert_true(len(budFin) == 1, "MN budget finalization not synced in node" + str(i))
+            assert_true(len(budFin) == 1, "MN budget finalization not synced in node " + str(i))
             budget = budFin[next(iter(budFin))]
             assert_equal(budget["VoteCount"], votesCount)
             assert_equal(budget["Status"], status)
@@ -176,6 +176,15 @@ def submit_proposals(self, props):
             self.log.info("proposal %s broadcast successful!" % entry.name)
         return props
 
+    def create_multisig_addr(self, node):
+        addr1 = node.getnewaddress()
+        addr2 = node.getnewaddress()
+        addr3 = node.getnewaddress()
+        sig_address_1 = node.validateaddress(addr1)
+        sig_address_2 = node.validateaddress(addr2)
+        sig_address_3 = node.validateaddress(addr3)
+        return node.addmultisigaddress(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']])
+
     def run_test(self):
         self.enable_mocktime()
         self.setup_3_masternodes_network()
@@ -208,6 +217,18 @@ def run_test(self):
         )
         self.submit_proposals([firstProposal])
 
+        multisigAddr = self.create_multisig_addr(self.miner)
+        # Submit multisig proposal
+        self.log.info("preparing budget proposal..")
+        multisigProposal = Proposal(
+            "we-like-multisig",
+            "https://forum.pivx.org/t/test-multisig",
+            2,
+            multisigAddr,
+            600
+        )
+        self.submit_proposals([multisigProposal])
+
         # Create 15 more proposals to have a higher tier two net gossip movement
         props = []
         for i in range(15):
@@ -223,17 +244,21 @@ def run_test(self):
         self.stake(7, [self.remoteOne, self.remoteTwo])
         # Check proposals existence
         for i in range(self.num_nodes):
-            assert_equal(len(self.nodes[i].getbudgetinfo()), 16)
+            assert_equal(len(self.nodes[i].getbudgetinfo()), 17)
 
         # now let's vote for the proposal with the first MN
         self.log.info("Voting with MN1...")
         voteResult = self.ownerOne.mnbudgetvote("alias", firstProposal.proposalHash, "yes", self.masternodeOneAlias, True)
         assert_equal(voteResult["detail"][0]["result"], "success")
 
+        multisigVoteResult = self.ownerOne.mnbudgetvote("alias", multisigProposal.proposalHash, "yes", self.masternodeOneAlias, True)
+        assert_equal(multisigVoteResult["detail"][0]["result"], "success")
+
         # check that the vote was accepted everywhere
         self.stake(1, [self.remoteOne, self.remoteTwo])
         self.check_vote_existence(firstProposal.name, self.mnOneCollateral.hash, "YES", True)
-        self.log.info("all good, MN1 vote accepted everywhere!")
+        self.check_vote_existence(multisigProposal.name, self.mnOneCollateral.hash, "YES", True)
+        self.log.info("all good, MN1 votes accepted everywhere!")
 
         # before broadcast the second vote, let's drop the budget data of ownerOne.
         # so the node is forced to send a single proposal sync when the, now orphan, proposal vote is received.
@@ -247,11 +272,15 @@ def run_test(self):
         voteResult = self.ownerTwo.mnbudgetvote("alias", firstProposal.proposalHash, "yes", self.masternodeTwoAlias, True)
         assert_equal(voteResult["detail"][0]["result"], "success")
 
+        multisigVoteResult = self.ownerTwo.mnbudgetvote("alias", multisigProposal.proposalHash, "yes", self.masternodeTwoAlias, True)
+        assert_equal(multisigVoteResult["detail"][0]["result"], "success")
         # check orphan vote proposal re-sync
         self.log.info("checking orphan vote based proposal re-sync...")
         time.sleep(5) # wait a bit before check it
         self.check_proposal_existence(firstProposal.name, firstProposal.proposalHash)
         self.check_vote_existence(firstProposal.name, self.mnOneCollateral.hash, "YES", True)
+        self.check_proposal_existence(multisigProposal.name, multisigProposal.proposalHash)
+        self.check_vote_existence(multisigProposal.name, self.mnOneCollateral.hash, "YES", True)
         self.log.info("all good, orphan vote based proposal re-sync succeeded")
 
         # check that the vote was accepted everywhere
@@ -275,19 +304,28 @@ def run_test(self):
         TotalPayment = firstProposal.amountPerCycle * firstProposal.cycles
         Allotted = firstProposal.amountPerCycle
         RemainingPaymentCount = firstProposal.cycles
+
+        multisigBlockEnd = blockStart + multisigProposal.cycles * 145
+        multisigTotalPayment = multisigProposal.amountPerCycle * multisigProposal.cycles
+        multisigAllotted = multisigProposal.amountPerCycle
+        multisigRemainingPaymentCount = multisigProposal.cycles
         expected_budget = [
             self.get_proposal_obj(firstProposal.name, firstProposal.link, firstProposal.proposalHash, firstProposal.feeTxId, blockStart,
                                   blockEnd, firstProposal.cycles, RemainingPaymentCount, firstProposal.paymentAddr, 1,
                                   3, 0, 0, satoshi_round(TotalPayment), satoshi_round(firstProposal.amountPerCycle),
-                                  True, True, satoshi_round(Allotted), satoshi_round(Allotted))
+                                  True, True, satoshi_round(Allotted), satoshi_round(Allotted)),
+            self.get_proposal_obj(multisigProposal.name, multisigProposal.link, multisigProposal.proposalHash, multisigProposal.feeTxId, blockStart,
+                                  multisigBlockEnd, multisigProposal.cycles, multisigRemainingPaymentCount, multisigProposal.paymentAddr, 1,
+                                  2, 0, 0, satoshi_round(multisigTotalPayment), satoshi_round(multisigProposal.amountPerCycle),
+                                  True, True, satoshi_round(multisigAllotted), satoshi_round(multisigAllotted + Allotted)),
                            ]
         self.check_budgetprojection(expected_budget)
 
         # Quick block count check.
-        assert_equal(self.ownerOne.getblockcount(), 279)
+        assert_equal(self.ownerOne.getblockcount(), 282)
 
         self.log.info("starting budget finalization sync test..")
-        self.stake(2, [self.remoteOne, self.remoteTwo])
+        self.stake(1, [self.remoteOne, self.remoteTwo])
 
         # assert that there is no budget finalization first.
         assert_equal(len(self.ownerOne.mnfinalbudget("show")), 0)
@@ -337,10 +375,14 @@ def run_test(self):
         addrInfo = self.miner.listreceivedbyaddress(0, False, False, firstProposal.paymentAddr)
         assert_equal(addrInfo[0]["amount"], firstProposal.amountPerCycle)
 
+        addrInfo = self.miner.listreceivedbyaddress(0, False, False, multisigProposal.paymentAddr)
+        assert_equal(addrInfo[0]["amount"], multisigProposal.amountPerCycle)
+
         self.log.info("budget proposal paid!, all good")
 
         # Check that the proposal info returns updated payment count
         expected_budget[0]["RemainingPaymentCount"] -= 1
+        expected_budget[1]["RemainingPaymentCount"] -= 1
         self.check_budgetprojection(expected_budget)
 
         self.stake(1, [self.remoteOne, self.remoteTwo])
@@ -356,7 +398,7 @@ def run_test(self):
         self.wait_until_mnsync_finished()
         self.check_budgetprojection(expected_budget)
         for i in range(self.num_nodes):
-            assert_equal(len(self.nodes[i].getbudgetinfo()), 16)
+            assert_equal(len(self.nodes[i].getbudgetinfo()), 17)
 
         self.log.info("resync (1): budget data resynchronized successfully!")
 
@@ -377,7 +419,7 @@ def run_test(self):
         self.log.info("syncing node..")
         self.wait_until_mnsync_finished()
         for i in range(self.num_nodes):
-            assert_equal(len(self.nodes[i].getbudgetinfo()), 16)
+            assert_equal(len(self.nodes[i].getbudgetinfo()), 17)
         self.log.info("resync (2): budget data resynchronized successfully!")
 
         # Let's now verify the remote budget data relay.
@@ -392,7 +434,7 @@ def run_test(self):
         time.sleep(5) # wait a little bit
         self.log.info("Checking budget sync..")
         for i in range(self.num_nodes):
-            assert_equal(len(self.nodes[i].getbudgetinfo()), 16)
+            assert_equal(len(self.nodes[i].getbudgetinfo()), 17)
         self.check_vote_existence(firstProposal.name, self.mnOneCollateral.hash, "YES", True)
         self.check_vote_existence(firstProposal.name, self.mnTwoCollateral.hash, "YES", True)
         self.check_vote_existence(firstProposal.name, self.proRegTx1, "YES", True)
@@ -424,7 +466,7 @@ def run_test(self):
         assert_equal(len(self.miner.mnfinalbudget("show")), 1)
         blocks_to_mine = nextSuperBlockHeight + 200 - self.miner.getblockcount()
         self.log.info("Mining %d more blocks to check expired budget removal..." % blocks_to_mine)
-        self.stake(blocks_to_mine - 1, [self.remoteTwo])
+        self.stake(blocks_to_mine, [self.remoteTwo])
         # finalized budget must still be there
         self.miner.checkbudgets()
         assert_equal(len(self.miner.mnfinalbudget("show")), 1)