From b0243da77c6ee8d8ca59b4423f333a179bff02cf Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Mon, 29 Aug 2011 17:03:08 -0400 Subject: [PATCH 1/3] Highlight mis-matching locks --- src/util.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index 24c7ed44..390b3a34 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -942,17 +942,21 @@ static std::map, LockStack> lock static boost::thread_specific_ptr lockstack; -static void potential_deadlock_detected(const LockStack& s1, const LockStack& s2) +static void potential_deadlock_detected(const std::pair& mismatch, const LockStack& s1, const LockStack& s2) { printf("POTENTIAL DEADLOCK DETECTED\n"); printf("Previous lock order was:\n"); BOOST_FOREACH(const PAIRTYPE(CCriticalSection*, CLockLocation)& i, s2) { + if (i.first == mismatch.first) printf(" (1)"); + if (i.first == mismatch.second) printf(" (2)"); printf(" %s %s:%d\n", i.second.mutexName.c_str(), i.second.sourceFile.c_str(), i.second.sourceLine); } printf("Current lock order is:\n"); BOOST_FOREACH(const PAIRTYPE(CCriticalSection*, CLockLocation)& i, s1) { + if (i.first == mismatch.first) printf(" (1)"); + if (i.first == mismatch.second) printf(" (2)"); printf(" %s %s:%d\n", i.second.mutexName.c_str(), i.second.sourceFile.c_str(), i.second.sourceLine); } } @@ -979,7 +983,7 @@ static void push_lock(CCriticalSection* c, const CLockLocation& locklocation) std::pair p2 = std::make_pair(c, i.first); if (lockorders.count(p2)) { - potential_deadlock_detected(lockorders[p2], lockorders[p1]); + potential_deadlock_detected(p1, lockorders[p2], lockorders[p1]); break; } } From 6cc4a62c0e696dcb9d90ba0504f688e4f644a10f Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Fri, 26 Aug 2011 14:37:23 -0400 Subject: [PATCH 2/3] Fix rpc-hanging deadlocks Collapsed multiple wallet mutexes to a single cs_wallet, to avoid deadlocks with wallet methods that acquired locks in different order. Also change master RPC call handler to acquire cs_main and cs_wallet locks before executing RPC calls; requiring each RPC call to acquire the right set of locks in the right order was too error-prone. --- src/db.cpp | 3 +- src/keystore.cpp | 8 +- src/keystore.h | 44 ++-- src/main.cpp | 2 +- src/rpc.cpp | 633 +++++++++++++++++++++-------------------------- src/script.cpp | 102 ++++---- src/ui.cpp | 17 +- src/wallet.cpp | 209 ++++++++-------- src/wallet.h | 15 +- 9 files changed, 472 insertions(+), 561 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index 72542705..a22b17e3 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -683,8 +683,7 @@ int CWalletDB::LoadWallet(CWallet* pwallet) #endif //// todo: shouldn't we catch exceptions and try to recover and continue? - CRITICAL_BLOCK(pwallet->cs_mapWallet) - CRITICAL_BLOCK(pwallet->cs_KeyStore) + CRITICAL_BLOCK(pwallet->cs_wallet) { // Get cursor Dbc* pcursor = GetCursor(); diff --git a/src/keystore.cpp b/src/keystore.cpp index 4c6848b4..5bf919cb 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -45,7 +45,7 @@ std::vector CCryptoKeyStore::GenerateNewKey() bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) { - CRITICAL_BLOCK(cs_vMasterKey) + CRITICAL_BLOCK(cs_KeyStore) { if (!SetCrypted()) return false; @@ -72,7 +72,6 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) bool CCryptoKeyStore::AddKey(const CKey& key) { CRITICAL_BLOCK(cs_KeyStore) - CRITICAL_BLOCK(cs_vMasterKey) { if (!IsCrypted()) return CBasicKeyStore::AddKey(key); @@ -106,7 +105,7 @@ bool CCryptoKeyStore::AddCryptedKey(const std::vector &vchPubKey, bool CCryptoKeyStore::GetKey(const CBitcoinAddress &address, CKey& keyOut) const { - CRITICAL_BLOCK(cs_vMasterKey) + CRITICAL_BLOCK(cs_KeyStore) { if (!IsCrypted()) return CBasicKeyStore::GetKey(address, keyOut); @@ -128,7 +127,7 @@ bool CCryptoKeyStore::GetKey(const CBitcoinAddress &address, CKey& keyOut) const bool CCryptoKeyStore::GetPubKey(const CBitcoinAddress &address, std::vector& vchPubKeyOut) const { - CRITICAL_BLOCK(cs_vMasterKey) + CRITICAL_BLOCK(cs_KeyStore) { if (!IsCrypted()) return CKeyStore::GetPubKey(address, vchPubKeyOut); @@ -146,7 +145,6 @@ bool CCryptoKeyStore::GetPubKey(const CBitcoinAddress &address, std::vector 0); + bool result; + CRITICAL_BLOCK(cs_KeyStore) + result = (mapKeys.count(address) > 0); + return result; } bool GetKey(const CBitcoinAddress &address, CKey& keyOut) const { - KeyMap::const_iterator mi = mapKeys.find(address); - if (mi != mapKeys.end()) + CRITICAL_BLOCK(cs_KeyStore) { - keyOut.SetSecret((*mi).second); - return true; + KeyMap::const_iterator mi = mapKeys.find(address); + if (mi != mapKeys.end()) + { + keyOut.SetSecret((*mi).second); + return true; + } } return false; } @@ -74,8 +81,6 @@ protected: bool Unlock(const CKeyingMaterial& vMasterKeyIn); public: - mutable CCriticalSection cs_vMasterKey; //No guarantees master key wont get locked before you can use it, so lock this first - CCryptoKeyStore() : fUseCrypto(false) { } @@ -89,18 +94,20 @@ public: { if (!IsCrypted()) return false; - return vMasterKey.empty(); + bool result; + CRITICAL_BLOCK(cs_KeyStore) + result = vMasterKey.empty(); + return result; } bool Lock() { - CRITICAL_BLOCK(cs_vMasterKey) - { - if (!SetCrypted()) - return false; + if (!SetCrypted()) + return false; + CRITICAL_BLOCK(cs_KeyStore) vMasterKey.clear(); - } + return true; } @@ -109,9 +116,12 @@ public: bool AddKey(const CKey& key); bool HaveKey(const CBitcoinAddress &address) const { - if (!IsCrypted()) - return CBasicKeyStore::HaveKey(address); - return mapCryptedKeys.count(address) > 0; + CRITICAL_BLOCK(cs_KeyStore) + { + if (!IsCrypted()) + return CBasicKeyStore::HaveKey(address); + return mapCryptedKeys.count(address) > 0; + } } bool GetKey(const CBitcoinAddress &address, CKey& keyOut) const; bool GetPubKey(const CBitcoinAddress &address, std::vector& vchPubKeyOut) const; diff --git a/src/main.cpp b/src/main.cpp index f4833120..390632aa 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2879,7 +2879,7 @@ bool CheckWork(CBlock* pblock, CWallet& wallet, CReserveKey& reservekey) reservekey.KeepKey(); // Track how many getdata requests this block gets - CRITICAL_BLOCK(wallet.cs_mapRequestCount) + CRITICAL_BLOCK(wallet.cs_wallet) wallet.mapRequestCount[pblock->GetHash()] = 0; // Process this block the same as if we had received it from another node diff --git a/src/rpc.cpp b/src/rpc.cpp index 62b2f82f..5eb5669a 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -346,55 +346,50 @@ Value getnewaddress(const Array& params, bool fHelp) CBitcoinAddress address(pwalletMain->GetOrReuseKeyFromPool()); // This could be done in the same main CS as GetKeyFromKeyPool. - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) - pwalletMain->SetAddressBookName(address, strAccount); + pwalletMain->SetAddressBookName(address, strAccount); return address.ToString(); } -// requires cs_main, cs_mapWallet, cs_mapAddressBook locks CBitcoinAddress GetAccountAddress(string strAccount, bool bForceNew=false) { CWalletDB walletdb(pwalletMain->strWalletFile); CAccount account; - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) - { - walletdb.ReadAccount(strAccount, account); + walletdb.ReadAccount(strAccount, account); - bool bKeyUsed = false; + bool bKeyUsed = false; - // Check if the current key has been used - if (!account.vchPubKey.empty()) + // Check if the current key has been used + if (!account.vchPubKey.empty()) + { + CScript scriptPubKey; + scriptPubKey.SetBitcoinAddress(account.vchPubKey); + for (map::iterator it = pwalletMain->mapWallet.begin(); + it != pwalletMain->mapWallet.end() && !account.vchPubKey.empty(); + ++it) { - CScript scriptPubKey; - scriptPubKey.SetBitcoinAddress(account.vchPubKey); - for (map::iterator it = pwalletMain->mapWallet.begin(); - it != pwalletMain->mapWallet.end() && !account.vchPubKey.empty(); - ++it) - { - const CWalletTx& wtx = (*it).second; - BOOST_FOREACH(const CTxOut& txout, wtx.vout) - if (txout.scriptPubKey == scriptPubKey) - bKeyUsed = true; - } + const CWalletTx& wtx = (*it).second; + BOOST_FOREACH(const CTxOut& txout, wtx.vout) + if (txout.scriptPubKey == scriptPubKey) + bKeyUsed = true; } + } - // Generate a new key - if (account.vchPubKey.empty() || bForceNew || bKeyUsed) + // Generate a new key + if (account.vchPubKey.empty() || bForceNew || bKeyUsed) + { + if (pwalletMain->GetKeyPoolSize() < 1) { - if (pwalletMain->GetKeyPoolSize() < 1) - { - if (bKeyUsed || bForceNew) - 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); - } + if (bKeyUsed || bForceNew) + 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); } } @@ -413,12 +408,7 @@ Value getaccountaddress(const Array& params, bool fHelp) Value ret; - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) - { - ret = GetAccountAddress(strAccount).ToString(); - } + ret = GetAccountAddress(strAccount).ToString(); return ret; } @@ -442,20 +432,15 @@ Value setaccount(const Array& params, bool fHelp) strAccount = AccountFromValue(params[1]); // Detect when changing the account of an address that is the 'unused current key' of another account: - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + if (pwalletMain->mapAddressBook.count(address)) { - if (pwalletMain->mapAddressBook.count(address)) - { - string strOldAccount = pwalletMain->mapAddressBook[address]; - if (address == GetAccountAddress(strOldAccount)) - GetAccountAddress(strOldAccount, true); - } - - pwalletMain->SetAddressBookName(address, strAccount); + string strOldAccount = pwalletMain->mapAddressBook[address]; + if (address == GetAccountAddress(strOldAccount)) + GetAccountAddress(strOldAccount, true); } + pwalletMain->SetAddressBookName(address, strAccount); + return Value::null; } @@ -472,12 +457,9 @@ Value getaccount(const Array& params, bool fHelp) throw JSONRPCError(-5, "Invalid bitcoin address"); string strAccount; - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) - { - map::iterator mi = pwalletMain->mapAddressBook.find(address); - if (mi != pwalletMain->mapAddressBook.end() && !(*mi).second.empty()) - strAccount = (*mi).second; - } + map::iterator mi = pwalletMain->mapAddressBook.find(address); + if (mi != pwalletMain->mapAddressBook.end() && !(*mi).second.empty()) + strAccount = (*mi).second; return strAccount; } @@ -493,15 +475,12 @@ Value getaddressesbyaccount(const Array& params, bool fHelp) // Find all addresses that have the given account Array ret; - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& item, pwalletMain->mapAddressBook) { - BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& item, pwalletMain->mapAddressBook) - { - const CBitcoinAddress& address = item.first; - const string& strName = item.second; - if (strName == strAccount) - ret.push_back(address.ToString()); - } + const CBitcoinAddress& address = item.first; + const string& strName = item.second; + if (strName == strAccount) + ret.push_back(address.ToString()); } return ret; } @@ -548,16 +527,12 @@ Value sendtoaddress(const Array& params, bool fHelp) if (params.size() > 3 && params[3].type() != null_type && !params[3].get_str().empty()) wtx.mapValue["to"] = params[3].get_str(); - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) - { - if(pwalletMain->IsLocked()) - throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); + if (pwalletMain->IsLocked()) + throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); - string strError = pwalletMain->SendMoneyToBitcoinAddress(address, nAmount, wtx); - if (strError != "") - throw JSONRPCError(-4, strError); - } + string strError = pwalletMain->SendMoneyToBitcoinAddress(address, nAmount, wtx); + if (strError != "") + throw JSONRPCError(-4, strError); return wtx.GetHash().GetHex(); } @@ -586,19 +561,16 @@ Value getreceivedbyaddress(const Array& params, bool fHelp) // Tally int64 nAmount = 0; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { - for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) - { - const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !wtx.IsFinal()) - continue; + const CWalletTx& wtx = (*it).second; + if (wtx.IsCoinBase() || !wtx.IsFinal()) + continue; - BOOST_FOREACH(const CTxOut& txout, wtx.vout) - if (txout.scriptPubKey == scriptPubKey) - if (wtx.GetDepthInMainChain() >= nMinDepth) - nAmount += txout.nValue; - } + BOOST_FOREACH(const CTxOut& txout, wtx.vout) + if (txout.scriptPubKey == scriptPubKey) + if (wtx.GetDepthInMainChain() >= nMinDepth) + nAmount += txout.nValue; } return ValueFromAmount(nAmount); @@ -607,15 +579,12 @@ Value getreceivedbyaddress(const Array& params, bool fHelp) void GetAccountAddresses(string strAccount, set& setAddress) { - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& item, pwalletMain->mapAddressBook) { - BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& item, pwalletMain->mapAddressBook) - { - const CBitcoinAddress& address = item.first; - const string& strName = item.second; - if (strName == strAccount) - setAddress.insert(address); - } + const CBitcoinAddress& address = item.first; + const string& strName = item.second; + if (strName == strAccount) + setAddress.insert(address); } } @@ -639,21 +608,18 @@ Value getreceivedbyaccount(const Array& params, bool fHelp) // Tally int64 nAmount = 0; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { - for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) - { - const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !wtx.IsFinal()) - continue; + const CWalletTx& wtx = (*it).second; + if (wtx.IsCoinBase() || !wtx.IsFinal()) + continue; - BOOST_FOREACH(const CTxOut& txout, wtx.vout) - { - CBitcoinAddress address; - if (ExtractAddress(txout.scriptPubKey, pwalletMain, address) && setAddress.count(address)) - if (wtx.GetDepthInMainChain() >= nMinDepth) - nAmount += txout.nValue; - } + BOOST_FOREACH(const CTxOut& txout, wtx.vout) + { + CBitcoinAddress address; + if (ExtractAddress(txout.scriptPubKey, pwalletMain, address) && setAddress.count(address)) + if (wtx.GetDepthInMainChain() >= nMinDepth) + nAmount += txout.nValue; } } @@ -664,27 +630,25 @@ Value getreceivedbyaccount(const Array& params, bool fHelp) int64 GetAccountBalance(CWalletDB& walletdb, const string& strAccount, int nMinDepth) { int64 nBalance = 0; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - { - // Tally wallet transactions - for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) - { - const CWalletTx& wtx = (*it).second; - if (!wtx.IsFinal()) - continue; - int64 nGenerated, nReceived, nSent, nFee; - wtx.GetAccountAmounts(strAccount, nGenerated, nReceived, nSent, nFee); + // Tally wallet transactions + for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) + { + const CWalletTx& wtx = (*it).second; + if (!wtx.IsFinal()) + continue; - if (nReceived != 0 && wtx.GetDepthInMainChain() >= nMinDepth) - nBalance += nReceived; - nBalance += nGenerated - nSent - nFee; - } + int64 nGenerated, nReceived, nSent, nFee; + wtx.GetAccountAmounts(strAccount, nGenerated, nReceived, nSent, nFee); - // Tally internal accounting entries - nBalance += walletdb.GetAccountCreditDebit(strAccount); + if (nReceived != 0 && wtx.GetDepthInMainChain() >= nMinDepth) + nBalance += nReceived; + nBalance += nGenerated - nSent - nFee; } + // Tally internal accounting entries + nBalance += walletdb.GetAccountCreditDebit(strAccount); + return nBalance; } @@ -763,33 +727,31 @@ Value movecmd(const Array& params, bool fHelp) if (params.size() > 4) strComment = params[4].get_str(); - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - { - CWalletDB walletdb(pwalletMain->strWalletFile); - walletdb.TxnBegin(); - - int64 nNow = GetAdjustedTime(); - - // Debit - CAccountingEntry debit; - debit.strAccount = strFrom; - debit.nCreditDebit = -nAmount; - debit.nTime = nNow; - debit.strOtherAccount = strTo; - debit.strComment = strComment; - walletdb.WriteAccountingEntry(debit); - - // Credit - CAccountingEntry credit; - credit.strAccount = strTo; - credit.nCreditDebit = nAmount; - credit.nTime = nNow; - credit.strOtherAccount = strFrom; - credit.strComment = strComment; - walletdb.WriteAccountingEntry(credit); - - walletdb.TxnCommit(); - } + CWalletDB walletdb(pwalletMain->strWalletFile); + walletdb.TxnBegin(); + + int64 nNow = GetAdjustedTime(); + + // Debit + CAccountingEntry debit; + debit.strAccount = strFrom; + debit.nCreditDebit = -nAmount; + debit.nTime = nNow; + debit.strOtherAccount = strTo; + debit.strComment = strComment; + walletdb.WriteAccountingEntry(debit); + + // Credit + CAccountingEntry credit; + credit.strAccount = strTo; + credit.nCreditDebit = nAmount; + credit.nTime = nNow; + credit.strOtherAccount = strFrom; + credit.strComment = strComment; + walletdb.WriteAccountingEntry(credit); + + walletdb.TxnCommit(); + return true; } @@ -822,23 +784,18 @@ Value sendfrom(const Array& params, bool fHelp) if (params.size() > 5 && params[5].type() != null_type && !params[5].get_str().empty()) wtx.mapValue["to"] = params[5].get_str(); - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) - { - if(pwalletMain->IsLocked()) - throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); - - // Check funds - int64 nBalance = GetAccountBalance(strAccount, nMinDepth); - if (nAmount > nBalance) - throw JSONRPCError(-6, "Account has insufficient funds"); - - // Send - string strError = pwalletMain->SendMoneyToBitcoinAddress(address, nAmount, wtx); - if (strError != "") - throw JSONRPCError(-4, strError); - } + if (pwalletMain->IsLocked()) + throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); + + // Check funds + int64 nBalance = GetAccountBalance(strAccount, nMinDepth); + if (nAmount > nBalance) + throw JSONRPCError(-6, "Account has insufficient funds"); + + // Send + string strError = pwalletMain->SendMoneyToBitcoinAddress(address, nAmount, wtx); + if (strError != "") + throw JSONRPCError(-4, strError); return wtx.GetHash().GetHex(); } @@ -889,31 +846,26 @@ Value sendmany(const Array& params, bool fHelp) vecSend.push_back(make_pair(scriptPubKey, nAmount)); } - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) + if (pwalletMain->IsLocked()) + throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); + + // Check funds + int64 nBalance = GetAccountBalance(strAccount, nMinDepth); + if (totalAmount > nBalance) + throw JSONRPCError(-6, "Account has insufficient funds"); + + // Send + CReserveKey keyChange(pwalletMain); + int64 nFeeRequired = 0; + bool fCreated = pwalletMain->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired); + if (!fCreated) { - if(pwalletMain->IsLocked()) - throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); - - // Check funds - int64 nBalance = GetAccountBalance(strAccount, nMinDepth); - if (totalAmount > nBalance) - throw JSONRPCError(-6, "Account has insufficient funds"); - - // Send - CReserveKey keyChange(pwalletMain); - int64 nFeeRequired = 0; - bool fCreated = pwalletMain->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired); - if (!fCreated) - { - if (totalAmount + nFeeRequired > pwalletMain->GetBalance()) - throw JSONRPCError(-6, "Insufficient funds"); - throw JSONRPCError(-4, "Transaction creation failed"); - } - if (!pwalletMain->CommitTransaction(wtx, keyChange)) - throw JSONRPCError(-4, "Transaction commit failed"); + if (totalAmount + nFeeRequired > pwalletMain->GetBalance()) + throw JSONRPCError(-6, "Insufficient funds"); + throw JSONRPCError(-4, "Transaction creation failed"); } + if (!pwalletMain->CommitTransaction(wtx, keyChange)) + throw JSONRPCError(-4, "Transaction commit failed"); return wtx.GetHash().GetHex(); } @@ -944,68 +896,62 @@ Value ListReceived(const Array& params, bool fByAccounts) // Tally map mapTally; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { - for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) - { - const CWalletTx& wtx = (*it).second; - if (wtx.IsCoinBase() || !wtx.IsFinal()) - continue; + const CWalletTx& wtx = (*it).second; + if (wtx.IsCoinBase() || !wtx.IsFinal()) + continue; - int nDepth = wtx.GetDepthInMainChain(); - if (nDepth < nMinDepth) - continue; + int nDepth = wtx.GetDepthInMainChain(); + if (nDepth < nMinDepth) + continue; - BOOST_FOREACH(const CTxOut& txout, wtx.vout) - { - CBitcoinAddress address; - if (!ExtractAddress(txout.scriptPubKey, pwalletMain, address) || !address.IsValid()) - continue; + BOOST_FOREACH(const CTxOut& txout, wtx.vout) + { + CBitcoinAddress address; + if (!ExtractAddress(txout.scriptPubKey, pwalletMain, address) || !address.IsValid()) + continue; - tallyitem& item = mapTally[address]; - item.nAmount += txout.nValue; - item.nConf = min(item.nConf, nDepth); - } + tallyitem& item = mapTally[address]; + item.nAmount += txout.nValue; + item.nConf = min(item.nConf, nDepth); } } // Reply Array ret; map mapAccountTally; - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& item, pwalletMain->mapAddressBook) { - BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& item, pwalletMain->mapAddressBook) - { - const CBitcoinAddress& address = item.first; - const string& strAccount = item.second; - map::iterator it = mapTally.find(address); - if (it == mapTally.end() && !fIncludeEmpty) - continue; + const CBitcoinAddress& address = item.first; + const string& strAccount = item.second; + map::iterator it = mapTally.find(address); + if (it == mapTally.end() && !fIncludeEmpty) + continue; - int64 nAmount = 0; - int nConf = INT_MAX; - if (it != mapTally.end()) - { - nAmount = (*it).second.nAmount; - nConf = (*it).second.nConf; - } + int64 nAmount = 0; + int nConf = INT_MAX; + if (it != mapTally.end()) + { + nAmount = (*it).second.nAmount; + nConf = (*it).second.nConf; + } - if (fByAccounts) - { - tallyitem& item = mapAccountTally[strAccount]; - item.nAmount += nAmount; - item.nConf = min(item.nConf, nConf); - } - else - { - Object obj; - obj.push_back(Pair("address", address.ToString())); - obj.push_back(Pair("account", strAccount)); - obj.push_back(Pair("label", strAccount)); // deprecated - obj.push_back(Pair("amount", ValueFromAmount(nAmount))); - obj.push_back(Pair("confirmations", (nConf == INT_MAX ? 0 : nConf))); - ret.push_back(obj); - } + if (fByAccounts) + { + tallyitem& item = mapAccountTally[strAccount]; + item.nAmount += nAmount; + item.nConf = min(item.nConf, nConf); + } + else + { + Object obj; + obj.push_back(Pair("address", address.ToString())); + obj.push_back(Pair("account", strAccount)); + obj.push_back(Pair("label", strAccount)); // deprecated + obj.push_back(Pair("amount", ValueFromAmount(nAmount))); + obj.push_back(Pair("confirmations", (nConf == INT_MAX ? 0 : nConf))); + ret.push_back(obj); } } @@ -1107,27 +1053,23 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe // Received if (listReceived.size() > 0 && wtx.GetDepthInMainChain() >= nMinDepth) - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, int64)& r, listReceived) { - BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, int64)& r, listReceived) + string account; + if (pwalletMain->mapAddressBook.count(r.first)) + account = pwalletMain->mapAddressBook[r.first]; + if (fAllAccounts || (account == strAccount)) { - string account; - if (pwalletMain->mapAddressBook.count(r.first)) - account = pwalletMain->mapAddressBook[r.first]; - if (fAllAccounts || (account == strAccount)) - { - Object entry; - entry.push_back(Pair("account", account)); - entry.push_back(Pair("address", r.first.ToString())); - entry.push_back(Pair("category", "receive")); - entry.push_back(Pair("amount", ValueFromAmount(r.second))); - if (fLong) - WalletTxToJSON(wtx, entry); - ret.push_back(entry); - } + Object entry; + entry.push_back(Pair("account", account)); + entry.push_back(Pair("address", r.first.ToString())); + entry.push_back(Pair("category", "receive")); + entry.push_back(Pair("amount", ValueFromAmount(r.second))); + if (fLong) + WalletTxToJSON(wtx, entry); + ret.push_back(entry); } } - } void AcentryToJSON(const CAccountingEntry& acentry, const string& strAccount, Array& ret) @@ -1167,41 +1109,38 @@ Value listtransactions(const Array& params, bool fHelp) Array ret; CWalletDB walletdb(pwalletMain->strWalletFile); - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - { - // Firs: get all CWalletTx and CAccountingEntry into a sorted-by-time multimap: - typedef pair TxPair; - typedef multimap TxItems; - TxItems txByTime; + // Firs: get all CWalletTx and CAccountingEntry into a sorted-by-time multimap: + typedef pair TxPair; + typedef multimap TxItems; + TxItems txByTime; - for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) - { - CWalletTx* wtx = &((*it).second); - txByTime.insert(make_pair(wtx->GetTxTime(), TxPair(wtx, (CAccountingEntry*)0))); - } - list acentries; - walletdb.ListAccountCreditDebit(strAccount, acentries); - BOOST_FOREACH(CAccountingEntry& entry, acentries) - { - txByTime.insert(make_pair(entry.nTime, TxPair((CWalletTx*)0, &entry))); - } + for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) + { + CWalletTx* wtx = &((*it).second); + txByTime.insert(make_pair(wtx->GetTxTime(), TxPair(wtx, (CAccountingEntry*)0))); + } + list acentries; + walletdb.ListAccountCreditDebit(strAccount, acentries); + BOOST_FOREACH(CAccountingEntry& entry, acentries) + { + txByTime.insert(make_pair(entry.nTime, TxPair((CWalletTx*)0, &entry))); + } - // Now: iterate backwards until we have nCount items to return: - TxItems::reverse_iterator it = txByTime.rbegin(); - if (txByTime.size() > nFrom) std::advance(it, nFrom); - for (; it != txByTime.rend(); ++it) - { - CWalletTx *const pwtx = (*it).second.first; - if (pwtx != 0) - ListTransactions(*pwtx, strAccount, 0, true, ret); - CAccountingEntry *const pacentry = (*it).second.second; - if (pacentry != 0) - AcentryToJSON(*pacentry, strAccount, ret); - - if (ret.size() >= nCount) break; - } - // ret is now newest to oldest + // Now: iterate backwards until we have nCount items to return: + TxItems::reverse_iterator it = txByTime.rbegin(); + if (txByTime.size() > nFrom) std::advance(it, nFrom); + for (; it != txByTime.rend(); ++it) + { + CWalletTx *const pwtx = (*it).second.first; + if (pwtx != 0) + ListTransactions(*pwtx, strAccount, 0, true, ret); + CAccountingEntry *const pacentry = (*it).second.second; + if (pacentry != 0) + AcentryToJSON(*pacentry, strAccount, ret); + + if (ret.size() >= nCount) break; } + // ret is now newest to oldest // Make sure we return only last nCount items (sends-to-self might give us an extra): if (ret.size() > nCount) @@ -1227,34 +1166,30 @@ Value listaccounts(const Array& params, bool fHelp) nMinDepth = params[0].get_int(); map mapAccountBalances; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) - { - BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& entry, pwalletMain->mapAddressBook) { - if (pwalletMain->HaveKey(entry.first)) // This address belongs to me - mapAccountBalances[entry.second] = 0; - } + BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& entry, pwalletMain->mapAddressBook) { + if (pwalletMain->HaveKey(entry.first)) // This address belongs to me + mapAccountBalances[entry.second] = 0; + } - for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) + for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) + { + const CWalletTx& wtx = (*it).second; + int64 nGeneratedImmature, nGeneratedMature, nFee; + string strSentAccount; + list > listReceived; + list > listSent; + wtx.GetAmounts(nGeneratedImmature, nGeneratedMature, listReceived, listSent, nFee, strSentAccount); + mapAccountBalances[strSentAccount] -= nFee; + BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, int64)& s, listSent) + mapAccountBalances[strSentAccount] -= s.second; + if (wtx.GetDepthInMainChain() >= nMinDepth) { - const CWalletTx& wtx = (*it).second; - int64 nGeneratedImmature, nGeneratedMature, nFee; - string strSentAccount; - list > listReceived; - list > listSent; - wtx.GetAmounts(nGeneratedImmature, nGeneratedMature, listReceived, listSent, nFee, strSentAccount); - mapAccountBalances[strSentAccount] -= nFee; - BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, int64)& s, listSent) - mapAccountBalances[strSentAccount] -= s.second; - if (wtx.GetDepthInMainChain() >= nMinDepth) - { - mapAccountBalances[""] += nGeneratedMature; - BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, int64)& r, listReceived) - if (pwalletMain->mapAddressBook.count(r.first)) - mapAccountBalances[pwalletMain->mapAddressBook[r.first]] += r.second; - else - mapAccountBalances[""] += r.second; - } + mapAccountBalances[""] += nGeneratedMature; + BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, int64)& r, listReceived) + if (pwalletMain->mapAddressBook.count(r.first)) + mapAccountBalances[pwalletMain->mapAddressBook[r.first]] += r.second; + else + mapAccountBalances[""] += r.second; } } @@ -1281,27 +1216,25 @@ Value gettransaction(const Array& params, bool fHelp) hash.SetHex(params[0].get_str()); Object entry; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) - { - if (!pwalletMain->mapWallet.count(hash)) - throw JSONRPCError(-5, "Invalid or non-wallet transaction id"); - const CWalletTx& wtx = pwalletMain->mapWallet[hash]; - int64 nCredit = wtx.GetCredit(); - int64 nDebit = wtx.GetDebit(); - int64 nNet = nCredit - nDebit; - int64 nFee = (wtx.IsFromMe() ? wtx.GetValueOut() - nDebit : 0); + if (!pwalletMain->mapWallet.count(hash)) + throw JSONRPCError(-5, "Invalid or non-wallet transaction id"); + const CWalletTx& wtx = pwalletMain->mapWallet[hash]; - entry.push_back(Pair("amount", ValueFromAmount(nNet - nFee))); - if (wtx.IsFromMe()) - entry.push_back(Pair("fee", ValueFromAmount(nFee))); + int64 nCredit = wtx.GetCredit(); + int64 nDebit = wtx.GetDebit(); + int64 nNet = nCredit - nDebit; + int64 nFee = (wtx.IsFromMe() ? wtx.GetValueOut() - nDebit : 0); - WalletTxToJSON(pwalletMain->mapWallet[hash], entry); + entry.push_back(Pair("amount", ValueFromAmount(nNet - nFee))); + if (wtx.IsFromMe()) + entry.push_back(Pair("fee", ValueFromAmount(nFee))); - Array details; - ListTransactions(pwalletMain->mapWallet[hash], "*", 0, false, details); - entry.push_back(Pair("details", details)); - } + WalletTxToJSON(pwalletMain->mapWallet[hash], entry); + + Array details; + ListTransactions(pwalletMain->mapWallet[hash], "*", 0, false, details); + entry.push_back(Pair("details", details)); return entry; } @@ -1332,13 +1265,10 @@ Value keypoolrefill(const Array& params, bool fHelp) "keypoolrefill\n" "Fills the keypool."); - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) - { - if (pwalletMain->IsLocked()) - throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); + if (pwalletMain->IsLocked()) + throw JSONRPCError(-13, "Error: Please enter the wallet passphrase with walletpassphrase first."); - pwalletMain->TopUpKeyPool(); - } + pwalletMain->TopUpKeyPool(); if (pwalletMain->GetKeyPoolSize() < GetArg("-keypool", 100)) throw JSONRPCError(-4, "Error refreshing keypool."); @@ -1407,24 +1337,21 @@ Value walletpassphrase(const Array& params, bool fHelp) mlock(&strWalletPass[0], strWalletPass.capacity()); strWalletPass = params[0].get_str(); - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) + if (strWalletPass.length() > 0) { - if (strWalletPass.length() > 0) + if (!pwalletMain->Unlock(strWalletPass)) { - if (!pwalletMain->Unlock(strWalletPass)) - { - fill(strWalletPass.begin(), strWalletPass.end(), '\0'); - munlock(&strWalletPass[0], strWalletPass.capacity()); - throw JSONRPCError(-14, "Error: The wallet passphrase entered was incorrect."); - } fill(strWalletPass.begin(), strWalletPass.end(), '\0'); munlock(&strWalletPass[0], strWalletPass.capacity()); + throw JSONRPCError(-14, "Error: The wallet passphrase entered was incorrect."); } - else - throw runtime_error( - "walletpassphrase \n" - "Stores the wallet decryption key in memory for seconds."); + fill(strWalletPass.begin(), strWalletPass.end(), '\0'); + munlock(&strWalletPass[0], strWalletPass.capacity()); } + else + throw runtime_error( + "walletpassphrase \n" + "Stores the wallet decryption key in memory for seconds."); CreateThread(ThreadTopUpKeyPool, NULL); int* pnSleepTime = new int(params[1].get_int()); @@ -1553,11 +1480,8 @@ Value validateaddress(const Array& params, bool fHelp) string currentAddress = address.ToString(); ret.push_back(Pair("address", currentAddress)); ret.push_back(Pair("ismine", (pwalletMain->HaveKey(address) > 0))); - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) - { - if (pwalletMain->mapAddressBook.count(address)) - ret.push_back(Pair("account", pwalletMain->mapAddressBook[address])); - } + if (pwalletMain->mapAddressBook.count(address)) + ret.push_back(Pair("account", pwalletMain->mapAddressBook[address])); } return ret; } @@ -2233,7 +2157,10 @@ void ThreadRPCServer2(void* parg) try { // Execute - Value result = (*(*mi).second)(params, false); + Value result; + CRITICAL_BLOCK(cs_main) + CRITICAL_BLOCK(pwalletMain->cs_wallet) + result = (*(*mi).second)(params, false); // Send reply string strReply = JSONRPCReply(result, Value::null, id); diff --git a/src/script.cpp b/src/script.cpp index d0c6a114..6e7bcb5e 100644 --- a/src/script.cpp +++ b/src/script.cpp @@ -1033,48 +1033,45 @@ bool Solver(const CKeyStore& keystore, const CScript& scriptPubKey, uint256 hash return false; // Compile solution - CRITICAL_BLOCK(keystore.cs_KeyStore) + BOOST_FOREACH(PAIRTYPE(opcodetype, valtype)& item, vSolution) { - BOOST_FOREACH(PAIRTYPE(opcodetype, valtype)& item, vSolution) + if (item.first == OP_PUBKEY) { - if (item.first == OP_PUBKEY) + // Sign + const valtype& vchPubKey = item.second; + CKey key; + if (!keystore.GetKey(Hash160(vchPubKey), key)) + return false; + if (key.GetPubKey() != vchPubKey) + return false; + if (hash != 0) { - // Sign - const valtype& vchPubKey = item.second; - CKey key; - if (!keystore.GetKey(Hash160(vchPubKey), key)) - return false; - if (key.GetPubKey() != vchPubKey) + vector vchSig; + if (!key.Sign(hash, vchSig)) return false; - if (hash != 0) - { - vector vchSig; - if (!key.Sign(hash, vchSig)) - return false; - vchSig.push_back((unsigned char)nHashType); - scriptSigRet << vchSig; - } + vchSig.push_back((unsigned char)nHashType); + scriptSigRet << vchSig; } - else if (item.first == OP_PUBKEYHASH) + } + else if (item.first == OP_PUBKEYHASH) + { + // Sign and give pubkey + CKey key; + if (!keystore.GetKey(uint160(item.second), key)) + return false; + if (hash != 0) { - // Sign and give pubkey - CKey key; - if (!keystore.GetKey(uint160(item.second), key)) + vector vchSig; + if (!key.Sign(hash, vchSig)) return false; - if (hash != 0) - { - vector vchSig; - if (!key.Sign(hash, vchSig)) - return false; - vchSig.push_back((unsigned char)nHashType); - scriptSigRet << vchSig << key.GetPubKey(); - } - } - else - { - return false; + vchSig.push_back((unsigned char)nHashType); + scriptSigRet << vchSig << key.GetPubKey(); } } + else + { + return false; + } } return true; @@ -1095,35 +1092,31 @@ bool IsMine(const CKeyStore &keystore, const CScript& scriptPubKey) return false; // Compile solution - CRITICAL_BLOCK(keystore.cs_KeyStore) + BOOST_FOREACH(PAIRTYPE(opcodetype, valtype)& item, vSolution) { - BOOST_FOREACH(PAIRTYPE(opcodetype, valtype)& item, vSolution) + if (item.first == OP_PUBKEY) { - if (item.first == OP_PUBKEY) - { - const valtype& vchPubKey = item.second; - vector vchPubKeyFound; - if (!keystore.GetPubKey(Hash160(vchPubKey), vchPubKeyFound)) - return false; - if (vchPubKeyFound != vchPubKey) - return false; - } - else if (item.first == OP_PUBKEYHASH) - { - if (!keystore.HaveKey(uint160(item.second))) - return false; - } - else - { + const valtype& vchPubKey = item.second; + vector vchPubKeyFound; + if (!keystore.GetPubKey(Hash160(vchPubKey), vchPubKeyFound)) return false; - } + if (vchPubKeyFound != vchPubKey) + return false; + } + else if (item.first == OP_PUBKEYHASH) + { + if (!keystore.HaveKey(uint160(item.second))) + return false; + } + else + { + return false; } } return true; } -// requires either keystore==0, or a lock on keystore->cs_KeyStore bool static ExtractAddressInner(const CScript& scriptPubKey, const CKeyStore* keystore, CBitcoinAddress& addressRet) { vector > vSolution; @@ -1146,8 +1139,7 @@ bool static ExtractAddressInner(const CScript& scriptPubKey, const CKeyStore* ke bool ExtractAddress(const CScript& scriptPubKey, const CKeyStore* keystore, CBitcoinAddress& addressRet) { if (keystore) - CRITICAL_BLOCK(keystore->cs_KeyStore) - return ExtractAddressInner(scriptPubKey, keystore, addressRet); + return ExtractAddressInner(scriptPubKey, keystore, addressRet); else return ExtractAddressInner(scriptPubKey, NULL, addressRet); return false; diff --git a/src/ui.cpp b/src/ui.cpp index 372808f1..867c9c0a 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -862,7 +862,7 @@ void CMainFrame::OnIdle(wxIdleEvent& event) // Collect list of wallet transactions and sort newest first bool fEntered = false; vector > vSorted; - TRY_CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + TRY_CRITICAL_BLOCK(pwalletMain->cs_wallet) { printf("RefreshListCtrl starting\n"); fEntered = true; @@ -890,7 +890,7 @@ void CMainFrame::OnIdle(wxIdleEvent& event) if (fShutdown) return; bool fEntered = false; - TRY_CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + TRY_CRITICAL_BLOCK(pwalletMain->cs_wallet) { fEntered = true; uint256& hash = vSorted[i++].second; @@ -913,7 +913,7 @@ void CMainFrame::OnIdle(wxIdleEvent& event) static int64 nLastTime; if (GetTime() > nLastTime + 30) { - TRY_CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + TRY_CRITICAL_BLOCK(pwalletMain->cs_wallet) { nLastTime = GetTime(); for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) @@ -937,7 +937,7 @@ void CMainFrame::RefreshStatusColumn() if (nTop == nLastTop && pindexLastBest == pindexBest) return; - TRY_CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + TRY_CRITICAL_BLOCK(pwalletMain->cs_wallet) { int nStart = nTop; int nEnd = min(nStart + 100, m_listCtrl->GetItemCount()); @@ -1057,7 +1057,7 @@ void CMainFrame::OnPaintListCtrl(wxPaintEvent& event) // Update listctrl contents if (!pwalletMain->vWalletUpdated.empty()) { - TRY_CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + TRY_CRITICAL_BLOCK(pwalletMain->cs_wallet) { string strTop; if (m_listCtrl->GetItemCount()) @@ -1075,7 +1075,7 @@ void CMainFrame::OnPaintListCtrl(wxPaintEvent& event) } // Balance total - TRY_CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + TRY_CRITICAL_BLOCK(pwalletMain->cs_wallet) { fPaintedBalance = true; m_staticTextBalance->SetLabel(FormatMoney(pwalletMain->GetBalance()) + " "); @@ -1420,7 +1420,7 @@ void CMainFrame::OnListItemActivated(wxListEvent& event) { uint256 hash((string)GetItemText(m_listCtrl, event.GetIndex(), 1)); CWalletTx wtx; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + CRITICAL_BLOCK(pwalletMain->cs_wallet) { map::iterator mi = pwalletMain->mapWallet.find(hash); if (mi == pwalletMain->mapWallet.end()) @@ -1662,7 +1662,7 @@ CTxDetailsDialog::CTxDetailsDialog(wxWindow* parent, CWalletTx wtx) : CTxDetails strHTML += HtmlEscape(wtx.ToString(), true); strHTML += "
Inputs:
"; - CRITICAL_BLOCK(pwalletMain->cs_mapWallet) + CRITICAL_BLOCK(pwalletMain->cs_wallet) { BOOST_FOREACH(const CTxIn& txin, wtx.vin) { @@ -2621,7 +2621,6 @@ CAddressBookDialog::CAddressBookDialog(wxWindow* parent, const wxString& strInit m_listCtrlReceiving->SetFocus(); // Fill listctrl with address book data - CRITICAL_BLOCK(pwalletMain->cs_KeyStore) CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) { string strDefaultReceiving = (string)pframeMain->m_textCtrlAddress->GetValue(); diff --git a/src/wallet.cpp b/src/wallet.cpp index 6519ac63..745fbefd 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -33,7 +33,7 @@ bool CWallet::AddCryptedKey(const vector &vchPubKey, const vector return false; if (!fFileBacked) return true; - CRITICAL_BLOCK(cs_pwalletdbEncryption) + CRITICAL_BLOCK(cs_wallet) { if (pwalletdbEncryption) return pwalletdbEncryption->WriteCryptedKey(vchPubKey, vchCryptedSecret); @@ -44,14 +44,13 @@ bool CWallet::AddCryptedKey(const vector &vchPubKey, const vector bool CWallet::Unlock(const string& strWalletPassphrase) { - CRITICAL_BLOCK(cs_vMasterKey) - { - if (!IsLocked()) - return false; + if (!IsLocked()) + return false; - CCrypter crypter; - CKeyingMaterial vMasterKey; + CCrypter crypter; + CKeyingMaterial vMasterKey; + CRITICAL_BLOCK(cs_wallet) BOOST_FOREACH(const MasterKeyMap::value_type& pMasterKey, mapMasterKeys) { if(!crypter.SetKeyFromPassphrase(strWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod)) @@ -61,16 +60,15 @@ bool CWallet::Unlock(const string& strWalletPassphrase) if (CCryptoKeyStore::Unlock(vMasterKey)) return true; } - } return false; } bool CWallet::ChangeWalletPassphrase(const string& strOldWalletPassphrase, const string& strNewWalletPassphrase) { - CRITICAL_BLOCK(cs_vMasterKey) - { - bool fWasLocked = IsLocked(); + bool fWasLocked = IsLocked(); + CRITICAL_BLOCK(cs_wallet) + { Lock(); CCrypter crypter; @@ -79,7 +77,7 @@ bool CWallet::ChangeWalletPassphrase(const string& strOldWalletPassphrase, const { if(!crypter.SetKeyFromPassphrase(strOldWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod)) return false; - if(!crypter.Decrypt(pMasterKey.second.vchCryptedKey, vMasterKey)) + if (!crypter.Decrypt(pMasterKey.second.vchCryptedKey, vMasterKey)) return false; if (CCryptoKeyStore::Unlock(vMasterKey)) { @@ -107,6 +105,7 @@ bool CWallet::ChangeWalletPassphrase(const string& strOldWalletPassphrase, const } } } + return false; } @@ -125,44 +124,42 @@ public: bool CWallet::EncryptWallet(const string& strWalletPassphrase) { - CRITICAL_BLOCK(cs_KeyStore) - CRITICAL_BLOCK(cs_vMasterKey) - CRITICAL_BLOCK(cs_pwalletdbEncryption) - { - if (IsCrypted()) - return false; + if (IsCrypted()) + return false; - CKeyingMaterial vMasterKey; - RandAddSeedPerfmon(); + CKeyingMaterial vMasterKey; + RandAddSeedPerfmon(); - vMasterKey.resize(WALLET_CRYPTO_KEY_SIZE); - RAND_bytes(&vMasterKey[0], WALLET_CRYPTO_KEY_SIZE); + vMasterKey.resize(WALLET_CRYPTO_KEY_SIZE); + RAND_bytes(&vMasterKey[0], WALLET_CRYPTO_KEY_SIZE); - CMasterKey kMasterKey; + CMasterKey kMasterKey; - RandAddSeedPerfmon(); - kMasterKey.vchSalt.resize(WALLET_CRYPTO_SALT_SIZE); - RAND_bytes(&kMasterKey.vchSalt[0], WALLET_CRYPTO_SALT_SIZE); + RandAddSeedPerfmon(); + kMasterKey.vchSalt.resize(WALLET_CRYPTO_SALT_SIZE); + RAND_bytes(&kMasterKey.vchSalt[0], WALLET_CRYPTO_SALT_SIZE); - CCrypter crypter; - int64 nStartTime = GetTimeMillis(); - crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, 25000, kMasterKey.nDerivationMethod); - kMasterKey.nDeriveIterations = 2500000 / ((double)(GetTimeMillis() - nStartTime)); + CCrypter crypter; + int64 nStartTime = GetTimeMillis(); + crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, 25000, kMasterKey.nDerivationMethod); + kMasterKey.nDeriveIterations = 2500000 / ((double)(GetTimeMillis() - nStartTime)); - nStartTime = GetTimeMillis(); - crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, kMasterKey.nDeriveIterations, kMasterKey.nDerivationMethod); - kMasterKey.nDeriveIterations = (kMasterKey.nDeriveIterations + kMasterKey.nDeriveIterations * 100 / ((double)(GetTimeMillis() - nStartTime))) / 2; + nStartTime = GetTimeMillis(); + crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, kMasterKey.nDeriveIterations, kMasterKey.nDerivationMethod); + kMasterKey.nDeriveIterations = (kMasterKey.nDeriveIterations + kMasterKey.nDeriveIterations * 100 / ((double)(GetTimeMillis() - nStartTime))) / 2; - if (kMasterKey.nDeriveIterations < 25000) - kMasterKey.nDeriveIterations = 25000; + if (kMasterKey.nDeriveIterations < 25000) + kMasterKey.nDeriveIterations = 25000; - printf("Encrypting Wallet with an nDeriveIterations of %i\n", kMasterKey.nDeriveIterations); + printf("Encrypting Wallet with an nDeriveIterations of %i\n", kMasterKey.nDeriveIterations); - if (!crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, kMasterKey.nDeriveIterations, kMasterKey.nDerivationMethod)) - return false; - if (!crypter.Encrypt(vMasterKey, kMasterKey.vchCryptedKey)) - return false; + if (!crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, kMasterKey.nDeriveIterations, kMasterKey.nDerivationMethod)) + return false; + if (!crypter.Encrypt(vMasterKey, kMasterKey.vchCryptedKey)) + return false; + CRITICAL_BLOCK(cs_wallet) + { mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; if (fFileBacked) { @@ -191,6 +188,7 @@ bool CWallet::EncryptWallet(const string& strWalletPassphrase) Lock(); } + return true; } @@ -199,7 +197,7 @@ void CWallet::WalletUpdateSpent(const CTransaction &tx) // Anytime a signature is successfully verified, it's proof the outpoint is spent. // Update the wallet spent flag if it doesn't know due to wallet.dat being // restored from backup or the user making copies of wallet.dat. - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { BOOST_FOREACH(const CTxIn& txin, tx.vin) { @@ -222,7 +220,7 @@ void CWallet::WalletUpdateSpent(const CTransaction &tx) bool CWallet::AddToWallet(const CWalletTx& wtxIn) { uint256 hash = wtxIn.GetHash(); - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { // Inserts only if not already there, returns tx inserted or tx found pair::iterator, bool> ret = mapWallet.insert(make_pair(hash, wtxIn)); @@ -290,18 +288,21 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn) bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate) { uint256 hash = tx.GetHash(); - bool fExisted = mapWallet.count(hash); - if (fExisted && !fUpdate) return false; - if (fExisted || IsMine(tx) || IsFromMe(tx)) + CRITICAL_BLOCK(cs_wallet) { - CWalletTx wtx(this,tx); - // Get merkle branch if transaction was found in a block - if (pblock) - wtx.SetMerkleBranch(pblock); - return AddToWallet(wtx); + bool fExisted = mapWallet.count(hash); + if (fExisted && !fUpdate) return false; + if (fExisted || IsMine(tx) || IsFromMe(tx)) + { + CWalletTx wtx(this,tx); + // Get merkle branch if transaction was found in a block + if (pblock) + wtx.SetMerkleBranch(pblock); + return AddToWallet(wtx); + } + else + WalletUpdateSpent(tx); } - else - WalletUpdateSpent(tx); return false; } @@ -309,7 +310,7 @@ bool CWallet::EraseFromWallet(uint256 hash) { if (!fFileBacked) return false; - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { if (mapWallet.erase(hash)) CWalletDB(strWalletFile).EraseTx(hash); @@ -320,7 +321,7 @@ bool CWallet::EraseFromWallet(uint256 hash) bool CWallet::IsMine(const CTxIn &txin) const { - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { map::const_iterator mi = mapWallet.find(txin.prevout.hash); if (mi != mapWallet.end()) @@ -336,7 +337,7 @@ bool CWallet::IsMine(const CTxIn &txin) const int64 CWallet::GetDebit(const CTxIn &txin) const { - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { map::const_iterator mi = mapWallet.find(txin.prevout.hash); if (mi != mapWallet.end()) @@ -352,17 +353,20 @@ int64 CWallet::GetDebit(const CTxIn &txin) const int64 CWalletTx::GetTxTime() const { - if (!fTimeReceivedIsTxTime && hashBlock != 0) + CRITICAL_BLOCK(cs_main) { - // If we did not receive the transaction directly, we rely on the block's - // time to figure out when it happened. We use the median over a range - // of blocks to try to filter out inaccurate block times. - map::iterator mi = mapBlockIndex.find(hashBlock); - if (mi != mapBlockIndex.end()) + if (!fTimeReceivedIsTxTime && hashBlock != 0) { - CBlockIndex* pindex = (*mi).second; - if (pindex) - return pindex->GetMedianTime(); + // If we did not receive the transaction directly, we rely on the block's + // time to figure out when it happened. We use the median over a range + // of blocks to try to filter out inaccurate block times. + map::iterator mi = mapBlockIndex.find(hashBlock); + if (mi != mapBlockIndex.end()) + { + CBlockIndex* pindex = (*mi).second; + if (pindex) + return pindex->GetMedianTime(); + } } } return nTimeReceived; @@ -372,7 +376,7 @@ int CWalletTx::GetRequestCount() const { // Returns -1 if it wasn't being tracked int nRequests = -1; - CRITICAL_BLOCK(pwallet->cs_mapRequestCount) + CRITICAL_BLOCK(pwallet->cs_wallet) { if (IsCoinBase()) { @@ -478,7 +482,7 @@ void CWalletTx::GetAccountAmounts(const string& strAccount, int64& nGenerated, i nSent += s.second; nFee = allFee; } - CRITICAL_BLOCK(pwallet->cs_mapAddressBook) + CRITICAL_BLOCK(pwallet->cs_wallet) { BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress,int64)& r, listReceived) { @@ -508,7 +512,7 @@ void CWalletTx::AddSupportingTransactions(CTxDB& txdb) vWorkQueue.push_back(txin.prevout.hash); // This critsect is OK because txdb is already open - CRITICAL_BLOCK(pwallet->cs_mapWallet) + CRITICAL_BLOCK(pwallet->cs_wallet) { map mapWalletPrev; set setAlreadyDone; @@ -564,7 +568,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) int ret = 0; CBlockIndex* pindex = pindexStart; - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { while (pindex) { @@ -585,7 +589,7 @@ void CWallet::ReacceptWalletTransactions() { CTxDB txdb("r"); bool fRepeat = true; - while (fRepeat) CRITICAL_BLOCK(cs_mapWallet) + while (fRepeat) CRITICAL_BLOCK(cs_wallet) { fRepeat = false; vector vMissingTx; @@ -688,7 +692,7 @@ void CWallet::ResendWalletTransactions() // Rebroadcast any of our txes that aren't in a block yet printf("ResendWalletTransactions()\n"); CTxDB txdb("r"); - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { // Sort them in chronological order multimap mapSorted; @@ -722,7 +726,7 @@ void CWallet::ResendWalletTransactions() int64 CWallet::GetBalance() const { int64 nTotal = 0; - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { for (map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { @@ -749,7 +753,7 @@ bool CWallet::SelectCoinsMinConf(int64 nTargetValue, int nConfMine, int nConfThe vector > > vValue; int64 nTotalLower = 0; - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { vector vCoins; vCoins.reserve(mapWallet.size()); @@ -907,10 +911,10 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CW wtxNew.pwallet = this; CRITICAL_BLOCK(cs_main) + CRITICAL_BLOCK(cs_wallet) { // txdb must be opened before the mapWallet lock CTxDB txdb("r"); - CRITICAL_BLOCK(cs_mapWallet) { nFeeRet = nTransactionFee; loop @@ -1021,9 +1025,9 @@ bool CWallet::CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& w bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) { CRITICAL_BLOCK(cs_main) + CRITICAL_BLOCK(cs_wallet) { printf("CommitTransaction:\n%s", wtxNew.ToString().c_str()); - CRITICAL_BLOCK(cs_mapWallet) { // This is only to keep the database open to defeat the auto-flush for the // duration of this scope. This is the only place where this optimization @@ -1053,8 +1057,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) } // Track how many getdata requests our transaction gets - CRITICAL_BLOCK(cs_mapRequestCount) - mapRequestCount[wtxNew.GetHash()] = 0; + mapRequestCount[wtxNew.GetHash()] = 0; // Broadcast if (!wtxNew.AcceptToMemoryPool()) @@ -1072,29 +1075,26 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) -// requires cs_main lock string CWallet::SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, bool fAskFee) { CReserveKey reservekey(this); int64 nFeeRequired; - CRITICAL_BLOCK(cs_vMasterKey) + + if (IsLocked()) { - if (IsLocked()) - { - string strError = _("Error: Wallet locked, unable to create transaction "); - printf("SendMoney() : %s", strError.c_str()); - return strError; - } - if (!CreateTransaction(scriptPubKey, nValue, wtxNew, reservekey, nFeeRequired)) - { - string strError; - if (nValue + nFeeRequired > GetBalance()) - strError = strprintf(_("Error: This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds "), FormatMoney(nFeeRequired).c_str()); - else - strError = _("Error: Transaction creation failed "); - printf("SendMoney() : %s", strError.c_str()); - return strError; - } + string strError = _("Error: Wallet locked, unable to create transaction "); + printf("SendMoney() : %s", strError.c_str()); + return strError; + } + if (!CreateTransaction(scriptPubKey, nValue, wtxNew, reservekey, nFeeRequired)) + { + string strError; + if (nValue + nFeeRequired > GetBalance()) + strError = strprintf(_("Error: This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds "), FormatMoney(nFeeRequired).c_str()); + else + strError = _("Error: Transaction creation failed "); + printf("SendMoney() : %s", strError.c_str()); + return strError; } if (fAskFee && !ThreadSafeAskFee(nFeeRequired, _("Sending..."), NULL)) @@ -1109,7 +1109,6 @@ string CWallet::SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, -// requires cs_main lock string CWallet::SendMoneyToBitcoinAddress(const CBitcoinAddress& address, int64 nValue, CWalletTx& wtxNew, bool fAskFee) { // Check amount @@ -1172,7 +1171,7 @@ bool CWallet::DelAddressBookName(const CBitcoinAddress& address) void CWallet::PrintWallet(const CBlock& block) { - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { if (mapWallet.count(block.vtx[0].GetHash())) { @@ -1185,7 +1184,7 @@ void CWallet::PrintWallet(const CBlock& block) bool CWallet::GetTransaction(const uint256 &hashTx, CWalletTx& wtx) { - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) { map::iterator mi = mapWallet.find(hashTx); if (mi != mapWallet.end()) @@ -1218,10 +1217,7 @@ bool GetWalletFile(CWallet* pwallet, string &strWalletFileOut) bool CWallet::TopUpKeyPool() { - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(cs_mapWallet) - CRITICAL_BLOCK(cs_setKeyPool) - CRITICAL_BLOCK(cs_vMasterKey) + CRITICAL_BLOCK(cs_wallet) { if (IsLocked()) return false; @@ -1248,9 +1244,7 @@ void CWallet::ReserveKeyFromKeyPool(int64& nIndex, CKeyPool& keypool) { nIndex = -1; keypool.vchPubKey.clear(); - CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(cs_mapWallet) - CRITICAL_BLOCK(cs_setKeyPool) + CRITICAL_BLOCK(cs_wallet) { if (!IsLocked()) TopUpKeyPool(); @@ -1278,10 +1272,7 @@ void CWallet::KeepKey(int64 nIndex) if (fFileBacked) { CWalletDB walletdb(strWalletFile); - CRITICAL_BLOCK(cs_main) - { - walletdb.ErasePool(nIndex); - } + walletdb.ErasePool(nIndex); } printf("keypool keep %"PRI64d"\n", nIndex); } @@ -1289,7 +1280,7 @@ void CWallet::KeepKey(int64 nIndex) void CWallet::ReturnKey(int64 nIndex) { // Return to key pool - CRITICAL_BLOCK(cs_setKeyPool) + CRITICAL_BLOCK(cs_wallet) setKeyPool.insert(nIndex); printf("keypool return %"PRI64d"\n", nIndex); } diff --git a/src/wallet.h b/src/wallet.h index 499f2a63..032284dd 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -20,14 +20,14 @@ private: bool SelectCoins(int64 nTargetValue, std::set >& setCoinsRet, int64& nValueRet) const; CWalletDB *pwalletdbEncryption; - CCriticalSection cs_pwalletdbEncryption; public: + mutable CCriticalSection cs_wallet; + bool fFileBacked; std::string strWalletFile; std::set setKeyPool; - CCriticalSection cs_setKeyPool; typedef std::map MasterKeyMap; MasterKeyMap mapMasterKeys; @@ -47,15 +47,12 @@ public: pwalletdbEncryption = NULL; } - mutable CCriticalSection cs_mapWallet; std::map mapWallet; std::vector vWalletUpdated; std::map mapRequestCount; - mutable CCriticalSection cs_mapRequestCount; std::map mapAddressBook; - mutable CCriticalSection cs_mapAddressBook; std::vector vchDefaultKey; @@ -107,7 +104,7 @@ public: { CBitcoinAddress address; if (ExtractAddress(txout.scriptPubKey, this, address)) - CRITICAL_BLOCK(cs_mapAddressBook) + CRITICAL_BLOCK(cs_wallet) if (!mapAddressBook.count(address)) return true; return false; @@ -171,15 +168,13 @@ public: int LoadWallet(bool& fFirstRunRet); // bool BackupWallet(const std::string& strDest); - // requires cs_mapAddressBook lock bool SetAddressBookName(const CBitcoinAddress& address, const std::string& strName); - // requires cs_mapAddressBook lock bool DelAddressBookName(const CBitcoinAddress& address); void UpdatedTransaction(const uint256 &hashTx) { - CRITICAL_BLOCK(cs_mapWallet) + CRITICAL_BLOCK(cs_wallet) vWalletUpdated.push_back(hashTx); } @@ -187,7 +182,7 @@ public: void Inventory(const uint256 &hash) { - CRITICAL_BLOCK(cs_mapRequestCount) + CRITICAL_BLOCK(cs_wallet) { std::map::iterator mi = mapRequestCount.find(hash); if (mi != mapRequestCount.end()) From 471426fb3b2c2fa37640c03819c4f7be69ba8301 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Wed, 31 Aug 2011 12:27:19 -0400 Subject: [PATCH 3/3] Fixed potential deadlocks in GUI code. Also changed semantics of CWalletTx::GetTxTime(); now always returns the time the transaction was received by this node, not the average block time. And added information about -DDEBUG_LOCKORDER to coding.txt. --- doc/coding.txt | 44 ++++++ src/ui.cpp | 358 ++++++++++++++++++++++++------------------------- src/wallet.cpp | 16 --- 3 files changed, 223 insertions(+), 195 deletions(-) diff --git a/doc/coding.txt b/doc/coding.txt index 47074766..ec31ccde 100644 --- a/doc/coding.txt +++ b/doc/coding.txt @@ -39,3 +39,47 @@ v vector or similar list objects map map or multimap set set or multiset bn CBigNum + +------------------------- +Locking/mutex usage notes + +The code is multi-threaded, and uses mutexes and the CRITICAL_BLOCK/TRY_CRITICAL_BLOCK macros to protect data structures. + +Deadlocks due to inconsistent lock ordering (thread 1 locks cs_main and then cs_wallet, while thread 2 locks them in the opposite order: result, deadlock as each waits for the other to release its lock) are a problem. Compile with -DDEBUG_LOCKORDER to get lock order inconsistencies reported in the debug.log file. + +Re-architecting the core code so there are better-defined interfaces between the various components is a goal, with any necessary locking done by the components (e.g. see the self-contained CKeyStore class and its cs_KeyStore lock for example). + +------- +Threads + +StartNode : Starts other threads. + +ThreadGetMyExternalIP : Determines outside-the-firewall IP address, sends addr message to connected peers when it determines it. + +ThreadIRCSeed : Joins IRC bootstrapping channel, watching for new peers and advertising this node's IP address. + +ThreadSocketHandler : Sends/Receives data from peers on port 8333. + +ThreadMessageHandler : Higher-level message handling (sending and receiving). + +ThreadOpenConnections : Initiates new connections to peers. + +ThreadTopUpKeyPool : replenishes the keystore's keypool. + +ThreadCleanWalletPassphrase : re-locks an encrypted wallet after user has unlocked it for a period of time. + +SendingDialogStartTransfer : used by pay-via-ip-address code (obsolete) + +ThreadDelayedRepaint : repaint the gui + +ThreadFlushWalletDB : Close the wallet.dat file if it hasn't been used in 500ms. + +ThreadRPCServer : Remote procedure call handler, listens on port 8332 for connections and services them. + +ThreadBitcoinMiner : Generates bitcoins + +ThreadMapPort : Universal plug-and-play startup/shutdown + +Shutdown : Does an orderly shutdown of everything + +ExitTimeout : Windows-only, sleeps 5 seconds then exits application diff --git a/src/ui.cpp b/src/ui.cpp index 867c9c0a..5ca66619 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -708,7 +708,7 @@ bool CMainFrame::InsertTransaction(const CWalletTx& wtx, bool fNew, int nIndex) CBitcoinAddress address; if (ExtractAddress(txout.scriptPubKey, pwalletMain, address)) { - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) { //strDescription += _("Received payment to "); //strDescription += _("Received with address "); @@ -792,7 +792,7 @@ bool CMainFrame::InsertTransaction(const CWalletTx& wtx, bool fNew, int nIndex) } string strDescription = _("To: "); - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) if (pwalletMain->mapAddressBook.count(address) && !pwalletMain->mapAddressBook[address].empty()) strDescription += pwalletMain->mapAddressBook[address] + " "; strDescription += strAddress; @@ -1032,6 +1032,7 @@ void MainFrameRepaint() printf("MainFrameRepaint\n"); wxPaintEvent event; pframeMain->fRefresh = true; + pframeMain->fRefreshListCtrl = true; pframeMain->GetEventHandler()->AddPendingEvent(event); } } @@ -1247,83 +1248,80 @@ void CMainFrame::OnMenuOptionsChangeWalletPassphrase(wxCommandEvent& event) strOldWalletPass = wxGetPasswordFromUser(_("Enter the current passphrase to the wallet."), _("Passphrase")).ToStdString(); - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) - { - bool fWasLocked = pwalletMain->IsLocked(); - pwalletMain->Lock(); + bool fWasLocked = pwalletMain->IsLocked(); + pwalletMain->Lock(); - if (!strOldWalletPass.size() || !pwalletMain->Unlock(strOldWalletPass)) - { - fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); - munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); - wxMessageBox(_("The passphrase entered for the wallet decryption was incorrect."), "Bitcoin", wxOK | wxICON_ERROR); - return; - } + if (!strOldWalletPass.size() || !pwalletMain->Unlock(strOldWalletPass)) + { + fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); + munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); + wxMessageBox(_("The passphrase entered for the wallet decryption was incorrect."), "Bitcoin", wxOK | wxICON_ERROR); + return; + } - if (fWasLocked) - pwalletMain->Lock(); + if (fWasLocked) + pwalletMain->Lock(); - string strNewWalletPass; - strNewWalletPass.reserve(100); - mlock(&strNewWalletPass[0], strNewWalletPass.capacity()); + string strNewWalletPass; + strNewWalletPass.reserve(100); + mlock(&strNewWalletPass[0], strNewWalletPass.capacity()); - // obtain new wallet encrypt/decrypt key, from passphrase - // Note that the passphrase is not mlock()d during this entry and could potentially - // be obtained from disk long after bitcoin has run. - strNewWalletPass = wxGetPasswordFromUser(_("Enter the new passphrase for the wallet."), - _("Passphrase")).ToStdString(); + // obtain new wallet encrypt/decrypt key, from passphrase + // Note that the passphrase is not mlock()d during this entry and could potentially + // be obtained from disk long after bitcoin has run. + strNewWalletPass = wxGetPasswordFromUser(_("Enter the new passphrase for the wallet."), + _("Passphrase")).ToStdString(); - if (!strNewWalletPass.size()) - { - fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); - fill(strNewWalletPass.begin(), strNewWalletPass.end(), '\0'); - munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); - munlock(&strNewWalletPass[0], strNewWalletPass.capacity()); - wxMessageBox(_("Error: The supplied passphrase was too short."), "Bitcoin", wxOK | wxICON_ERROR); - return; - } + if (!strNewWalletPass.size()) + { + fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); + fill(strNewWalletPass.begin(), strNewWalletPass.end(), '\0'); + munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); + munlock(&strNewWalletPass[0], strNewWalletPass.capacity()); + wxMessageBox(_("Error: The supplied passphrase was too short."), "Bitcoin", wxOK | wxICON_ERROR); + return; + } - string strNewWalletPassTest; - strNewWalletPassTest.reserve(100); - mlock(&strNewWalletPassTest[0], strNewWalletPassTest.capacity()); + string strNewWalletPassTest; + strNewWalletPassTest.reserve(100); + mlock(&strNewWalletPassTest[0], strNewWalletPassTest.capacity()); - // obtain new wallet encrypt/decrypt key, from passphrase - // Note that the passphrase is not mlock()d during this entry and could potentially - // be obtained from disk long after bitcoin has run. - strNewWalletPassTest = wxGetPasswordFromUser(_("Re-enter the new passphrase for the wallet."), - _("Passphrase")).ToStdString(); + // obtain new wallet encrypt/decrypt key, from passphrase + // Note that the passphrase is not mlock()d during this entry and could potentially + // be obtained from disk long after bitcoin has run. + strNewWalletPassTest = wxGetPasswordFromUser(_("Re-enter the new passphrase for the wallet."), + _("Passphrase")).ToStdString(); - if (strNewWalletPassTest != strNewWalletPass) - { - fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); - fill(strNewWalletPass.begin(), strNewWalletPass.end(), '\0'); - fill(strNewWalletPassTest.begin(), strNewWalletPassTest.end(), '\0'); - munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); - munlock(&strNewWalletPass[0], strNewWalletPass.capacity()); - munlock(&strNewWalletPassTest[0], strNewWalletPassTest.capacity()); - wxMessageBox(_("Error: the supplied passphrases didn't match."), "Bitcoin", wxOK | wxICON_ERROR); - return; - } + if (strNewWalletPassTest != strNewWalletPass) + { + fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); + fill(strNewWalletPass.begin(), strNewWalletPass.end(), '\0'); + fill(strNewWalletPassTest.begin(), strNewWalletPassTest.end(), '\0'); + munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); + munlock(&strNewWalletPass[0], strNewWalletPass.capacity()); + munlock(&strNewWalletPassTest[0], strNewWalletPassTest.capacity()); + wxMessageBox(_("Error: the supplied passphrases didn't match."), "Bitcoin", wxOK | wxICON_ERROR); + return; + } - if (!pwalletMain->ChangeWalletPassphrase(strOldWalletPass, strNewWalletPass)) - { - fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); - fill(strNewWalletPass.begin(), strNewWalletPass.end(), '\0'); - fill(strNewWalletPassTest.begin(), strNewWalletPassTest.end(), '\0'); - munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); - munlock(&strNewWalletPass[0], strNewWalletPass.capacity()); - munlock(&strNewWalletPassTest[0], strNewWalletPassTest.capacity()); - wxMessageBox(_("The passphrase entered for the wallet decryption was incorrect."), "Bitcoin", wxOK | wxICON_ERROR); - return; - } + if (!pwalletMain->ChangeWalletPassphrase(strOldWalletPass, strNewWalletPass)) + { fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); fill(strNewWalletPass.begin(), strNewWalletPass.end(), '\0'); fill(strNewWalletPassTest.begin(), strNewWalletPassTest.end(), '\0'); munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); munlock(&strNewWalletPass[0], strNewWalletPass.capacity()); munlock(&strNewWalletPassTest[0], strNewWalletPassTest.capacity()); - wxMessageBox(_("Wallet Passphrase Changed."), "Bitcoin"); + wxMessageBox(_("The passphrase entered for the wallet decryption was incorrect."), "Bitcoin", wxOK | wxICON_ERROR); + return; } + fill(strOldWalletPass.begin(), strOldWalletPass.end(), '\0'); + fill(strNewWalletPass.begin(), strNewWalletPass.end(), '\0'); + fill(strNewWalletPassTest.begin(), strNewWalletPassTest.end(), '\0'); + munlock(&strOldWalletPass[0], strOldWalletPass.capacity()); + munlock(&strNewWalletPass[0], strNewWalletPass.capacity()); + munlock(&strNewWalletPassTest[0], strNewWalletPassTest.capacity()); + wxMessageBox(_("Wallet Passphrase Changed."), "Bitcoin"); } void CMainFrame::OnMenuOptionsOptions(wxCommandEvent& event) @@ -1387,21 +1385,19 @@ void CMainFrame::OnButtonNew(wxCommandEvent& event) string strName = dialog.GetValue(); string strAddress; - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) - { - bool fWasLocked = pwalletMain->IsLocked(); - if (!GetWalletPassphrase()) - return; - // Generate new key - strAddress = CBitcoinAddress(pwalletMain->GetOrReuseKeyFromPool()).ToString(); + bool fWasLocked = pwalletMain->IsLocked(); + if (!GetWalletPassphrase()) + return; - if (fWasLocked) - pwalletMain->Lock(); - } + // Generate new key + strAddress = CBitcoinAddress(pwalletMain->GetOrReuseKeyFromPool()).ToString(); + + if (fWasLocked) + pwalletMain->Lock(); // Save - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) pwalletMain->SetAddressBookName(strAddress, strName); SetDefaultReceivingAddress(strAddress); } @@ -1451,7 +1447,7 @@ CTxDetailsDialog::CTxDetailsDialog(wxWindow* parent, CWalletTx wtx) : CTxDetails #ifdef __WXMSW__ SetSize(nScaleX * GetSize().GetWidth(), nScaleY * GetSize().GetHeight()); #endif - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) { string strHTML; strHTML.reserve(4000); @@ -2160,38 +2156,39 @@ void CSendDialog::OnButtonSend(wxCommandEvent& event) if (fBitcoinAddress) { + bool fWasLocked = pwalletMain->IsLocked(); + if (!GetWalletPassphrase()) + return; + + string strError; CRITICAL_BLOCK(cs_main) - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) + CRITICAL_BLOCK(pwalletMain->cs_wallet) { - bool fWasLocked = pwalletMain->IsLocked(); - if (!GetWalletPassphrase()) - return; - // Send to bitcoin address CScript scriptPubKey; scriptPubKey.SetBitcoinAddress(address); - string strError = pwalletMain->SendMoney(scriptPubKey, nValue, wtx, true); - if (strError == "") - wxMessageBox(_("Payment sent "), _("Sending...")); - else if (strError == "ABORTED") - { - if (fWasLocked) - pwalletMain->Lock(); - return; // leave send dialog open - } - else - { - wxMessageBox(strError + " ", _("Sending...")); - EndModal(false); - if (fWasLocked) - pwalletMain->Lock(); - return; - } - + strError = pwalletMain->SendMoney(scriptPubKey, nValue, wtx, true); + } + if (strError == "") + wxMessageBox(_("Payment sent "), _("Sending...")); + else if (strError == "ABORTED") + { if (fWasLocked) pwalletMain->Lock(); - } + return; // leave send dialog open + } + else + { + wxMessageBox(strError + " ", _("Sending...")); + EndModal(false); + if (fWasLocked) + pwalletMain->Lock(); + return; + } + + if (fWasLocked) + pwalletMain->Lock(); } else { @@ -2212,7 +2209,7 @@ void CSendDialog::OnButtonSend(wxCommandEvent& event) return; } - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) if (!pwalletMain->mapAddressBook.count(address)) pwalletMain->SetAddressBookName(strAddress, ""); @@ -2464,83 +2461,89 @@ void CSendingDialog::OnReply2(CDataStream& vRecv) return; } - CRITICAL_BLOCK(cs_main) + // Pay + if (!Status(_("Creating transaction..."))) + return; + if (nPrice + nTransactionFee > pwalletMain->GetBalance()) { - // Pay - if (!Status(_("Creating transaction..."))) - return; - if (nPrice + nTransactionFee > pwalletMain->GetBalance()) - { - Error(_("Insufficient funds")); - return; - } + Error(_("Insufficient funds")); + return; + } - CReserveKey reservekey(pwalletMain); - int64 nFeeRequired; - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) - { - bool fWasLocked = pwalletMain->IsLocked(); - if (!GetWalletPassphrase()) - return; + CReserveKey reservekey(pwalletMain); + int64 nFeeRequired; + bool fWasLocked = pwalletMain->IsLocked(); + if (!GetWalletPassphrase()) + return; - if (!pwalletMain->CreateTransaction(scriptPubKey, nPrice, wtx, reservekey, nFeeRequired)) - { - if (nPrice + nFeeRequired > pwalletMain->GetBalance()) - Error(strprintf(_("This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds"), FormatMoney(nFeeRequired).c_str())); - else - Error(_("Transaction creation failed")); - return; - } + bool fTxCreated = false; + CRITICAL_BLOCK(cs_main) + CRITICAL_BLOCK(pwalletMain->cs_wallet) + { + fTxCreated = pwalletMain->CreateTransaction(scriptPubKey, nPrice, wtx, reservekey, nFeeRequired); + } + if (!fTxCreated) + { + if (nPrice + nFeeRequired > pwalletMain->GetBalance()) + Error(strprintf(_("This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds"), FormatMoney(nFeeRequired).c_str())); + else + Error(_("Transaction creation failed")); + return; + } - if (fWasLocked) - pwalletMain->Lock(); - } + if (fWasLocked) + pwalletMain->Lock(); - // Transaction fee - if (!ThreadSafeAskFee(nFeeRequired, _("Sending..."), this)) - { - Error(_("Transaction aborted")); - return; - } + // Transaction fee + if (!ThreadSafeAskFee(nFeeRequired, _("Sending..."), this)) + { + Error(_("Transaction aborted")); + return; + } - // Make sure we're still connected - CNode* pnode = ConnectNode(addr, 2 * 60 * 60); - if (!pnode) - { - Error(_("Lost connection, transaction cancelled")); - return; - } + // Make sure we're still connected + CNode* pnode = ConnectNode(addr, 2 * 60 * 60); + if (!pnode) + { + Error(_("Lost connection, transaction cancelled")); + return; + } - // Last chance to cancel - Sleep(50); + // Last chance to cancel + Sleep(50); + if (!Status()) + return; + fCanCancel = false; + if (fAbort) + { + fCanCancel = true; if (!Status()) return; fCanCancel = false; - if (fAbort) - { - fCanCancel = true; - if (!Status()) - return; - fCanCancel = false; - } - if (!Status(_("Sending payment..."))) - return; + } + if (!Status(_("Sending payment..."))) + return; - // Commit - if (!pwalletMain->CommitTransaction(wtx, reservekey)) - { - Error(_("The transaction was rejected. This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here.")); - return; - } + // Commit + bool fTxCommitted = false; + CRITICAL_BLOCK(cs_main) + CRITICAL_BLOCK(pwalletMain->cs_wallet) + { + fTxCommitted = pwalletMain->CommitTransaction(wtx, reservekey); + } + if (!fTxCommitted) + { + Error(_("The transaction was rejected. This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here.")); + return; + } - // Send payment tx to seller, with response going to OnReply3 via event handler - CWalletTx wtxSend = wtx; - wtxSend.fFromMe = false; - pnode->PushRequest("submitorder", wtxSend, SendingDialogOnReply3, this); + // Send payment tx to seller, with response going to OnReply3 via event handler + CWalletTx wtxSend = wtx; + wtxSend.fFromMe = false; + pnode->PushRequest("submitorder", wtxSend, SendingDialogOnReply3, this); - Status(_("Waiting for confirmation...")); - MainFrameRepaint(); - } + Status(_("Waiting for confirmation...")); + MainFrameRepaint(); } void SendingDialogOnReply3(void* parg, CDataStream& vRecv) @@ -2621,7 +2624,7 @@ CAddressBookDialog::CAddressBookDialog(wxWindow* parent, const wxString& strInit m_listCtrlReceiving->SetFocus(); // Fill listctrl with address book data - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) { string strDefaultReceiving = (string)pframeMain->m_textCtrlAddress->GetValue(); BOOST_FOREACH(const PAIRTYPE(CBitcoinAddress, string)& item, pwalletMain->mapAddressBook) @@ -2682,7 +2685,7 @@ void CAddressBookDialog::OnListEndLabelEdit(wxListEvent& event) if (event.IsEditCancelled()) return; string strAddress = (string)GetItemText(m_listCtrl, event.GetIndex(), 1); - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) pwalletMain->SetAddressBookName(strAddress, string(event.GetText())); pframeMain->RefreshListCtrl(); } @@ -2718,7 +2721,7 @@ void CAddressBookDialog::OnButtonDelete(wxCommandEvent& event) if (m_listCtrl->GetItemState(nIndex, wxLIST_STATE_SELECTED)) { string strAddress = (string)GetItemText(m_listCtrl, nIndex, 1); - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) pwalletMain->DelAddressBookName(strAddress); m_listCtrl->DeleteItem(nIndex); } @@ -2778,7 +2781,7 @@ void CAddressBookDialog::OnButtonEdit(wxCommandEvent& event) } // Write back - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) { if (strAddress != strAddressOrg) pwalletMain->DelAddressBookName(strAddressOrg); @@ -2818,22 +2821,19 @@ void CAddressBookDialog::OnButtonNew(wxCommandEvent& event) return; strName = dialog.GetValue(); - CRITICAL_BLOCK(pwalletMain->cs_vMasterKey) - { - bool fWasLocked = pwalletMain->IsLocked(); - if (!GetWalletPassphrase()) - return; + bool fWasLocked = pwalletMain->IsLocked(); + if (!GetWalletPassphrase()) + return; - // Generate new key - strAddress = CBitcoinAddress(pwalletMain->GetOrReuseKeyFromPool()).ToString(); + // Generate new key + strAddress = CBitcoinAddress(pwalletMain->GetOrReuseKeyFromPool()).ToString(); - if (fWasLocked) - pwalletMain->Lock(); - } + if (fWasLocked) + pwalletMain->Lock(); } // Add to list and select it - CRITICAL_BLOCK(pwalletMain->cs_mapAddressBook) + CRITICAL_BLOCK(pwalletMain->cs_wallet) pwalletMain->SetAddressBookName(strAddress, strName); int nIndex = InsertLine(m_listCtrl, strName, strAddress); SetSelection(m_listCtrl, nIndex); diff --git a/src/wallet.cpp b/src/wallet.cpp index 745fbefd..1daec98d 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -353,22 +353,6 @@ int64 CWallet::GetDebit(const CTxIn &txin) const int64 CWalletTx::GetTxTime() const { - CRITICAL_BLOCK(cs_main) - { - if (!fTimeReceivedIsTxTime && hashBlock != 0) - { - // If we did not receive the transaction directly, we rely on the block's - // time to figure out when it happened. We use the median over a range - // of blocks to try to filter out inaccurate block times. - map::iterator mi = mapBlockIndex.find(hashBlock); - if (mi != mapBlockIndex.end()) - { - CBlockIndex* pindex = (*mi).second; - if (pindex) - return pindex->GetMedianTime(); - } - } - } return nTimeReceived; }