From 9c8eca7704e88b3f4ee38cf85bef0f1febc440e5 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 8 Dec 2017 12:50:46 -0500 Subject: [PATCH] Split up key and script metadata for better type safety Suggested by Matt Corallo https://github.com/bitcoin/bitcoin/pull/11403#discussion_r155599383 Combining the maps was probably never a good arrangement but is more problematic now in presence of WitnessV0ScriptHash and WitnessV0KeyHash types. --- src/rpc/misc.cpp | 27 +++++++++++++++++---------- src/wallet/wallet.cpp | 14 +++++++++++--- src/wallet/wallet.h | 11 +++++++---- src/wallet/walletdb.cpp | 30 +++++++++++++----------------- 4 files changed, 48 insertions(+), 34 deletions(-) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index c511aa0eb..327af2e23 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -187,17 +187,24 @@ UniValue validateaddress(const JSONRPCRequest& request) ret.push_back(Pair("account", pwallet->mapAddressBook[dest].name)); } if (pwallet) { - const auto& meta = pwallet->mapKeyMetadata; - const CKeyID *keyID = boost::get(&dest); - auto it = keyID ? meta.find(*keyID) : meta.end(); - if (it == meta.end()) { - it = meta.find(CScriptID(scriptPubKey)); + const CKeyMetadata* meta = nullptr; + if (const CKeyID* key_id = boost::get(&dest)) { + auto it = pwallet->mapKeyMetadata.find(*key_id); + if (it != pwallet->mapKeyMetadata.end()) { + meta = &it->second; + } + } + if (!meta) { + auto it = pwallet->m_script_metadata.find(CScriptID(scriptPubKey)); + if (it != pwallet->m_script_metadata.end()) { + meta = &it->second; + } } - if (it != meta.end()) { - ret.push_back(Pair("timestamp", it->second.nCreateTime)); - if (!it->second.hdKeypath.empty()) { - ret.push_back(Pair("hdkeypath", it->second.hdKeypath)); - ret.push_back(Pair("hdmasterkeyid", it->second.hdMasterKeyID.GetHex())); + if (meta) { + ret.push_back(Pair("timestamp", meta->nCreateTime)); + if (!meta->hdKeypath.empty()) { + ret.push_back(Pair("hdkeypath", meta->hdKeypath)); + ret.push_back(Pair("hdmasterkeyid", meta->hdMasterKeyID.GetHex())); } } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ee1894501..ed1d67b6f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -284,7 +284,7 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, } } -bool CWallet::LoadKeyMetadata(const CTxDestination& keyID, const CKeyMetadata &meta) +bool CWallet::LoadKeyMetadata(const CKeyID& keyID, const CKeyMetadata &meta) { AssertLockHeld(cs_wallet); // mapKeyMetadata UpdateTimeFirstKey(meta.nCreateTime); @@ -292,6 +292,14 @@ bool CWallet::LoadKeyMetadata(const CTxDestination& keyID, const CKeyMetadata &m return true; } +bool CWallet::LoadScriptMetadata(const CScriptID& script_id, const CKeyMetadata &meta) +{ + AssertLockHeld(cs_wallet); // m_script_metadata + UpdateTimeFirstKey(meta.nCreateTime); + m_script_metadata[script_id] = meta; + return true; +} + bool CWallet::LoadCryptedKey(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret) { return CCryptoKeyStore::AddCryptedKey(vchPubKey, vchCryptedSecret); @@ -340,7 +348,7 @@ bool CWallet::AddWatchOnly(const CScript& dest) { if (!CCryptoKeyStore::AddWatchOnly(dest)) return false; - const CKeyMetadata& meta = mapKeyMetadata[CScriptID(dest)]; + const CKeyMetadata& meta = m_script_metadata[CScriptID(dest)]; UpdateTimeFirstKey(meta.nCreateTime); NotifyWatchonlyChanged(true); return CWalletDB(*dbw).WriteWatchOnly(dest, meta); @@ -348,7 +356,7 @@ bool CWallet::AddWatchOnly(const CScript& dest) bool CWallet::AddWatchOnly(const CScript& dest, int64_t nCreateTime) { - mapKeyMetadata[CScriptID(dest)].nCreateTime = nCreateTime; + m_script_metadata[CScriptID(dest)].nCreateTime = nCreateTime; return AddWatchOnly(dest); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1bd0be7bd..93d1857c7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -761,9 +761,11 @@ public: void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool); - // Map from Key ID (for regular keys) or Script ID (for watch-only keys) to - // key metadata. - std::map mapKeyMetadata; + // Map from Key ID to key metadata. + std::map mapKeyMetadata; + + // Map from Script ID to key metadata (for watch-only keys). + std::map m_script_metadata; typedef std::map MasterKeyMap; MasterKeyMap mapMasterKeys; @@ -874,7 +876,8 @@ public: //! Adds a key to the store, without saving it to disk (used by LoadWallet) bool LoadKey(const CKey& key, const CPubKey &pubkey) { return CCryptoKeyStore::AddKeyPubKey(key, pubkey); } //! Load metadata (used by LoadWallet) - bool LoadKeyMetadata(const CTxDestination& pubKey, const CKeyMetadata &metadata); + bool LoadKeyMetadata(const CKeyID& keyID, const CKeyMetadata &metadata); + bool LoadScriptMetadata(const CScriptID& script_id, const CKeyMetadata &metadata); bool LoadMinVersion(int nVersion) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; } void UpdateTimeFirstKey(int64_t nCreateTime); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5116d6419..efc50f72e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -423,27 +423,23 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } wss.fIsEncrypted = true; } - else if (strType == "keymeta" || strType == "watchmeta") + else if (strType == "keymeta") { - CTxDestination keyID; - if (strType == "keymeta") - { - CPubKey vchPubKey; - ssKey >> vchPubKey; - keyID = vchPubKey.GetID(); - } - else if (strType == "watchmeta") - { - CScript script; - ssKey >> script; - keyID = CScriptID(script); - } - + CPubKey vchPubKey; + ssKey >> vchPubKey; CKeyMetadata keyMeta; ssValue >> keyMeta; wss.nKeyMeta++; - - pwallet->LoadKeyMetadata(keyID, keyMeta); + pwallet->LoadKeyMetadata(vchPubKey.GetID(), keyMeta); + } + else if (strType == "watchmeta") + { + CScript script; + ssKey >> script; + CKeyMetadata keyMeta; + ssValue >> keyMeta; + wss.nKeyMeta++; + pwallet->LoadScriptMetadata(CScriptID(script), keyMeta); } else if (strType == "defaultkey") {