Merge #17381: LegacyScriptPubKeyMan code cleanups

05b224a175065aee4d6d9c471722bc4503f01fdf Add missing SetupGeneration error handling in EncryptWallet (Russell Yanofsky)
bfd826a675445801adec86a469040f3ceb8172ee Clean up nested scope in GetReservedDestination (Russell Yanofsky)
491a599b37f3e3a648e52aebed677ca11b0615e2 Get rid of confusing LegacyScriptPubKeyMan::TopUpKeyPool method (Russell Yanofsky)
4a0abf694ee10cf186f25a67ca35c3fce0c10874 Pass CTxDestination to ScriptPubKeyMan::GetMetadata (Russell Yanofsky)
b07b07cd8779355ba1dd16e7eb4af42e0ae1c587 Add EnsureLegacyScriptPubKeyMan and use in rpcwallet.cpp (Russell Yanofsky)

Pull request description:

  This PR implements suggested code cleanups from #17300 and #17304 review comments

ACKs for top commit:
  Sjors:
    re-ACK 05b224a
  laanwj:
    Code review ACK 05b224a175065aee4d6d9c471722bc4503f01fdf

Tree-SHA512: 12fd86637088515b744c028e0501c5d21a9cf9ee9c9cfd70e9cb65d44611ea5643abd5f6f101105caa5aff015d74de606f074f08af7dae8429f929d21288ab45

1 files changed, 22 insertions, 23 deletions

diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index bb13db11b..3eaaf3786 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -14,7 +14,7 @@
 bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error)
 {
     error.clear();
-    TopUpKeyPool();
+    TopUp();
 
     // Generate a new key that is added to wallet
     CPubKey new_key;
@@ -264,10 +264,8 @@ bool LegacyScriptPubKeyMan::EncryptKeys(CKeyingMaterial& vMasterKeyIn)
 
 bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool)
 {
-    {
-        if (!ReserveKeyFromKeyPool(index, keypool, internal)) {
-            return false;
-        }
+    if (!ReserveKeyFromKeyPool(index, keypool, internal)) {
+        return false;
     }
     return true;
 }
@@ -282,11 +280,6 @@ void LegacyScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, cons
     ReturnKey(index, internal, pubkey);
 }
 
-bool LegacyScriptPubKeyMan::TopUp(unsigned int size)
-{
-    return TopUpKeyPool(size);
-}
-
 void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
 {
     AssertLockHeld(cs_wallet);
@@ -297,7 +290,7 @@ void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
             WalletLogPrintf("%s: Detected a used keypool key, mark all keypool key up to this key as used\n", __func__);
             MarkReserveKeysAsUsed(mi->second);
 
-            if (!TopUpKeyPool()) {
+            if (!TopUp()) {
                 WalletLogPrintf("%s: Topping up keypool failed (locked wallet)\n", __func__);
             }
         }
@@ -401,7 +394,7 @@ bool LegacyScriptPubKeyMan::Upgrade(int prev_version, std::string& error)
     }
     // Regenerate the keypool if upgraded to HD
     if (hd_upgrade) {
-        if (!TopUpKeyPool()) {
+        if (!TopUp()) {
             error = _("Unable to generate keys").translated;
             return false;
         }
@@ -476,18 +469,24 @@ int64_t LegacyScriptPubKeyMan::GetTimeFirstKey() const
     return nTimeFirstKey;
 }
 
-const CKeyMetadata* LegacyScriptPubKeyMan::GetMetadata(uint160 id) const
+const CKeyMetadata* LegacyScriptPubKeyMan::GetMetadata(const CTxDestination& dest) const
 {
     AssertLockHeld(cs_wallet);
-    auto it = mapKeyMetadata.find(CKeyID(id));
-    if (it != mapKeyMetadata.end()) {
-        return &it->second;
-    } else {
-        auto it2 = m_script_metadata.find(CScriptID(id));
-        if (it2 != m_script_metadata.end()) {
-            return &it2->second;
+
+    CKeyID key_id = GetKeyForDestination(*this, dest);
+    if (!key_id.IsNull()) {
+        auto it = mapKeyMetadata.find(key_id);
+        if (it != mapKeyMetadata.end()) {
+            return &it->second;
         }
     }
+
+    CScript scriptPubKey = GetScriptForDestination(dest);
+    auto it = m_script_metadata.find(CScriptID(scriptPubKey));
+    if (it != m_script_metadata.end()) {
+        return &it->second;
+    }
+
     return nullptr;
 }
 
@@ -1023,7 +1022,7 @@ bool LegacyScriptPubKeyMan::NewKeyPool()
 
         m_pool_key_to_index.clear();
 
-        if (!TopUpKeyPool()) {
+        if (!TopUp()) {
             return false;
         }
         WalletLogPrintf("LegacyScriptPubKeyMan::NewKeyPool rewrote keypool\n");
@@ -1031,7 +1030,7 @@ bool LegacyScriptPubKeyMan::NewKeyPool()
     return true;
 }
 
-bool LegacyScriptPubKeyMan::TopUpKeyPool(unsigned int kpSize)
+bool LegacyScriptPubKeyMan::TopUp(unsigned int kpSize)
 {
     if (!CanGenerateKeys()) {
         return false;
@@ -1148,7 +1147,7 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key
     {
         LOCK(cs_wallet);
 
-        TopUpKeyPool();
+        TopUp();
 
         bool fReturningInternal = fRequestedInternal;
         fReturningInternal &= (IsHDEnabled() && m_storage.CanSupportFeature(FEATURE_HD_SPLIT)) || m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);