Merge #18973: [0.20] Final backports for rc2

245c862cfd4883ea91b53d766abb00a9c3c1ea5c test: disable script fuzz tests (fanquake)
9a8fb4cf4ba472a5c3e1b9b71d31673f881a4896 fuzz: Remove enumeration of expected deserialization exceptions in ProcessMessage(...) fuzzer (practicalswift)
6161c94a6108ebddafe4e95c14bde4cdc3f8c01c [net processing] Only send a getheaders for one block in an INV (John Newbery)
cf2a6e2a390ad18a616d7f2718688375f2576577 test: Remove const to work around compiler error on xenial (Wladimir J. van der Laan)
cc7d34465bbb0195d8bcd9143097840a2e9765f2 miner: Avoid stack-use-after-return in validationinterface (MarcoFalke)
37a620748bd3578eda1c74daad8df8451d13b989 test: Add unregister_validation_interface_race test (MarcoFalke)
ff4dc2075031e9a49220cc27a270aeabe8954989 gui: Fix manual coin control with multiple wallets loaded (João Barbosa)
ed0afe8c1ff37926cc5bdcb0e8e4983e194e6d61 test: Add test for conflicted wallet tx notifications (Russell Yanofsky)
251e321ad7d9ddb938e8a07ddfbe90739f0bafdd rpc: Relock wallet only if most recent callback (João Barbosa)
ca4dac48c5675af3fc53db6740a0b70fef622b0a rpc: Add mutex to guard deadlineTimers (João Barbosa)
a3fe458a1e477cacd19e7e0edb8e7bb965067115 [docs] Improve commenting in ProcessGetData() (John Newbery)
011532e380bb1a42eac9e79a17b35531f768becf [test] test that an invalid GETDATA doesn't prevent processing of future messages (Amiti Uttarwar)
1e73d7248a10863dc99a93f1db36d035c17f29d7 [net processing] ignore unknown INV types in GETDATA messages (Amiti Uttarwar)
fb821731eb12906996bffdf4b3633d7fe47c85a7 [net processing] ignore tx GETDATA from blocks-only peers (Amiti Uttarwar)
315ae14f3f5c98ae4c4476e4bb260b9086c773a4 gui: Fix itemWalletAddress leak when not tree mode (João Barbosa)

Pull request description:

  Backports the following PRs to the 0.20 branch:

  * https://github.com/bitcoin/bitcoin/pull/18578: gui: Fix leak in CoinControlDialog::updateView
  * https://github.com/bitcoin/bitcoin/pull/18808: [net processing] Drop unknown types in getdata
  * https://github.com/bitcoin/bitcoin/pull/18814: rpc: Relock wallet only if most recent callback
  * https://github.com/bitcoin/bitcoin/pull/18878: test: Add test for conflicted wallet tx notifications
  * https://github.com/bitcoin/bitcoin/pull/18894: gui: Fix manual coin control with multiple wallets loaded
  * https://github.com/bitcoin/bitcoin/pull/18742: miner: Avoid stack-use-after-return in validationinterface
  * https://github.com/bitcoin/bitcoin/pull/18962: net processing: Only send a getheaders for one block in an INV
  * https://github.com/bitcoin/bitcoin/pull/18975: test: Remove const to work around compiler error on xenial

ACKs for top commit:
  promag:
    Tested ACK 245c862cfd4883ea91b53d766abb00a9c3c1ea5c coin control with multiple wallets.
  laanwj:
    ACK 245c862cfd4883ea91b53d766abb00a9c3c1ea5c
  MarcoFalke:
    ACK 245c862cfd solved the conflicts myself as a sanity check. Did not re-review 🍷

Tree-SHA512: 285e5a5fad5bbeba6032742c65dc68836e8eccfcceda9e69fec4ddd162a3f61679a96f9bbe3d434267835af67c21ac4c05accf6f63e827c2eb47203c6daafe31

16 files changed, 274 insertions, 151 deletions

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index d53477793c..56c887af78 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -104,8 +104,6 @@ FUZZ_TARGETS = \
   test/fuzz/script \
   test/fuzz/script_deserialize \
   test/fuzz/script_flags \
-  test/fuzz/script_ops \
-  test/fuzz/scriptnum_ops \
   test/fuzz/service_deserialize \
   test/fuzz/signature_checker \
   test/fuzz/snapshotmetadata_deserialize \
@@ -893,17 +891,18 @@ test_fuzz_script_flags_LDADD = $(FUZZ_SUITE_LD_COMMON)
 test_fuzz_script_flags_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
 test_fuzz_script_flags_SOURCES = test/fuzz/script_flags.cpp
 
-test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
-test_fuzz_script_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
-test_fuzz_script_ops_LDADD = $(FUZZ_SUITE_LD_COMMON)
-test_fuzz_script_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
-test_fuzz_script_ops_SOURCES = test/fuzz/script_ops.cpp
-
-test_fuzz_scriptnum_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
-test_fuzz_scriptnum_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
-test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON)
-test_fuzz_scriptnum_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
-test_fuzz_scriptnum_ops_SOURCES = test/fuzz/scriptnum_ops.cpp
+# Disabled for now, as #18413 has not been backported.
+# test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
+# test_fuzz_script_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
+# test_fuzz_script_ops_LDADD = $(FUZZ_SUITE_LD_COMMON)
+# test_fuzz_script_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
+# test_fuzz_script_ops_SOURCES = test/fuzz/script_ops.cpp
+
+# test_fuzz_scriptnum_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
+# test_fuzz_scriptnum_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
+# test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON)
+# test_fuzz_scriptnum_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
+# test_fuzz_scriptnum_ops_SOURCES = test/fuzz/scriptnum_ops.cpp
 
 test_fuzz_service_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DSERVICE_DESERIALIZE=1
 test_fuzz_service_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index d3089c4176..2a61f84a08 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1564,26 +1564,32 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
     std::vector<CInv> vNotFound;
     const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
 
-    // Note that if we receive a getdata for a MSG_TX or MSG_WITNESS_TX from a
-    // block-relay-only outbound peer, we will stop processing further getdata
-    // messages from this peer (likely resulting in our peer eventually
-    // disconnecting us).
-    if (pfrom->m_tx_relay != nullptr) {
-        // mempool entries added before this time have likely expired from mapRelay
-        const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME;
-        const std::chrono::seconds mempool_req = pfrom->m_tx_relay->m_last_mempool_req.load();
+    // mempool entries added before this time have likely expired from mapRelay
+    const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME;
+    // Get last mempool request time
+    const std::chrono::seconds mempool_req = pfrom->m_tx_relay != nullptr ? pfrom->m_tx_relay->m_last_mempool_req.load()
+                                                                          : std::chrono::seconds::min();
 
+    {
         LOCK(cs_main);
 
+        // Process as many TX items from the front of the getdata queue as
+        // possible, since they're common and it's efficient to batch process
+        // them.
         while (it != pfrom->vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX)) {
             if (interruptMsgProc)
                 return;
-            // Don't bother if send buffer is too full to respond anyway
+            // The send buffer provides backpressure. If there's no space in
+            // the buffer, pause processing until the next call.
             if (pfrom->fPauseSend)
                 break;
 
-            const CInv &inv = *it;
-            it++;
+            const CInv &inv = *it++;
+
+            if (pfrom->m_tx_relay == nullptr) {
+                // Ignore GETDATA requests for transactions from blocks-only peers.
+                continue;
+            }
 
             // Send stream from relay memory
             bool push = false;
@@ -1611,19 +1617,17 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
         }
     } // release cs_main
 
+    // Only process one BLOCK item per call, since they're uncommon and can be
+    // expensive to process.
     if (it != pfrom->vRecvGetData.end() && !pfrom->fPauseSend) {
-        const CInv &inv = *it;
+        const CInv &inv = *it++;
         if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK || inv.type == MSG_WITNESS_BLOCK) {
-            it++;
             ProcessGetBlockData(pfrom, chainparams, inv, connman);
         }
+        // else: If the first item on the queue is an unknown type, we erase it
+        // and continue processing the queue on the next call.
     }
 
-    // Unknown types in the GetData stay in vRecvGetData and block any future
-    // message from this peer, see vRecvGetData check in ProcessMessages().
-    // Depending on future p2p changes, we might either drop unknown getdata on
-    // the floor or disconnect the peer.
-
     pfrom->vRecvGetData.erase(pfrom->vRecvGetData.begin(), it);
 
     if (!vNotFound.empty()) {
@@ -2257,6 +2261,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
 
         uint32_t nFetchFlags = GetFetchFlags(pfrom);
         const auto current_time = GetTime<std::chrono::microseconds>();
+        uint256* best_block{nullptr};
 
         for (CInv &inv : vInv)
         {
@@ -2273,17 +2278,14 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
             if (inv.type == MSG_BLOCK) {
                 UpdateBlockAvailability(pfrom->GetId(), inv.hash);
                 if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) {
-                    // We used to request the full block here, but since headers-announcements are now the
-                    // primary method of announcement on the network, and since, in the case that a node
-                    // fell back to inv we probably have a reorg which we should get the headers for first,
-                    // we now only provide a getheaders response here. When we receive the headers, we will
-                    // then ask for the blocks we need.
-                    connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), inv.hash));
-                    LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, inv.hash.ToString(), pfrom->GetId());
+                    // Headers-first is the primary method of announcement on
+                    // the network. If a node fell back to sending blocks by inv,
+                    // it's probably for a re-org. The final block hash
+                    // provided should be the highest, so send a getheaders and
+                    // then fetch the blocks we need to catch up.
+                    best_block = &inv.hash;
                 }
-            }
-            else
-            {
+            } else {
                 pfrom->AddInventoryKnown(inv);
                 if (fBlocksOnly) {
                     LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom->GetId());
@@ -2294,6 +2296,12 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
                 }
             }
         }
+
+        if (best_block != nullptr) {
+            connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), *best_block));
+            LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, best_block->ToString(), pfrom->GetId());
+        }
+
         return true;
     }
 
diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp
index 9495ba389a..88e7304383 100644
--- a/src/qt/coincontroldialog.cpp
+++ b/src/qt/coincontroldialog.cpp
@@ -41,10 +41,11 @@ bool CCoinControlWidgetItem::operator<(const QTreeWidgetItem &other) const {
     return QTreeWidgetItem::operator<(other);
 }
 
-CoinControlDialog::CoinControlDialog(const PlatformStyle *_platformStyle, QWidget *parent) :
+CoinControlDialog::CoinControlDialog(CCoinControl& coin_control, WalletModel* _model, const PlatformStyle *_platformStyle, QWidget *parent) :
     QDialog(parent),
     ui(new Ui::CoinControlDialog),
-    model(nullptr),
+    m_coin_control(coin_control),
+    model(_model),
     platformStyle(_platformStyle)
 {
     ui->setupUi(this);
@@ -134,6 +135,13 @@ CoinControlDialog::CoinControlDialog(const PlatformStyle *_platformStyle, QWidge
         ui->radioTreeMode->click();
     if (settings.contains("nCoinControlSortColumn") && settings.contains("nCoinControlSortOrder"))
         sortView(settings.value("nCoinControlSortColumn").toInt(), (static_cast<Qt::SortOrder>(settings.value("nCoinControlSortOrder").toInt())));
+
+    if(_model->getOptionsModel() && _model->getAddressTableModel())
+    {
+        updateView();
+        updateLabelLocked();
+        CoinControlDialog::updateLabels(m_coin_control, _model, this);
+    }
 }
 
 CoinControlDialog::~CoinControlDialog()
@@ -146,18 +154,6 @@ CoinControlDialog::~CoinControlDialog()
     delete ui;
 }
 
-void CoinControlDialog::setModel(WalletModel *_model)
-{
-    this->model = _model;
-
-    if(_model && _model->getOptionsModel() && _model->getAddressTableModel())
-    {
-        updateView();
-        updateLabelLocked();
-        CoinControlDialog::updateLabels(_model, this);
-    }
-}
-
 // ok button
 void CoinControlDialog::buttonBoxClicked(QAbstractButton* button)
 {
@@ -183,8 +179,8 @@ void CoinControlDialog::buttonSelectAllClicked()
                 ui->treeWidget->topLevelItem(i)->setCheckState(COLUMN_CHECKBOX, state);
     ui->treeWidget->setEnabled(true);
     if (state == Qt::Unchecked)
-        coinControl()->UnSelectAll(); // just to be sure
-    CoinControlDialog::updateLabels(model, this);
+        m_coin_control.UnSelectAll(); // just to be sure
+    CoinControlDialog::updateLabels(m_coin_control, model, this);
 }
 
 // context menu
@@ -369,15 +365,15 @@ void CoinControlDialog::viewItemChanged(QTreeWidgetItem* item, int column)
         COutPoint outpt(uint256S(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), item->data(COLUMN_ADDRESS, VOutRole).toUInt());
 
         if (item->checkState(COLUMN_CHECKBOX) == Qt::Unchecked)
-            coinControl()->UnSelect(outpt);
+            m_coin_control.UnSelect(outpt);
         else if (item->isDisabled()) // locked (this happens if "check all" through parent node)
             item->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked);
         else
-            coinControl()->Select(outpt);
+            m_coin_control.Select(outpt);
 
         // selection changed -> update labels
         if (ui->treeWidget->isEnabled()) // do not update on every click for (un)select all
-            CoinControlDialog::updateLabels(model, this);
+            CoinControlDialog::updateLabels(m_coin_control, model, this);
     }
 
     // TODO: Remove this temporary qt5 fix after Qt5.3 and Qt5.4 are no longer used.
@@ -402,7 +398,7 @@ void CoinControlDialog::updateLabelLocked()
     else ui->labelLocked->setVisible(false);
 }
 
-void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
+void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel *model, QDialog* dialog)
 {
     if (!model)
         return;
@@ -434,7 +430,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
     bool fWitness               = false;
 
     std::vector<COutPoint> vCoinControl;
-    coinControl()->ListSelected(vCoinControl);
+    m_coin_control.ListSelected(vCoinControl);
 
     size_t i = 0;
     for (const auto& out : model->wallet().getCoins(vCoinControl)) {
@@ -445,7 +441,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
         const COutPoint& outpt = vCoinControl[i++];
         if (out.is_spent)
         {
-            coinControl()->UnSelect(outpt);
+            m_coin_control.UnSelect(outpt);
             continue;
         }
 
@@ -498,7 +494,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
                 nBytes -= 34;
 
         // Fee
-        nPayFee = model->wallet().getMinimumFee(nBytes, *coinControl(), nullptr /* returned_target */, nullptr /* reason */);
+        nPayFee = model->wallet().getMinimumFee(nBytes, m_coin_control, nullptr /* returned_target */, nullptr /* reason */);
 
         if (nPayAmount > 0)
         {
@@ -590,12 +586,6 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
         label->setVisible(nChange < 0);
 }
 
-CCoinControl* CoinControlDialog::coinControl()
-{
-    static CCoinControl coin_control;
-    return &coin_control;
-}
-
 void CoinControlDialog::updateView()
 {
     if (!model || !model->getOptionsModel() || !model->getAddressTableModel())
@@ -612,8 +602,7 @@ void CoinControlDialog::updateView()
     int nDisplayUnit = model->getOptionsModel()->getDisplayUnit();
 
     for (const auto& coins : model->wallet().listCoins()) {
-        CCoinControlWidgetItem *itemWalletAddress = new CCoinControlWidgetItem();
-        itemWalletAddress->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked);
+        CCoinControlWidgetItem* itemWalletAddress{nullptr};
         QString sWalletAddress = QString::fromStdString(EncodeDestination(coins.first));
         QString sWalletLabel = model->getAddressTableModel()->labelForAddress(sWalletAddress);
         if (sWalletLabel.isEmpty())
@@ -622,7 +611,7 @@ void CoinControlDialog::updateView()
         if (treeMode)
         {
             // wallet address
-            ui->treeWidget->addTopLevelItem(itemWalletAddress);
+            itemWalletAddress = new CCoinControlWidgetItem(ui->treeWidget);
 
             itemWalletAddress->setFlags(flgTristate);
             itemWalletAddress->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked);
@@ -696,13 +685,13 @@ void CoinControlDialog::updateView()
              // disable locked coins
             if (model->wallet().isLockedCoin(output))
             {
-                coinControl()->UnSelect(output); // just to be sure
+                m_coin_control.UnSelect(output); // just to be sure
                 itemOutput->setDisabled(true);
                 itemOutput->setIcon(COLUMN_CHECKBOX, platformStyle->SingleColorIcon(":/icons/lock_closed"));
             }
 
             // set checkbox
-            if (coinControl()->IsSelected(output))
+            if (m_coin_control.IsSelected(output))
                 itemOutput->setCheckState(COLUMN_CHECKBOX, Qt::Checked);
         }
 
diff --git a/src/qt/coincontroldialog.h b/src/qt/coincontroldialog.h
index efc06a7656..c8a91e5f58 100644
--- a/src/qt/coincontroldialog.h
+++ b/src/qt/coincontroldialog.h
@@ -31,7 +31,6 @@ class CCoinControlWidgetItem : public QTreeWidgetItem
 {
 public:
     explicit CCoinControlWidgetItem(QTreeWidget *parent, int type = Type) : QTreeWidgetItem(parent, type) {}
-    explicit CCoinControlWidgetItem(int type = Type) : QTreeWidgetItem(type) {}
     explicit CCoinControlWidgetItem(QTreeWidgetItem *parent, int type = Type) : QTreeWidgetItem(parent, type) {}
 
     bool operator<(const QTreeWidgetItem &other) const;
@@ -43,20 +42,18 @@ class CoinControlDialog : public QDialog
     Q_OBJECT
 
 public:
-    explicit CoinControlDialog(const PlatformStyle *platformStyle, QWidget *parent = nullptr);
+    explicit CoinControlDialog(CCoinControl& coin_control, WalletModel* model, const PlatformStyle *platformStyle, QWidget *parent = nullptr);
     ~CoinControlDialog();
 
-    void setModel(WalletModel *model);
-
     // static because also called from sendcoinsdialog
-    static void updateLabels(WalletModel*, QDialog*);
+    static void updateLabels(CCoinControl& m_coin_control, WalletModel*, QDialog*);
 
     static QList<CAmount> payAmounts;
-    static CCoinControl *coinControl();
     static bool fSubtractFeeFromAmount;
 
 private:
     Ui::CoinControlDialog *ui;
+    CCoinControl& m_coin_control;
     WalletModel *model;
     int sortColumn;
     Qt::SortOrder sortOrder;
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index a8c82aaf6c..2a6ef41221 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -57,6 +57,7 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *p
     ui(new Ui::SendCoinsDialog),
     clientModel(nullptr),
     model(nullptr),
+    m_coin_control(new CCoinControl),
     fNewRecipientAllowed(true),
     fFeeMinimized(true),
     platformStyle(_platformStyle)
@@ -262,14 +263,9 @@ void SendCoinsDialog::on_sendButton_clicked()
     WalletModelTransaction currentTransaction(recipients);
     WalletModel::SendCoinsReturn prepareStatus;
 
-    // Always use a CCoinControl instance, use the CoinControlDialog instance if CoinControl has been enabled
-    CCoinControl ctrl;
-    if (model->getOptionsModel()->getCoinControlFeatures())
-        ctrl = *CoinControlDialog::coinControl();
+    updateCoinControlState(*m_coin_control);
 
-    updateCoinControlState(ctrl);
-
-    prepareStatus = model->prepareTransaction(currentTransaction, ctrl);
+    prepareStatus = model->prepareTransaction(currentTransaction, *m_coin_control);
 
     // process prepareStatus and on error generate message shown to user
     processSendCoinsReturn(prepareStatus,
@@ -413,7 +409,7 @@ void SendCoinsDialog::on_sendButton_clicked()
     }
     if (!send_failure) {
         accept();
-        CoinControlDialog::coinControl()->UnSelectAll();
+        m_coin_control->UnSelectAll();
         coinControlUpdateLabels();
     }
     fNewRecipientAllowed = true;
@@ -422,7 +418,7 @@ void SendCoinsDialog::on_sendButton_clicked()
 void SendCoinsDialog::clear()
 {
     // Clear coin control settings
-    CoinControlDialog::coinControl()->UnSelectAll();
+    m_coin_control->UnSelectAll();
     ui->checkBoxCoinControlChange->setChecked(false);
     ui->lineEditCoinControlChange->clear();
     coinControlUpdateLabels();
@@ -645,17 +641,11 @@ void SendCoinsDialog::on_buttonMinimizeFee_clicked()
 
 void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry)
 {
-    // Get CCoinControl instance if CoinControl is enabled or create a new one.
-    CCoinControl coin_control;
-    if (model->getOptionsModel()->getCoinControlFeatures()) {
-        coin_control = *CoinControlDialog::coinControl();
-    }
-
     // Include watch-only for wallets without private key
-    coin_control.fAllowWatchOnly = model->wallet().privateKeysDisabled();
+    m_coin_control->fAllowWatchOnly = model->wallet().privateKeysDisabled();
 
     // Calculate available amount to send.
-    CAmount amount = model->wallet().getAvailableBalance(coin_control);
+    CAmount amount = model->wallet().getAvailableBalance(*m_coin_control);
     for (int i = 0; i < ui->entries->count(); ++i) {
         SendCoinsEntry* e = qobject_cast<SendCoinsEntry*>(ui->entries->itemAt(i)->widget());
         if (e && !e->isHidden() && e != entry) {
@@ -714,12 +704,11 @@ void SendCoinsDialog::updateSmartFeeLabel()
 {
     if(!model || !model->getOptionsModel())
         return;
-    CCoinControl coin_control;
-    updateCoinControlState(coin_control);
-    coin_control.m_feerate.reset(); // Explicitly use only fee estimation rate for smart fee labels
+    updateCoinControlState(*m_coin_control);
+    m_coin_control->m_feerate.reset(); // Explicitly use only fee estimation rate for smart fee labels
     int returned_target;
     FeeReason reason;
-    CFeeRate feeRate = CFeeRate(model->wallet().getMinimumFee(1000, coin_control, &returned_target, &reason));
+    CFeeRate feeRate = CFeeRate(model->wallet().getMinimumFee(1000, *m_coin_control, &returned_target, &reason));
 
     ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), feeRate.GetFeePerK()) + "/kB");
 
@@ -790,7 +779,7 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked)
     ui->frameCoinControl->setVisible(checked);
 
     if (!checked && model) // coin control features disabled
-        CoinControlDialog::coinControl()->SetNull();
+        m_coin_control->SetNull();
 
     coinControlUpdateLabels();
 }
@@ -798,8 +787,7 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked)
 // Coin Control: button inputs -> show actual coin control dialog
 void SendCoinsDialog::coinControlButtonClicked()
 {
-    CoinControlDialog dlg(platformStyle);
-    dlg.setModel(model);
+    CoinControlDialog dlg(*m_coin_control, model, platformStyle);
     dlg.exec();
     coinControlUpdateLabels();
 }
@@ -809,7 +797,7 @@ void SendCoinsDialog::coinControlChangeChecked(int state)
 {
     if (state == Qt::Unchecked)
     {
-        CoinControlDialog::coinControl()->destChange = CNoDestination();
+        m_coin_control->destChange = CNoDestination();
         ui->labelCoinControlChangeLabel->clear();
     }
     else
@@ -825,7 +813,7 @@ void SendCoinsDialog::coinControlChangeEdited(const QString& text)
     if (model && model->getAddressTableModel())
     {
         // Default to no change address until verified
-        CoinControlDialog::coinControl()->destChange = CNoDestination();
+        m_coin_control->destChange = CNoDestination();
         ui->labelCoinControlChangeLabel->setStyleSheet("QLabel{color:red;}");
 
         const CTxDestination dest = DecodeDestination(text.toStdString());
@@ -848,7 +836,7 @@ void SendCoinsDialog::coinControlChangeEdited(const QString& text)
                     QMessageBox::Yes | QMessageBox::Cancel, QMessageBox::Cancel);
 
                 if(btnRetVal == QMessageBox::Yes)
-                    CoinControlDialog::coinControl()->destChange = dest;
+                    m_coin_control->destChange = dest;
                 else
                 {
                     ui->lineEditCoinControlChange->setText("");
@@ -867,7 +855,7 @@ void SendCoinsDialog::coinControlChangeEdited(const QString& text)
                 else
                     ui->labelCoinControlChangeLabel->setText(tr("(no label)"));
 
-                CoinControlDialog::coinControl()->destChange = dest;
+                m_coin_control->destChange = dest;
             }
         }
     }
@@ -879,7 +867,7 @@ void SendCoinsDialog::coinControlUpdateLabels()
     if (!model || !model->getOptionsModel())
         return;
 
-    updateCoinControlState(*CoinControlDialog::coinControl());
+    updateCoinControlState(*m_coin_control);
 
     // set pay amounts
     CoinControlDialog::payAmounts.clear();
@@ -897,10 +885,10 @@ void SendCoinsDialog::coinControlUpdateLabels()
         }
     }
 
-    if (CoinControlDialog::coinControl()->HasSelected())
+    if (m_coin_control->HasSelected())
     {
         // actual coin control calculation
-        CoinControlDialog::updateLabels(model, this);
+        CoinControlDialog::updateLabels(*m_coin_control, model, this);
 
         // show coin control stats
         ui->labelCoinControlAutomaticallySelected->hide();
diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h
index 86422c4030..8eadc5a225 100644
--- a/src/qt/sendcoinsdialog.h
+++ b/src/qt/sendcoinsdialog.h
@@ -12,6 +12,7 @@
 #include <QString>
 #include <QTimer>
 
+class CCoinControl;
 class ClientModel;
 class PlatformStyle;
 class SendCoinsEntry;
@@ -60,6 +61,7 @@ private:
     Ui::SendCoinsDialog *ui;
     ClientModel *clientModel;
     WalletModel *model;
+    std::unique_ptr<CCoinControl> m_coin_control;
     bool fNewRecipientAllowed;
     bool fFeeMinimized;
     const PlatformStyle *platformStyle;
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index da9d583fa7..94590e0da4 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -724,7 +724,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
     return result;
 }
 
-class submitblock_StateCatcher : public CValidationInterface
+class submitblock_StateCatcher final : public CValidationInterface
 {
 public:
     uint256 hash;
@@ -792,17 +792,17 @@ static UniValue submitblock(const JSONRPCRequest& request)
     }
 
     bool new_block;
-    submitblock_StateCatcher sc(block.GetHash());
-    RegisterValidationInterface(&sc);
+    auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash());
+    RegisterSharedValidationInterface(sc);
     bool accepted = ProcessNewBlock(Params(), blockptr, /* fForceProcessing */ true, /* fNewBlock */ &new_block);
-    UnregisterValidationInterface(&sc);
+    UnregisterSharedValidationInterface(sc);
     if (!new_block && accepted) {
         return "duplicate";
     }
-    if (!sc.found) {
+    if (!sc->found) {
         return "inconclusive";
     }
-    return BIP22ValidationResult(sc.state);
+    return BIP22ValidationResult(sc->state);
 }
 
 static UniValue submitheader(const JSONRPCRequest& request)
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index e2618c16da..219979f095 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -25,7 +25,8 @@ static std::string rpcWarmupStatus GUARDED_BY(cs_rpcWarmup) = "RPC server starte
 /* Timer-creating functions */
 static RPCTimerInterface* timerInterface = nullptr;
 /* Map of name to timer. */
-static std::map<std::string, std::unique_ptr<RPCTimerBase> > deadlineTimers;
+static Mutex g_deadline_timers_mutex;
+static std::map<std::string, std::unique_ptr<RPCTimerBase> > deadlineTimers GUARDED_BY(g_deadline_timers_mutex);
 static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler);
 
 struct RPCCommandExecutionInfo
@@ -298,7 +299,7 @@ void InterruptRPC()
 void StopRPC()
 {
     LogPrint(BCLog::RPC, "Stopping RPC\n");
-    deadlineTimers.clear();
+    WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear());
     DeleteAuthCookie();
     g_rpcSignals.Stopped();
 }
@@ -486,6 +487,7 @@ void RPCRunLater(const std::string& name, std::function<void()> func, int64_t nS
 {
     if (!timerInterface)
         throw JSONRPCError(RPC_INTERNAL_ERROR, "No timer handler registered for RPC");
+    LOCK(g_deadline_timers_mutex);
     deadlineTimers.erase(name);
     LogPrint(BCLog::RPC, "queue run of timer %s in %i seconds (using %s)\n", name, nSeconds, timerInterface->Name());
     deadlineTimers.emplace(name, std::unique_ptr<RPCTimerBase>(timerInterface->NewTimer(func, nSeconds*1000)));
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
index 9e3586d162..4255031904 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -19,16 +19,13 @@
 #include <validationinterface.h>
 #include <version.h>
 
-#include <algorithm>
 #include <atomic>
 #include <cassert>
 #include <chrono>
 #include <cstdint>
 #include <iosfwd>
 #include <iostream>
-#include <map>
 #include <memory>
-#include <set>
 #include <string>
 #include <vector>
 
@@ -44,19 +41,6 @@ const std::string LIMIT_TO_MESSAGE_TYPE{TO_STRING(MESSAGE_TYPE)};
 const std::string LIMIT_TO_MESSAGE_TYPE;
 #endif
 
-const std::map<std::string, std::set<std::string>> EXPECTED_DESERIALIZATION_EXCEPTIONS = {
-    {"CDataStream::read(): end of data: iostream error", {"addr", "block", "blocktxn", "cmpctblock", "feefilter", "filteradd", "filterload", "getblocks", "getblocktxn", "getdata", "getheaders", "headers", "inv", "notfound", "ping", "sendcmpct", "tx"}},
-    {"CompactSize exceeds limit of type: iostream error", {"cmpctblock"}},
-    {"differential value overflow: iostream error", {"getblocktxn"}},
-    {"index overflowed 16 bits: iostream error", {"getblocktxn"}},
-    {"index overflowed 16-bits: iostream error", {"cmpctblock"}},
-    {"indexes overflowed 16 bits: iostream error", {"getblocktxn"}},
-    {"non-canonical ReadCompactSize(): iostream error", {"addr", "block", "blocktxn", "cmpctblock", "filteradd", "filterload", "getblocks", "getblocktxn", "getdata", "getheaders", "headers", "inv", "notfound", "tx"}},
-    {"ReadCompactSize(): size too large: iostream error", {"addr", "block", "blocktxn", "cmpctblock", "filteradd", "filterload", "getblocks", "getblocktxn", "getdata", "getheaders", "headers", "inv", "notfound", "tx"}},
-    {"Superfluous witness record: iostream error", {"block", "blocktxn", "cmpctblock", "tx"}},
-    {"Unknown transaction optional data: iostream error", {"block", "blocktxn", "cmpctblock", "tx"}},
-};
-
 const RegTestingSetup* g_setup;
 } // namespace
 
@@ -86,13 +70,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     g_setup->m_node.peer_logic->InitializeNode(&p2p_node);
     try {
         (void)ProcessMessage(&p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.mempool, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic<bool>{false});
-    } catch (const std::ios_base::failure& e) {
-        const std::string exception_message{e.what()};
-        const auto p = EXPECTED_DESERIALIZATION_EXCEPTIONS.find(exception_message);
-        if (p == EXPECTED_DESERIALIZATION_EXCEPTIONS.cend() || p->second.count(random_message_type) == 0) {
-            std::cout << "Unexpected exception when processing message type \"" << random_message_type << "\": " << exception_message << std::endl;
-            assert(false);
-        }
+    } catch (const std::ios_base::failure&) {
     }
     SyncWithValidationInterfaceQueue();
 }
diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
index afb3db36a2..e61ea13b02 100644
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -32,7 +32,7 @@ struct MinerTestingSetup : public RegTestingSetup {
 
 BOOST_FIXTURE_TEST_SUITE(validation_block_tests, MinerTestingSetup)
 
-struct TestSubscriber : public CValidationInterface {
+struct TestSubscriber final : public CValidationInterface {
     uint256 m_expected_tip;
 
     explicit TestSubscriber(uint256 tip) : m_expected_tip(tip) {}
@@ -175,8 +175,8 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
         LOCK(cs_main);
         initial_tip = ::ChainActive().Tip();
     }
-    TestSubscriber sub(initial_tip->GetBlockHash());
-    RegisterValidationInterface(&sub);
+    auto sub = std::make_shared<TestSubscriber>(initial_tip->GetBlockHash());
+    RegisterSharedValidationInterface(sub);
 
     // create a bunch of threads that repeatedly process a block generated above at random
     // this will create parallelism and randomness inside validation - the ValidationInterface
@@ -208,10 +208,10 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
         UninterruptibleSleep(std::chrono::milliseconds{100});
     }
 
-    UnregisterValidationInterface(&sub);
+    UnregisterSharedValidationInterface(sub);
 
     LOCK(cs_main);
-    BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash());
+    BOOST_CHECK_EQUAL(sub->m_expected_tip, ::ChainActive().Tip()->GetBlockHash());
 }
 
 /**
diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp
index 208be92852..ceba689e52 100644
--- a/src/test/validationinterface_tests.cpp
+++ b/src/test/validationinterface_tests.cpp
@@ -12,6 +12,40 @@
 
 BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup)
 
+struct TestSubscriberNoop final : public CValidationInterface {
+    void BlockChecked(const CBlock&, const BlockValidationState&) override {}
+};
+
+BOOST_AUTO_TEST_CASE(unregister_validation_interface_race)
+{
+    std::atomic<bool> generate{true};
+
+    // Start thread to generate notifications
+    std::thread gen{[&] {
+        const CBlock block_dummy;
+        BlockValidationState state_dummy;
+        while (generate) {
+            GetMainSignals().BlockChecked(block_dummy, state_dummy);
+        }
+    }};
+
+    // Start thread to consume notifications
+    std::thread sub{[&] {
+        // keep going for about 1 sec, which is 250k iterations
+        for (int i = 0; i < 250000; i++) {
+            auto sub = std::make_shared<TestSubscriberNoop>();
+            RegisterSharedValidationInterface(sub);
+            UnregisterSharedValidationInterface(sub);
+        }
+        // tell the other thread we are done
+        generate = false;
+    }};
+
+    gen.join();
+    sub.join();
+    BOOST_CHECK(!generate);
+}
+
 class TestInterface : public CValidationInterface
 {
 public:
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 61ad2f1198..a5036413c8 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -1917,6 +1917,9 @@ static UniValue walletpassphrase(const JSONRPCRequest& request)
             }.Check(request);
 
     int64_t nSleepTime;
+    int64_t relock_time;
+    // Prevent concurrent calls to walletpassphrase with the same wallet.
+    LOCK(pwallet->m_unlock_mutex);
     {
         auto locked_chain = pwallet->chain().lock();
         LOCK(pwallet->cs_wallet);
@@ -1955,6 +1958,7 @@ static UniValue walletpassphrase(const JSONRPCRequest& request)
         pwallet->TopUpKeyPool();
 
         pwallet->nRelockTime = GetTime() + nSleepTime;
+        relock_time = pwallet->nRelockTime;
     }
 
     // rpcRunLater must be called without cs_wallet held otherwise a deadlock
@@ -1966,9 +1970,11 @@ static UniValue walletpassphrase(const JSONRPCRequest& request)
     // wallet before the following callback is called. If a valid shared pointer
     // is acquired in the callback then the wallet is still loaded.
     std::weak_ptr<CWallet> weak_wallet = wallet;
-    pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet] {
+    pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet, relock_time] {
         if (auto shared_wallet = weak_wallet.lock()) {
             LOCK(shared_wallet->cs_wallet);
+            // Skip if this is not the most recent rpcRunLater callback.
+            if (shared_wallet->nRelockTime != relock_time) return;
             shared_wallet->Lock();
             shared_wallet->nRelockTime = 0;
         }
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 6c54c72e76..7446a4889a 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -867,8 +867,10 @@ public:
     std::vector<std::string> GetDestValues(const std::string& prefix) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     //! Holds a timestamp at which point the wallet is scheduled (externally) to be relocked. Caller must arrange for actual relocking to occur via Lock().
-    int64_t nRelockTime = 0;
+    int64_t nRelockTime GUARDED_BY(cs_wallet){0};
 
+    // Used to prevent concurrent calls to walletpassphrase RPC.
+    Mutex m_unlock_mutex;
     bool Unlock(const SecureString& strWalletPassphrase, bool accept_no_keys = false);
     bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase);
     bool EncryptWallet(const SecureString& strWalletPassphrase);
diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py
index b110a559c0..47200b6cc6 100755
--- a/test/functional/feature_notifications.py
+++ b/test/functional/feature_notifications.py
@@ -5,12 +5,14 @@
 """Test the -alertnotify, -blocknotify and -walletnotify options."""
 import os
 
-from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE
+from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE, keyhash_to_p2pkh
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     wait_until,
     connect_nodes,
+    disconnect_nodes,
+    hex_str_to_bytes,
 )
 
 # Linux allow all characters other than \x00
@@ -81,8 +83,72 @@ class NotificationsTest(BitcoinTestFramework):
             # directory content should equal the generated transaction hashes
             txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count)))
             assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir)))
+            for tx_file in os.listdir(self.walletnotify_dir):
+                os.remove(os.path.join(self.walletnotify_dir, tx_file))
+
+            # Conflicting transactions tests. Give node 0 same wallet seed as
+            # node 1, generate spends from node 0, and check notifications
+            # triggered by node 1
+            self.log.info("test -walletnotify with conflicting transactions")
+            self.nodes[0].sethdseed(seed=self.nodes[1].dumpprivkey(keyhash_to_p2pkh(hex_str_to_bytes(self.nodes[1].getwalletinfo()['hdseedid'])[::-1])))
+            self.nodes[0].rescanblockchain()
+            self.nodes[0].generatetoaddress(100, ADDRESS_BCRT1_UNSPENDABLE)
+
+            # Generate transaction on node 0, sync mempools, and check for
+            # notification on node 1.
+            tx1 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True)
+            assert_equal(tx1 in self.nodes[0].getrawmempool(), True)
+            self.sync_mempools()
+            self.expect_wallet_notify([tx1])
+
+            # Generate bump transaction, sync mempools, and check for bump1
+            # notification. In the future, per
+            # https://github.com/bitcoin/bitcoin/pull/9371, it might be better
+            # to have notifications for both tx1 and bump1.
+            bump1 = self.nodes[0].bumpfee(tx1)["txid"]
+            assert_equal(bump1 in self.nodes[0].getrawmempool(), True)
+            self.sync_mempools()
+            self.expect_wallet_notify([bump1])
+
+            # Add bump1 transaction to new block, checking for a notification
+            # and the correct number of confirmations.
+            self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
+            self.sync_blocks()
+            self.expect_wallet_notify([bump1])
+            assert_equal(self.nodes[1].gettransaction(bump1)["confirmations"], 1)
+
+            # Generate a second transaction to be bumped.
+            tx2 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True)
+            assert_equal(tx2 in self.nodes[0].getrawmempool(), True)
+            self.sync_mempools()
+            self.expect_wallet_notify([tx2])
+
+            # Bump tx2 as bump2 and generate a block on node 0 while
+            # disconnected, then reconnect and check for notifications on node 1
+            # about newly confirmed bump2 and newly conflicted tx2. Currently
+            # only the bump2 notification is sent. Ideally, notifications would
+            # be sent both for bump2 and tx2, which was the previous behavior
+            # before being broken by an accidental change in PR
+            # https://github.com/bitcoin/bitcoin/pull/16624. The bug is reported
+            # in issue https://github.com/bitcoin/bitcoin/issues/18325.
+            disconnect_nodes(self.nodes[0], 1)
+            bump2 = self.nodes[0].bumpfee(tx2)["txid"]
+            self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
+            assert_equal(self.nodes[0].gettransaction(bump2)["confirmations"], 1)
+            assert_equal(tx2 in self.nodes[1].getrawmempool(), True)
+            connect_nodes(self.nodes[0], 1)
+            self.sync_blocks()
+            self.expect_wallet_notify([bump2])
+            assert_equal(self.nodes[1].gettransaction(bump2)["confirmations"], 1)
 
         # TODO: add test for `-alertnotify` large fork notifications
 
+    def expect_wallet_notify(self, tx_ids):
+        wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_ids), timeout=10)
+        assert_equal(sorted(notify_outputname(self.wallet, tx_id) for tx_id in tx_ids), sorted(os.listdir(self.walletnotify_dir)))
+        for tx_file in os.listdir(self.walletnotify_dir):
+            os.remove(os.path.join(self.walletnotify_dir, tx_file))
+
+
 if __name__ == '__main__':
     NotificationsTest().main()
diff --git a/test/functional/p2p_getdata.py b/test/functional/p2p_getdata.py
new file mode 100755
index 0000000000..fd94a09d80
--- /dev/null
+++ b/test/functional/p2p_getdata.py
@@ -0,0 +1,51 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test GETDATA processing behavior"""
+from collections import defaultdict
+
+from test_framework.messages import (
+    CInv,
+    msg_getdata,
+)
+from test_framework.mininode import (
+    mininode_lock,
+    P2PInterface,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import wait_until
+
+class P2PStoreBlock(P2PInterface):
+
+    def __init__(self):
+        super().__init__()
+        self.blocks = defaultdict(int)
+
+    def on_block(self, message):
+        message.block.calc_sha256()
+        self.blocks[message.block.sha256] += 1
+
+class GetdataTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+
+    def run_test(self):
+        self.nodes[0].add_p2p_connection(P2PStoreBlock())
+
+        self.log.info("test that an invalid GETDATA doesn't prevent processing of future messages")
+
+        # Send invalid message and verify that node responds to later ping
+        invalid_getdata = msg_getdata()
+        invalid_getdata.inv.append(CInv(t=0, h=0))  # INV type 0 is invalid.
+        self.nodes[0].p2ps[0].send_and_ping(invalid_getdata)
+
+        # Check getdata still works by fetching tip block
+        best_block = int(self.nodes[0].getbestblockhash(), 16)
+        good_getdata = msg_getdata()
+        good_getdata.inv.append(CInv(t=2, h=best_block))
+        self.nodes[0].p2ps[0].send_and_ping(good_getdata)
+        wait_until(lambda: self.nodes[0].p2ps[0].blocks[best_block] == 1, timeout=30, lock=mininode_lock)
+
+if __name__ == '__main__':
+    GetdataTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index faa2dee4ed..9f885ae4b0 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -145,6 +145,7 @@ BASE_SCRIPTS = [
     'rpc_deprecated.py',
     'wallet_disable.py',
     'p2p_addr_relay.py',
+    'p2p_getdata.py',
     'rpc_net.py',
     'wallet_keypool.py',
     'p2p_mempool.py',