From 37b79f9c39db5a4a61d360a6a29c8853bb5c7ac0 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Tue, 30 Jan 2024 18:51:23 +0100
Subject: [PATCH] net_processing: stop private broadcast of a transaction after
 round-trip

Remove the transaction from the list of transactions to broadcast after
we receive it from the network.

Only remove the transaction if it is the same as the one we sent: has
the same wtxid (and it follows the same txid). Don't remove transactions
that have the same txid and different wtxid. Such transactions show that
some of the private broadcast recipients malleated the witness and the
transaction made it back to us. The witness could be either:
* invalid, in which case the transaction will not be accepted in
  anybody's pool; or
* valid, in which case either the original or the malleated transaction
  will make it to nodes' mempools and eventually be mined. Our response
  is to keep broadcasting the original. If the malleated transaction
  wins then we will eventually stop broadcasting the original when it
  gets stale and gets removed from the "to broadcast" storage cause it
  is not acceptable in our mempool.
---
 src/net_processing.cpp    | 11 +++++++++++
 src/private_broadcast.cpp | 12 ++++++++++++
 src/private_broadcast.h   | 11 +++++++++++
 3 files changed, 34 insertions(+)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index abfcb673d7d..2f539a160a9 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -4425,6 +4425,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         const uint256& hash = peer->m_wtxid_relay ? wtxid.ToUint256() : txid.ToUint256();
         AddKnownTx(*peer, hash);
 
+        if (const auto num_broadcasted{m_tx_for_private_broadcast.Remove(ptx)}) {
+            LogInfo("[privatebroadcast] Received our privately broadcast transaction (txid=%s) from the "
+                    "network from peer=%d%s; stopping private broadcast attempts",
+                    txid.ToString(), pfrom.GetId(), pfrom.LogIP(fLogIPs));
+            if (NUM_PRIVATE_BROADCAST_PER_TX > num_broadcasted.value()) {
+                // Not all of the initial NUM_PRIVATE_BROADCAST_PER_TX connections were needed.
+                // Tell CConnman it does not need to start the remaining ones.
+                m_connman.m_private_broadcast.NumToOpenSub(NUM_PRIVATE_BROADCAST_PER_TX - num_broadcasted.value());
+            }
+        }
+
         LOCK2(cs_main, m_tx_download_mutex);
 
         const auto& [should_validate, package_to_validate] = m_txdownloadman.ReceivedTx(pfrom.GetId(), ptx);
diff --git a/src/private_broadcast.cpp b/src/private_broadcast.cpp
index 6d312ac95c0..395b8dd0100 100644
--- a/src/private_broadcast.cpp
+++ b/src/private_broadcast.cpp
@@ -15,6 +15,18 @@ bool PrivateBroadcast::Add(const CTransactionRef& tx)
     return inserted;
 }
 
+std::optional<size_t> PrivateBroadcast::Remove(const CTransactionRef& tx)
+    EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+{
+    LOCK(m_mutex);
+    const auto handle{m_transactions.extract(tx)};
+    if (handle) {
+        const auto p{DerivePriority(handle.mapped())};
+        return p.num_confirmed;
+    }
+    return std::nullopt;
+}
+
 std::optional<CTransactionRef> PrivateBroadcast::PickTxForSend(const NodeId& will_send_to_nodeid)
     EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
 {
diff --git a/src/private_broadcast.h b/src/private_broadcast.h
index 5b8634a0ca2..3a1c6e29c8f 100644
--- a/src/private_broadcast.h
+++ b/src/private_broadcast.h
@@ -20,6 +20,7 @@
 /**
  * Store a list of transactions to be broadcast privately. Supports the following operations:
  * - Add a new transaction
+ * - Remove a transaction
  * - Pick a transaction for sending to one recipient
  * - Query which transaction has been picked for sending to a given recipient node
  * - Mark that a given recipient node has confirmed receipt of a transaction
@@ -38,6 +39,16 @@ public:
     bool Add(const CTransactionRef& tx)
         EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
+    /**
+     * Forget a transaction.
+     * @param[in] tx Transaction to forget.
+     * @retval !nullopt The number of times the transaction was sent and confirmed
+     * by the recipient (if the transaction existed and was removed).
+     * @retval nullopt The transaction was not in the storage.
+     */
+    std::optional<size_t> Remove(const CTransactionRef& tx)
+        EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+
     /**
      * Pick the transaction with the fewest send attempts, and confirmations,
      * and oldest send/confirm times.