Skip to content

Commit

Permalink
feat: add multiaddr with range checks for use with universe (#6557)
Browse files Browse the repository at this point in the history
Description
---
Added `MultiaddrRange`, which implements range checks for `Multiaddr`,
when using IP4 with TCP. This enables specifying a range of IP4 with TCP
addresses. As an exmple, any communication node can enable test
addresses to connect to them (`allow_test_addresses = true`), but
refrain from dialling any test addresses in return
(`excluded_dial_addresses = ["/ip4/127.*.*.*/tcp/*"]`).

With application to universe:
- TCP seed node settings:
   ```
   allow_test_addresses = true
   excluded_dial_addresses = [
         "/ip4/127.*.*.*/tcp/0:18188", 
         "/ip4/127.*.*.*/tcp/18190:65534",
         "/ip4/127.0.0.0/tcp/18189",
         "/ip4/127.1:255.1:255.2:255/tcp/18189"
   ] # Only '/ip4/127.0.0.1/tcp/0:18189' allowed
   ```
- Universe base node settings:
   ```
   type = "tcp"
   public_addresses = ["/ip4/127.0.0.1/tcp/18189"]
   tcp.listener_address = "/ip4/0.0.0.0/tcp/18189"
   allow_test_addresses = true
   public_addresses = ["/ip4/127.0.0.1/tcp/18189"]
   #excluded_dial_addresses = []
   ```
- Universe wallet settings:
   ```
   dns_seeds = []
custom_base_node =
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::/ip4/127.0.0.1/tcp/18189"
   type = "tcp"
   public_addresses = ["/ip4/127.0.0.1/tcp/18188"]
   tcp.listener_address = "/ip4/0.0.0.0/tcp/18188"
   allow_test_addresses = true
   public_addresses = ["/ip4/127.0.0.1/tcp/18188"]
   #excluded_dial_addresses = []
   ```

Motivation and Context
---
Currently, Universe base nodes and wallets use
`/ip4/172.2.3.4/tcp/18189` and `/ip4/172.2.3.4/tcp/18188` as their
public addresses respectively, but any node trying to contact them is
not able to. This results in many wasted resources. The Universe wallets
also maintain connections with the seed nodes, which is not ideal.

How Has This Been Tested?
---
- Added new unit tests
- System-level testing using the suggested settings^ (simulated seed
node, simulated universe base node, simulated universe wallet)

**From the seed node to the universe wallet**
```
>> add-peer 4602fb85883fec887e6b5e5a93cbc9547f19817685e78ff0a9e585826e322b44 /ip4/127.0.0.1/tcp/18188
Peer with node id '9a7764f742bf4e4bae6c5bd4f6' was added to the base node.
>> ☎️  Dialing peer...
☠️ ConnectionFailed: All peer addresses are excluded for peer 9a7764f742bf4e4bae6c5bd4f6
```
**From the seed node to the universe base node**
```
>> add-peer ee9ad9dce31a2d4a9225f4965e50df98ae4f85b58f94b34b9db9cc44f2aa2921 /ip4/127.0.0.1/tcp/18189
Peer with node id '998eb49cf4f2dd3b3d5a394c8e' was added to the base node.
☎️  Dialing peer...
⚡️ Peer connected in 0ms!
Connection: Id: 1, Node ID: 998eb49cf4f2dd3b, Direction: Inbound, Peer Address: /ip4/192.168.5.114/tcp/62398, Age: 1913s, #Substreams: 2, #Refs: 2
```
**From the universe base node to the universe wallet**
```
>> add-peer 4602fb85883fec887e6b5e5a93cbc9547f19817685e78ff0a9e585826e322b44 /ip4/127.0.0.1/tcp/18188
Peer with node id '9a7764f742bf4e4bae6c5bd4f6' was added to the base node.
☎️  Dialing peer...
⚡️ Peer connected in 0ms!
Connection: Id: 8, Node ID: 9a7764f742bf4e4b, Direction: Inbound, Peer Address: /ip4/127.0.0.1/tcp/62412, Age: 2054s, #Substreams: 6, #Refs: 2
```
**From the universe base node to the seed node**
```
>> add-peer 6677c4d401b98f403de671712a98ad8bf2976db27a1d411b08bedfd86751e048 /ip4/192.168.5.114/tcp/9991
Peer with node id '85d605836f02951c65651f99d0' was added to the base node.
☎️  Dialing peer...
>> ⚡️ Peer connected in 0ms!
Connection: Id: 0, Node ID: 85d605836f02951c, Direction: Outbound, Peer Address: /ip4/192.168.5.114/tcp/9991, Age: 2050s, #Substreams: 2, #Refs: 3
```

What process can a PR reviewer use to test or verify this change?
---
- Code review
- System-level testing

<!-- 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 -->
  • Loading branch information
hansieodendaal authored Sep 18, 2024
1 parent fe90e64 commit aca61f3
Show file tree
Hide file tree
Showing 22 changed files with 606 additions and 40 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion base_layer/contacts/tests/contacts_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ pub fn setup_contacts_service<T: ContactsBackend + 'static>(
auto_request: true,
..Default::default()
},
excluded_dial_addresses: vec![],
..Default::default()
},
allow_test_addresses: true,
Expand Down
2 changes: 1 addition & 1 deletion base_layer/p2p/src/initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ async fn configure_comms_and_dht(
.with_listener_liveness_allowlist_cidrs(listener_liveness_allowlist_cidrs)
.with_dial_backoff(ConstantBackoff::new(Duration::from_millis(500)))
.with_peer_storage(peer_database, Some(file_lock))
.with_excluded_dial_addresses(config.dht.excluded_dial_addresses.clone());
.with_excluded_dial_addresses(config.dht.excluded_dial_addresses.clone().into_vec().clone());

let mut comms = match config.auxiliary_tcp_listener_address {
Some(ref addr) => builder.with_auxiliary_tcp_listener_address(addr.clone()).build()?,
Expand Down
6 changes: 6 additions & 0 deletions base_layer/wallet_ffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub enum InterfaceError {
InvalidEmojiId,
#[error("An error has occurred due to an invalid argument: `{0}`")]
InvalidArgument(String),
#[error("An internal error has occurred: `{0}`")]
InternalError(String),
#[error("Balance Unavailable")]
BalanceError,
}
Expand Down Expand Up @@ -106,6 +108,10 @@ impl From<InterfaceError> for LibWalletError {
code: 9,
message: format!("Pointer error on {}:{:?}", p, v),
},
InterfaceError::InternalError(_) => Self {
code: 10,
message: format!("{:?}", v),
},
}
}
}
Expand Down
31 changes: 30 additions & 1 deletion base_layer/wallet_ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ use tari_common_types::{
};
use tari_comms::{
multiaddr::Multiaddr,
net_address::{MultiaddrRange, MultiaddrRangeList, IP4_TCP_TEST_ADDR_RANGE},
peer_manager::{NodeIdentity, PeerQuery},
transports::MemoryTransport,
types::CommsPublicKey,
Expand Down Expand Up @@ -5199,6 +5200,7 @@ pub unsafe extern "C" fn transport_config_destroy(transport: *mut TariTransportC
/// `database_path` - The database path char array pointer which. This is the folder path where the
/// database files will be created and the application has write access to
/// `discovery_timeout_in_secs`: specify how long the Discovery Timeout for the wallet is.
/// `exclude_dial_test_addresses`: exclude dialing of test addresses; this should be 'true' for production wallets
/// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions
/// as an out parameter.
///
Expand All @@ -5217,6 +5219,7 @@ pub unsafe extern "C" fn comms_config_create(
datastore_path: *const c_char,
discovery_timeout_in_secs: c_ulonglong,
saf_message_duration_in_secs: c_ulonglong,
exclude_dial_test_addresses: bool,
error_out: *mut c_int,
) -> *mut TariCommsConfig {
let mut error = 0;
Expand Down Expand Up @@ -5294,6 +5297,20 @@ pub unsafe extern "C" fn comms_config_create(
MultiaddrList::from(vec![public_address])
};

let excluded_dial_addresses = if exclude_dial_test_addresses {
let multi_addr_range = match MultiaddrRange::from_str(IP4_TCP_TEST_ADDR_RANGE) {
Ok(val) => val,
Err(e) => {
error = LibWalletError::from(InterfaceError::InternalError(e)).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
},
};
MultiaddrRangeList::from(vec![multi_addr_range])
} else {
MultiaddrRangeList::from(vec![])
};

let config = TariCommsConfig {
override_from: None,
public_addresses: addresses,
Expand Down Expand Up @@ -5326,7 +5343,7 @@ pub unsafe extern "C" fn comms_config_create(
minimum_desired_tcpv4_node_ratio: 0.0,
..Default::default()
},
excluded_dial_addresses: vec![],
excluded_dial_addresses,
..Default::default()
},
allow_test_addresses: true,
Expand Down Expand Up @@ -10237,6 +10254,7 @@ mod test {
db_path_alice_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -10401,6 +10419,7 @@ mod test {
db_path_alice_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -10628,6 +10647,7 @@ mod test {
db_path_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -10691,6 +10711,7 @@ mod test {
db_path_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -10774,6 +10795,7 @@ mod test {
db_path_alice_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -10951,6 +10973,7 @@ mod test {
db_path_alice_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -11089,6 +11112,7 @@ mod test {
db_path_alice_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -11308,6 +11332,7 @@ mod test {
db_path_alice_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -11534,6 +11559,7 @@ mod test {
db_path_alice_str,
20,
10800,
false,
error_ptr,
);

Expand Down Expand Up @@ -11795,6 +11821,7 @@ mod test {
db_path_str,
20,
10800,
false,
error_ptr,
);
let passphrase: *const c_char = CString::into_raw(CString::new("niao").unwrap()) as *const c_char;
Expand Down Expand Up @@ -12175,6 +12202,7 @@ mod test {
alice_db_path_str,
20,
10800,
false,
error_ptr,
);
let passphrase: *const c_char = CString::into_raw(CString::new("niao").unwrap()) as *const c_char;
Expand Down Expand Up @@ -12239,6 +12267,7 @@ mod test {
bob_db_path_str,
20,
10800,
false,
error_ptr,
);
let passphrase: *const c_char = CString::into_raw(CString::new("niao").unwrap()) as *const c_char;
Expand Down
2 changes: 2 additions & 0 deletions base_layer/wallet_ffi/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -2758,6 +2758,7 @@ void transport_config_destroy(TariTransportConfig *transport);
* `database_path` - The database path char array pointer which. This is the folder path where the
* database files will be created and the application has write access to
* `discovery_timeout_in_secs`: specify how long the Discovery Timeout for the wallet is.
* `exclude_dial_test_addresses`: exclude dialing of test addresses; this should be 'true' for production wallets
* `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions
* as an out parameter.
*
Expand All @@ -2774,6 +2775,7 @@ TariCommsConfig *comms_config_create(const char *public_address,
const char *datastore_path,
unsigned long long discovery_timeout_in_secs,
unsigned long long saf_message_duration_in_secs,
bool exclude_dial_test_addresses,
int *error_out);

/**
Expand Down
1 change: 1 addition & 0 deletions clients/ffi_client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ try {
"./wallet",
30,
600,
false,
err
);

Expand Down
1 change: 1 addition & 0 deletions clients/ffi_client/recovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ try {
"./recovery",
30,
600,
false,
err
);

Expand Down
10 changes: 6 additions & 4 deletions common/config/presets/c_base_node_c.toml
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,6 @@ database_url = "data/base_node/dht.db"
#ban_duration = 21_600 # 6 * 60 * 60
# Length of time to ban a peer for a "short" duration. Default: 60 mins
#ban_duration_short = 3_600 # 60 * 60
# This allows the use of test addresses in the network like 127.0.0.1. Default: false
#allow_test_addresses = false
# The maximum number of messages over `flood_ban_timespan` to allow before banning the peer (for `ban_duration_short`)
# Default: 100_000 messages
#flood_ban_max_msg_count = 100_000
Expand All @@ -316,5 +314,9 @@ database_url = "data/base_node/dht.db"
# In a situation where a node is not well-connected and many nodes are locally marked as offline, we can retry
# peers that were previously tried. Default: 2 hours
#offline_peer_cooldown = 7_200 # 2 * 60 * 60
# Addresses that should never be dialed (default value = [])
#excluded_dial_addresses = ["/ip4/x.x.x.x/tcp/xxxx", "/ip4/x.y.x.y/tcp/xyxy"]
# Addresses that should never be dialed (default value = []). This can be a specific address or an IPv4/TCP range.
# Example: When used in conjunction with `allow_test_addresses = true` (but it could be any other range)
# `excluded_dial_addresses = ["/ip4/127.*.0:49.*/tcp/*", "/ip4/127.*.101:255.*/tcp/*"]`
# or
# `excluded_dial_addresses = ["/ip4/127.0:0.1/tcp/122", "/ip4/127.0:0.1/tcp/1000:2000"]`
#excluded_dial_addresses = []
10 changes: 6 additions & 4 deletions common/config/presets/d_console_wallet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,6 @@ network_discovery.initial_peer_sync_delay = 25
#ban_duration = 21_600 # 6 * 60 * 60
# Length of time to ban a peer for a "short" duration. Default: 60 mins
#ban_duration_short = 3_600 # 60 * 60
# This allows the use of test addresses in the network like 127.0.0.1. Default: false
#allow_test_addresses = false
# The maximum number of messages over `flood_ban_timespan` to allow before banning the peer (for `ban_duration_short`)
# Default: 100_000 messages
#flood_ban_max_msg_count = 100_000
Expand All @@ -360,5 +358,9 @@ network_discovery.initial_peer_sync_delay = 25
# In a situation where a node is not well-connected and many nodes are locally marked as offline, we can retry
# peers that were previously tried. Default: 2 hours
#offline_peer_cooldown = 7_200 # 2 * 60 * 60
# Addresses that should never be dialed (default value = [])
#excluded_dial_addresses = ["/ip4/x.x.x.x/tcp/xxxx", "/ip4/x.y.x.y/tcp/xyxy"]
# Addresses that should never be dialed (default value = []). This can be a specific address or an IPv4/TCP range.
# Example: When used in conjunction with `allow_test_addresses = true` (but it could be any other range)
# `excluded_dial_addresses = ["/ip4/127.*.0:49.*/tcp/*", "/ip4/127.*.101:255.*/tcp/*"]`
# or
# `excluded_dial_addresses = ["/ip4/127.0:0.1/tcp/122", "/ip4/127.0:0.1/tcp/1000:2000"]`
#excluded_dial_addresses = []
1 change: 1 addition & 0 deletions comms/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ zeroize = "1"
[dev-dependencies]
tari_test_utils = { path = "../../infrastructure/test_utils" }
tari_comms_rpc_macros = { path = "../rpc_macros" }
toml = { version = "0.5"}

env_logger = "0.7.0"
serde_json = "1.0.39"
Expand Down
3 changes: 2 additions & 1 deletion comms/core/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use crate::{
connection_manager::{ConnectionManagerConfig, ConnectionManagerRequester},
connectivity::{ConnectivityConfig, ConnectivityRequester},
multiaddr::Multiaddr,
net_address::MultiaddrRange,
peer_manager::{NodeIdentity, PeerManager},
peer_validator::PeerValidatorConfig,
protocol::{NodeNetworkInfo, ProtocolExtensions},
Expand Down Expand Up @@ -242,7 +243,7 @@ impl CommsBuilder {
self
}

pub fn with_excluded_dial_addresses(mut self, excluded_addresses: Vec<Multiaddr>) -> Self {
pub fn with_excluded_dial_addresses(mut self, excluded_addresses: Vec<MultiaddrRange>) -> Self {
self.connection_manager_config.excluded_dial_addresses = excluded_addresses;
self
}
Expand Down
6 changes: 3 additions & 3 deletions comms/core/src/connection_manager/dialer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use crate::{
},
multiaddr::Multiaddr,
multiplexing::Yamux,
net_address::PeerAddressSource,
net_address::{MultiaddrRange, PeerAddressSource},
noise::{NoiseConfig, NoiseSocket},
peer_manager::{NodeId, NodeIdentity, Peer, PeerManager},
protocol::ProtocolId,
Expand Down Expand Up @@ -557,7 +557,7 @@ where
noise_config: &NoiseConfig,
transport: &TTransport,
network_byte: u8,
excluded_dial_addresses: Vec<Multiaddr>,
excluded_dial_addresses: Vec<MultiaddrRange>,
) -> (
DialState,
Result<(NoiseSocket<TTransport::Output>, Multiaddr), ConnectionManagerError>,
Expand All @@ -568,7 +568,7 @@ where
.clone()
.into_vec()
.iter()
.filter(|&a| !excluded_dial_addresses.iter().any(|excluded| a == excluded))
.filter(|&a| !excluded_dial_addresses.iter().any(|excluded| excluded.contains(a)))
.cloned()
.collect::<Vec<_>>();
if addresses.is_empty() {
Expand Down
3 changes: 2 additions & 1 deletion comms/core/src/connection_manager/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use crate::{
backoff::Backoff,
connection_manager::ConnectionId,
multiplexing::Substream,
net_address::MultiaddrRange,
noise::NoiseConfig,
peer_manager::{NodeId, NodeIdentity, PeerManagerError},
peer_validator::PeerValidatorConfig,
Expand Down Expand Up @@ -134,7 +135,7 @@ pub struct ConnectionManagerConfig {
/// Peer validation configuration. See [PeerValidatorConfig]
pub peer_validation_config: PeerValidatorConfig,
/// Addresses that should never be dialed
pub excluded_dial_addresses: Vec<Multiaddr>,
pub excluded_dial_addresses: Vec<MultiaddrRange>,
}

impl Default for ConnectionManagerConfig {
Expand Down
2 changes: 1 addition & 1 deletion comms/core/src/connection_manager/tests/listener_dialer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ async fn excluded_yes() {
let (request_tx, request_rx) = mpsc::channel(1);
let peer_manager2 = build_peer_manager();
let connection_manager_config = ConnectionManagerConfig {
excluded_dial_addresses: vec![address.clone()],
excluded_dial_addresses: vec![address.to_string().parse().unwrap()],
..Default::default()
};
let mut dialer = Dialer::new(
Expand Down
3 changes: 3 additions & 0 deletions comms/core/src/net_address/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,6 @@ pub use multiaddr_with_stats::{MultiaddrWithStats, PeerAddressSource};

mod mutliaddresses_with_stats;
pub use mutliaddresses_with_stats::MultiaddressesWithStats;

mod multiaddr_range;
pub use multiaddr_range::{MultiaddrRange, MultiaddrRangeList, IP4_TCP_TEST_ADDR_RANGE};
Loading

0 comments on commit aca61f3

Please sign in to comment.