Compare commits

...

2 Commits

Author SHA1 Message Date
Tommy Verrall 1381599c13 Revert node filtering changes per Andrew's feedback 2025-10-17 15:21:58 +02:00
Tommy Verrall 62967f736d Refactor domain fronting to use configuration-based approach
How it works:
1. VPN configures NymNetworkDetails with front_hosts
2. SDK passes network_details to BaseClientBuilder via with_network_details()
3. BaseClientBuilder uses ClientBuilder::from_network() when available
4. from_network() automatically enables domain fronting if front_hosts configured

Benefits:
- Configuration-based (network_details), not object-based (cleaner)
- No breaking changes for existing clients

Changes:
- Added network_details: Option<NymNetworkDetails> to BaseClientBuilder
- Updated tests
2025-10-17 15:11:18 +02:00
8 changed files with 52 additions and 255 deletions
Generated
-1
View File
@@ -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",
+1 -1
View File
@@ -36,7 +36,7 @@ nym-bandwidth-controller = { path = "../bandwidth-controller" }
nym-crypto = { path = "../crypto" }
nym-gateway-client = { path = "../client-libs/gateway-client" }
nym-gateway-requests = { path = "../gateway-requests" }
nym-http-api-client = { path = "../http-api-client" }
nym-http-api-client = { path = "../http-api-client", features = ["network-defaults"] }
nym-nonexhaustive-delayqueue = { path = "../nonexhaustive-delayqueue" }
nym-sphinx = { path = "../nymsphinx" }
nym-statistics-common = { path = "../statistics" }
@@ -46,6 +46,7 @@ use nym_gateway_client::client::config::GatewayClientConfig;
use nym_gateway_client::{
AcknowledgementReceiver, GatewayClient, GatewayConfig, MixnetMessageReceiver, PacketRouter,
};
use nym_network_defaults::NymNetworkDetails;
use nym_sphinx::acknowledgements::AckKey;
use nym_sphinx::addressing::clients::Recipient;
use nym_sphinx::addressing::nodes::NodeIdentity;
@@ -212,13 +213,15 @@ pub struct BaseClientBuilder<C, S: MixnetClientStorage> {
client_store: S,
dkg_query_client: Option<C>,
// Optional network details for domain fronting support
network_details: Option<NymNetworkDetails>,
wait_for_gateway: bool,
custom_topology_provider: Option<Box<dyn TopologyProvider + Send + Sync>>,
custom_gateway_transceiver: Option<Box<dyn GatewayTransceiver + Send>>,
shutdown: Option<ShutdownTracker>,
event_tx: Option<EventSender>,
user_agent: Option<UserAgent>,
custom_nym_api_client: Option<nym_http_api_client::Client>,
setup_method: GatewaySetup,
@@ -242,13 +245,13 @@ where
config: base_config,
client_store,
dkg_query_client,
network_details: None,
wait_for_gateway: false,
custom_topology_provider: None,
custom_gateway_transceiver: None,
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,9 +259,13 @@ where
}
}
/// Provide network details for domain fronting support
///
/// When provided, the SDK will use `from_network()` to build the nym-api client,
/// which automatically handles domain fronting if configured in the network details.
#[must_use]
pub fn with_nym_api_client(mut self, client: nym_http_api_client::Client) -> Self {
self.custom_nym_api_client = Some(client);
pub fn with_network_details(mut self, network_details: NymNetworkDetails) -> Self {
self.network_details = Some(network_details);
self
}
@@ -871,19 +878,29 @@ where
}
fn construct_nym_api_client(
network_details: Option<&NymNetworkDetails>,
config: &Config,
user_agent: Option<UserAgent>,
custom_client: Option<nym_http_api_client::Client>,
) -> Result<nym_http_api_client::Client, ClientCoreError> {
// 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);
// If network details are provided, use from_network() which handles domain fronting
if let Some(network_details) = network_details {
tracing::debug!(
"Building nym-api client from network details (with domain fronting support)"
);
let mut builder = nym_http_api_client::ClientBuilder::from_network(network_details)
.map_err(ClientCoreError::from)?;
if let Some(user_agent) = user_agent {
builder = builder.with_user_agent(user_agent);
}
return builder.build().map_err(ClientCoreError::from);
}
tracing::debug!("Creating default nym-api HTTP client from config");
// Fallback to basic client for backwards compatibility
tracing::debug!("Building basic nym-api HTTP client from config endpoints");
// Otherwise, create a basic client
let mut nym_api_urls = config.get_nym_api_endpoints();
nym_api_urls.shuffle(&mut thread_rng());
@@ -980,9 +997,9 @@ where
.map(|client| BandwidthController::new(credential_store, client));
let nym_api_client = Self::construct_nym_api_client(
self.network_details.as_ref(),
&self.config,
self.user_agent.clone(),
self.custom_nym_api_client,
)?;
let key_rotation_config = Self::determine_key_rotation_state(&nym_api_client).await?;
+5 -126
View File
@@ -89,22 +89,16 @@ async fn get_all_basic_entry_nodes_with_metadata(
client: &nym_http_api_client::Client,
use_bincode: bool,
) -> Result<SkimmedNodesWithMetadata, ClientCoreError> {
// 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<u32> = 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"
);
}
}
+9 -6
View File
@@ -594,10 +594,9 @@ impl ClientBuilder {
let urls = network
.nym_api_urls
.as_ref()
.ok_or_else(|| {
HttpClientError::GenericRequestFailure(
"No API URLs configured in network details".to_string(),
)
.ok_or_else(|| HttpClientError::InternalResponseInconsistency {
url: ::url::Url::parse("about:blank").unwrap(),
details: "No API URLs configured in network details".to_string(),
})?
.iter()
.map(|api_url| {
@@ -611,8 +610,12 @@ impl ClientBuilder {
.iter()
.map(|host| format!("https://{}", host))
.collect();
url = Url::new(api_url.url.clone(), Some(fronts))
.map_err(|e| HttpClientError::GenericRequestFailure(e.to_string()))?;
url = Url::new(api_url.url.clone(), Some(fronts)).map_err(|e| {
HttpClientError::MalformedUrl {
raw: api_url.url.clone(),
source: e,
}
})?;
}
Ok(url)
-1
View File
@@ -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" }
+1 -36
View File
@@ -34,7 +34,6 @@ pub struct BuilderConfig {
pub two_hops: bool,
pub user_agent: UserAgent,
pub custom_topology_provider: Box<dyn TopologyProvider + Send + Sync>,
pub custom_nym_api_client: Option<nym_http_api_client::Client>,
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"
);
}
}
+7 -72
View File
@@ -54,7 +54,6 @@ pub struct MixnetClientBuilder<S: MixnetClientStorage = Ephemeral> {
custom_topology_provider: Option<Box<dyn TopologyProvider + Send + Sync>>,
custom_gateway_transceiver: Option<Box<dyn GatewayTransceiver + Send + Sync>>,
custom_shutdown: Option<ShutdownTracker>,
custom_nym_api_client: Option<nym_http_api_client::Client>,
event_tx: Option<EventSender>,
force_tls: bool,
user_agent: Option<UserAgent>,
@@ -94,7 +93,6 @@ impl MixnetClientBuilder<OnDiskPersistent> {
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<Box<dyn GatewayTransceiver + Send + Sync>>,
/// Custom nym-api HTTP client (for domain fronting support)
custom_nym_api_client: Option<nym_http_api_client::Client>,
/// 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,
@@ -722,6 +707,7 @@ where
let mut base_builder: BaseClientBuilder<_, _> =
BaseClientBuilder::new(base_config, self.storage, self.dkg_query_client)
.with_network_details(self.config.network_details.clone())
.with_wait_for_gateway(self.wait_for_gateway)
.with_forget_me(&self.forget_me)
.with_remember_me(&self.remember_me)
@@ -731,11 +717,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);
}
@@ -906,70 +887,24 @@ mod tests {
use super::*;
#[test]
fn test_mixnet_builder_default_no_custom_client() {
fn test_mixnet_builder_default() {
let builder = MixnetClientBuilder::new_ephemeral();
assert!(
builder.build().is_ok(),
"Builder should succeed without custom client"
"Builder should succeed with default configuration"
);
}
#[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");
fn test_mixnet_builder_with_network_details() {
use nym_network_defaults::NymNetworkDetails;
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 builder = MixnetClientBuilder::new_ephemeral();
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"
"Builder should succeed and domain fronting is handled via network_details"
);
}
}