Removes the logic which allows private broadcasting to connect via tor exit relays.
This mitigates the bug noted in #35319 , simplifies the logic, and is "less surprising" to me as I thought that's how it worked before.
The proxy logic could still be fixed/removed/??? but left it in place for smallest changeset that raises assurance on what we're shipping.
Proposed alternative to #35319 , at least for backport
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35323.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | willcl-ark, w0xlt |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Concept ACK.
I was wondering for a bit "what's wrong with using an exit node", but realise now thats something of a red herring to the fix.
I prefer the approach here of only selecting a tor/i2p peer for private broadcast (not ipv4/6 peer over tor proxy/exit node). I agree it's cleaner/easier to reason about than having to ensure the proxy override is preserved perfectly over retries and reconnections.
Concept ACK.
I prefer the approach here of only selecting a tor/i2p peer for private broadcast (not ipv4/6 peer over tor proxy/exit node).
Agreed.
It is much easier to spin Tor nodes than IPv4 or IPv6 nodes. I think being able to send transactions to IPv4 and IPv6 peers is very much desired.
This PR would cripple a feature without fixing the actual problem which is elsewhere. The actual problem is forgetting to use the proxy_override given to OpenNetworkConnection() during the v2->v1 reconnect.
Documentation can be improved.
It is much easier to spin Tor nodes than IPv4 or IPv6 nodes. I think being able to send transactions to IPv4 and IPv6 peers is very much desired.
Agreed. The last thing we want is to incentivize people to effectively Sybil-attack the Bitcoin-Tor network.
Agreed. The last thing we want is to incentivize people to effectively Sybil-attack the Bitcoin-Tor network.
Exit nodes being honest + ip costs as anti-sybil mechanism is valid enough if the worry is that we'd tip the scales in favor of even more sybil nodes. Would also give some design credence to the random choice of network being made today (though perhaps we could try preferred networks first, then fallback as we fail to complete, since maybe fewer people would have been exposed to this bug).
Will think more on it.
This PR would cripple a feature without fixing the actual problem which is elsewhere. The actual problem is forgetting to use the proxy_override given to OpenNetworkConnection() during the v2->v1 reconnect.
Well, this PR essentially rips out the source of the entire bug, and all this proxy passing through infra is unnecessary if taken. If there are other good reasons to not do it and retain it, fine.