Display aggregate vote stats in listgovproposalvotes

src/masternodes/rpc_proposals.cpp

@@ -123,6 +123,24 @@ UniValue proposalVoteToJSON(const CProposalId &propId, uint8_t cycle, const uint
     return ret;
 }
 
+void proposalVoteAccounting(const CProposalVoteType &vote, uint256 propId, std::map<std::string, VotingInfo> &map) {
+    const auto voteString = CProposalVoteToString(vote);
+
+    if (map.find(propId.GetHex()) == map.end())
+        map.emplace(propId.GetHex(), VotingInfo{0, 0, 0, 0, 0, 0});
+
+    auto entry = &map.find(propId.GetHex())->second;
+
+    if (voteString == "YES")
+        ++entry->votesYes;
+    else if (voteString == "NEUTRAL")
+        ++entry->votesNeutral;
+    else if (voteString == "NO")
+        ++entry->votesNo;
+
+    ++entry->votesPresent;
+}
+
 /*
  *  Issued by: any
  */
@@ -568,7 +586,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) {
         "listgovproposalvotes",
         "\nReturns information about proposal votes.\n",
         {
-          {"proposalId", RPCArg::Type::STR, RPCArg::Optional::NO, "The proposal id)"},
+          {"proposalId", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The proposal id)"},
           {"masternode", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "mine/all/id (default = mine)"},
           {"cycle",
              RPCArg::Type::NUM,
@@ -594,7 +612,14 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) {
                      RPCArg::Optional::OMITTED,
                      "Maximum number of votes to return, 100 by default"},
                 },
-            }, },
+            },
+            {
+                "aggregate",
+                RPCArg::Type::BOOL,
+                RPCArg::Optional::OMITTED,
+                "0: return raw vote data, 1: return total votes by type"
+            }
+          },
         RPCResult{"{id:{...},...}     (array) Json object with proposal vote information\n"},
         RPCExamples{HelpExampleCli("listgovproposalvotes", "txid") + HelpExampleRpc("listgovproposalvotes", "txid")},
     }
@@ -603,24 +628,32 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) {
     UniValue optionsObj(UniValue::VOBJ);
 
     if (!request.params[0].isObject() && !optionsObj.read(request.params[0].getValStr()))
-        RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VSTR, UniValue::VNUM, UniValue::VOBJ}, true);
+        RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VSTR, UniValue::VNUM, UniValue::VOBJ, UniValue::VBOOL}, true);
     else if (request.params[0].isObject())
         optionsObj = request.params[0].get_obj();
 
     CCustomCSView view(*pcustomcsview);
 
     uint256 mnId;
     uint256 propId;
-    bool isMine = true;
+    bool isMine = false;
     uint8_t cycle{1};
     int8_t inputCycle{0};
+    bool aggregate = true;
+    bool latestOnly = true;
 
     size_t limit         = 100;
     size_t start         = 0;
     bool including_start = true;
 
     if (!optionsObj.empty()) {
-        propId          = ParseHashV(optionsObj["proposalId"].get_str(), "proposalId");
+
+        if (!optionsObj["proposalId"].isNull()) {
+            propId = ParseHashV(optionsObj["proposalId"].get_str(), "proposalId");
+            aggregate = false;
+            isMine = true;
+            latestOnly = false;
+        }
 
         if (!optionsObj["masternode"].isNull()) {
             if (optionsObj["masternode"].get_str() == "all") {
@@ -633,21 +666,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) {
 
         if (!optionsObj["cycle"].isNull()) {
             inputCycle = optionsObj["cycle"].get_int();
-        }
-
-        if (inputCycle == 0) {
-            auto prop = view.GetProposal(propId);
-            if (!prop) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER,
-                                   strprintf("Proposal <%s> does not exist", propId.GetHex()));
-            }
-            cycle = prop->cycle;
-        } else if (inputCycle > 0) {
-            cycle = inputCycle;
-        } else if (inputCycle == -1) {
-            cycle = 1;
-        } else {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Incorrect cycle value");
+            latestOnly = false;
         }
 
         if (!optionsObj["pagination"].isNull()) {
@@ -666,8 +685,21 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) {
                 ++start;
             }
         }
+
+        if (!optionsObj["aggregate"].isNull()) {
+            aggregate = optionsObj["aggregate"].getBool();
+        }
+
+        if (limit == 0) {
+            limit = std::numeric_limits<decltype(limit)>::max();
+        }
     } else {
-        propId = ParseHashV(request.params[0].get_str(), "proposalId");
+        if (!request.params.empty()) {
+            propId = ParseHashV(request.params[0].get_str(), "proposalId");
+            aggregate = false;
+            isMine = true;
+            latestOnly = false;
+        }
 
         if (request.params.size() > 1) {
             auto str = request.params[1].get_str();
@@ -681,21 +713,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) {
 
         if (request.params.size() > 2) {
             inputCycle = request.params[2].get_int();
-        }
-
-        if (inputCycle == 0) {
-            auto prop = view.GetProposal(propId);
-            if (!prop) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER,
-                                   strprintf("Proposal <%s> does not exist", propId.GetHex()));
-            }
-            cycle = prop->cycle;
-        } else if (inputCycle > 0) {
-            cycle = inputCycle;
-        } else if (inputCycle == -1) {
-            cycle = 1;
-        } else {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Incorrect cycle value");
+            latestOnly = false;
         }
 
         if (request.params.size() > 3) {
@@ -714,56 +732,106 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) {
                 ++start;
             }
         }
+
+        if (request.params.size() > 4) {
+            aggregate = request.params[4].getBool();
+        }
+
         if (limit == 0) {
             limit = std::numeric_limits<decltype(limit)>::max();
         }
     }
 
+    if (inputCycle == 0) {
+        if (!propId.IsNull()) {
+            auto prop = view.GetProposal(propId);
+            if (!prop) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Proposal <%s> does not exist", propId.GetHex()));
+            }
+            cycle = prop->cycle;
+        } else {
+            inputCycle = -1;
+        }
+    } else if (inputCycle > 0) {
+        cycle = inputCycle;
+    } else if (inputCycle == -1) {
+        cycle = 1;
+    } else {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Incorrect cycle value");
+    }
+
     UniValue ret(UniValue::VARR);
 
+    std::map<std::string, VotingInfo> map;
+
     view.ForEachProposalVote(
         [&](const CProposalId &pId, uint8_t propCycle, const uint256 &id, CProposalVoteType vote) {
-            if (pId != propId) {
+            if (!propId.IsNull() && pId != propId) {
                 return false;
             }
 
             if (inputCycle != -1 && cycle != propCycle) {
                 return false;
             }
 
+            if (aggregate && latestOnly && propCycle != view.GetProposal(pId)->cycle)
+                return true;
+
             if (isMine) {
                 auto node = view.GetMasternode(id);
                 if (!node) {
                     return true;
                 }
 
                 // skip entries until we reach start index
-                if (start != 0) {
+                if (!aggregate && start != 0) {
                     --start;
                     return true;
                 }
 
                 auto ownerDest = node->ownerType == 1 ? CTxDestination(PKHash(node->ownerAuthAddress))
                                                       : CTxDestination(WitnessV0KeyHash(node->ownerAuthAddress));
                 if (::IsMineCached(*pwallet, GetScriptForDestination(ownerDest))) {
-                    ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote));
-                    limit--;
+                    if (!aggregate) {
+                        ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote));
+                        limit--;
+                    } else {
+                        proposalVoteAccounting(vote, pId, map);
+                    }
                 }
             } else if (mnId.IsNull() || mnId == id) {
                 // skip entries until we reach start index
-                if (start != 0) {
+                if (!aggregate && start != 0) {
                     --start;
                     return true;
                 }
 
-                ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote));
-                limit--;
+                if (!aggregate) {
+                    ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote));
+                    limit--;
+                } else {
+                    proposalVoteAccounting(vote, pId, map);
+                }
             }
 
             return limit != 0;
         },
         CMnVotePerCycle{propId, cycle, mnId});
 
+    if(aggregate) {
+        for (const auto& entry : map) {
+            UniValue stats(UniValue::VOBJ);
+
+            stats.pushKV("proposalId", entry.first);
+            stats.pushKV("total", entry.second.votesPresent);
+            stats.pushKV("yes", entry.second.votesYes);
+            stats.pushKV("neutral", entry.second.votesNeutral);
+            stats.pushKV("no", entry.second.votesNo);
+
+            ret.push_back(stats);
+        }
+    }
+
     return ret;
 }
 

test/functional/feature_on_chain_government.py

@@ -16,6 +16,7 @@
 
 class OnChainGovernanceTest(DefiTestFramework):
     mns = None
+    proposalId = ""
 
     def set_test_params(self):
         self.num_nodes = 4
@@ -691,15 +692,34 @@ def run_test(self):
                 # otherwise tx1 is the last proposal
                 break
 
-        assert_equal(self.nodes[0].listgovproposals({"status": "voting", "pagination": {"start": tx1, "including_start": False, "limit": 1}}), nextProposal)
+        assert_equal(self.nodes[0].listgovproposals(
+            {"status": "voting", "pagination": {"start": tx1, "including_start": False, "limit": 1}}), nextProposal)
 
+        self.test_aggregation(propId)
         self.test_default_cycles_fix()
+        self.aggregate_all_votes()
+
+    def test_aggregation(self, propId):
+        """
+        Tests vote aggregation for a specific proposal. It should respect all provided filters.
+        """
+        votes = self.nodes[0].listgovproposalvotes(propId, 'all', -1, {})
+        totalVotes = len(votes)
+        yesVotes = len([x for x in votes if x["vote"] == "YES"])
+        noVotes = len([x for x in votes if x["vote"] == "NO"])
+        neutralVotes = len([x for x in votes if x["vote"] == "NEUTRAL"])
+
+        votes_aggregate = self.nodes[0].listgovproposalvotes(propId, 'all', -1, {}, True)[0]
+        assert_equal(votes_aggregate["proposalId"], propId)
+        assert_equal(votes_aggregate["total"], totalVotes)
+        assert_equal(votes_aggregate["yes"], yesVotes)
+        assert_equal(votes_aggregate["neutral"], neutralVotes)
+        assert_equal(votes_aggregate["no"], noVotes)
 
     def test_default_cycles_fix(self):
         """
         Tests fix for an issue for when the cycles argument is not provided, the
         votes for cycle 1 are returned instead of the latest cycle.
-
         https://github.com/DeFiCh/ain/pull/1701
         """
         tx1 = self.nodes[0].creategovcfp({"title": "1111",
@@ -711,16 +731,16 @@ def test_default_cycles_fix(self):
         self.sync_blocks()
 
         endHeight = self.nodes[0].getgovproposal(tx1)["cycleEndHeight"]
-        proposalId = self.nodes[0].getgovproposal(tx1)["proposalId"]
+        self.proposalId = self.nodes[0].getgovproposal(tx1)["proposalId"]
 
         # cycle 1 votes
         for mn in range(len(self.mns)):
-            self.nodes[mn].votegov(proposalId, self.mns[mn], "yes")
+            self.nodes[mn].votegov(self.proposalId, self.mns[mn], "yes")
             self.nodes[mn].generate(1)
             self.sync_blocks()
 
         # should show cycle 1 votes
-        votes = self.nodes[0].listgovproposalvotes(proposalId, 'all')
+        votes = self.nodes[0].listgovproposalvotes(self.proposalId, 'all')
         for vote in votes:
             assert_equal(vote["vote"], "YES")  # there are only YES votes in cycle 1
 
@@ -729,15 +749,34 @@ def test_default_cycles_fix(self):
 
         # cycle 2 votes
         for mn in range(len(self.mns)):
-            self.nodes[mn].votegov(proposalId, self.mns[mn], "no")
+            self.nodes[mn].votegov(self.proposalId, self.mns[mn], "no")
             self.nodes[mn].generate(1)
             self.sync_blocks()
 
-        votes = self.nodes[0].listgovproposalvotes(proposalId, 'all')
+        votes = self.nodes[0].listgovproposalvotes(self.proposalId, 'all')
         for vote in votes:
             # there are only NO votes in cycle 2, this should fail if cycle defaults to 1
             assert_equal(vote["vote"], "NO")
 
+    def aggregate_all_votes(self):
+        """
+        Tests aggregation of all latest cycle votes for all proposals
+        when no arguments are provided in listgovproposalvotes.
+        """
+        votes = self.nodes[0].listgovproposalvotes()
+        proposalVotes = self.nodes[0].listgovproposalvotes(self.proposalId, "all", 0, {}, True)
+        filteredVotes = list(filter(lambda vote: vote["proposalId"] == self.proposalId, votes))
+        assert_equal(filteredVotes, proposalVotes)
+
+        props = self.nodes[0].listgovproposals()
+        missing = []
+        for prop in props:
+            if prop["proposalId"] not in [x["proposalId"] for x in votes]:
+                missing.append(prop["proposalId"])
+
+        for miss in missing:
+            # proposals missing from entry must have 0 votes in the latest cycle
+            assert_equal(len(self.nodes[0].listgovproposalvotes(miss, "all", 0)), 0)
 
 if __name__ == '__main__':
     OnChainGovernanceTest().main()

test/functional/feature_on_chain_government_voting_scenarios.py

@@ -11,9 +11,9 @@
     assert_raises_rpc_error
 )
 
-APPROVAL_THRESHOLD=50
-QUORUM=50
-VOTING_PERIOD=10
+APPROVAL_THRESHOLD = 50
+QUORUM = 50
+VOTING_PERIOD = 10
 
 class OCGVotingScenarionTest(DefiTestFramework):
     def set_test_params(self):