[mempool] Allow rebroadcast for same-txid-different-wtxid transactions

This commit fixes some slightly unexpected behaviour when:

- there is already transaction in the mempool (the "mempool tx")
- BroadcastTransaction() is called for a transaction with the same txid
  as the mempool transaction but a different witness (the "new tx")

Prior to this commit, if BroadcastTransaction() is called with
relay=true, then it'll call RelayTransaction() using the txid/wtxid of
the new tx, not the txid/wtxid of the mempool tx. For wtxid relay peers,
in SendMessages(), the wtxid of the new tx will be taken from
setInventoryTxToSend, but will then be filtered out from the vector of
wtxids to announce, since m_mempool.info() won't find the transaction
(the mempool contains the mempool tx, which has a different wtxid from
the new tx).

Fix this by calling RelayTransaction() with the wtxid of the mempool
transaction in this case.

1 files changed, 15 insertions, 5 deletions

diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
index 926dd0e31d..0c1295c0b5 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -34,7 +34,8 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
     assert(node.peerman);
     assert(node.mempool);
     std::promise<void> promise;
-    uint256 hashTx = tx->GetHash();
+    uint256 txid = tx->GetHash();
+    uint256 wtxid = tx->GetWitnessHash();
     bool callback_set = false;
 
     { // cs_main scope
@@ -44,12 +45,21 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
     // and return early.
     CCoinsViewCache &view = node.chainman->ActiveChainstate().CoinsTip();
     for (size_t o = 0; o < tx->vout.size(); o++) {
-        const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o));
+        const Coin& existingCoin = view.AccessCoin(COutPoint(txid, o));
         // IsSpent doesn't mean the coin is spent, it means the output doesn't exist.
         // So if the output does exist, then this transaction exists in the chain.
         if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN;
     }
-    if (!node.mempool->exists(hashTx)) {
+    if (auto mempool_tx = node.mempool->get(txid); mempool_tx) {
+        // There's already a transaction in the mempool with this txid. Don't
+        // try to submit this transaction to the mempool (since it'll be
+        // rejected as a TX_CONFLICT), but do attempt to reannounce the mempool
+        // transaction if relay=true.
+        //
+        // The mempool transaction may have the same or different witness (and
+        // wtxid) as this transaction. Use the mempool's wtxid for reannouncement.
+        wtxid = mempool_tx->GetWitnessHash();
+    } else {
         // Transaction is not already in the mempool.
         if (max_tx_fee > 0) {
             // First, call ATMP with test_accept and check the fee. If ATMP
@@ -74,7 +84,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
         if (relay) {
             // the mempool tracks locally submitted transactions to make a
             // best-effort of initial broadcast
-            node.mempool->AddUnbroadcastTx(hashTx);
+            node.mempool->AddUnbroadcastTx(txid);
         }
 
         if (wait_callback) {
@@ -102,7 +112,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
     }
 
     if (relay) {
-        node.peerman->RelayTransaction(hashTx, tx->GetWitnessHash());
+        node.peerman->RelayTransaction(txid, wtxid);
     }
 
     return TransactionError::OK;