Merge pull request #3413

d31ad26 qt: Add missing lock in WalletModel::listCoins (Wladimir J. van der Laan)
28352af qt: protect SetAddressBook with cs_wallet lock everywhere (Wladimir J. van der Laan)
aaf8d15 qt: Add missing LOCKs for locked coin functions (Wladimir J. van der Laan)
4757e92 qt: add missing cs_wallet lock in AddressTableModel::setData (Wladimir J. van der Laan)

Author: laanwj

src/qt/addresstablemodel.cpp

@@ -244,49 +244,46 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
 
     if(role == Qt::EditRole)
     {
-        switch(index.column())
+        LOCK(wallet->cs_wallet); /* For SetAddressBook / DelAddressBook */
+        CTxDestination curAddress = CBitcoinAddress(rec->address.toStdString()).Get();
+        if(index.column() == Label)
         {
-        case Label:
             // Do nothing, if old label == new label
             if(rec->label == value.toString())
             {
                 editStatus = NO_CHANGES;
                 return false;
             }
-            wallet->SetAddressBook(CBitcoinAddress(rec->address.toStdString()).Get(), value.toString().toStdString(), strPurpose);
-            break;
-        case Address:
-            // Do nothing, if old address == new address
-            if(CBitcoinAddress(rec->address.toStdString()) == CBitcoinAddress(value.toString().toStdString()))
+            wallet->SetAddressBook(curAddress, value.toString().toStdString(), strPurpose);
+        } else if(index.column() == Address) {
+            CTxDestination newAddress = CBitcoinAddress(value.toString().toStdString()).Get();
+            // Refuse to set invalid address, set error status and return false
+            if(boost::get<CNoDestination>(&newAddress))
             {
-                editStatus = NO_CHANGES;
+                editStatus = INVALID_ADDRESS;
                 return false;
             }
-            // Refuse to set invalid address, set error status and return false
-            else if(!walletModel->validateAddress(value.toString()))
+            // Do nothing, if old address == new address
+            else if(newAddress == curAddress)
             {
-                editStatus = INVALID_ADDRESS;
+                editStatus = NO_CHANGES;
                 return false;
             }
             // Check for duplicate addresses to prevent accidental deletion of addresses, if you try
             // to paste an existing address over another address (with a different label)
-            else if(wallet->mapAddressBook.count(CBitcoinAddress(value.toString().toStdString()).Get()))
+            else if(wallet->mapAddressBook.count(newAddress))
             {
                 editStatus = DUPLICATE_ADDRESS;
                 return false;
             }
             // Double-check that we're not overwriting a receiving address
             else if(rec->type == AddressTableEntry::Sending)
             {
-                {
-                    LOCK(wallet->cs_wallet);
-                    // Remove old entry
-                    wallet->DelAddressBook(CBitcoinAddress(rec->address.toStdString()).Get());
-                    // Add new entry with new address
-                    wallet->SetAddressBook(CBitcoinAddress(value.toString().toStdString()).Get(), rec->label.toStdString(), strPurpose);
-                }
+                // Remove old entry
+                wallet->DelAddressBook(curAddress);
+                // Add new entry with new address
+                wallet->SetAddressBook(newAddress, rec->label.toStdString(), strPurpose);
             }
-            break;
         }
         return true;
     }

src/qt/paymentserver.cpp

@@ -548,6 +548,7 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, SendCoinsRecipient recipien
     else {
         CPubKey newKey;
         if (wallet->GetKeyFromPool(newKey)) {
+            LOCK(wallet->cs_wallet); // SetAddressBook
             CKeyID keyID = newKey.GetID();
             wallet->SetAddressBook(keyID, strAccount, "refund");
 

src/qt/walletmodel.cpp

@@ -502,6 +502,7 @@ void WalletModel::listCoins(std::map<QString, std::vector<COutput> >& mapCoins)
     std::vector<COutput> vCoins;
     wallet->AvailableCoins(vCoins);
 
+    LOCK(wallet->cs_wallet); // ListLockedCoins, mapWallet
     std::vector<COutPoint> vLockedCoins;
     wallet->ListLockedCoins(vLockedCoins);
 
@@ -531,20 +532,24 @@ void WalletModel::listCoins(std::map<QString, std::vector<COutput> >& mapCoins)
 
 bool WalletModel::isLockedCoin(uint256 hash, unsigned int n) const
 {
+    LOCK(wallet->cs_wallet);
     return wallet->IsLockedCoin(hash, n);
 }
 
 void WalletModel::lockCoin(COutPoint& output)
 {
+    LOCK(wallet->cs_wallet);
     wallet->LockCoin(output);
 }
 
 void WalletModel::unlockCoin(COutPoint& output)
 {
+    LOCK(wallet->cs_wallet);
     wallet->UnlockCoin(output);
 }
 
 void WalletModel::listLockedCoins(std::vector<COutPoint>& vOutpts)
 {
+    LOCK(wallet->cs_wallet);
     wallet->ListLockedCoins(vOutpts);
 }