#34271 net_processing: make m_tx_for_private_broadcast optional

full analysis

https://github.com/bitcoin/bitcoin/pull/34271 · vasild · +91/-40 in 2 files, 1 commits · labels: Needs rebase, Private Broadcast · draft

Goal

  • Avoid holding idle private broadcast state when the feature is disabled

This pull request wraps `PeerManagerImpl::m_tx_for_private_broadcast` in `std::optional`, initializing it only when `-privatebroadcast=1` is configured. It adds guards across private broadcast call sites in `net_processing` to disconnect or ignore requests if invoked when disengaged.

Problem: When private broadcast is disabled, `PeerManagerImpl` still instantiates and holds an idle `PrivateBroadcast` object. However, `std::optional` retains inline storage regardless, providing minimal memory benefit while introducing defensive checks for theoretically unreachable states.

Category: P2P (#65 of 65)

P4 · cleanup

  • P4 because it is a minor internal cleanup with no user-facing impact or performance gains
  • Memory usage is not reduced and it adds defensive branches for unreachable states

P4 because the change is a minor internal refactoring without user-facing impact, performance benefit, or memory savings. Because std::optional reserves inline storage, it does not reduce PeerManagerImpl memory footprint, while adding defensive error branches that the author noted add 'visual clutter and unreachable code'.

Membership: Touches PeerManagerImpl in src/net_processing.cpp and carries the Private Broadcast label.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0

Reviewability: Stale: Needs rebase

  • Needs rebase and has merge conflicts
  • Author has been inactive for over 60 days

The PR has merge conflicts with master, has the 'Needs rebase' label, and the author has been inactive for over 60 days.

Author status: Silent for over 60 days; expressed neutrality (~0) on whether the change is net-positive.

Open concerns:

  • Making the member optional requires defensive runtime checks or assertions in private broadcast methods that should never be reachable if disabled.
  • std::optional preserves inline storage within PeerManagerImpl, so it does not reduce object size without pointer indirection.
  • Decoy transaction proposals and other private broadcast followups may move private broadcast into core functionality rather than keeping it strictly opt-in.

Resolved concerns:

  • Deferred initialization was moved from the first call of InitiateTxBroadcastPrivate to the PeerManagerImpl constructor based on configuration options.
  • Handled silent conflicts where newly added private broadcast methods needed optional checks.

Agreement: Mild

  • Concept and approach support, but questioned if worth the visual clutter (vasild)
  • Does not reduce memory footprint without pointer indirection (l0rinc)
  • Future private broadcast plans may make opt-in handling obsolete (mzumsande)
  • Moved to draft pending direction on private broadcast (fanquake)

Mild: reviewers supported the concept, but the author and others question whether it is worth the visual clutter and whether future private broadcast plans make it obsolete.

Reviewers offered Concept and Approach ACKs, but mzumsande and the author noted architectural reservations about whether private broadcast remains an isolated opt-in feature, and the author noted that wrapping it in std::optional creates clutter and defensive branches for impossible states.

  • vasild noted: 'I am ~0 on making m_tx_for_private_broadcast optional... it adds visual clutter and unreachable code.'
  • l0rinc noted: 'Note that this still reserves storage for a PrivateBroadcast inside every PeerManagerImpl... avoiding the object-size footprint too would require indirection'
  • mzumsande noted: 'If in the future even nodes that don't opt into private broadcast... would send private transactions, then this would move private broadcast into core functionality, and making m_tx_for_private_broadcast optional wouldn't make sense.'
  • fanquake moved to draft: 'given it needs rebase, and there needs to be some decisions made on general private-broadcast direction.'

Review verdicts (DrahtBot): 0 (+1)

Files

File list not available for this run.

Card

This PR modifies PeerManagerImpl in net_processing to make m_tx_for_private_broadcast an std::optional, instantiating it only when private broadcast mode is active. It addresses a post-merge review comment from #29415 seeking cleaner separation of opt-in private broadcast state. Because std::optional retains inline storage, the change provides no memory footprint savings and adds defensive checks for unreachable states, leading both the author and reviewers to question its utility. The PR is currently marked draft and needs a rebase while broader architectural decisions on private broadcast evolve.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:50, confidence high, input hash 1a9e30e3b5196074