Merge #15749: Fix: importmulti only imports origin info for PKH outputs

b5d398772 Take non-importing keys into account for spendability warning in descriptor import (Pieter Wuille)
6e597001a Import all origin info in importmulti; even for non-importing pubkeys (Pieter Wuille)
9a93c91c8 Keep full pubkeys in FlatSigningProvider::origins (Pieter Wuille)

Pull request description:

  This fixes #15743 and #15742.

  Since #15263, pubkeys are no longer imported for non-PKH (or WPKH, or any wrapped form of those) outputs, as that would incorrectly mark outputs to single-key versions of multisig policies as watched.

  As a side effect, this change also caused origin info not to be imported anymore for multisig policies.

  Fix this by plumbing through the full pubkey information for origins in FlatSigningProvider, and then importing all origin info we have in `importmulti` (knowing more never hurts, and additional origin information has no negative consequences like importing the pubkeys themselves).

ACKs for commit b5d398:
  MeshCollider:
    utACK https://github.com/bitcoin/bitcoin/pull/15749/commits/b5d39877242504160a7f2c24c60c07a845d7a064

Tree-SHA512: 37caa2be8d01b8baa12f70a58eaa7c583f5f0afbe012e02936dd8790dc5dc852f880b77258b34ddb68cae30c029585f2d1c4f5d00015380557a1e8b471e500f3

5 files changed, 19 insertions, 11 deletions

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 43448d7222..a333d4d4ac 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -436,7 +436,7 @@ public:
         pubkeys.reserve(entries.size());
         for (auto& entry : entries) {
             pubkeys.push_back(entry.first);
-            out.origins.emplace(entry.first.GetID(), std::move(entry.second));
+            out.origins.emplace(entry.first.GetID(), std::make_pair<CPubKey, KeyOriginInfo>(CPubKey(entry.first), std::move(entry.second)));
         }
         if (m_script_arg) {
             for (const auto& subscript : subscripts) {
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 320956d0c4..36dd68a3d8 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -483,7 +483,13 @@ bool HidingSigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& inf
 
 bool FlatSigningProvider::GetCScript(const CScriptID& scriptid, CScript& script) const { return LookupHelper(scripts, scriptid, script); }
 bool FlatSigningProvider::GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const { return LookupHelper(pubkeys, keyid, pubkey); }
-bool FlatSigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const { return LookupHelper(origins, keyid, info); }
+bool FlatSigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const
+{
+    std::pair<CPubKey, KeyOriginInfo> out;
+    bool ret = LookupHelper(origins, keyid, out);
+    if (ret) info = std::move(out.second);
+    return ret;
+}
 bool FlatSigningProvider::GetKey(const CKeyID& keyid, CKey& key) const { return LookupHelper(keys, keyid, key); }
 
 FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvider& b)
diff --git a/src/script/sign.h b/src/script/sign.h
index 491fb54c45..f746325b90 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -77,7 +77,7 @@ struct FlatSigningProvider final : public SigningProvider
 {
     std::map<CScriptID, CScript> scripts;
     std::map<CKeyID, CPubKey> pubkeys;
-    std::map<CKeyID, KeyOriginInfo> origins;
+    std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>> origins;
     std::map<CKeyID, CKey> keys;
 
     bool GetCScript(const CScriptID& scriptid, CScript& script) const override;
diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp
index ff2b8d4fc9..826615cb51 100644
--- a/src/test/descriptor_tests.cpp
+++ b/src/test/descriptor_tests.cpp
@@ -154,8 +154,8 @@ void Check(const std::string& prv, const std::string& pub, int flags, const std:
             // Test whether the observed key path is present in the 'paths' variable (which contains expected, unobserved paths),
             // and then remove it from that set.
             for (const auto& origin : script_provider.origins) {
-                BOOST_CHECK_MESSAGE(paths.count(origin.second.path), "Unexpected key path: " + prv);
-                left_paths.erase(origin.second.path);
+                BOOST_CHECK_MESSAGE(paths.count(origin.second.second.path), "Unexpected key path: " + prv);
+                left_paths.erase(origin.second.second.path);
             }
         }
     }
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 9c5dae3623..c339e111ba 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -900,7 +900,7 @@ struct ImportData
     // Output data
     std::set<CScript> import_scripts;
     std::map<CKeyID, bool> used_keys; //!< Import these private keys if available (the value indicates whether if the key is required for solvability)
-    std::map<CKeyID, KeyOriginInfo> key_origins;
+    std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>> key_origins;
 };
 
 enum class ScriptContext
@@ -1197,6 +1197,9 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
     bool spendable = std::all_of(pubkey_map.begin(), pubkey_map.end(),
         [&](const std::pair<CKeyID, CPubKey>& used_key) {
             return privkey_map.count(used_key.first) > 0;
+        }) && std::all_of(import_data.key_origins.begin(), import_data.key_origins.end(),
+        [&](const std::pair<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& entry) {
+            return privkey_map.count(entry.first) > 0;
         });
     if (!watch_only && !spendable) {
         warnings.push_back("Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag.");
@@ -1274,7 +1277,10 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
                  throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
              }
              pwallet->UpdateTimeFirstKey(timestamp);
-         }
+        }
+        for (const auto& entry : import_data.key_origins) {
+            pwallet->AddKeyOrigin(entry.second.first, entry.second.second);
+        }
         for (const CKeyID& id : ordered_pubkeys) {
             auto entry = pubkey_map.find(id);
             if (entry == pubkey_map.end()) {
@@ -1285,10 +1291,6 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
              if (!pwallet->GetPubKey(id, temp) && !pwallet->AddWatchOnly(GetScriptForRawPubKey(pubkey), timestamp)) {
                 throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
             }
-            const auto& key_orig_it = import_data.key_origins.find(id);
-            if (key_orig_it != import_data.key_origins.end()) {
-                pwallet->AddKeyOrigin(pubkey, key_orig_it->second);
-            }
             pwallet->mapKeyMetadata[id].nCreateTime = timestamp;
 
             // Add to keypool only works with pubkeys