rpc: votegov: Add support for owner and operator address (#1717)

* feat: add possibility to vote with owner/operator address instead of maternode id * Add test case for votegov when address does not own or operate a masternode * add test case for votegov where address is not in the proper format * Make a proper check for address passed in votegov rpc * fix logic for votegov * fix logic for votegov rpc * fix thrown error for rpc_proposal votegov * fix throw error if voting address is not P2PKH or P2WPKH * remove duplicate line in votegov * Update src/masternodes/rpc_proposals.cpp Co-authored-by: Shoham Chakraborty <[email protected]> * replace ternary operation by if for better readability --------- Co-authored-by: Shoham Chakraborty <[email protected]> Co-authored-by: Peter John Bushnell <[email protected]>
DeFiCh · Feb 6, 2023 · 3e92abf · 3e92abf
1 parent aa9549b
commit 3e92abf
Show file tree

Hide file tree

Showing 2 changed files with 110 additions and 7 deletions.
diff --git a/src/masternodes/rpc_proposals.cpp b/src/masternodes/rpc_proposals.cpp
@@ -431,7 +431,7 @@ UniValue votegov(const JSONRPCRequest &request) {
         "\nVote for community proposal" + HelpRequiringPassphrase(pwallet) + "\n",
         {
                                                     {"proposalId", RPCArg::Type::STR, RPCArg::Optional::NO, "The proposal txid"},
-                                                    {"masternodeId", RPCArg::Type::STR, RPCArg::Optional::NO, "The masternode id which made the vote"},
+                                                    {"masternodeId", RPCArg::Type::STR, RPCArg::Optional::NO, "The masternode id / owner address / operator address which made the vote"},
                                                     {"decision", RPCArg::Type::STR, RPCArg::Optional::NO, "The vote decision (yes/no/neutral)"},
                                                     {
                 "inputs",
@@ -465,7 +465,8 @@ UniValue votegov(const JSONRPCRequest &request) {
     RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VSTR, UniValue::VSTR, UniValue::VARR}, true);
 
     auto propId = ParseHashV(request.params[0].get_str(), "proposalId");
-    auto mnId   = ParseHashV(request.params[1].get_str(), "masternodeId");
+    std::string id = request.params[1].get_str();
+    uint256 mnId;
     auto vote   = CProposalVoteType::VoteNeutral;
     auto voteStr = ToLower(request.params[2].get_str());
     auto neutralVotesAllowed = gArgs.GetBoolArg("-rpc-governance-accept-neutral", DEFAULT_RPC_GOV_NEUTRAL);
@@ -493,9 +494,32 @@ UniValue votegov(const JSONRPCRequest &request) {
             throw JSONRPCError(RPC_INVALID_PARAMETER,
                                strprintf("Proposal <%s> is not in voting period", propId.GetHex()));
         }
+        if (id.length() == 64) {
+            mnId = ParseHashV(id, "masternodeId");
+        } else {
+            CTxDestination dest = DecodeDestination(id);
+            if (!IsValidDestination(dest)) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER,
+                                    strprintf("The masternode id or address is not valid: %s", id));
+            }
+            CKeyID ckeyId;
+            if (dest.index() == PKHashType) { 
+                ckeyId = CKeyID(std::get<PKHash>(dest));
+            } else if (dest.index() == WitV0KeyHashType) {
+                ckeyId =  CKeyID(std::get<WitnessV0KeyHash>(dest));
+            } else {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s does not refer to a P2PKH or P2WPKH address", id));
+            }
+            if (auto masterNodeIdByOwner = view.GetMasternodeIdByOwner(ckeyId)) {
+                mnId = masterNodeIdByOwner.value();
+            } else if (auto masterNodeIdByOperator = view.GetMasternodeIdByOperator(ckeyId)) {
+                mnId = masterNodeIdByOperator.value();
+            }
+        }
         auto node = view.GetMasternode(mnId);
         if (!node) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("The masternode %s does not exist", mnId.ToString()));
+                    throw JSONRPCError(RPC_INVALID_PARAMETER,
+                                    strprintf("The masternode does not exist or the address doesn't own a masternode: %s", id));
         }
         ownerDest = node->ownerType == 1 ? CTxDestination(PKHash(node->ownerAuthAddress))
                                          : CTxDestination(WitnessV0KeyHash(node->ownerAuthAddress));

diff --git a/test/functional/feature_on_chain_government_voting_scenarios.py b/test/functional/feature_on_chain_government_voting_scenarios.py
@@ -8,6 +8,7 @@
 from test_framework.test_framework import DefiTestFramework
 from test_framework.util import (
     assert_equal,
+    assert_raises_rpc_error
 )
 
 APPROVAL_THRESHOLD=50
@@ -24,22 +25,22 @@ def set_test_params(self):
 
     def setup_masternodes(self, nMasternodes = 19):
         self.nodes[0].mns = []
-        operatorAddresses = []
+        self.operatorAddresses = []
 
         for _ in range(nMasternodes):
             address = self.nodes[0].getnewaddress('', 'legacy')
             self.nodes[0].mns.append(self.nodes[0].createmasternode(address))
-            operatorAddresses.append(address)
+            self.operatorAddresses.append(address)
             self.nodes[0].generate(1)
 
         self.nodes[0].generate(20) # Enables all MNs
         self.sync_blocks(timeout=120)
 
         # restart node with masternode_operator addresses to be able to mint with every MNs
-        self.restart_node(0, self.nodes[0].extra_args + ['-masternode_operator={}'.format(address) for address in operatorAddresses])
+        self.restart_node(0, self.nodes[0].extra_args + ['-masternode_operator={}'.format(address) for address in self.operatorAddresses])
 
         # Mint with every MNs to meet voting eligibility criteria
-        for address in operatorAddresses:
+        for address in self.operatorAddresses:
             self.nodes[0].generatetoaddress(1, address)
 
     def setup(self):
@@ -92,38 +93,114 @@ def test_vote_on_cfp(self, yesVote, noVote, neutralVote, expectedStatus):
 
         self.rollback_to(height)
 
+    def test_vote_on_cfp_with_address(self, yesVote, noVote, neutralVote, expectedStatus):
+        height = self.nodes[0].getblockcount()
+
+        # Create address for CFP
+        address = self.nodes[0].getnewaddress()
+        context = "<Git issue url>"
+        title = "Create test community fund proposal"
+        amount = 100
+
+        # Create CFP
+        propId = self.nodes[0].creategovcfp({"title": title, "context": context, "amount": amount, "cycles": 1, "payoutAddress": address})
+        self.nodes[0].generate(1)
+
+        addressIterator = iter(self.operatorAddresses)
+
+        for _ in range(yesVote):
+            mnId = next(addressIterator)
+            self.nodes[0].votegov(propId, mnId, 'yes')
+
+        for _ in range(noVote):
+            mnId = next(addressIterator)
+            self.nodes[0].votegov(propId, mnId, 'no')
+
+        for _ in range(neutralVote):
+            mnId = next(addressIterator)
+            self.nodes[0].votegov(propId, mnId, 'neutral')
+
+        self.nodes[0].generate(1)
+
+        self.nodes[0].generate(VOTING_PERIOD * 2)
+        proposal = self.nodes[0].getgovproposal(propId)
+
+        assert_equal(proposal['status'], expectedStatus)
+
+        self.rollback_to(height)
+
+    def test_vote_with_address_without_masternode(self):
+        # Create address for CFP
+        address = self.nodes[0].getnewaddress()
+        context = "<Git issue url>"
+        title = "Create test community fund proposal"
+        amount = 100
+
+        # Create CFP
+        propId = self.nodes[0].creategovcfp({"title": title, "context": context, "amount": amount, "cycles": 1, "payoutAddress": address})
+        self.nodes[0].generate(1)
+
+        address = self.nodes[0].getnewaddress('', 'legacy')
+
+        assert_raises_rpc_error(-8, "The masternode does not exist or the address doesn't own a masternode: {}".format(address), self.nodes[0].votegov, propId, address, 'yes')
+
+    def test_vote_with_invalid_address(self):
+        # Create address for CFP
+        address = self.nodes[0].getnewaddress()
+        context = "<Git issue url>"
+        title = "Create test community fund proposal"
+        amount = 100
+
+        # Create CFP
+        propId = self.nodes[0].creategovcfp({"title": title, "context": context, "amount": amount, "cycles": 1, "payoutAddress": address})
+        self.nodes[0].generate(1)
+
+        address = "fake_address"
+        assert_raises_rpc_error(-8, "The masternode id or address is not valid: {}".format(address), self.nodes[0].votegov, propId, address, 'yes')
+
     def test_scenario_below_approval_threshold(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=4, noVote=6, neutralVote=2, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=4, noVote=6, neutralVote=2, expectedStatus=expectedStatus)
 
     def test_scenario_at_approval_threshold(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=8, noVote=8, neutralVote=0, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=8, noVote=8, neutralVote=0, expectedStatus=expectedStatus)
 
     def test_scenario_above_approval_threshold(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=10, noVote=6, neutralVote=2, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=10, noVote=6, neutralVote=2, expectedStatus=expectedStatus)
 
     def test_scenario_below_quorum(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=6, noVote=2, neutralVote=1, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=6, noVote=2, neutralVote=1, expectedStatus=expectedStatus)
 
     def test_scenario_at_quorum(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=6, noVote=2, neutralVote=2, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=6, noVote=2, neutralVote=2, expectedStatus=expectedStatus)
 
     def test_scenario_above_quorum(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=6, noVote=3, neutralVote=2, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=6, noVote=3, neutralVote=2, expectedStatus=expectedStatus)
 
     def test_scenario_high_neutral_vote(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=8, noVote=3, neutralVote=5, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=8, noVote=3, neutralVote=5, expectedStatus=expectedStatus)
 
     def test_scenario_only_yes_and_neutral(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=8, noVote=0, neutralVote=8, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=8, noVote=0, neutralVote=8, expectedStatus=expectedStatus)
 
     def test_scenario_66_6_percent_approval_full_yes_votes(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=len(self.nodes[0].mns), noVote=0, neutralVote=0, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=len(self.nodes[0].mns), noVote=0, neutralVote=0, expectedStatus=expectedStatus)
 
     def test_scenario_66_6_percent_approval_full_no_votes(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=0, noVote=len(self.nodes[0].mns), neutralVote=0, expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=0, noVote=len(self.nodes[0].mns), neutralVote=0, expectedStatus=expectedStatus)
 
     def test_scenario_66_6_percent_approval_full_neutral_votes(self, expectedStatus):
         self.test_vote_on_cfp(yesVote=0, noVote=0, neutralVote=len(self.nodes[0].mns), expectedStatus=expectedStatus)
+        self.test_vote_on_cfp_with_address(yesVote=0, noVote=0, neutralVote=len(self.nodes[0].mns), expectedStatus=expectedStatus)
 
     def scenarios_test(self):
         self.nodes[0].setgov({"ATTRIBUTES":{
@@ -166,6 +243,8 @@ def run_test(self):
         self.setup()
 
         self.scenarios_test()
+        self.test_vote_with_address_without_masternode()
+        self.test_vote_with_invalid_address()
 
 if __name__ == '__main__':
     OCGVotingScenarionTest().main ()