diff --git a/common/client-core/src/cli_helpers/client_add_gateway.rs b/common/client-core/src/cli_helpers/client_add_gateway.rs index 5662a25a3d..dc4280ba29 100644 --- a/common/client-core/src/cli_helpers/client_add_gateway.rs +++ b/common/client-core/src/cli_helpers/client_add_gateway.rs @@ -114,13 +114,12 @@ where })?; hardcoded_topology.entry_capable_nodes().cloned().collect() } else { - let mut rng = rand::thread_rng(); crate::init::helpers::gateways_for_init( - &mut rng, &core.client.nym_api_urls, user_agent, core.debug.topology.minimum_gateway_performance, core.debug.topology.ignore_ingress_epoch_role, + None, ) .await? }; diff --git a/common/client-core/src/cli_helpers/client_init.rs b/common/client-core/src/cli_helpers/client_init.rs index a59d73e261..feda3ab8d1 100644 --- a/common/client-core/src/cli_helpers/client_init.rs +++ b/common/client-core/src/cli_helpers/client_init.rs @@ -173,13 +173,12 @@ where })?; hardcoded_topology.entry_capable_nodes().cloned().collect() } else { - let mut rng = rand::thread_rng(); crate::init::helpers::gateways_for_init( - &mut rng, &core.client.nym_api_urls, user_agent, core.debug.topology.minimum_gateway_performance, core.debug.topology.ignore_ingress_epoch_role, + None, ) .await? }; diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index f4e34f5ffd..e574466701 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -45,6 +45,7 @@ type WsConn = JSWebsocket; const CONCURRENT_GATEWAYS_MEASURED: usize = 20; const MEASUREMENTS: usize = 3; +const DEFAULT_NYM_API_RETRIES: usize = 3; #[cfg(not(target_arch = "wasm32"))] const CONN_TIMEOUT: Duration = Duration::from_millis(1500); @@ -132,25 +133,27 @@ impl<'a, G: ConnectableGateway> GatewayWithLatency<'a, G> { } } -pub async fn gateways_for_init( - rng: &mut R, +pub async fn gateways_for_init( nym_apis: &[Url], user_agent: Option, minimum_performance: u8, ignore_epoch_roles: bool, + retry_count: Option, ) -> Result, ClientCoreError> { - let nym_api = nym_apis - .choose(rng) - .ok_or(ClientCoreError::ListOfNymApisIsEmpty)?; + // Build client with ALL URLs for fallback support + let nym_api_urls: Vec = nym_apis + .iter() + .map(|url| nym_http_api_client::Url::from(url.clone())) + .collect(); - // Use the unified HTTP client directly with optional user agent - let mut builder = nym_http_api_client::Client::builder(nym_api.clone()) - .map_err(|e| { - ClientCoreError::ValidatorClientError(nym_validator_client::ValidatorClientError::from( - e, - )) - })? - .with_bincode(); // Use bincode for better performance + if nym_api_urls.is_empty() { + return Err(ClientCoreError::ListOfNymApisIsEmpty); + } + + let retry_count = retry_count.unwrap_or(DEFAULT_NYM_API_RETRIES); + let mut builder = nym_http_api_client::ClientBuilder::new_with_urls(nym_api_urls.clone()) + .with_retries(retry_count) + .with_bincode(); if let Some(user_agent) = user_agent { builder = builder.with_user_agent(user_agent); @@ -160,7 +163,7 @@ pub async fn gateways_for_init( ClientCoreError::ValidatorClientError(nym_validator_client::ValidatorClientError::from(e)) })?; - tracing::debug!("Fetching list of gateways from: {nym_api}"); + tracing::debug!("Fetching list of gateways from: {:?}", nym_api_urls); // Use our helper to handle pagination let gateways = get_all_basic_entry_nodes_with_metadata(&client, true) @@ -172,17 +175,15 @@ pub async fn gateways_for_init( // filter out gateways below minimum performance and ones that could operate as a mixnode // (we don't want instability) - let valid_gateways = gateways + let valid_gateways: Vec = gateways .iter() .filter(|g| ignore_epoch_roles || !g.supported_roles.mixnode) .filter(|g| g.performance.round_to_integer() >= minimum_performance) .filter_map(|gateway| gateway.try_into().ok()) - .collect::>(); - tracing::debug!("After checking validity: {}", valid_gateways.len()); - tracing::trace!("Valid gateways: {valid_gateways:#?}"); + .collect(); tracing::info!( - "and {} after validity and performance filtering", + "Found {} valid gateways after filtering", valid_gateways.len() ); @@ -345,13 +346,20 @@ pub(super) fn get_specified_gateway( must_use_tls: bool, ) -> Result { tracing::debug!("Requesting specified gateway: {gateway_identity}"); + let user_gateway = ed25519::PublicKey::from_base58_string(gateway_identity) .map_err(ClientCoreError::UnableToCreatePublicKeyFromGatewayId)?; let gateway = gateways .iter() .find(|gateway| gateway.identity_key == user_gateway) - .ok_or_else(|| ClientCoreError::NoGatewayWithId(gateway_identity.to_string()))?; + .ok_or_else(|| { + tracing::debug!( + "Gateway {gateway_identity} not found in {} available gateways", + gateways.len() + ); + ClientCoreError::NoGatewayWithId(gateway_identity.to_string()) + })?; let Some(entry_details) = gateway.entry.as_ref() else { return Err(ClientCoreError::UnsupportedEntry { @@ -414,3 +422,52 @@ pub(super) async fn register_with_gateway( authenticated_ephemeral_client: gateway_client, }) } + +#[cfg(test)] +mod tests { + use url::Url; + + #[test] + fn test_single_url_builds_without_retries() { + let urls = [Url::parse("https://api.nym.com").unwrap()]; + + let nym_api_urls: Vec = urls + .iter() + .map(|url| nym_http_api_client::Url::from(url.clone())) + .collect(); + + assert_eq!(nym_api_urls.len(), 1, "Should have exactly one URL"); + } + + #[test] + fn test_multiple_urls_prepared_for_retries() { + let urls = vec![ + Url::parse("https://api1.nym.com").unwrap(), + Url::parse("https://api2.nym.com").unwrap(), + Url::parse("https://api3.nym.com").unwrap(), + ]; + + let nym_api_urls: Vec = urls + .iter() + .map(|url| nym_http_api_client::Url::from(url.clone())) + .collect(); + + assert_eq!(nym_api_urls.len(), 3, "Should have all three URLs"); + assert!( + nym_api_urls.len() > 1, + "Multiple URLs trigger retry behavior" + ); + } + + #[test] + fn test_empty_url_list_is_detected() { + let urls: Vec = vec![]; + + let nym_api_urls: Vec = urls + .iter() + .map(|url| nym_http_api_client::Url::from(url.clone())) + .collect(); + + assert!(nym_api_urls.is_empty(), "Empty list should remain empty"); + } +} diff --git a/common/wasm/client-core/src/helpers.rs b/common/wasm/client-core/src/helpers.rs index 23aaee4cad..de5cd6883a 100644 --- a/common/wasm/client-core/src/helpers.rs +++ b/common/wasm/client-core/src/helpers.rs @@ -160,13 +160,12 @@ pub async fn setup_gateway_from_api( minimum_performance: u8, ignore_epoch_roles: bool, ) -> Result { - let mut rng = thread_rng(); let gateways = gateways_for_init( - &mut rng, nym_apis, None, minimum_performance, ignore_epoch_roles, + None, ) .await?; setup_gateway_wasm(client_store, force_tls, chosen_gateway, gateways).await @@ -178,13 +177,12 @@ pub async fn current_gateways_wasm( minimum_performance: u8, ignore_epoch_roles: bool, ) -> Result, ClientCoreError> { - let mut rng = thread_rng(); gateways_for_init( - &mut rng, nym_apis, user_agent, minimum_performance, ignore_epoch_roles, + None, ) .await } diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index 349534d555..d1a342faa9 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -56,6 +56,53 @@ pub struct MixnetClientConfig { } impl BuilderConfig { + /// Creates a new BuilderConfig with all required parameters. + /// + /// However, consider using `BuilderConfig::builder()` instead. + #[allow(clippy::too_many_arguments)] + pub fn new( + entry_node: NymNodeWithKeys, + exit_node: NymNodeWithKeys, + data_path: Option, + mixnet_client_config: MixnetClientConfig, + two_hops: bool, + user_agent: UserAgent, + custom_topology_provider: Box, + network_env: NymNetworkDetails, + cancel_token: CancellationToken, + #[cfg(unix)] connection_fd_callback: Arc, + ) -> Self { + Self { + entry_node, + exit_node, + data_path, + mixnet_client_config, + two_hops, + user_agent, + custom_topology_provider, + network_env, + cancel_token, + #[cfg(unix)] + connection_fd_callback, + } + } + + /// Creates a builder for BuilderConfig + /// + /// This is the preferred way to construct a BuilderConfig. + /// + /// # Example + /// ```ignore + /// let config = BuilderConfig::builder() + /// .entry_node(entry) + /// .exit_node(exit) + /// .user_agent(agent) + /// .build()?; + /// ``` + pub fn builder() -> BuilderConfigBuilder { + BuilderConfigBuilder::default() + } + pub fn mixnet_client_debug_config(&self) -> DebugConfig { if self.two_hops { two_hop_debug_config(&self.mixnet_client_config) @@ -116,6 +163,7 @@ impl BuilderConfig { .credentials_mode(true) .with_remember_me(remember_me) .custom_topology_provider(self.custom_topology_provider); + #[cfg(unix)] let builder = builder.with_connection_fd_callback(self.connection_fd_callback); @@ -205,3 +253,205 @@ fn log_mixnet_client_config(debug_config: &DebugConfig) { fn true_to_disabled(val: bool) -> &'static str { if val { "disabled" } else { "enabled" } } + +/// Error type for BuilderConfig validation +#[derive(Debug, Clone, thiserror::Error)] +#[allow(clippy::enum_variant_names)] +pub enum BuilderConfigError { + #[error("entry_node is required")] + MissingEntryNode, + #[error("exit_node is required")] + MissingExitNode, + #[error("mixnet_client_config is required")] + MissingMixnetClientConfig, + #[error("user_agent is required")] + MissingUserAgent, + #[error("custom_topology_provider is required")] + MissingTopologyProvider, + #[error("network_env is required")] + MissingNetworkEnv, + #[error("cancel_token is required")] + MissingCancelToken, + #[cfg(unix)] + #[error("connection_fd_callback is required")] + MissingConnectionFdCallback, +} + +/// Builder for `BuilderConfig` +/// +/// This provides a more convenient way to construct a `BuilderConfig` compared to the +/// `new()` constructor with many arguments. +#[derive(Default)] +pub struct BuilderConfigBuilder { + entry_node: Option, + exit_node: Option, + data_path: Option, + mixnet_client_config: Option, + two_hops: bool, + user_agent: Option, + custom_topology_provider: Option>, + network_env: Option, + cancel_token: Option, + #[cfg(unix)] + connection_fd_callback: Option>, +} + +impl BuilderConfigBuilder { + pub fn new() -> Self { + Self::default() + } + + pub fn entry_node(mut self, entry_node: NymNodeWithKeys) -> Self { + self.entry_node = Some(entry_node); + self + } + + pub fn exit_node(mut self, exit_node: NymNodeWithKeys) -> Self { + self.exit_node = Some(exit_node); + self + } + + pub fn data_path(mut self, data_path: Option) -> Self { + self.data_path = data_path; + self + } + + pub fn mixnet_client_config(mut self, mixnet_client_config: MixnetClientConfig) -> Self { + self.mixnet_client_config = Some(mixnet_client_config); + self + } + + pub fn two_hops(mut self, two_hops: bool) -> Self { + self.two_hops = two_hops; + self + } + + pub fn user_agent(mut self, user_agent: UserAgent) -> Self { + self.user_agent = Some(user_agent); + self + } + + pub fn custom_topology_provider( + mut self, + custom_topology_provider: Box, + ) -> Self { + self.custom_topology_provider = Some(custom_topology_provider); + self + } + + pub fn network_env(mut self, network_env: NymNetworkDetails) -> Self { + self.network_env = Some(network_env); + self + } + + pub fn cancel_token(mut self, cancel_token: CancellationToken) -> Self { + self.cancel_token = Some(cancel_token); + self + } + + #[cfg(unix)] + pub fn connection_fd_callback( + mut self, + connection_fd_callback: Arc, + ) -> Self { + self.connection_fd_callback = Some(connection_fd_callback); + self + } + + /// Builds the `BuilderConfig`. + /// + /// Returns an error if any required field is missing. + pub fn build(self) -> Result { + Ok(BuilderConfig { + entry_node: self + .entry_node + .ok_or(BuilderConfigError::MissingEntryNode)?, + exit_node: self.exit_node.ok_or(BuilderConfigError::MissingExitNode)?, + data_path: self.data_path, + mixnet_client_config: self + .mixnet_client_config + .ok_or(BuilderConfigError::MissingMixnetClientConfig)?, + two_hops: self.two_hops, + user_agent: self + .user_agent + .ok_or(BuilderConfigError::MissingUserAgent)?, + custom_topology_provider: self + .custom_topology_provider + .ok_or(BuilderConfigError::MissingTopologyProvider)?, + network_env: self + .network_env + .ok_or(BuilderConfigError::MissingNetworkEnv)?, + cancel_token: self + .cancel_token + .ok_or(BuilderConfigError::MissingCancelToken)?, + #[cfg(unix)] + connection_fd_callback: self + .connection_fd_callback + .ok_or(BuilderConfigError::MissingConnectionFdCallback)?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_mixnet_client_config_default_values() { + let config = MixnetClientConfig::default(); + assert!(!config.disable_poisson_rate); + assert!(!config.disable_background_cover_traffic); + assert_eq!(config.min_mixnode_performance, None); + assert_eq!(config.min_gateway_performance, None); + } + + #[test] + fn test_builder_config_builder_fails_without_required_fields() { + // Building without any fields should fail with specific error + let result = BuilderConfig::builder().build(); + assert!(result.is_err()); + match result { + Err(BuilderConfigError::MissingEntryNode) => (), // Expected + Err(e) => panic!("Expected MissingEntryNode, got: {}", e), + Ok(_) => panic!("Expected error, got Ok"), + } + } + + #[test] + fn test_builder_config_builder_validates_all_required_fields() { + // Test that each required field is validated + let result = BuilderConfig::builder().build(); + assert!(result.is_err()); + + // Short-circuits at first missing field, so we just verify it's one of the expected errors + #[allow(unreachable_patterns)] // All variants are covered, but keeping catch-all for safety + match result { + Err(BuilderConfigError::MissingEntryNode) + | Err(BuilderConfigError::MissingExitNode) + | Err(BuilderConfigError::MissingMixnetClientConfig) + | Err(BuilderConfigError::MissingUserAgent) + | Err(BuilderConfigError::MissingTopologyProvider) + | Err(BuilderConfigError::MissingNetworkEnv) + | Err(BuilderConfigError::MissingCancelToken) => (), + #[cfg(unix)] + Err(BuilderConfigError::MissingConnectionFdCallback) => (), + Err(e) => panic!("Unexpected error: {}", e), + Ok(_) => panic!("Expected validation error, got Ok"), + } + } + + #[test] + fn test_builder_config_builder_method_chaining() { + // Test that builder methods chain properly and return Self + let builder = BuilderConfig::builder(); + + // Verify the builder returns itself for chaining + let builder = builder.two_hops(true); + let builder = builder.two_hops(false); + let builder = builder.data_path(None); + + // Builder should still fail because required fields are missing + let result = builder.build(); + assert!(result.is_err()); + } +} diff --git a/sdk/rust/nym-sdk/src/mixnet/client.rs b/sdk/rust/nym-sdk/src/mixnet/client.rs index d24a918989..258d2f3c37 100644 --- a/sdk/rust/nym-sdk/src/mixnet/client.rs +++ b/sdk/rust/nym-sdk/src/mixnet/client.rs @@ -555,21 +555,22 @@ where async fn available_gateways(&mut self) -> Result, ClientCoreError> { if let Some(ref mut custom_provider) = self.custom_topology_provider { if let Some(topology) = custom_provider.get_new_topology().await { - return Ok(topology.entry_gateways().cloned().collect()); + // Use entry_capable_nodes() instead of entry_gateways() to include + // all entry-capable nodes, not just actively assigned ones + return Ok(topology.entry_capable_nodes().cloned().collect()); } } let nym_api_endpoints = self.get_api_endpoints(); let topology_cfg = &self.config.debug_config.topology; let user_agent = self.user_agent.clone(); - let mut rng = OsRng; gateways_for_init( - &mut rng, &nym_api_endpoints, user_agent, topology_cfg.minimum_gateway_performance, topology_cfg.ignore_ingress_epoch_role, + None, ) .await } @@ -880,3 +881,17 @@ impl IncludedSurbs { Self::ExposeSelfAddress } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_mixnet_builder_default_no_custom_client() { + let builder = MixnetClientBuilder::new_ephemeral(); + assert!( + builder.build().is_ok(), + "Builder should succeed without custom client" + ); + } +}