Merge dashpay#7025: backport: bitcoin#22323, bitcoin#22929, bitcoin#23172, bitcoin#23202, bitcoin#23659, bitcoin#23688, bitcoin#23835, bitcoin#24083, bitcoin#25679, bitcoin#25931, bitcoin#26156 (improvements for external signer: rpc, tests)

d59ce85 Merge bitcoin#24083: Revert "Add to spends only transcations from me" (Andrew Chow)
702813f Merge bitcoin#26156: test: check that `listdescriptors` descriptor strings are sorted (MacroFake)
59ce4ca fix: re-order fields of output of RPC listdescriptors accordingly doc (Konstantin Akimov)
b50a1f9 Merge bitcoin#25931: rpc: sort listdescriptors result (Andrew Chow)
cd07de4 Merge bitcoin#23202: wallet: allow psbtbumpfee to work with txs with external inputs (fanquake)
4e61c42 Merge bitcoin#25679: wallet: Correctly identify external inputs that are also in the wallet (fanquake)
a97cc0f Merge bitcoin#23835: test: check for invalid listtransactions RPC parameters (MarcoFalke)
7f66259 Merge bitcoin#23688: test: remove unneeded sync_all() calls in wallet_listtransactions.py (MarcoFalke)
8d760a5 Merge bitcoin#23659: test: fix: remove outdated TestNode.generate calls (fanquake)
0dff162 Merge bitcoin#22929: wallet: Automatically add receiving destinations to the address book (W. J. van der Laan)
c19a4b5 Merge bitcoin#23172: doc: Extract FundTxDoc in rpcwallet (fanquake)
9bef4ce partial Merge bitcoin#22323: doc: spelling update (MarcoFalke)

Pull request description:

  ## What was done?

  More backports to improve support of External signer.
  This PR have mostly backports related to tests and RPC

  `listdescriptors` returns descriptors now sorted. It's not a breaking change, because before they has been returned in "undefined" sorting order.

  ## How Has This Been Tested?
  Run unit & 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 _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK d59ce85

Tree-SHA512: 3ba3fcb07dcb9478ee5eb425e514c3cd3234b51d8b26fd370df65f1cb2f1abd5fe93c5fd6258e221ebee9330f106aaa1442492a6611b4464ebc70ea0a9f4bb64

Author: PastaPastaPasta

src/node/interfaces.cpp

@@ -580,7 +580,7 @@ class NodeImpl : public Node
         }
         return result;
 #else
-        // This result is undistinguisable from a succesful call that returns
+        // This result is indistinguishable from a successful call that returns
         // no signers. For the current GUI this doesn't matter, because the wallet
         // creation dialog disables the external signer checkbox in both
         // cases. The return type could be changed to std::optional<std::vector>

src/script/interpreter.h

@@ -192,10 +192,10 @@ using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CM
 class DeferringSignatureChecker : public BaseSignatureChecker
 {
 protected:
-    BaseSignatureChecker& m_checker;
+    const BaseSignatureChecker& m_checker;
 
 public:
-    DeferringSignatureChecker(BaseSignatureChecker& checker) : m_checker(checker) {}
+    DeferringSignatureChecker(const BaseSignatureChecker& checker) : m_checker(checker) {}
 
     bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override
     {

src/script/sign.cpp

@@ -350,8 +350,11 @@ class DummySignatureChecker final : public BaseSignatureChecker
     DummySignatureChecker() = default;
     bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override { return true; }
 };
-const DummySignatureChecker DUMMY_CHECKER;
+}
+
+const BaseSignatureChecker& DUMMY_CHECKER = DummySignatureChecker();
 
+namespace {
 class DummySignatureCreator final : public BaseSignatureCreator {
 private:
     char m_r_len = 32;

src/script/sign.h

@@ -50,6 +50,8 @@ class MutableTransactionSignatureCreator : public BaseSignatureCreator
     bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override;
 };
 
+/** A signature checker that accepts all signatures */
+extern const BaseSignatureChecker& DUMMY_CHECKER;
 /** A signature creator that just produces 71-byte empty signatures. */
 extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR;
 /** A signature creator that just produces 72-byte empty signatures. */

src/wallet/rpc/addresses.cpp

@@ -506,7 +506,12 @@ RPCHelpMan getaddressinfo()
         ret.pushKV("desc", InferDescriptor(scriptPubKey, *provider)->ToString());
     }
 
-    DescriptorScriptPubKeyMan* desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(pwallet->GetScriptPubKeyMan(scriptPubKey));
+    const auto& spk_mans = pwallet->GetScriptPubKeyMans(scriptPubKey);
+    // In most cases there is only one matching ScriptPubKey manager and we can't resolve ambiguity in a better way
+    ScriptPubKeyMan* spk_man{nullptr};
+    if (spk_mans.size()) spk_man = *spk_mans.begin();
+
+    DescriptorScriptPubKeyMan* desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man);
     if (desc_spk_man) {
         std::string desc_str;
         if (desc_spk_man->GetDescriptorString(desc_str, /*priv=*/false)) {
@@ -521,7 +526,6 @@ RPCHelpMan getaddressinfo()
 
     ret.pushKV("ischange", ScriptIsChange(*pwallet, scriptPubKey));
 
-    ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey);
     if (spk_man) {
         if (const std::unique_ptr<CKeyMetadata> meta = spk_man->GetMetadata(dest)) {
             ret.pushKV("timestamp", meta->nCreateTime);

src/wallet/rpc/backup.cpp

@@ -1987,7 +1987,7 @@ RPCHelpMan listdescriptors()
         },
         RPCResult{RPCResult::Type::OBJ, "", "", {
             {RPCResult::Type::STR, "wallet_name", "Name of wallet this operation was performed on"},
-            {RPCResult::Type::ARR, "descriptors", "Array of descriptor objects",
+            {RPCResult::Type::ARR, "descriptors", "Array of descriptor objects (sorted by descriptor string representation)",
             {
                 {RPCResult::Type::OBJ, "", "", {
                     {RPCResult::Type::STR, "desc", "Descriptor string representation"},
@@ -2026,51 +2026,85 @@ RPCHelpMan listdescriptors()
 
     LOCK(wallet->cs_wallet);
 
-    UniValue descriptors(UniValue::VARR);
     const auto active_spk_mans = wallet->GetActiveScriptPubKeyMans();
+
+    struct WalletDescInfo {
+        std::string descriptor;
+        uint64_t creation_time;
+        bool active;
+        std::optional<bool> internal;
+        std::optional<std::pair<int64_t,int64_t>> range;
+        int64_t next_index;
+        bool is_coinjoin;
+        SecureString mnemonic;
+        SecureString mnemonic_passphrase;
+    };
+
+    std::vector<WalletDescInfo> wallet_descriptors;
     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();
         std::string descriptor;
-
         if (!desc_spk_man->GetDescriptorString(descriptor, priv)) {
             throw JSONRPCError(RPC_WALLET_ERROR, "Can't get descriptor string.");
         }
+        SecureString mnemonic;
+        SecureString mnemonic_passphrase;
         if (priv) {
-            SecureString mnemonic;
-            SecureString mnemonic_passphrase;
-            if (desc_spk_man->GetMnemonicString(mnemonic, mnemonic_passphrase) && !mnemonic.empty()) {
-                spk.pushKV("mnemonic", mnemonic);
-                spk.pushKV("mnemonicpassphrase", mnemonic_passphrase);
+            if (!desc_spk_man->GetMnemonicString(mnemonic, mnemonic_passphrase)) {
+                mnemonic.clear();
+                mnemonic_passphrase.clear();
             }
         }
-        spk.pushKV("desc", descriptor);
-        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 (type != std::nullopt) {
-            std::string match = strprintf("/%d'/%s'/4'/0'", BIP32_PURPOSE_FEATURE, Params().ExtCoinType());
-            bool is_cj = descriptor.find(match) != std::string::npos;
-            if (is_cj) {
-                spk.pushKV("coinjoin", is_cj);
-            }
+        std::string match = strprintf("/%d'/%s'/4'/0'", BIP32_PURPOSE_FEATURE, Params().ExtCoinType());
+        bool is_cj = type != std::nullopt && descriptor.find(match) != std::string::npos;
+
+        const bool is_range = wallet_descriptor.descriptor->IsRange();
+        wallet_descriptors.push_back({
+            descriptor,
+            wallet_descriptor.creation_time,
+            active_spk_mans.count(desc_spk_man) != 0,
+            wallet->IsInternalScriptPubKeyMan(desc_spk_man),
+            is_range ? std::optional(std::make_pair(wallet_descriptor.range_start, wallet_descriptor.range_end)) : std::nullopt,
+            wallet_descriptor.next_index,
+            is_cj,
+            mnemonic,
+            mnemonic_passphrase,
+        });
+    }
+
+    std::sort(wallet_descriptors.begin(), wallet_descriptors.end(), [](const auto& a, const auto& b) {
+        return a.descriptor < b.descriptor;
+    });
+
+    UniValue descriptors(UniValue::VARR);
+    for (const WalletDescInfo& info : wallet_descriptors) {
+        UniValue spk(UniValue::VOBJ);
+        spk.pushKV("desc", info.descriptor);
+        if (!info.mnemonic.empty()) {
+            spk.pushKV("mnemonic", info.mnemonic);
+            spk.pushKV("mnemonicpassphrase", info.mnemonic_passphrase);
         }
-        if (wallet_descriptor.descriptor->IsRange()) {
+        spk.pushKV("timestamp", info.creation_time);
+        spk.pushKV("active", info.active);
+        if (info.internal.has_value()) {
+            spk.pushKV("internal", info.internal.value());
+        }
+        if (info.range.has_value()) {
             UniValue range(UniValue::VARR);
-            range.push_back(wallet_descriptor.range_start);
-            range.push_back(wallet_descriptor.range_end - 1);
+            range.push_back(info.range->first);
+            range.push_back(info.range->second - 1);
+            if (info.is_coinjoin) {
+                spk.pushKV("coinjoin", info.is_coinjoin);
+            }
             spk.pushKV("range", range);
-            spk.pushKV("next", wallet_descriptor.next_index);
-            spk.pushKV("next_index", wallet_descriptor.next_index);
+            spk.pushKV("next", info.next_index);
+            spk.pushKV("next_index", info.next_index);
         }
         descriptors.push_back(spk);
     }