Merge #17537: wallet: Cleanup and move opportunistic and superfluous TopUp()s

6e77a7b65c keypool: Add comment about TopUp and when to use it (Andrew Chow)
ea50e34b28 keypool: Move opportunistic TopUps from LegacyScriptPubKeyMan to CWallet and ReserveDestination (Andrew Chow)
bb2c8ce23c keypool: Remove  superfluous topup from CWallet::GetNewChangeDestination (Andrew Chow)

Pull request description:

  * The `TopUp()` in `CWallet::GetNewChangeDestination` is unnecessary as currently m_spk_man calls TopUp further down the call stack inside LegacyScriptPubKeyMan::ReserveKeyFromKeyPool (called by LegacyScriptPubKeyMan::GetReservedDestination). This also lets us prepare for future changes with multiple ScriptPubKeyMans in the wallet.
  * An opportunistic `TopUp()` is moved from `LegacyScriptPubKeyMan::GetNewDestination` to `CWallet::GetNewDestination`.
  * Another opportunistic `TopUp()` is moved from `LegacyScriptPubKeyMan::ReserveKeyFromKeyPool`

  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.

  See also: https://github.com/bitcoin/bitcoin/pull/17373#discussion_r348598174

ACKs for top commit:
  instagibbs:
    utACK 6e77a7b65c only change is slight elaboration on comment
  ryanofsky:
    Code review ACK 6e77a7b65c. Only the comment changed since my previous review.

Tree-SHA512: bdfc8d303842c3fb7c3d40af7abfa6d9dac4ef71a24922bb92229674ee89bfe3113ebb46d3903ac48ef99f0a7d6eaac33282495844f2b31f91b8df55084c421f
This commit is contained in:
fanquake
2019-12-17 11:50:25 -05:00
3 changed files with 7 additions and 5 deletions

View File

@@ -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;
@@ -1153,8 +1152,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();