#34322 node: Persist private broadcast transactions over node restarts

ReviewabilityReviewsAgreementSize
#34322 andrewtoth node: Persist private broadcast transactions over node restartsAuthor silent 244d0Mild243 + 134 tests
  • Prevent loss of pending private broadcast transactions when a node shuts down or restarts
  • Ensure transactions submitted for private broadcast over Tor or I2P are relayed after reboot
  • Needs rebase
  • Author silent for over 200 days
  • Concept support for avoiding transaction loss across restarts (vasild, craigraw)
  • Unaddressed objection: unrestricted file permissions could leak private broadcast data (vasild)
  • Privacy concern that storing broadcast data on disk expands the attack surface (tankyleo)
  • Suggested a full broadcast pool would better handle crashes and mempool eviction (craigraw)
  • 243 lines added or modified outside tests
  • 134 lines added or modified in tests
  • 0 lines removed in total
  • 14 files, 5 commits
P2P#24 of 71
P2 · bug fix
  • P2 because it prevents silent transaction loss across restarts for nodes using private broadcast
  • Protects relay reliability and liveness for transactions waiting for Tor or I2P broadcast
  • Ranking pass: Ranked below the leak fixes because the private broadcast rework (#36628/#36608) may rewrite the state it persists.
Ranking notes
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

Serializes pending private broadcast transactions to `privatebroadcast.dat` on node shutdown and reloads them on startup when `-privatebroadcast` is enabled. It adds methods to inspect broadcast state and hooks into the node initialization and shutdown sequence, deleting the file after load.

Problem: Private broadcast transactions exist solely in memory inside the peer manager. If a node shuts down or restarts before completing private broadcasts over Tor or I2P, those pending transactions are dropped and will never be broadcast.

Evidence of importance:

  • Description notes: 'Currently private broadcast transactions are stored in peer manager and do not persist over restarts. A submitted transaction can be lost if the node restarts before it is privately broadcast.'
  • Follow-up to #29415 which added the private broadcast mechanism.

Scope: Adds a new fuzz test target and unit tests for file persistence, in addition to modifying functional tests.

Risk of a mistake: medium (p2p). A flaw in disk serialization or deserialization during startup or shutdown could lead to startup crashes, data corruption, or unhandled exceptions.

Changed since the previous assessment: Diff provided for analysis: introduces `DumpPrivateBroadcast` and `LoadPrivateBroadcast` helpers using obfuscated `AutoFile` storage, adds `PrivateBroadcast::GetBroadcastInfo()`, adds a fuzz test target `private_broadcast_persist`, and adds test coverage.

Reviewability

Stale · Author silent 244d

No author activity for 244 days; a review may wait a long time for a response.

Agreement

Mild

objection that needs an answer has none yet (vasild)

Thread notes: Initial record for draft PR #34322. Tankyleo raised privacy/security concerns about writing broadcast records to disk on non-wallet nodes; Vasild provided Concept ACK with a reminder about 0600 permissions; Craigraw supported the concept but suggested a broader broadcast pool architecture, which the author agreed with as a better long-term design.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
tankyleo (none)objection, safetydumping sensitive transaction data to disk on broadcaster-only nodes expands the attack surface and privacy riskanswered · reply on record; standing not assessed yet · v1, not yet re-checkedansweredyes2026-01-18: 'I have reservations about dumping sensitive data to disk on node shutdown. Previously a non-wallet, broadcaster-only node would not persist any sensitive data to disk, but with this PR it now does.'
vasild (contributor)objection, safetyunrestricted permissions on privatebroadcast.dat could leak sensitive private broadcast dataopen · v1, not yet re-checkedansweredno2026-01-26: 'Make sure the file gets permissions `0600`.'
craigraw (none)objection, approacha shutdown-dump approach does not solve transaction eviction from mempools or crashes, unlike a proper persistent broadcast poolanswered · reply on record; standing not assessed yet · v1, not yet re-checkedansweredyes2026-01-28: 'However, I don't think this change goes far enough (or at least, should be considered a first step). Losing broadcasted transactions due to a node restart is probably quite rare.'

Support:

  • vasild (contributor) (Concept ACK, 2026-01-26): benefits of not losing transactions across restarts outweigh concerns about storing on disk
  • craigraw (none) (Concept ACK, 2026-01-28): agrees with persisting broadcast transactions, though suggests a broader broadcast pool approach

Participants:

  • tankyleo (none, objection, 2 statements 2026-01-18 to 2026-01-19): raised concerns about persisting sensitive broadcast tx data to disk on broadcaster-only nodes
  • vasild (contributor, support, 1 statements 2026-01-26 to 2026-01-26): Concept ACK; notes benefits outweigh disk concerns, suggested 0600 file permissions and retention option
  • craigraw (none, support, 1 statements 2026-01-28 to 2026-01-28): Concept ACK; suggested the approach doesn't go far enough and should consider a persistent broadcast pool

Files

134 lines under test/bench/ci.

  • src/node/private_broadcast_persist.cpp +106/-0
  • src/test/private_broadcast_tests.cpp +59/-0
  • src/test/fuzz/private_broadcast_persist.cpp +44/-0
  • test/functional/p2p_private_broadcast.py +30/-0
  • src/node/private_broadcast_persist.h +25/-0
  • src/private_broadcast.cpp +25/-0
  • src/node/private_broadcast_persist_args.h +19/-0
  • src/node/private_broadcast_persist_args.cpp +17/-0
  • src/init.cpp +15/-0
  • src/private_broadcast.h +14/-0
  • src/net_processing.cpp +13/-0
  • src/net_processing.h +7/-0
  • src/CMakeLists.txt +2/-0
  • src/test/fuzz/CMakeLists.txt +1/-0

Card

The short text the ranking pass compares across PRs.

Persists in-flight private broadcast transactions across node restarts by writing them to `privatebroadcast.dat` on shutdown and reloading them on startup. Transactions scheduled for private broadcast via Tor or I2P are otherwise lost if the node restarts before they are dispatched. The change adds disk serialization, startup loading, and associated unit and fuzz test coverage. It builds on the private broadcast feature added in #29415.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-25T00:41, confidence high, input hash b3bb3e4f0249cd86
thread read through 2026-03-02 (10 statements), head 29c8277d74
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Needs rebase, Private Broadcast · draft

Categories

P2P

P2 · bug fix (ranking pass: Ranked below the leak fixes because the private broadcast rework (#36628/#36608) may rewrite the state it persists.)

Prevents silent transaction loss during restarts for nodes using private broadcast. As the PR notes, 'a submitted transaction can be lost if the node restarts before it is privately broadcast,' so persisting pending transactions to disk addresses an important relay liveness and reliability issue.

Membership: Carries the 'Private Broadcast' label, updates net_processing, and directly manages the relay lifecycle of private broadcast transactions.

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