net_processing: stop private broadcast of a transaction after round-trip
What the developer wrote
Authored by Vasil Dimov
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.
What changed, and why it matters
This Bitcoin Core change improves how the node stops privately re-broadcasting a transaction once it sees that same transaction come back from the network. It is a hardening/efficiency fix for the new private transaction broadcast feature, not a fix for a known remote exploit. The patch makes the node stop its private re-broadcast attempts when the exact transaction it sent is seen again, while still continuing to broadcast the original if a peer returns a malleated (altered-witness) version.
Treat as a routine hardening/improvement commit. No emergency action required. Reviewers may want to confirm that Remove() is only triggered for the exact wtxid match and that the priority/num_confirmed value returned is used correctly to cancel pending outbound connections.
Security signals we found
Reduces network-side metadata leakage from private transaction broadcast
Prevents unnecessary repeated private broadcast attempts after transaction propagation is confirmed
Handles witness malleation correctly by not removing original tx when a malleated version is seen
Adds defensive state management to new private-broadcast subsystem
Evidence from the diff
The commit adds a PrivateBroadcast::Remove() method and calls it from net_processing when a ‘tx’ or ‘wtx’ message is received. If the received transaction matches one in the private-broadcast set by wtxid (and txid), it is removed and any unstarted private broadcast connections are cancelled. If the txid matches but the wtxid differs, the transaction is not removed, so the node keeps broadcasting its original version in case a malleated witness wins. This prevents redundant private broadcasts and reduces information leakage about the originating node, but the diff itself is a functional improvement rather than a patch for a demonstrated vulnerability.
Changed components
src/net_processing.cppsrc/private_broadcast.cppsrc/private_broadcast.hPrivate transaction broadcast featureInspect captured patch +34 / −0
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index abfcb673..2f539a16 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 6d312ac9..395b8dd0 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 5b8634a0..3a1c6e29 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.
Why this scored 36/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.