Skip to content

Commit b0cfe30

Browse files
fix: tcp node peers discovery (#7438)
Description --- Fixes base node connectivity issue with other peers when tor is disabled. Filters out peers addresses which are unsupported on host machine - this includes: - `onion` addresses if user has run `minotari_node` without tor. - IPv6 addresses if host machine has no IPv6 address This reduce amount of failed dial attempts to other peers and improves connectivity overall. Motivation and Context --- Some users might not want to run tor with base node so we must ensure it's still working without it. How Has This Been Tested? --- Run test cases in `peer_manager::storage::database::tests` which validates peer filtering. Build `minotari_node` and replaced one in the Tari Universe and I have removed my node data directories including peer_db. I was able to pass bootstrap and enter header sync with 6 peers on mainnet. What process can a PR reviewer use to test or verify this change? --- Code review, running test and same as above - run node and wait until it get some connections. Additional check `network.log` for base node to verify if `minotari_node` was attempting to peers that had only onion address. It should dial only if address is reachable. <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Introduced TransportProtocol to enumerate network address schemes and configure supported protocols during startup. - Builder option to set transport protocols; transports now advertise supported protocols and auto-detect IPv6. - Peer selection, discovery and dialing honor protocol reachability for more reliable connections. - Documentation - Clarified that peer propagation considers address reachability. - Tests - Updated tests to support protocol-aware APIs. - Chores - Added runtime dependency to detect IPv6. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: SW van Heerden <swvheerden@gmail.com>
1 parent 1022aa8 commit b0cfe30

File tree

32 files changed

+545
-82
lines changed

32 files changed

+545
-82
lines changed

Cargo.lock

Lines changed: 38 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

base_layer/core/src/test_helpers/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use tari_comms::{
4444
PeerFeatures,
4545
PeerFlags,
4646
},
47-
types::CommsPublicKey,
47+
types::{CommsPublicKey, TransportProtocol},
4848
PeerManager,
4949
};
5050
use tari_crypto::keys::SecretKey;
@@ -233,7 +233,7 @@ fn create_test_peer() -> Peer {
233233
pub fn create_peer_manager() -> Arc<PeerManager> {
234234
let db_connection = DbConnection::connect_temp_file_and_migrate(MIGRATIONS).unwrap();
235235
let peers_db = PeerDatabaseSql::new(db_connection, &create_test_peer()).unwrap();
236-
Arc::new(PeerManager::new(peers_db).unwrap())
236+
Arc::new(PeerManager::new(peers_db, TransportProtocol::get_all()).unwrap())
237237
}
238238

239239
pub fn create_chain_header(header: BlockHeader, prev_accum: &BlockHeaderAccumulatedData) -> ChainHeader {

base_layer/p2p/src/initialization.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ use tari_comms::{
5757
NodeNetworkInfo,
5858
ProtocolId,
5959
},
60-
tor,
61-
tor::{HiddenServiceControllerError, TorIdentity},
60+
tor::{self, HiddenServiceControllerError, TorIdentity},
6261
transports::{
6362
predicate::FalsePredicate,
6463
HiddenServiceTransport,
@@ -332,8 +331,9 @@ async fn configure_comms_and_dht(
332331
.with_listener_liveness_max_sessions(config.listener_liveness_max_sessions)
333332
.with_listener_liveness_allowlist_cidrs(listener_liveness_allowlist_cidrs)
334333
.with_dial_backoff(ConstantBackoff::new(Duration::from_millis(500)))
334+
.with_transport_protocols(config.transport.transport_type.get_supported_protocols())
335335
.with_peer_storage(peer_database)
336-
.with_excluded_dial_addresses(config.dht.excluded_dial_addresses.clone().into_vec().clone());
336+
.with_excluded_dial_addresses(config.dht.excluded_dial_addresses.clone().into_vec());
337337

338338
let mut comms = match config.auxiliary_tcp_listener_address {
339339
Some(ref addr) => builder.with_auxiliary_tcp_listener_address(addr.clone()).build()?,

base_layer/p2p/src/services/liveness/service.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,7 @@ mod test {
468468
PeerFlags,
469469
},
470470
test_utils::mocks::create_connectivity_mock,
471+
types::TransportProtocol,
471472
};
472473
use tari_comms_dht::{
473474
envelope::{DhtMessageHeader, DhtMessageType},
@@ -491,7 +492,7 @@ mod test {
491492
pub fn build_peer_manager() -> Arc<PeerManager> {
492493
let db_connection = DbConnection::connect_temp_file_and_migrate(MIGRATIONS).unwrap();
493494
let peers_db = PeerDatabaseSql::new(db_connection, &create_test_peer()).unwrap();
494-
Arc::new(PeerManager::new(peers_db).unwrap())
495+
Arc::new(PeerManager::new(peers_db, TransportProtocol::get_all()).unwrap())
495496
}
496497

497498
#[tokio::test]

base_layer/p2p/src/transport.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ use serde::{Deserialize, Serialize};
2525
use tari_comms::{
2626
multiaddr::Multiaddr,
2727
socks,
28-
tor,
29-
tor::TorIdentity,
28+
tor::{self, TorIdentity},
3029
transports::{predicate::FalsePredicate, SocksConfig},
30+
types::TransportProtocol,
3131
utils::multiaddr::multiaddr_to_socketaddr,
3232
};
3333

@@ -101,6 +101,25 @@ pub enum TransportType {
101101
Socks5,
102102
}
103103

104+
impl TransportType {
105+
pub fn get_supported_protocols(&self) -> Vec<TransportProtocol> {
106+
match self {
107+
TransportType::Memory => vec![TransportProtocol::Memory],
108+
TransportType::Tcp => vec![TransportProtocol::Ipv4, TransportProtocol::Ipv6],
109+
TransportType::Tor => vec![
110+
TransportProtocol::Onion,
111+
TransportProtocol::Ipv4,
112+
TransportProtocol::Ipv6,
113+
],
114+
TransportType::Socks5 => vec![
115+
TransportProtocol::Onion,
116+
TransportProtocol::Ipv4,
117+
TransportProtocol::Ipv6,
118+
],
119+
}
120+
}
121+
}
122+
104123
impl Default for TransportType {
105124
fn default() -> Self {
106125
// The tor transport configures itself as long as it has access to the control port at

comms/core/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ tower = { version = "0.4", features = ["util"] }
6565
tracing = "0.1.26"
6666
yamux = "0.13.2"
6767
zeroize = "1"
68+
local-ip-address = "0.6.5"
6869

6970
[dev-dependencies]
7071
tari_test_utils = { workspace = true }

comms/core/src/builder/mod.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ use crate::{
5050
peer_validator::PeerValidatorConfig,
5151
protocol::{NodeNetworkInfo, ProtocolExtensions},
5252
tor,
53-
types::CommsDatabase,
53+
types::{CommsDatabase, TransportProtocol},
5454
};
5555

5656
/// # CommsBuilder
@@ -124,6 +124,7 @@ pub struct CommsBuilder {
124124
connectivity_config: ConnectivityConfig,
125125
shutdown_signal: Option<ShutdownSignal>,
126126
maintain_n_closest_connections_only: Option<usize>,
127+
transport_protocols: Vec<TransportProtocol>,
127128
}
128129

129130
impl Default for CommsBuilder {
@@ -137,6 +138,7 @@ impl Default for CommsBuilder {
137138
connectivity_config: ConnectivityConfig::default(),
138139
shutdown_signal: None,
139140
maintain_n_closest_connections_only: None,
141+
transport_protocols: TransportProtocol::get_all(),
140142
}
141143
}
142144
}
@@ -310,10 +312,18 @@ impl CommsBuilder {
310312
self
311313
}
312314

315+
/// Set the transport protocols to use for communication
316+
/// if not provided defaults to IP4, IP6, TOR and memory address
317+
pub fn with_transport_protocols(mut self, protocols: Vec<TransportProtocol>) -> Self {
318+
self.transport_protocols = protocols;
319+
self
320+
}
321+
313322
fn make_peer_manager(&mut self) -> Result<Arc<PeerManager>, CommsBuilderError> {
314323
match self.peer_storage.take() {
315324
Some(storage) => {
316-
let peer_manager = PeerManager::new(storage).map_err(CommsBuilderError::PeerManagerError)?;
325+
let peer_manager = PeerManager::new(storage, self.transport_protocols.clone())
326+
.map_err(CommsBuilderError::PeerManagerError)?;
317327
Ok(Arc::new(peer_manager))
318328
},
319329
None => Err(CommsBuilderError::PeerStorageNotProvided),

comms/core/src/connection_manager/dialer.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ use crate::connection_manager::metrics;
5252
use crate::{
5353
backoff::Backoff,
5454
connection_manager::{
55-
common,
56-
common::ValidatedPeerIdentityExchange,
55+
common::{self, ValidatedPeerIdentityExchange},
5756
dial_state::DialState,
5857
manager::{ConnectionManagerConfig, ConnectionManagerEvent},
5958
peer_connection,
@@ -589,7 +588,7 @@ where
589588
}
590589
}
591590

592-
/// Attempts to dial a peer sequentially on all addresses; if connections are to be minimized only.
591+
/// Attempts to dial a peer sequentially on supported addresses; if connections are to be minimized only.
593592
/// Returns ownership of the given `DialState` and a success or failure result for the dial,
594593
/// or None if the dial was cancelled inflight
595594
#[allow(clippy::too_many_lines)]
@@ -603,15 +602,22 @@ where
603602
DialState,
604603
Result<(NoiseSocket<TTransport::Output>, Multiaddr), ConnectionManagerError>,
605604
) {
605+
let supported_transport_protocols = transport.supported_protocols();
606+
trace!(target: LOG_TARGET, "Supported transport protocols: {:?}", supported_transport_protocols);
607+
606608
let addresses = dial_state
607609
.peer()
608610
.addresses
609611
.clone()
610612
.into_vec()
611613
.iter()
612-
.filter(|&a| !excluded_dial_addresses.iter().any(|excluded| excluded.contains(a)))
614+
.filter(|&a| {
615+
!excluded_dial_addresses.iter().any(|excluded| excluded.contains(a)) &&
616+
supported_transport_protocols.contains(a.into())
617+
})
613618
.cloned()
614619
.collect::<Vec<_>>();
620+
615621
if addresses.is_empty() {
616622
let node_id_hex = dial_state.peer().node_id.clone().to_hex();
617623
trace!(

0 commit comments

Comments
 (0)