Follow-up from #29415
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.
This change dumps the set of private broadcast transactions to a privatebroadcast.dat file on shutdown, and adds the transactions back to the private broadcast data structure on restart.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34322.
See the guideline for information on the review process. A summary of reviews will appear here.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):
2026-01-18
Introduce helpers to serialize and deserialize in-flight private broadcast
transactions to a new on-disk file, privatebroadcast.dat.
On startup, load transactions from privatebroadcast.dat into PrivateBroadcast
when -privatebroadcast is enabled, and remove the file afterward.
On shutdown, dump any remaining private broadcast transactions to disk.
PrivateBroadcast::GetBroadcastInfo() which has more information than we need for this change, but it is the same first commit in #34329.
Casual observer here, very excited by the progress on this feature.
This change dumps the set of private broadcast transactions to a privatebroadcast.dat file on shutdown
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. This changes the risk profile. How do you assess this tradeoff ? I am aware I am missing most of the context here, thank you very much for your time.
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.
Overall, I expect the wallet to take care of persisting any signed transactions that are in the “broadcast queue”, and make any broadcast retries as necessary following node shutdowns.
Previously a non-wallet, broadcaster-only node would not persist any sensitive data to disk, but with this PR it now does.
This PR only changes behavior of nodes configured with privatebroadcast=1, so a previous node would not have any behavior change unless it is intentionally reconfigured.
The usecase I envision is for quickly restarting a node shortly after a transaction is sent, in which case the file is deleted as soon as the node starts up again. It wouldn’t make sense to me to shut down your node right after you call sendrawtransaction and then leave it off before your transaction was successfully broadcast. This would cause the file to stay on your disk, but why would you shut it down before your send completes? Your intention was to broadcast the transaction. Do you see any other scenario where we would leave the file intact on disk for an extended period?
Overall, I expect the wallet to take care of persisting any signed transactions that are in the “broadcast queue”, and make any broadcast retries as necessary following node shutdowns.
Currently privatebroadcast only works with the sendrawtransaction RPC and not any of the wallet RPCs that broadcast transactions. If an external application calls sendrawtransaction on the node, it would expect that the transaction would be broadcast to the network. It should not have to be concerned with whether the node is restarted shortly after sending.
This PR only changes behavior of nodes configured with privatebroadcast=1, so a previous node would not have any behavior change unless it is intentionally reconfigured.
I agree that there is no change by default. My concern is that turning on privatebroadcast=1 expands the attack surface I have to worry about on a non-wallet, broadcaster only node, to include the disk. The disk now could potentially hold a list of some transactions I (or the users of my service / wallet / app) originated. This piece of data is much more sensitive than any other data persisted to the disk on this kind of node (non-wallet, broadcaster only).
The usecase I envision is for quickly restarting a node shortly after a transaction is sent, in which case the file is deleted as soon as the node starts up again. It wouldn’t make sense to me to shut down your node right after you call sendrawtransaction and then leave it off before your transaction was successfully broadcast. This would cause the file to stay on your disk, but why would you shut it down before your send completes? Your intention was to broadcast the transaction. Do you see any other scenario where we would leave the file intact on disk for an extended period?
I have in mind accidental / non-intentional / maliciously triggered shutdowns, or even intentional shutdowns followed by some unexpected failure to start the node again.
In general, I would expect wallets to call sendrawtransaction again if the transaction does not appear in the mempool after some timeout, which would handle the quick restart you describe.
Currently privatebroadcast only works with the sendrawtransaction RPC and not any of the wallet RPCs that broadcast transactions.
Right earlier I was referring more generally to my conviction that wallets alone should persist transactions from the originator, not the node (including but not limited to Bitcoin Core’s wallet).
If an external application calls sendrawtransaction on the node, it would expect that the transaction would be broadcast to the network. It should not have to be concerned with whether the node is restarted shortly after sending.
I agree that the wallet should not be aware of whether the node restarted. Nonetheless the wallet should handle the case: “hey the node crashed / shutdown / encountered a temporary failure during broadcast, and the transaction got lost. Try broadcasting again please.”
@tankyleo Thank you for your thoughts.
My concern is that turning on privatebroadcast=1 expands the attack surface I have to worry about on a non-wallet, broadcaster only node, to include the disk. The disk now could potentially hold a list of some transactions I (or the users of my service / wallet / app) originated. This piece of data is much more sensitive than any other data persisted to the disk on this kind of node (non-wallet, broadcaster only).
Note that this node must also be configured with disablewallet=1, otherwise RPC clients could use the disk for wallets even if not intended by the node runner.
As of right now, all this information is already stored in the debug.log (see #34267). This change could potentially store even less information to disk but also obfuscated. I don’t agree that this is a major concern, but I suppose it should be documented.
Also, decoy transactions (also discussed in #29415) would mitigate this risk. It would create plausible deniability about the origin of the transactions stored in this file.
I have in mind accidental / non-intentional / maliciously triggered shutdowns, or even intentional shutdowns followed by some unexpected failure to start the node again.
This scenario feels far fetched to me. A private broadcast is in the middle of being sent, and an accident or adversary causes the node to shutdown gracefully. Then, the node does not get started up again before this file’s contents is copied somewhere else.
Right now this PR removes the privatebroadcast.dat if it exists after the mempool is loaded, even if it fails to parse it somehow. I suppose this could be moved further up the loading path, so it gets removed as early as possible. This should mitigate any “failure to start the node again”.
In general, I would expect wallets to call sendrawtransaction again if the transaction does not appear in the mempool after some timeout, which would handle the quick restart you describe.
Right earlier I was referring more generally to my conviction that wallets alone should persist transactions from the originator, not the node (including but not limited to Bitcoin Core’s wallet).
I agree that the wallet should not be aware of whether the node restarted. Nonetheless the wallet should handle the case: “hey the node crashed / shutdown / encountered a temporary failure during broadcast, and the transaction got lost. Try broadcasting again please.”
I agree that wallets should do all these things, but I don’t think we should expect that behavior. We should aim to make our features as robust as possible, which this change does. Users should expect that Core will “just work” when they call sendtorawtransaction.