Merge #10516: Backports for 0.14.3

ff274d3 [doc] Add hint about getmempoolentry to getrawmempool help. (Karl-Johan Alm)
76f9cf9 contrib: Update location of seeds.txt (Wladimir J. van der Laan)
12adedf Trivial: remove extra character from comment (CryptAxe)
d2ec969 Fixed typo in documentation for merkleblock.h (Mikerah)
3612219 contrib/init/bitcoind.openrcconf: Don't disable wallet by default (Luke Dashjr)
692dbb0 [doc] Minor corrections to osx dependencies (fanquake)
87a21d5 Fix: make CCoinsViewDbCursor::Seek work for missing keys (Pieter Wuille)
28b8b8b [wallet] Securely erase potentially sensitive keys/values (Thomas Snider)
ff13f59 [wallet] Make sure pindex is non-null before possibly referencing in LogPrintf call. (Karl-Johan Alm)
e23cef0 Fix some empty vector references (Pieter Wuille)
6ad45b8 Re-enable upnp support in contrib/debian (Matt Corallo)
e9a0d89 Build with QT5 on Debian-based systems using contrib/debian (Matt Corallo)
2ea0358 Bump minimum boost version in contrib/debian (Matt Corallo)
c94e262 Update contrib/debian to latest Ubuntu PPA upload. (Matt Corallo)
96c7f2c Add CheckQueue Tests (Jeremy Rubin)
e207342 Fix CCheckQueue IsIdle (potential) race condition and remove dangerous constructors. (Jeremy Rubin)
ef810c4 [trivial] Fix a typo (introduced two days ago) in the default fee warning (practicalswift)
7abe7bb Qt/Send: Give fallback fee a reasonable indent (Luke Dashjr)
3e4d7bf Qt/Send: Figure a decent warning colour from theme (Luke Dashjr)
c5adf8f [Qt] Show more significant warning if we fall back to the default fee (Jonas Schnelli)
ee1a60d [tests] update disconnect_ban.py test case to work with listbanned (John Newbery)
d289b56 [net] listbanned RPC and QT should show correct banned subnets (John Newbery)
0422693 [tests] disconnect_ban: remove dependency on urllib (John Newbery)
98bd0c3 [tests] disconnect_ban: use wait_until instead of sleep (John Newbery)
bfd1cf6 [tests] disconnectban test - only use two nodes (John Newbery)
5bc75bb [tests] fix nodehandling.py flake8 warnings (John Newbery)
c25d0a8 Update release notes to include RPC error code changes. (John Newbery)
f5efe82 Return correct error codes in fundrawtransaction(). (John Newbery)
4943d7a Return correct error codes in setban(). (John Newbery)
18c109d Return correct error codes in removeprunedfunds(). (John Newbery)
fe51c89 Return correct error codes in blockchain.cpp. (John Newbery)
3ad00b4 Return correct error codes in bumpfee(). (John Newbery)
71463a7 [qa] Test prioritise_transaction / getblocktemplate interaction (Suhas Daftuar)
d28d583 Bugfix: PrioritiseTransaction updates the mempool tx counter (Suhas Daftuar)

Tree-SHA512: fa3628527c8e176e438de992b9c5815cc2f3c296dbe5d81b592d17a907554e9c6af7eb595e96a2c345de399ba5326c07b4791a91b7b07f89dce0787c85891206

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
     }