Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

offer alternative methods for customizing TX broadcast privacy #1056

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

conduition
Copy link
Contributor

Transactions which are first seen being broadcast from a user's node are more likely to be heuristically associated with the node runner personally. To break this heuristic, node runners might choose to broadcast a TX indirectly through alternative channels to disassociate that TX from the bitcoin node. Unfortunately bitcoind does not offer this option natively (yet).

This PR adds two new ways for users to broadcast transactions submitted to an electrs server:

  1. The pushtx crate. This initiates a short-lived set of P2P connections which publishes a transaction to random peers resolved from DNS seeds. The most private way to do this is using TOR, hiding the IP address of the sender.

  2. A custom script option. This allows users to customize the behavior of electrs broadcasting. It should return a zero status if the broadcast was successful, and emit an error message with a non-zero status otherwise.

Transactions which are first seen being broadcast from a user's node are
more likely to be heuristically associated with the node runner personally.
To break this heuristic, node runners might choose to broadcast a TX
indirectly through alternative channels to disassociate that TX from
the bitcoin node.

This PR adds two new ways for users to broadcast transactions through electrs:

1. The `pushtx` crate. This initiates a short-lived set of P2P connections
which publishes a transaction to random peers resolved from DNS seeds. The
most private way to do this is using TOR, hiding the IP address of the sender.

2. A custom script option. This allows users to customize the behavior of
electrs broadcasting. It should return a zero status if the broadcast was
successful, and emit an error message with a non-zero status otherwise.
Copy link
Contributor

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I doubt that sending a tx to random peers over clearnet helps. If a node detects another node connecting to it, then just sending a single tx and then disconnecting it can just assume the transaction is being broadcasted by the connecting client. So in effect it likely leaks privacy. As such I would probably refrain from adding it.

You need Dandelion for this to work privately.

src/config.rs Outdated
@@ -352,6 +411,8 @@ impl Config {
sync_once: config.sync_once,
skip_block_download_wait: config.skip_block_download_wait,
disable_electrum_rpc: config.disable_electrum_rpc,
tx_broadcast_method: config.tx_broadcast_method,
tx_broadcast_script: config.tx_broadcast_script,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be better to have another internal enum TxBroadcastMethod that carries Script(String) variant and do the conversion where you do the checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a try with this commit: d695a22

i think adding another type makes the namespacing a bit messier as now we have pub types TxBroadcastMethod, TxBroadcastMethodConfigOption, and TxBroadcaster. But it does help us avoid an unwrap call in electrum.rs, so either works for me.

@conduition
Copy link
Contributor Author

I doubt that sending a tx to random peers over clearnet helps

@Kixunil I strongly agree, and that's why the pushtx-tor option is preferable for most people. But it's possible that the user's networking setup already accounts for this, and so we shouldn't force them to use a TOR localhost proxy if they don't want to. For instance, electrs may be running on a machine with a VPN connection which the user relies on for network-level TX broadcasting privacy, while the bitcoind node runs on a separate machine without such protections.

Perhaps I could add some extra warnings to the pushtx-clear option to make it clear this option will degrade privacy if used incorrectly?

@conduition
Copy link
Contributor Author

Added extra warnings for pushtx-clear: 36b7bc4

@Kixunil
Copy link
Contributor

Kixunil commented Aug 6, 2024

But it's possible that the user's networking setup already accounts for this, and so we shouldn't force them to use a TOR localhost proxy if they don't want to.

Good point but this should be explicitly documented, so good that you did. FTR this is also useful on Whonix I think.

src/config.rs Outdated Show resolved Hide resolved
doc/config_example.toml Outdated Show resolved Hide resolved
@conduition
Copy link
Contributor Author

Thanks for the suggestions @Kixunil, added both 🙂

src/electrum.rs Outdated
@@ -360,7 +376,7 @@ impl Rpc {
fn transaction_broadcast(&self, (tx_hex,): &(String,)) -> Result<Value> {
let tx_bytes = Vec::from_hex(tx_hex).context("non-hex transaction")?;
let tx = deserialize(&tx_bytes).context("invalid transaction")?;
let txid = self.daemon.broadcast(&tx)?;
let txid = self.tx_broadcaster.broadcast(&tx)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use Arc<Daemon> instead of just Daemon? A &Daemon can be passed to the broadcast method here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An excellent idea. Applied in 2dd759d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants