review comments

base_layer/wallet_ffi/src/error.rs

@@ -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,
 }
@@ -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),
+            },
         }
     }
 }

base_layer/wallet_ffi/src/lib.rs

@@ -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,
@@ -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.
 ///
@@ -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;
@@ -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,
@@ -5326,8 +5343,7 @@ pub unsafe extern "C" fn comms_config_create(
                         minimum_desired_tcpv4_node_ratio: 0.0,
                         ..Default::default()
                     },
-                    // FIXME: This should be set to 'IP4_TCP_TEST_ADDR_RANGE', but cucumber tests need to use that range
-                    excluded_dial_addresses: vec![].into(),
+                    excluded_dial_addresses,
                     ..Default::default()
                 },
                 allow_test_addresses: true,
@@ -10238,6 +10254,7 @@ mod test {
                 db_path_alice_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -10402,6 +10419,7 @@ mod test {
                 db_path_alice_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -10629,6 +10647,7 @@ mod test {
                 db_path_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -10692,6 +10711,7 @@ mod test {
                 db_path_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -10775,6 +10795,7 @@ mod test {
                 db_path_alice_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -10952,6 +10973,7 @@ mod test {
                 db_path_alice_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -11090,6 +11112,7 @@ mod test {
                 db_path_alice_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -11309,6 +11332,7 @@ mod test {
                 db_path_alice_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -11535,6 +11559,7 @@ mod test {
                 db_path_alice_str,
                 20,
                 10800,
+                false,
                 error_ptr,
             );
 
@@ -11796,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;
@@ -12176,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;
@@ -12240,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;

base_layer/wallet_ffi/wallet.h

@@ -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.
  *
@@ -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);
 
 /**

clients/ffi_client/index.js

@@ -39,6 +39,7 @@ try {
     "./wallet",
     30,
     600,
+    false,
     err
   );
 

clients/ffi_client/recovery.js

@@ -53,6 +53,7 @@ try {
     "./recovery",
     30,
     600,
+    false,
     err
   );
 

comms/core/src/net_address/mod.rs

@@ -29,4 +29,4 @@ mod mutliaddresses_with_stats;
 pub use mutliaddresses_with_stats::MultiaddressesWithStats;
 
 mod multiaddr_range;
-pub use multiaddr_range::{serde_multiaddr_range, MultiaddrRange, MultiaddrRangeList, IP4_TCP_TEST_ADDR_RANGE};
+pub use multiaddr_range::{MultiaddrRange, MultiaddrRangeList, IP4_TCP_TEST_ADDR_RANGE};

comms/core/src/net_address/multiaddr_range.rs

@@ -19,25 +19,19 @@ pub const IP4_TCP_TEST_ADDR_RANGE: &str = "/ip4/127.*.*.*/tcp/*";
 /// A struct containing either an Ipv4AddrRange or a Multiaddr. If a range of IP addresses and/or ports needs to be
 /// specified, the MultiaddrRange can be used, but it only supports IPv4 addresses with the TCP protocol.
 #[derive(Debug, Clone, Serialize, PartialEq, Eq)]
-pub struct MultiaddrRange {
-    ipv4_addr_range: Option<Ipv4AddrRange>,
-    multiaddr: Option<Multiaddr>,
+pub enum MultiaddrRange {
+    Ipv4AddrRange(Ipv4AddrRange),
+    Multiaddr(Multiaddr),
 }
 
 impl FromStr for MultiaddrRange {
     type Err = String;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         if let Ok(multiaddr) = Multiaddr::from_str(s) {
-            Ok(MultiaddrRange {
-                ipv4_addr_range: None,
-                multiaddr: Some(multiaddr),
-            })
+            Ok(MultiaddrRange::Multiaddr(multiaddr))
         } else if let Ok(ipv4_addr_range) = Ipv4AddrRange::from_str(s) {
-            Ok(MultiaddrRange {
-                ipv4_addr_range: Some(ipv4_addr_range),
-                multiaddr: None,
-            })
+            Ok(MultiaddrRange::Ipv4AddrRange(ipv4_addr_range))
         } else {
             Err("Invalid format for both Multiaddr and Ipv4AddrRange".to_string())
         }
@@ -46,33 +40,27 @@ impl FromStr for MultiaddrRange {
 
 impl fmt::Display for MultiaddrRange {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if let Some(ipv4_addr_range) = &self.ipv4_addr_range {
-            write!(f, "{}", ipv4_addr_range)
-        } else if let Some(multiaddr) = &self.multiaddr {
-            write!(f, "{}", multiaddr)
-        } else {
-            write!(f, "None")
+        match self {
+            MultiaddrRange::Ipv4AddrRange(ipv4_addr_range) => write!(f, "{}", ipv4_addr_range),
+            MultiaddrRange::Multiaddr(multiaddr) => write!(f, "{}", multiaddr),
         }
     }
 }
 
 impl MultiaddrRange {
     /// Check if the given Multiaddr is contained within the MultiaddrRange range
     pub fn contains(&self, addr: &Multiaddr) -> bool {
-        if let Some(ipv4_addr_range) = &self.ipv4_addr_range {
-            return ipv4_addr_range.contains(addr);
-        }
-        if let Some(multiaddr) = &self.multiaddr {
-            return multiaddr == addr;
+        match self {
+            MultiaddrRange::Ipv4AddrRange(ipv4_addr_range) => ipv4_addr_range.contains(addr),
+            MultiaddrRange::Multiaddr(multiaddr) => multiaddr == addr,
         }
-        false
     }
 }
 
-// ----------------- Ipv4AddrRange -----------------
-// A struct containing an Ipv4Range and a PortRange
+/// ----------------- Ipv4AddrRange -----------------
+/// A struct containing an Ipv4Range and a PortRange
 #[derive(Debug, Clone, Serialize, PartialEq, Eq)]
-struct Ipv4AddrRange {
+pub struct Ipv4AddrRange {
     ip_range: Ipv4Range,
     port_range: PortRange,
 }
@@ -280,6 +268,7 @@ impl fmt::Display for PortRange {
     }
 }
 
+/// ----------------- MultiaddrRangeList -----------------
 /// Supports deserialization from a sequence of strings or comma-delimited strings
 #[derive(Debug, Default, Clone, Serialize, PartialEq, Eq)]
 pub struct MultiaddrRangeList(Vec<MultiaddrRange>);
@@ -397,29 +386,6 @@ impl<'de> Deserialize<'de> for MultiaddrRange {
     }
 }
 
-pub mod serde_multiaddr_range {
-    use std::str::FromStr;
-
-    use serde::{de::Error, Deserialize, Deserializer, Serializer};
-
-    use crate::net_address::MultiaddrRange;
-
-    pub fn serialize<S>(value: &[MultiaddrRange], serializer: S) -> Result<S::Ok, S::Error>
-    where S: Serializer {
-        let strings: Vec<String> = value.iter().map(|v| v.to_string()).collect();
-        serializer.serialize_str(&strings.join(","))
-    }
-
-    pub fn deserialize<'de, D>(deserializer: D) -> Result<Vec<MultiaddrRange>, D::Error>
-    where D: Deserializer<'de> {
-        let strings: Vec<String> = Vec::deserialize(deserializer)?;
-        strings
-            .into_iter()
-            .map(|item| MultiaddrRange::from_str(&item).map_err(D::Error::custom))
-            .collect()
-    }
-}
-
 #[cfg(test)]
 mod test {
     use std::{

integration_tests/src/ffi/comms_config.rs

@@ -49,6 +49,7 @@ impl CommsConfig {
                 CString::new(base_dir).unwrap().into_raw(),
                 30,
                 600,
+                false, // This needs to be 'false' for the tests to pass
                 &mut error,
             );
             if error > 0 {

integration_tests/src/ffi/ffi_import.rs

@@ -367,6 +367,7 @@ extern "C" {
         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;
     pub fn comms_config_destroy(wc: *mut TariCommsConfig);