From 7db3b75b3e38c2088596f49cb51fe1c9c7e8b433 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Fri, 12 Aug 2011 16:32:07 -0400 Subject: [PATCH 1/3] 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 | 2 +- src/rpc.cpp | 22 +++++++++------------- src/ui.cpp | 8 ++++++-- src/wallet.cpp | 31 ++++++++++++++++++++++++------- src/wallet.h | 2 +- 5 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 36fdaa72f..203b8d5f9 100644 --- a/src/main.cpp +++ b/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; diff --git a/src/rpc.cpp b/src/rpc.cpp index 5eb5669ac..161606af3 100644 --- a/src/rpc.cpp +++ b/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]); + if (!pwalletMain->IsLocked()) + pwalletMain->TopUpKeyPool(); + // Generate a new key that is added to wallet - CBitcoinAddress address(pwalletMain->GetOrReuseKeyFromPool()); + std::vector 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); } diff --git a/src/ui.cpp b/src/ui.cpp index 5ca666192..c820cf7df 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -1391,7 +1391,9 @@ void CMainFrame::OnButtonNew(wxCommandEvent& event) return; // Generate new key - strAddress = CBitcoinAddress(pwalletMain->GetOrReuseKeyFromPool()).ToString(); + std::vector 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 newKey; + pwalletMain->GetKeyFromPool(newKey, true); + strAddress = CBitcoinAddress(newKey).ToString(); if (fWasLocked) pwalletMain->Lock(); diff --git a/src/wallet.cpp b/src/wallet.cpp index 1daec98d3..e861416e5 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -268,8 +268,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn) { if (txout.scriptPubKey == scriptDefaultKey) { - SetDefaultKey(GetOrReuseKeyFromPool()); - SetAddressBookName(CBitcoinAddress(vchDefaultKey), ""); + std::vector newDefaultKey; + if (GetKeyFromPool(newDefaultKey, false)) + { + SetDefaultKey(newDefaultKey); + SetAddressBookName(CBitcoinAddress(vchDefaultKey), ""); + } } } @@ -1126,7 +1130,10 @@ int CWallet::LoadWallet(bool& fFirstRunRet) // Create new keyUser and set as default key RandAddSeedPerfmon(); - SetDefaultKey(GetOrReuseKeyFromPool()); + std::vector 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 CWallet::GetOrReuseKeyFromPool() +bool CWallet::GetKeyFromPool(vector& result, bool fAllowReuse) { int64 nIndex = 0; CKeyPool keypool; ReserveKeyFromKeyPool(nIndex, keypool); - if(nIndex == -1) - return vchDefaultKey; + if (nIndex == -1) + { + 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() diff --git a/src/wallet.h b/src/wallet.h index 032284dd3..1dd2e5126 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -85,7 +85,7 @@ public: void ReserveKeyFromKeyPool(int64& nIndex, CKeyPool& keypool); void KeepKey(int64 nIndex); void ReturnKey(int64 nIndex); - std::vector GetOrReuseKeyFromPool(); + bool GetKeyFromPool(std::vector &key, bool fAllowReuse=true); int64 GetOldestKeyPoolTime(); bool IsMine(const CTxIn& txin) const; From 123e5bd9982376b5feba8c32c67ff0286697a867 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Mon, 15 Aug 2011 11:07:25 -0400 Subject: [PATCH 2/3] Fix RPC call name in error message. --- src/rpc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc.cpp b/src/rpc.cpp index 161606af3..ec2bb2a97 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -382,7 +382,7 @@ CBitcoinAddress GetAccountAddress(string strAccount, bool bForceNew=false) if (pwalletMain->GetKeyPoolSize() < 1) { if (!pwalletMain->GetKeyFromPool(account.vchPubKey, false)) - throw JSONRPCError(-12, "Error: Keypool ran out, please call topupkeypool first"); + throw JSONRPCError(-12, "Error: Keypool ran out, please call keypoolrefill first"); pwalletMain->SetAddressBookName(CBitcoinAddress(account.vchPubKey), strAccount); walletdb.WriteAccount(strAccount, account); From ed02c95d505ce48451b600ff40720841a000fd50 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Thu, 1 Sep 2011 10:58:08 -0400 Subject: [PATCH 3/3] obtain cs_wallet mutex to protect vchDefaultKey --- src/wallet.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/wallet.cpp b/src/wallet.cpp index e861416e5..8bbb80cf2 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -1280,20 +1280,23 @@ bool CWallet::GetKeyFromPool(vector& result, bool fAllowReuse) { int64 nIndex = 0; CKeyPool keypool; - ReserveKeyFromKeyPool(nIndex, keypool); - if (nIndex == -1) + CRITICAL_BLOCK(cs_wallet) { - if (fAllowReuse && !vchDefaultKey.empty()) + ReserveKeyFromKeyPool(nIndex, keypool); + if (nIndex == -1) { - result = vchDefaultKey; + if (fAllowReuse && !vchDefaultKey.empty()) + { + result = vchDefaultKey; + return true; + } + if (IsLocked()) return false; + result = GenerateNewKey(); return true; } - if (IsLocked()) return false; - result = GenerateNewKey(); - return true; + KeepKey(nIndex); + result = keypool.vchPubKey; } - KeepKey(nIndex); - result = keypool.vchPubKey; return true; }