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()
This commit is contained in:
Tommy Verrall
2025-10-17 14:20:12 +02:00
parent c9d4d62446
commit 41ff3f7824
8 changed files with 36 additions and 133 deletions
@@ -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,
@@ -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,
@@ -877,11 +877,11 @@ where
) -> 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::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();
@@ -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"
);
}
}
+9 -62
View File
@@ -144,8 +144,7 @@ impl<'a, G: ConnectableGateway> GatewayWithLatency<'a, G> {
}
}
pub async fn gateways_for_init<R: Rng>(
_rng: &mut R,
pub async fn gateways_for_init(
nym_apis: &[Url],
user_agent: Option<UserAgent>,
minimum_performance: u8,
@@ -161,18 +160,9 @@ pub async fn gateways_for_init<R: Rng>(
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<R: Rng>(
// 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<RoutingNode> = 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<RoutingNode, ClientCoreError> {
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())
})?;
+2 -11
View File
@@ -160,15 +160,8 @@ pub async fn setup_gateway_from_api(
minimum_performance: u8,
ignore_epoch_roles: bool,
) -> Result<InitialisationResult, WasmCoreError> {
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<Vec<RoutingNode>, ClientCoreError> {
let mut rng = thread_rng();
gateways_for_init(
&mut rng,
nym_apis,
user_agent,
minimum_performance,
@@ -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)]
+1 -5
View File
@@ -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 {