From bb2c8ce23c9d7ba8d0e5538243e07218443c85b4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 20 Nov 2019 12:38:44 -0500 Subject: keypool: Remove superfluous topup from CWallet::GetNewChangeDestination This does not change behavior. This TopUp() is unnecessary as currently m_spk_man calls TopUp further down the call stack inside LegacyScriptPubKeyMan::ReserveKeyFromKeyPool (called by LegacyScriptPubKeyMan::GetReservedDestination) By removing this here, we also prepare for future changes where CWallet has multiple ScriptPubKeyMans instead of m_spk_man. --- src/wallet/wallet.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'src') diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b1e1385ca3..65ac679825 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3122,8 +3122,6 @@ bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& des { error.clear(); - m_spk_man->TopUp(); - ReserveDestination reservedest(this, type); if (!reservedest.GetReservedDestination(dest, true)) { error = "Error: Keypool ran out, please call keypoolrefill first"; -- cgit v1.2.3 From ea50e34b287e0da0806c1116bb55ade730e8ff6c Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 20 Nov 2019 12:42:10 -0500 Subject: keypool: Move opportunistic TopUps from LegacyScriptPubKeyMan to CWallet and ReserveDestination An opportunistic TopUp is moved from LegacyScriptPubKeyMan::GetNewDestination to CWallet::GetNewDestination. Another opportunistic TopUp is moved from LegacyScriptPubKeyMan::ReserveKeyFromKeyPool (called by LegacyScriptPubKeyMan::GetReservedDestination) to ReserveDestination::GetReservedDestination. Moving opportunistic TopUps ensures that ScriptPubKeyMans will always be topped up before requesting Destinations from them as we cannot always rely on future ScriptPubKeyMan implementaions topping up internally. As such, it is also unnecessary to keep the TopUp calls in the LegacyScriptPubKeyMan functions so they are moved. This does not change behavior as TopUp calls are moved up the call stack. --- src/wallet/scriptpubkeyman.cpp | 3 --- src/wallet/wallet.cpp | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 3eaaf3786c..6ed0a6a6ad 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -14,7 +14,6 @@ bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) { error.clear(); - TopUp(); // Generate a new key that is added to wallet CPubKey new_key; @@ -1147,8 +1146,6 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key { LOCK(cs_wallet); - TopUp(); - bool fReturningInternal = fRequestedInternal; fReturningInternal &= (IsHDEnabled() && m_storage.CanSupportFeature(FEATURE_HD_SPLIT)) || m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); bool use_split_keypool = set_pre_split_keypool.empty(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 65ac679825..e8abb437e9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3109,6 +3109,7 @@ bool CWallet::GetNewDestination(const OutputType type, const std::string label, bool result = false; auto spk_man = m_spk_man.get(); if (spk_man) { + spk_man->TopUp(); result = spk_man->GetNewDestination(type, dest, error); } if (result) { @@ -3302,6 +3303,8 @@ bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool inter if (nIndex == -1) { + m_spk_man->TopUp(); + CKeyPool keypool; if (!m_spk_man->GetReservedDestination(type, internal, nIndex, keypool)) { return false; -- cgit v1.2.3 From 6e77a7b65cda1b46ce42f0c99ca91562255aeb28 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 20 Nov 2019 15:41:20 -0500 Subject: keypool: Add comment about TopUp and when to use it --- src/wallet/scriptpubkeyman.h | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 4f17156792..37b34223d9 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -154,6 +154,10 @@ public: virtual void KeepDestination(int64_t index) {} virtual void ReturnDestination(int64_t index, bool internal, const CPubKey& pubkey) {} + /** Fills internal address pool. Use within ScriptPubKeyMan implementations should be used sparingly and only + * when something from the address pool is removed, excluding GetNewDestination and GetReservedDestination. + * External wallet code is primarily responsible for topping up prior to fetching new addresses + */ virtual bool TopUp(unsigned int size = 0) { return false; } //! Mark unused addresses as being used -- cgit v1.2.3