Skip to content

Commit

Permalink
Merge #6094: feat: support descriptor wallets for RPC governance vote…
Browse files Browse the repository at this point in the history 
…many, votealias

c72ec70 feat: implement governance RPCs votealias and votemany for descriptor wallets (Konstantin Akimov)
4908329 refactor: new method to generate a signing message in CGovernanceVote (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  RPCs `governance votemany` and `governance votealias` use forcely LegacyScriptPubKeyMan instead using CWallet's interface.
  It causes a failures such as
  ```
  test_framework.authproxy.JSONRPCException: This type of wallet does not support this command (-4)
  ```
  See dashpay/dash-issues#59 to track progress

  ## What was done?
  Use CWallet's interfaces instead LegacyScriptPubKeyMan

  ## How Has This Been Tested?
  Functional tests `feature_governance.py` and `feature_governance_cl.py` to run by both ways - legacy and descriptor wallets.

  Run unit and functional tests.

  Extra test done locally:
  ```diff
  --- a/test/functional/test_framework/test_framework.py
  +++ b/test/functional/test_framework/test_framework.py
  @@ -242,10 +242,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):

           if self.options.descriptors is None:
               # Prefer BDB unless it isn't available
  -            if self.is_bdb_compiled():
  -                self.options.descriptors = False
  -            elif self.is_sqlite_compiled():
  +            if self.is_sqlite_compiled():
                   self.options.descriptors = True
  +            elif self.is_bdb_compiled():
  +                self.options.descriptors = False
  ```

  to flip flag descriptor wallets/legacy wallets for all functional tests.

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK c72ec70
  PastaPastaPasta:
    utACK c72ec70

Tree-SHA512: 2c18f0d4acb1c4d57da81bf54f0d155682f558eeb7271df7e6fe75c126ef7f047562794a6730e3ca5351abc4e2daded06b874c2ab77f9c47b840c89d8a158c9f
  • Loading branch information
@PastaPastaPasta
PastaPastaPasta committed Jul 16, 2024
2 parents 1394c41 + c72ec70 commit f16025f
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 65 deletions.
50 changes: 9 additions & 41 deletions src/governance/vote.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,41 +162,6 @@ uint256 CGovernanceVote::GetSignatureHash() const
return SerializeHash(*this);
}

bool CGovernanceVote::Sign(const CKey& key, const CKeyID& keyID)
{
std::string strError;

// Harden Spork6 so that it is active on testnet and no other networks
if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
uint256 signatureHash = GetSignatureHash();

if (!CHashSigner::SignHash(signatureHash, key, vchSig)) {
LogPrintf("CGovernanceVote::Sign -- SignHash() failed\n");
return false;
}

if (!CHashSigner::VerifyHash(signatureHash, keyID, vchSig, strError)) {
LogPrintf("CGovernanceVote::Sign -- VerifyHash() failed, error: %s\n", strError);
return false;
}
} else {
std::string strMessage = masternodeOutpoint.ToStringShort() + "|" + nParentHash.ToString() + "|" +
::ToString(nVoteSignal) + "|" + ::ToString(nVoteOutcome) + "|" + ::ToString(nTime);

if (!CMessageSigner::SignMessage(strMessage, vchSig, key)) {
LogPrintf("CGovernanceVote::Sign -- SignMessage() failed\n");
return false;
}

if (!CMessageSigner::VerifyMessage(keyID, vchSig, strMessage, strError)) {
LogPrintf("CGovernanceVote::Sign -- VerifyMessage() failed, error: %s\n", strError);
return false;
}
}

return true;
}

bool CGovernanceVote::CheckSignature(const CKeyID& keyID) const
{
std::string strError;
Expand All @@ -208,12 +173,7 @@ bool CGovernanceVote::CheckSignature(const CKeyID& keyID) const
return false;
}
} else {
std::string strMessage = masternodeOutpoint.ToStringShort() + "|" + nParentHash.ToString() + "|" +
::ToString(nVoteSignal) + "|" +
::ToString(nVoteOutcome) + "|" +
::ToString(nTime);

if (!CMessageSigner::VerifyMessage(keyID, vchSig, strMessage, strError)) {
if (!CMessageSigner::VerifyMessage(keyID, vchSig, GetSignatureString(), strError)) {
LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- VerifyMessage() failed, error: %s\n", strError);
return false;
}
Expand Down Expand Up @@ -275,6 +235,14 @@ bool CGovernanceVote::IsValid(const CDeterministicMNList& tip_mn_list, bool useV
}
}

std::string CGovernanceVote::GetSignatureString() const
{
return masternodeOutpoint.ToStringShort() + "|" + nParentHash.ToString() + "|" +
::ToString(nVoteSignal) + "|" +
::ToString(nVoteOutcome) + "|" +
::ToString(nTime);
}

bool operator==(const CGovernanceVote& vote1, const CGovernanceVote& vote2)
{
bool fResult = ((vote1.masternodeOutpoint == vote2.masternodeOutpoint) &&
Expand Down
1 change: 1 addition & 0 deletions src/governance/vote.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class CGovernanceVote
bool Sign(const CActiveMasternodeManager& mn_activeman);
bool CheckSignature(const CBLSPublicKey& pubKey) const;
bool IsValid(const CDeterministicMNList& tip_mn_list, bool useVotingKey) const;
std::string GetSignatureString() const;
void Relay(PeerManager& peerman, const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list) const;

const COutPoint& GetMasternodeOutpoint() const { return masternodeOutpoint; }
Expand Down
77 changes: 53 additions & 24 deletions src/rpc/governance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,37 @@ static RPCHelpMan gobject_submit()
};
}

static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const std::map<uint256, CKey>& keys,
#ifdef ENABLE_WALLET
static bool SignVote(const CWallet& wallet, const CKeyID& keyID, CGovernanceVote& vote)
{
// Special implementation for testnet (Harden Spork6 that has not been deployed to other networks)
if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
std::vector<unsigned char> signature;
if (!wallet.SignSpecialTxPayload(vote.GetSignatureHash(), keyID, signature)) {
LogPrintf("SignVote -- SignHash() failed\n");
return false;
}
vote.SetSignature(signature);
return true;
} // end of testnet implementation

std::string strMessage{vote.GetSignatureString()};
std::string signature;
SigningResult err = wallet.SignMessage(strMessage, PKHash{keyID}, signature);
if (err != SigningResult::OK) {
LogPrintf("SignVote failed due to: %s\n", SigningResultString(err));
return false;
}
bool ret = true;
const auto decoded = DecodeBase64(signature, &ret);
assert(!ret); // it should not fail

vote.SetSignature(std::vector<unsigned char>(decoded.data(), decoded.data() + decoded.size()));
return true;
}

static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet& wallet,
const std::map<uint256, CKeyID>& votingKeys,
const uint256& hash, vote_signal_enum_t eVoteSignal,
vote_outcome_enum_t eVoteOutcome)
{
Expand All @@ -425,9 +455,9 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const std::ma

UniValue resultsObj(UniValue::VOBJ);

for (const auto& p : keys) {
for (const auto& p : votingKeys) {
const auto& proTxHash = p.first;
const auto& key = p.second;
const auto& keyID = p.second;

UniValue statusObj(UniValue::VOBJ);

Expand All @@ -441,7 +471,8 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const std::ma
}

CGovernanceVote vote(dmn->collateralOutpoint, hash, eVoteSignal, eVoteOutcome);
if (!vote.Sign(key, key.GetPubKey().GetID())) {

if (!SignVote(wallet, keyID, vote) || !vote.CheckSignature(keyID)) {
nFailed++;
statusObj.pushKV("result", "failed");
statusObj.pushKV("errorMessage", "Failure to sign.");
Expand Down Expand Up @@ -471,7 +502,14 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const std::ma
return returnObj;
}

#ifdef ENABLE_WALLET
static bool CheckWalletOwnsKey(const CWallet& wallet, const CKeyID& keyid)
{
const CScript script{GetScriptForDestination(PKHash(keyid))};
LOCK(wallet.cs_wallet);

return wallet.IsMine(script) == isminetype::ISMINE_SPENDABLE;
}

static RPCHelpMan gobject_vote_many()
{
return RPCHelpMan{"gobject vote-many",
Expand Down Expand Up @@ -510,22 +548,17 @@ static RPCHelpMan gobject_vote_many()

EnsureWalletIsUnlocked(wallet.get());

LegacyScriptPubKeyMan* spk_man = wallet->GetLegacyScriptPubKeyMan();
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}

std::map<uint256, CKey> votingKeys;
std::map<uint256, CKeyID> votingKeys;

auto mnList = node.dmnman->GetListAtChainTip();
mnList.ForEachMN(true, [&](auto& dmn) {
CKey votingKey;
if (spk_man->GetKey(dmn.pdmnState->keyIDVoting, votingKey)) {
votingKeys.emplace(dmn.proTxHash, votingKey);
const bool is_mine = CheckWalletOwnsKey(*wallet, dmn.pdmnState->keyIDVoting);
if (is_mine) {
votingKeys.emplace(dmn.proTxHash, dmn.pdmnState->keyIDVoting);
}
});

return VoteWithMasternodes(request, votingKeys, hash, eVoteSignal, eVoteOutcome);
return VoteWithMasternodes(request, *wallet, votingKeys, hash, eVoteSignal, eVoteOutcome);
},
};
}
Expand Down Expand Up @@ -575,20 +608,16 @@ static RPCHelpMan gobject_vote_alias()
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid or unknown proTxHash");
}

LegacyScriptPubKeyMan* spk_man = wallet->GetLegacyScriptPubKeyMan();
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}

CKey votingKey;
if (!spk_man->GetKey(dmn->pdmnState->keyIDVoting, votingKey)) {
const bool is_mine = CheckWalletOwnsKey(*wallet, dmn->pdmnState->keyIDVoting);
if (!is_mine) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Private key for voting address %s not known by wallet", EncodeDestination(PKHash(dmn->pdmnState->keyIDVoting))));
}

std::map<uint256, CKey> votingKeys;
votingKeys.emplace(proTxHash, votingKey);
std::map<uint256, CKeyID> votingKeys;
votingKeys.emplace(proTxHash, dmn->pdmnState->keyIDVoting);

return VoteWithMasternodes(request, votingKeys, hash, eVoteSignal, eVoteOutcome);
return VoteWithMasternodes(request, *wallet, votingKeys, hash, eVoteSignal, eVoteOutcome);
},
};
}
Expand Down
2 changes: 2 additions & 0 deletions test/functional/test_runner.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,9 @@
'feature_new_quorum_type_activation.py',
'feature_governance_objects.py',
'feature_governance.py --legacy-wallet',
'feature_governance.py --descriptors',
'feature_governance_cl.py --legacy-wallet',
'feature_governance_cl.py --descriptors',
'rpc_uptime.py',
'feature_discover.py',
'wallet_resendwallettransactions.py --legacy-wallet',
Expand Down

0 comments on commit f16025f

Please sign in to comment.