bitcoin icon indicating copy to clipboard operation
bitcoin copied to clipboard

Broadcast own transactions only via short-lived Tor or I2P connections

Open vasild opened this issue 1 year ago • 106 comments

To improve privacy, broadcast locally submitted transactions (from the sendrawtransaction RPC) to the P2P network only via Tor or I2P short-lived connections, or to IPv4/IPv6 peers but through the Tor network.

  • Introduce a new connection type for private broadcast of transactions with the following properties:

    • started whenever there are local transactions to be sent
    • opened to Tor or I2P peers or IPv4/IPv6 via the Tor proxy
    • opened regardless of max connections limits
    • after handshake is completed one local transaction is pushed to the peer, PING is sent and after receiving PONG the connection is closed
    • ignore all incoming messages after handshake is completed (except PONG)
  • Broadcast transactions submitted via sendrawtransaction using this new mechanism, to a few peers. Keep doing this until we receive back this transaction from one of our ordinary peers (this takes about 1 second on mainnet).

  • The transaction is stored in peerman and does not enter the mempool.

  • Once we get an INV from one of our ordinary peers, then the normal flow executes: we request the transaction with GETDATA, receive it with a TX message, put it in our mempool and broadcast it to all our existent connections (as if we see it for the first time).

  • After we receive the full transaction as a TX message, in reply to our GETDATA request, only then consider the transaction has propagated through the network and remove it from the storage in peerman, ending the private broadcast attempts.

The messages exchange should look like this:

tx-sender >--- connect -------> tx-recipient
tx-sender >--- VERSION -------> tx-recipient (dummy VERSION with no revealing data)
tx-sender <--- VERSION -------< tx-recipient
tx-sender <--- WTXIDRELAY ----< tx-recipient (maybe)
tx-sender <--- SENDADDRV2 ----< tx-recipient (maybe)
tx-sender <--- SENDTXRCNCL ---< tx-recipient (maybe)
tx-sender <--- VERACK --------< tx-recipient
tx-sender >--- VERACK --------> tx-recipient
tx-sender >--- INV/TX --------> tx-recipient (if we take the last commit: fixup!)
tx-sender <--- GETDATA/TX ----< tx-recipient (if we take the last commit: fixup!)
tx-sender >--- TX ------------> tx-recipient
tx-sender >--- PING ----------> tx-recipient
tx-sender <--- PONG ----------< tx-recipient
tx-sender disconnects

Whenever a new transaction is received from sendrawtransaction RPC, the node will send it to a few (NUM_PRIVATE_BROADCAST_PER_TX) recipients right away. If after 10-15 mins we still have not heard anything about the transaction from the network, then it will be sent to 1 more peer (see PeerManagerImpl::ReattemptPrivateBroadcast()).

A few considerations:

  • The short-lived private broadcast connections are very cheap and fast wrt network traffic. It is expected that some of those peers could blackhole the transaction. Just one honest/proper peer is enough for successful propagation.
  • The peers that receive the transaction could deduce that this is initial transaction broadcast from the transaction originator. This is ok, they can't identify the sender.

How to test this?

Thank you, @stratospher and @andrewtoth!

Start bitcoind with -privatebroadcast=1 -debug=privatebroadcast.

Create a wallet and get a new address, go to the Signet faucet and request some coins to that address:

build/bin/bitcoin-cli -chain="signet" createwallet test
build/bin/bitcoin-cli -chain="signet" getnewaddress

Get a new address for the test transaction recipient:

build/bin/bitcoin-cli -chain="signet" loadwallet test
new_address=$(build/bin/bitcoin-cli -chain="signet" getnewaddress)

Create the transaction:

# Option 1: `createrawtransaction` and `signrawtransactionwithwallet`:

txid=$(build/bin/bitcoin-cli -chain="signet" listunspent | jq -r '.[0] | .txid')
vout=$(build/bin/bitcoin-cli -chain="signet" listunspent | jq -r '.[0] | .vout')
echo "txid: $txid"
echo "vout: $vout"

tx=$(build/bin/bitcoin-cli -chain="signet" createrawtransaction "[{\"txid\": \"$txid\", \"vout\": $vout}]" "[{\"$new_address\": 0.00001000}]" 0 false)
echo "tx: $tx"

signed_tx=$(build/bin/bitcoin-cli -chain="signet" signrawtransactionwithwallet "$tx" | jq -r '.hex')
echo "signed_tx: $signed_tx"

# OR Option 2: `walletcreatefundedpsbt` and `walletprocesspsbt`:
# This makes it not have to worry about inputs and also automatically sends back change to the wallet.
# Start `bitcoind` with `-fallbackfee=0.00003000` for instance for 3 sat/vbyte fee.

psbt=$(build/bin/bitcoin-cli -chain="signet" walletcreatefundedpsbt "[]" "[{\"$new_address\": 0.00001000}]" | jq -r '.psbt')
echo "psbt: $psbt"

signed_tx=$(build/bin/bitcoin-cli -chain="signet" walletprocesspsbt "$psbt" | jq -r '.hex')
echo "signed_tx: $signed_tx"

Finally, send the transaction:

raw_tx=$(build/bin/bitcoin-cli -chain="signet" sendrawtransaction "$signed_tx")
echo "raw_tx: $raw_tx"

High-level explanation of the commits
  • New logging category and config option to enable private broadcast

    • log: introduce a new category for private broadcast
    • init: introduce a new option to enable/disable private broadcast
  • Implement the private broadcast connection handling on the CConnman side:

    • net: introduce a new connection type for private broadcast
    • net: support overriding the proxy selection in ConnectNode()
    • net: implement opening PRIVATE_BROADCAST connections
  • Prepare BroadcastTransaction() for private broadcast requests:

    • net_processing: rename RelayTransaction to better describe what it does
    • node: change a tx-relay on/off flag to a tri-state
    • net_processing: store transactions for private broadcast in PeerManager
  • Implement the private broadcast connection handling on the PeerManager side:

    • net_processing: reorder the code that handles the VERSION message
    • net_processing: handle ConnectionType::PRIVATE_BROADCAST connections
    • net_processing: stop private broadcast of a transaction after round-trip
    • net_processing: retry private broadcast
  • Engage the new functionality from sendrawtransaction:

    • rpc: use private broadcast from sendrawtransaction RPC if -privatebroadcast is ON
  • Independent test framework improvement:

    • test: move create_malleated_version() to messages.py for reuse
  • New functional test that exercies some of the new code:

    • test: add functional test for local tx relay
  • Add an intermediate step that sends INV message and waits for a request for the transaction. If reviewers like this, then I will squash it into the relevant commit, or otherwise drop it:

    • fixup! net_processing: handle ConnectionType::PRIVATE_BROADCAST connections

This PR would close the following issues: https://github.com/bitcoin/bitcoin/issues/3828 Clients leak IPs if they are recipients of a transaction https://github.com/bitcoin/bitcoin/issues/14692 Can't configure bitocoind to only send tx via Tor but receive clearnet transactions https://github.com/bitcoin/bitcoin/issues/19042 Tor-only transaction broadcast onlynet=onion alternative https://github.com/bitcoin/bitcoin/issues/24557 Option for receive events with all networks, but send transactions and/or blocks only with anonymous network[s]? https://github.com/bitcoin/bitcoin/issues/25450 Ability to broadcast wallet transactions only via dedicated oneshot Tor connections https://github.com/bitcoin/bitcoin/issues/32235 Tor: TX circuit isolation

Issues that are related, but (maybe?) not to be closed by this PR: https://github.com/bitcoin/bitcoin/issues/21876 Broadcast a transaction to specific nodes https://github.com/bitcoin/bitcoin/issues/28636 new RPC: sendrawtransactiontopeer


Further extensions:

  • Have the wallet do the private broadcast as well, https://github.com/bitcoin/bitcoin/issues/11887 would have to be resolved.
  • Add some stats via RPC, so that the user can better monitor what is going on during and after the broadcast. Currently this can be done via the debug log, but that is not convenient.
  • Make the private broadcast storage, currently in peerman, persistent over node restarts.
  • Add (optional) random delay before starting to broadcast the transaction in order to avoid correlating unrelated transactions based on the time when they were broadcast. Suggested independently of this PR here.
  • Consider periodically sending transactions that did not originate from the node as decoy, discussed here.

A previous incarnation of this can be found at https://github.com/bitcoin/bitcoin/pull/27509. It puts the transaction in the mempool and (tries to) hide it from the outside observers. This turned out to be too error prone or maybe even impossible.

vasild avatar Feb 09 '24 13:02 vasild

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK andrewtoth, zzzi2p, nothingmuch
Stale ACK pinheadmz

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #30529 (Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior by ryanofsky)
  • #30381 ([WIP] net: return result from addnode RPC by willcl-ark)
  • #30343 (wallet, logging: Replace WalletLogPrintf() with LogInfo() by ryanofsky)
  • #30315 (Stratum v2 Transport by Sjors)
  • #30065 (init: fixes file descriptor accounting by sr-gi)
  • #29680 (wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict by Eunovo)
  • #29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)
  • #29605 (net: Favor peers from addrman over fetching seednodes by sr-gi)
  • #29500 (test: create assert_not_equal util by kevkevinpal)
  • #29432 (Stratum v2 Template Provider (take 3) by Sjors)
  • #29346 (Stratum v2 Noise Protocol by Sjors)
  • #29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)
  • #28521 (net, net_processing: additional and consistent disconnect logging by Sjors)
  • #28488 (p2p: Evict outbound peers with high minFeeRate by naumenkogs)
  • #28463 (p2p: Increase inbound capacity for block-relay only connections by mzumsande)
  • #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
  • #27826 (validation: log which peer sent us a header by Sjors)
  • #26697 (logging: use bitset for categories by LarryRuane)
  • #26619 (log: expand BCLog::LogFlags (categories) to 64 bits by LarryRuane)

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.

DrahtBot avatar Feb 09 '24 13:02 DrahtBot

I am not sure about this approach to improve privacy. Is it necessary to open new short lived tor/i2p connections for broadcasting the transaction? What are the trade-offs in this implementation vs a simple implementation to relay tx to one or more peers that our node is already connected to?

Related issues:

https://github.com/bitcoin/bitcoin/issues/21876 https://github.com/bitcoin/bitcoin/issues/28636

1440000bytes avatar Feb 09 '24 14:02 1440000bytes

@1440000bytes, thanks for asking! There is some discussion at https://github.com/bitcoin/bitcoin/pull/27509 (the previous attempt on this).

Is it necessary to open new short lived tor/i2p connections for broadcasting the transaction?

Yes, it is. See below.

What are the trade-offs in this implementation vs a simple implementation to relay tx to one or more peers that our node is already connected to?

Sending the transaction over clearnet reveals the IP address/geolocation of the sender. A spy with many connections to the network could try to guess who was the originator. So, why not send it to our Tor peers only? Because it is relatively easy for a spy to fingerprint and link clearnet and Tor connections to the same peer. That is, a long running connection over Tor could be linked to a long running clearnet connection. This is why the proposed changes open a short-lived connection that does not reveal any of the identity of the sender.

Would this benefit nodes that don't have clearnet connections, e.g. Tor/I2P-only nodes? Yes! In the case where the sender sends two otherwise unrelated transactions over the same long-running Tor connection, the recipient will know that they have the same origin, even though they are not related on-chain. Using single shot connections fixes that too.

Related issues:

Linked in the OP, thanks!

vasild avatar Feb 10 '24 12:02 vasild

v2 Transport will be enabled by default in the next release (https://github.com/bitcoin/bitcoin/pull/29347).

If there were eventually a change to force clearnet transactions over v2 transport (so the details of the communications were encrypted), would that solve the same problem that this PR is aiming to solve?

epiccurious avatar Feb 10 '24 21:02 epiccurious

@epiccurious, p2p encryption "solves" the spying from intermediate routers on clearnet (aka man-in-the-middle). Tor, I2P and CJDNS solve that too. While this PR uses only Tor and I2P it would solve that problem. But there is more - it will as well solve issues with spying bitcoin nodes.

vasild avatar Feb 11 '24 14:02 vasild

74ba7c7fb5...6fad02cf03: rebase due to conflicts

vasild avatar Feb 29 '24 13:02 vasild

concept ACK 6fad02cf03

(code review in progress)

I am also testing this feature in Warnet, which deploys a regtest network and even has an internal Tor DA so I can simulate onion routing locally. Currently using a 20-node graph and a scenario which connects the graph, adds onion addresses to the test node, and then sends a raw transaction from the node running this branch.

The private broadcast succeeds frequently but not always. In Warnet anyway I had better luck when the test node had -listenonion=0, I tried that after suspecting that inbound onion connections were removing potential peers from the private broadcast list, but I'm not sure.

I think I noticed this in the original PR as well, if multiple transactions are sent, the count keeps going up without a limit:

 [privatebroadcast] Requested to open 60 connection(s), trying to open one

Screenshot below, I managed to capture a private broadcast connection! I'll mention when i get to that commit in review as well, but the connection type "privbcast" is breaking the very nice -netinfo table :-)

So far I have a few questions about the strategy:

  1. How do we pick the onion peers to relay to? If we avoid reusing peers then (especially in my miniature network) we can run out quickly, and nothing ever gets broadcast.

  2. Are we using fresh Tor identities for these connections? I think Wasabi does something like this:

Wasabi connects to each peer through a different Tor stream.

  1. Do you think we need any RPC to get the status of private tx sends or provide the option to abandon (or revert to clearnet?)

  2. I assume you are restricting the feature to sendraw so the wallet doesn't get involved and try something silly like rebroadcasting over clearnet or inserting in to our own mempool? Do you think future work will integrate the wallet?

privbroadcast

pinheadmz avatar Mar 08 '24 16:03 pinheadmz

@pinheadmz, excellent! Thank you for looking into this! Replies inline:

The private broadcast succeeds frequently but not always

Lets debug this in a sub-thread in order not to overwhelm the PR main thread: here.

suspecting that inbound onion connections were removing potential peers from the private broadcast list

No, this cannot be the case. Inbound onion connections appear as IPv4 connections coming from the Tor daemon. They do not have any source address associated with them, other than the one of the Tor daemon itself (usually 127.0.0.1).

I think I noticed this in the original PR as well, if multiple transactions are sent, the count keeps going up without a limit

That is now capped at size_t MAX_PRIVATE_BROADCAST_CONNECTIONS{64}.

the connection type "privbcast" is breaking the very nice -netinfo table

This better be addressed in the visualization engine, e.g. by using %5.5s

How do we pick the onion peers to relay to? If we avoid reusing peers then...

We pick a random Tor (or I2P) peer. In the same way we choose the address when we want to have an outbound connection to a peer from a particular network. No attempt to avoid reuse. But we don't connect to already connected peers. This is the logic as in master.

Are we using fresh Tor identities for these connections?

Yes, if -proxyrandomize=1 (the default). This is the same in master - separate Tor circuit per connection. This PR does the same as Wasabi: https://docs.wasabiwallet.app/why-wasabi/NetworkLevelPrivacy.html#wasabi-wallet-light-node

Do you think we need any RPC to get the status of private tx sends or provide the option to abandon (or revert to clearnet?)

Yes, both would be useful, but as a followup PR. I am afraid putting those here would bloat this PR too much. I could however implement that and publish it under Draft PR that depends on this one, so people can use it during testing.

I assume you are restricting the feature to sendraw so the wallet doesn't get involved and try something silly like rebroadcasting over clearnet or inserting in to our own mempool?

The reason to omit the wallet is that it currently does not count non-mempool change. For example, if we have 1 BTC and send 0.6 to somebody and 0.4 as change, the wallet will show a balance of 0 (or 1, unchanged? somebody correct me if I am wrong) until the transaction enters the mempool (or gets mined in a block). This would be quite rough user experience. Note that this is already the case in master if -walletbroadcast=0.

Do you think future work will integrate the wallet?

Yes, but I left that for another PR. Omitting it here reduces the size of this PR and allows to deal with the wallet in isolation, after all the distracting infrastructure is in (e.g. log category, new type of connection, the "net" code, etc).

vasild avatar Mar 08 '24 18:03 vasild

6fad02cf03...d10a0649b0: rebase due to conflicts and address some suggestions

vasild avatar Mar 11 '24 17:03 vasild

d10a0649b0...5ca891f4e0: address some suggestions

vasild avatar Mar 12 '24 18:03 vasild

5ca891f4e0...b6dce67064: rebase due to conflicts

vasild avatar Mar 12 '24 18:03 vasild

b6dce67064...cc867ebd62: rebase due to conflicts

vasild avatar Mar 13 '24 12:03 vasild

Is this going to be on by default or just an option?

Haven't had a chance to look at the code submissions yet but I do have concerns if this is on by default that many corporate networks will start blocking installation or that transactions will not be submitted as many corporate networks block Tor or I2P connections by default at the corporate firewall level.

I assume it has failovers as well if it is by default?

iotamega avatar Mar 15 '24 17:03 iotamega

Is this going to be on by default or just an option?

This PR only affects transactions sent with RPC sendrawtransaction and only if -privatebroadcast=1 is configured, which is not default

pinheadmz avatar Mar 15 '24 19:03 pinheadmz

cc867ebd62...4828d46209: rebase and address suggestions

vasild avatar Mar 18 '24 14:03 vasild

Concept ACK

I made similar solutions for EPS and CoreLightning, which I'm happy to see being obviated by adding this functionality to the source!

Curious why this is just pushing an unsolicited TX message instead of using INV/GETDATA/TX? This conversation seems to convince me the latter is better? Also, if the submitted tx is spending unconfirmed outputs, should we support sending the parents to the peer if requested?

andrewtoth avatar Mar 19 '24 20:03 andrewtoth

Curious why this is just pushing an unsolicited TX message instead of using INV/GETDATA/TX? https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1186579724 seems to convince me the latter is better?

I followed up in that conversion. For now I am keeping it as it is, but if you are happy with all the rest and would otherwise ACK, then say so! ;-)

Also, if the submitted tx is spending unconfirmed outputs, should we support sending the parents to the peer if requested?

Right now in master sendrawtransaction RPC operates on a single transaction. Its parents must already be in the mempool. This PR keeps it that way. Parent+child is more for the submitpackage RPC. That + the wallet I will deal with in a followup PRs.

vasild avatar Mar 21 '24 17:03 vasild

Curious why this is just pushing an unsolicited TX message instead of using INV/GETDATA/TX? #27509 (comment) seems to convince me the latter is better?

I followed up in that conversion. For now I am keeping it as it is, but if you are happy with all the rest and would otherwise ACK, then say so! ;-)

Thanks. I read through the proposal to not accept unsolicited TX messages, and the reasoning to reject it makes sense to me. I suppose if you know you're the originator of a tx, the only reason not to push the tx unsolicited to all your peers is for privacy reasons. However, in this case the privacy reasons are moot because of the ephemeral private connections.

Also, if the submitted tx is spending unconfirmed outputs, should we support sending the parents to the peer if requested?

Right now in master sendrawtransaction RPC operates on a single transaction. Its parents must already be in the mempool. This PR keeps it that way. Parent+child is more for the submitpackage RPC. That + the wallet I will deal with in a followup PRs.

I mean the case where we have a parent unconfirmed tx in our mempool, and the peer we connected to does not. If we push a tx spending that unconfirmed parent, the peer will respond with a GETDATA for the parent tx. So we can either:

  1. Ignore the GETDATA . This will result in our tx not relaying through this connection if the peer doesn't have all unconfirmed parents.
  2. Wait for the GETDATA and respond with a TX message. We won't know if the peer will eventually ask for the parent, so this will be tricky and we will have to wait for some time. However, in this time if we receive an INV for it from another peer we can close because we know we were successful.
  3. Push all unconfirmed parents along with the tx we are sending. This will result in potentially wasted bandwidth.

andrewtoth avatar Mar 22 '24 14:03 andrewtoth

Oh wow just saw this! Thank you. Will review.

jb55 avatar Mar 27 '24 15:03 jb55

Concept ACK

This is consistent with i2p guidelines on resource usage. Good work!

zzzi2p avatar Mar 28 '24 19:03 zzzi2p

4828d46209...09ad469cc1: rebase and address suggestions

vasild avatar Mar 29 '24 10:03 vasild

I mean the case where we have a parent unconfirmed tx in our mempool, and the peer we connected to does not. If we push a tx spending that unconfirmed parent, the peer will respond with a GETDATA for the parent tx...

I will keep it simple stupid for now, meaning that in this case that particular peer will not relay the transaction further. Others should succeed. If "nobody" has the parent in their mempool, then somehow we are trying to broadcast an orphaned transaction with parent(s) not seen by the network. Can say that this is not supported at this point and will best be supported by a future submitpackage which does private broadcast.

vasild avatar Mar 29 '24 11:03 vasild

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/23235305634

DrahtBot avatar Mar 29 '24 11:03 DrahtBot

c7e4d5cb2b...413574e81e: rebase and address suggestions

vasild avatar Apr 01 '24 16:04 vasild

413574e81e...390e628a07: rebase due to conflicts and address suggestion

vasild avatar Apr 02 '24 16:04 vasild

390e628a07...9297437af2: rebase and address suggestions

Do we need test coverage for retrying stale txs and having multiple concurrent txs queued for private broadcast?

Yes.

vasild avatar Apr 04 '24 12:04 vasild

9297437af2...cc760207b8: rebase and address suggestions:

  • Give a startup warning if -privatebroadcast=1, -proxyrandomize=0 and the Tor network is reachable (i.e. we will use Tor for private broadcast).
  • Enforce -walletbroadcast=0 if -privatebroadcast=1 because it would be confusing to have the wallet do the traditional broadcast while the sendrawtransaction RPC does a private broadcast. Furthermore if a wallet transaction is sent via sendrawtransaction and ends up in the mempool from outside and is not mined for some time, then the wallet will try to broadcast it using the traditional mechanism.
  • Private broadcast also to IPv4 and IPv6 peers (!) through the Tor proxy.
  • Remove conflicting Tor address from the functional test and don't log every address added to addrman.

vasild avatar Apr 23 '24 17:04 vasild

Avoiding already connected peers would work around this, but perhaps it's sufficient just to warn on startup if privatebroadcast=1 and proxyrandomize=0

In master we already avoid connecting to an already connected address, regardless of the connection type:

https://github.com/bitcoin/bitcoin/blob/256e1703197fdddd78bc6d659431cd0fc3b63cde/src/net.cpp#L2862

I added a startup warning anyway.

I assume you are restricting the feature to sendraw so the wallet doesn't get involved and try something silly like rebroadcasting over clearnet or inserting in to our own mempool?

The reason to omit the wallet is that it currently does not count non-mempool change.

If I understand correctly, the concern with rebroadcasting still applies, but in the case that private broadcast was successful?

Yes, I enforced walletbroadcast=0 to avoid the wallet rebroadcasting.

vasild avatar Apr 23 '24 17:04 vasild

cc760207b8...ea1ca3715e: adjust feature_config_args.py after forbidding -walletbroadcast when -privatebroadcast is enabled.

vasild avatar Apr 24 '24 07:04 vasild

fd603b8f5c...42cb080600: rebase and address suggestions

vasild avatar May 01 '24 13:05 vasild