From 2b237dec1f7941aab461a71c781cfd2e0967407d Mon Sep 17 00:00:00 2001 From: benedettadavico Date: Thu, 30 Apr 2026 08:52:46 +0200 Subject: [PATCH] Remove in-prove remove in-probe test, it isn't needed. --- nym-gateway-probe/src/common/types.rs | 30 +++-- nym-gateway-probe/src/lib.rs | 116 +----------------- nym-gateway-probe/src/run.rs | 2 +- .../src/cli/run_probe.rs | 6 +- .../nym-node-status-api/src/cli/mod.rs | 5 +- .../nym-node-status-api/src/db/models.rs | 100 ++++++++++++++- .../src/db/queries/testruns.rs | 8 +- .../nym-node-status-api/src/db/tests.rs | 95 ++++++++++++++ .../src/http/api/testruns.rs | 14 ++- .../src/ticketbook_manager/mod.rs | 2 - 10 files changed, 233 insertions(+), 145 deletions(-) diff --git a/nym-gateway-probe/src/common/types.rs b/nym-gateway-probe/src/common/types.rs index ce7bba1dce..6750ca5c59 100644 --- a/nym-gateway-probe/src/common/types.rs +++ b/nym-gateway-probe/src/common/types.rs @@ -9,40 +9,38 @@ pub use nym_credentials::ecash::bandwidth::serialiser::VersionedSerialise; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PortsCheckSummary { pub all_pass: bool, - pub failed_ports: Vec, pub error: Option, - pub ports_tested: usize, + pub port_check_target: String, + pub failed_ports: Vec, } impl PortsCheckSummary { - pub fn from_port_map(can_register: bool, ports: &BTreeMap) -> Self { - let failed_ports: Vec = ports + pub fn from_port_map( + can_register: bool, + port_check_target: impl Into, + ports: &BTreeMap, + ) -> Self { + let failed_ports: Vec = ports .iter() - .filter_map(|(port, open)| { - if *open { - None - } else { - port.parse::().ok() - } - }) + .filter_map(|(port, open)| if *open { None } else { Some(port.clone()) }) .collect(); let all_pass = can_register && failed_ports.is_empty() && !ports.is_empty(); Self { all_pass, - failed_ports, error: None, - ports_tested: ports.len(), + port_check_target: port_check_target.into(), + failed_ports, } } - pub fn probe_error(_can_register: bool, message: impl Into) -> Self { + pub fn probe_error(port_check_target: impl Into, message: impl Into) -> Self { Self { all_pass: false, - failed_ports: vec![], error: Some(message.into()), - ports_tested: 0, + port_check_target: port_check_target.into(), + failed_ports: vec![], } } } diff --git a/nym-gateway-probe/src/lib.rs b/nym-gateway-probe/src/lib.rs index 84bbcc907a..2af0971f7e 100644 --- a/nym-gateway-probe/src/lib.rs +++ b/nym-gateway-probe/src/lib.rs @@ -8,7 +8,7 @@ use crate::common::probe_tests::{ do_ping, do_socks5_connectivity_test, lp_registration_probe, wg_probe, }; use crate::common::types::{Entry, LpProbeResults}; -use crate::config::{CredentialArgs, CredentialMode, EXIT_POLICY_PORTS, NetstackArgs, ProbeConfig}; +use crate::config::{CredentialArgs, CredentialMode, NetstackArgs, ProbeConfig}; use nym_authenticator_client::{ AuthClientMixnetListener, AuthClientMixnetListenerHandle, AuthenticatorClient, }; @@ -36,29 +36,6 @@ mod common; pub use common::types; pub mod config; -#[derive(Debug, Clone, Copy)] -pub enum AgentPortsSchedule { - NsAgent { last_ports_check_utc: Option }, -} - -// In-probe fallback TTL for the exit-policy port scan. It only fires -// when the regular probe runs and finds the gateway hasn't been scanned recently. -const PORTS_CHECK_FALLBACK_TTL_SECS: i64 = 14 * 24 * 60 * 60; - -fn exit_policy_ports_check_due(last_ports_check_utc: Option, now: i64) -> bool { - match last_ports_check_utc { - None => true, - Some(ts) => now.saturating_sub(ts) >= PORTS_CHECK_FALLBACK_TTL_SECS, - } -} - -fn unix_timestamp_secs() -> i64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0) -} - pub struct Probe { /// Entry node entry_node: TestedNodeDetails, @@ -335,7 +312,6 @@ impl Probe { pub async fn probe_run_agent( mut self, credential_args: CredentialArgs, - ports_schedule: Option, ) -> anyhow::Result { let storage = Ephemeral::default(); @@ -389,8 +365,7 @@ impl Probe { None }; - self.do_probe_test(mixnet_client, bandwidth_provider, ports_schedule) - .await + self.do_probe_test(mixnet_client, bandwidth_provider).await } /// Run a probe on unannounced gateway(s) some tests will not be available @@ -428,7 +403,7 @@ impl Probe { nym_client_core::init::generate_new_client_keys(&mut rng, key_store).await?; } - self.do_probe_test(None, bandwidth_provider, None).await + self.do_probe_test(None, bandwidth_provider).await } pub async fn probe_run( @@ -496,8 +471,7 @@ impl Probe { None }; - self.do_probe_test(mixnet_client, bandwidth_provider, None) - .await + self.do_probe_test(mixnet_client, bandwidth_provider).await } pub async fn run_ports( @@ -673,7 +647,6 @@ impl Probe { self, mixnet_client: Option>, bandwith_provider: Box, - ports_schedule: Option, ) -> anyhow::Result { // Setup exit node let entry_under_test = self.exit_node.is_none(); @@ -808,87 +781,6 @@ impl Probe { // Add wg results to probe result probe_result.outcome.wg = Some(outcome); - if let Some(AgentPortsSchedule::NsAgent { - last_ports_check_utc, - }) = ports_schedule - { - let now = unix_timestamp_secs(); - if exit_policy_ports_check_due(last_ports_check_utc, now) { - if let Some(ref wg1) = probe_result.outcome.wg { - if wg1.can_register { - info!( - "Running scheduled exit-policy port scan (stale or unset last_ports_check_utc)" - ); - let mut netstack_ports = self.config.netstack_args.clone(); - netstack_ports.port_check_ports = EXIT_POLICY_PORTS.to_vec(); - - let credential2 = bandwith_provider - .get_ecash_ticket(wg_ticket_type, credential_provider, 1) - .await? - .data; - - let mut rng2 = rand::thread_rng(); - let auth_client2 = AuthenticatorClient::new( - mixnet_listener_task.subscribe(), - mixnet_listener_task.mixnet_sender(), - nym_address, - authenticator, - exit_node.authenticator_version, - Arc::new(x25519::KeyPair::new(&mut rng2)), - ip_address, - ); - - match wg_probe( - auth_client2, - ip_address, - exit_node.authenticator_version, - self.config.amnezia_args.clone(), - netstack_ports, - true, - credential2, - ) - .await - { - Ok(scan) => { - if let Some(ref mut wg) = probe_result.outcome.wg { - wg.port_check_results = - scan.port_check_results.clone(); - } - probe_result.ports_check = Some( - match &scan.port_check_results { - Some(m) if !m.is_empty() => { - PortsCheckSummary::from_port_map( - scan.can_register, - m, - ) - } - _ => PortsCheckSummary::probe_error( - scan.can_register, - "exit-policy port scan returned no per-port data", - ), - }, - ); - } - Err(err) => { - warn!("Scheduled exit-policy port scan failed: {err}"); - probe_result.ports_check = - Some(PortsCheckSummary::probe_error( - false, - format!( - "exit-policy port scan failed: {err:#}" - ), - )); - } - } - } - } - } else { - trace!( - "Skipping exit-policy port scan: checked within the last 14 days" - ); - } - } - mixnet_listener_task.stop().await; } else { warn!("Not enough information to run WireGuard via mixnet registration tests"); diff --git a/nym-gateway-probe/src/run.rs b/nym-gateway-probe/src/run.rs index 1619de05fa..06fcbbb1d8 100644 --- a/nym-gateway-probe/src/run.rs +++ b/nym-gateway-probe/src/run.rs @@ -348,7 +348,7 @@ pub(crate) async fn run() -> anyhow::Result { let trial = nym_gateway_probe::Probe::new_for_agent(entry_gateway, network, probe_config) .await?; - Box::pin(trial.probe_run_agent(credential_args, None)) + Box::pin(trial.probe_run_agent(credential_args)) .await .map(ProbeOutput::Standard) } diff --git a/nym-node-status-api/nym-node-status-agent/src/cli/run_probe.rs b/nym-node-status-api/nym-node-status-agent/src/cli/run_probe.rs index c7746b7632..6e4da33cab 100644 --- a/nym-node-status-api/nym-node-status-agent/src/cli/run_probe.rs +++ b/nym-node-status-api/nym-node-status-agent/src/cli/run_probe.rs @@ -1,7 +1,6 @@ use crate::cli::ServerConfig; use crate::cli::common; use crate::log_capture::LogCapture; -use nym_gateway_probe::AgentPortsSchedule; use tracing::instrument; pub(crate) async fn run_probe( @@ -49,10 +48,7 @@ pub(crate) async fn run_probe( // Run the probe, capturing all tracing output log_capture.start(); - let ports_schedule = Some(AgentPortsSchedule::NsAgent { - last_ports_check_utc: testrun.assignment.last_ports_check_utc, - }); - let probe_result = Box::pin(probe.probe_run_agent(credentials_args, ports_schedule)).await?; + let probe_result = Box::pin(probe.probe_run_agent(credentials_args)).await?; let probe_log = log_capture.stop_and_drain(); // Inspect the probe output for socks5 field diff --git a/nym-node-status-api/nym-node-status-api/src/cli/mod.rs b/nym-node-status-api/nym-node-status-api/src/cli/mod.rs index 22b6bdb8a4..fbbd87ce1e 100644 --- a/nym-node-status-api/nym-node-status-api/src/cli/mod.rs +++ b/nym-node-status-api/nym-node-status-api/src/cli/mod.rs @@ -177,12 +177,11 @@ pub(crate) struct TicketbookArgs { pub(crate) quorum_check_interval: Duration, /// Specify types of tickets to buffer - /// one V1WireguardEntry for wg test, additional for lp tests, - /// one V1WireguardExit for the wg exit test, additional for the in-probe exit-policy port scan + /// one V1WireguardEntry for wg test, additional for lp tests #[clap( long, env = "NYM_NODE_STATUS_BUFFERED_TICKET_TYPES", - default_values_t = [TicketType::V1MixnetEntry, TicketType::V1WireguardEntry, TicketType::V1WireguardExit, TicketType::V1WireguardEntry, TicketType::V1WireguardExit] + default_values_t = [TicketType::V1MixnetEntry, TicketType::V1WireguardEntry, TicketType::V1WireguardExit, TicketType::V1WireguardEntry] )] #[arg(value_delimiter = ',')] pub(crate) buffered_ticket_types: Vec, diff --git a/nym-node-status-api/nym-node-status-api/src/db/models.rs b/nym-node-status-api/nym-node-status-api/src/db/models.rs index c739c9b180..4e0b75127b 100644 --- a/nym-node-status-api/nym-node-status-api/src/db/models.rs +++ b/nym-node-status-api/nym-node-status-api/src/db/models.rs @@ -38,6 +38,103 @@ fn strip_wg_port_check_results_from_last_probe(value: &mut serde_json::Value) { wg.remove("port_check_results"); } +fn build_ports_check_summary_json( + can_register: bool, + port_check_target: Option, + ports: Option<&serde_json::Map>, + error: Option, +) -> serde_json::Value { + let failed_ports = ports + .map(|ports| { + ports + .iter() + .filter_map(|(port, open)| open.as_bool().filter(|is_open| !is_open).map(|_| port)) + .cloned() + .collect::>() + }) + .unwrap_or_default(); + + let has_ports = ports.is_some_and(|p| !p.is_empty()); + let all_pass = can_register && failed_ports.is_empty() && has_ports; + + serde_json::json!({ + "all_pass": all_pass, + "error": error, + "port_check_target": port_check_target, + "failed_ports": failed_ports, + }) +} + +pub(crate) fn ports_check_summary_json_from_result( + result: &nym_gateway_probe::PortCheckResult, +) -> serde_json::Value { + let ports = result + .ports + .iter() + .map(|(k, v)| (k.clone(), serde_json::Value::Bool(*v))) + .collect::>(); + + build_ports_check_summary_json( + result.can_register, + Some(result.port_check_target.clone()), + Some(&ports), + result.error.clone(), + ) +} + +pub(crate) fn normalize_ports_check_payload(value: serde_json::Value) -> Option { + let serde_json::Value::Object(map) = value else { + return None; + }; + + // New shape is already in place; pass through untouched. + if map.contains_key("all_pass") + && map.contains_key("error") + && map.contains_key("port_check_target") + && map.contains_key("failed_ports") + { + return Some(serde_json::Value::Object(map)); + } + + // Legacy dedicated shape: { gateway, can_register, port_check_target, ports, error } + if map.contains_key("can_register") && map.contains_key("ports") { + let can_register = map + .get("can_register") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + let port_check_target = map + .get("port_check_target") + .and_then(serde_json::Value::as_str) + .map(ToOwned::to_owned); + let ports = map.get("ports").and_then(serde_json::Value::as_object); + let error = map + .get("error") + .and_then(serde_json::Value::as_str) + .map(ToOwned::to_owned); + + return Some(build_ports_check_summary_json( + can_register, + port_check_target, + ports, + error, + )); + } + + if map.contains_key("all_pass") && map.contains_key("failed_ports") { + let mut normalized = map; + normalized.remove("ports_tested"); + normalized + .entry("port_check_target".to_string()) + .or_insert(serde_json::Value::Null); + normalized + .entry("error".to_string()) + .or_insert(serde_json::Value::Null); + return Some(serde_json::Value::Object(normalized)); + } + + Some(serde_json::Value::Object(map)) +} + macro_rules! serialize_opt_to_value { ($var:expr) => {{ match $var { @@ -119,7 +216,8 @@ impl TryFrom for http::models::Gateway { .ports_check .clone() .or(nested_ports) - .filter(|v| !v.is_null()); + .filter(|v| !v.is_null()) + .and_then(normalize_ports_check_payload); let last_ports_check_utc = value .last_ports_check_utc .map(unix_timestamp_to_utc_rfc3339); diff --git a/nym-node-status-api/nym-node-status-api/src/db/queries/testruns.rs b/nym-node-status-api/nym-node-status-api/src/db/queries/testruns.rs index dd1a582923..5cf1122fa3 100644 --- a/nym-node-status-api/nym-node-status-api/src/db/queries/testruns.rs +++ b/nym-node-status-api/nym-node-status-api/src/db/queries/testruns.rs @@ -231,9 +231,10 @@ pub(crate) async fn update_gateway_ports_check_only( gateway_pk: i32, port_check_result: &nym_gateway_probe::PortCheckResult, ) -> anyhow::Result<()> { + use crate::db::models::ports_check_summary_json_from_result; + let now_ts = now_utc().unix_timestamp(); - let value = serde_json::to_value(port_check_result) - .map_err(|e| anyhow::anyhow!("Invalid port_check_result JSON: {e}"))?; + let value = ports_check_summary_json_from_result(port_check_result); sqlx::query( r#"UPDATE gateways SET @@ -292,8 +293,7 @@ pub(crate) async fn insert_external_ports_check_testrun( pub(crate) async fn enqueue_due_ports_check_testruns(db: &DbPool) -> anyhow::Result { let mut conn = db.acquire().await?; let now = now_utc().unix_timestamp(); - // 14 days soft TTL — must match the in-probe fallback TTL in - // `nym_gateway_probe::exit_policy_ports_check_due`. + // 14 days soft TTL for dedicated ports-check queueing. let cutoff = now - time::Duration::days(14).whole_seconds(); let res = sqlx::query!( diff --git a/nym-node-status-api/nym-node-status-api/src/db/tests.rs b/nym-node-status-api/nym-node-status-api/src/db/tests.rs index 28df2db76e..59d322f1a3 100644 --- a/nym-node-status-api/nym-node-status-api/src/db/tests.rs +++ b/nym-node-status-api/nym-node-status-api/src/db/tests.rs @@ -76,6 +76,101 @@ mod db_tests { assert_eq!(http_gateway.last_probe_result, Some(probe)); } + #[test] + fn test_gateway_dto_normalizes_legacy_dedicated_ports_check_shape() { + let legacy_ports = serde_json::json!({ + "gateway": "gw1", + "can_register": true, + "port_check_target": "portquiz.net", + "ports": { + "20": false, + "21": true, + "43": false + }, + "error": null + }); + let gateway_dto = crate::db::models::GatewayDto { + gateway_identity_key: "id1".to_string(), + bonded: true, + performance: 50, + self_described: Some("{}".to_string()), + explorer_pretty_bond: Some("{}".to_string()), + last_probe_result: None, + last_probe_log: None, + ports_check: Some(legacy_ports), + last_ports_check_utc: Some(1672531200), + last_testrun_utc: None, + last_updated_utc: 1672531200, + moniker: "m".to_string(), + security_contact: "c".to_string(), + details: "d".to_string(), + website: "w".to_string(), + bridges: None, + }; + + let http_gateway: crate::http::models::Gateway = gateway_dto.try_into().unwrap(); + let normalized = http_gateway.ports_check.unwrap(); + + assert_eq!( + normalized.get("all_pass"), + Some(&serde_json::Value::Bool(false)) + ); + assert_eq!( + normalized.get("port_check_target"), + Some(&serde_json::Value::String("portquiz.net".to_string())) + ); + assert_eq!(normalized.get("error"), Some(&serde_json::Value::Null)); + assert_eq!( + normalized.get("failed_ports"), + Some(&serde_json::json!(["20", "43"])) + ); + assert!(normalized.get("gateway").is_none()); + assert!(normalized.get("ports").is_none()); + } + + #[test] + fn test_gateway_dto_normalizes_legacy_in_probe_summary_shape() { + let legacy_summary = serde_json::json!({ + "all_pass": true, + "failed_ports": [], + "error": null, + "ports_tested": 32 + }); + let gateway_dto = crate::db::models::GatewayDto { + gateway_identity_key: "id2".to_string(), + bonded: true, + performance: 50, + self_described: Some("{}".to_string()), + explorer_pretty_bond: Some("{}".to_string()), + last_probe_result: None, + last_probe_log: None, + ports_check: Some(legacy_summary), + last_ports_check_utc: Some(1672531200), + last_testrun_utc: None, + last_updated_utc: 1672531200, + moniker: "m".to_string(), + security_contact: "c".to_string(), + details: "d".to_string(), + website: "w".to_string(), + bridges: None, + }; + + let http_gateway: crate::http::models::Gateway = gateway_dto.try_into().unwrap(); + let normalized = http_gateway.ports_check.unwrap(); + + assert_eq!( + normalized.get("all_pass"), + Some(&serde_json::Value::Bool(true)) + ); + assert_eq!(normalized.get("failed_ports"), Some(&serde_json::json!([]))); + assert_eq!(normalized.get("error"), Some(&serde_json::Value::Null)); + assert_eq!( + normalized.get("port_check_target"), + Some(&serde_json::Value::Null) + ); + assert!(normalized.get("ports_tested").is_none()); + } + #[test] fn test_mixnode_dto_try_from() { let mixnode_dto = crate::db::models::MixnodeDto { diff --git a/nym-node-status-api/nym-node-status-api/src/http/api/testruns.rs b/nym-node-status-api/nym-node-status-api/src/http/api/testruns.rs index 45563b1a61..fdf51521e7 100644 --- a/nym-node-status-api/nym-node-status-api/src/http/api/testruns.rs +++ b/nym-node-status-api/nym-node-status-api/src/http/api/testruns.rs @@ -1,5 +1,5 @@ use crate::db::DbConnection; -use crate::db::models::{TestRunDto, TestRunStatus}; +use crate::db::models::{TestRunDto, TestRunKind, TestRunStatus}; use crate::db::queries; use crate::utils::{now_utc, unix_timestamp_to_utc_rfc3339}; use crate::{ @@ -422,6 +422,18 @@ async fn submit_ports_check_testrun_v2( match queries::testruns::get_testrun_by_id(&mut conn, submitted_testrun_id).await { Ok(testrun) => { + if testrun.kind != TestRunKind::PortsCheck as i16 { + tracing::warn!( + "Testrun {} has wrong kind for ports-check submit: {}", + submitted_testrun_id, + testrun.kind + ); + return Err(HttpError::invalid_input(format!( + "Testrun {} is not a ports-check run", + submitted_testrun_id + ))); + } + let gw_identity = queries::select_gateway_identity(&mut conn, testrun.gateway_id) .await .map_err(HttpError::internal_with_logging)?; diff --git a/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/mod.rs b/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/mod.rs index 16d7bbb852..0a7f6c8e11 100644 --- a/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/mod.rs +++ b/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/mod.rs @@ -53,8 +53,6 @@ impl Default for TicketbookManagerConfig { V1WireguardExit, // additional wg entry for lp tests V1WireguardEntry, - // additional wg exit for the in-probe exit-policy port scan - V1WireguardExit, ], } }