Pass CTxDestination to ScriptPubKeyMan::GetMetadata

ryanofsky · ryanofsky · commit 4a0abf694ee1 · 2019-11-05T10:36:55.000-05:00
Pass CTxDestination instead of more ambiguous uint160 hash value. This is more type safe and more efficient since it avoids doing map lookups that will always fail and were not done previously before a18edd7 from bitcoin#17304 Change suggested by Andrew Chow <achow101-github@achow101.com> in bitcoin#17304 (comment) and bitcoin#17381 (comment)
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -3765,15 +3765,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
 
     ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan();
     if (spk_man) {
-        CKeyID key_id = GetKeyForDestination(*provider, dest);
-        const CKeyMetadata* meta = nullptr;
-        if (!key_id.IsNull()) {
-            meta = spk_man->GetMetadata(key_id);
-        }
-        if (!meta) {
-            meta = spk_man->GetMetadata(CScriptID(scriptPubKey));
-        }
-        if (meta) {
+        if (const CKeyMetadata* meta = spk_man->GetMetadata(dest)) {
             ret.pushKV("timestamp", meta->nCreateTime);
             if (meta->has_key_origin) {
                 ret.pushKV("hdkeypath", WriteHDKeypath(meta->key_origin.path));
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
@@ -476,18 +476,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;
 }
 
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
@@ -186,7 +186,8 @@ class ScriptPubKeyMan
 
     virtual int64_t GetTimeFirstKey() const { return 0; }
 
-    virtual const CKeyMetadata* GetMetadata(uint160 id) const { return nullptr; }
+    //! Return address metadata
+    virtual const CKeyMetadata* GetMetadata(const CTxDestination& dest) const { return nullptr; }
 };
 
 class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProvider
@@ -302,7 +303,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
 
     int64_t GetTimeFirstKey() const override;
 
-    const CKeyMetadata* GetMetadata(uint160 id) const override;
+    const CKeyMetadata* GetMetadata(const CTxDestination& dest) const override;
 
     bool CanGetAddresses(bool internal = false) override;
 
