Logic running with -keypool=0 was wrong (empty keys were being returned). Fixes #445

Renames GetOrReuseKeyFromKeyPool to GetKeyFromPool, with fAllowReuse arg and bool result.

src/main.cpp

@@ -2183,7 +2183,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
 
         // Keep giving the same key to the same ip until they use it
         if (!mapReuseKey.count(pfrom->addr.ip))
-            mapReuseKey[pfrom->addr.ip] = pwalletMain->GetOrReuseKeyFromPool();
+            pwalletMain->GetKeyFromPool(mapReuseKey[pfrom->addr.ip], true);
 
         // Send back approval of order and pubkey to use
         CScript scriptPubKey;

src/rpc.cpp

@@ -331,21 +331,20 @@ Value getnewaddress(const Array& params, bool fHelp)
             "If [account] is specified (recommended), it is added to the address book "
             "so payments received with the address will be credited to [account].");
 
-    if (!pwalletMain->IsLocked())
-        pwalletMain->TopUpKeyPool();
-
-    if (pwalletMain->GetKeyPoolSize() < 1)
-        throw JSONRPCError(-12, "Error: Keypool ran out, please call keypoolrefill first");
-
     // Parse the account first so we don't generate a key if there's an error
     string strAccount;
     if (params.size() > 0)
         strAccount = AccountFromValue(params[0]);
 
-    // Generate a new key that is added to wallet
-    CBitcoinAddress address(pwalletMain->GetOrReuseKeyFromPool());
+    if (!pwalletMain->IsLocked())
+        pwalletMain->TopUpKeyPool();
+
+    // Generate a new key that is added to wallet
+    std::vector<unsigned char> newKey;
+    if (!pwalletMain->GetKeyFromPool(newKey, false))
+        throw JSONRPCError(-12, "Error: Keypool ran out, please call keypoolrefill first");
+    CBitcoinAddress address(newKey);
 
-    // This could be done in the same main CS as GetKeyFromKeyPool.
     pwalletMain->SetAddressBookName(address, strAccount);
 
     return address.ToString();
@@ -382,12 +381,9 @@ CBitcoinAddress GetAccountAddress(string strAccount, bool bForceNew=false)
     {
         if (pwalletMain->GetKeyPoolSize() < 1)
         {
-            if (bKeyUsed || bForceNew)
+            if (!pwalletMain->GetKeyFromPool(account.vchPubKey, false))
                 throw JSONRPCError(-12, "Error: Keypool ran out, please call topupkeypool first");
-        }
-        else
-        {
-            account.vchPubKey = pwalletMain->GetOrReuseKeyFromPool();
+
             pwalletMain->SetAddressBookName(CBitcoinAddress(account.vchPubKey), strAccount);
             walletdb.WriteAccount(strAccount, account);
         }

src/ui.cpp

@@ -1391,7 +1391,9 @@ void CMainFrame::OnButtonNew(wxCommandEvent& event)
         return;
 
     // Generate new key
-    strAddress = CBitcoinAddress(pwalletMain->GetOrReuseKeyFromPool()).ToString();
+    std::vector<unsigned char> newKey;
+    pwalletMain->GetKeyFromPool(newKey, true);
+    strAddress = CBitcoinAddress(newKey).ToString();
 
     if (fWasLocked)
         pwalletMain->Lock();
@@ -2826,7 +2828,9 @@ void CAddressBookDialog::OnButtonNew(wxCommandEvent& event)
             return;
 
         // Generate new key
-        strAddress = CBitcoinAddress(pwalletMain->GetOrReuseKeyFromPool()).ToString();
+        std::vector<unsigned char> newKey;
+        pwalletMain->GetKeyFromPool(newKey, true);
+        strAddress = CBitcoinAddress(newKey).ToString();
 
         if (fWasLocked)
             pwalletMain->Lock();

src/wallet.cpp

@@ -268,10 +268,14 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn)
         {
             if (txout.scriptPubKey == scriptDefaultKey)
             {
-                SetDefaultKey(GetOrReuseKeyFromPool());
+                std::vector<unsigned char> newDefaultKey;
+                if (GetKeyFromPool(newDefaultKey, false))
+                {
+                    SetDefaultKey(newDefaultKey);
                     SetAddressBookName(CBitcoinAddress(vchDefaultKey), "");
                 }
             }
+        }
 
         // Notify UI
         vWalletUpdated.push_back(hash);
@@ -1126,7 +1130,10 @@ int CWallet::LoadWallet(bool& fFirstRunRet)
         // Create new keyUser and set as default key
         RandAddSeedPerfmon();
 
-        SetDefaultKey(GetOrReuseKeyFromPool());
+        std::vector<unsigned char> newDefaultKey;
+        if (!GetKeyFromPool(newDefaultKey, false))
+            return DB_LOAD_FAIL;
+        SetDefaultKey(newDefaultKey);
         if (!SetAddressBookName(CBitcoinAddress(vchDefaultKey), ""))
             return DB_LOAD_FAIL;
     }
@@ -1269,15 +1276,25 @@ void CWallet::ReturnKey(int64 nIndex)
     printf("keypool return %"PRI64d"\n", nIndex);
 }
 
-vector<unsigned char> CWallet::GetOrReuseKeyFromPool()
+bool CWallet::GetKeyFromPool(vector<unsigned char>& result, bool fAllowReuse)
 {
     int64 nIndex = 0;
     CKeyPool keypool;
     ReserveKeyFromKeyPool(nIndex, keypool);
     if (nIndex == -1)
-        return vchDefaultKey;
+    {
+        if (fAllowReuse && !vchDefaultKey.empty())
+        {
+            result = vchDefaultKey;
+            return true;
+        }
+        if (IsLocked()) return false;
+        result = GenerateNewKey();
+        return true;
+    }
     KeepKey(nIndex);
-    return keypool.vchPubKey;
+    result = keypool.vchPubKey;
+    return true;
 }
 
 int64 CWallet::GetOldestKeyPoolTime()

src/wallet.h

@@ -85,7 +85,7 @@ public:
     void ReserveKeyFromKeyPool(int64& nIndex, CKeyPool& keypool);
     void KeepKey(int64 nIndex);
     void ReturnKey(int64 nIndex);
-    std::vector<unsigned char> GetOrReuseKeyFromPool();
+    bool GetKeyFromPool(std::vector<unsigned char> &key, bool fAllowReuse=true);
     int64 GetOldestKeyPoolTime();
 
     bool IsMine(const CTxIn& txin) const;