Get rid of confusing LegacyScriptPubKeyMan::TopUpKeyPool method

ryanofsky · ryanofsky · commit 491a599b37f3 · 2019-11-05T10:43:36.000-05:00
Previous discussion #17304 (comment)
diff --git 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,15 +1024,15 @@ bool LegacyScriptPubKeyMan::NewKeyPool()
 
         m_pool_key_to_index.clear();
 
-        if (!TopUpKeyPool()) {
+        if (!TopUp()) {
             return false;
         }
         WalletLogPrintf("LegacyScriptPubKeyMan::NewKeyPool rewrote keypool\n");
     }
     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);
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
@@ -356,7 +356,6 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
 
     //! Load a keypool entry
     void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
-    bool TopUpKeyPool(unsigned int kpSize = 0);
     bool NewKeyPool();
     void MarkPreSplitKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@`
`14`	`14`	`bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error)`
`15`	`15`	`{`
`16`	`16`	`error.clear();`
`17`		`- TopUpKeyPool();`
	`17`	`+ TopUp();`
`18`	`18`
`19`	`19`	`// Generate a new key that is added to wallet`
`20`	`20`	`CPubKey new_key;`
`@@ -282,11 +282,6 @@ void LegacyScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, cons`
`282`	`282`	`ReturnKey(index, internal, pubkey);`
`283`	`283`	`}`
`284`	`284`
`285`		`-bool LegacyScriptPubKeyMan::TopUp(unsigned int size)`
`286`		`-{`
`287`		`- return TopUpKeyPool(size);`
`288`		`-}`
`289`		`-`
`290`	`285`	`void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)`
`291`	`286`	`{`
`292`	`287`	`AssertLockHeld(cs_wallet);`
`@@ -297,7 +292,7 @@ void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)`
`297`	`292`	`WalletLogPrintf("%s: Detected a used keypool key, mark all keypool key up to this key as used\n", __func__);`
`298`	`293`	`MarkReserveKeysAsUsed(mi->second);`
`299`	`294`
`300`		`- if (!TopUpKeyPool()) {`
	`295`	`+ if (!TopUp()) {`
`301`	`296`	`WalletLogPrintf("%s: Topping up keypool failed (locked wallet)\n", __func__);`
`302`	`297`	`}`
`303`	`298`	`}`
`@@ -401,7 +396,7 @@ bool LegacyScriptPubKeyMan::Upgrade(int prev_version, std::string& error)`
`401`	`396`	`}`
`402`	`397`	`// Regenerate the keypool if upgraded to HD`
`403`	`398`	`if (hd_upgrade) {`
`404`		`- if (!TopUpKeyPool()) {`
	`399`	`+ if (!TopUp()) {`
`405`	`400`	`error = _("Unable to generate keys").translated;`
`406`	`401`	`return false;`
`407`	`402`	`}`
`@@ -1029,15 +1024,15 @@ bool LegacyScriptPubKeyMan::NewKeyPool()`
`1029`	`1024`
`1030`	`1025`	`m_pool_key_to_index.clear();`
`1031`	`1026`
`1032`		`- if (!TopUpKeyPool()) {`
	`1027`	`+ if (!TopUp()) {`
`1033`	`1028`	`return false;`
`1034`	`1029`	`}`
`1035`	`1030`	`WalletLogPrintf("LegacyScriptPubKeyMan::NewKeyPool rewrote keypool\n");`
`1036`	`1031`	`}`
`1037`	`1032`	`return true;`
`1038`	`1033`	`}`
`1039`	`1034`
`1040`		`-bool LegacyScriptPubKeyMan::TopUpKeyPool(unsigned int kpSize)`
	`1035`	`+bool LegacyScriptPubKeyMan::TopUp(unsigned int kpSize)`
`1041`	`1036`	`{`
`1042`	`1037`	`if (!CanGenerateKeys()) {`
`1043`	`1038`	`return false;`
`@@ -1154,7 +1149,7 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key`
`1154`	`1149`	`{`
`1155`	`1150`	`LOCK(cs_wallet);`
`1156`	`1151`
`1157`		`- TopUpKeyPool();`
	`1152`	`+ TopUp();`
`1158`	`1153`
`1159`	`1154`	`bool fReturningInternal = fRequestedInternal;`
`1160`	`1155`	`fReturningInternal &= (IsHDEnabled() && m_storage.CanSupportFeature(FEATURE_HD_SPLIT)) \|\| m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);`