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.
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
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.
Concept ACK
Previously a non-wallet, broadcaster-only node would not persist any sensitive data to disk, but with this PR it now does …
True.
I think that the benefits of not losing the transaction if the node is restarted before a successful completion out-weight the concerns about storing the transaction on disk. Make sure the file gets permissions 0600. If an attacker has an access to the filesystem with the same user bitcoind is running as then they can debug or trace the running process to extract the information even if it is not stored on disk. Still there is a scenario where the machine is shutdown, stolen and inspected at a later time.
We can probably have the best from both worlds if a new option is introduced to instruct bitcoind how long to keep records, both in-memory and on-disk. With a value of 0 meaning “don’t keep records”.
Concept ACK. 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. A much more common UX issue is broadcasted transactions being lost from the user’s local mempool due to fee rate environment changes. This is disconcerting to the user as the transaction mysteriously disappears from their client wallet and they no longer have access to fee bump it. This is a poor user experience, and leads users to consider out-of-band solutions, or to increase the size of their local mempool which is resource inefficient and counterproductive to the network as a whole. It is reasonable to assume that a user’s broadcasted transactions carry much more economic value to them than most other transactions.
A further common need is scheduling a transaction broadcast so that it is stored locally and submitted to the network at some point in the future. Avoiding timing analysis is a common need, for example when migrating a wallet or regularly rotating UTXOs in a decaying multisig. Most bitcoin nodes (as opposed to client wallet software) are run as continuously online servers, and therefore are ideal candidates to handle this otherwise onerous task.
I won’t belabour these points, as they are described in detail in the feature request in #30471, which was partially (and independently) repeated in the feature request #34118. I believe there is substantial interest in addressing these common pain points.
I think that the benefits of not losing the transaction if the node is restarted before a successful completion out-weight the concerns about storing the transaction on disk.
I agree, although the idea of decoy transactions seems like a good addition.
Thanks for your thoughts @craigraw. Indeed, we could make the private broadcast storage a broadcast pool. However, I think the approach I have taken here is not suitable for that. This change dumps unfinished txs to disk and reloads them. If a node is not shut down gracefully, then these records are lost.
For a broadcast pool, we would want to keep a persistent SQLite file on disk and keep storing these txs until they expire. That would be a better first step here. Read a sqlite file on startup (create if doesn’t exist and privatebroadcast=1) and clear all entries past expiry time. Load entries that are not yet expired and try broadcasting again if privatebroadcast=1.
Future enhancements like storing a time at which a tx should be broadcast and decoy txs could be built on top of something like this. I think #34329 should be merged first, which persists finished txs at least in memory.