From 20469d83dd2fd7d8efacf94f017b926be7c92e63 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Thu, 15 Aug 2013 21:53:21 -0700 Subject: [PATCH 1/2] [QT] Don't ask for a passphrase to getnewaddress. With an encrypted wallet the GUI was prompting for a passphrase every time the user requested a new address. This is unnecessary, increases the exposure to keyboard sniffers, and discourages using fresh addresses for every transaction. Instead only prompt for a passphrase when the keypool runs out, also call the new address function with the flag that prevents reuse. Thanks to AlexNagy on IRC for pointing this out and who wouldn't take any lip from a curmudgeonly developer and insisted on what he knew to be true. --- src/qt/addresstablemodel.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index dcc70222cc9..d4a7a92876f 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -355,18 +355,21 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con else if(type == Receive) { // Generate a new address to associate with given label - WalletModel::UnlockContext ctx(walletModel->requestUnlock()); - if(!ctx.isValid()) - { - // Unlock wallet failed or was cancelled - editStatus = WALLET_UNLOCK_FAILURE; - return QString(); - } CPubKey newKey; - if(!wallet->GetKeyFromPool(newKey, true)) + if(!wallet->GetKeyFromPool(newKey, false)) { - editStatus = KEY_GENERATION_FAILURE; - return QString(); + WalletModel::UnlockContext ctx(walletModel->requestUnlock()); + if(!ctx.isValid()) + { + // Unlock wallet failed or was cancelled + editStatus = WALLET_UNLOCK_FAILURE; + return QString(); + } + if(!wallet->GetKeyFromPool(newKey, false)) + { + editStatus = KEY_GENERATION_FAILURE; + return QString(); + } } strAddress = CBitcoinAddress(newKey.GetID()).ToString(); } From 71ac5052d83fcba21a09e5e2b7ad66faea6bd42a Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Fri, 23 Aug 2013 12:54:50 -0700 Subject: [PATCH 2/2] Remove fAllowReuse from GetKeyFromPool. With the GUI password fix this was always false. --- src/init.cpp | 2 +- src/qt/addresstablemodel.cpp | 4 ++-- src/qt/paymentserver.cpp | 2 +- src/rpcwallet.cpp | 4 ++-- src/wallet.cpp | 9 ++------- src/wallet.h | 2 +- 6 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 72635650472..c607fe14d38 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -893,7 +893,7 @@ bool AppInit2(boost::thread_group& threadGroup) RandAddSeedPerfmon(); CPubKey newDefaultKey; - if (pwalletMain->GetKeyFromPool(newDefaultKey, false)) { + if (pwalletMain->GetKeyFromPool(newDefaultKey)) { pwalletMain->SetDefaultKey(newDefaultKey); if (!pwalletMain->SetAddressBook(pwalletMain->vchDefaultKey.GetID(), "", "receive")) strErrors << _("Cannot write default address") << "\n"; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index d4a7a92876f..03517c657fd 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -356,7 +356,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con { // Generate a new address to associate with given label CPubKey newKey; - if(!wallet->GetKeyFromPool(newKey, false)) + if(!wallet->GetKeyFromPool(newKey)) { WalletModel::UnlockContext ctx(walletModel->requestUnlock()); if(!ctx.isValid()) @@ -365,7 +365,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con editStatus = WALLET_UNLOCK_FAILURE; return QString(); } - if(!wallet->GetKeyFromPool(newKey, false)) + if(!wallet->GetKeyFromPool(newKey)) { editStatus = KEY_GENERATION_FAILURE; return QString(); diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index a9f71315a90..ff3c2a09815 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -531,7 +531,7 @@ PaymentServer::fetchPaymentACK(CWallet* wallet, SendCoinsRecipient recipient, QB } else { CPubKey newKey; - if (wallet->GetKeyFromPool(newKey, false)) { + if (wallet->GetKeyFromPool(newKey)) { CKeyID keyID = newKey.GetID(); wallet->SetAddressBook(keyID, strAccount, "refund"); diff --git a/src/rpcwallet.cpp b/src/rpcwallet.cpp index d07d3408b96..34bd4ffccfa 100644 --- a/src/rpcwallet.cpp +++ b/src/rpcwallet.cpp @@ -110,7 +110,7 @@ Value getnewaddress(const Array& params, bool fHelp) // Generate a new key that is added to wallet CPubKey newKey; - if (!pwalletMain->GetKeyFromPool(newKey, false)) + if (!pwalletMain->GetKeyFromPool(newKey)) throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); CKeyID keyID = newKey.GetID(); @@ -148,7 +148,7 @@ CBitcoinAddress GetAccountAddress(string strAccount, bool bForceNew=false) // Generate a new key if (!account.vchPubKey.IsValid() || bForceNew || bKeyUsed) { - if (!pwalletMain->GetKeyFromPool(account.vchPubKey, false)) + if (!pwalletMain->GetKeyFromPool(account.vchPubKey)) throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); pwalletMain->SetAddressBook(account.vchPubKey.GetID(), strAccount, "receive"); diff --git a/src/wallet.cpp b/src/wallet.cpp index ddfd71efda0..7a3855c0253 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -493,7 +493,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn) if (txout.scriptPubKey == scriptDefaultKey) { CPubKey newDefaultKey; - if (GetKeyFromPool(newDefaultKey, false)) + if (GetKeyFromPool(newDefaultKey)) { SetDefaultKey(newDefaultKey); SetAddressBook(vchDefaultKey.GetID(), "", "receive"); @@ -1647,7 +1647,7 @@ void CWallet::ReturnKey(int64 nIndex) printf("keypool return %"PRI64d"\n", nIndex); } -bool CWallet::GetKeyFromPool(CPubKey& result, bool fAllowReuse) +bool CWallet::GetKeyFromPool(CPubKey& result) { int64 nIndex = 0; CKeyPool keypool; @@ -1656,11 +1656,6 @@ bool CWallet::GetKeyFromPool(CPubKey& result, bool fAllowReuse) ReserveKeyFromKeyPool(nIndex, keypool); if (nIndex == -1) { - if (fAllowReuse && vchDefaultKey.IsValid()) - { - result = vchDefaultKey; - return true; - } if (IsLocked()) return false; result = GenerateNewKey(); return true; diff --git a/src/wallet.h b/src/wallet.h index d47416d2722..d61548433ef 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -220,7 +220,7 @@ public: void ReserveKeyFromKeyPool(int64& nIndex, CKeyPool& keypool); void KeepKey(int64 nIndex); void ReturnKey(int64 nIndex); - bool GetKeyFromPool(CPubKey &key, bool fAllowReuse=true); + bool GetKeyFromPool(CPubKey &key); int64 GetOldestKeyPoolTime(); void GetAllReserveKeys(std::set& setAddress) const;