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.

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

merged 14 commits into from
Feb 6, 2023

Conversation

DocteurPing
Copy link

@DocteurPing DocteurPing commented Jan 30, 2023

Summary

Enables support for address support on votegov to vote with an address in addition to the currently accepted masternode ID.

How

  • Over the wire, TX still uses masternode ID.
  • The RPC API now auto selects the masternode ID based on the address provided.
  • The selection order:
    • MasternodeID
    • OwnerAdress
    • OperatorAddress

Limitations

  • Throws an error when there is no reasonable choice available (invalid address/masternodeId, address does not own/operate a masternode).

@prasannavl prasannavl changed the title feat: add possibility to vote with owner/operator address instead of … rpc: votegov: Add support for owner and operator address Jan 31, 2023
@prasannavl prasannavl mentioned this pull request Feb 9, 2023
19 tasks
@@ -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 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.

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) {
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);
Copy link
Member

Choose a reason for hiding this comment

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

Check that masterNodeIdByOperator is valid or you will get a std::bad_optional_access thrown on masterNodeIdByOperator.value(). I'd structure it like the following:

if (auto masterNodeIdByOwner = view.GetMasternodeIdByOwner(ckeyId)) {
    mnId = masterNodeIdByOwner.value();
} else if (auto masterNodeIdByOperator = view.GetMasternodeIdByOperator(ckeyId)) {
    mnId = masterNodeIdByOperator.value();
}

throw JSONRPCError(RPC_INVALID_PARAMETER,
strprintf("The masternode id or address is not valid: %s", id));
}
const CKeyID ckeyId = dest.index() == PKHashType ? CKeyID(std::get<PKHash>(dest)) : CKeyID(std::get<WitnessV0KeyHash>(dest));
Copy link
Member

Choose a reason for hiding this comment

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

We need to make sure that the provided address is either P2PKH or a P2WPKH, otherwise if someone passes a different variant then std::get will throw a std::bad_variant_access.

            const CKeyID ckeyId  = dest.index() == PKHashType ? CKeyID(std::get<PKHash>(dest))
                                 : dest.index() == WitV0KeyHashType ? CKeyID(std::get<WitnessV0KeyHash>(dest))
                                 : CKeyID();

Bushstar
Bushstar previously approved these changes Feb 2, 2023
@Mixa84 Mixa84 added v/3.2.4 and removed v/3.2.3 labels Feb 3, 2023
throw JSONRPCError(RPC_INVALID_PARAMETER,
strprintf("The masternode id or address is not valid: %s", id));
}
const CKeyID ckeyId = dest.index() == PKHashType ?
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit too hard to read IMO, better to break it down into ifs.

src/masternodes/rpc_proposals.cpp Outdated Show resolved Hide resolved
@Bushstar Bushstar merged commit 3e92abf into DeFiCh:master Feb 6, 2023
@DocteurPing DocteurPing deleted the feat/vote_with_address_for_proposal branch February 7, 2023 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants