highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-04-08 06:37:50 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#34267: net: avoid unconditional privatebroadcast logging (+ warn for debug logs)

Browse Source 
b39291f4cd doc: fix `-logips` description to clarify that non-debug logs can also contain IP addresses (Lőrinc)
c7028d3368 init: log that additional logs may contain privacy-sensitive information (Lőrinc)
31b771a942 net: move `privatebroadcast` logs to debug category (Lőrinc)

Pull request description:

  ### Motivation
  The recently merged [private broadcast](https://github.com/bitcoin/bitcoin/pull/29415) is a privacy feature, and users may share `debug.log` with support.
  Unconditional `LogInfo()` messages that mention private broadcast and/or include (w)txids can leak sensitive context (e.g. which transactions a user originated).
  Since it's meant to be a private broadcast, we should minimize leaks.
  It's a best effort, it's not invalidated by other logs possibly leaking identifiable information, those can be addressed separately.
  We're not promising that the logs won't ever contain data that could be used against the user, but we should still try to minimize that data, especially for a feature that's advertised as privacy-focused.

  Follow up to [#29415 (comment)](https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2637012294)

  ### Changes
  * Move private-broadcast event logs from `LogInfo()` to `LogDebug(BCLog::PRIVBROADCAST, ...)`, so they are only emitted when `-debug=privatebroadcast` was explicitly provided.
  * Remove hardcoded `"[privatebroadcast]"` log-string prefixes (category logging already adds the prefix).
  * Keep warning at the default log level for startup failures.
  * Add an init log (not a warning since that would require excessive test framework updates) when any `-debug` categories are enabled that additional logs may contain privacy-sensitive information and should not be shared publicly.
  * Update a related startup arg (`-logips`) to clarify that clarify that non-debug logs can also contain IP addresses.

  ### Reproducer
  The new warning can be checked with:
  ```bash
  ./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 | grep 'Debug logging is enabled' | wc -l
         0
  ./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 -debug | grep 'Debug logging is enabled' | wc  -l
         1
  ```

ACKs for top commit:
  janb84:
    re ACK b39291f4cd
  vasild:
    ACK b39291f4cd
  andrewtoth:
    ACK b39291f4cd
  frankomosh:
    crACK b39291f4cd .The approach and implementation look good. Moving private broadcast logs to debug only would effectively reduce privacy leaks for users sharing logs.
  sedited:
    ACK b39291f4cd

Tree-SHA512: feca25ebe72a03948ba436e25f9a682947966c4c09627e8f20201ef3872ddbce1c636cd82f06be1afdc09cb80da305058667c0c2eaeadeb351311155325ea06f
This commit is contained in:
merge-script
2026-01-27 12:59:33 +01:00
parent 8593d96519 b39291f4cd
commit f970cb39fb
3 changed files with 26 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
src/net_processing.cpp
View File

@@ -1653,9 +1653,9 @@ void PeerManagerImpl::ReattemptPrivateBroadcast(CScheduler& scheduler)
stale_tx->GetHash().ToString(), stale_tx->GetWitnessHash().ToString());
++num_for_rebroadcast;
} else {
LogInfo("[privatebroadcast] Giving up broadcast attempts for txid=%s wtxid=%s: %s",
stale_tx->GetHash().ToString(), stale_tx->GetWitnessHash().ToString(),
mempool_acceptable.m_state.ToString());
LogDebug(BCLog::PRIVBROADCAST, "Giving up broadcast attempts for txid=%s wtxid=%s: %s",
stale_tx->GetHash().ToString(), stale_tx->GetWitnessHash().ToString(),
mempool_acceptable.m_state.ToString());
m_tx_for_private_broadcast.Remove(stale_tx);
}
}
@@ -3536,9 +3536,9 @@ void PeerManagerImpl::PushPrivateBroadcastTx(CNode& node)
}
const CTransactionRef& tx{*opt_tx};
LogInfo("[privatebroadcast] P2P handshake completed, sending INV for txid=%s%s, peer=%d%s",
tx->GetHash().ToString(), tx->HasWitness() ? strprintf(", wtxid=%s", tx->GetWitnessHash().ToString()) : "",
node.GetId(), node.LogIP(fLogIPs));
LogDebug(BCLog::PRIVBROADCAST, "P2P handshake completed, sending INV for txid=%s%s, peer=%d%s",
tx->GetHash().ToString(), tx->HasWitness() ? strprintf(", wtxid=%s", tx->GetWitnessHash().ToString()) : "",
node.GetId(), node.LogIP(fLogIPs));
MakeAndPushMessage(node, NetMsgType::INV, std::vector<CInv>{{CInv{MSG_TX, tx->GetHash().ToUint256()}}});
}
@@ -3677,8 +3677,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
if (fRelay) {
MakeAndPushMessage(pfrom, NetMsgType::VERACK);
} else {
LogInfo("[privatebroadcast] Disconnecting: does not support transactions relay (connected in vain), peer=%d%s",
pfrom.GetId(), pfrom.LogIP(fLogIPs));
LogDebug(BCLog::PRIVBROADCAST, "Disconnecting: does not support transaction relay (connected in vain), peer=%d%s",
pfrom.GetId(), pfrom.LogIP(fLogIPs));
pfrom.fDisconnect = true;
}
return;
@@ -4203,8 +4203,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
if (pfrom.IsPrivateBroadcastConn()) {
const auto pushed_tx_opt{m_tx_for_private_broadcast.GetTxForNode(pfrom.GetId())};
if (!pushed_tx_opt) {
LogInfo("[privatebroadcast] Disconnecting: got GETDATA without sending an INV, peer=%d%s",
pfrom.GetId(), fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : "");
LogDebug(BCLog::PRIVBROADCAST, "Disconnecting: got GETDATA without sending an INV, peer=%d%s",
pfrom.GetId(), fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : "");
pfrom.fDisconnect = true;
return;
}
@@ -4220,8 +4220,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
peer->m_ping_queued = true; // Ensure a ping will be sent: mimic a request via RPC.
MaybeSendPing(pfrom, *peer, GetTime<std::chrono::microseconds>());
} else {
LogInfo("[privatebroadcast] Disconnecting: got an unexpected GETDATA message, peer=%d%s",
pfrom.GetId(), fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : "");
LogDebug(BCLog::PRIVBROADCAST, "Disconnecting: got an unexpected GETDATA message, peer=%d%s",
pfrom.GetId(), fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : "");
pfrom.fDisconnect = true;
}
return;
@@ -4465,9 +4465,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
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));
LogDebug(BCLog::PRIVBROADCAST, "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.
@@ -4981,8 +4981,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
pfrom.PongReceived(ping_time);
if (pfrom.IsPrivateBroadcastConn()) {
m_tx_for_private_broadcast.NodeConfirmedReception(pfrom.GetId());
LogInfo("[privatebroadcast] Got a PONG (the transaction will probably reach the network), marking for disconnect, peer=%d%s",
pfrom.GetId(), pfrom.LogIP(fLogIPs));
LogDebug(BCLog::PRIVBROADCAST, "Got a PONG (the transaction will probably reach the network), marking for disconnect, peer=%d%s",
pfrom.GetId(), pfrom.LogIP(fLogIPs));
pfrom.fDisconnect = true;
}
} else {
@@ -5712,8 +5712,8 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// not sent. This here is just an optimization.
if (pto->IsPrivateBroadcastConn()) {
if (pto->m_connected + PRIVATE_BROADCAST_MAX_CONNECTION_LIFETIME < current_time) {
LogInfo("[privatebroadcast] Disconnecting: did not complete the transaction send within %d seconds, peer=%d%s",
count_seconds(PRIVATE_BROADCAST_MAX_CONNECTION_LIFETIME), pto->GetId(), pto->LogIP(fLogIPs));
LogDebug(BCLog::PRIVBROADCAST, "Disconnecting: did not complete the transaction send within %d seconds, peer=%d%s",
count_seconds(PRIVATE_BROADCAST_MAX_CONNECTION_LIFETIME), pto->GetId(), pto->LogIP(fLogIPs));
pto->fDisconnect = true;
}
return true;