From 596f6460f9fd8273665c8754ccd673d93a4f25f0 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 7 Oct 2019 14:11:34 -0400 Subject: Key pool: Move CanGetAddresses call Call LegacyScriptPubKeyMan::CanGetAddresses directly instead of calling CWallet::CanGetAddresses to only query the relevant key manager This is a minor change in behavior: call now only happens if a new key needs to be reserved, since if a key is already reserved it might fail unnecessarily. This change also serves as a sanity check https://github.com/bitcoin/bitcoin/pull/16341#discussion_r331238394 --- src/wallet/scriptpubkeyman.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 3eaaf3786..9c5f2b509 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -264,6 +264,10 @@ bool LegacyScriptPubKeyMan::EncryptKeys(CKeyingMaterial& vMasterKeyIn) bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { + if (!CanGetAddresses(internal)) { + return false; + } + if (!ReserveKeyFromKeyPool(index, keypool, internal)) { return false; } -- cgit v1.2.3 From 9fcf8ce7ae02bf170b9bf0c2887fd709d752cbf7 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 19 Nov 2019 18:33:20 -0500 Subject: Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapper There is no reason to have Keep/ReturnDestination to be a wrapper for Keep/ReturnKey. Instead just make them the same function. --- src/wallet/scriptpubkeyman.cpp | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 9c5f2b509..76a678d06 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -274,16 +274,6 @@ bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool i return true; } -void LegacyScriptPubKeyMan::KeepDestination(int64_t index) -{ - KeepKey(index); -} - -void LegacyScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CPubKey& pubkey) -{ - ReturnKey(index, internal, pubkey); -} - void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script) { AssertLockHeld(cs_wallet); @@ -1096,7 +1086,7 @@ void LegacyScriptPubKeyMan::AddKeypoolPubkeyWithDB(const CPubKey& pubkey, const m_pool_key_to_index[pubkey.GetID()] = index; } -void LegacyScriptPubKeyMan::KeepKey(int64_t nIndex) +void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex) { // Remove from key pool WalletBatch batch(m_storage.GetDatabase()); @@ -1104,7 +1094,7 @@ void LegacyScriptPubKeyMan::KeepKey(int64_t nIndex) WalletLogPrintf("keypool keep %d\n", nIndex); } -void LegacyScriptPubKeyMan::ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey) +void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, const CPubKey& pubkey) { // Return to key pool { @@ -1138,7 +1128,7 @@ bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, bool internal) result = GenerateNewKey(batch, internal); return true; } - KeepKey(nIndex); + KeepDestination(nIndex); result = keypool.vchPubKey; } return true; -- cgit v1.2.3 From 65833a74076cddf986037c6eb3b29a9b9dbe31c5 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 26 Nov 2019 11:52:51 -0500 Subject: Add OutputType and CPubKey parameters to KeepDestination These need to be added so that LearnRelatedScripts can be called from within KeepDestination later. --- src/wallet/scriptpubkeyman.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 76a678d06..f7153a751 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -18,7 +18,7 @@ bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestinat // Generate a new key that is added to wallet CPubKey new_key; - if (!GetKeyFromPool(new_key)) { + if (!GetKeyFromPool(new_key, type)) { error = "Error: Keypool ran out, please call keypoolrefill first"; return false; } @@ -1086,7 +1086,7 @@ void LegacyScriptPubKeyMan::AddKeypoolPubkeyWithDB(const CPubKey& pubkey, const m_pool_key_to_index[pubkey.GetID()] = index; } -void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex) +void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex, const OutputType& type, const CPubKey& pubkey) { // Remove from key pool WalletBatch batch(m_storage.GetDatabase()); @@ -1112,7 +1112,7 @@ void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, co WalletLogPrintf("keypool return %d\n", nIndex); } -bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, bool internal) +bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, const OutputType type, bool internal) { if (!CanGetAddresses(internal)) { return false; @@ -1128,7 +1128,7 @@ bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, bool internal) result = GenerateNewKey(batch, internal); return true; } - KeepDestination(nIndex); + KeepDestination(nIndex, type, keypool.vchPubKey); result = keypool.vchPubKey; } return true; -- cgit v1.2.3 From ba41aa4969169cd73d6b4f57444ed7d8d875de10 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 7 Oct 2019 14:11:34 -0400 Subject: Key pool: Move LearnRelated and GetDestination calls Addresses are determined by LegacyScriptPubKeyMan::GetReservedDestination instead of ReserveDestination::GetReservedDestination as other ScriptPubKeyMan implementations may construct addresses differently This does not change behavior. --- src/wallet/scriptpubkeyman.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index f7153a751..4986871fb 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -262,7 +262,7 @@ bool LegacyScriptPubKeyMan::EncryptKeys(CKeyingMaterial& vMasterKeyIn) return true; } -bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) +bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) { if (!CanGetAddresses(internal)) { return false; @@ -271,6 +271,7 @@ bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool i if (!ReserveKeyFromKeyPool(index, keypool, internal)) { return false; } + address = GetDestinationForKey(keypool.vchPubKey, type); return true; } @@ -1091,6 +1092,7 @@ void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex, const OutputType& ty // Remove from key pool WalletBatch batch(m_storage.GetDatabase()); batch.ErasePool(nIndex); + LearnRelatedScripts(pubkey, type); WalletLogPrintf("keypool keep %d\n", nIndex); } -- cgit v1.2.3 From 386a994b853bc5b3a2ed0d812673465b8ffa4849 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 7 Oct 2019 14:11:34 -0400 Subject: Key pool: Change ReturnDestination interface to take address instead of key In order for ScriptPubKeyMan to be generic and work with future ScriptPubKeyMans, ScriptPubKeyMan::ReturnDestination is changed to take a CTxDestination instead of a CPubKey. Since LegacyScriptPubKeyMan still deals with keys internally, a new map m_reserved_key_to_index is added in order to track the keypool indexes that have been reserved. The CPubKey argument of KeepDestination is also removed so that it is more generic. Instead of taking a CPubKey or a CTxDestination, we just use the nIndex given to find the pubkey. --- src/wallet/scriptpubkeyman.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 4986871fb..e5b45a81a 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1087,16 +1087,20 @@ void LegacyScriptPubKeyMan::AddKeypoolPubkeyWithDB(const CPubKey& pubkey, const m_pool_key_to_index[pubkey.GetID()] = index; } -void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex, const OutputType& type, const CPubKey& pubkey) +void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex, const OutputType& type) { // Remove from key pool WalletBatch batch(m_storage.GetDatabase()); batch.ErasePool(nIndex); + CPubKey pubkey; + bool have_pk = GetPubKey(m_index_to_reserved_key.at(nIndex), pubkey); + assert(have_pk); LearnRelatedScripts(pubkey, type); + m_index_to_reserved_key.erase(nIndex); WalletLogPrintf("keypool keep %d\n", nIndex); } -void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, const CPubKey& pubkey) +void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, const CTxDestination&) { // Return to key pool { @@ -1108,7 +1112,9 @@ void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, co } else { setExternalKeyPool.insert(nIndex); } - m_pool_key_to_index[pubkey.GetID()] = nIndex; + CKeyID& pubkey_id = m_index_to_reserved_key.at(nIndex); + m_pool_key_to_index[pubkey_id] = nIndex; + m_index_to_reserved_key.erase(nIndex); NotifyCanGetAddressesChanged(); } WalletLogPrintf("keypool return %d\n", nIndex); @@ -1130,7 +1136,7 @@ bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, const OutputType typ result = GenerateNewKey(batch, internal); return true; } - KeepDestination(nIndex, type, keypool.vchPubKey); + KeepDestination(nIndex, type); result = keypool.vchPubKey; } return true; @@ -1175,6 +1181,8 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key throw std::runtime_error(std::string(__func__) + ": keypool entry invalid"); } + assert(m_index_to_reserved_key.count(nIndex) == 0); + m_index_to_reserved_key[nIndex] = keypool.vchPubKey.GetID(); m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); WalletLogPrintf("keypool reserve %d\n", nIndex); } -- cgit v1.2.3 From 886f1731bec4393dd342403ac34069a3a4f95eea Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 7 Oct 2019 14:11:34 -0400 Subject: Key pool: Fix omitted pre-split count in GetKeyPoolSize This is a bugfix: https://github.com/bitcoin/bitcoin/pull/16341#discussion_r330669214 --- src/wallet/scriptpubkeyman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/wallet/scriptpubkeyman.cpp') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index e5b45a81a..2b9e8fbf2 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -455,7 +455,7 @@ size_t LegacyScriptPubKeyMan::KeypoolCountExternalKeys() unsigned int LegacyScriptPubKeyMan::GetKeyPoolSize() const { AssertLockHeld(cs_wallet); - return setInternalKeyPool.size() + setExternalKeyPool.size(); + return setInternalKeyPool.size() + setExternalKeyPool.size() + set_pre_split_keypool.size(); } int64_t LegacyScriptPubKeyMan::GetTimeFirstKey() const -- cgit v1.2.3