[codex] Implement transport peer selection modes by CharlieLZ · Pull Request #7853 · tari-project/tari

CharlieLZ · 2026-05-25T16:34:00Z

Fixes #7830

Summary

Add the requested public peer-selection transport modes: Tor, Tcp, TorTcp, and TcpTor, with TcpTor as the new default.
Propagate the configured transport protocol order into both peer candidate selection and dial address selection.
Sort and filter peer candidates and dial addresses by the configured protocol preference, while retaining the existing Memory and Socks5 transport variants for compatibility.
Add unit coverage for protocol ordering/filtering and a cucumber feature covering all four requested modes.

Validation

cargo test -p tari_p2p --no-default-features
cargo test -p tari_comms transport_protocol_preference --no-default-features
cargo test -p tari_integration_tests --test cucumber --no-run
git diff --check

Notes

I also attempted the full cargo test -p tari_comms --no-default-features suite locally. The transport-focused tests passed, but the full package run hit local baseline failures in temp SQLite/DNS-related tests outside this change.
cargo fmt --check is not usable in my local stable toolchain for this repo because rustfmt.toml enables nightly-only rustfmt options and stable reports broad baseline diffs. No source files were modified by that check.

github-actions · 2026-05-25T16:34:08Z

⚠️ This PR contains unsigned commits. To get your PR merged, please sign those commits (git rebase --exec 'git commit -S --amend --no-edit -n' @{upstream}) and force push them to this branch (git push --force-with-lease).

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.

Learn how to use 1Password to sign your commits.

gemini-code-assist

Code Review

This pull request introduces new transport types (TorTcp and TcpTor) to support preferred peer selection and dial address ordering based on transport protocols. The reviewer provided valuable feedback on optimizing address filtering to avoid cloning metadata, using sort_by_cached_key to prevent redundant string allocations during peer sorting, and addressing a potential non-determinism issue in SQLite when ordering distinct queries.

gemini-code-assist · 2026-05-25T16:35:32Z

+        let addresses = peer
+            .addresses
+            .clone()
+            .into_vec()
+            .into_iter()
+            .filter(|address| {
+                let protocol = TransportProtocol::from(address);
+                !excluded_dial_addresses
+                    .iter()
+                    .any(|excluded| excluded.contains(address))
+                    && preferred_transport_protocols.contains(&protocol)
+            })
+            .collect::<Vec<_>>();


Cloning the entire MultiaddressesWithStats struct and converting it to a vector is inefficient because it clones all address statistics and metadata only to discard them. Instead, you can iterate over the addresses slice directly using .addresses() and clone only the Multiaddr values that match the filter criteria.

let addresses = peer .addresses .addresses() .iter() .filter_map(|address| { let addr = address.address(); let protocol = TransportProtocol::from(addr); if !excluded_dial_addresses.iter().any(|excluded| excluded.contains(addr)) && preferred_transport_protocols.contains(&protocol) { Some(addr.clone()) } else { None } }) .collect::<Vec<_>>();

Current patch already iterates peer.addresses.addresses() and only clones the Multiaddr values that pass the filter. It does not clone the whole MultiaddressesWithStats value.

gemini-code-assist · 2026-05-25T16:35:32Z

+    fn order_peers_by_node_id_order(peers: &mut [Peer], node_ids: &[String]) {
+        let node_id_positions = node_ids
+            .iter()
+            .enumerate()
+            .map(|(index, node_id)| (node_id.clone(), index))
+            .collect::<HashMap<_, _>>();
+
+        peers.sort_by_key(|peer| {
+            node_id_positions
+                .get(&peer.node_id.to_hex())
+                .copied()
+                .unwrap_or(usize::MAX)
+        });
+    }


Using sort_by_key with peer.node_id.to_hex() will perform a string allocation and hex conversion O(n log n) times during sorting. Using sort_by_cached_key instead ensures that the key function is evaluated exactly once per element, reducing the number of allocations to O(n).

Suggested change

fn order_peers_by_node_id_order(peers: &mut [Peer], node_ids: &[String]) {

let node_id_positions = node_ids

.iter()

.enumerate()

.map(|(index, node_id)| (node_id.clone(), index))

.collect::<HashMap<_, _>>();

peers.sort_by_key(|peer| {

node_id_positions

.get(&peer.node_id.to_hex())

.copied()

.unwrap_or(usize::MAX)

});

}

fn order_peers_by_node_id_order(peers: &mut [Peer], node_ids: &[String]) {

let node_id_positions = node_ids

.iter()

.enumerate()

.map(|(index, node_id)| (node_id.clone(), index))

.collect::<HashMap<_, _>>();

peers.sort_by_cached_key(|peer| {

node_id_positions

.get(&peer.node_id.to_hex())

.copied()

.unwrap_or(usize::MAX)

});

}

Current patch already uses sort_by_cached_key here, so peer.node_id.to_hex() is evaluated once per peer during sorting.

gemini-code-assist · 2026-05-25T16:35:32Z

+    fn build_addr_preference_order_sql(transport_protocols: &[TransportProtocol]) -> Option<String> {
+        if transport_protocols.is_empty() {
+            return None;
+        }
+
+        let conditions = transport_protocols
+            .iter()
+            .enumerate()
+            .map(|(index, protocol)| {
+                format!(
+                    "WHEN multi_addresses.address LIKE '{}%' THEN {}",
+                    sql_escape(protocol.get_prefix()),
+                    index
+                )
+            })
+            .collect::<Vec<_>>();
+
+        Some(format!("CASE {} ELSE {} END", conditions.join(" "), transport_protocols.len()))
+    }


When a peer has multiple addresses (e.g., both Onion and IPv4), the joined query produces multiple rows for that peer. Because get_available_dial_candidates uses DISTINCT peers.node_id, SQLite collapses these rows. Ordering by multi_addresses.address (via order_sql) on a DISTINCT query is non-deterministic in SQLite because the value used for ordering the collapsed row is arbitrary. This can lead to incorrect candidates being selected when LIMIT is applied.

This query does not use DISTINCT; it selects ordered address rows, over-fetches by MAX_MULTIADDRESSES_PER_PEER, deduplicates node IDs in Rust while preserving row order, then fetches the peers and reapplies transport preference in memory. That avoids SQLite choosing an arbitrary address value for a collapsed distinct peer row.

SWvheerden · 2026-05-26T11:08:27Z

                .await?
        },
-        TransportType::Tor => {
+        TransportType::Tor | TransportType::TorTcp | TransportType::TcpTor => {


you need the handle the cases above also

SWvheerden · 2026-05-26T11:09:32Z

+        self.connection_manager_config.transport_protocols = protocols.clone();
        self.transport_protocols = protocols;


why clone it and store it twice

SWvheerden · 2026-05-26T11:11:16Z

+        supported_transport_protocols: &[TransportProtocol],
+        preferred_transport_protocols: &[TransportProtocol],


these seem like duplicates?

SWvheerden · 2026-05-26T11:21:49Z

+                query = query.limit(limit);
+            }
+
+            let node_ids = Self::unique_node_ids_preserving_order(query.load::<String>(conn)?, Some(n));


this combo will break the limit and qeury more

SWvheerden · 2026-05-29T08:06:16Z

closing in favour of #7851

gemini-code-assist Bot reviewed May 25, 2026

View reviewed changes

CharlieLZ force-pushed the codex/7830-transport-types branch from dd9e230 to e68f552 Compare May 25, 2026 16:35

CharlieLZ marked this pull request as ready for review May 25, 2026 16:36

This was referenced May 25, 2026

feat: add transport protocol preference modes #7852

Closed

Connection types #7830

Closed

Implement transport peer selection modes

8e94e76

CharlieLZ force-pushed the codex/7830-transport-types branch from e68f552 to 8e94e76 Compare May 25, 2026 17:02

SWvheerden requested changes May 26, 2026

View reviewed changes

SWvheerden closed this May 29, 2026

		self.connection_manager_config.transport_protocols = protocols.clone();
		self.transport_protocols = protocols;

		supported_transport_protocols: &[TransportProtocol],
		preferred_transport_protocols: &[TransportProtocol],

Conversation

CharlieLZ commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Notes

Uh oh!

github-actions Bot commented May 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Zhenvip May 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Zhenvip May 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Zhenvip May 25, 2026

Choose a reason for hiding this comment

Uh oh!

SWvheerden May 26, 2026

Choose a reason for hiding this comment

Uh oh!

SWvheerden May 26, 2026

Choose a reason for hiding this comment

Uh oh!

SWvheerden May 26, 2026

Choose a reason for hiding this comment

Uh oh!

SWvheerden May 26, 2026

Choose a reason for hiding this comment

Uh oh!

SWvheerden commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CharlieLZ commented May 25, 2026 •

edited

Loading