[codex] Add transport peer selection modes#7854
Conversation
|
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
1870845 to
5297ef2
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors the transport and peer selection configuration to support more flexible peer transport modes (such as preferring Tor or TCP/IP). It introduces new transport types (TorTcp and TcpTor), updates the address filtering logic, and deprecates the old tor_socks_address setting for TCP transport. The review feedback suggests fully removing the deprecated tor_socks_address handling from the TCP comms stack initialization since it is now dead code.
| let config = transport_config.tcp; | ||
| debug!(target: LOG_TARGET, "Building TCP comms stack"); |
There was a problem hiding this comment.
Since tor_socks_address is deprecated and ignored for TransportType::Tcp, we should avoid implementing warning logic or other handling for it. Instead, consider removing the deprecated field and its associated code entirely, as it is now dead code.
| let config = transport_config.tcp; | |
| debug!(target: LOG_TARGET, "Building TCP comms stack"); | |
| debug!(target: LOG_TARGET, "Building TCP comms stack"); |
References
- Instead of implementing logic for a feature that relies on an unpopulated field, consider removing the feature and its associated code as it may be dead code.
Implements the four public TransportType peer-selection modes required by tari-project#7830: - tor: onion only - tcp: TCP only - tor_tcp: onion preferred, TCP fallback - tcp_tor: TCP preferred, onion fallback, and the default Adds unit coverage for protocol ordering/address filtering and cucumber coverage for the four mode expectations. Fixes tari-project#7830
dc6bede to
e076b65
Compare
| .await; | ||
|
|
||
| let comms = if p2p_config.transport.transport_type == TransportType::Tor { | ||
| let comms = if p2p_config.transport.uses_tor_hidden_service() { |
There was a problem hiding this comment.
need to handle the cases above here as well
| .spawn_with_transport(transport) | ||
| .await? | ||
| }, | ||
| let comms = if transport_config.is_memory_transport() { |
There was a problem hiding this comment.
this nested if is very hard to follow
| .spawn_with_transport(transport) | ||
| .await? | ||
| } else { | ||
| match transport_config.transport_type { |
There was a problem hiding this comment.
you are not properly setting up tcp or tor here, look at the change diff
Fixes #7830.
Summary
TransportTypemodes withTor,Tcp,TorTcp, andTcpTor, withTcpToras the default.TransportConfigoverrides for existing tests/config compatibility.Validation
cargo test -p tari_p2p transport_type --no-default-features --features tari_common_sqlite/bundledcargo test -p tari_comms dial_address_selection_filters_and_orders_transport_modes --no-default-features --features tari_common_sqlite/bundledcargo check -p minotari_app_utilities --no-default-featurescargo check -p tari_libtor --no-default-featuresNotes
cargo check -p minotari_node --no-default-featuresand cucumber compile-only were attempted locally but stopped before checking this patch becauserandomx-rsrequirescmake, which is not installed in this Windows environment.