Merge #5911: backport: bitcoin#18788, bitcoin#18974, bitcoin#19230, b…

…itcoin#19239, bitcoin#19441, bitcoin#19568, bitcoin#19668, bitcoin#20226, bitcoin#21049, bitcoin#22446 (descriptor wallets part II)

85fa6e4 Merge bitcoin#22446: test: Fix wallet_listdescriptors.py if bdb is not compiled (fanquake)
9a02f7d fix: drop requirement of HD in CanGetAddresses for watch-only wallets (Konstantin Akimov)
d04c1a7 fix: unify ScriptPubKeyMan implementation with bitcoin's (Konstantin Akimov)
953b670 Merge bitcoin#18788: tests: Update more tests to work with descriptor wallets (Wladimir J. van der Laan)
da2a7ed Merge bitcoin#21049: Add release notes for listdescriptors RPC (MarcoFalke)
56d9fe9 Merge bitcoin#20226: wallet, rpc: add listdescriptors command (Samuel Dobson)
59c30e6 fix: add missing dashification for bitcoin#18067 (Konstantin Akimov)
3575a6c Merge bitcoin#19568: Wallet should not override signing errors (fanquake)
9600f9c Merge bitcoin#19441: walletdb: don't reinitialize desc cache with multiple cache entries (Samuel Dobson)
a1e3885 Merge bitcoin#19239: tests: move generate_wif_key to wallet_util.py (MarcoFalke)
eaf31ad Merge bitcoin#19230: [TESTS] Move base58 to own module to break circular dependency (MarcoFalke)
6668e21 Merge bitcoin#18974: test: Check that invalid witness destinations can not be imported (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  This PR continues supporting of Descriptor Wallets, see #5579 for prior work.
  See dashpay/dash-issues#59 for Check-list issue

  ## What was done?

  Particularly:
   - bitcoin#18974
   - bitcoin#19230
   - bitcoin#19239
   - bitcoin#19441
   - bitcoin#19568
   - bitcoin#20226
   - bitcoin#21049
   - bitcoin#18788
   - bitcoin#22446

  Beside that fixed:
   - drop requirement of HD in CanGetAddresses for watch-only wallets
   - unify ScriptPubKeyMan implementation for KeyOrigin with bitcoin's
   - minor dashification for  bitcoin#18067

  ## How Has This Been Tested?
  Run unit + functional test. Backport bitcoin#18788 enables descriptor wallets for multiple functional tests.

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] 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 85fa6e4

Tree-SHA512: 4759e834c04d0f66e765399e3941afcaad2b63503d79a5cef2c04393bbeccaa8e85c06ae3633504c74ad7854180d4d7f3d4eb11c1c3a27ab9ac166a9811b90df

doc/release-notes-21049.md

@@ -0,0 +1,6 @@
+Wallet
+------
+
+- A new `listdescriptors` RPC is available to inspect the contents of descriptor-enabled wallets.
+  The RPC returns public versions of all imported descriptors, including their timestamp and flags.
+  For ranged descriptors, it also returns the range boundaries and the next index to generate addresses from. (dash#5911)

src/wallet/rpcdump.cpp

@@ -1908,3 +1908,72 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) {
 
     return response;
 }
+
+RPCHelpMan listdescriptors()
+{
+    return RPCHelpMan{
+        "listdescriptors",
+        "\nList descriptors imported into a descriptor-enabled wallet.",
+        {},
+        RPCResult{
+            RPCResult::Type::ARR, "", "Response is an array of descriptor objects",
+            {
+                {RPCResult::Type::OBJ, "", "", {
+                    {RPCResult::Type::STR, "desc", "Descriptor string representation"},
+                    {RPCResult::Type::NUM, "timestamp", "The creation time of the descriptor"},
+                    {RPCResult::Type::BOOL, "active", "Activeness flag"},
+                    {RPCResult::Type::BOOL, "internal", true, "Whether this is internal or external descriptor; defined only for active descriptors"},
+                    {RPCResult::Type::ARR_FIXED, "range", true, "Defined only for ranged descriptors", {
+                        {RPCResult::Type::NUM, "", "Range start inclusive"},
+                        {RPCResult::Type::NUM, "", "Range end inclusive"},
+                    }},
+                    {RPCResult::Type::NUM, "next", true, "The next index to generate addresses from; defined only for ranged descriptors"},
+                }},
+            }
+        },
+        RPCExamples{
+            HelpExampleCli("listdescriptors", "") + HelpExampleRpc("listdescriptors", "")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    if (!wallet) return NullUniValue;
+
+    if (!wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
+        throw JSONRPCError(RPC_WALLET_ERROR, "listdescriptors is not available for non-descriptor wallets");
+    }
+
+    LOCK(wallet->cs_wallet);
+
+    UniValue response(UniValue::VARR);
+    const auto active_spk_mans = wallet->GetActiveScriptPubKeyMans();
+    for (const auto& spk_man : wallet->GetAllScriptPubKeyMans()) {
+        const auto desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man);
+        if (!desc_spk_man) {
+            throw JSONRPCError(RPC_WALLET_ERROR, "Unexpected ScriptPubKey manager type.");
+        }
+        UniValue spk(UniValue::VOBJ);
+        LOCK(desc_spk_man->cs_desc_man);
+        const auto& wallet_descriptor = desc_spk_man->GetWalletDescriptor();
+        spk.pushKV("desc", wallet_descriptor.descriptor->ToString());
+        spk.pushKV("timestamp", wallet_descriptor.creation_time);
+        const bool active = active_spk_mans.count(desc_spk_man) != 0;
+        spk.pushKV("active", active);
+        const auto& type = wallet_descriptor.descriptor->GetOutputType();
+        if (active && type != std::nullopt) {
+            spk.pushKV("internal", wallet->GetScriptPubKeyMan(true) == desc_spk_man);
+        }
+        if (wallet_descriptor.descriptor->IsRange()) {
+            UniValue range(UniValue::VARR);
+            range.push_back(wallet_descriptor.range_start);
+            range.push_back(wallet_descriptor.range_end - 1);
+            spk.pushKV("range", range);
+            spk.pushKV("next", wallet_descriptor.next_index);
+        }
+        response.push_back(spk);
+    }
+
+    return response;
+},
+    };
+}

src/wallet/rpcwallet.cpp

@@ -4183,6 +4183,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request);
 UniValue removeprunedfunds(const JSONRPCRequest& request);
 UniValue importmulti(const JSONRPCRequest& request);
 UniValue importdescriptors(const JSONRPCRequest& request);
+RPCHelpMan listdescriptors();
 UniValue dumphdinfo(const JSONRPCRequest& request);
 UniValue importelectrumwallet(const JSONRPCRequest& request);
 
@@ -4443,6 +4444,7 @@ static const CRPCCommand commands[] =
     { "wallet",             "keypoolrefill",                    &keypoolrefill,                 {"newsize"} },
     { "wallet",             "listaddressbalances",              &listaddressbalances,           {"minamount"} },
     { "wallet",             "listaddressgroupings",             &listaddressgroupings,          {} },
+    { "wallet",             "listdescriptors",                  &listdescriptors,               {} },
     { "wallet",             "listlabels",                       &listlabels,                    {"purpose"} },
     { "wallet",             "listlockunspent",                  &listlockunspent,               {} },
     { "wallet",             "listreceivedbyaddress",            &listreceivedbyaddress,         {"minconf","addlocked","include_empty","include_watchonly","address_filter"} },

src/wallet/scriptpubkeyman.cpp

@@ -77,8 +77,8 @@ bool HaveKeys(const std::vector<valtype>& pubkeys, const LegacyScriptPubKeyMan&
 //!
 //! @param keystore            legacy key and script store
 //! @param script              script to solve
-//! @param sigversion          script type (top-level / redeemscript / witnessscript)
-//! @param recurse_scripthash  whether to recurse into nested p2sh and p2wsh
+//! @param sigversion          script type (top-level / redeemscript)
+//! @param recurse_scripthash  whether to recurse into nested p2sh
 //!                            scripts or simply treat any script that has been
 //!                            stored in the keystore as spendable
 IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& scriptPubKey, IsMineSigVersion sigversion, bool recurse_scripthash=true)
@@ -299,6 +299,7 @@ bool LegacyScriptPubKeyMan::GetReservedDestination(bool internal, CTxDestination
     if (!ReserveKeyFromKeyPool(index, keypool, internal)) {
         return false;
     }
+    // TODO: unify with bitcoin and use here GetDestinationForKey even if we have no type
     address = PKHash(keypool.vchPubKey);
     return true;
 }
@@ -610,7 +611,7 @@ bool LegacyScriptPubKeyMan::CanGetAddresses(bool internal) const
     LOCK(cs_KeyStore);
     // Check if the keypool has keys
     bool keypool_has_keys;
-    if (internal && m_storage.CanSupportFeature(FEATURE_HD)) {
+    if (internal) {
         keypool_has_keys = setInternalKeyPool.size() > 0;
     } else {
         keypool_has_keys = KeypoolCountExternalKeys() > 0;
@@ -1214,13 +1215,13 @@ bool LegacyScriptPubKeyMan::GetKeyOrigin(const CKeyID& keyID, KeyOriginInfo& inf
     return true;
 }
 
-bool LegacyScriptPubKeyMan::AddKeyOrigin(const CPubKey& pubkey, const KeyOriginInfo& info)
+bool LegacyScriptPubKeyMan::AddKeyOriginWithDB(WalletBatch& batch, const CPubKey& pubkey, const KeyOriginInfo& info)
 {
     LOCK(cs_KeyStore);
     std::copy(info.fingerprint, info.fingerprint + 4, mapKeyMetadata[pubkey.GetID()].key_origin.fingerprint);
     mapKeyMetadata[pubkey.GetID()].key_origin.path = info.path;
     mapKeyMetadata[pubkey.GetID()].has_key_origin = true;
-    return WriteKeyMetadata(mapKeyMetadata[pubkey.GetID()], pubkey, true);
+    return batch.WriteKeyMetadata(mapKeyMetadata[pubkey.GetID()], pubkey, true);
 }
 
 bool LegacyScriptPubKeyMan::GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const
@@ -1367,6 +1368,9 @@ void LegacyScriptPubKeyMan::LoadKeyPool(int64_t nIndex, const CKeyPool &keypool)
 bool LegacyScriptPubKeyMan::CanGenerateKeys() const
 {
     LOCK(cs_KeyStore);
+    // TODO : unify with bitcoin after backporting SetupGeneration
+    // return IsHDEnabled() || !m_storage.CanSupportFeature(FEATURE_HD);
+
     if (m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) || m_storage.IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET)) {
         return false;
     }
@@ -1713,6 +1717,9 @@ bool LegacyScriptPubKeyMan::ImportPrivKeys(const std::map<CKeyID, CKey>& privkey
 bool LegacyScriptPubKeyMan::ImportPubKeys(const std::vector<CKeyID>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp)
 {
     WalletBatch batch(m_storage.GetDatabase());
+    for (const auto& entry : key_origins) {
+        AddKeyOriginWithDB(batch, entry.second.first, entry.second.second);
+    }
     for (const CKeyID& id : ordered_pubkeys) {
         auto entry = pubkey_map.find(id);
         if (entry == pubkey_map.end()) {
@@ -1735,9 +1742,6 @@ bool LegacyScriptPubKeyMan::ImportPubKeys(const std::vector<CKeyID>& ordered_pub
             NotifyCanGetAddressesChanged();
         }
     }
-    for (const auto& entry : key_origins) {
-        AddKeyOrigin(entry.second.first, entry.second.second);
-    }
     return true;
 }
 

src/wallet/scriptpubkeyman.h

@@ -433,8 +433,6 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
     bool AddCScript(const CScript& redeemScript) override;
     /** Implement lookup of key origin information through wallet key metadata. */
     bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override;
-    /** Add a KeyOriginInfo to the wallet */
-    bool AddKeyOrigin(const CPubKey& pubkey, const KeyOriginInfo& info);
 
     void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool);
     bool NewKeyPool();

src/wallet/wallet.cpp

@@ -2960,23 +2960,6 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint,
     }
 
     // At this point, one input was not fully signed otherwise we would have exited already
-    // Find that input and figure out what went wrong.
-    for (unsigned int i = 0; i < tx.vin.size(); i++) {
-        // Get the prevout
-        CTxIn& txin = tx.vin[i];
-        auto coin = coins.find(txin.prevout);
-        if (coin == coins.end() || coin->second.IsSpent()) {
-            input_errors[i] = "Input not found or already spent";
-            continue;
-        }
-
-        // Check if this input is complete
-        SignatureData sigdata = DataFromTransaction(tx, i, coin->second.out);
-        if (!sigdata.complete) {
-            input_errors[i] = "Unable to sign input, missing keys";
-            continue;
-        }
-    }
     return false;
 }
 

src/wallet/walletdb.cpp

@@ -605,9 +605,6 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
             ssValue >> ser_xpub;
             CExtPubKey xpub;
             xpub.Decode(ser_xpub.data());
-            if (wss.m_descriptor_caches.count(desc_id)) {
-                wss.m_descriptor_caches[desc_id] = DescriptorCache();
-            }
             if (parent) {
                 wss.m_descriptor_caches[desc_id].CacheParentExtPubKey(key_exp_index, xpub);
             } else {