Merge #6066: feat: support descriptor wallets for RPC protx updateregistar

c9a600e0fa fix: linkage error - message signer better to be common code rather than libconsensus (Konstantin Akimov)
8299b3b369 feat: protxregistar implementation for descriptor wallets (Konstantin Akimov)
6f45432f76 refactor: removed unused SignSpecialTxPayloadByString (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  RPC `protx updateregistar` uses 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)
  ```

  ## What was done?
  New method `SignSpecialTxPayloadByHash` is implemented in interface instead exporting raw private key for some address.

  See https://github.com/dashpay/dash-issues/issues/59 to track progress

  ## How Has This Been Tested?
  Functional test `feature_dip3_deterministicmns.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:
  PastaPastaPasta:
    utACK c9a600e0fa

Tree-SHA512: 00aea1cd9db3537b9a9dcdee096d47ea48337edeea3f52ad54aea91781678b8641ac2dd86b67f61f87e3912945bcb5361a42a3279b6c08bb8d9f096bed8fe842
This commit is contained in:
pasta 2024-06-25 09:45:49 -05:00
commit 4a520991db
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
7 changed files with 59 additions and 25 deletions

View File

@ -480,7 +480,6 @@ libbitcoin_server_a_SOURCES = \
masternode/payments.cpp \
masternode/sync.cpp \
masternode/utils.cpp \
messagesigner.cpp \
miner.cpp \
net.cpp \
netfulfilledman.cpp \
@ -770,6 +769,7 @@ libbitcoin_util_a_SOURCES = \
fs.cpp \
interfaces/handler.cpp \
logging.cpp \
messagesigner.cpp \
random.cpp \
randomenv.cpp \
rpc/request.cpp \

View File

@ -294,25 +294,13 @@ static void UpdateSpecialTxInputsHash(const CMutableTransaction& tx, SpecialTxPa
}
template<typename SpecialTxPayload>
static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxPayload& payload, const CKey& key)
static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxPayload& payload, const CKeyID& keyID, const CWallet& wallet)
{
UpdateSpecialTxInputsHash(tx, payload);
payload.vchSig.clear();
uint256 hash = ::SerializeHash(payload);
if (!CHashSigner::SignHash(hash, key, payload.vchSig)) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "failed to sign special tx");
}
}
template<typename SpecialTxPayload>
static void SignSpecialTxPayloadByString(const CMutableTransaction& tx, SpecialTxPayload& payload, const CKey& key)
{
UpdateSpecialTxInputsHash(tx, payload);
payload.vchSig.clear();
std::string m = payload.MakeSignString();
if (!CMessageSigner::SignMessage(m, payload.vchSig, key)) {
const uint256 hash = ::SerializeHash(payload);
if (!wallet.SignSpecialTxPayload(hash, keyID, payload.vchSig)) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "failed to sign special tx");
}
}
@ -1111,14 +1099,12 @@ static UniValue protx_update_registrar_wrapper(const JSONRPCRequest& request, CC
ptx.scriptPayout = GetScriptForDestination(payoutDest);
}
LegacyScriptPubKeyMan* spk_man = wallet->GetLegacyScriptPubKeyMan();
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
CKey keyOwner;
if (!spk_man->GetKey(dmn->pdmnState->keyIDOwner, keyOwner)) {
throw std::runtime_error(strprintf("Private key for owner address %s not found in your wallet", EncodeDestination(PKHash(dmn->pdmnState->keyIDOwner))));
{
const auto pkhash{PKHash(dmn->pdmnState->keyIDOwner)};
LOCK(wallet->cs_wallet);
if (wallet->IsMine(GetScriptForDestination(pkhash)) != isminetype::ISMINE_SPENDABLE) {
throw std::runtime_error(strprintf("Private key for owner address %s not found in your wallet", EncodeDestination(pkhash)));
}
}
CMutableTransaction tx;
@ -1136,7 +1122,7 @@ static UniValue protx_update_registrar_wrapper(const JSONRPCRequest& request, CC
}
FundSpecialTx(wallet.get(), tx, ptx, feeSourceDest);
SignSpecialTxPayloadByHash(tx, ptx, keyOwner);
SignSpecialTxPayloadByHash(tx, ptx, dmn->pdmnState->keyIDOwner, *wallet);
SetTxPayload(tx, ptx);
return SignAndSendSpecialTx(request, chain_helper, chainman, tx);

View File

@ -5,6 +5,7 @@
#include <key_io.h>
#include <chainparams.h>
#include <logging.h>
#include <messagesigner.h>
#include <script/descriptor.h>
#include <script/sign.h>
#include <shutdown.h>
@ -726,6 +727,16 @@ SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, con
return SigningResult::SIGNING_FAILED;
}
bool LegacyScriptPubKeyMan::SignSpecialTxPayload(const uint256& hash, const CKeyID& keyid, std::vector<unsigned char>& vchSig) const
{
CKey key;
if (!GetKey(keyid, key)) {
return false;
}
return CHashSigner::SignHash(hash, key, vchSig);
}
TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const
{
if (n_signed) {
@ -2196,6 +2207,21 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message,
return SigningResult::OK;
}
bool DescriptorScriptPubKeyMan::SignSpecialTxPayload(const uint256& hash, const CKeyID& keyid, std::vector<unsigned char>& vchSig) const
{
std::unique_ptr<FlatSigningProvider> keys = GetSigningProvider(GetScriptForDestination(PKHash(keyid)), true);
if (!keys) {
return false;
}
CKey key;
if (!keys->GetKey(keyid, key)) {
return false;
}
return CHashSigner::SignHash(hash, key, vchSig);
}
TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const
{
if (n_signed) {

View File

@ -211,6 +211,7 @@ public:
virtual bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const { return false; }
/** Sign a message with the given script */
virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; };
virtual bool SignSpecialTxPayload(const uint256& hash, const CKeyID& keyid, std::vector<unsigned char>& vchSig) const { return false; }
/** Adds script and derivation path information to a PSBT, and optionally signs it. */
virtual TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const { return TransactionError::INVALID_PSBT; }
@ -358,6 +359,7 @@ public:
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const override;
SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
bool SignSpecialTxPayload(const uint256& hash, const CKeyID& keyid, std::vector<unsigned char>& vchSig) const override;
TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override;
uint256 GetID() const override;
@ -588,6 +590,7 @@ public:
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const override;
SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
bool SignSpecialTxPayload(const uint256& hash, const CKeyID& keyid, std::vector<unsigned char>& vchSig) const override;
TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override;
uint256 GetID() const override;

View File

@ -3070,6 +3070,19 @@ SigningResult CWallet::SignMessage(const std::string& message, const PKHash& pkh
return SigningResult::PRIVATE_KEY_NOT_AVAILABLE;
}
bool CWallet::SignSpecialTxPayload(const uint256& hash, const CKeyID& keyid, std::vector<unsigned char>& vchSig) const
{
SignatureData sigdata;
CScript script_pub_key = GetScriptForDestination(PKHash(keyid));
for (const auto& spk_man_pair : m_spk_managers) {
if (spk_man_pair.second->CanProvide(script_pub_key, sigdata)) {
LOCK(cs_wallet); // DescriptorScriptPubKeyMan calls IsLocked which can lock cs_wallet in a deadlocking order
return spk_man_pair.second->SignSpecialTxPayload(hash, keyid, vchSig);
}
}
return false;
}
bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl)
{
std::vector<CRecipient> vecSend;

View File

@ -1107,6 +1107,11 @@ public:
/** Sign the tx given the input coins and sighash. */
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const;
SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const;
/** Sign the payload of special transaction.
* Because wallet is not aware about special transactions entity,
* but it should work for any its type, we pass there directly a hash of payload.
*/
bool SignSpecialTxPayload(const uint256& hash, const CKeyID& keyid, std::vector<unsigned char>& vchSig) const;
/**
* Fills out a PSBT with information from the wallet. Fills in UTXOs if we have

View File

@ -93,6 +93,7 @@ BASE_SCRIPTS = [
# Scripts that are run by default.
# Longest test should go first, to favor running tests in parallel
'feature_dip3_deterministicmns.py --legacy-wallet', # NOTE: needs dash_hash to pass
'feature_dip3_deterministicmns.py --descriptors', # NOTE: needs dash_hash to pass
'feature_llmq_data_recovery.py',
'wallet_hd.py --legacy-wallet',
'wallet_hd.py --descriptors',