private broadcast: enforce sending to unique node ids
What the developer wrote
Authored by Vasil Dimov
private broadcast: enforce sending to unique node ids
Sending more than one transaction to a given node would be a privacy
leak and thus enforce that this is not done. `GetSendStatusByNode()`
assumes unique node ids.
Note that sending more than one transaction to a given address is fine,
if that is done via separate connections, in which case the node ids
would be different.
What changed, and why it matters
This change adds a safety check inside Bitcoin Core's private transaction-broadcast feature. It prevents the same node from being chosen more than once to receive different transactions, because doing so could let that node figure out which transactions belong to the same wallet. The patch uses an internal assumption check and returns nothing if a duplicate node is about to be reused.
Review as a privacy-hardening fix. Verify that callers of PickTxForSend() handle std::nullopt correctly and that no code path can accidentally trigger the new Assume(false) under normal operation. Consider whether an automated test covers duplicate-node scenarios.
Security signals we found
privacy leak mitigation in transaction broadcast
defensive invariant enforcement with Assume(false)
prevents node-id reuse during private broadcast rounds
no cryptographic or network-layer change
Evidence from the diff
In PrivateBroadcast::PickTxForSend(), the commit adds a guard that calls GetSendStatusByNode(will_send_to_nodeid) and, if a status already exists for that node, triggers Assume(false) and returns std::nullopt. This enforces the invariant that each NodeId receives at most one transaction during a private-broadcast round. The header comment is updated to document the new restriction. The change is defensive and appears to address a privacy leak rather than a memory-safety or remote-exploitable bug.
Changed components
src/private_broadcast.cppsrc/private_broadcast.hPrivateBroadcast::PickTxForSend()Inspect captured patch +8 / −1
diff --git a/src/private_broadcast.cpp b/src/private_broadcast.cpp
index 9c107da6..1d78a6eb 100644
--- a/src/private_broadcast.cpp
+++ b/src/private_broadcast.cpp
@@ -33,6 +33,11 @@ std::optional<CTransactionRef> PrivateBroadcast::PickTxForSend(const NodeId& wil
{
LOCK(m_mutex);
+ if (GetSendStatusByNode(will_send_to_nodeid).has_value()) { // nodeid reuse, shouldn't send >1 tx to a given node
+ Assume(false);
+ return std::nullopt;
+ }
+
const auto it{std::ranges::max_element(
m_transactions,
[](const auto& a, const auto& b) { return a < b; },
diff --git a/src/private_broadcast.h b/src/private_broadcast.h
index b53a2cfb..ae456ef8 100644
--- a/src/private_broadcast.h
+++ b/src/private_broadcast.h
@@ -73,7 +73,9 @@ public:
* Pick the transaction with the fewest send attempts, and confirmations,
* and oldest send/confirm times.
* @param[in] will_send_to_nodeid Will remember that the returned transaction
- * was picked for sending to this node.
+ * was picked for sending to this node. Calling this method more than once with
+ * the same `will_send_to_nodeid` is not allowed because sending more than one
+ * transaction to one node would be a privacy leak.
* @param[in] will_send_to_address Address of the peer to which this transaction
* will be sent.
* @return Most urgent transaction or nullopt if there are no transactions.
Why this scored 47/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.