From 4e5ccf79260759eff2ad75fa78cbe6d042550214 Mon Sep 17 00:00:00 2001 From: durch Date: Fri, 22 Aug 2025 18:13:36 +0200 Subject: [PATCH] fix: provide all API URLs for automatic failover in endpoint rotation Previously, when rotating API endpoints, only a single URL was provided to the HTTP client, defeating the purpose of having multiple URLs for resilience. Changes: - NymApiTopologyProvider now provides all URLs in rotated order when switching endpoints - NymApisClient similarly provides all URLs starting from the working endpoint - Added clarifying comments for broadcast/exhaustive query methods where single URLs are intentionally used - This enables the HTTP client's built-in failover mechanism while maintaining endpoint rotation behavior The fix ensures that if the primary endpoint fails, the client can automatically failover to alternative endpoints without manual intervention, improving overall network resilience. --- .../topology_control/nym_api_provider.rs | 19 +++++++--- .../validator-client/src/nym_api/mod.rs | 21 +++++++++++ common/wasm/client-core/src/helpers.rs | 36 +++++++++++-------- .../src/metrics_scraper/mod.rs | 8 ++--- .../nym-node-status-api/src/monitor/mod.rs | 24 ++++++------- nym-node/src/node/nym_apis_client.rs | 18 ++++++++-- nym-statistics-api/src/network_view.rs | 18 +++++----- 7 files changed, 93 insertions(+), 51 deletions(-) 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 15ffc8d482..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 @@ -61,7 +61,7 @@ impl NymApiTopologyProvider { currently_used_api: 0, use_bincode: true, }; - // Set the initial API URL + // Set all API URLs - the client will try them in order with automatic failover provider.validator_client.change_base_urls( provider .nym_api_urls @@ -87,10 +87,19 @@ impl NymApiTopologyProvider { } self.currently_used_api = (self.currently_used_api + 1) % self.nym_api_urls.len(); - self.validator_client - .change_base_urls(vec![self.nym_api_urls[self.currently_used_api] - .clone() - .into()]) + + // Provide all URLs starting from the next one in rotation order + // This enables automatic failover to other endpoints + let rotated_urls: Vec<_> = self + .nym_api_urls + .iter() + .cycle() + .skip(self.currently_used_api) + .take(self.nym_api_urls.len()) + .map(|u| u.clone().into()) + .collect(); + + self.validator_client.change_base_urls(rotated_urls) } async fn get_current_compatible_topology(&mut self) -> Option { diff --git a/common/client-libs/validator-client/src/nym_api/mod.rs b/common/client-libs/validator-client/src/nym_api/mod.rs index 115cfcf5df..eeea853cba 100644 --- a/common/client-libs/validator-client/src/nym_api/mod.rs +++ b/common/client-libs/validator-client/src/nym_api/mod.rs @@ -50,7 +50,9 @@ use time::format_description::BorrowedFormatItem; use time::Date; use tracing::instrument; +use crate::ValidatorClientError; pub use nym_coconut_dkg_common::types::EpochId; + pub mod error; pub mod routes; @@ -426,6 +428,25 @@ pub trait NymApiClientExt: ApiClient { .await } + async fn get_all_bonded_nym_nodes(&self) -> Result, ValidatorClientError> { + // TODO: deal with paging in macro or some helper function or something, because it's the same pattern everywhere + let mut page = 0; + let mut bonds = Vec::new(); + + loop { + let mut res = self.get_nym_nodes(Some(page), None).await?; + + bonds.append(&mut res.data); + if bonds.len() < res.pagination.total { + page += 1 + } else { + break; + } + } + + Ok(bonds) + } + #[deprecated] #[tracing::instrument(level = "debug", skip_all)] async fn get_basic_mixnodes(&self) -> Result, NymAPIError> { diff --git a/common/wasm/client-core/src/helpers.rs b/common/wasm/client-core/src/helpers.rs index e90c2c6718..3b86e6663d 100644 --- a/common/wasm/client-core/src/helpers.rs +++ b/common/wasm/client-core/src/helpers.rs @@ -19,7 +19,7 @@ use nym_client_core::init::{ use nym_sphinx::addressing::clients::Recipient; use nym_sphinx::anonymous_replies::requests::AnonymousSenderTag; use nym_topology::wasm_helpers::WasmFriendlyNymTopology; -use nym_topology::{NymTopology, RoutingNode, EpochRewardedSet}; +use nym_topology::{EpochRewardedSet, NymTopology, RoutingNode}; use nym_validator_client::client::IdentityKey; use nym_validator_client::{nym_api::NymApiClientExt, UserAgent}; use rand::thread_rng; @@ -72,17 +72,19 @@ pub async fn current_network_topology_async( } }; - let api_client = - nym_http_api_client::Client::builder::<_, nym_validator_client::models::RequestError>(url.clone()) - .map_err(|_err| WasmCoreError::MalformedUrl { - raw: nym_api_url.to_string(), - source: url::ParseError::EmptyHost, - })? - .build::() - .map_err(|_err| WasmCoreError::MalformedUrl { - raw: nym_api_url.to_string(), - source: url::ParseError::EmptyHost, - })?; + let api_client = nym_http_api_client::Client::builder::< + _, + nym_validator_client::models::RequestError, + >(url.clone()) + .map_err(|_err| WasmCoreError::MalformedUrl { + raw: nym_api_url.to_string(), + source: url::ParseError::EmptyHost, + })? + .build::() + .map_err(|_err| WasmCoreError::MalformedUrl { + raw: nym_api_url.to_string(), + source: url::ParseError::EmptyHost, + })?; let rewarded_set = api_client.get_current_rewarded_set().await?; let mixnodes_res = api_client .get_all_basic_active_mixing_assigned_nodes_with_metadata() @@ -101,9 +103,13 @@ pub async fn current_network_topology_async( let gateways = gateways_res.nodes; let epoch_rewarded_set: EpochRewardedSet = rewarded_set.into(); - let topology = NymTopology::new(metadata.to_topology_metadata(), epoch_rewarded_set, Vec::new()) - .with_skimmed_nodes(&mixnodes) - .with_skimmed_nodes(&gateways); + let topology = NymTopology::new( + metadata.to_topology_metadata(), + epoch_rewarded_set, + Vec::new(), + ) + .with_skimmed_nodes(&mixnodes) + .with_skimmed_nodes(&gateways); Ok(topology.into()) } diff --git a/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs b/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs index bc3cd23fe7..406b8e184e 100644 --- a/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs +++ b/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs @@ -5,11 +5,11 @@ use nym_node_requests::api::{client::NymNodeApiClientExt, v1::metrics::models::S use nym_validator_client::{ client::{NodeId, NymNodeDetails}, models::{DescribedNodeType, NymNodeDescription}, - NymApiClient, }; use time::OffsetDateTime; use nym_statistics_common::types::SessionType; +use nym_validator_client::client::NymApiClientExt; use std::collections::HashMap; use tokio::time::Duration; use tracing::instrument; @@ -62,11 +62,9 @@ async fn run( .with_timeout(nym_api_client_timeout) .build::<&str>()?; - let api_client = NymApiClient::from(nym_api); - //SW TBC what nodes exactly need to be scraped, the skimmed node endpoint seems to return more nodes - let bonded_nodes = api_client.get_all_bonded_nym_nodes().await?; - let all_nodes = api_client.get_all_described_nodes().await?; //legacy node that did not upgrade the contract bond yet + let bonded_nodes = nym_api.get_all_bonded_nym_nodes().await?; + let all_nodes = nym_api.get_all_described_nodes().await?; //legacy node that did not upgrade the contract bond yet tracing::debug!("Fetched {} total nodes", all_nodes.len()); let mut nodes_to_scrape: HashMap = bonded_nodes diff --git a/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs b/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs index d41d0cedc9..2a55866c48 100644 --- a/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs +++ b/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs @@ -19,7 +19,7 @@ use nym_validator_client::{ use nym_validator_client::{ nym_nodes::{NodeRole, SkimmedNode}, nyxd::{contract_traits::PagedMixnetQueryClient, AccountId}, - NymApiClient, QueryHttpRpcNyxdClient, + QueryHttpRpcNyxdClient, }; use std::{ collections::{HashMap, HashSet}, @@ -111,9 +111,7 @@ impl Monitor { .with_timeout(self.nym_api_client_timeout) .build::<&str>()?; - let api_client = NymApiClient::from(nym_api); - - let described_nodes = api_client + let described_nodes = nym_api .get_all_described_nodes() .await .log_error("get_all_described_nodes")? @@ -135,7 +133,7 @@ impl Monitor { tracing::info!("🟣 🚪 gateway nodes: {}", gateways.len()); - let bonded_nym_nodes = api_client + let bonded_nym_nodes = nym_api .get_all_bonded_nym_nodes() .await? .into_iter() @@ -146,10 +144,11 @@ impl Monitor { tracing::info!("🟣 bonded_nodes: {}", bonded_nym_nodes.len()); // returns only bonded nodes - let nym_nodes = api_client - .get_all_basic_nodes() + let nym_nodes = nym_api + .get_all_basic_nodes_with_metadata() .await - .log_error("get_all_basic_nodes")?; + .log_error("get_all_basic_nodes")? + .nodes; let nym_node_count = nym_nodes.len(); tracing::info!("🟣 get_all_basic_nodes: {}", nym_node_count); @@ -167,8 +166,7 @@ impl Monitor { self.location_cached(node_description).await; } - let mixnodes_detailed = api_client - .nym_api + let mixnodes_detailed = nym_api .get_mixnodes_detailed_unfiltered() .await .log_error("get_mixnodes_detailed_unfiltered")?; @@ -190,15 +188,13 @@ impl Monitor { }) .collect::>(); - let mixnodes_described = api_client - .nym_api + let mixnodes_described = nym_api .get_mixnodes_described() .await .log_error("get_mixnodes_described")?; tracing::info!("🟣 mixnodes_described: {}", mixnodes_described.len()); - let mixing_assigned_nodes = api_client - .nym_api + let mixing_assigned_nodes = nym_api .get_basic_active_mixing_assigned_nodes(false, None, None, false) .await .log_error("get_basic_active_mixing_assigned_nodes")? diff --git a/nym-node/src/node/nym_apis_client.rs b/nym-node/src/node/nym_apis_client.rs index 7e6b490f56..869270f163 100644 --- a/nym-node/src/node/nym_apis_client.rs +++ b/nym-node/src/node/nym_apis_client.rs @@ -87,9 +87,19 @@ impl NymApisClient { if guard.currently_used_api != last_working_endpoint { drop(guard); let mut guard = self.inner.write().await; - let next_url = guard.available_urls[last_working_endpoint].clone(); guard.currently_used_api = last_working_endpoint; - guard.active_client.change_base_urls(vec![next_url.into()]); + + // Provide all URLs starting from the working endpoint for automatic failover + let rotated_urls: Vec<_> = guard + .available_urls + .iter() + .cycle() + .skip(last_working_endpoint) + .take(guard.available_urls.len()) + .map(|u| u.clone().into()) + .collect(); + + guard.active_client.change_base_urls(rotated_urls); } Ok(res) @@ -123,6 +133,8 @@ impl InnerClient { let broadcast_fut = stream::iter(self.available_urls.clone()).for_each_concurrent(None, |url| { let mut nym_api = self.active_client.clone(); + // For broadcast, we intentionally set a single URL per client + // to ensure each endpoint receives the request nym_api.change_base_urls(vec![url.clone().into()]); let req_fut = req(nym_api, request_body); async move { @@ -170,6 +182,8 @@ impl InnerClient { .chain(self.available_urls.iter().enumerate().take(last_working)) { let mut nym_api = self.active_client.clone(); + // For exhaustive query, we test each endpoint individually in sequence + // to find a working one - so single URL is correct here nym_api.change_base_urls(vec![url.clone().into()]); let timeout_fut = sleep(timeout_duration); diff --git a/nym-statistics-api/src/network_view.rs b/nym-statistics-api/src/network_view.rs index ca8352d6b0..04aaff2b7c 100644 --- a/nym-statistics-api/src/network_view.rs +++ b/nym-statistics-api/src/network_view.rs @@ -6,7 +6,6 @@ use nym_task::ShutdownToken; use celes::Country; use nym_validator_client::models::NymNodeDescription; -use nym_validator_client::NymApiClient; use std::collections::HashMap; use std::time::Duration; use std::{net::IpAddr, sync::Arc}; @@ -14,6 +13,8 @@ use tokio::sync::RwLock; use tokio::time::interval; use url::Url; +use nym_http_api_client::Client; +use nym_validator_client::client::NymApiClientExt; use tracing::{error, info, trace, warn}; const NETWORK_CACHE_TTL: Duration = Duration::from_secs(600); @@ -22,7 +23,7 @@ type IpToCountryMap = HashMap>; // SW this should use a proper NS API client once it exists struct NodesQuerier { - client: NymApiClient, + client: Client, } impl NodesQuerier { @@ -99,14 +100,11 @@ impl NetworkRefresher { this } - fn build_http_api_client(url: Url) -> Result { - Ok( - nym_http_api_client::Client::builder::<_, anyhow::Error>(url)? - .no_hickory_dns() - .with_user_agent("node-statistics-api") - .build::()? - .into(), - ) + fn build_http_api_client(url: Url) -> Result { + Ok(Client::builder::<_, anyhow::Error>(url)? + .no_hickory_dns() + .with_user_agent("node-statistics-api") + .build::()?) } async fn refresh_network_nodes(&mut self) -> Result<()> {