[wallet] Notify conflicted transactions in TransactionRemovedFromMempool

The only CValidationInterface client that cares about transactions that
are removed from the mempool because of CONFLICT is the wallet.

Start using the TransactionRemovedFromMempool method to notify about
conflicted transactions instead of using the vtxConflicted vector in
BlockConnected.

3 files changed, 31 insertions, 8 deletions

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 5768219f3a..f04f13e6e0 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -408,7 +408,11 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
 {
     CTransactionRef ptx = it->GetSharedTx();
     NotifyEntryRemoved(ptx, reason);
-    if (reason != MemPoolRemovalReason::BLOCK && reason != MemPoolRemovalReason::CONFLICT) {
+    if (reason != MemPoolRemovalReason::BLOCK) {
+        // Notify clients that a transaction has been removed from the mempool
+        // for any reason except being included in a block. Clients interested
+        // in transactions included in blocks can subscribe to the BlockConnected
+        // notification.
         GetMainSignals().TransactionRemovedFromMempool(ptx);
     }
 
diff --git a/src/validationinterface.h b/src/validationinterface.h
index ed6c560944..3bb4639f47 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -92,10 +92,32 @@ protected:
     /**
      * Notifies listeners of a transaction leaving mempool.
      *
-     * This only fires for transactions which leave mempool because of expiry,
-     * size limiting, reorg (changes in lock times/coinbase maturity), or
-     * replacement. This does not include any transactions which are included
-     * in BlockConnectedDisconnected either in block->vtx or in txnConflicted.
+     * This notification fires for transactions that are removed from the
+     * mempool for the following reasons:
+     *
+     * - EXPIRY (expired from mempool after -mempoolexpiry hours)
+     * - SIZELIMIT (removed in size limiting if the mempool exceeds -maxmempool megabytes)
+     * - REORG (removed during a reorg)
+     * - CONFLICT (removed because it conflicts with in-block transaction)
+     * - REPLACED (removed due to RBF replacement)
+     *
+     * This does not fire for transactions that are removed from the mempool
+     * because they have been included in a block. Any client that is interested
+     * in transactions removed from the mempool for inclusion in a block can learn
+     * about those transactions from the BlockConnected notification.
+     *
+     * Transactions that are removed from the mempool because they conflict
+     * with a transaction in the new block will have
+     * TransactionRemovedFromMempool events fired *before* the BlockConnected
+     * event is fired. If multiple blocks are connected in one step, then the
+     * ordering could be:
+     *
+     * - TransactionRemovedFromMempool(tx1 from block A)
+     * - TransactionRemovedFromMempool(tx2 from block A)
+     * - TransactionRemovedFromMempool(tx1 from block B)
+     * - TransactionRemovedFromMempool(tx2 from block B)
+     * - BlockConnected(A)
+     * - BlockConnected(B)
      *
      * Called on a background thread.
      */
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 02fe59b601..8c63f5b907 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1123,9 +1123,6 @@ void CWallet::BlockConnected(const CBlock& block, const std::vector<CTransaction
         SyncTransaction(block.vtx[index], confirm);
         TransactionRemovedFromMempool(block.vtx[index]);
     }
-    for (const CTransactionRef& ptx : vtxConflicted) {
-        TransactionRemovedFromMempool(ptx);
-    }
 }
 
 void CWallet::BlockDisconnected(const CBlock& block, int height)