Fix for Payment Request reorganizations

qa/rpc-tests/cfund-paymentrequest-extract-funds.py

@@ -47,15 +47,21 @@ def run_test(self):
 
         # Create 6 payment requests
         paymentRequests = []
-        for x in range(6):
+        for x in range(5):
             paymentRequests.append(self.nodes[0].createpaymentrequest(proposalid0, 20, "test0")["hash"])
 
+	# The last 1 should be rejected as it excedes proposal balance
+        try:
+            paymentRequests.append(self.nodes[0].createpaymentrequest(proposalid0, 20, "test0")["hash"])
+            raise ValueError("Error should be thrown for invalid prequest")
+        except JSONRPCException as e:
+            assert("The transaction was rejected" in e.error['message'])
+
         slow_gen(self.nodes[0], 1)
 
         # One of them should have been rejected at creation
         valid = 0
         invalid = 0
-        invalid_pos = -1
 
         for paymentReq in paymentRequests:
             try:
@@ -64,13 +70,10 @@ def run_test(self):
                 valid = valid + 1
             except JSONRPCException:
                 invalid = invalid + 1
-                invalid_pos = valid 
                 continue
 
         assert(valid == 5)
-        assert(invalid == 1)
-
-        paymentRequests.pop(invalid_pos)
+        assert(invalid == 0)
 
         assert(self.nodes[0].cfundstats()["funds"]["locked"] == locked_accepted)
 

qa/rpc-tests/cfund-rawtx-paymentrequest-create.py

@@ -111,8 +111,16 @@ def run_test(self):
 
         # Create multiple payment requests
         self.send_raw_paymentrequest(6, self.goodAddress, self.goodProposalHash, "sum_to_too_large_amount0")
-        self.send_raw_paymentrequest(6, self.goodAddress, self.goodProposalHash, "sum_to_too_large_amount1")
-        self.send_raw_paymentrequest(6, self.goodAddress, self.goodProposalHash, "sum_to_too_large_amount2")
+        try:
+            self.send_raw_paymentrequest(6, self.goodAddress, self.goodProposalHash, "sum_to_too_large_amount1")
+            raise ValueError("Error should be thrown for invalid rawtx (prequest)")
+        except JSONRPCException as e:
+            assert("bad-cfund-payment-request" in e.error['message'])
+        try:
+            self.send_raw_paymentrequest(6, self.goodAddress, self.goodProposalHash, "sum_to_too_large_amount2")
+            raise ValueError("Error should be thrown for invalid rawtx (prequest)")
+        except JSONRPCException as e:
+            assert("bad-cfund-payment-request" in e.error['message'])
 
         slow_gen(self.nodes[0], 1)
 

src/consensus/cfund.cpp

@@ -188,7 +188,7 @@ bool CFund::RemoveVotePaymentRequest(uint256 proposalHash)
     return RemoveVotePaymentRequest(proposalHash.ToString());
 }
 
-bool CFund::IsValidPaymentRequest(CTransaction tx, int nMaxVersion)
+bool CFund::IsValidPaymentRequest(CTransaction tx, CCoinsViewCache& coins, int nMaxVersion)
 {    
     if(tx.strDZeel.length() > 1024)
         return error("%s: Too long strdzeel for payment request %s", __func__, tx.GetHash().ToString());
@@ -258,9 +258,9 @@ bool CFund::IsValidPaymentRequest(CTransaction tx, int nMaxVersion)
     if (!pubkey.RecoverCompact(ss.GetHash(), vchSig) || pubkey.GetID() != keyID)
         return error("%s: Invalid signature for payment request %s", __func__, tx.GetHash().ToString());
 
-    if(nAmount > proposal.GetAvailable(true))
+    if(nAmount > proposal.GetAvailable(coins, true))
         return error("%s: Invalid requested amount for payment request %s (%d vs %d available)",
-                     __func__, tx.GetHash().ToString(), nAmount, proposal.GetAvailable());
+                     __func__, tx.GetHash().ToString(), nAmount, proposal.GetAvailable(coins, true));
 
     bool ret = (nVersion <= nMaxVersion);
 
@@ -271,11 +271,23 @@ bool CFund::IsValidPaymentRequest(CTransaction tx, int nMaxVersion)
 
 }
 
-bool CFund::CPaymentRequest::CanVote() const {
+bool CFund::CPaymentRequest::CanVote(CCoinsViewCache& coins) const
+{
+    AssertLockHeld(cs_main);
+
+    CBlockIndex* pindex;
+    if(txblockhash == uint256() || !mapBlockIndex.count(txblockhash))
+        return false;
+
+    pindex = mapBlockIndex[txblockhash];
+    if(!chainActive.Contains(pindex))
+        return false;
+
     CFund::CProposal proposal;
     if(!CFund::FindProposal(proposalhash, proposal))
         return false;
-    return nAmount <= proposal.GetAvailable() && fState != ACCEPTED && fState != REJECTED && fState != EXPIRED && !ExceededMaxVotingCycles();
+
+    return nAmount <= proposal.GetAvailable(coins) && fState != ACCEPTED && fState != REJECTED && fState != EXPIRED && !ExceededMaxVotingCycles();
 }
 
 bool CFund::CPaymentRequest::IsExpired() const {
@@ -388,6 +400,16 @@ bool CFund::CProposal::IsRejected() const {
 }
 
 bool CFund::CProposal::CanVote() const {
+    AssertLockHeld(cs_main);
+
+    CBlockIndex* pindex;
+    if(txblockhash == uint256() || !mapBlockIndex.count(txblockhash))
+        return false;
+
+    pindex = mapBlockIndex[txblockhash];
+    if(!chainActive.Contains(pindex))
+        return false;
+
     return (fState == NIL) && (!ExceededMaxVotingCycles());
 }
 
@@ -418,6 +440,57 @@ bool CFund::CProposal::ExceededMaxVotingCycles() const {
     return nVotingCycle > Params().GetConsensus().nCyclesProposalVoting;
 }
 
+CAmount CFund::CProposal::GetAvailable(CCoinsViewCache& coins, bool fIncludeRequests) const
+{
+    AssertLockHeld(cs_main);
+
+    CAmount initial = nAmount;
+    for (unsigned int i = 0; i < vPayments.size(); i++)
+    {
+        CFund::CPaymentRequest prequest;
+        if(FindPaymentRequest(vPayments[i], prequest))
+        {
+            if (!coins.HaveCoins(prequest.hash))
+            {
+                CBlockIndex* pindex;
+                if(prequest.txblockhash == uint256() || !mapBlockIndex.count(prequest.txblockhash))
+                    continue;
+                pindex = mapBlockIndex[prequest.txblockhash];
+                if(!chainActive.Contains(pindex))
+                    continue;
+            }
+            if((fIncludeRequests && prequest.fState != REJECTED && prequest.fState != EXPIRED) || (!fIncludeRequests && prequest.fState == ACCEPTED))
+                initial -= prequest.nAmount;
+        }
+    }
+    return initial;
+}
+
+std::string CFund::CProposal::ToString(CCoinsViewCache& coins, uint32_t currentTime) const {
+    std::string str;
+    str += strprintf("CProposal(hash=%s, nVersion=%i, nAmount=%f, available=%f, nFee=%f, address=%s, nDeadline=%u, nVotesYes=%u, "
+                     "nVotesNo=%u, nVotingCycle=%u, fState=%s, strDZeel=%s, blockhash=%s)",
+                     hash.ToString(), nVersion, (float)nAmount/COIN, (float)GetAvailable(coins)/COIN, (float)nFee/COIN, Address, nDeadline,
+                     nVotesYes, nVotesNo, nVotingCycle, GetState(currentTime), strDZeel, blockhash.ToString().substr(0,10));
+    for (unsigned int i = 0; i < vPayments.size(); i++) {
+        CFund::CPaymentRequest prequest;
+        if(FindPaymentRequest(vPayments[i], prequest))
+            str += "\n    " + prequest.ToString();
+    }
+    return str + "\n";
+}
+
+bool CFund::CProposal::HasPendingPaymentRequests(CCoinsViewCache& coins) const {
+    for (unsigned int i = 0; i < vPayments.size(); i++)
+    {
+        CFund::CPaymentRequest prequest;
+        if(FindPaymentRequest(vPayments[i], prequest))
+            if(prequest.CanVote(coins))
+                return true;
+    }
+    return false;
+}
+
 std::string CFund::CProposal::GetState(uint32_t currentTime) const {
     std::string sFlags = "pending";
     if(IsAccepted()) {
@@ -444,13 +517,15 @@ std::string CFund::CProposal::GetState(uint32_t currentTime) const {
     return sFlags;
 }
 
-void CFund::CProposal::ToJson(UniValue& ret) const {
+void CFund::CProposal::ToJson(UniValue& ret, CCoinsViewCache& coins) const {
+    AssertLockHeld(cs_main);
+
     ret.push_back(Pair("version", nVersion));
     ret.push_back(Pair("hash", hash.ToString()));
     ret.push_back(Pair("blockHash", txblockhash.ToString()));
     ret.push_back(Pair("description", strDZeel));
     ret.push_back(Pair("requestedAmount", FormatMoney(nAmount)));
-    ret.push_back(Pair("notPaidYet", FormatMoney(GetAvailable())));
+    ret.push_back(Pair("notPaidYet", FormatMoney(GetAvailable(coins))));
     ret.push_back(Pair("userPaidFee", FormatMoney(nFee)));
     ret.push_back(Pair("paymentAddress", Address));
     if(nVersion >= 2) {

src/consensus/cfund.h

@@ -15,6 +15,7 @@
 using namespace std;
 
 class CTransaction;
+class CCoinsViewCache;
 
 extern std::vector<std::pair<std::string, bool>> vAddedProposalVotes;
 extern std::vector<std::pair<std::string, bool>> vAddedPaymentRequestVotes;
@@ -48,7 +49,7 @@ bool VotePaymentRequest(string strProp, bool vote, bool &duplicate);
 bool VotePaymentRequest(uint256 proposalHash, bool vote, bool &duplicate);
 bool RemoveVotePaymentRequest(string strProp);
 bool RemoveVotePaymentRequest(uint256 proposalHash);
-bool IsValidPaymentRequest(CTransaction tx, int nMaxVersion);
+bool IsValidPaymentRequest(CTransaction tx, CCoinsViewCache& coins, int nMaxVersion);
 bool IsValidProposal(CTransaction tx, int nMaxVersion);
 
 class CPaymentRequest
@@ -125,7 +126,7 @@ class CPaymentRequest
 
     bool ExceededMaxVotingCycles() const;
 
-    bool CanVote() const;
+    bool CanVote(CCoinsViewCache& coins) const;
 
     ADD_SERIALIZE_METHODS;
 
@@ -212,23 +213,10 @@ class CProposal
                 && nVotesNo == 0 && nDeadline == 0 && strDZeel == "");
     }
 
-    std::string ToString(uint32_t currentTime = 0) const {
-        std::string str;
-        str += strprintf("CProposal(hash=%s, nVersion=%i, nAmount=%f, available=%f, nFee=%f, address=%s, nDeadline=%u, nVotesYes=%u, "
-                         "nVotesNo=%u, nVotingCycle=%u, fState=%s, strDZeel=%s, blockhash=%s)",
-                         hash.ToString(), nVersion, (float)nAmount/COIN, (float)GetAvailable()/COIN, (float)nFee/COIN, Address, nDeadline,
-                         nVotesYes, nVotesNo, nVotingCycle, GetState(currentTime), strDZeel, blockhash.ToString().substr(0,10));
-        for (unsigned int i = 0; i < vPayments.size(); i++) {
-            CFund::CPaymentRequest prequest;
-            if(FindPaymentRequest(vPayments[i], prequest))
-                str += "\n    " + prequest.ToString();
-        }
-        return str + "\n";
-    }
-
+    std::string ToString(CCoinsViewCache& coins, uint32_t currentTime = 0) const;
     std::string GetState(uint32_t currentTime) const;
 
-    void ToJson(UniValue& ret) const;
+    void ToJson(UniValue& ret, CCoinsViewCache& coins) const;
 
     bool IsAccepted() const;
 
@@ -246,29 +234,9 @@ class CProposal
         return fState == ACCEPTED;
     }
 
-    bool HasPendingPaymentRequests() const {
-        for (unsigned int i = 0; i < vPayments.size(); i++)
-        {
-            CFund::CPaymentRequest prequest;
-            if(FindPaymentRequest(vPayments[i], prequest))
-                if(prequest.CanVote())
-                    return true;
-        }
-        return false;
-    }
+    bool HasPendingPaymentRequests(CCoinsViewCache& coins) const;
 
-    CAmount GetAvailable(bool fIncludeRequests = false) const
-    {
-        CAmount initial = nAmount;
-        for (unsigned int i = 0; i < vPayments.size(); i++)
-        {
-            CFund::CPaymentRequest prequest;
-            if(FindPaymentRequest(vPayments[i], prequest))
-                if((fIncludeRequests && prequest.fState != REJECTED) || (!fIncludeRequests && prequest.fState == ACCEPTED))
-                    initial -= prequest.nAmount;
-        }
-        return initial;
-    }
+    CAmount GetAvailable(CCoinsViewCache& coins, bool fIncludeRequests = false) const;
 
     ADD_SERIALIZE_METHODS;