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

@@ -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);

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)
 {

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.