From 9fa408b72f63a6bf2a9640a679ba86d81984c19e Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Mon, 30 Jan 2023 12:09:57 +0800 Subject: [PATCH 01/10] Add aggregate stats support to listgovproposalvotes --- src/masternodes/rpc_proposals.cpp | 91 +++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 11 deletions(-) diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index be3913df09..03eb2317b4 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/src/masternodes/rpc_proposals.cpp @@ -107,6 +107,26 @@ UniValue proposalVoteToJSON(const CProposalId &propId, uint8_t cycle, const uint return ret; } +void proposalVoteAccounting(const CProposalVoteType &vote, + int &totalVotes, + int &yesVotes, + int &neutralVotes, + int &noVotes, + int &unknownVotes) { + const auto voteString = CProposalVoteToString(vote); + + if (voteString == "YES") + ++yesVotes; + else if (voteString == "NEUTRAL") + ++neutralVotes; + else if (voteString == "NO") + ++noVotes; + else + ++unknownVotes; + + ++totalVotes; +} + /* * Issued by: any */ @@ -525,7 +545,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, @@ -551,7 +571,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")}, } @@ -569,6 +596,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { bool isMine = true; uint8_t cycle{1}; int8_t inputCycle{0}; + bool aggregate = true; size_t limit = 100; size_t start = 0; @@ -576,7 +604,11 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { if (request.params[0].isObject()) { auto optionsObj = request.params[0].get_obj(); - propId = ParseHashV(optionsObj["proposalId"].get_str(), "proposalId"); + + if (!optionsObj["proposalId"].isNull()) { + propId = ParseHashV(optionsObj["proposalId"].get_str(), "proposalId"); + aggregate = false; + } if (!optionsObj["masternode"].isNull()) { if (optionsObj["masternode"].get_str() == "all") { @@ -621,8 +653,19 @@ 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; + } if (request.params.size() > 1) { auto str = request.params[1].get_str(); @@ -669,6 +712,11 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { ++start; } } + + if (request.params.size() > 4) { + aggregate = request.params[4].getBool(); + } + if (limit == 0) { limit = std::numeric_limits::max(); } @@ -676,9 +724,11 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { UniValue ret(UniValue::VARR); + int totalVotes{0}, yesVotes{0}, neutralVotes{0}, noVotes{0}, unknownVotes{0}; + view.ForEachProposalVote( [&](const CProposalId &pId, uint8_t propCycle, const uint256 &id, CProposalVoteType vote) { - if (pId != propId) { + if (!propId.IsNull() && pId != propId) { return false; } @@ -693,7 +743,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { } // skip entries until we reach start index - if (start != 0) { + if (!aggregate && start != 0) { --start; return true; } @@ -701,24 +751,43 @@ 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, totalVotes, yesVotes, neutralVotes, noVotes, unknownVotes); + } } } 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, totalVotes, yesVotes, neutralVotes, noVotes, unknownVotes); + } } return limit != 0; }, CMnVotePerCycle{propId, cycle, mnId}); + if(aggregate) { + UniValue stats(UniValue::VOBJ); + stats.pushKV("totalVotes", totalVotes); + stats.pushKV("yesVotes", yesVotes); + stats.pushKV("neutralVotes", neutralVotes); + stats.pushKV("noVotes", noVotes); + stats.pushKV("unknownVotes", unknownVotes); + + ret.push_back(stats); + } + return ret; } From 8b072df6f56afcee71a39d9950d0f6f7fe1d17eb Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Wed, 1 Feb 2023 14:45:48 +0800 Subject: [PATCH 02/10] Fix output schema --- src/masternodes/rpc_proposals.cpp | 57 ++++++++++++------- .../functional/feature_on_chain_government.py | 21 ++++++- 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index 03eb2317b4..0adf4e687c 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/src/masternodes/rpc_proposals.cpp @@ -107,24 +107,33 @@ UniValue proposalVoteToJSON(const CProposalId &propId, uint8_t cycle, const uint return ret; } -void proposalVoteAccounting(const CProposalVoteType &vote, - int &totalVotes, - int &yesVotes, - int &neutralVotes, - int &noVotes, - int &unknownVotes) { +struct VoteAccounting { + int totalVotes; + int yesVotes; + int neutralVotes; + int noVotes; + int unknownVotes; +}; + +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(), VoteAccounting{0,0,0,0,0}); + + auto entry = &map.find(propId.GetHex())->second; + + if (voteString == "YES") - ++yesVotes; + ++entry->yesVotes; else if (voteString == "NEUTRAL") - ++neutralVotes; + ++entry->neutralVotes; else if (voteString == "NO") - ++noVotes; + ++entry->noVotes; else - ++unknownVotes; + ++entry->unknownVotes; - ++totalVotes; + ++entry->totalVotes; } /* @@ -724,7 +733,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { UniValue ret(UniValue::VARR); - int totalVotes{0}, yesVotes{0}, neutralVotes{0}, noVotes{0}, unknownVotes{0}; + std::map map; view.ForEachProposalVote( [&](const CProposalId &pId, uint8_t propCycle, const uint256 &id, CProposalVoteType vote) { @@ -755,7 +764,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote)); limit--; } else { - proposalVoteAccounting(vote, totalVotes, yesVotes, neutralVotes, noVotes, unknownVotes); + proposalVoteAccounting(vote, propId, map); } } } else if (mnId.IsNull() || mnId == id) { @@ -769,7 +778,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote)); limit--; } else { - proposalVoteAccounting(vote, totalVotes, yesVotes, neutralVotes, noVotes, unknownVotes); + proposalVoteAccounting(vote, propId, map); } } @@ -778,14 +787,18 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { CMnVotePerCycle{propId, cycle, mnId}); if(aggregate) { - UniValue stats(UniValue::VOBJ); - stats.pushKV("totalVotes", totalVotes); - stats.pushKV("yesVotes", yesVotes); - stats.pushKV("neutralVotes", neutralVotes); - stats.pushKV("noVotes", noVotes); - stats.pushKV("unknownVotes", unknownVotes); - - ret.push_back(stats); + for (const auto& entry : map) { + UniValue stats(UniValue::VOBJ); + + stats.pushKV("proposalId", entry.first); + stats.pushKV("total", entry.second.totalVotes); + stats.pushKV("yes", entry.second.yesVotes); + stats.pushKV("neutral", entry.second.neutralVotes); + stats.pushKV("no", entry.second.noVotes); + stats.pushKV("unknown", entry.second.unknownVotes); + + 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 2b0262fb1b..3b596dbf68 100755 --- a/test/functional/feature_on_chain_government.py +++ b/test/functional/feature_on_chain_government.py @@ -676,7 +676,26 @@ 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) + + # test listgovproposalvotes aggregation + 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"]) + unknownVotes = totalVotes - yesVotes - noVotes - neutralVotes + + 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) + assert_equal(votes_aggregate["unknown"], unknownVotes) + + print(self.nodes[0].listgovproposalvotes()) if __name__ == '__main__': OnChainGovernanceTest().main() From 7246e04441eacc970d79cdfd5f40d33c8d399c11 Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Wed, 1 Feb 2023 14:51:04 +0800 Subject: [PATCH 03/10] Fix multiple proposal output when aggregating --- src/masternodes/rpc_proposals.cpp | 4 ++-- test/functional/feature_on_chain_government.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index 0adf4e687c..9961b2e3bf 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/src/masternodes/rpc_proposals.cpp @@ -764,7 +764,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote)); limit--; } else { - proposalVoteAccounting(vote, propId, map); + proposalVoteAccounting(vote, pId, map); } } } else if (mnId.IsNull() || mnId == id) { @@ -778,7 +778,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { ret.push_back(proposalVoteToJSON(propId, propCycle, id, vote)); limit--; } else { - proposalVoteAccounting(vote, propId, map); + proposalVoteAccounting(vote, pId, map); } } diff --git a/test/functional/feature_on_chain_government.py b/test/functional/feature_on_chain_government.py index 3b596dbf68..87eb7ac76c 100755 --- a/test/functional/feature_on_chain_government.py +++ b/test/functional/feature_on_chain_government.py @@ -695,7 +695,7 @@ def run_test(self): assert_equal(votes_aggregate["no"], noVotes) assert_equal(votes_aggregate["unknown"], unknownVotes) - print(self.nodes[0].listgovproposalvotes()) + assert_equal(len(self.nodes[0].listgovproposalvotes()), 3) if __name__ == '__main__': OnChainGovernanceTest().main() From 44d0a02687cf4112c19cb0e0fb3ae25c1d03d039 Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Wed, 1 Feb 2023 15:08:29 +0800 Subject: [PATCH 04/10] Fix isMine default --- src/masternodes/rpc_proposals.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index 9961b2e3bf..3da188dff5 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/src/masternodes/rpc_proposals.cpp @@ -596,13 +596,13 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { if (request.params[0].isObject()) RPCTypeCheck(request.params, {UniValue::VOBJ}, true); else - 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); CCustomCSView view(*pcustomcsview); uint256 mnId; uint256 propId; - bool isMine = true; + bool isMine = false; uint8_t cycle{1}; int8_t inputCycle{0}; bool aggregate = true; @@ -617,6 +617,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { if (!optionsObj["proposalId"].isNull()) { propId = ParseHashV(optionsObj["proposalId"].get_str(), "proposalId"); aggregate = false; + isMine = true; } if (!optionsObj["masternode"].isNull()) { @@ -674,6 +675,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { if (!request.params.empty()) { propId = ParseHashV(request.params[0].get_str(), "proposalId"); aggregate = false; + isMine = true; } if (request.params.size() > 1) { From e47befd2bf0c77765075118855f09fc84cad0398 Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Thu, 2 Feb 2023 14:25:13 +0800 Subject: [PATCH 05/10] Default cycles fix --- src/masternodes/rpc_proposals.cpp | 58 ++++++++-------- .../functional/feature_on_chain_government.py | 69 ++++++++++++++++++- 2 files changed, 95 insertions(+), 32 deletions(-) diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index 3da188dff5..9644ab6c0c 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/src/masternodes/rpc_proposals.cpp @@ -119,11 +119,10 @@ void proposalVoteAccounting(const CProposalVoteType &vote, uint256 propId, std:: const auto voteString = CProposalVoteToString(vote); if (map.find(propId.GetHex()) == map.end()) - map.emplace(propId.GetHex(), VoteAccounting{0,0,0,0,0}); + map.emplace(propId.GetHex(), VoteAccounting{0, 0, 0, 0, 0}); auto entry = &map.find(propId.GetHex())->second; - if (voteString == "YES") ++entry->yesVotes; else if (voteString == "NEUTRAL") @@ -606,6 +605,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { uint8_t cycle{1}; int8_t inputCycle{0}; bool aggregate = true; + bool latestOnly = true; size_t limit = 100; size_t start = 0; @@ -618,6 +618,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { propId = ParseHashV(optionsObj["proposalId"].get_str(), "proposalId"); aggregate = false; isMine = true; + latestOnly = false; } if (!optionsObj["masternode"].isNull()) { @@ -631,20 +632,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()) { @@ -676,6 +664,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { propId = ParseHashV(request.params[0].get_str(), "proposalId"); aggregate = false; isMine = true; + latestOnly = false; } if (request.params.size() > 1) { @@ -690,21 +679,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) { @@ -733,6 +708,24 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { } } + 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; @@ -747,6 +740,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) { diff --git a/test/functional/feature_on_chain_government.py b/test/functional/feature_on_chain_government.py index 87eb7ac76c..5943be86a0 100755 --- a/test/functional/feature_on_chain_government.py +++ b/test/functional/feature_on_chain_government.py @@ -15,6 +15,9 @@ from decimal import Decimal class OnChainGovernanceTest(DefiTestFramework): + mns = None + proposalId = "" + def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True @@ -37,6 +40,7 @@ def run_test(self): mn1 = self.nodes[1].getmininginfo()['masternodes'][0]['id'] mn2 = self.nodes[2].getmininginfo()['masternodes'][0]['id'] mn3 = self.nodes[3].getmininginfo()['masternodes'][0]['id'] + self.mns = [mn0, mn1, mn2, mn3] # Generate chain self.nodes[0].generate(100) @@ -695,7 +699,70 @@ def run_test(self): assert_equal(votes_aggregate["no"], noVotes) assert_equal(votes_aggregate["unknown"], unknownVotes) - assert_equal(len(self.nodes[0].listgovproposalvotes()), 3) + self.test_default_cycles_fix() + self.aggregate_all_votes() + + 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", + "context": "", + "amount": 50, + "cycles": 2, + "payoutAddress": self.nodes[0].getnewaddress()}) + self.nodes[0].generate(1) + self.sync_blocks() + + endHeight = self.nodes[0].getgovproposal(tx1)["cycleEndHeight"] + self.proposalId = self.nodes[0].getgovproposal(tx1)["proposalId"] + + # cycle 1 votes + for mn in range(len(self.mns)): + 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(self.proposalId, 'all') + for vote in votes: + assert_equal(vote["vote"], "YES") # there are only YES votes in cycle 1 + + # move to next cycle + self.nodes[0].generate(endHeight + 1 - self.nodes[0].getblockcount()) + + # cycle 2 votes + for mn in range(len(self.mns)): + self.nodes[mn].votegov(self.proposalId, self.mns[mn], "no") + self.nodes[mn].generate(1) + self.sync_blocks() + + 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() From bc55adae6f8fcd7446901dcb99419ca371319ded Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Thu, 2 Feb 2023 14:27:51 +0800 Subject: [PATCH 06/10] Refactor tests --- test/functional/feature_on_chain_government.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/functional/feature_on_chain_government.py b/test/functional/feature_on_chain_government.py index 5943be86a0..c5a78c8fd3 100755 --- a/test/functional/feature_on_chain_government.py +++ b/test/functional/feature_on_chain_government.py @@ -683,7 +683,14 @@ def run_test(self): assert_equal(self.nodes[0].listgovproposals( {"status": "voting", "pagination": {"start": tx1, "including_start": False, "limit": 1}}), nextProposal) - # test listgovproposalvotes aggregation + 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"]) @@ -699,9 +706,6 @@ def run_test(self): assert_equal(votes_aggregate["no"], noVotes) assert_equal(votes_aggregate["unknown"], unknownVotes) - self.test_default_cycles_fix() - self.aggregate_all_votes() - def test_default_cycles_fix(self): """ Tests fix for an issue for when the cycles argument is not provided, the From 808ed6e5c5a5726b0f3cb6d44047e6a359f2c112 Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Thu, 2 Feb 2023 14:49:20 +0800 Subject: [PATCH 07/10] Redo neutral changes --- src/masternodes/rpc_proposals.cpp | 9 +++++++-- test/functional/feature_on_chain_government.py | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index 9644ab6c0c..5ef0aae43f 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/src/masternodes/rpc_proposals.cpp @@ -3,6 +3,8 @@ #include +const bool DEFAULT_RPC_GOV_NEUTRAL = false; + struct VotingInfo { int32_t votesPossible; int32_t votesPresent; @@ -479,14 +481,17 @@ UniValue votegov(const JSONRPCRequest &request) { auto propId = ParseHashV(request.params[0].get_str(), "proposalId"); auto mnId = ParseHashV(request.params[1].get_str(), "masternodeId"); auto vote = CProposalVoteType::VoteNeutral; - auto voteStr = ToLower(request.params[2].get_str()); + auto neutralVotesAllowed = gArgs.GetBoolArg("-rpc-governance-accept-neutral", DEFAULT_RPC_GOV_NEUTRAL); + if (voteStr == "no") { vote = CProposalVoteType::VoteNo; } else if (voteStr == "yes") { vote = CProposalVoteType::VoteYes; - } else if (voteStr != "neutral") { + } else if (neutralVotesAllowed && voteStr != "neutral") { throw JSONRPCError(RPC_INVALID_PARAMETER, "decision supports yes/no/neutral"); + } else if (!neutralVotesAllowed) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Decision supports yes or no. Neutral is currently disabled because of issue https://github.com/DeFiCh/ain/issues/1704"); } int targetHeight; diff --git a/test/functional/feature_on_chain_government.py b/test/functional/feature_on_chain_government.py index c5a78c8fd3..76d1e44108 100755 --- a/test/functional/feature_on_chain_government.py +++ b/test/functional/feature_on_chain_government.py @@ -147,6 +147,8 @@ def run_test(self): # cannot vote by non owning masternode assert_raises_rpc_error(-5, "Incorrect authorization", self.nodes[0].votegov, cfp1, mn1, "yes") + assert_raises_rpc_error(-8, "Decision supports yes or no. Neutral is currently disabled because of issue https://github.com/DeFiCh/ain/issues/1704", self.nodes[0].votegov, cfp1, mn0, "neutral") + # Vote on proposal self.nodes[0].votegov(cfp1, mn0, "yes") self.nodes[0].generate(1) @@ -157,7 +159,7 @@ def run_test(self): self.sync_blocks() # Try and vote with non-staked MN - assert_raises_rpc_error(None, "does not mine at least one block", self.nodes[3].votegov, cfp1, mn3, "neutral") + assert_raises_rpc_error(None, "does not mine at least one block", self.nodes[3].votegov, cfp1, mn3, "yes") # voting period votingPeriod = 70 From e51834a8df492d7891a1914f85f3bb4e8b5a0cc3 Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Thu, 2 Feb 2023 15:38:06 +0800 Subject: [PATCH 08/10] Fix test --- .../feature_on_chain_government_voting_scenarios.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/feature_on_chain_government_voting_scenarios.py b/test/functional/feature_on_chain_government_voting_scenarios.py index aa09fb6757..fe9611d5ac 100755 --- a/test/functional/feature_on_chain_government_voting_scenarios.py +++ b/test/functional/feature_on_chain_government_voting_scenarios.py @@ -10,16 +10,16 @@ assert_equal, ) -APPROVAL_THRESHOLD=50 -QUORUM=50 -VOTING_PERIOD=10 +APPROVAL_THRESHOLD = 50 +QUORUM = 50 +VOTING_PERIOD = 10 class OCGVotingScenarionTest(DefiTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True self.extra_args = [ - ['-jellyfish_regtest=1', '-dummypos=0', '-txnotokens=0', '-amkheight=50', '-bayfrontheight=51', '-eunosheight=80', '-fortcanningheight=82', '-fortcanninghillheight=84', '-fortcanningroadheight=86', '-fortcanningcrunchheight=88', '-fortcanningspringheight=90', '-fortcanninggreatworldheight=94', '-grandcentralheight=101', '-simulatemainnet=1'], + ['-jellyfish_regtest=1', '-dummypos=0', '-txnotokens=0', '-amkheight=50', '-bayfrontheight=51', '-eunosheight=80', '-fortcanningheight=82', '-fortcanninghillheight=84', '-fortcanningroadheight=86', '-fortcanningcrunchheight=88', '-fortcanningspringheight=90', '-fortcanninggreatworldheight=94', '-grandcentralheight=101', '-simulatemainnet=1', '-rpc-governance-accept-neutral=1'], ] def setup_masternodes(self, nMasternodes = 19): From 15fb0e5694666fbd4b8b96623fded08fd64097ee Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Fri, 3 Feb 2023 12:38:41 +0800 Subject: [PATCH 09/10] Use VotingInfo, remove unknown votes from output --- src/masternodes/rpc_proposals.cpp | 42 +++++++++++++------------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp index 5ef0aae43f..7da3406ab0 100644 --- a/src/masternodes/rpc_proposals.cpp +++ b/src/masternodes/rpc_proposals.cpp @@ -6,9 +6,12 @@ const bool DEFAULT_RPC_GOV_NEUTRAL = false; struct VotingInfo { - int32_t votesPossible; - int32_t votesPresent; - int32_t votesYes; + int32_t votesPossible = 0; + int32_t votesPresent = 0; + int32_t votesYes = 0; + int32_t votesNo = 0; + int32_t votesNeutral = 0; + int32_t votesInvalid = 0; }; UniValue proposalToJSON(const CProposalId &propId, @@ -109,32 +112,22 @@ UniValue proposalVoteToJSON(const CProposalId &propId, uint8_t cycle, const uint return ret; } -struct VoteAccounting { - int totalVotes; - int yesVotes; - int neutralVotes; - int noVotes; - int unknownVotes; -}; - -void proposalVoteAccounting(const CProposalVoteType &vote, uint256 propId, std::map &map) { +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(), VoteAccounting{0, 0, 0, 0, 0}); + map.emplace(propId.GetHex(), VotingInfo{0, 0, 0, 0, 0, 0}); auto entry = &map.find(propId.GetHex())->second; if (voteString == "YES") - ++entry->yesVotes; + ++entry->votesYes; else if (voteString == "NEUTRAL") - ++entry->neutralVotes; + ++entry->votesNeutral; else if (voteString == "NO") - ++entry->noVotes; - else - ++entry->unknownVotes; + ++entry->votesNo; - ++entry->totalVotes; + ++entry->votesPresent; } /* @@ -733,7 +726,7 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { UniValue ret(UniValue::VARR); - std::map map; + std::map map; view.ForEachProposalVote( [&](const CProposalId &pId, uint8_t propCycle, const uint256 &id, CProposalVoteType vote) { @@ -794,11 +787,10 @@ UniValue listgovproposalvotes(const JSONRPCRequest &request) { UniValue stats(UniValue::VOBJ); stats.pushKV("proposalId", entry.first); - stats.pushKV("total", entry.second.totalVotes); - stats.pushKV("yes", entry.second.yesVotes); - stats.pushKV("neutral", entry.second.neutralVotes); - stats.pushKV("no", entry.second.noVotes); - stats.pushKV("unknown", entry.second.unknownVotes); + 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); } From 32c929509dc5a53678c2b502040c936cdcce79a2 Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Fri, 3 Feb 2023 13:11:51 +0800 Subject: [PATCH 10/10] Update test --- test/functional/feature_on_chain_government.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/functional/feature_on_chain_government.py b/test/functional/feature_on_chain_government.py index 76d1e44108..7b95591f45 100755 --- a/test/functional/feature_on_chain_government.py +++ b/test/functional/feature_on_chain_government.py @@ -698,7 +698,6 @@ def test_aggregation(self, propId): 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"]) - unknownVotes = totalVotes - yesVotes - noVotes - neutralVotes votes_aggregate = self.nodes[0].listgovproposalvotes(propId, 'all', -1, {}, True)[0] assert_equal(votes_aggregate["proposalId"], propId) @@ -706,7 +705,6 @@ def test_aggregation(self, propId): assert_equal(votes_aggregate["yes"], yesVotes) assert_equal(votes_aggregate["neutral"], neutralVotes) assert_equal(votes_aggregate["no"], noVotes) - assert_equal(votes_aggregate["unknown"], unknownVotes) def test_default_cycles_fix(self): """