Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Feb 6, 2023
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
64d81a2
feat: add possibility to vote with owner/operator address instead of …
Jan 30, 2023
9843389
Add test case for votegov when address does not own or operate a mas…
Jan 31, 2023
f7ceb92
add test case for votegov where address is not in the proper format
Jan 31, 2023
b58549c
Make a proper check for address passed in votegov rpc
Feb 1, 2023
26349e0
fix logic for votegov
Feb 1, 2023
5de0fd4
fix logic for votegov rpc
Feb 1, 2023
d1849d5
fix thrown error for rpc_proposal votegov
Feb 2, 2023
f9411ff
fix throw error if voting address is not P2PKH or P2WPKH
Feb 2, 2023
287b57f
remove duplicate line in votegov
Feb 2, 2023
ed8cbe9
Merge branch 'master' into feat/vote_with_address_for_proposal
DocteurPing Feb 2, 2023
5c903ac
Merge branch 'master' into feat/vote_with_address_for_proposal
DocteurPing Feb 3, 2023
06e24c2
Update src/masternodes/rpc_proposals.cpp
DocteurPing Feb 6, 2023
72fbf43
replace ternary operation by if for better readability
Feb 6, 2023
054bf17
Merge branch 'master' into feat/vote_with_address_for_proposal
Bushstar Feb 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions src/masternodes/rpc_proposals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,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/ownerAddress/operatorAdress", RPCArg::Type::STR, RPCArg::Optional::NO, "The masternode id / owner address / operator address which made the vote"},
DocteurPing marked this conversation as resolved.
Show resolved Hide resolved
{"decision", RPCArg::Type::STR, RPCArg::Optional::NO, "The vote decision (yes/no/neutral)"},
{
"inputs",
Expand Down Expand Up @@ -449,7 +449,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 = uint256();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you only need as uint256 will default initialise itself as the class has a default constructor.

uint256 mnId;

auto vote = CProposalVoteType::VoteNeutral;

auto voteStr = ToLower(request.params[2].get_str());
Expand All @@ -474,9 +475,30 @@ 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");
}
auto node = view.GetMasternode(mnId);
if (!node) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("The masternode %s does not exist", mnId.ToString()));
if (id.length() == 34) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address can be 26-35 chars long for P2PKH and 42 for P2WPKH. I'd work on getting the correct mnId before calling GetMasternode.

CTxDestination dest = DecodeDestination(id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to call IsValidDestination on dest to make sure it is valid. You can do this in the else statement.

        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));
            }
            // Call GetMasternodeIdByOwner and GetMasternodeIdByOperator here.
        }

const CKeyID ckeyId = dest.index() == PKHashType ? CKeyID(std::get<PKHash>(dest)) : CKeyID(std::get<WitnessV0KeyHash>(dest));
auto masterNodeIdByOwner = view.GetMasternodeIdByOwner(ckeyId);
if (!masterNodeIdByOwner) {
auto masterNodeIdByOperator = view.GetMasternodeIdByOperator(ckeyId);
if (!masterNodeIdByOperator) {
throw JSONRPCError(RPC_INVALID_PARAMETER,
strprintf("The masternode does not exist or the address doesn't own a masternode: %s", id));
}
mnId = masterNodeIdByOperator.value();
} else {
mnId = masterNodeIdByOwner.value();
}
node = view.GetMasternode(mnId);
} else {
throw JSONRPCError(RPC_INVALID_PARAMETER,
strprintf("The masternode id or address is not valid: %s", id));
}
}
ownerDest = node->ownerType == 1 ? CTxDestination(PKHash(node->ownerAuthAddress))
: CTxDestination(WitnessV0KeyHash(node->ownerAuthAddress));
Expand Down
87 changes: 83 additions & 4 deletions test/functional/feature_on_chain_government_voting_scenarios.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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":{
Expand Down Expand Up @@ -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 ()