Merge pull request #6126 from nymtech/multiple-fall-back-urls

Changes:

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