From edcf2b1204fccaebef57c32029ee22e258308b9f Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Thu, 16 Oct 2025 16:22:57 +0200 Subject: [PATCH 01/16] enable URL rotation and retries for mixnet gateway init --- common/client-core/src/init/helpers.rs | 78 +++++++++++++++++++++++--- 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index f4e34f5ffd..bcb1b42f8b 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -133,24 +133,34 @@ impl<'a, G: ConnectableGateway> GatewayWithLatency<'a, G> { } pub async fn gateways_for_init( - rng: &mut R, + _rng: &mut R, nym_apis: &[Url], user_agent: Option, minimum_performance: u8, ignore_epoch_roles: bool, ) -> 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() + .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .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| { + if nym_api_urls.is_empty() { + return Err(ClientCoreError::ListOfNymApisIsEmpty); + } + + let mut builder = if nym_api_urls.len() == 1 { + nym_http_api_client::Client::builder(nym_api_urls[0].clone()).map_err(|e| { ClientCoreError::ValidatorClientError(nym_validator_client::ValidatorClientError::from( e, )) })? - .with_bincode(); // Use bincode for better performance + } else { + nym_http_api_client::ClientBuilder::new_with_urls(nym_api_urls.clone()).with_retries(3) + // Enable URL rotation on failure + }; + + builder = builder.with_bincode(); if let Some(user_agent) = user_agent { builder = builder.with_user_agent(user_agent); @@ -160,7 +170,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) @@ -414,3 +424,53 @@ pub(super) async fn register_with_gateway( authenticated_ephemeral_client: gateway_client, }) } + +#[cfg(test)] +mod tests { + use super::*; + use url::Url; + + #[test] + fn test_single_url_builds_without_retries() { + let urls = vec![Url::parse("https://api.nym.com").unwrap()]; + + let nym_api_urls: Vec = urls + .iter() + .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .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() + .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .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() + .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .collect(); + + assert!(nym_api_urls.is_empty(), "Empty list should remain empty"); + } +} From cd61f930bf43ba30573257f33a32c9c041ff6f73 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Fri, 17 Oct 2025 08:36:23 +0200 Subject: [PATCH 02/16] feat: pass custom HTTP client through SDK stack for domain fronting - Add with_nym_api_client() to BaseClientBuilder, MixnetClientBuilder, and RegistrationClientBuilderConfig - Modify nym_api_provider to fetch all nodes then filter by supported_roles.entry (fixes metadata inconsistency) - Update helpers.rs to build HTTP client with all nym_apis URLs and retries for fallback support - Fix SDK to use entry_capable_nodes() instead of entry_gateways() for broader gateway selection This enables domain fronting and URL rotation throughout the entire SDK stack, improving censorship resistance and connection reliability. All changes are backward compatible - custom client is optional. --- .../client-core/src/client/base_client/mod.rs | 24 ++- .../topology_control/nym_api_provider.rs | 67 +++++-- common/client-core/src/init/helpers.rs | 188 +++++++++++++++++- nym-registration-client/Cargo.toml | 1 + nym-registration-client/src/builder/config.rs | 81 +++++++- sdk/rust/nym-sdk/src/mixnet/client.rs | 99 ++++++++- 6 files changed, 426 insertions(+), 34 deletions(-) diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index a093977bbb..868f281e6c 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -199,6 +199,7 @@ pub struct BaseClientBuilder { custom_gateway_transceiver: Option>, shutdown: Option, user_agent: Option, + custom_nym_api_client: Option, setup_method: GatewaySetup, @@ -227,6 +228,7 @@ where custom_gateway_transceiver: None, shutdown: None, user_agent: None, + custom_nym_api_client: None, setup_method: GatewaySetup::MustLoad { gateway_id: None }, #[cfg(unix)] connection_fd_callback: None, @@ -234,6 +236,12 @@ where } } + #[must_use] + pub fn with_nym_api_client(mut self, client: nym_http_api_client::Client) -> Self { + self.custom_nym_api_client = Some(client); + self + } + #[must_use] pub fn with_derivation_material( mut self, @@ -820,7 +828,17 @@ where fn construct_nym_api_client( config: &Config, user_agent: Option, + custom_client: Option, ) -> Result { + // If a custom client was provided (e.g., with domain fronting support), use it + if let Some(client) = custom_client { + tracing::info!("Using CUSTOM nym-api HTTP client (with domain fronting support)"); + return Ok(client); + } + + tracing::warn!("No custom HTTP client provided - creating DEFAULT client from config"); + + // Otherwise, create a basic client let mut nym_api_urls = config.get_nym_api_endpoints(); nym_api_urls.shuffle(&mut thread_rng()); @@ -911,7 +929,11 @@ where .dkg_query_client .map(|client| BandwidthController::new(credential_store, client)); - let nym_api_client = Self::construct_nym_api_client(&self.config, self.user_agent.clone())?; + let nym_api_client = Self::construct_nym_api_client( + &self.config, + self.user_agent.clone(), + self.custom_nym_api_client, + )?; let key_rotation_config = Self::determine_key_rotation_state(&nym_api_client).await?; let topology_provider = Self::setup_topology_provider( diff --git a/common/client-core/src/client/topology_control/nym_api_provider.rs b/common/client-core/src/client/topology_control/nym_api_provider.rs index a81118665c..b4dbfebb19 100644 --- a/common/client-core/src/client/topology_control/nym_api_provider.rs +++ b/common/client-core/src/client/topology_control/nym_api_provider.rs @@ -6,6 +6,7 @@ use nym_mixnet_contract_common::EpochRewardedSet; use nym_topology::provider_trait::{ToTopologyMetadata, TopologyProvider}; use nym_topology::NymTopology; use nym_validator_client::nym_api::NymApiClientExt; +use nym_validator_client::nym_nodes::NodeRole; use rand::prelude::SliceRandom; use rand::thread_rng; use std::cmp::min; @@ -135,31 +136,31 @@ impl NymApiTopologyProvider { } else { // if we're not using extended topology, we're only getting active set mixnodes and gateways - let mixnodes_fut = self - .validator_client - .get_all_basic_active_mixing_assigned_nodes_with_metadata(); + // Get ALL bonded nodes and filter for both mixnodes and gateways + // This ensures consistent metadata between both calls + let all_nodes_fut = self.validator_client.get_all_basic_nodes_with_metadata(); - // TODO: we really should be getting ACTIVE gateways only - let gateways_fut = self - .validator_client - .get_all_basic_entry_assigned_nodes_with_metadata(); + let (rewarded_set, all_nodes_res) = futures::try_join!(rewarded_set_fut, all_nodes_fut) + .inspect_err(|err| { + error!("failed to get network nodes: {err}"); + }) + .ok()?; - let (rewarded_set, mixnodes_res, gateways_res) = - futures::try_join!(rewarded_set_fut, mixnodes_fut, gateways_fut) - .inspect_err(|err| { - error!("failed to get network nodes: {err}"); - }) - .ok()?; + let metadata = all_nodes_res.metadata; + let all_nodes = all_nodes_res.nodes; - let metadata = mixnodes_res.metadata; - let mixnodes = mixnodes_res.nodes; + // Filter for active mixing nodes (role=Mixnode) + let mixnodes: Vec<_> = all_nodes + .iter() + .filter(|n| n.supported_roles.mixnode && matches!(n.role, NodeRole::Mixnode { .. })) + .cloned() + .collect(); - if !gateways_res.metadata.consistency_check(&metadata) { - warn!("inconsistent nodes metadata between mixnodes and gateways calls! {metadata:?} and {:?}", gateways_res.metadata); - return None; - } - - let gateways = gateways_res.nodes; + // Filter for entry-capable nodes (supported_roles.entry == true) + let gateways: Vec<_> = all_nodes + .into_iter() + .filter(|n| n.supported_roles.entry) + .collect(); debug!( "there are {} mixnodes and {} gateways in total (before performance filtering)", @@ -221,3 +222,27 @@ impl TopologyProvider for NymApiTopologyProvider { Some(topology) } } + +#[cfg(test)] +mod tests { + #[test] + fn test_filtering_logic() { + // This test verifies the filtering logic used in get_current_compatible_topology + // We test that the role matching pattern works correctly + use nym_validator_client::nym_nodes::NodeRole; + + // Test that the pattern matching for mixnodes works + let mixnode_role = NodeRole::Mixnode { layer: 1 }; + assert!( + matches!(mixnode_role, NodeRole::Mixnode { .. }), + "Should match mixnode role pattern" + ); + + // Test that other roles don't match + let entry_role = NodeRole::EntryGateway; + assert!( + !matches!(entry_role, NodeRole::Mixnode { .. }), + "Entry gateway should not match mixnode pattern" + ); + } +} diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index bcb1b42f8b..6ebd9d56a0 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -89,16 +89,22 @@ async fn get_all_basic_entry_nodes_with_metadata( client: &nym_http_api_client::Client, use_bincode: bool, ) -> Result { - // Get first page to obtain metadata + // Get ALL nodes (not just entry-assigned) because in some environments (like sandbox), + // nodes may be capable of entry gateway role but not currently assigned to it let mut page = 0; let res = client - .get_basic_entry_assigned_nodes_v2(false, Some(page), None, use_bincode) + .get_basic_nodes_v2(false, Some(page), None, use_bincode) .await?; let mut nodes = res.nodes.data; let metadata = res.metadata; if res.nodes.pagination.total == nodes.len() { - return Ok(SkimmedNodesWithMetadata::new(nodes, metadata)); + // Filter for entry-capable nodes (nodes with supported_roles.entry == true) + let entry_nodes: Vec<_> = nodes + .into_iter() + .filter(|n| n.supported_roles.entry) + .collect(); + return Ok(SkimmedNodesWithMetadata::new(entry_nodes, metadata)); } page += 1; @@ -106,7 +112,7 @@ async fn get_all_basic_entry_nodes_with_metadata( // Collect remaining pages loop { let mut res = client - .get_basic_entry_assigned_nodes_v2(false, Some(page), None, use_bincode) + .get_basic_nodes_v2(false, Some(page), None, use_bincode) .await?; if !metadata.consistency_check(&res.metadata) { @@ -123,7 +129,13 @@ async fn get_all_basic_entry_nodes_with_metadata( } } - Ok(SkimmedNodesWithMetadata::new(nodes, metadata)) + // Filter for entry-capable nodes (nodes with supported_roles.entry == true) + let entry_nodes: Vec<_> = nodes + .into_iter() + .filter(|n| n.supported_roles.entry) + .collect(); + + Ok(SkimmedNodesWithMetadata::new(entry_nodes, metadata)) } impl<'a, G: ConnectableGateway> GatewayWithLatency<'a, G> { @@ -182,13 +194,41 @@ 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 total_fetched = gateways.len(); + + let after_role_filter_count = gateways + .iter() + .filter(|g| ignore_epoch_roles || !g.supported_roles.mixnode) + .count(); + tracing::debug!( + "After role filter: {} (removed {} mixnode-capable)", + after_role_filter_count, + total_fetched - after_role_filter_count + ); + + let after_performance_filter_count = gateways + .iter() + .filter(|g| ignore_epoch_roles || !g.supported_roles.mixnode) + .filter(|g| g.performance.round_to_integer() >= minimum_performance) + .count(); + tracing::debug!( + "After performance filter (>= {}%): {} (removed {} low-performance)", + minimum_performance, + after_performance_filter_count, + after_role_filter_count - after_performance_filter_count + ); + + 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()); + .collect(); + tracing::debug!( + "After conversion to RoutingNode: {} (removed {} invalid)", + valid_gateways.len(), + after_performance_filter_count - valid_gateways.len() + ); tracing::trace!("Valid gateways: {valid_gateways:#?}"); tracing::info!( @@ -196,6 +236,14 @@ pub async fn gateways_for_init( valid_gateways.len() ); + for gw in &valid_gateways { + tracing::debug!( + "Available gateway: {} (node_id: {})", + gw.identity_key.to_base58_string(), + gw.node_id + ); + } + Ok(valid_gateways) } @@ -355,13 +403,25 @@ pub(super) fn get_specified_gateway( must_use_tls: bool, ) -> Result { tracing::debug!("Requesting specified gateway: {gateway_identity}"); + tracing::debug!("Searching in {} available gateways", gateways.len()); + for gw in gateways { + tracing::debug!( + "Gateway in list: {} (node_id: {})", + gw.identity_key.to_base58_string(), + gw.node_id + ); + } + 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::error!("Gateway {gateway_identity} NOT FOUND in available gateways list!"); + ClientCoreError::NoGatewayWithId(gateway_identity.to_string()) + })?; let Some(entry_details) = gateway.entry.as_ref() else { return Err(ClientCoreError::UnsupportedEntry { @@ -427,7 +487,6 @@ pub(super) async fn register_with_gateway( #[cfg(test)] mod tests { - use super::*; use url::Url; #[test] @@ -473,4 +532,113 @@ mod tests { assert!(nym_api_urls.is_empty(), "Empty list should remain empty"); } + + #[test] + fn test_gateway_filtering_logic() { + // NOTE: This test validates the filtering logic in isolation. + // It does NOT test the actual implementation in get_all_basic_entry_nodes_with_metadata + // or gateways_for_init (which would require mocking the HTTP client). + // The real proof is building and running the daemon. + // + // Test the core filtering logic used in gateways_for_init: + // 1. Filter by supported_roles.entry (not by epoch role assignment) + // 2. Filter by performance + // + // This test verifies the fix where nodes have role=Inactive + // but supported_roles.entry=true + + #[derive(Debug)] + struct TestNode { + id: u32, + entry_capable: bool, // supported_roles.entry + mixnode_capable: bool, // supported_roles.mixnode + performance: u8, // 0-100 + } + + let nodes = vec![ + // Node 53: entry-capable, good performance + TestNode { + id: 53, + entry_capable: true, + mixnode_capable: false, + performance: 100, + }, + // Node 97: entry-capable (but role=Inactive in sandbox), good performance + TestNode { + id: 97, + entry_capable: true, + mixnode_capable: false, + performance: 100, + }, + // Node 75: NOT entry-capable (mixnode only) + TestNode { + id: 75, + entry_capable: false, + mixnode_capable: true, + performance: 100, + }, + // Node 99: entry-capable but low performance + TestNode { + id: 99, + entry_capable: true, + mixnode_capable: false, + performance: 0, + }, + ]; + + let minimum_performance = 50; + let ignore_epoch_roles = true; + + // Step 1: Filter by supported_roles.entry (this is what the fix enables) + let entry_capable: Vec<_> = nodes.iter().filter(|n| n.entry_capable).collect(); + + assert_eq!( + entry_capable.len(), + 3, + "Should have 3 entry-capable nodes (53, 97, 99) - this includes Inactive nodes!" + ); + + // Step 2: Filter by role (exclude mixnode-capable if not ignoring epoch roles) + let after_role_filter: Vec<_> = entry_capable + .iter() + .filter(|g| ignore_epoch_roles || !g.mixnode_capable) + .collect(); + + assert_eq!( + after_role_filter.len(), + 3, + "All entry-capable nodes pass role filter with ignore_epoch_roles=true" + ); + + // Step 3: Filter by performance + let after_performance_filter: Vec<_> = after_role_filter + .iter() + .filter(|g| g.performance >= minimum_performance) + .collect(); + + assert_eq!( + after_performance_filter.len(), + 2, + "Should have 2 nodes after performance filter (53 and 97, excluding 99 with 0%)" + ); + + // Verify the correct nodes made it through + let node_ids: Vec = after_performance_filter.iter().map(|n| n.id).collect(); + assert!( + node_ids.contains(&53), + "Node 53 (actively assigned entry gateway) should be included" + ); + assert!( + node_ids.contains(&97), + "Node 97 (Inactive but entry-capable) should be included - THIS IS THE FIX!" + ); + assert!( + !node_ids.contains(&75), + "Node 75 (mixnode-only, not entry-capable) should be excluded" + ); + assert!( + !node_ids.contains(&99), + "Node 99 (low performance) should be excluded" + ); + } } diff --git a/nym-registration-client/Cargo.toml b/nym-registration-client/Cargo.toml index faa96b86cc..6dc3335357 100644 --- a/nym-registration-client/Cargo.toml +++ b/nym-registration-client/Cargo.toml @@ -22,6 +22,7 @@ nym-authenticator-client = { path = "../nym-authenticator-client" } nym-bandwidth-controller = { path = "../common/bandwidth-controller" } nym-credential-storage = { path = "../common/credential-storage" } nym-credentials-interface = { path = "../common/credentials-interface" } +nym-http-api-client = { path = "../common/http-api-client" } nym-ip-packet-client = { path = "../nym-ip-packet-client" } nym-registration-common = { path = "../common/registration" } nym-sdk = { path = "../sdk/rust/nym-sdk" } diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index 494e3fb2a1..526934277c 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -34,6 +34,7 @@ pub struct BuilderConfig { pub two_hops: bool, pub user_agent: UserAgent, pub custom_topology_provider: Box, + pub custom_nym_api_client: Option, pub network_env: NymNetworkDetails, pub cancel_token: CancellationToken, #[cfg(unix)] @@ -56,6 +57,44 @@ pub struct MixnetClientConfig { } impl BuilderConfig { + /// Create a new BuilderConfig without domain fronting support + /// + /// For domain fronting support, set `custom_nym_api_client` to Some(client) after creation + #[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, + custom_nym_api_client: None, + network_env, + cancel_token, + #[cfg(unix)] + connection_fd_callback, + } + } + + /// Set a custom nym-api HTTP client (for domain fronting support) + pub fn with_nym_api_client(mut self, client: nym_http_api_client::Client) -> Self { + self.custom_nym_api_client = Some(client); + self + } + pub fn mixnet_client_debug_config(&self) -> DebugConfig { if self.two_hops { two_hop_debug_config(&self.mixnet_client_config) @@ -108,7 +147,7 @@ impl BuilderConfig { RememberMe::new_mixnet() }; - let builder = builder + let mut builder = builder .with_user_agent(self.user_agent) .request_gateway(self.entry_node.node.identity.to_string()) .network_details(self.network_env) @@ -117,6 +156,13 @@ impl BuilderConfig { .with_remember_me(remember_me) .custom_topology_provider(self.custom_topology_provider); + if let Some(nym_api_client) = self.custom_nym_api_client { + tracing::info!("Registration client: Passing custom nym-api HTTP client to SDK"); + builder = builder.with_nym_api_client(nym_api_client); + } else { + tracing::warn!("Registration client: No custom nym-api HTTP client provided"); + } + #[cfg(unix)] let builder = builder.with_connection_fd_callback(self.connection_fd_callback); @@ -206,3 +252,36 @@ fn log_mixnet_client_config(debug_config: &DebugConfig) { fn true_to_disabled(val: bool) -> &'static str { if val { "disabled" } else { "enabled" } } + +#[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_has_custom_client_field() { + // Verify that BuilderConfig has the custom_nym_api_client field + // by creating a simple HTTP client + let http_client = nym_http_api_client::Client::builder( + nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), + ) + .expect("Failed to create client builder") + .build() + .expect("Failed to build client"); + + // Verify the client works + let urls = http_client.base_urls(); + assert!( + !urls.is_empty() && urls[0].as_str().contains("validator.nymtech.net"), + "HTTP client should be configured with correct URL" + ); + } +} diff --git a/sdk/rust/nym-sdk/src/mixnet/client.rs b/sdk/rust/nym-sdk/src/mixnet/client.rs index 5729b40393..44663142ec 100644 --- a/sdk/rust/nym-sdk/src/mixnet/client.rs +++ b/sdk/rust/nym-sdk/src/mixnet/client.rs @@ -54,6 +54,7 @@ pub struct MixnetClientBuilder { custom_topology_provider: Option>, custom_gateway_transceiver: Option>, custom_shutdown: Option, + custom_nym_api_client: Option, force_tls: bool, user_agent: Option, #[cfg(unix)] @@ -92,6 +93,7 @@ impl MixnetClientBuilder { socks5_config: None, wait_for_gateway: false, custom_topology_provider: None, + custom_nym_api_client: None, storage: storage_paths .initialise_default_persistent_storage() .await?, @@ -129,6 +131,7 @@ where wait_for_gateway: false, custom_topology_provider: None, custom_gateway_transceiver: None, + custom_nym_api_client: None, custom_shutdown: None, force_tls: false, user_agent: None, @@ -152,6 +155,7 @@ where wait_for_gateway: self.wait_for_gateway, custom_topology_provider: self.custom_topology_provider, custom_gateway_transceiver: self.custom_gateway_transceiver, + custom_nym_api_client: self.custom_nym_api_client, custom_shutdown: self.custom_shutdown, force_tls: self.force_tls, user_agent: self.user_agent, @@ -283,6 +287,12 @@ where self } + #[must_use] + pub fn with_nym_api_client(mut self, client: nym_http_api_client::Client) -> Self { + self.custom_nym_api_client = Some(client); + self + } + #[must_use] pub fn with_statistics_reporting(mut self, config: StatsReporting) -> Self { self.config.debug_config.stats_reporting = config; @@ -323,6 +333,7 @@ where client.custom_gateway_transceiver = self.custom_gateway_transceiver; client.custom_topology_provider = self.custom_topology_provider; + client.custom_nym_api_client = self.custom_nym_api_client; client.custom_shutdown = self.custom_shutdown; client.wait_for_gateway = self.wait_for_gateway; client.force_tls = self.force_tls; @@ -372,6 +383,9 @@ where /// advanced usage of custom gateways custom_gateway_transceiver: Option>, + /// Custom nym-api HTTP client (for domain fronting support) + custom_nym_api_client: Option, + /// Attempt to wait for the selected gateway (if applicable) to come online if its currently not bonded. wait_for_gateway: bool, @@ -440,6 +454,7 @@ where storage, custom_topology_provider: None, custom_gateway_transceiver: None, + custom_nym_api_client: None, wait_for_gateway: false, force_tls: false, custom_shutdown: None, @@ -535,7 +550,9 @@ 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()); } } @@ -696,6 +713,13 @@ where base_builder = base_builder.with_user_agent(user_agent); } + if let Some(nym_api_client) = self.custom_nym_api_client { + tracing::info!("SDK: Passing custom nym-api HTTP client to BaseClientBuilder"); + base_builder = base_builder.with_nym_api_client(nym_api_client); + } else { + tracing::warn!("SDK: No custom nym-api HTTP client configured"); + } + if let Some(topology_provider) = self.custom_topology_provider { base_builder = base_builder.with_topology_provider(topology_provider); } @@ -858,3 +882,76 @@ 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" + ); + } + + #[test] + fn test_mixnet_builder_with_custom_client() { + let http_client = nym_http_api_client::Client::builder( + nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), + ) + .expect("Failed to create client builder") + .build() + .expect("Failed to build client"); + + let builder = MixnetClientBuilder::new_ephemeral().with_nym_api_client(http_client); + + assert!( + builder.build().is_ok(), + "Builder should succeed with custom client" + ); + } + + // Note: Tests for entry_capable_nodes() vs entry_gateways() are in nym-topology crate + // These tests verify the builder functionality only + + #[test] + fn test_custom_client_transfer_through_build() { + let http_client = nym_http_api_client::Client::builder( + nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), + ) + .expect("Failed to create client builder") + .build() + .expect("Failed to build client"); + + let builder = MixnetClientBuilder::new_ephemeral().with_nym_api_client(http_client); + let disconnected_client = builder.build(); + + assert!( + disconnected_client.is_ok(), + "Build should transfer custom_nym_api_client successfully" + ); + } + + #[test] + fn test_builder_storage_transfer_includes_custom_client() { + use nym_client_core::client::base_client::storage::Ephemeral; + + let http_client = nym_http_api_client::Client::builder( + nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), + ) + .expect("Failed to create client builder") + .build() + .expect("Failed to build client"); + + let builder = MixnetClientBuilder::new_ephemeral().with_nym_api_client(http_client); + let new_storage = Ephemeral::default(); + let builder_with_new_storage = builder.set_storage(new_storage); + + assert!( + builder_with_new_storage.build().is_ok(), + "set_storage should preserve custom_nym_api_client" + ); + } +} From c9d4d62446e7811fe8c188edb473f4ccf1c5f528 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Fri, 17 Oct 2025 13:30:30 +0200 Subject: [PATCH 03/16] Fix clippy warnings: use arrays instead of vec! in tests --- common/client-core/src/init/helpers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index 6ebd9d56a0..bee127b911 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -491,7 +491,7 @@ mod tests { #[test] fn test_single_url_builds_without_retries() { - let urls = vec![Url::parse("https://api.nym.com").unwrap()]; + let urls = [Url::parse("https://api.nym.com").unwrap()]; let nym_api_urls: Vec = urls .iter() @@ -555,7 +555,7 @@ mod tests { performance: u8, // 0-100 } - let nodes = vec![ + let nodes = [ // Node 53: entry-capable, good performance TestNode { id: 53, From 41ff3f7824f6cd763462aaf211bf851db6ce565d Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Fri, 17 Oct 2025 14:20:12 +0200 Subject: [PATCH 04/16] Address PR feedback: simplify code and reduce log noise - Reverted all changes to topology_control/nym_api_provider.rs - Changed info/warn logs to debug for custom client messages - Removed unused _rng parameter from gateways_for_init() - Simplified URL builder to always use new_with_urls() --- .../src/cli_helpers/client_add_gateway.rs | 2 - .../src/cli_helpers/client_init.rs | 2 - .../client-core/src/client/base_client/mod.rs | 4 +- .../topology_control/nym_api_provider.rs | 67 ++++++----------- common/client-core/src/init/helpers.rs | 71 +++---------------- common/wasm/client-core/src/helpers.rs | 13 +--- nym-registration-client/src/builder/config.rs | 4 +- sdk/rust/nym-sdk/src/mixnet/client.rs | 6 +- 8 files changed, 36 insertions(+), 133 deletions(-) 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..2f724a5014 100644 --- a/common/client-core/src/cli_helpers/client_add_gateway.rs +++ b/common/client-core/src/cli_helpers/client_add_gateway.rs @@ -114,9 +114,7 @@ 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, diff --git a/common/client-core/src/cli_helpers/client_init.rs b/common/client-core/src/cli_helpers/client_init.rs index a59d73e261..dc0ac2a37f 100644 --- a/common/client-core/src/cli_helpers/client_init.rs +++ b/common/client-core/src/cli_helpers/client_init.rs @@ -173,9 +173,7 @@ 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, diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index 7e2a6c04da..584288b78a 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -877,11 +877,11 @@ where ) -> Result { // If a custom client was provided (e.g., with domain fronting support), use it if let Some(client) = custom_client { - tracing::info!("Using CUSTOM nym-api HTTP client (with domain fronting support)"); + tracing::debug!("Using custom nym-api HTTP client"); return Ok(client); } - tracing::warn!("No custom HTTP client provided - creating DEFAULT client from config"); + tracing::debug!("Creating default nym-api HTTP client from config"); // Otherwise, create a basic client let mut nym_api_urls = config.get_nym_api_endpoints(); diff --git a/common/client-core/src/client/topology_control/nym_api_provider.rs b/common/client-core/src/client/topology_control/nym_api_provider.rs index b4dbfebb19..a81118665c 100644 --- a/common/client-core/src/client/topology_control/nym_api_provider.rs +++ b/common/client-core/src/client/topology_control/nym_api_provider.rs @@ -6,7 +6,6 @@ use nym_mixnet_contract_common::EpochRewardedSet; use nym_topology::provider_trait::{ToTopologyMetadata, TopologyProvider}; use nym_topology::NymTopology; use nym_validator_client::nym_api::NymApiClientExt; -use nym_validator_client::nym_nodes::NodeRole; use rand::prelude::SliceRandom; use rand::thread_rng; use std::cmp::min; @@ -136,31 +135,31 @@ impl NymApiTopologyProvider { } else { // if we're not using extended topology, we're only getting active set mixnodes and gateways - // Get ALL bonded nodes and filter for both mixnodes and gateways - // This ensures consistent metadata between both calls - let all_nodes_fut = self.validator_client.get_all_basic_nodes_with_metadata(); + let mixnodes_fut = self + .validator_client + .get_all_basic_active_mixing_assigned_nodes_with_metadata(); - let (rewarded_set, all_nodes_res) = futures::try_join!(rewarded_set_fut, all_nodes_fut) - .inspect_err(|err| { - error!("failed to get network nodes: {err}"); - }) - .ok()?; + // TODO: we really should be getting ACTIVE gateways only + let gateways_fut = self + .validator_client + .get_all_basic_entry_assigned_nodes_with_metadata(); - let metadata = all_nodes_res.metadata; - let all_nodes = all_nodes_res.nodes; + let (rewarded_set, mixnodes_res, gateways_res) = + futures::try_join!(rewarded_set_fut, mixnodes_fut, gateways_fut) + .inspect_err(|err| { + error!("failed to get network nodes: {err}"); + }) + .ok()?; - // Filter for active mixing nodes (role=Mixnode) - let mixnodes: Vec<_> = all_nodes - .iter() - .filter(|n| n.supported_roles.mixnode && matches!(n.role, NodeRole::Mixnode { .. })) - .cloned() - .collect(); + let metadata = mixnodes_res.metadata; + let mixnodes = mixnodes_res.nodes; - // Filter for entry-capable nodes (supported_roles.entry == true) - let gateways: Vec<_> = all_nodes - .into_iter() - .filter(|n| n.supported_roles.entry) - .collect(); + if !gateways_res.metadata.consistency_check(&metadata) { + warn!("inconsistent nodes metadata between mixnodes and gateways calls! {metadata:?} and {:?}", gateways_res.metadata); + return None; + } + + let gateways = gateways_res.nodes; debug!( "there are {} mixnodes and {} gateways in total (before performance filtering)", @@ -222,27 +221,3 @@ impl TopologyProvider for NymApiTopologyProvider { Some(topology) } } - -#[cfg(test)] -mod tests { - #[test] - fn test_filtering_logic() { - // This test verifies the filtering logic used in get_current_compatible_topology - // We test that the role matching pattern works correctly - use nym_validator_client::nym_nodes::NodeRole; - - // Test that the pattern matching for mixnodes works - let mixnode_role = NodeRole::Mixnode { layer: 1 }; - assert!( - matches!(mixnode_role, NodeRole::Mixnode { .. }), - "Should match mixnode role pattern" - ); - - // Test that other roles don't match - let entry_role = NodeRole::EntryGateway; - assert!( - !matches!(entry_role, NodeRole::Mixnode { .. }), - "Entry gateway should not match mixnode pattern" - ); - } -} diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index bee127b911..a8ed7ef1eb 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -144,8 +144,7 @@ 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, @@ -161,18 +160,9 @@ pub async fn gateways_for_init( return Err(ClientCoreError::ListOfNymApisIsEmpty); } - let mut builder = if nym_api_urls.len() == 1 { - nym_http_api_client::Client::builder(nym_api_urls[0].clone()).map_err(|e| { - ClientCoreError::ValidatorClientError(nym_validator_client::ValidatorClientError::from( - e, - )) - })? - } else { - nym_http_api_client::ClientBuilder::new_with_urls(nym_api_urls.clone()).with_retries(3) - // Enable URL rotation on failure - }; - - builder = builder.with_bincode(); + let mut builder = nym_http_api_client::ClientBuilder::new_with_urls(nym_api_urls.clone()) + .with_retries(3) + .with_bincode(); if let Some(user_agent) = user_agent { builder = builder.with_user_agent(user_agent); @@ -194,56 +184,18 @@ 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 total_fetched = gateways.len(); - - let after_role_filter_count = gateways - .iter() - .filter(|g| ignore_epoch_roles || !g.supported_roles.mixnode) - .count(); - tracing::debug!( - "After role filter: {} (removed {} mixnode-capable)", - after_role_filter_count, - total_fetched - after_role_filter_count - ); - - let after_performance_filter_count = gateways - .iter() - .filter(|g| ignore_epoch_roles || !g.supported_roles.mixnode) - .filter(|g| g.performance.round_to_integer() >= minimum_performance) - .count(); - tracing::debug!( - "After performance filter (>= {}%): {} (removed {} low-performance)", - minimum_performance, - after_performance_filter_count, - after_role_filter_count - after_performance_filter_count - ); - 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 conversion to RoutingNode: {} (removed {} invalid)", - valid_gateways.len(), - after_performance_filter_count - valid_gateways.len() - ); - tracing::trace!("Valid gateways: {valid_gateways:#?}"); tracing::info!( - "and {} after validity and performance filtering", + "Found {} valid gateways after filtering", valid_gateways.len() ); - for gw in &valid_gateways { - tracing::debug!( - "Available gateway: {} (node_id: {})", - gw.identity_key.to_base58_string(), - gw.node_id - ); - } - Ok(valid_gateways) } @@ -403,14 +355,6 @@ pub(super) fn get_specified_gateway( must_use_tls: bool, ) -> Result { tracing::debug!("Requesting specified gateway: {gateway_identity}"); - tracing::debug!("Searching in {} available gateways", gateways.len()); - for gw in gateways { - tracing::debug!( - "Gateway in list: {} (node_id: {})", - gw.identity_key.to_base58_string(), - gw.node_id - ); - } let user_gateway = ed25519::PublicKey::from_base58_string(gateway_identity) .map_err(ClientCoreError::UnableToCreatePublicKeyFromGatewayId)?; @@ -419,7 +363,10 @@ pub(super) fn get_specified_gateway( .iter() .find(|gateway| gateway.identity_key == user_gateway) .ok_or_else(|| { - tracing::error!("Gateway {gateway_identity} NOT FOUND in available gateways list!"); + tracing::debug!( + "Gateway {gateway_identity} not found in {} available gateways", + gateways.len() + ); ClientCoreError::NoGatewayWithId(gateway_identity.to_string()) })?; diff --git a/common/wasm/client-core/src/helpers.rs b/common/wasm/client-core/src/helpers.rs index 23aaee4cad..83fd977978 100644 --- a/common/wasm/client-core/src/helpers.rs +++ b/common/wasm/client-core/src/helpers.rs @@ -160,15 +160,8 @@ 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, - ) - .await?; + let gateways = + gateways_for_init(nym_apis, None, minimum_performance, ignore_epoch_roles).await?; setup_gateway_wasm(client_store, force_tls, chosen_gateway, gateways).await } @@ -178,9 +171,7 @@ 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, diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index 526934277c..b7070d1c5b 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -157,10 +157,8 @@ impl BuilderConfig { .custom_topology_provider(self.custom_topology_provider); if let Some(nym_api_client) = self.custom_nym_api_client { - tracing::info!("Registration client: Passing custom nym-api HTTP client to SDK"); + tracing::debug!("Using custom nym-api HTTP client"); builder = builder.with_nym_api_client(nym_api_client); - } else { - tracing::warn!("Registration client: No custom nym-api HTTP client provided"); } #[cfg(unix)] diff --git a/sdk/rust/nym-sdk/src/mixnet/client.rs b/sdk/rust/nym-sdk/src/mixnet/client.rs index 8287379963..e333f4e656 100644 --- a/sdk/rust/nym-sdk/src/mixnet/client.rs +++ b/sdk/rust/nym-sdk/src/mixnet/client.rs @@ -579,10 +579,8 @@ where 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, @@ -734,10 +732,8 @@ where } if let Some(nym_api_client) = self.custom_nym_api_client { - tracing::info!("SDK: Passing custom nym-api HTTP client to BaseClientBuilder"); + tracing::debug!("Using custom nym-api HTTP client"); base_builder = base_builder.with_nym_api_client(nym_api_client); - } else { - tracing::warn!("SDK: No custom nym-api HTTP client configured"); } if let Some(topology_provider) = self.custom_topology_provider { From 1be5ba310acef74a7b5e4305def2fe08d7285b4b Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Fri, 17 Oct 2025 14:27:31 +0200 Subject: [PATCH 05/16] Remove domain fronting code to keep gateway changes only This branch now contains only gateway registration improvements: - Multiple URL fallback support in gateways_for_init() - Get all entry-capable nodes for registration - Performance and code quality improvements --- Cargo.lock | 1 - .../client-core/src/client/base_client/mod.rs | 24 +----------- nym-registration-client/Cargo.toml | 1 - nym-registration-client/src/builder/config.rs | 37 +------------------ sdk/rust/nym-sdk/src/mixnet/client.rs | 20 ---------- 5 files changed, 2 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 917fe3c6a0..04f9da4092 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6790,7 +6790,6 @@ dependencies = [ "nym-bandwidth-controller", "nym-credential-storage", "nym-credentials-interface", - "nym-http-api-client", "nym-ip-packet-client", "nym-registration-common", "nym-sdk", diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index 584288b78a..af065b3dec 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -218,7 +218,6 @@ pub struct BaseClientBuilder { shutdown: Option, event_tx: Option, user_agent: Option, - custom_nym_api_client: Option, setup_method: GatewaySetup, @@ -248,7 +247,6 @@ where shutdown: None, event_tx: None, user_agent: None, - custom_nym_api_client: None, setup_method: GatewaySetup::MustLoad { gateway_id: None }, #[cfg(unix)] connection_fd_callback: None, @@ -256,12 +254,6 @@ where } } - #[must_use] - pub fn with_nym_api_client(mut self, client: nym_http_api_client::Client) -> Self { - self.custom_nym_api_client = Some(client); - self - } - #[must_use] pub fn with_derivation_material( mut self, @@ -873,17 +865,7 @@ where fn construct_nym_api_client( config: &Config, user_agent: Option, - custom_client: Option, ) -> Result { - // If a custom client was provided (e.g., with domain fronting support), use it - if let Some(client) = custom_client { - tracing::debug!("Using custom nym-api HTTP client"); - return Ok(client); - } - - tracing::debug!("Creating default nym-api HTTP client from config"); - - // Otherwise, create a basic client let mut nym_api_urls = config.get_nym_api_endpoints(); nym_api_urls.shuffle(&mut thread_rng()); @@ -979,11 +961,7 @@ where .dkg_query_client .map(|client| BandwidthController::new(credential_store, client)); - let nym_api_client = Self::construct_nym_api_client( - &self.config, - self.user_agent.clone(), - self.custom_nym_api_client, - )?; + let nym_api_client = Self::construct_nym_api_client(&self.config, self.user_agent.clone())?; let key_rotation_config = Self::determine_key_rotation_state(&nym_api_client).await?; let topology_provider = Self::setup_topology_provider( diff --git a/nym-registration-client/Cargo.toml b/nym-registration-client/Cargo.toml index dbff96026c..e468af0998 100644 --- a/nym-registration-client/Cargo.toml +++ b/nym-registration-client/Cargo.toml @@ -23,7 +23,6 @@ nym-authenticator-client = { path = "../nym-authenticator-client" } nym-bandwidth-controller = { path = "../common/bandwidth-controller" } nym-credential-storage = { path = "../common/credential-storage" } nym-credentials-interface = { path = "../common/credentials-interface" } -nym-http-api-client = { path = "../common/http-api-client" } nym-ip-packet-client = { path = "../nym-ip-packet-client" } nym-registration-common = { path = "../common/registration" } nym-sdk = { path = "../sdk/rust/nym-sdk" } diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index b7070d1c5b..47f5f7c8e8 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -34,7 +34,6 @@ pub struct BuilderConfig { pub two_hops: bool, pub user_agent: UserAgent, pub custom_topology_provider: Box, - pub custom_nym_api_client: Option, pub network_env: NymNetworkDetails, pub cancel_token: CancellationToken, #[cfg(unix)] @@ -57,9 +56,6 @@ pub struct MixnetClientConfig { } impl BuilderConfig { - /// Create a new BuilderConfig without domain fronting support - /// - /// For domain fronting support, set `custom_nym_api_client` to Some(client) after creation #[allow(clippy::too_many_arguments)] pub fn new( entry_node: NymNodeWithKeys, @@ -81,7 +77,6 @@ impl BuilderConfig { two_hops, user_agent, custom_topology_provider, - custom_nym_api_client: None, network_env, cancel_token, #[cfg(unix)] @@ -89,12 +84,6 @@ impl BuilderConfig { } } - /// Set a custom nym-api HTTP client (for domain fronting support) - pub fn with_nym_api_client(mut self, client: nym_http_api_client::Client) -> Self { - self.custom_nym_api_client = Some(client); - self - } - pub fn mixnet_client_debug_config(&self) -> DebugConfig { if self.two_hops { two_hop_debug_config(&self.mixnet_client_config) @@ -147,7 +136,7 @@ impl BuilderConfig { RememberMe::new_mixnet() }; - let mut builder = builder + let builder = builder .with_user_agent(self.user_agent) .request_gateway(self.entry_node.node.identity.to_string()) .network_details(self.network_env) @@ -156,11 +145,6 @@ impl BuilderConfig { .with_remember_me(remember_me) .custom_topology_provider(self.custom_topology_provider); - if let Some(nym_api_client) = self.custom_nym_api_client { - tracing::debug!("Using custom nym-api HTTP client"); - builder = builder.with_nym_api_client(nym_api_client); - } - #[cfg(unix)] let builder = builder.with_connection_fd_callback(self.connection_fd_callback); @@ -263,23 +247,4 @@ mod tests { assert_eq!(config.min_mixnode_performance, None); assert_eq!(config.min_gateway_performance, None); } - - #[test] - fn test_builder_config_has_custom_client_field() { - // Verify that BuilderConfig has the custom_nym_api_client field - // by creating a simple HTTP client - let http_client = nym_http_api_client::Client::builder( - nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), - ) - .expect("Failed to create client builder") - .build() - .expect("Failed to build client"); - - // Verify the client works - let urls = http_client.base_urls(); - assert!( - !urls.is_empty() && urls[0].as_str().contains("validator.nymtech.net"), - "HTTP client should be configured with correct URL" - ); - } } diff --git a/sdk/rust/nym-sdk/src/mixnet/client.rs b/sdk/rust/nym-sdk/src/mixnet/client.rs index e333f4e656..3c5b122205 100644 --- a/sdk/rust/nym-sdk/src/mixnet/client.rs +++ b/sdk/rust/nym-sdk/src/mixnet/client.rs @@ -54,7 +54,6 @@ pub struct MixnetClientBuilder { custom_topology_provider: Option>, custom_gateway_transceiver: Option>, custom_shutdown: Option, - custom_nym_api_client: Option, event_tx: Option, force_tls: bool, user_agent: Option, @@ -94,7 +93,6 @@ impl MixnetClientBuilder { socks5_config: None, wait_for_gateway: false, custom_topology_provider: None, - custom_nym_api_client: None, storage: storage_paths .initialise_default_persistent_storage() .await?, @@ -133,7 +131,6 @@ where wait_for_gateway: false, custom_topology_provider: None, custom_gateway_transceiver: None, - custom_nym_api_client: None, custom_shutdown: None, event_tx: None, force_tls: false, @@ -158,7 +155,6 @@ where wait_for_gateway: self.wait_for_gateway, custom_topology_provider: self.custom_topology_provider, custom_gateway_transceiver: self.custom_gateway_transceiver, - custom_nym_api_client: self.custom_nym_api_client, custom_shutdown: self.custom_shutdown, event_tx: self.event_tx, force_tls: self.force_tls, @@ -298,12 +294,6 @@ where self } - #[must_use] - pub fn with_nym_api_client(mut self, client: nym_http_api_client::Client) -> Self { - self.custom_nym_api_client = Some(client); - self - } - #[must_use] pub fn with_statistics_reporting(mut self, config: StatsReporting) -> Self { self.config.debug_config.stats_reporting = config; @@ -348,7 +338,6 @@ where client.custom_gateway_transceiver = self.custom_gateway_transceiver; client.custom_topology_provider = self.custom_topology_provider; - client.custom_nym_api_client = self.custom_nym_api_client; client.custom_shutdown = self.custom_shutdown; client.wait_for_gateway = self.wait_for_gateway; client.force_tls = self.force_tls; @@ -398,9 +387,6 @@ where /// advanced usage of custom gateways custom_gateway_transceiver: Option>, - /// Custom nym-api HTTP client (for domain fronting support) - custom_nym_api_client: Option, - /// Attempt to wait for the selected gateway (if applicable) to come online if its currently not bonded. wait_for_gateway: bool, @@ -473,7 +459,6 @@ where storage, custom_topology_provider: None, custom_gateway_transceiver: None, - custom_nym_api_client: None, wait_for_gateway: false, force_tls: false, custom_shutdown: None, @@ -731,11 +716,6 @@ where base_builder = base_builder.with_user_agent(user_agent); } - if let Some(nym_api_client) = self.custom_nym_api_client { - tracing::debug!("Using custom nym-api HTTP client"); - base_builder = base_builder.with_nym_api_client(nym_api_client); - } - if let Some(topology_provider) = self.custom_topology_provider { base_builder = base_builder.with_topology_provider(topology_provider); } From db813b6e3e6ef7d3baaef06f078e45b384fb8ca1 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Fri, 17 Oct 2025 15:18:28 +0200 Subject: [PATCH 06/16] Revert node filtering changes per Andrew's feedback Andrew clarified that get_basic_entry_assigned_nodes_v2() already filters by supported_roles.entry --- common/client-core/src/init/helpers.rs | 131 +------------------------ 1 file changed, 5 insertions(+), 126 deletions(-) diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index a8ed7ef1eb..9a593db3c5 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -89,22 +89,16 @@ async fn get_all_basic_entry_nodes_with_metadata( client: &nym_http_api_client::Client, use_bincode: bool, ) -> Result { - // Get ALL nodes (not just entry-assigned) because in some environments (like sandbox), - // nodes may be capable of entry gateway role but not currently assigned to it + // Get first page to obtain metadata let mut page = 0; let res = client - .get_basic_nodes_v2(false, Some(page), None, use_bincode) + .get_basic_entry_assigned_nodes_v2(false, Some(page), None, use_bincode) .await?; let mut nodes = res.nodes.data; let metadata = res.metadata; if res.nodes.pagination.total == nodes.len() { - // Filter for entry-capable nodes (nodes with supported_roles.entry == true) - let entry_nodes: Vec<_> = nodes - .into_iter() - .filter(|n| n.supported_roles.entry) - .collect(); - return Ok(SkimmedNodesWithMetadata::new(entry_nodes, metadata)); + return Ok(SkimmedNodesWithMetadata::new(nodes, metadata)); } page += 1; @@ -112,7 +106,7 @@ async fn get_all_basic_entry_nodes_with_metadata( // Collect remaining pages loop { let mut res = client - .get_basic_nodes_v2(false, Some(page), None, use_bincode) + .get_basic_entry_assigned_nodes_v2(false, Some(page), None, use_bincode) .await?; if !metadata.consistency_check(&res.metadata) { @@ -129,13 +123,7 @@ async fn get_all_basic_entry_nodes_with_metadata( } } - // Filter for entry-capable nodes (nodes with supported_roles.entry == true) - let entry_nodes: Vec<_> = nodes - .into_iter() - .filter(|n| n.supported_roles.entry) - .collect(); - - Ok(SkimmedNodesWithMetadata::new(entry_nodes, metadata)) + Ok(SkimmedNodesWithMetadata::new(nodes, metadata)) } impl<'a, G: ConnectableGateway> GatewayWithLatency<'a, G> { @@ -479,113 +467,4 @@ mod tests { assert!(nym_api_urls.is_empty(), "Empty list should remain empty"); } - - #[test] - fn test_gateway_filtering_logic() { - // NOTE: This test validates the filtering logic in isolation. - // It does NOT test the actual implementation in get_all_basic_entry_nodes_with_metadata - // or gateways_for_init (which would require mocking the HTTP client). - // The real proof is building and running the daemon. - // - // Test the core filtering logic used in gateways_for_init: - // 1. Filter by supported_roles.entry (not by epoch role assignment) - // 2. Filter by performance - // - // This test verifies the fix where nodes have role=Inactive - // but supported_roles.entry=true - - #[derive(Debug)] - struct TestNode { - id: u32, - entry_capable: bool, // supported_roles.entry - mixnode_capable: bool, // supported_roles.mixnode - performance: u8, // 0-100 - } - - let nodes = [ - // Node 53: entry-capable, good performance - TestNode { - id: 53, - entry_capable: true, - mixnode_capable: false, - performance: 100, - }, - // Node 97: entry-capable (but role=Inactive in sandbox), good performance - TestNode { - id: 97, - entry_capable: true, - mixnode_capable: false, - performance: 100, - }, - // Node 75: NOT entry-capable (mixnode only) - TestNode { - id: 75, - entry_capable: false, - mixnode_capable: true, - performance: 100, - }, - // Node 99: entry-capable but low performance - TestNode { - id: 99, - entry_capable: true, - mixnode_capable: false, - performance: 0, - }, - ]; - - let minimum_performance = 50; - let ignore_epoch_roles = true; - - // Step 1: Filter by supported_roles.entry (this is what the fix enables) - let entry_capable: Vec<_> = nodes.iter().filter(|n| n.entry_capable).collect(); - - assert_eq!( - entry_capable.len(), - 3, - "Should have 3 entry-capable nodes (53, 97, 99) - this includes Inactive nodes!" - ); - - // Step 2: Filter by role (exclude mixnode-capable if not ignoring epoch roles) - let after_role_filter: Vec<_> = entry_capable - .iter() - .filter(|g| ignore_epoch_roles || !g.mixnode_capable) - .collect(); - - assert_eq!( - after_role_filter.len(), - 3, - "All entry-capable nodes pass role filter with ignore_epoch_roles=true" - ); - - // Step 3: Filter by performance - let after_performance_filter: Vec<_> = after_role_filter - .iter() - .filter(|g| g.performance >= minimum_performance) - .collect(); - - assert_eq!( - after_performance_filter.len(), - 2, - "Should have 2 nodes after performance filter (53 and 97, excluding 99 with 0%)" - ); - - // Verify the correct nodes made it through - let node_ids: Vec = after_performance_filter.iter().map(|n| n.id).collect(); - assert!( - node_ids.contains(&53), - "Node 53 (actively assigned entry gateway) should be included" - ); - assert!( - node_ids.contains(&97), - "Node 97 (Inactive but entry-capable) should be included - THIS IS THE FIX!" - ); - assert!( - !node_ids.contains(&75), - "Node 75 (mixnode-only, not entry-capable) should be excluded" - ); - assert!( - !node_ids.contains(&99), - "Node 99 (low performance) should be excluded" - ); - } } From 6f4dfd1dab0e8b706467460f15d3327aaadb9791 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 11:15:31 +0200 Subject: [PATCH 07/16] fix conversion type && make the retry count configurable --- .../src/cli_helpers/client_add_gateway.rs | 1 + common/client-core/src/cli_helpers/client_init.rs | 1 + common/client-core/src/init/helpers.rs | 12 +++++++----- common/wasm/client-core/src/helpers.rs | 11 +++++++++-- sdk/rust/nym-sdk/src/mixnet/client.rs | 1 + 5 files changed, 19 insertions(+), 7 deletions(-) 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 2f724a5014..dc4280ba29 100644 --- a/common/client-core/src/cli_helpers/client_add_gateway.rs +++ b/common/client-core/src/cli_helpers/client_add_gateway.rs @@ -119,6 +119,7 @@ where 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 dc0ac2a37f..feda3ab8d1 100644 --- a/common/client-core/src/cli_helpers/client_init.rs +++ b/common/client-core/src/cli_helpers/client_init.rs @@ -178,6 +178,7 @@ where 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 9a593db3c5..67a0769534 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -137,19 +137,21 @@ pub async fn gateways_for_init( user_agent: Option, minimum_performance: u8, ignore_epoch_roles: bool, + retry_count: Option, ) -> Result, ClientCoreError> { // Build client with ALL URLs for fallback support let nym_api_urls: Vec = nym_apis .iter() - .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .map(|url| nym_http_api_client::Url::from(url.clone())) .collect(); if nym_api_urls.is_empty() { return Err(ClientCoreError::ListOfNymApisIsEmpty); } + let retry_count = retry_count.unwrap_or(3); let mut builder = nym_http_api_client::ClientBuilder::new_with_urls(nym_api_urls.clone()) - .with_retries(3) + .with_retries(retry_count) .with_bincode(); if let Some(user_agent) = user_agent { @@ -430,7 +432,7 @@ mod tests { let nym_api_urls: Vec = urls .iter() - .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .map(|url| nym_http_api_client::Url::from(url.clone())) .collect(); assert_eq!(nym_api_urls.len(), 1, "Should have exactly one URL"); @@ -446,7 +448,7 @@ mod tests { let nym_api_urls: Vec = urls .iter() - .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .map(|url| nym_http_api_client::Url::from(url.clone())) .collect(); assert_eq!(nym_api_urls.len(), 3, "Should have all three URLs"); @@ -462,7 +464,7 @@ mod tests { let nym_api_urls: Vec = urls .iter() - .filter_map(|url| nym_http_api_client::Url::parse(url.as_str()).ok()) + .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 83fd977978..de5cd6883a 100644 --- a/common/wasm/client-core/src/helpers.rs +++ b/common/wasm/client-core/src/helpers.rs @@ -160,8 +160,14 @@ pub async fn setup_gateway_from_api( minimum_performance: u8, ignore_epoch_roles: bool, ) -> Result { - let gateways = - gateways_for_init(nym_apis, None, minimum_performance, ignore_epoch_roles).await?; + let gateways = gateways_for_init( + nym_apis, + None, + minimum_performance, + ignore_epoch_roles, + None, + ) + .await?; setup_gateway_wasm(client_store, force_tls, chosen_gateway, gateways).await } @@ -176,6 +182,7 @@ pub async fn current_gateways_wasm( user_agent, minimum_performance, ignore_epoch_roles, + None, ) .await } diff --git a/sdk/rust/nym-sdk/src/mixnet/client.rs b/sdk/rust/nym-sdk/src/mixnet/client.rs index 3c5b122205..1e5371c5ee 100644 --- a/sdk/rust/nym-sdk/src/mixnet/client.rs +++ b/sdk/rust/nym-sdk/src/mixnet/client.rs @@ -570,6 +570,7 @@ where user_agent, topology_cfg.minimum_gateway_performance, topology_cfg.ignore_ingress_epoch_role, + None, ) .await } From a266137278a6311d438eef8d19e875cf4d22d909 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 11:39:50 +0200 Subject: [PATCH 08/16] Add optional builder pattern for BuilderConfig (non-breaking) Addresses @jstuczyn's feedback about too many arguments by adding BuilderConfigBuilder as an alternative to the existing new() method. --- nym-registration-client/src/builder/config.rs | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index 47f5f7c8e8..f19e981f5a 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -56,6 +56,9 @@ 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, @@ -84,6 +87,22 @@ impl BuilderConfig { } } + /// 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) @@ -235,6 +254,113 @@ fn true_to_disabled(val: bool) -> &'static str { if val { "disabled" } else { "enabled" } } +/// Builder for `BuilderConfig` +/// +/// This provides a more convinient 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("entry_node is required")?, + exit_node: self.exit_node.ok_or("exit_node is required")?, + data_path: self.data_path, + mixnet_client_config: self + .mixnet_client_config + .ok_or("mixnet_client_config is required")?, + two_hops: self.two_hops, + user_agent: self.user_agent.ok_or("user_agent is required")?, + custom_topology_provider: self + .custom_topology_provider + .ok_or("custom_topology_provider is required")?, + network_env: self.network_env.ok_or("network_env is required")?, + cancel_token: self.cancel_token.ok_or("cancel_token is required")?, + #[cfg(unix)] + connection_fd_callback: self + .connection_fd_callback + .ok_or("connection_fd_callback is required")?, + }) + } +} + #[cfg(test)] mod tests { use super::*; @@ -247,4 +373,49 @@ mod tests { 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 + let result = BuilderConfig::builder().build(); + assert!(result.is_err()); + if let Err(err) = result { + assert!(err.contains("entry_node is required")); + } + } + + #[test] + fn test_builder_config_builder_fails_missing_individual_fields() { + // Test that each required field is validated + let result = BuilderConfig::builder().build(); + assert!(result.is_err()); + + // We can only test the first error message since build() short-circuits + if let Err(err) = result { + assert!( + err.contains("entry_node") + || err.contains("exit_node") + || err.contains("mixnet_client_config") + || err.contains("user_agent") + || err.contains("custom_topology_provider") + || err.contains("network_env") + || err.contains("cancel_token") + ); + } + } + + #[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()); + } } From c448ec823a2304b9900dbaa149cf5acd2a93b23a Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 11:52:04 +0200 Subject: [PATCH 09/16] Remove tests for removed with_nym_api_client method These tests were referencing with_nym_api_client() which was removed when cleaning domain fronting code from this branch --- sdk/rust/nym-sdk/src/mixnet/client.rs | 59 --------------------------- 1 file changed, 59 deletions(-) diff --git a/sdk/rust/nym-sdk/src/mixnet/client.rs b/sdk/rust/nym-sdk/src/mixnet/client.rs index 1e5371c5ee..258d2f3c37 100644 --- a/sdk/rust/nym-sdk/src/mixnet/client.rs +++ b/sdk/rust/nym-sdk/src/mixnet/client.rs @@ -894,63 +894,4 @@ mod tests { "Builder should succeed without custom client" ); } - - #[test] - fn test_mixnet_builder_with_custom_client() { - let http_client = nym_http_api_client::Client::builder( - nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), - ) - .expect("Failed to create client builder") - .build() - .expect("Failed to build client"); - - let builder = MixnetClientBuilder::new_ephemeral().with_nym_api_client(http_client); - - assert!( - builder.build().is_ok(), - "Builder should succeed with custom client" - ); - } - - // Note: Tests for entry_capable_nodes() vs entry_gateways() are in nym-topology crate - // These tests verify the builder functionality only - - #[test] - fn test_custom_client_transfer_through_build() { - let http_client = nym_http_api_client::Client::builder( - nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), - ) - .expect("Failed to create client builder") - .build() - .expect("Failed to build client"); - - let builder = MixnetClientBuilder::new_ephemeral().with_nym_api_client(http_client); - let disconnected_client = builder.build(); - - assert!( - disconnected_client.is_ok(), - "Build should transfer custom_nym_api_client successfully" - ); - } - - #[test] - fn test_builder_storage_transfer_includes_custom_client() { - use nym_client_core::client::base_client::storage::Ephemeral; - - let http_client = nym_http_api_client::Client::builder( - nym_http_api_client::Url::parse("https://validator.nymtech.net/api").unwrap(), - ) - .expect("Failed to create client builder") - .build() - .expect("Failed to build client"); - - let builder = MixnetClientBuilder::new_ephemeral().with_nym_api_client(http_client); - let new_storage = Ephemeral::default(); - let builder_with_new_storage = builder.set_storage(new_storage); - - assert!( - builder_with_new_storage.build().is_ok(), - "set_storage should preserve custom_nym_api_client" - ); - } } From 18cebdfedc7b3001e754bc41d0fa977ac7def770 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 14:33:57 +0200 Subject: [PATCH 10/16] Add accessor methods for Url internals Add inner_url() and fronts() accessor methods to nym_http_api_client::Url for VPN client integration --- common/http-api-client/src/url.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/common/http-api-client/src/url.rs b/common/http-api-client/src/url.rs index d964a6a3f4..e0aafbdd4c 100644 --- a/common/http-api-client/src/url.rs +++ b/common/http-api-client/src/url.rs @@ -191,6 +191,16 @@ impl Url { false } + /// Returns the inner `url::Url` reference + pub fn inner_url(&self) -> &url::Url { + &self.url + } + + /// Returns the front URLs if configured + pub fn fronts(&self) -> Option<&Vec> { + self.fronts.as_ref() + } + /// Return the string representation of the current front host (domain or IP address) for this /// URL, if any. pub fn front_str(&self) -> Option<&str> { From ae6539e07c1768808d3e7bc52dc73d3fa26e0d86 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 15:14:48 +0200 Subject: [PATCH 11/16] Merge resolution --- common/http-api-client/src/url.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/common/http-api-client/src/url.rs b/common/http-api-client/src/url.rs index d964a6a3f4..e0aafbdd4c 100644 --- a/common/http-api-client/src/url.rs +++ b/common/http-api-client/src/url.rs @@ -191,6 +191,16 @@ impl Url { false } + /// Returns the inner `url::Url` reference + pub fn inner_url(&self) -> &url::Url { + &self.url + } + + /// Returns the front URLs if configured + pub fn fronts(&self) -> Option<&Vec> { + self.fronts.as_ref() + } + /// Return the string representation of the current front host (domain or IP address) for this /// URL, if any. pub fn front_str(&self) -> Option<&str> { From d1cb9afaf06eef29f31cd666ca2d84f447cb37c0 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 15:20:24 +0200 Subject: [PATCH 12/16] not sure what happened but it's fixed --- common/http-api-client/src/url.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/common/http-api-client/src/url.rs b/common/http-api-client/src/url.rs index 0f0e2e80fa..8b3b1a65b8 100644 --- a/common/http-api-client/src/url.rs +++ b/common/http-api-client/src/url.rs @@ -196,16 +196,6 @@ impl Url { false } - /// Returns the inner `url::Url` reference - pub fn inner_url(&self) -> &url::Url { - &self.url - } - - /// Returns the front URLs if configured - pub fn fronts(&self) -> Option<&Vec> { - self.fronts.as_ref() - } - /// Return the string representation of the current front host (domain or IP address) for this /// URL, if any. pub fn front_str(&self) -> Option<&str> { From 35ea7e4926299e346d066e1de7338ec1894bc6aa Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 16:51:07 +0200 Subject: [PATCH 13/16] - Add DEFAULT_NYM_API_RETRIES constant (replaces magic number 3) - Run cargo fmt on all affected packages - All clippy warnings resolved --- common/client-core/src/init/helpers.rs | 3 ++- nym-registration-client/src/builder/config.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index 67a0769534..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); @@ -149,7 +150,7 @@ pub async fn gateways_for_init( return Err(ClientCoreError::ListOfNymApisIsEmpty); } - let retry_count = retry_count.unwrap_or(3); + 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(); diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index f19e981f5a..c3bbbf3668 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -256,7 +256,7 @@ fn true_to_disabled(val: bool) -> &'static str { /// Builder for `BuilderConfig` /// -/// This provides a more convinient way to construct a `BuilderConfig` compared to the +/// This provides a more convenient way to construct a `BuilderConfig` compared to the /// `new()` constructor with many arguments. #[derive(Default)] pub struct BuilderConfigBuilder { From 923c1fa184926dfa770f5548ab977aaf3856ecaa Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Mon, 20 Oct 2025 16:57:31 +0200 Subject: [PATCH 14/16] Improve error handling Changes: - Replace String error with BuilderConfigError enum in BuilderConfigBuilder - Update tests to use pattern matching instead of string assertions --- nym-registration-client/src/builder/config.rs | 80 +++++++++++++------ 1 file changed, 56 insertions(+), 24 deletions(-) diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index c3bbbf3668..5dada8f81a 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -254,6 +254,28 @@ fn true_to_disabled(val: bool) -> &'static str { if val { "disabled" } else { "enabled" } } +/// Error type for BuilderConfig validation +#[derive(Debug, Clone, thiserror::Error)] +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 @@ -338,25 +360,33 @@ impl BuilderConfigBuilder { /// Builds the `BuilderConfig`. /// /// Returns an error if any required field is missing. - pub fn build(self) -> Result { + pub fn build(self) -> Result { Ok(BuilderConfig { - entry_node: self.entry_node.ok_or("entry_node is required")?, - exit_node: self.exit_node.ok_or("exit_node is required")?, + 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("mixnet_client_config is required")?, + .ok_or(BuilderConfigError::MissingMixnetClientConfig)?, two_hops: self.two_hops, - user_agent: self.user_agent.ok_or("user_agent is required")?, + user_agent: self + .user_agent + .ok_or(BuilderConfigError::MissingUserAgent)?, custom_topology_provider: self .custom_topology_provider - .ok_or("custom_topology_provider is required")?, - network_env: self.network_env.ok_or("network_env is required")?, - cancel_token: self.cancel_token.ok_or("cancel_token is required")?, + .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("connection_fd_callback is required")?, + .ok_or(BuilderConfigError::MissingConnectionFdCallback)?, }) } } @@ -376,31 +406,33 @@ mod tests { #[test] fn test_builder_config_builder_fails_without_required_fields() { - // Building without any fields should fail + // Building without any fields should fail with specific error let result = BuilderConfig::builder().build(); assert!(result.is_err()); - if let Err(err) = result { - assert!(err.contains("entry_node is required")); + 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_fails_missing_individual_fields() { + 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()); - // We can only test the first error message since build() short-circuits - if let Err(err) = result { - assert!( - err.contains("entry_node") - || err.contains("exit_node") - || err.contains("mixnet_client_config") - || err.contains("user_agent") - || err.contains("custom_topology_provider") - || err.contains("network_env") - || err.contains("cancel_token") - ); + // Short-circuits at first missing field, so we just verify it's one of the expected errors + match result { + Err(BuilderConfigError::MissingEntryNode) + | Err(BuilderConfigError::MissingExitNode) + | Err(BuilderConfigError::MissingMixnetClientConfig) + | Err(BuilderConfigError::MissingUserAgent) + | Err(BuilderConfigError::MissingTopologyProvider) + | Err(BuilderConfigError::MissingNetworkEnv) + | Err(BuilderConfigError::MissingCancelToken) => (), // Expected + Err(e) => panic!("Unexpected error: {}", e), + Ok(_) => panic!("Expected validation error, got Ok"), } } From a0cb812eff1eddb9c24ea82221287fc3852870c8 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Tue, 21 Oct 2025 10:35:57 +0200 Subject: [PATCH 15/16] Allow clippy::enum_variant_names for BuilderConfigError --- nym-registration-client/src/builder/config.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index 5dada8f81a..e893606c4a 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -256,6 +256,7 @@ fn true_to_disabled(val: bool) -> &'static str { /// Error type for BuilderConfig validation #[derive(Debug, Clone, thiserror::Error)] +#[allow(clippy::enum_variant_names)] pub enum BuilderConfigError { #[error("entry_node is required")] MissingEntryNode, From a07a24db00651cda508edbf189b0278d7c5f6810 Mon Sep 17 00:00:00 2001 From: Tommy Verrall Date: Tue, 21 Oct 2025 11:01:04 +0200 Subject: [PATCH 16/16] Fix CI issues --- nym-registration-client/src/builder/config.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nym-registration-client/src/builder/config.rs b/nym-registration-client/src/builder/config.rs index e893606c4a..d1a342faa9 100644 --- a/nym-registration-client/src/builder/config.rs +++ b/nym-registration-client/src/builder/config.rs @@ -424,6 +424,7 @@ mod tests { 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) @@ -431,7 +432,9 @@ mod tests { | Err(BuilderConfigError::MissingUserAgent) | Err(BuilderConfigError::MissingTopologyProvider) | Err(BuilderConfigError::MissingNetworkEnv) - | Err(BuilderConfigError::MissingCancelToken) => (), // Expected + | Err(BuilderConfigError::MissingCancelToken) => (), + #[cfg(unix)] + Err(BuilderConfigError::MissingConnectionFdCallback) => (), Err(e) => panic!("Unexpected error: {}", e), Ok(_) => panic!("Expected validation error, got Ok"), }