From 4a0abf694ee10cf186f25a67ca35c3fce0c10874 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 5 Nov 2019 10:36:55 -0500 Subject: Pass CTxDestination to ScriptPubKeyMan::GetMetadata 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 a18edd7b383d667b15b6d4b87aa3a055a9fa5051 from https://github.com/bitcoin/bitcoin/pull/17304 Change suggested by Andrew Chow in https://github.com/bitcoin/bitcoin/pull/17304#discussion_r340345745 and https://github.com/bitcoin/bitcoin/pull/17381#issuecomment-549994944 --- src/wallet/scriptpubkeyman.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index bb13db11b..48fe1843a 100644 --- 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; } -- cgit v1.2.3 From 491a599b37f3e3a648e52aebed677ca11b0615e2 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 5 Nov 2019 10:43:36 -0500 Subject: Get rid of confusing LegacyScriptPubKeyMan::TopUpKeyPool method Previous discussion https://github.com/bitcoin/bitcoin/pull/17304#discussion_r340307903 --- src/wallet/scriptpubkeyman.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 48fe1843a..ce7cf3692 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; @@ -282,11 +282,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 +292,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 +396,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; } @@ -1029,7 +1024,7 @@ bool LegacyScriptPubKeyMan::NewKeyPool() m_pool_key_to_index.clear(); - if (!TopUpKeyPool()) { + if (!TopUp()) { return false; } WalletLogPrintf("LegacyScriptPubKeyMan::NewKeyPool rewrote keypool\n"); @@ -1037,7 +1032,7 @@ bool LegacyScriptPubKeyMan::NewKeyPool() return true; } -bool LegacyScriptPubKeyMan::TopUpKeyPool(unsigned int kpSize) +bool LegacyScriptPubKeyMan::TopUp(unsigned int kpSize) { if (!CanGenerateKeys()) { return false; @@ -1154,7 +1149,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); -- cgit v1.2.3 From bfd826a675445801adec86a469040f3ceb8172ee Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 5 Nov 2019 10:47:07 -0500 Subject: Clean up nested scope in GetReservedDestination Suggested https://github.com/bitcoin/bitcoin/pull/17304#discussion_r341194391 by Gregory Sanders Reason for keeping the `return true` `return false` verbosity is that more code will be added after the ReserveKeyFromKeyPool() call before returning. --- src/wallet/scriptpubkeyman.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index ce7cf3692..3eaaf3786 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -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; } -- cgit v1.2.3