#36628 net: reimplement private broadcast as jobs isolated from the node
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36628 net: reimplement private broadcast as jobs isolated from the node | Needs rebase | 0 | Mild | 4,728 + 1,613 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Replaces the `-privatebroadcast` implementation with isolated, stateless background jobs managed under a new `src/privbcast` module. Submitted transactions bypass the local mempool and connection manager, running on dedicated threads that resolve DNS seeds via Tor and announce transactions over BIP324 on a fixed schedule using isolated proxy credentials for every connection. Completely removes legacy `ConnectionType::PRIVATE_BROADCAST` connections, retry tasks, and special-case handling from `CConnman` and `PeerManager`.
Problem: The existing private broadcast mechanism was intertwined with the node's main connection manager and peer logic, introducing special cases across addrman, peer discouragement, and connection tracking. This coupling risked deanonymization and IP linkage through shared node state while complicating maintainability of core P2P networking.
Evidence of importance:
- PR description: 'test profile focusing on protecting aginst IP address linkage and not crashing the node on adversarial inputs (which is a leak in itself)'
- Commit 03435a45ef: 'The plan is drawn when the job is made, before any socket exists... slots share only the loop, so a stalled stream holds up no other slot'
- PR description: 'Split out of #36608, the overall private broadcast rework to support ease of review for like-for-like feature.'
Scope: Very large diff (+6341/-3046 across 78 files) that adds a standalone byte-driven SOCKS5 and BIP324 network engine in src/privbcast/, introduces a formal invariant specification in doc/design/private-broadcast.md, and removes hundreds of lines of legacy private broadcast plumbing from net.cpp and net_processing.cpp.
Risk of a mistake: medium (p2p, rpc_api). Flaws in the isolated proxy or P2P state machine could deanonymize user transactions or exhaust system file descriptors.
Changed since the previous assessment: Rebased onto newer master commits; the core architecture, specification, and module structure remain identical.
Reviewability
Ready · Needs rebase
Conflicts with master; review the logic now, the rebase is mechanical.
Waiting on (from the thread read):
- author: push promised comment fix and address design direction / external hook proposal 2026-11-01
Agreement
Mild
objection that needs a change was answered, no reply from the objector (vasild: Relying permanently on centralized DNS seeds for each private broadcast creates a single point of failure and makes the ; vasild: Broadcasting exclusively to IP peers via Tor makes private broadcast fragile to Tor outages and exit node failures, lack; vasild: Restricting broadcast recipients to IP peers degrades network-wide privacy by preventing Tor and I2P nodes from acting a)
Thread notes: ajtowns reiterated their suggestion to deprecate the existing in-node feature and provide hooks for external tools rather than reviewing a large PR with competing design approaches.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fametrano (none) | objection, safety | Tests do not verify that attempts use unique nonces and BIP324 keys, allowing regressions that let recipients link broadcasts to pass undetected. | answered · author initially deferred but edited reply stating they changed their mind and brought it forward | answered | yes | 2026-10-25: 'This PR's tests do not check that each attempt gets its own VERSION nonce, PING nonce and BIP324 key (A4 in `doc/design/private-broadcast.md`), so a regression there would pass.' Status from: 2026-10-26: instagibbs: 'changed my mind and brought it forward' |
| vasild (member) | objection, approach | Relying permanently on centralized DNS seeds for each private broadcast creates a single point of failure and makes the feature vulnerable to outages or censorship. | answered · author replied explaining seed downtime only stops broadcasts without leaking identity, whereas addrman introduces sybil leaks | changed | yes | 2026-10-26: 'I do not think that it is reasonable to start relying on a few servers permanently (not just once in a lifetime, but every time a node wants to privately broadcast a transaction).' Status from: 2026-10-26: instagibbs: 'If the seeds go offline, private broadcast stops; it doesn't leak.' |
| vasild (member) | objection, safety | Broadcasting exclusively to IP peers via Tor makes private broadcast fragile to Tor outages and exit node failures, lacking an I2P fallback. | answered · author replied that Tor privacy outweighs its centralization and gathering addresses without seeder nodes reintroduces leaks | changed | yes | 2026-10-26: 'Only broadcasting to IP peers via the Tor network would make the feature too fragile - it would rely on the Tor network and the Tor exit nodes working properly.' Status from: 2026-10-26: instagibbs: 'Tor isn't decentralized either, but we accept utility in terms of privacy.' |
| vasild (member) | objection, safety | Restricting broadcast recipients to IP peers degrades network-wide privacy by preventing Tor and I2P nodes from acting as indistinguishable recipients. | answered · author replied that master's recipient model allows easy sybiling leading to IP leaks and offers limited attack resistance | changed | yes | 2026-10-26: 'Even moderate adoption of private broadcast in its current form improves the privacy of everybody because everybody could be a recipient. ... That would be spoiled if we only send to IP peers.' Status from: 2026-10-26: instagibbs: 'The attacks it resists are quite limited today, and therefore it's sort of in purgatory where we can't make any promises' |
| andrewtoth (member) | question, approach | How peers can be influenced into dialing fingerprinting peers, and whether that can be fixed upstream in addrman | answered · author explained addrman design goals differ and allowing peers to write to it enables fingerprinting | answered | yes | 2026-10-27: 'Can this design be expanded on about how they can be influenced, and why that is by design? Would this mean the problem is with the design that allows peers to be influenced into dialing to peers that' Status from: 2026-10-27: instagibbs: 'Addrman is built for partition resistance... private broadcast loses privacy to one dishonest peer that can "read" these' |
| andrewtoth (member) | question, safety | How an I2P sybil attack leaks a clearnet IP address, and how sybiling affects sending transactions through ephemeral I2P tunnels | answered · author explained small I2P population facilitates sybiling recipient slots and correlating clearnet probes | answered | yes | 2026-10-27: 'Would that be a problem with i2p in general, and not private broadcast? I'm not sure I understand how a sybil attack in i2p could leak a clearnet IP address.' Status from: 2026-10-27: instagibbs: 'The population of i2p is small, ... Once the attacker is a recipient, it can act on a suspect over clearnet before, duri' |
| andrewtoth (member) | question, safety | What specific attacks the current private broadcast implementation cannot resist | answered · author enumerated past CVE-like fixes and detailed the active adversary model that correlates recipient slots to clearnet probes | answered | yes | 2026-10-27: 'Since we no longer make any promises, and this is marked experimental, can we elaborate on the attacks it cannot resist?' Status from: 2026-10-27: instagibbs: 'The fixes so far: #35032 (addrman updated from private connections), #35410 (v1 retry went over clearnet), #36277 (what ' |
| ajtowns (member) | suggestion, approach | Publish the design as a BIP or post to the mailing list for broader discussion on design tradeoffs | answered · author agreed on value in wider discussion and noted the current implementation also bypassed the BIP process | answered | yes | 2026-10-27: 'Shouldn't it be a BIP, or at least posted to the mailing list for broader discussion on exactly the sort of tradeoffs vasild raises above?' Status from: 2026-10-27: instagibbs: 'I think there's definitely value increasing anonymity set if nothing else. By the same token, the current implementation' |
| ajtowns (member) | suggestion, approach | Implement private broadcast as a separate binary interacting lightly with the node rather than in-process jobs | answered · author replied that they originally designed it as a separate tool but aimed for a like-for-like replacement in bitcoind, while noting the code structure supports separation | answered | yes | 2026-10-27: 'If the idea is to separate this from the node, it seems like having a separate binary interacting lightly with the node might be a better structure.' Status from: 2026-10-27: instagibbs: 'Originally I designed it as a separate tool, but I was hoping for a like-for-like replacement. However the architecture ' |
| ajtowns (member) | suggestion, correctness | Correct or remove misleading comment regarding protocol version tracking across releases | answered · author agreed the comment was bad and unnecessary and promised to change it | changed | yes | 2026-10-27: '70016 is what current private broadcast versions track, and avoiding varying these things across releases is exactly why we have a special profile.' Status from: 2026-10-27: instagibbs: 'yeah this is a bad comment and probably not needed, it should *not* update as a profile break which is what the static a' |
| ajtowns (member) | suggestion, maintenance | Rewrite design doc to provide a high-level summary and avoid unnecessary jargon | answered · author agreed a hand-written design doc not targeting reimplementation is worth writing | answered | yes | 2026-10-28: 'FWIW, it feels overly long, and poorly focused to me; same as most AI spec work. It doesn't really give you a TLDR high-level summary, mixes high level and low level stuff in the same sentence, and ma' Status from: 2026-10-28: instagibbs: 'Definitely worth putting in a hand-written design doc that's not targeting reimplementation' |
| ajtowns (member) | suggestion, usefulness | Consider adding only a hook and letting the private broadcast implementation live out of tree given low transaction usage | answered · author argued the reimplementation is far easier to reason about than master, or else private broadcast should be removed entirely | answered | yes | 2026-10-28: 'That usage level doesn't feel like it warrants huge PRs to me. Could also plausibly just add the hook, and let the implementation live entirely out of tree, with any privacy issues dealt with external' Status from: 2026-10-28: instagibbs: 'If that's "too much work", we should remove it entirely, replaced with maybe a hook like you say.' |
| andrewtoth (member) | suggestion, approach | Do not use built-in DNS seeds to source addresses for private broadcast, and do not drop I2P or direct Tor onion connections | answered · author defended using DNS seeds as a liveness requirement to fit a definable, auditable security model | answered | yes | 2026-10-28: 'I don't think using the built-in DNS seeds to source addresses for private broadcast is the right approach, and I don't think we should drop i2p or direct tor onion connections.' Status from: 2026-10-29: instagibbs: 'I'm currently putting DNS seeds as a liveness requirement for a privacy tool.' |
| ajtowns (member) | suggestion, approach | Use MSG_TX and do not send wtxidrelay to maintain a larger anonymity set matching current code | answered · author explained wtxidrelay is intentional to support package relay and avoid future profile breaks | answered | yes | 2026-10-28: 'Using `MSG_TX` and not sending wtxidrelay (matching the current code) seems likely to have a larger anonymity set, without losing any meaningful performance afaics.' Status from: 2026-10-28: instagibbs: 'There's massive profile breaks everywhere in this PR. If we're going to break it, now is the time' |
| ajtowns (member) | suggestion, approach | Abandon in-node private broadcast, deprecate the existing feature, and provide hooks for external broadcast tools instead | contested · ajtowns pushed back against in-node reimplementation, arguing for deprecation and external hooks | answered | by others | 2026-10-29: 'abandon the approach of in-node support for private broadcast -- there's too much risk of state contamination that defeats the entire purpose of the feature for realistic adversarial scenarios' Status from: 2026-10-30: ajtowns: 'You should be able to conceive of the possibility of a theoretical attack being made both more more impactful and more p' |
Support:
- darosior (member) (Concept ACK, 2026-10-27): (no reason given) [not substantive]
- darosior (member) (2026-10-28) [read: private broadcast]: Isolating private broadcast connections from node state is the only way to avoid correlation leaks with clearnet connections; master's implementation is an active footgun that worsens privacy once adversaries correlate recipient slots
- darosior (member) (2026-10-29) [read: private broadcast]: Favored salvaging private broadcast in-tree rather than deprecating it, so node distributions can enable it to produce false positives against transaction origin analysis
Participants:
- fametrano (none, objection, 1 statements 2026-10-25 to 2026-10-25): concerned tests lack nonce and BIP324 key isolation checks
- vasild (member, objection, 2 statements 2026-10-26 to 2026-10-29): finds reliance on DNS seeds and excluding Tor/I2P peers unacceptable
- ajtowns (member, neutral, 8 statements 2026-10-27 to 2026-11-01): suggested abandoning in-node broadcast for external hooks, questioned PR size, and argued for deprecating existing implementation
- andrewtoth (member, question, 3 statements 2026-10-27 to 2026-10-29): asked about addrman attacks, and suggested avoiding DNS seeds and retaining I2P/Tor connections
- darosior (member, support, 3 statements 2026-10-27 to 2026-10-29): Concept ACK; argued state isolation prevents correlation leaks and supported keeping/fixing the feature in-tree
- spacexm406-web (none, neutral, 2 statements 2026-10-29 to 2026-10-29): posted an AI summary of review comments and an unelaborated review note
Dependencies
Enables:
Files
3448 lines under test/bench/ci.
- test/functional/p2p_private_broadcast.py +466/-390
- doc/design/private-broadcast.md +505/-0
- src/privbcast/job.cpp +505/-0
- src/node/privbcast.cpp +501/-0
- src/test/fuzz/privbcast_attempt.cpp +365/-0
- src/test/private_broadcast_tests.cpp +0/-334
- src/privbcast/attempt.cpp +325/-0
- src/node/privbcast.h +320/-0
- test/functional/test_framework/privbcast.py +318/-0
- src/private_broadcast.h +0/-271
- src/net_processing.cpp +15/-251
- src/test/fuzz/p2p_private_broadcast.cpp +0/-258
- src/test/fuzz/private_broadcast.cpp +0/-250
- src/net.cpp +16/-218
- src/privbcast/stream.cpp +234/-0
- src/rpc/mempool.cpp +160/-73
- src/privbcast/job.h +210/-0
- src/private_broadcast.cpp +0/-209
- src/privbcast/socks5.cpp +207/-0
- test/functional/p2p_private_broadcast_retry_v1.py +0/-199
- src/privbcast/attempt.h +189/-0
- src/privbcast/discovery.cpp +171/-0
- src/privbcast/params.h +151/-0
- src/privbcast/report.cpp +143/-0
- test/functional/p2p_private_broadcast_cap.py +50/-73
- src/privbcast/assign.cpp +121/-0
- src/privbcast/socks5.h +121/-0
- src/privbcast/stream.h +120/-0
- test/functional/test_framework/test_framework.py +0/-120
- test/functional/test_framework/socks5.py +101/-13
- src/privbcast/assign.h +104/-0
- src/net.h +3/-99
- src/privbcast/discovery.h +101/-0
- src/privbcast/report.h +100/-0
- test/functional/p2p_private_broadcast_completion.py +0/-99
- doc/private-broadcast.md +98/-0
- src/init.cpp +61/-30
- test/functional/p2p_private_broadcast_peers.py +79/-0
- src/privbcast/plan.cpp +75/-0
- src/test/util/privbcast.h +68/-0
- doc/release-notes-privbcast.md +63/-0
- test/functional/p2p_private_broadcast_proxy.py +60/-0
- src/privbcast/plan.h +53/-0
- test/functional/feature_config_args.py +21/-17
- src/test/denialofservice_tests.cpp +1/-34
- src/test/net_tests.cpp +0/-30
- src/test/privbcast_queue_tests.cpp +30/-0
- src/net_processing.h +0/-29
- src/test/privbcast_plan_tests.cpp +28/-0
- src/node/transaction.cpp +21/-1
- src/CMakeLists.txt +9/-1
- test/functional/feature_signet.py +9/-0
- src/test/fuzz/connman.cpp +1/-7
- src/node/connection_types.h +0/-7
- src/node/types.h +4/-2
- src/rpc/net.cpp +2/-4
- src/test/util/net.cpp +2/-4
- test/functional/feature_init.py +6/-0
- src/qt/rpcconsole.cpp +1/-4
- src/common/messages.cpp +4/-0
- test/functional/test_framework/p2p.py +3/-1
- test/functional/test_runner.py +2/-2
- src/netbase.h +3/-0
- src/node/context.h +3/-0
- src/primitives/transaction.h +1/-2
- src/test/CMakeLists.txt +2/-1
- src/test/fuzz/CMakeLists.txt +1/-2
- src/bitcoin-cli.cpp +0/-2
- src/kernel/chainparams.cpp +2/-0
- src/netbase.cpp +1/-1
- src/node/connection_types.cpp +0/-2
- src/node/peerman_args.cpp +0/-2
- src/node/txorphanage.h +1/-1
- src/qt/guiutil.cpp +0/-2
- src/rpc/util.cpp +2/-0
- doc/README.md +1/-0
- src/node/context.cpp +1/-0
- src/test/util/net.h +0/-1
Card
The short text the ranking pass compares across PRs.
Replaces the existing private broadcast subsystem with isolated, stateless background jobs managed under a new src/privbcast module. When -privatebroadcast is enabled, transactions bypass the local mempool and connection manager, executing on dedicated threads that discover peers via DNS seeds and announce over BIP324 strictly through Tor using isolated proxy credentials and a fixed schedule. This eliminates IP linkage and node-state correlation vectors while removing invasive private-broadcast special-case logic from CConnman and PeerManager. It unblocks follow-up package relay support for private broadcast in #36608.
Data
dossier JSON · extract JSON · model ledger, generated 2026-11-02T01:12, confidence high, input hash e69d3be7e29591ff
thread read through 2026-11-01 (28 statements), head 8e34f76058
labels: P2P, Needs rebase
thread read through 2026-11-01 (28 statements), head 8e34f76058
labels: P2P, Needs rebase
Categories
P2P
P1 · bug fix
The rewrite addresses severe deanonymization risks in master's private broadcast implementation. As reviewers noted, coupling broadcast connections to shared node state represents an 'active footgun that worsens privacy once adversaries adapt', making origin privacy protection a category-critical issue.
Membership: P2P label, extensive changes across net.cpp and net_processing.cpp to isolate transaction relay and connection management into dedicated Tor/BIP324 jobs.
Factors: security/stability 2, bug 2, performance 0, user value 2, leverage 1