Merge dashpay#6003: feat: support rpc protx-register for descriptor wallets - part VI

a33dcb3 fix: CheckWalletOwnsScript/CheckWalletOwnsKey to use wallet instead of SPK (Konstantin Akimov)
b2ede8b feat: update list of tests that still doesn't support descriptor wallets (Konstantin Akimov)
838d06f feat: enable descriptor wallets for more tests (Konstantin Akimov)
5ab108c feat: implementation of RPC 'protx register' for descriptor wallets (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Many rpc such as `protx register` 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)
  ```
  for all functional tests that uses Masternodes/evo nodes.

  See dashpay/dash-issues#59 to track progress

  ## What was done?
  Some direct usages of LegacyScriptPubKeyMan refactored to use CWallet's functionality.
  There are still 4 functional tests that doesn't work for descriptor wallets:
   - feature_dip3_deterministicmns.py (no rpc `protx updateregistar`)
   - feature_governance.py: no rpc for `governance votemany` and `governance votealias`
   - interface_zmq_dash.py (see governance)

  That's part I of changes, other changes are not PR-ready yet, WIP.

  ## How Has This Been Tested?
  Firstly, the flag `--legacy-wallets` are removed for many functional tests.
  Secondly, the flag `--descriptors` is inverted in default value:
  ```
  diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
  index 585a6a7..9ad5fd1 100755
  --- 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
               else:
                   # If neither are compiled, tests requiring a wallet will be skipped and the value of self.options.descriptors won't matter
                   # It still needs to exist and be None in order for tests to work however.
  ```

  ## Breaking Changes
  N/A, descriptor wallets have not been publicly released yet

  ## 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
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK a33dcb3

Tree-SHA512: 24e22ac91e30a3804d1ac9af864eb9d073208bbb6d1f3326c7f0438f3ccce2b553aa46450989e48cb5c6e0d838ff1c88c6522f195e7aa2bd89342710f3ecef77

Author: PastaPastaPasta

src/rpc/evo.cpp

@@ -776,18 +776,23 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request,
                 ret.pushKV("signMessage", ptx.MakeSignString());
                 return ret;
             } else {
-                // lets prove we own the collateral
-                // TODO: make collateral works with Descriptor wallets too
-                const LegacyScriptPubKeyMan* spk_man = wallet->GetLegacyScriptPubKeyMan();
-                if (!spk_man) {
-                    throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
-                }
-
-                CKey key;
-                if (!spk_man->GetKey(ToKeyID(*pkhash), key)) {
-                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("collateral key not in wallet: %s", EncodeDestination(txDest)));
-                }
-                SignSpecialTxPayloadByString(tx, ptx, key);
+                {
+                    LOCK(wallet->cs_wallet);
+                    // lets prove we own the collateral
+                    CScript scriptPubKey = GetScriptForDestination(txDest);
+                    std::unique_ptr<SigningProvider> provider = wallet->GetSolvingProvider(scriptPubKey);
+
+                    std::string signed_payload;
+                    SigningResult err = wallet->SignMessage(ptx.MakeSignString(), *pkhash, signed_payload);
+                    if (err == SigningResult::SIGNING_FAILED) {
+                        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, SigningResultString(err));
+                    } else if (err != SigningResult::OK){
+                        throw JSONRPCError(RPC_WALLET_ERROR, SigningResultString(err));
+                    }
+                    bool invalid = false;
+                    ptx.vchSig = DecodeBase64(signed_payload.c_str(), &invalid);
+                    if (invalid) throw JSONRPCError(RPC_INTERNAL_ERROR, "failed to decode base64 ready signature for protx");
+                } // cs_wallet
                 SetTxPayload(tx, ptx);
                 return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit);
             }
@@ -1259,33 +1264,15 @@ static void protx_list_help(const JSONRPCRequest& request)
 }
 
 #ifdef ENABLE_WALLET
-static bool CheckWalletOwnsKey(const CWallet* const pwallet, const CKeyID& keyID) {
-    if (!pwallet) {
-        return false;
-    }
-    const LegacyScriptPubKeyMan* const spk_man = pwallet->GetLegacyScriptPubKeyMan();
-    if (!spk_man) {
-        return false;
-    }
-    return spk_man->HaveKey(keyID);
-}
-
 static bool CheckWalletOwnsScript(const CWallet* const pwallet, const CScript& script) {
     if (!pwallet) {
         return false;
     }
-    const LegacyScriptPubKeyMan* const spk_man = pwallet->GetLegacyScriptPubKeyMan();
-    if (!spk_man) {
-        return false;
-    }
+    return WITH_LOCK(pwallet->cs_wallet, return pwallet->IsMine(script)) == isminetype::ISMINE_SPENDABLE;
+}
 
-    CTxDestination dest;
-    if (ExtractDestination(script, dest)) {
-        if ((std::get_if<PKHash>(&dest) && spk_man->HaveKey(ToKeyID(*std::get_if<PKHash>(&dest)))) || (std::get_if<ScriptHash>(&dest) && spk_man->HaveCScript(CScriptID{ScriptHash(*std::get_if<ScriptHash>(&dest))}))) {
-            return true;
-        }
-    }
-    return false;
+static bool CheckWalletOwnsKey(const CWallet* const pwallet, const CKeyID& keyID) {
+    return CheckWalletOwnsScript(pwallet, GetScriptForDestination(PKHash(keyID)));
 }
 #endif
 

test/functional/test_runner.py

@@ -93,7 +93,7 @@
     # 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_llmq_data_recovery.py --legacy-wallet',
+    'feature_llmq_data_recovery.py',
     'wallet_hd.py --legacy-wallet',
     'wallet_hd.py --descriptors',
     'wallet_backup.py --legacy-wallet',
@@ -105,9 +105,9 @@
     'rpc_fundrawtransaction.py --legacy-wallet',
     'rpc_fundrawtransaction.py --legacy-wallet --nohd',
     'rpc_fundrawtransaction.py --descriptors',
-    'p2p_quorum_data.py --legacy-wallet',
+    'p2p_quorum_data.py',
     # vv Tests less than 2m vv
-    'p2p_instantsend.py --legacy-wallet',
+    'p2p_instantsend.py',
     'wallet_basic.py --legacy-wallet',
     'wallet_basic.py --descriptors',
     'wallet_labels.py --legacy-wallet',
@@ -120,20 +120,20 @@
     'wallet_listtransactions.py --legacy-wallet',
     'wallet_listtransactions.py --descriptors',
     'feature_multikeysporks.py',
-    'feature_dip3_v19.py --legacy-wallet',
-    'feature_llmq_signing.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_signing.py --spork21 --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_chainlocks.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_rotation.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_connections.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_evo.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_simplepose.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_is_cl_conflicts.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_is_retroactive.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_llmq_dkgerrors.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_dip4_coinbasemerkleroots.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_asset_locks.py --legacy-wallet', # NOTE: needs dash_hash to pass
-    'feature_mnehf.py --legacy-wallet', # NOTE: needs dash_hash to pass
+    'feature_dip3_v19.py',
+    'feature_llmq_signing.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_signing.py --spork21', # NOTE: needs dash_hash to pass
+    'feature_llmq_chainlocks.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_rotation.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_connections.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_evo.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_simplepose.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_is_cl_conflicts.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_is_retroactive.py', # NOTE: needs dash_hash to pass
+    'feature_llmq_dkgerrors.py', # NOTE: needs dash_hash to pass
+    'feature_dip4_coinbasemerkleroots.py', # NOTE: needs dash_hash to pass
+    'feature_asset_locks.py', # NOTE: needs dash_hash to pass
+    'feature_mnehf.py', # NOTE: needs dash_hash to pass
     # vv Tests less than 60s vv
     'p2p_sendheaders.py', # NOTE: needs dash_hash to pass
     'p2p_sendheaders_compressed.py', # NOTE: needs dash_hash to pass
@@ -151,6 +151,7 @@
     'feature_abortnode.py',
     # vv Tests less than 30s vv
     'rpc_quorum.py --legacy-wallet',
+    'rpc_quorum.py --descriptors',
     'wallet_keypool_topup.py --legacy-wallet',
     'wallet_keypool_topup.py --descriptors',
     'feature_fee_estimation.py',
@@ -227,7 +228,7 @@
     'feature_backwards_compatibility.py --legacy-wallet',
     'feature_backwards_compatibility.py --descriptors',
     'wallet_txn_clone.py --mineblock',
-    'feature_notifications.py --legacy-wallet',
+    'feature_notifications.py',
     'rpc_getblockfilter.py',
     'rpc_invalidateblock.py',
     'feature_txindex.py',
@@ -290,10 +291,10 @@
     'p2p_unrequested_blocks.py', # NOTE: needs dash_hash to pass
     'feature_shutdown.py',
     'rpc_coinjoin.py',
-    'rpc_masternode.py --legacy-wallet',
+    'rpc_masternode.py',
     'rpc_mnauth.py',
-    'rpc_verifyislock.py --legacy-wallet',
-    'rpc_verifychainlock.py --legacy-wallet',
+    'rpc_verifyislock.py',
+    'rpc_verifychainlock.py',
     'wallet_create_tx.py --legacy-wallet',
     'wallet_send.py --legacy-wallet',
     'wallet_send.py --descriptors',