From 25ef049d60535ac02508ba71ef60f17d8349f120 Mon Sep 17 00:00:00 2001
From: James O'Beirne <james.obeirne@pm.me>
Date: Fri, 28 Oct 2022 18:59:41 -0400
Subject: [PATCH] log: mempool: log removal reason in validation interface

Currently the exact reason a transaction is removed from the mempool isn't
logged. It is sometimes detectable from context, but adding the `reason` to
the validation interface logs (where it is already passed) seems like an easy
way to disambiguate.

For example, in the case of mempool expiry, the logs look like this:

```
[validationinterface.cpp:220] [TransactionRemovedFromMempool] [validation] Enqueuing TransactionRemovedFromMempool: txid=<txid> wtxid=<wtxid>
[txmempool.cpp:1050] [RemoveUnbroadcastTx] [mempool] Removed <txid> from set of unbroadcast txns before confirmation that txn was sent out
[validationinterface.cpp:220] [operator()] [validation] TransactionRemovedFromMempool: txid=<txid> wtxid=<wtxid>
[validation.cpp:267] [LimitMempoolSize] [mempool] Expired 1 transactions from the memory pool
```

There is no context-free way to know $txid was evicted on the basis of expiry.
This change will make that case (and probably others) clear.
---
 src/txmempool.cpp           | 14 ++++++++++++++
 src/txmempool.h             |  2 ++
 src/validationinterface.cpp |  7 +++++--
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 84ed2e9ef5..6a4cd842fb 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -1182,3 +1182,17 @@ void CTxMemPool::SetLoadTried(bool load_tried)
     LOCK(cs);
     m_load_tried = load_tried;
 }
+
+
+const std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept
+{
+    switch (r) {
+        case MemPoolRemovalReason::EXPIRY: return "expiry";
+        case MemPoolRemovalReason::SIZELIMIT: return "sizelimit";
+        case MemPoolRemovalReason::REORG: return "reorg";
+        case MemPoolRemovalReason::BLOCK: return "block";
+        case MemPoolRemovalReason::CONFLICT: return "conflict";
+        case MemPoolRemovalReason::REPLACED: return "replaced";
+    }
+    assert(false);
+}
diff --git a/src/txmempool.h b/src/txmempool.h
index 4afaac0506..50d9a8236b 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -355,6 +355,8 @@ enum class MemPoolRemovalReason {
     REPLACED,    //!< Removed for replacement
 };
 
+const std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept;
+
 /**
  * CTxMemPool stores valid-according-to-the-current-best-chain transactions
  * that may be included in the next block.
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index 613c5b65ef..740c39d99d 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -17,6 +17,8 @@
 #include <unordered_map>
 #include <utility>
 
+const std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept;
+
 /**
  * MainSignalsImpl manages a list of shared_ptr<CValidationInterface> callbacks.
  *
@@ -215,9 +217,10 @@ void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef& tx, MemP
     auto event = [tx, reason, mempool_sequence, this] {
         m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(tx, reason, mempool_sequence); });
     };
-    ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__,
+    ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s reason=%s", __func__,
                           tx->GetHash().ToString(),
-                          tx->GetWitnessHash().ToString());
+                          tx->GetWitnessHash().ToString(),
+                          RemovalReasonToString(reason));
 }
 
 void CMainSignals::BlockConnected(const std::shared_ptr<const CBlock> &pblock, const CBlockIndex *pindex) {