diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index b4f68ab6b7..9a8d4b6bad 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/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 &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,7 +628,7 @@ 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(); @@ -611,16 +636,24 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { 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::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,16 +732,41 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { ++start; } } + + if (request.params.size() > 4) { + aggregate = request.params[4].getBool(); + } + if (limit == 0) { limit = std::numeric_limits::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 map; + view.ForEachProposalVote( [&](const CProposalId &pId, uint8_t propCycle, const uint256 &id, CProposalVoteType vote) { - if (pId != propId) { + if (!propId.IsNull() && pId != propId) { return false; } @@ -731,6 +774,9 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { return false; } + if (aggregate && latestOnly && propCycle != view.GetProposal(pId)->cycle) + return true; + if (isMine) { auto node = view.GetMasternode(id); if (!node) { @@ -738,7 +784,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { } // skip entries until we reach start index - if (start != 0) { + if (!aggregate && start != 0) { --start; return true; } @@ -746,24 +792,46 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { 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; } diff --git a/test/functional/feature_on_chain_government.py b/test/functional/feature_on_chain_government.py index 50914ce234..678e01fcea 100755 --- a/test/functional/feature_on_chain_government.py +++ b/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() diff --git a/test/functional/feature_on_chain_government_voting_scenarios.py b/test/functional/feature_on_chain_government_voting_scenarios.py old mode 100755 new mode 100644 index 68f0358910..c93db1a7cc --- a/test/functional/feature_on_chain_government_voting_scenarios.py +++ b/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):