Merge pull request #3407 from PastaPastaPasta/backports-0.16-pr17

Backports 0.16 pr17

src/crypto/common.h

@@ -82,12 +82,12 @@ void static inline WriteBE64(unsigned char* ptr, uint64_t x)
 /** Return the smallest number n such that (x >> n) == 0 (or 64 if the highest bit in x is set. */
 uint64_t static inline CountBits(uint64_t x)
 {
-#ifdef HAVE_DECL___BUILTIN_CLZL
+#if HAVE_DECL___BUILTIN_CLZL
     if (sizeof(unsigned long) >= sizeof(uint64_t)) {
         return x ? 8 * sizeof(unsigned long) - __builtin_clzl(x) : 0;
     }
 #endif
-#ifdef HAVE_DECL___BUILTIN_CLZLL
+#if HAVE_DECL___BUILTIN_CLZLL
     if (sizeof(unsigned long long) >= sizeof(uint64_t)) {
         return x ? 8 * sizeof(unsigned long long) - __builtin_clzll(x) : 0;
     }

src/init.cpp

@@ -422,7 +422,7 @@ void OnRPCStopped()
 {
     uiInterface.NotifyBlockTip.disconnect(&RPCNotifyBlockChange);
     RPCNotifyBlockChange(false, nullptr);
-    cvBlockChange.notify_all();
+    g_best_block_cv.notify_all();
     LogPrint(BCLog::RPC, "RPC stopped.\n");
 }
 

src/net.cpp

@@ -102,8 +102,8 @@ bool fDiscover = true;
 bool fListen = true;
 bool fRelayTxes = true;
 CCriticalSection cs_mapLocalHost;
-std::map<CNetAddr, LocalServiceInfo> mapLocalHost;
+std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(cs_mapLocalHost);
-static bool vfLimited[NET_MAX] = {};
+static bool vfLimited[NET_MAX] GUARDED_BY(cs_mapLocalHost) = {};
 std::string strSubVersion;
 
 void CConnman::AddOneShot(const std::string& strDest)
@@ -142,7 +142,7 @@ bool GetLocal(CService& addr, const CNetAddr *paddrPeer)
     return nBestScore >= 0;
 }
 
-//! Convert the pnSeeds6 array into usable address objects.
+//! Convert the pnSeed6 array into usable address objects.
 static std::vector<CAddress> convertSeed6(const std::vector<SeedSpec6> &vSeedsIn)
 {
     // It'll only connect to one or two seed nodes because once it connects,
@@ -907,16 +907,7 @@ const uint256& CNetMessage::GetMessageHash() const
     return data_hash;
 }
 
-
+size_t CConnman::SocketSendData(CNode *pnode) const EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_vSend)
-
-
-
-
-
-
-
-// requires LOCK(cs_vSend)
-size_t CConnman::SocketSendData(CNode *pnode) const
 {
     auto it = pnode->vSendMsg.begin();
     size_t nSentSize = 0;

src/net.h

@@ -530,12 +530,12 @@ private:
 
     std::vector<ListenSocket> vhListenSocket;
     std::atomic<bool> fNetworkActive;
-    banmap_t setBanned;
+    banmap_t setBanned GUARDED_BY(cs_setBanned);
     CCriticalSection cs_setBanned;
-    bool setBannedIsDirty;
+    bool setBannedIsDirty GUARDED_BY(cs_setBanned);
     bool fAddressesInitialized;
     CAddrMan addrman;
-    std::deque<std::string> vOneShots;
+    std::deque<std::string> vOneShots GUARDED_BY(cs_vOneShots);
     CCriticalSection cs_vOneShots;
     std::vector<std::string> vAddedNodes GUARDED_BY(cs_vAddedNodes);
     CCriticalSection cs_vAddedNodes;
@@ -670,7 +670,7 @@ struct LocalServiceInfo {
 };
 
 extern CCriticalSection cs_mapLocalHost;
-extern std::map<CNetAddr, LocalServiceInfo> mapLocalHost;
+extern std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(cs_mapLocalHost);
 typedef std::map<std::string, uint64_t> mapMsgCmdSize; //command, total bytes
 
 class CNodeStats
@@ -762,23 +762,23 @@ class CNode
 public:
     // socket
     std::atomic<ServiceFlags> nServices;
-    SOCKET hSocket;
+    SOCKET hSocket GUARDED_BY(cs_hSocket);
     size_t nSendSize; // total size of all vSendMsg entries
     size_t nSendOffset; // offset inside the first vSendMsg already sent
-    uint64_t nSendBytes;
+    uint64_t nSendBytes GUARDED_BY(cs_vSend);
-    std::list<std::vector<unsigned char>> vSendMsg;
+    std::list<std::vector<unsigned char>> vSendMsg GUARDED_BY(cs_vSend);
     CCriticalSection cs_vSend;
     CCriticalSection cs_hSocket;
     CCriticalSection cs_vRecv;
 
     CCriticalSection cs_vProcessMsg;
-    std::list<CNetMessage> vProcessMsg;
+    std::list<CNetMessage> vProcessMsg GUARDED_BY(cs_vProcessMsg);
     size_t nProcessQueueSize;
 
     CCriticalSection cs_sendProcessing;
 
     std::deque<CInv> vRecvGetData;
-    uint64_t nRecvBytes;
+    uint64_t nRecvBytes GUARDED_BY(cs_vRecv);
     std::atomic<int> nRecvVersion;
 
     std::atomic<int64_t> nLastSend;
@@ -798,7 +798,7 @@ public:
     // to be printed out, displayed to humans in various forms and so on. So we sanitize it and
     // store the sanitized version in cleanSubVer. The original should be used when dealing with
     // the network or wire types and the cleaned string used when displayed or logged.
-    std::string strSubVer, cleanSubVer;
+    std::string strSubVer GUARDED_BY(cs_SubVer), cleanSubVer GUARDED_BY(cs_SubVer);
     CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer
     bool fWhitelisted; // This peer can bypass DoS banning.
     bool fFeeler; // If true this node is being used as a short lived feeler.
@@ -821,7 +821,7 @@ public:
     bool fMasternodeProbe;
     CSemaphoreGrant grantOutbound;
     CCriticalSection cs_filter;
-    std::unique_ptr<CBloomFilter> pfilter;
+    std::unique_ptr<CBloomFilter> pfilter PT_GUARDED_BY(cs_filter);
     std::atomic<int> nRefCount;
 
     const uint64_t nKeyedNetGroup;
@@ -831,7 +831,7 @@ public:
 protected:
 
     mapMsgCmdSize mapSendBytesPerMsgCmd;
-    mapMsgCmdSize mapRecvBytesPerMsgCmd;
+    mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv);
 
 public:
     uint256 hashContinue;
@@ -842,18 +842,18 @@ public:
     CRollingBloomFilter addrKnown;
     bool fGetAddr;
     std::set<uint256> setKnown;
-    int64_t nNextAddrSend;
+    int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing);
-    int64_t nNextLocalAddrSend;
+    int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing);
 
     // inventory based relay
-    CRollingBloomFilter filterInventoryKnown;
+    CRollingBloomFilter filterInventoryKnown GUARDED_BY(cs_inventory);
     // Set of transaction ids we still have to announce.
     // They are sorted by the mempool before relay, so the order is not important.
     std::set<uint256> setInventoryTxToSend;
     // List of block ids we still have announce.
     // There is no final sorting before sending, as they are always sent immediately
     // and in the order requested.
-    std::vector<uint256> vInventoryBlockToSend;
+    std::vector<uint256> vInventoryBlockToSend GUARDED_BY(cs_inventory);
     // List of non-tx/non-block inventory items
     std::vector<CInv> vInventoryOtherToSend;
     CCriticalSection cs_inventory;
@@ -914,10 +914,10 @@ private:
     std::list<CNetMessage> vRecvMsg;  // Used only by SocketHandler thread
 
     mutable CCriticalSection cs_addrName;
-    std::string addrName;
+    std::string addrName GUARDED_BY(cs_addrName);
 
     // Our address, as reported by the peer
-    CService addrLocal;
+    CService addrLocal GUARDED_BY(cs_addrLocal);
     mutable CCriticalSection cs_addrLocal;
 public:
 

src/netbase.cpp

@@ -29,9 +29,9 @@
 #endif
 
 // Settings
-static proxyType proxyInfo[NET_MAX];
-static proxyType nameProxy;
 static CCriticalSection cs_proxyInfos;
+static proxyType proxyInfo[NET_MAX] GUARDED_BY(cs_proxyInfos);
+static proxyType nameProxy GUARDED_BY(cs_proxyInfos);
 int nConnectTimeout = DEFAULT_CONNECT_TIMEOUT;
 bool fNameLookup = DEFAULT_NAME_LOOKUP;
 

src/qt/bitcoingui.cpp

@@ -1207,6 +1207,7 @@ void BitcoinGUI::message(const QString &title, const QString &message, unsigned
 
         showNormalIfMinimized();
         QMessageBox mBox((QMessageBox::Icon)nMBoxIcon, strTitle, message, buttons, this);
+        mBox.setTextFormat(Qt::PlainText);
         int r = mBox.exec();
         if (ret != nullptr)
             *ret = r == QMessageBox::Ok;

src/qt/dash.cpp

@@ -725,7 +725,7 @@ int main(int argc, char *argv[])
     // Allow parameter interaction before we create the options model
     app.parameterSetup();
     // Load GUI settings from QSettings
-    app.createOptionsModel(gArgs.IsArgSet("-resetguisettings"));
+    app.createOptionsModel(gArgs.GetBoolArg("-resetguisettings", false));
 
     // Subscribe to global signals from core
     uiInterface.InitMessage.connect(InitMessage);

src/qt/sendcoinsdialog.cpp

@@ -468,6 +468,12 @@ void SendCoinsDialog::send(QList<SendCoinsRecipient> recipients)
 
 void SendCoinsDialog::clear()
 {
+    // Clear coin control settings
+    CoinControlDialog::coinControl()->UnSelectAll();
+    ui->checkBoxCoinControlChange->setChecked(false);
+    ui->lineEditCoinControlChange->clear();
+    coinControlUpdateLabels();
+
     // Remove entries until only one left
     while(ui->entries->count())
     {

src/rpc/mining.cpp

@@ -511,10 +511,10 @@ UniValue getblocktemplate(const JSONRPCRequest& request)
         {
             checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1);
 
-            WaitableLock lock(csBestBlock);
+            WaitableLock lock(g_best_block_mutex);
-            while (chainActive.Tip()->GetBlockHash() == hashWatchedChain && IsRPCRunning())
+            while (g_best_block == hashWatchedChain && IsRPCRunning())
             {
-                if (cvBlockChange.wait_until(lock, checktxtime) == std::cv_status::timeout)
+                if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout)
                 {
                     // Timeout: Check transactions for update
                     if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP)

src/test/rpc_tests.cpp

@@ -254,14 +254,14 @@ BOOST_AUTO_TEST_CASE(rpc_ban)
     ar = r.get_array();
     BOOST_CHECK_EQUAL(ar.size(), 0);
 
-    BOOST_CHECK_NO_THROW(r = CallRPC(std::string("setban 127.0.0.0/24 add 1607731200 true")));
+    BOOST_CHECK_NO_THROW(r = CallRPC(std::string("setban 127.0.0.0/24 add 9907731200 true")));
     BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned")));
     ar = r.get_array();
     o1 = ar[0].get_obj();
     adr = find_value(o1, "address");
     UniValue banned_until = find_value(o1, "banned_until");
     BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24");
-    BOOST_CHECK_EQUAL(banned_until.get_int64(), 1607731200); // absolute time check
+    BOOST_CHECK_EQUAL(banned_until.get_int64(), 9907731200); // absolute time check
 
     BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned")));
 

src/test/streams_tests.cpp

@@ -6,11 +6,8 @@
 #include <support/allocators/zeroafterfree.h>
 #include <test/test_dash.h>
 
-#include <boost/assign/std/vector.hpp> // for 'operator+=()'
 #include <boost/test/unit_test.hpp>
 
-using namespace boost::assign; // bring 'operator+=()' into scope
-
 BOOST_FIXTURE_TEST_SUITE(streams_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(streams_vector_writer)
@@ -80,14 +77,17 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
 
     // Degenerate case
     
-    key += '\x00','\x00';
+    key.push_back('\x00');
+    key.push_back('\x00');
     ds.Xor(key);
     BOOST_CHECK_EQUAL(
             std::string(expected_xor.begin(), expected_xor.end()), 
             std::string(ds.begin(), ds.end()));
 
-    in += '\x0f','\xf0';
+    in.push_back('\x0f');
-    expected_xor += '\xf0','\x0f';
+    in.push_back('\xf0');
+    expected_xor.push_back('\xf0');
+    expected_xor.push_back('\x0f');
     
     // Single character key
 
@@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
     ds.insert(ds.begin(), in.begin(), in.end());
     key.clear();
 
-    key += '\xff';
+    key.push_back('\xff');
     ds.Xor(key);
     BOOST_CHECK_EQUAL(
             std::string(expected_xor.begin(), expected_xor.end()), 
@@ -105,14 +105,17 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
 
     in.clear();
     expected_xor.clear();
-    in += '\xf0','\x0f';
+    in.push_back('\xf0');
-    expected_xor += '\x0f','\x00';
+    in.push_back('\x0f');
+    expected_xor.push_back('\x0f');
+    expected_xor.push_back('\x00');
                         
     ds.clear();
     ds.insert(ds.begin(), in.begin(), in.end());
 
     key.clear();
-    key += '\xff','\x0f';
+    key.push_back('\xff');
+    key.push_back('\x0f');
 
     ds.Xor(key);
     BOOST_CHECK_EQUAL(

src/validation.cpp

@@ -216,8 +216,9 @@ BlockMap& mapBlockIndex = g_chainstate.mapBlockIndex;
 PrevBlockMap& mapPrevBlockIndex = g_chainstate.mapPrevBlockIndex;
 CChain& chainActive = g_chainstate.chainActive;
 CBlockIndex *pindexBestHeader = nullptr;
-CWaitableCriticalSection csBestBlock;
+CWaitableCriticalSection g_best_block_mutex;
-CConditionVariable cvBlockChange;
+CConditionVariable g_best_block_cv;
+uint256 g_best_block;
 int nScriptCheckThreads = 0;
 std::atomic_bool fImporting(false);
 std::atomic_bool fReindex(false);
@@ -2507,7 +2508,11 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar
     // New best block
     mempool.AddTransactionsUpdated(1);
 
-    cvBlockChange.notify_all();
+    {
+        WaitableLock lock(g_best_block_mutex);
+        g_best_block = pindexNew->GetBlockHash();
+        g_best_block_cv.notify_all();
+    }
 
     std::vector<std::string> warningMessages;
     if (!IsInitialBlockDownload())

src/validation.h

@@ -156,8 +156,9 @@ extern PrevBlockMap& mapPrevBlockIndex;
 extern uint64_t nLastBlockTx;
 extern uint64_t nLastBlockSize;
 extern const std::string strMessageMagic;
-extern CWaitableCriticalSection csBestBlock;
+extern CWaitableCriticalSection g_best_block_mutex;
-extern CConditionVariable cvBlockChange;
+extern CConditionVariable g_best_block_cv;
+extern uint256 g_best_block;
 extern std::atomic_bool fImporting;
 extern std::atomic_bool fReindex;
 extern int nScriptCheckThreads;

src/wallet/rpcwallet.cpp

@@ -1216,6 +1216,8 @@ UniValue addmultisigaddress(const JSONRPCRequest& request)
         std::string msg = "addmultisigaddress nrequired [\"key\",...] ( \"account\" )\n"
             "\nAdd a nrequired-to-sign multisignature address to the wallet. Requires a new wallet backup.\n"
             "Each key is a Dash address or hex-encoded public key.\n"
+            "This functionality is only intended for use with non-watchonly addresses.\n"
+            "See `importaddress` for watchonly p2sh address support.\n"
             "If 'account' is specified (DEPRECATED), assign address to that account.\n"
 
             "\nArguments:\n"
@@ -1423,7 +1425,7 @@ UniValue listreceivedbyaddress(const JSONRPCRequest& request)
             "                                                 configured for transactions locked via InstantSend\n"
             "    \"label\" : \"label\",               (string) A comment for the address/transaction, if any\n"
             "    \"txids\": [\n"
-            "       n,                                (numeric) The ids of transactions received with the address \n"
+            "       \"txid\",                         (string) The ids of transactions received with the address \n"
             "       ...\n"
             "    ]\n"
             "  }\n"

src/wallet/wallet.cpp

@@ -1063,11 +1063,10 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
     CWalletTx& wtx = (*ret.first).second;
     wtx.BindWallet(this);
     bool fInsertedNew = ret.second;
-    if (fInsertedNew)
+    if (fInsertedNew) {
-    {
         wtx.nTimeReceived = GetAdjustedTime();
         wtx.nOrderPos = IncOrderPosNext(&walletdb);
-        wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr)));
+        wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr)));
         wtx.nTimeSmart = ComputeTimeSmart(wtx);
         AddToSpends(hash);
 
@@ -1141,9 +1140,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
 bool CWallet::LoadToWallet(const CWalletTx& wtxIn)
 {
     uint256 hash = wtxIn.GetHash();
-    CWalletTx& wtx = mapWallet.emplace(hash, wtxIn).first->second;
+    const auto& ins = mapWallet.emplace(hash, wtxIn);
+    CWalletTx& wtx = ins.first->second;
     wtx.BindWallet(this);
-    wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr)));
+    if (/* insertion took place */ ins.second) {
+        wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr)));
+    }
     AddToSpends(hash);
     for (const CTxIn& txin : wtx.tx->vin) {
         auto it = mapWallet.find(txin.prevout.hash);
@@ -1276,7 +1278,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
             walletdb.WriteTx(wtx);
             NotifyTransactionChanged(this, wtx.GetHash(), CT_UPDATED);
             // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too
-            TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(hashTx, 0));
+            TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(now, 0));
             while (iter != mapTxSpends.end() && iter->first.hash == now) {
                 if (!done.count(iter->second)) {
                     todo.insert(iter->second);
@@ -4172,8 +4174,11 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256
 {
     AssertLockHeld(cs_wallet); // mapWallet
     DBErrors nZapSelectTxRet = CWalletDB(*dbw,"cr+").ZapSelectTx(vHashIn, vHashOut);
-    for (uint256 hash : vHashOut)
+    for (uint256 hash : vHashOut) {
-        mapWallet.erase(hash);
+        const auto& it = mapWallet.find(hash);
+        wtxOrdered.erase(it->second.m_it_wtxOrdered);
+        mapWallet.erase(it);
+    }
 
     if (nZapSelectTxRet == DB_NEED_REWRITE)
     {

src/wallet/wallet.h

@@ -331,6 +331,7 @@ public:
     char fFromMe;
     std::string strFromAccount;
     int64_t nOrderPos; //!< position in ordered transaction list
+    std::multimap<int64_t, std::pair<CWalletTx*, CAccountingEntry*>>::const_iterator m_it_wtxOrdered;
 
     // memory only
     mutable bool fDebitCached;

test/functional/abandonconflict.py

@@ -64,9 +64,17 @@ class AbandonConflictTest(BitcoinTestFramework):
         signed2 = self.nodes[0].signrawtransaction(self.nodes[0].createrawtransaction(inputs, outputs))
         txABC2 = self.nodes[0].sendrawtransaction(signed2["hex"])
 
+        # Create a child tx spending ABC2
+        signed3_change = Decimal("24.999")
+        inputs = [ {"txid":txABC2, "vout":0} ]
+        outputs = { self.nodes[0].getnewaddress(): signed3_change }
+        signed3 = self.nodes[0].signrawtransactionwithwallet(self.nodes[0].createrawtransaction(inputs, outputs))
+        # note tx is never directly referenced, only abandoned as a child of the above
+        self.nodes[0].sendrawtransaction(signed3["hex"])
+
         # In mempool txs from self should increase balance from change
         newbalance = self.nodes[0].getbalance()
-        assert_equal(newbalance, balance - Decimal("30") + Decimal("24.9996"))
+        assert_equal(newbalance, balance - Decimal("30") + signed3_change)
         balance = newbalance
 
         # Restart the node with a higher min relay fee so the parent tx is no longer in mempool
@@ -81,7 +89,7 @@ class AbandonConflictTest(BitcoinTestFramework):
         # Not in mempool txs from self should only reduce balance
         # inputs are still spent, but change not received
         newbalance = self.nodes[0].getbalance()
-        assert_equal(newbalance, balance - Decimal("24.9996"))
+        assert_equal(newbalance, balance - signed3_change)
         # Unconfirmed received funds that are not in mempool, also shouldn't show
         # up in unconfirmed balance
         unconfbalance = self.nodes[0].getunconfirmedbalance() + self.nodes[0].getbalance()

test/functional/receivedby.py

@@ -6,10 +6,13 @@
 from decimal import Decimal
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (assert_array_result,
+from test_framework.util import (
-                                 assert_equal,
+    assert_array_result,
-                                 assert_raises_rpc_error,
+    assert_equal,
-                                 )
+    assert_raises_rpc_error,
+    sync_blocks,
+)
+
 
 class ReceivedByTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -18,6 +21,7 @@ class ReceivedByTest(BitcoinTestFramework):
     def run_test(self):
         # Generate block to get out of IBD
         self.nodes[0].generate(1)
+        sync_blocks(self.nodes)
 
         self.log.info("listreceivedbyaddress Test")
 

test/functional/sendheaders.py

@@ -115,6 +115,7 @@ class BaseNode(P2PInterface):
 
         self.block_announced = False
         self.last_blockhash_announced = None
+        self.recent_headers_announced = []
 
     def send_get_data(self, block_hashes):
         """Request data for a list of block hashes."""
@@ -162,40 +163,45 @@ class BaseNode(P2PInterface):
     def on_headers(self, message):
         if len(message.headers):
             self.block_announced = True
-            message.headers[-1].calc_sha256()
+            for x in message.headers:
+                x.calc_sha256()
+                # append because headers may be announced over multiple messages.
+                self.recent_headers_announced.append(x.sha256)
             self.last_blockhash_announced = message.headers[-1].sha256
 
-    def clear_last_announcement(self):
+    def clear_block_announcements(self):
         with mininode_lock:
             self.block_announced = False
             self.last_message.pop("inv", None)
             self.last_message.pop("headers", None)
+            self.recent_headers_announced = []
 
-    def check_last_announcement(self, headers=None, inv=None):
-        """Test whether the last announcement received had the right header or the right inv.
 
-        inv and headers should be lists of block hashes."""
+    def check_last_headers_announcement(self, headers):
+        """Test whether the last headers announcements received are right.
+           Headers may be announced across more than one message."""
+        test_function = lambda: (len(self.recent_headers_announced) >= len(headers))
+        wait_until(test_function, timeout=60, lock=mininode_lock)
+        with mininode_lock:
+            assert_equal(self.recent_headers_announced, headers)
+            self.block_announced = False
+            self.last_message.pop("headers", None)
+            self.recent_headers_announced = []
+
+    def check_last_inv_announcement(self, inv):
+        """Test whether the last announcement received had the right inv.
+        inv should be a list of block hashes."""
 
         test_function = lambda: self.block_announced
         wait_until(test_function, timeout=60, lock=mininode_lock)
 
         with mininode_lock:
-            self.block_announced = False
-
             compare_inv = []
             if "inv" in self.last_message:
                 compare_inv = [x.hash for x in self.last_message["inv"].inv]
-            if inv is not None:
+            assert_equal(compare_inv, inv)
-                assert_equal(compare_inv, inv)
+            self.block_announced = False
-
-            compare_headers = []
-            if "headers" in self.last_message:
-                compare_headers = [x.sha256 for x in self.last_message["headers"].headers]
-            if headers is not None:
-                assert_equal(compare_headers, headers)
-
             self.last_message.pop("inv", None)
-            self.last_message.pop("headers", None)
 
 class SendHeadersTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -205,8 +211,8 @@ class SendHeadersTest(BitcoinTestFramework):
     def mine_blocks(self, count):
         """Mine count blocks and return the new tip."""
 
-        # Clear out last block announcement from each p2p listener
+        # Clear out block announcements from each p2p listener
-        [x.clear_last_announcement() for x in self.nodes[0].p2ps]
+        [x.clear_block_announcements() for x in self.nodes[0].p2ps]
         self.nodes[0].generate(count)
         return int(self.nodes[0].getbestblockhash(), 16)
 
@@ -221,7 +227,7 @@ class SendHeadersTest(BitcoinTestFramework):
         self.sync_blocks(self.nodes, wait=0.1)
         for x in self.nodes[0].p2ps:
             x.wait_for_block_announcement(int(self.nodes[0].getbestblockhash(), 16))
-            x.clear_last_announcement()
+            x.clear_block_announcements()
 
         tip_height = self.nodes[1].getblockcount()
         hash_to_invalidate = self.nodes[1].getblockhash(tip_height - (length - 1))
@@ -254,25 +260,25 @@ class SendHeadersTest(BitcoinTestFramework):
         tip = self.nodes[0].getblockheader(self.nodes[0].generate(1)[0])
         tip_hash = int(tip["hash"], 16)
 
-        inv_node.check_last_announcement(inv=[tip_hash], headers=[])
+        inv_node.check_last_inv_announcement(inv=[tip_hash])
-        test_node.check_last_announcement(inv=[tip_hash], headers=[])
+        test_node.check_last_inv_announcement(inv=[tip_hash])
 
         self.log.info("Verify getheaders with null locator and valid hashstop returns headers.")
-        test_node.clear_last_announcement()
+        test_node.clear_block_announcements()
         test_node.send_get_headers(locator=[], hashstop=tip_hash)
-        test_node.check_last_announcement(headers=[tip_hash])
+        test_node.check_last_headers_announcement(headers=[tip_hash])
 
         self.log.info("Verify getheaders with null locator and invalid hashstop does not return headers.")
         block = create_block(int(tip["hash"], 16), create_coinbase(tip["height"] + 1), tip["mediantime"] + 1)
         block.solve()
         test_node.send_header_for_blocks([block])
-        test_node.clear_last_announcement()
+        test_node.clear_block_announcements()
         test_node.send_get_headers(locator=[], hashstop=int(block.hash, 16))
         test_node.sync_with_ping()
         assert_equal(test_node.block_announced, False)
-        inv_node.clear_last_announcement()
+        inv_node.clear_block_announcements()
         test_node.send_message(msg_block(block))
-        inv_node.check_last_announcement(inv=[int(block.hash, 16)], headers=[])
+        inv_node.check_last_inv_announcement(inv=[int(block.hash, 16)])
 
     def test_nonnull_locators(self, test_node, inv_node):
         tip = int(self.nodes[0].getbestblockhash(), 16)
@@ -283,8 +289,8 @@ class SendHeadersTest(BitcoinTestFramework):
         for i in range(4):
             old_tip = tip
             tip = self.mine_blocks(1)
-            inv_node.check_last_announcement(inv=[tip], headers=[])
+            inv_node.check_last_inv_announcement(inv=[tip])
-            test_node.check_last_announcement(inv=[tip], headers=[])
+            test_node.check_last_inv_announcement(inv=[tip])
             # Try a few different responses; none should affect next announcement
             if i == 0:
                 # first request the block
@@ -295,7 +301,7 @@ class SendHeadersTest(BitcoinTestFramework):
                 test_node.send_get_headers(locator=[old_tip], hashstop=tip)
                 test_node.send_get_data([tip])
                 test_node.wait_for_block(tip)
-                test_node.clear_last_announcement()  # since we requested headers...
+                test_node.clear_block_announcements()  # since we requested headers...
             elif i == 2:
                 # this time announce own block via headers
                 height = self.nodes[0].getblockcount()
@@ -307,8 +313,8 @@ class SendHeadersTest(BitcoinTestFramework):
                 test_node.wait_for_getdata([new_block.sha256])
                 test_node.send_message(msg_block(new_block))
                 test_node.sync_with_ping()  # make sure this block is processed
-                inv_node.clear_last_announcement()
+                inv_node.clear_block_announcements()
-                test_node.clear_last_announcement()
+                test_node.clear_block_announcements()
 
         self.log.info("Part 1: success!")
         self.log.info("Part 2: announce blocks with headers after sendheaders message...")
@@ -322,8 +328,8 @@ class SendHeadersTest(BitcoinTestFramework):
 
         # Now that we've synced headers, headers announcements should work
         tip = self.mine_blocks(1)
-        inv_node.check_last_announcement(inv=[tip], headers=[])
+        inv_node.check_last_inv_announcement(inv=[tip])
-        test_node.check_last_announcement(headers=[tip])
+        test_node.check_last_headers_announcement(headers=[tip])
 
         height = self.nodes[0].getblockcount() + 1
         block_time += 10  # Advance far enough ahead
@@ -367,8 +373,8 @@ class SendHeadersTest(BitcoinTestFramework):
                 assert "inv" not in inv_node.last_message
                 assert "headers" not in inv_node.last_message
                 tip = self.mine_blocks(1)
-                inv_node.check_last_announcement(inv=[tip], headers=[])
+                inv_node.check_last_inv_announcement(inv=[tip])
-                test_node.check_last_announcement(headers=[tip])
+                test_node.check_last_headers_announcement(headers=[tip])
                 height += 1
                 block_time += 1
 
@@ -382,16 +388,16 @@ class SendHeadersTest(BitcoinTestFramework):
             # First try mining a reorg that can propagate with header announcement
             new_block_hashes = self.mine_reorg(length=7)
             tip = new_block_hashes[-1]
-            inv_node.check_last_announcement(inv=[tip], headers=[])
+            inv_node.check_last_inv_announcement(inv=[tip])
-            test_node.check_last_announcement(headers=new_block_hashes)
+            test_node.check_last_headers_announcement(headers=new_block_hashes)
 
             block_time += 8
 
             # Mine a too-large reorg, which should be announced with a single inv
             new_block_hashes = self.mine_reorg(length=8)
             tip = new_block_hashes[-1]
-            inv_node.check_last_announcement(inv=[tip], headers=[])
+            inv_node.check_last_inv_announcement(inv=[tip])
-            test_node.check_last_announcement(inv=[tip], headers=[])
+            test_node.check_last_inv_announcement(inv=[tip])
 
             block_time += 9
 
@@ -400,15 +406,15 @@ class SendHeadersTest(BitcoinTestFramework):
 
             # Use getblocks/getdata
             test_node.send_getblocks(locator=[fork_point])
-            test_node.check_last_announcement(inv=new_block_hashes, headers=[])
+            test_node.check_last_inv_announcement(inv=new_block_hashes)
             test_node.send_get_data(new_block_hashes)
             test_node.wait_for_block(new_block_hashes[-1])
 
             for i in range(3):
                 # Mine another block, still should get only an inv
                 tip = self.mine_blocks(1)
-                inv_node.check_last_announcement(inv=[tip], headers=[])
+                inv_node.check_last_inv_announcement(inv=[tip])
-                test_node.check_last_announcement(inv=[tip], headers=[])
+                test_node.check_last_inv_announcement(inv=[tip])
                 if i == 0:
                     # Just get the data -- shouldn't cause headers announcements to resume
                     test_node.send_get_data([tip])
@@ -433,8 +439,8 @@ class SendHeadersTest(BitcoinTestFramework):
                         test_node.sync_with_ping()
             # New blocks should now be announced with header
             tip = self.mine_blocks(1)
-            inv_node.check_last_announcement(inv=[tip], headers=[])
+            inv_node.check_last_inv_announcement(inv=[tip])
-            test_node.check_last_announcement(headers=[tip])
+            test_node.check_last_headers_announcement(headers=[tip])
 
         self.log.info("Part 3: success!")