19 files changed, 560 insertions, 67 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 4d44b35bb..99c25a1d4 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -89,6 +89,7 @@ BITCOIN_TESTS =\
   test/blockencodings_tests.cpp \
   test/bloom_tests.cpp \
   test/bswap_tests.cpp \
+  test/checkqueue_tests.cpp \
   test/coins_tests.cpp \
   test/compress_tests.cpp \
   test/crypto_tests.cpp \
diff --git a/src/checkqueue.h b/src/checkqueue.h
index 32e25d5c8..ea12df66d 100644
--- a/src/checkqueue.h
+++ b/src/checkqueue.h
@@ -127,6 +127,9 @@ private:
     }
 
 public:
+    //! Mutex to ensure only one concurrent CCheckQueueControl
+    boost::mutex ControlMutex;
+
     //! Create a new check queue
     CCheckQueue(unsigned int nBatchSizeIn) : nIdle(0), nTotal(0), fAllOk(true), nTodo(0), fQuit(false), nBatchSize(nBatchSizeIn) {}
 
@@ -161,12 +164,6 @@ public:
     {
     }
 
-    bool IsIdle()
-    {
-        boost::unique_lock<boost::mutex> lock(mutex);
-        return (nTotal == nIdle && nTodo == 0 && fAllOk == true);
-    }
-
 };
 
 /** 
@@ -177,16 +174,18 @@ template <typename T>
 class CCheckQueueControl
 {
 private:
-    CCheckQueue<T>* pqueue;
+    CCheckQueue<T> * const pqueue;
     bool fDone;
 
 public:
-    CCheckQueueControl(CCheckQueue<T>* pqueueIn) : pqueue(pqueueIn), fDone(false)
+    CCheckQueueControl() = delete;
+    CCheckQueueControl(const CCheckQueueControl&) = delete;
+    CCheckQueueControl& operator=(const CCheckQueueControl&) = delete;
+    explicit CCheckQueueControl(CCheckQueue<T> * const pqueueIn) : pqueue(pqueueIn), fDone(false)
     {
         // passed queue is supposed to be unused, or NULL
         if (pqueue != NULL) {
-            bool isIdle = pqueue->IsIdle();
-            assert(isIdle);
+            ENTER_CRITICAL_SECTION(pqueue->ControlMutex);
         }
     }
 
@@ -209,6 +208,9 @@ public:
     {
         if (!fDone)
             Wait();
+        if (pqueue != NULL) {
+            LEAVE_CRITICAL_SECTION(pqueue->ControlMutex);
+        }
     }
 };
 
diff --git a/src/merkleblock.cpp b/src/merkleblock.cpp
index e3f3e4621..78d7cd600 100644
--- a/src/merkleblock.cpp
+++ b/src/merkleblock.cpp
@@ -65,7 +65,7 @@ uint256 CPartialMerkleTree::CalcHash(int height, unsigned int pos, const std::ve
     } else {
         // calculate left hash
         uint256 left = CalcHash(height-1, pos*2, vTxid), right;
-        // calculate right hash if not beyond the end of the array - copy left hash otherwise1
+        // calculate right hash if not beyond the end of the array - copy left hash otherwise
         if (pos*2+1 < CalcTreeWidth(height-1))
             right = CalcHash(height-1, pos*2+1, vTxid);
         else
diff --git a/src/merkleblock.h b/src/merkleblock.h
index 73cbf670e..de4c5c8d2 100644
--- a/src/merkleblock.h
+++ b/src/merkleblock.h
@@ -23,7 +23,7 @@
  * storing a bit for each traversed node, signifying whether the node is the
  * parent of at least one matched leaf txid (or a matched txid itself). In
  * case we are at the leaf level, or this bit is 0, its merkle node hash is
- * stored, and its children are not explorer further. Otherwise, no hash is
+ * stored, and its children are not explored further. Otherwise, no hash is
  * stored, but we recurse into both (or the only) child branch. During
  * decoding, the same depth-first traversal is performed, consuming bits and
  * hashes as they written during encoding.
diff --git a/src/net.cpp b/src/net.cpp
index 1752c7707..5bc886c34 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -413,10 +413,10 @@ void CConnman::DumpBanlist()
 
     CBanDB bandb;
     banmap_t banmap;
-    SetBannedSetDirty(false);
     GetBanned(banmap);
-    if (!bandb.Write(banmap))
-        SetBannedSetDirty(true);
+    if (bandb.Write(banmap)) {
+        SetBannedSetDirty(false);
+    }
 
     LogPrint("net", "Flushed %d banned node ips/subnets to banlist.dat  %dms\n",
         banmap.size(), GetTimeMillis() - nStart);
@@ -536,6 +536,8 @@ bool CConnman::Unban(const CSubNet &subNet) {
 void CConnman::GetBanned(banmap_t &banMap)
 {
     LOCK(cs_setBanned);
+    // Sweep the banlist so expired bans are not returned
+    SweepBanned();
     banMap = setBanned; //create a thread safe copy
 }
 
diff --git a/src/qt/forms/sendcoinsdialog.ui b/src/qt/forms/sendcoinsdialog.ui
index ca8ecffaf..f13b0d9cf 100644
--- a/src/qt/forms/sendcoinsdialog.ui
+++ b/src/qt/forms/sendcoinsdialog.ui
@@ -760,10 +760,32 @@
            </layout>
           </item>
           <item>
+           <widget class="QLabel" name="fallbackFeeWarningLabel">
+            <property name="toolTip">
+             <string>Using the fallbackfee can result in sending a transaction that will take several hours or days (or never) to confirm. Consider choosing your fee manually or wait until your have validated the complete chain.</string>
+            </property>
+            <property name="font">
+            <font>
+                <weight>75</weight>
+                <bold>true</bold>
+            </font>
+            </property>
+            <property name="text">
+             <string>Warning: Fee estimation is currently not possible.</string>
+            </property>
+            <property name="wordWrap">
+             <bool>false</bool>
+            </property>
+           </widget>
+          </item>
+          <item>
            <spacer name="horizontalSpacer_4">
             <property name="orientation">
              <enum>Qt::Horizontal</enum>
             </property>
+            <property name="sizeType">
+             <enum>QSizePolicy::MinimumExpanding</enum>
+            </property>
             <property name="sizeHint" stdset="0">
              <size>
               <width>40</width>
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index 1c0ed663c..65ca00755 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -23,6 +23,7 @@
 #include "txmempool.h"
 #include "wallet/wallet.h"
 
+#include <QFontMetrics>
 #include <QMessageBox>
 #include <QScrollBar>
 #include <QSettings>
@@ -656,6 +657,11 @@ void SendCoinsDialog::updateSmartFeeLabel()
                                                                 std::max(CWallet::fallbackFee.GetFeePerK(), CWallet::GetRequiredFee(1000))) + "/kB");
         ui->labelSmartFee2->show(); // (Smart fee not initialized yet. This usually takes a few blocks...)
         ui->labelFeeEstimation->setText("");
+        ui->fallbackFeeWarningLabel->setVisible(true);
+        int lightness = ui->fallbackFeeWarningLabel->palette().color(QPalette::WindowText).lightness();
+        QColor warning_colour(255 - (lightness / 5), 176 - (lightness / 3), 48 - (lightness / 14));
+        ui->fallbackFeeWarningLabel->setStyleSheet("QLabel { color: " + warning_colour.name() + "; }");
+        ui->fallbackFeeWarningLabel->setIndent(QFontMetrics(ui->fallbackFeeWarningLabel->font()).width("x"));
     }
     else
     {
@@ -663,6 +669,7 @@ void SendCoinsDialog::updateSmartFeeLabel()
                                                                 std::max(feeRate.GetFeePerK(), CWallet::GetRequiredFee(1000))) + "/kB");
         ui->labelSmartFee2->hide();
         ui->labelFeeEstimation->setText(tr("Estimated to begin confirmation within %n block(s).", "", estimateFoundAtBlocks));
+        ui->fallbackFeeWarningLabel->setVisible(false);
     }
 
     updateFeeMinimizedLabel();
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 7b69c81ff..fd8f52a5c 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -417,6 +417,7 @@ UniValue getrawmempool(const JSONRPCRequest& request)
         throw runtime_error(
             "getrawmempool ( verbose )\n"
             "\nReturns all transaction ids in memory pool as a json array of string transaction ids.\n"
+            "\nHint: use getmempoolentry to fetch a specific transaction from the mempool.\n"
             "\nArguments:\n"
             "1. verbose (boolean, optional, default=false) True for a json object, false for array of transaction ids\n"
             "\nResult: (for verbose = false):\n"
@@ -744,10 +745,15 @@ UniValue getblock(const JSONRPCRequest& request)
     CBlockIndex* pblockindex = mapBlockIndex[hash];
 
     if (fHavePruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0)
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Block not available (pruned data)");
+        throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)");
 
-    if(!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus()))
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk");
+    if (!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus()))
+        // Block not found on disk. This could be because we have the block
+        // header in our index but don't have the block (for example if a
+        // non-whitelisted node sends us an unrequested long chain of valid
+        // blocks, we add the headers to our index, but don't accept the
+        // block).
+        throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk");
 
     if (!fVerbose)
     {
@@ -829,7 +835,7 @@ UniValue pruneblockchain(const JSONRPCRequest& request)
             + HelpExampleRpc("pruneblockchain", "1000"));
 
     if (!fPruneMode)
-        throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Cannot prune blocks because node is not in prune mode.");
+        throw JSONRPCError(RPC_MISC_ERROR, "Cannot prune blocks because node is not in prune mode.");
 
     LOCK(cs_main);
 
@@ -843,7 +849,7 @@ UniValue pruneblockchain(const JSONRPCRequest& request)
         // Add a 2 hour buffer to include blocks which might have had old timestamps
         CBlockIndex* pindex = chainActive.FindEarliestAtLeast(heightParam - 7200);
         if (!pindex) {
-            throw JSONRPCError(RPC_INTERNAL_ERROR, "Could not find block with at least the specified timestamp.");
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Could not find block with at least the specified timestamp.");
         }
         heightParam = pindex->nHeight;
     }
@@ -851,7 +857,7 @@ UniValue pruneblockchain(const JSONRPCRequest& request)
     unsigned int height = (unsigned int) heightParam;
     unsigned int chainHeight = (unsigned int) chainActive.Height();
     if (chainHeight < Params().PruneAfterHeight())
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Blockchain is too short for pruning.");
+        throw JSONRPCError(RPC_MISC_ERROR, "Blockchain is too short for pruning.");
     else if (height > chainHeight)
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Blockchain is shorter than the attempted prune height.");
     else if (height > chainHeight - MIN_BLOCKS_TO_KEEP) {
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 8706b4295..42f5db94f 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -506,7 +506,7 @@ UniValue setban(const JSONRPCRequest& request)
         LookupSubNet(request.params[0].get_str().c_str(), subNet);
 
     if (! (isSubnet ? subNet.IsValid() : netAddr.IsValid()) )
-        throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: Invalid IP/Subnet");
+        throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Invalid IP/Subnet");
 
     if (strCommand == "add")
     {
@@ -526,7 +526,7 @@ UniValue setban(const JSONRPCRequest& request)
     else if(strCommand == "remove")
     {
         if (!( isSubnet ? g_connman->Unban(subNet) : g_connman->Unban(netAddr) ))
-            throw JSONRPCError(RPC_MISC_ERROR, "Error: Unban failed");
+            throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned.");
     }
     return NullUniValue;
 }
diff --git a/src/streams.h b/src/streams.h
index 1d3b55c91..e49a9e354 100644
--- a/src/streams.h
+++ b/src/streams.h
@@ -248,7 +248,8 @@ public:
 
     void insert(iterator it, std::vector<char>::const_iterator first, std::vector<char>::const_iterator last)
     {
-        assert(last - first >= 0);
+        if (last == first) return;
+        assert(last - first > 0);
         if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos)
         {
             // special case for inserting at the front when there's room
@@ -261,7 +262,8 @@ public:
 
     void insert(iterator it, const char* first, const char* last)
     {
-        assert(last - first >= 0);
+        if (last == first) return;
+        assert(last - first > 0);
         if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos)
         {
             // special case for inserting at the front when there's room
@@ -339,6 +341,8 @@ public:
 
     void read(char* pch, size_t nSize)
     {
+        if (nSize == 0) return;
+
         // Read from the beginning of the buffer
         unsigned int nReadPosNext = nReadPos + nSize;
         if (nReadPosNext >= vch.size())
diff --git a/src/support/cleanse.h b/src/support/cleanse.h
index 3e02aa8fd..f020216c7 100644
--- a/src/support/cleanse.h
+++ b/src/support/cleanse.h
@@ -8,6 +8,7 @@
 
 #include <stdlib.h>
 
+// Attempt to overwrite data in the specified memory span.
 void memory_cleanse(void *ptr, size_t len);
 
 #endif // BITCOIN_SUPPORT_CLEANSE_H
diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp
new file mode 100644
index 000000000..d89f9b770
--- /dev/null
+++ b/src/test/checkqueue_tests.cpp
@@ -0,0 +1,442 @@
+// Copyright (c) 2012-2017 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include "util.h"
+#include "utiltime.h"
+#include "validation.h"
+
+#include "test/test_bitcoin.h"
+#include "checkqueue.h"
+#include <boost/test/unit_test.hpp>
+#include <boost/thread.hpp>
+#include <atomic>
+#include <thread>
+#include <vector>
+#include <mutex>
+#include <condition_variable>
+
+#include <unordered_set>
+#include <memory>
+#include "random.h"
+
+// BasicTestingSetup not sufficient because nScriptCheckThreads is not set
+// otherwise.
+BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, TestingSetup)
+
+static const int QUEUE_BATCH_SIZE = 128;
+
+struct FakeCheck {
+    bool operator()()
+    {
+        return true;
+    }
+    void swap(FakeCheck& x){};
+};
+
+struct FakeCheckCheckCompletion {
+    static std::atomic<size_t> n_calls;
+    bool operator()()
+    {
+        ++n_calls;
+        return true;
+    }
+    void swap(FakeCheckCheckCompletion& x){};
+};
+
+struct FailingCheck {
+    bool fails;
+    FailingCheck(bool fails) : fails(fails){};
+    FailingCheck() : fails(true){};
+    bool operator()()
+    {
+        return !fails;
+    }
+    void swap(FailingCheck& x)
+    {
+        std::swap(fails, x.fails);
+    };
+};
+
+struct UniqueCheck {
+    static std::mutex m;
+    static std::unordered_multiset<size_t> results;
+    size_t check_id;
+    UniqueCheck(size_t check_id_in) : check_id(check_id_in){};
+    UniqueCheck() : check_id(0){};
+    bool operator()()
+    {
+        std::lock_guard<std::mutex> l(m);
+        results.insert(check_id);
+        return true;
+    }
+    void swap(UniqueCheck& x) { std::swap(x.check_id, check_id); };
+};
+
+
+struct MemoryCheck {
+    static std::atomic<size_t> fake_allocated_memory;
+    bool b {false};
+    bool operator()()
+    {
+        return true;
+    }
+    MemoryCheck(){};
+    MemoryCheck(const MemoryCheck& x)
+    {
+        // We have to do this to make sure that destructor calls are paired
+        //
+        // Really, copy constructor should be deletable, but CCheckQueue breaks
+        // if it is deleted because of internal push_back.
+        fake_allocated_memory += b;
+    };
+    MemoryCheck(bool b_) : b(b_)
+    {
+        fake_allocated_memory += b;
+    };
+    ~MemoryCheck(){
+        fake_allocated_memory -= b;
+    
+    };
+    void swap(MemoryCheck& x) { std::swap(b, x.b); };
+};
+
+struct FrozenCleanupCheck {
+    static std::atomic<uint64_t> nFrozen;
+    static std::condition_variable cv;
+    static std::mutex m;
+    // Freezing can't be the default initialized behavior given how the queue
+    // swaps in default initialized Checks.
+    bool should_freeze {false};
+    bool operator()()
+    {
+        return true;
+    }
+    FrozenCleanupCheck() {}
+    ~FrozenCleanupCheck()
+    {
+        if (should_freeze) {
+            std::unique_lock<std::mutex> l(m);
+            nFrozen = 1;
+            cv.notify_one();
+            cv.wait(l, []{ return nFrozen == 0;});
+        }
+    }
+    void swap(FrozenCleanupCheck& x){std::swap(should_freeze, x.should_freeze);};
+};
+
+// Static Allocations
+std::mutex FrozenCleanupCheck::m{};
+std::atomic<uint64_t> FrozenCleanupCheck::nFrozen{0};
+std::condition_variable FrozenCleanupCheck::cv{};
+std::mutex UniqueCheck::m;
+std::unordered_multiset<size_t> UniqueCheck::results;
+std::atomic<size_t> FakeCheckCheckCompletion::n_calls{0};
+std::atomic<size_t> MemoryCheck::fake_allocated_memory{0};
+
+// Queue Typedefs
+typedef CCheckQueue<FakeCheckCheckCompletion> Correct_Queue;
+typedef CCheckQueue<FakeCheck> Standard_Queue;
+typedef CCheckQueue<FailingCheck> Failing_Queue;
+typedef CCheckQueue<UniqueCheck> Unique_Queue;
+typedef CCheckQueue<MemoryCheck> Memory_Queue;
+typedef CCheckQueue<FrozenCleanupCheck> FrozenCleanup_Queue;
+
+
+/** This test case checks that the CCheckQueue works properly
+ * with each specified size_t Checks pushed.
+ */
+void Correct_Queue_range(std::vector<size_t> range)
+{
+    auto small_queue = std::unique_ptr<Correct_Queue>(new Correct_Queue {QUEUE_BATCH_SIZE});
+    boost::thread_group tg;
+    for (auto x = 0; x < nScriptCheckThreads; ++x) {
+       tg.create_thread([&]{small_queue->Thread();});
+    }
+    // Make vChecks here to save on malloc (this test can be slow...)
+    std::vector<FakeCheckCheckCompletion> vChecks;
+    for (auto i : range) {
+        size_t total = i;
+        FakeCheckCheckCompletion::n_calls = 0;
+        CCheckQueueControl<FakeCheckCheckCompletion> control(small_queue.get());
+        while (total) {
+            vChecks.resize(std::min(total, (size_t) GetRand(10)));
+            total -= vChecks.size();
+            control.Add(vChecks);
+        }
+        BOOST_REQUIRE(control.Wait());
+        if (FakeCheckCheckCompletion::n_calls != i) {
+            BOOST_REQUIRE_EQUAL(FakeCheckCheckCompletion::n_calls, i);
+            BOOST_TEST_MESSAGE("Failure on trial " << i << " expected, got " << FakeCheckCheckCompletion::n_calls);
+        }
+    }
+    tg.interrupt_all();
+    tg.join_all();
+}
+
+/** Test that 0 checks is correct
+ */
+BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Zero)
+{
+    std::vector<size_t> range;
+    range.push_back((size_t)0);
+    Correct_Queue_range(range);
+}
+/** Test that 1 check is correct
+ */
+BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_One)
+{
+    std::vector<size_t> range;
+    range.push_back((size_t)1);
+    Correct_Queue_range(range);
+}
+/** Test that MAX check is correct
+ */
+BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Max)
+{
+    std::vector<size_t> range;
+    range.push_back(100000);
+    Correct_Queue_range(range);
+}
+/** Test that random numbers of checks are correct
+ */
+BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Random)
+{
+    std::vector<size_t> range;
+    range.reserve(100000/1000);
+    for (size_t i = 2; i < 100000; i += std::max((size_t)1, (size_t)GetRand(std::min((size_t)1000, ((size_t)100000) - i))))
+        range.push_back(i);
+    Correct_Queue_range(range);
+}
+
+
+/** Test that failing checks are caught */
+BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure)
+{
+    auto fail_queue = std::unique_ptr<Failing_Queue>(new Failing_Queue {QUEUE_BATCH_SIZE});
+
+    boost::thread_group tg;
+    for (auto x = 0; x < nScriptCheckThreads; ++x) {
+       tg.create_thread([&]{fail_queue->Thread();});
+    }
+
+    for (size_t i = 0; i < 1001; ++i) {
+        CCheckQueueControl<FailingCheck> control(fail_queue.get());
+        size_t remaining = i;
+        while (remaining) {
+            size_t r = GetRand(10);
+
+            std::vector<FailingCheck> vChecks;
+            vChecks.reserve(r);
+            for (size_t k = 0; k < r && remaining; k++, remaining--)
+                vChecks.emplace_back(remaining == 1);
+            control.Add(vChecks);
+        }
+        bool success = control.Wait();
+        if (i > 0) {
+            BOOST_REQUIRE(!success);
+        } else if (i == 0) {
+            BOOST_REQUIRE(success);
+        }
+    }
+    tg.interrupt_all();
+    tg.join_all();
+}
+// Test that a block validation which fails does not interfere with
+// future blocks, ie, the bad state is cleared.
+BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure)
+{
+    auto fail_queue = std::unique_ptr<Failing_Queue>(new Failing_Queue {QUEUE_BATCH_SIZE});
+    boost::thread_group tg;
+    for (auto x = 0; x < nScriptCheckThreads; ++x) {
+       tg.create_thread([&]{fail_queue->Thread();});
+    }
+
+    for (auto times = 0; times < 10; ++times) {
+        for (bool end_fails : {true, false}) {
+            CCheckQueueControl<FailingCheck> control(fail_queue.get());
+            {
+                std::vector<FailingCheck> vChecks;
+                vChecks.resize(100, false);
+                vChecks[99] = end_fails;
+                control.Add(vChecks);
+            }
+            bool r =control.Wait();
+            BOOST_REQUIRE(r || end_fails);
+        }
+    }
+    tg.interrupt_all();
+    tg.join_all();
+}
+
+// Test that unique checks are actually all called individually, rather than
+// just one check being called repeatedly. Test that checks are not called
+// more than once as well
+BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
+{
+    auto queue = std::unique_ptr<Unique_Queue>(new Unique_Queue {QUEUE_BATCH_SIZE});
+    boost::thread_group tg;
+    for (auto x = 0; x < nScriptCheckThreads; ++x) {
+       tg.create_thread([&]{queue->Thread();});
+
+    }
+
+    size_t COUNT = 100000;
+    size_t total = COUNT;
+    {
+        CCheckQueueControl<UniqueCheck> control(queue.get());
+        while (total) {
+            size_t r = GetRand(10);
+            std::vector<UniqueCheck> vChecks;
+            for (size_t k = 0; k < r && total; k++)
+                vChecks.emplace_back(--total);
+            control.Add(vChecks);
+        }
+    }
+    bool r = true;
+    BOOST_REQUIRE_EQUAL(UniqueCheck::results.size(), COUNT);
+    for (size_t i = 0; i < COUNT; ++i)
+        r = r && UniqueCheck::results.count(i) == 1;
+    BOOST_REQUIRE(r);
+    tg.interrupt_all();
+    tg.join_all();
+}
+
+
+// Test that blocks which might allocate lots of memory free their memory agressively.
+//
+// This test attempts to catch a pathological case where by lazily freeing
+// checks might mean leaving a check un-swapped out, and decreasing by 1 each
+// time could leave the data hanging across a sequence of blocks.
+BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory)
+{
+    auto queue = std::unique_ptr<Memory_Queue>(new Memory_Queue {QUEUE_BATCH_SIZE});
+    boost::thread_group tg;
+    for (auto x = 0; x < nScriptCheckThreads; ++x) {
+       tg.create_thread([&]{queue->Thread();});
+    }
+    for (size_t i = 0; i < 1000; ++i) {
+        size_t total = i;
+        {
+            CCheckQueueControl<MemoryCheck> control(queue.get());
+            while (total) {
+                size_t r = GetRand(10);
+                std::vector<MemoryCheck> vChecks;
+                for (size_t k = 0; k < r && total; k++) {
+                    total--;
+                    // Each iteration leaves data at the front, back, and middle
+                    // to catch any sort of deallocation failure
+                    vChecks.emplace_back(total == 0 || total == i || total == i/2);
+                }
+                control.Add(vChecks);
+            }
+        }
+        BOOST_REQUIRE_EQUAL(MemoryCheck::fake_allocated_memory, 0);
+    }
+    tg.interrupt_all();
+    tg.join_all();
+}
+
+// Test that a new verification cannot occur until all checks 
+// have been destructed
+BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup)
+{
+    auto queue = std::unique_ptr<FrozenCleanup_Queue>(new FrozenCleanup_Queue {QUEUE_BATCH_SIZE});
+    boost::thread_group tg;
+    bool fails = false;
+    for (auto x = 0; x < nScriptCheckThreads; ++x) {
+        tg.create_thread([&]{queue->Thread();});
+    }
+    std::thread t0([&]() {
+        CCheckQueueControl<FrozenCleanupCheck> control(queue.get());
+        std::vector<FrozenCleanupCheck> vChecks(1);
+        // Freezing can't be the default initialized behavior given how the queue
+        // swaps in default initialized Checks (otherwise freezing destructor
+        // would get called twice).
+        vChecks[0].should_freeze = true;
+        control.Add(vChecks);
+        control.Wait(); // Hangs here
+    });
+    {
+        std::unique_lock<std::mutex> l(FrozenCleanupCheck::m);
+        // Wait until the queue has finished all jobs and frozen
+        FrozenCleanupCheck::cv.wait(l, [](){return FrozenCleanupCheck::nFrozen == 1;});
+        // Try to get control of the queue a bunch of times
+        for (auto x = 0; x < 100 && !fails; ++x) {
+            fails = queue->ControlMutex.try_lock();
+        }
+        // Unfreeze
+        FrozenCleanupCheck::nFrozen = 0;
+    }
+    // Awaken frozen destructor
+    FrozenCleanupCheck::cv.notify_one();
+    // Wait for control to finish
+    t0.join();
+    tg.interrupt_all();
+    tg.join_all();
+    BOOST_REQUIRE(!fails);
+}
+
+
+/** Test that CCheckQueueControl is threadsafe */
+BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
+{
+    auto queue = std::unique_ptr<Standard_Queue>(new Standard_Queue{QUEUE_BATCH_SIZE});
+    {
+        boost::thread_group tg;
+        std::atomic<int> nThreads {0};
+        std::atomic<int> fails {0};
+        for (size_t i = 0; i < 3; ++i) {
+            tg.create_thread(
+                    [&]{
+                    CCheckQueueControl<FakeCheck> control(queue.get());
+                    // While sleeping, no other thread should execute to this point
+                    auto observed = ++nThreads;
+                    MilliSleep(10);
+                    fails += observed  != nThreads;
+                    });
+        }
+        tg.join_all();
+        BOOST_REQUIRE_EQUAL(fails, 0);
+    }
+    {
+        boost::thread_group tg;
+        std::mutex m;
+        bool has_lock {false};
+        bool has_tried {false};
+        bool done {false};
+        bool done_ack {false};
+        std::condition_variable cv;
+        {
+            std::unique_lock<std::mutex> l(m);
+            tg.create_thread([&]{
+                    CCheckQueueControl<FakeCheck> control(queue.get());
+                    std::unique_lock<std::mutex> l(m);
+                    has_lock = true;
+                    cv.notify_one();
+                    cv.wait(l, [&]{return has_tried;});
+                    done = true;
+                    cv.notify_one();
+                    // Wait until the done is acknowledged
+                    //
+                    cv.wait(l, [&]{return done_ack;});
+                    });
+            // Wait for thread to get the lock
+            cv.wait(l, [&](){return has_lock;});
+            bool fails = false;
+            for (auto x = 0; x < 100 && !fails; ++x) {
+                fails = queue->ControlMutex.try_lock();
+            }
+            has_tried = true;
+            cv.notify_one();
+            cv.wait(l, [&](){return done;});
+            // Acknowledge the done
+            done_ack = true;
+            cv.notify_one();
+            BOOST_REQUIRE(!fails);
+        }
+        tg.join_all();
+    }
+}
+BOOST_AUTO_TEST_SUITE_END()
+
diff --git a/src/txdb.cpp b/src/txdb.cpp
index 1a30bb58a..662e7cb06 100644
--- a/src/txdb.cpp
+++ b/src/txdb.cpp
@@ -98,7 +98,11 @@ CCoinsViewCursor *CCoinsViewDB::Cursor() const
        that restriction.  */
     i->pcursor->Seek(DB_COINS);
     // Cache key of first record
-    i->pcursor->GetKey(i->keyTmp);
+    if (i->pcursor->Valid()) {
+        i->pcursor->GetKey(i->keyTmp);
+    } else {
+        i->keyTmp.first = 0; // Make sure Valid() and GetKey() return false
+    }
     return i;
 }
 
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 91040fb9b..72547b582 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -945,6 +945,7 @@ void CTxMemPool::PrioritiseTransaction(const uint256 hash, const std::string str
             BOOST_FOREACH(txiter descendantIt, setDescendants) {
                 mapTx.modify(descendantIt, update_ancestor_state(0, nFeeDelta, 0, 0));
             }
+            ++nTransactionsUpdated;
         }
     }
     LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", strHash, dPriorityDelta, FormatMoney(nFeeDelta));
diff --git a/src/txmempool.h b/src/txmempool.h
index db1a02455..12c9e59f5 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -432,7 +432,7 @@ class CTxMemPool
 {
 private:
     uint32_t nCheckFrequency; //!< Value n means that n times in 2^32 we check.
-    unsigned int nTransactionsUpdated;
+    unsigned int nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation
     CBlockPolicyEstimator* minerPolicyEstimator;
 
     uint64_t totalTxSize;      //!< sum of all mempool tx's virtual sizes. Differs from serialized tx size since witness data is discounted. Defined in BIP 141.
diff --git a/src/wallet/db.h b/src/wallet/db.h
index b4ce044e7..ea5620032 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -126,22 +126,23 @@ protected:
         Dbt datValue;
         datValue.set_flags(DB_DBT_MALLOC);
         int ret = pdb->get(activeTxn, &datKey, &datValue, 0);
-        memset(datKey.get_data(), 0, datKey.get_size());
-        if (datValue.get_data() == NULL)
-            return false;
-
-        // Unserialize value
-        try {
-            CDataStream ssValue((char*)datValue.get_data(), (char*)datValue.get_data() + datValue.get_size(), SER_DISK, CLIENT_VERSION);
-            ssValue >> value;
-        } catch (const std::exception&) {
-            return false;
+        memory_cleanse(datKey.get_data(), datKey.get_size());
+        bool success = false;
+        if (datValue.get_data() != NULL) {
+            // Unserialize value
+            try {
+                CDataStream ssValue((char*)datValue.get_data(), (char*)datValue.get_data() + datValue.get_size(), SER_DISK, CLIENT_VERSION);
+                ssValue >> value;
+                success = true;
+            } catch (const std::exception&) {
+                // In this case success remains 'false'
+            }
+
+            // Clear and free memory
+            memory_cleanse(datValue.get_data(), datValue.get_size());
+            free(datValue.get_data());
         }
-
-        // Clear and free memory
-        memset(datValue.get_data(), 0, datValue.get_size());
-        free(datValue.get_data());
-        return (ret == 0);
+        return ret == 0 && success;
     }
 
     template <typename K, typename T>
@@ -168,8 +169,8 @@ protected:
         int ret = pdb->put(activeTxn, &datKey, &datValue, (fOverwrite ? 0 : DB_NOOVERWRITE));
 
         // Clear memory in case it was a private key
-        memset(datKey.get_data(), 0, datKey.get_size());
-        memset(datValue.get_data(), 0, datValue.get_size());
+        memory_cleanse(datKey.get_data(), datKey.get_size());
+        memory_cleanse(datValue.get_data(), datValue.get_size());
         return (ret == 0);
     }
 
@@ -191,7 +192,7 @@ protected:
         int ret = pdb->del(activeTxn, &datKey, 0);
 
         // Clear memory
-        memset(datKey.get_data(), 0, datKey.get_size());
+        memory_cleanse(datKey.get_data(), datKey.get_size());
         return (ret == 0 || ret == DB_NOTFOUND);
     }
 
@@ -211,7 +212,7 @@ protected:
         int ret = pdb->exists(activeTxn, &datKey, 0);
 
         // Clear memory
-        memset(datKey.get_data(), 0, datKey.get_size());
+        memory_cleanse(datKey.get_data(), datKey.get_size());
         return (ret == 0);
     }
 
@@ -254,8 +255,8 @@ protected:
         ssValue.write((char*)datValue.get_data(), datValue.get_size());
 
         // Clear and free memory
-        memset(datKey.get_data(), 0, datKey.get_size());
-        memset(datValue.get_data(), 0, datValue.get_size());
+        memory_cleanse(datKey.get_data(), datKey.get_size());
+        memory_cleanse(datValue.get_data(), datValue.get_size());
         free(datKey.get_data());
         free(datValue.get_data());
         return 0;
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index cd6ec207f..dc61a4505 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -340,11 +340,11 @@ UniValue removeprunedfunds(const JSONRPCRequest& request)
     vector<uint256> vHashOut;
 
     if(pwalletMain->ZapSelectTx(vHash, vHashOut) != DB_LOAD_OK) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Could not properly delete the transaction.");
+        throw JSONRPCError(RPC_WALLET_ERROR, "Could not properly delete the transaction.");
     }
 
     if(vHashOut.empty()) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction does not exist in wallet.");
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction does not exist in wallet.");
     }
 
     return NullUniValue;
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 5cf159237..7cc032016 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2603,7 +2603,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
             CBitcoinAddress address(options["changeAddress"].get_str());
 
             if (!address.IsValid())
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "changeAddress must be a valid bitcoin address");
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "changeAddress must be a valid bitcoin address");
 
             changeAddress = address.Get();
         }
@@ -2657,7 +2657,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
     string strFailReason;
 
     if(!pwalletMain->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, setSubtractFeeFromOutputs, reserveChangeKey, changeAddress))
-        throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);
+        throw JSONRPCError(RPC_WALLET_ERROR, strFailReason);
 
     UniValue result(UniValue::VOBJ);
     result.push_back(Pair("hex", EncodeHexTx(tx)));
@@ -2756,33 +2756,33 @@ UniValue bumpfee(const JSONRPCRequest& request)
     CWalletTx& wtx = pwalletMain->mapWallet[hash];
 
     if (pwalletMain->HasWalletSpend(hash)) {
-        throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the wallet");
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the wallet");
     }
 
     {
         LOCK(mempool.cs);
         auto it = mempool.mapTx.find(hash);
         if (it != mempool.mapTx.end() && it->GetCountWithDescendants() > 1) {
-            throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the mempool");
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the mempool");
         }
     }
 
     if (wtx.GetDepthInMainChain() != 0) {
-        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction has been mined, or is conflicted with a mined transaction");
+        throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has been mined, or is conflicted with a mined transaction");
     }
 
     if (!SignalsOptInRBF(wtx)) {
-        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction is not BIP 125 replaceable");
+        throw JSONRPCError(RPC_WALLET_ERROR, "Transaction is not BIP 125 replaceable");
     }
 
     if (wtx.mapValue.count("replaced_by_txid")) {
-        throw JSONRPCError(RPC_INVALID_REQUEST, strprintf("Cannot bump transaction %s which was already bumped by transaction %s", hash.ToString(), wtx.mapValue.at("replaced_by_txid")));
+        throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Cannot bump transaction %s which was already bumped by transaction %s", hash.ToString(), wtx.mapValue.at("replaced_by_txid")));
     }
 
     // check that original tx consists entirely of our inputs
     // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
     if (!pwalletMain->IsAllFromMe(wtx, ISMINE_SPENDABLE)) {
-        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction contains inputs that don't belong to this wallet");
+        throw JSONRPCError(RPC_WALLET_ERROR, "Transaction contains inputs that don't belong to this wallet");
     }
 
     // figure out which output was change
@@ -2791,13 +2791,13 @@ UniValue bumpfee(const JSONRPCRequest& request)
     for (size_t i = 0; i < wtx.tx->vout.size(); ++i) {
         if (pwalletMain->IsChange(wtx.tx->vout[i])) {
             if (nOutput != -1) {
-                throw JSONRPCError(RPC_MISC_ERROR, "Transaction has multiple change outputs");
+                throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has multiple change outputs");
             }
             nOutput = i;
         }
     }
     if (nOutput == -1) {
-        throw JSONRPCError(RPC_MISC_ERROR, "Transaction does not have a change output");
+        throw JSONRPCError(RPC_WALLET_ERROR, "Transaction does not have a change output");
     }
 
     // Calculate the expected size of the new transaction.
@@ -2888,7 +2888,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
 
     // Check that in all cases the new fee doesn't violate maxTxFee
      if (nNewFee > maxTxFee) {
-         throw JSONRPCError(RPC_MISC_ERROR,
+         throw JSONRPCError(RPC_WALLET_ERROR,
                             strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)",
                                       FormatMoney(nNewFee), FormatMoney(maxTxFee)));
      }
@@ -2900,7 +2900,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
     // moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment.
     CFeeRate minMempoolFeeRate = mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
     if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
-        throw JSONRPCError(RPC_MISC_ERROR, strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK())));
+        throw JSONRPCError(RPC_WALLET_ERROR, strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK())));
     }
 
     // Now modify the output to increase the fee.
@@ -2910,7 +2910,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
     CMutableTransaction tx(*(wtx.tx));
     CTxOut* poutput = &(tx.vout[nOutput]);
     if (poutput->nValue < nDelta) {
-        throw JSONRPCError(RPC_MISC_ERROR, "Change output is too small to bump the fee");
+        throw JSONRPCError(RPC_WALLET_ERROR, "Change output is too small to bump the fee");
     }
 
     // If the output would become dust, discard it (converting the dust to fee)
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 3e93a56d5..2697a3769 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1572,6 +1572,10 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f
         {
             if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0)
                 ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((GuessVerificationProgress(chainParams.TxData(), pindex) - dProgressStart) / (dProgressTip - dProgressStart) * 100))));
+            if (GetTime() >= nNow + 60) {
+                nNow = GetTime();
+                LogPrintf("Still rescanning. At block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex));
+            }
 
             CBlock block;
             if (ReadBlockFromDisk(block, pindex, Params().GetConsensus())) {
@@ -1585,10 +1589,6 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f
                 ret = nullptr;
             }
             pindex = chainActive.Next(pindex);
-            if (GetTime() >= nNow + 60) {
-                nNow = GetTime();
-                LogPrintf("Still rescanning. At block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex));
-            }
         }
         ShowProgress(_("Rescanning..."), 100); // hide progress dialog in GUI
     }