From f0e9d2807b89aa33f6e90e81506cea3d0e006e03 Mon Sep 17 00:00:00 2001 From: andrussal Date: Thu, 18 Dec 2025 14:46:27 +0100 Subject: [PATCH] cfgsync: add typed config build errors and validation --- .../tools/cfgsync/src/config/builder.rs | 179 +++++++++++++----- .../tools/cfgsync/src/config/providers.rs | 119 ++++++++---- testing-framework/tools/cfgsync/src/repo.rs | 34 +++- testing-framework/tools/cfgsync/src/server.rs | 6 + 4 files changed, 252 insertions(+), 86 deletions(-) diff --git a/testing-framework/tools/cfgsync/src/config/builder.rs b/testing-framework/tools/cfgsync/src/config/builder.rs index 9fc3375..22f779e 100644 --- a/testing-framework/tools/cfgsync/src/config/builder.rs +++ b/testing-framework/tools/cfgsync/src/config/builder.rs @@ -15,17 +15,20 @@ use testing_framework_config::topology::configs::{ consensus, consensus::{ConsensusParams, create_consensus_configs, create_genesis_tx_with_declarations}, da, - da::{DaParams, create_da_configs}, + da::{DaParams, try_create_da_configs}, network, network::{NetworkParams, create_network_configs}, time::default_time_config, wallet::WalletConfig, }; +use thiserror::Error; use crate::{ config::{ - kms::create_kms_configs, providers::create_providers, tracing::update_tracing_identifier, - validation::validate_inputs, + kms::create_kms_configs, + providers::{ProviderBuildError, try_create_providers}, + tracing::update_tracing_identifier, + validation::{ValidationError, validate_inputs}, }, host::{Host, HostKind, sort_hosts}, network::rewrite_initial_peers, @@ -42,6 +45,54 @@ pub fn create_node_configs( blend_ports: Option>, hosts: Vec, ) -> HashMap { + try_create_node_configs( + consensus_params, + da_params, + tracing_settings, + wallet_config, + ids, + da_ports, + blend_ports, + hosts, + ) + .expect("failed to build cfgsync node configs") +} + +#[derive(Debug, Error)] +pub enum NodeConfigBuildError { + #[error(transparent)] + Validation(#[from] ValidationError), + #[error(transparent)] + Providers(#[from] ProviderBuildError), + #[error(transparent)] + Da(#[from] da::DaConfigError), + #[error("failed to allocate an available UDP port")] + PortAllocFailed, + #[error("failed to parse multiaddr '{value}': {message}")] + InvalidMultiaddr { value: String, message: String }, + #[error("failed to parse socket addr '{value}': {source}")] + InvalidSocketAddr { + value: String, + source: std::net::AddrParseError, + }, + #[error("failed to decode ed25519 secret key bytes")] + InvalidEd25519SecretKey, + #[error("config generation requires at least one consensus config")] + MissingConsensusConfig, + #[error("host/config length mismatch")] + HostConfigLenMismatch, +} + +pub fn try_create_node_configs( + consensus_params: &ConsensusParams, + da_params: &DaParams, + tracing_settings: &TracingSettings, + wallet_config: &WalletConfig, + ids: Option>, + da_ports: Option>, + blend_ports: Option>, + hosts: Vec, +) -> Result, NodeConfigBuildError> { let hosts = sort_hosts(hosts); validate_inputs( @@ -50,11 +101,10 @@ pub fn create_node_configs( ids.as_ref(), da_ports.as_ref(), blend_ports.as_ref(), - ) - .expect("invalid cfgsync inputs"); + )?; let ids = generate_ids(consensus_params.n_participants, ids); - let ports = resolve_da_ports(consensus_params.n_participants, da_ports); + let ports = resolve_da_ports(consensus_params.n_participants, da_ports)?; let blend_ports = resolve_blend_ports(&hosts, blend_ports); let BaseConfigs { @@ -70,9 +120,9 @@ pub fn create_node_configs( &ids, &ports, &blend_ports, - ); + )?; - let api_configs = build_api_configs(&hosts); + let api_configs = build_api_configs(&hosts)?; let mut configured_hosts = HashMap::new(); let initial_peer_templates: Vec> = network_configs @@ -83,7 +133,7 @@ pub fn create_node_configs( .iter() .map(|cfg| cfg.backend.swarm.port) .collect(); - let peer_ids = build_peer_ids(&ids); + let peer_ids = build_peer_ids(&ids)?; let host_network_init_peers = rewrite_initial_peers( &initial_peer_templates, @@ -92,13 +142,12 @@ pub fn create_node_configs( &peer_ids, ); - let providers = create_providers(&hosts, &consensus_configs, &blend_configs, &da_configs); + let providers = try_create_providers(&hosts, &consensus_configs, &blend_configs, &da_configs)?; - let ledger_tx = consensus_configs[0] - .genesis_tx - .mantle_tx() - .ledger_tx - .clone(); + let first_consensus = consensus_configs + .get(0) + .ok_or(NodeConfigBuildError::MissingConsensusConfig)?; + let ledger_tx = first_consensus.genesis_tx.mantle_tx().ledger_tx.clone(); let genesis_tx = create_genesis_tx_with_declarations(ledger_tx, providers); for c in &mut consensus_configs { @@ -108,15 +157,29 @@ pub fn create_node_configs( let kms_configs = create_kms_configs(&blend_configs, &da_configs); for (i, host) in hosts.into_iter().enumerate() { + if i >= consensus_configs.len() + || i >= api_configs.len() + || i >= da_configs.len() + || i >= network_configs.len() + || i >= blend_configs.len() + || i >= host_network_init_peers.len() + || i >= kms_configs.len() + || i >= bootstrap_configs.len() + { + return Err(NodeConfigBuildError::HostConfigLenMismatch); + } + let consensus_config = consensus_configs[i].clone(); let api_config = api_configs[i].clone(); let mut da_config = da_configs[i].clone(); - da_config.listening_address = Multiaddr::from_str(&format!( - "/ip4/0.0.0.0/udp/{}/quic-v1", - host.da_network_port, - )) - .unwrap(); + let da_addr_value = format!("/ip4/0.0.0.0/udp/{}/quic-v1", host.da_network_port); + da_config.listening_address = Multiaddr::from_str(&da_addr_value).map_err(|source| { + NodeConfigBuildError::InvalidMultiaddr { + value: da_addr_value, + message: source.to_string(), + } + })?; if matches!(host.kind, HostKind::Validator) { da_config.policy_settings.min_dispersal_peers = 0; } @@ -125,17 +188,26 @@ pub fn create_node_configs( network_config.backend.swarm.host = Ipv4Addr::from_str("0.0.0.0").unwrap(); network_config.backend.swarm.port = host.network_port; network_config.backend.initial_peers = host_network_init_peers[i].clone(); + let nat_value = format!("/ip4/{}/udp/{}/quic-v1", host.ip, host.network_port); + let nat_addr = Multiaddr::from_str(&nat_value).map_err(|source| { + NodeConfigBuildError::InvalidMultiaddr { + value: nat_value, + message: source.to_string(), + } + })?; network_config.backend.swarm.nat_config = nomos_libp2p::NatSettings::Static { - external_address: Multiaddr::from_str(&format!( - "/ip4/{}/udp/{}/quic-v1", - host.ip, host.network_port - )) - .unwrap(), + external_address: nat_addr, }; let mut blend_config = blend_configs[i].clone(); + let blend_value = format!("/ip4/0.0.0.0/udp/{}/quic-v1", host.blend_port); blend_config.backend_core.listening_address = - Multiaddr::from_str(&format!("/ip4/0.0.0.0/udp/{}/quic-v1", host.blend_port)).unwrap(); + Multiaddr::from_str(&blend_value).map_err(|source| { + NodeConfigBuildError::InvalidMultiaddr { + value: blend_value, + message: source.to_string(), + } + })?; let tracing_config = update_tracing_identifier(tracing_settings.clone(), host.identifier.clone()); @@ -157,7 +229,7 @@ pub fn create_node_configs( ); } - configured_hosts + Ok(configured_hosts) } fn generate_ids(count: usize, ids: Option>) -> Vec<[u8; 32]> { @@ -172,10 +244,13 @@ fn generate_ids(count: usize, ids: Option>) -> Vec<[u8; 32]> { }) } -fn resolve_da_ports(count: usize, da_ports: Option>) -> Vec { - da_ports.unwrap_or_else(|| { +fn resolve_da_ports( + count: usize, + da_ports: Option>, +) -> Result, NodeConfigBuildError> { + da_ports.map(Ok).unwrap_or_else(|| { (0..count) - .map(|_| get_available_udp_port().unwrap()) + .map(|_| get_available_udp_port().ok_or(NodeConfigBuildError::PortAllocFailed)) .collect() }) } @@ -191,35 +266,49 @@ fn build_base_configs( ids: &[[u8; 32]], da_ports: &[u16], blend_ports: &[u16], -) -> BaseConfigs { - BaseConfigs { +) -> Result { + Ok(BaseConfigs { consensus_configs: create_consensus_configs(ids, consensus_params, wallet_config), bootstrap_configs: create_bootstrap_configs(ids, SHORT_PROLONGED_BOOTSTRAP_PERIOD), - da_configs: create_da_configs(ids, da_params, da_ports), + da_configs: try_create_da_configs(ids, da_params, da_ports)?, network_configs: create_network_configs(ids, &NetworkParams::default()), blend_configs: create_blend_configs(ids, blend_ports), - } + }) } -fn build_api_configs(hosts: &[Host]) -> Vec { +fn build_api_configs(hosts: &[Host]) -> Result, NodeConfigBuildError> { hosts .iter() - .map(|host| GeneralApiConfig { - address: format!("0.0.0.0:{}", host.api_port).parse().unwrap(), - testing_http_address: format!("0.0.0.0:{}", host.testing_http_port) - .parse() - .unwrap(), + .map(|host| { + let address_value = format!("0.0.0.0:{}", host.api_port); + let testing_value = format!("0.0.0.0:{}", host.testing_http_port); + Ok(GeneralApiConfig { + address: address_value.parse().map_err(|source| { + NodeConfigBuildError::InvalidSocketAddr { + value: address_value, + source, + } + })?, + testing_http_address: testing_value.parse().map_err(|source| { + NodeConfigBuildError::InvalidSocketAddr { + value: testing_value, + source, + } + })?, + }) }) - .collect::>() + .collect() } -fn build_peer_ids(ids: &[[u8; 32]]) -> Vec { +fn build_peer_ids(ids: &[[u8; 32]]) -> Result, NodeConfigBuildError> { ids.iter() .map(|bytes| { let mut key_bytes = *bytes; - let secret = - ed25519::SecretKey::try_from_bytes(&mut key_bytes).expect("valid ed25519 key"); - PeerId::from_public_key(&ed25519::Keypair::from(secret).public().into()) + let secret = ed25519::SecretKey::try_from_bytes(&mut key_bytes) + .map_err(|_| NodeConfigBuildError::InvalidEd25519SecretKey)?; + Ok(PeerId::from_public_key( + &ed25519::Keypair::from(secret).public().into(), + )) }) .collect() } diff --git a/testing-framework/tools/cfgsync/src/config/providers.rs b/testing-framework/tools/cfgsync/src/config/providers.rs index b61345f..40ae3c2 100644 --- a/testing-framework/tools/cfgsync/src/config/providers.rs +++ b/testing-framework/tools/cfgsync/src/config/providers.rs @@ -7,47 +7,98 @@ use testing_framework_config::topology::configs::{ consensus::{GeneralConsensusConfig, ProviderInfo}, da::GeneralDaConfig, }; +use thiserror::Error; use crate::host::Host; +#[derive(Debug, Error)] +pub enum ProviderBuildError { + #[error("consensus configs are empty")] + MissingConsensusConfigs, + #[error( + "config length mismatch (hosts={hosts}, da_configs={da_configs}, blend_configs={blend_configs})" + )] + HostConfigLenMismatch { + hosts: usize, + da_configs: usize, + blend_configs: usize, + }, + #[error("consensus notes length mismatch (da_notes={da_notes}, blend_notes={blend_notes})")] + NoteLenMismatch { da_notes: usize, blend_notes: usize }, + #[error("failed to parse multiaddr '{value}': {message}")] + InvalidMultiaddr { value: String, message: String }, +} + +pub fn try_create_providers( + hosts: &[Host], + consensus_configs: &[GeneralConsensusConfig], + blend_configs: &[GeneralBlendConfig], + da_configs: &[GeneralDaConfig], +) -> Result, ProviderBuildError> { + let first = consensus_configs + .first() + .ok_or(ProviderBuildError::MissingConsensusConfigs)?; + + if hosts.len() != da_configs.len() || hosts.len() != blend_configs.len() { + return Err(ProviderBuildError::HostConfigLenMismatch { + hosts: hosts.len(), + da_configs: da_configs.len(), + blend_configs: blend_configs.len(), + }); + } + if first.da_notes.len() < da_configs.len() || first.blend_notes.len() < blend_configs.len() { + return Err(ProviderBuildError::NoteLenMismatch { + da_notes: first.da_notes.len(), + blend_notes: first.blend_notes.len(), + }); + } + + let mut providers = Vec::with_capacity(da_configs.len() + blend_configs.len()); + + for (i, da_conf) in da_configs.iter().enumerate() { + let value = format!( + "/ip4/{}/udp/{}/quic-v1", + hosts[i].ip, hosts[i].da_network_port + ); + let locator = + Multiaddr::from_str(&value).map_err(|source| ProviderBuildError::InvalidMultiaddr { + value, + message: source.to_string(), + })?; + providers.push(ProviderInfo { + service_type: ServiceType::DataAvailability, + provider_sk: da_conf.signer.clone(), + zk_sk: da_conf.secret_zk_key.clone(), + locator: Locator(locator), + note: first.da_notes[i].clone(), + }); + } + + for (i, blend_conf) in blend_configs.iter().enumerate() { + let value = format!("/ip4/{}/udp/{}/quic-v1", hosts[i].ip, hosts[i].blend_port); + let locator = + Multiaddr::from_str(&value).map_err(|source| ProviderBuildError::InvalidMultiaddr { + value, + message: source.to_string(), + })?; + providers.push(ProviderInfo { + service_type: ServiceType::BlendNetwork, + provider_sk: blend_conf.signer.clone(), + zk_sk: blend_conf.secret_zk_key.clone(), + locator: Locator(locator), + note: first.blend_notes[i].clone(), + }); + } + + Ok(providers) +} + pub fn create_providers( hosts: &[Host], consensus_configs: &[GeneralConsensusConfig], blend_configs: &[GeneralBlendConfig], da_configs: &[GeneralDaConfig], ) -> Vec { - let mut providers: Vec<_> = da_configs - .iter() - .enumerate() - .map(|(i, da_conf)| ProviderInfo { - service_type: ServiceType::DataAvailability, - provider_sk: da_conf.signer.clone(), - zk_sk: da_conf.secret_zk_key.clone(), - locator: Locator( - Multiaddr::from_str(&format!( - "/ip4/{}/udp/{}/quic-v1", - hosts[i].ip, hosts[i].da_network_port - )) - .unwrap(), - ), - note: consensus_configs[0].da_notes[i].clone(), - }) - .collect(); - providers.extend(blend_configs.iter().enumerate().map(|(i, blend_conf)| { - ProviderInfo { - service_type: ServiceType::BlendNetwork, - provider_sk: blend_conf.signer.clone(), - zk_sk: blend_conf.secret_zk_key.clone(), - locator: Locator( - Multiaddr::from_str(&format!( - "/ip4/{}/udp/{}/quic-v1", - hosts[i].ip, hosts[i].blend_port - )) - .unwrap(), - ), - note: consensus_configs[0].blend_notes[i].clone(), - } - })); - - providers + try_create_providers(hosts, consensus_configs, blend_configs, da_configs) + .expect("failed to build providers") } diff --git a/testing-framework/tools/cfgsync/src/repo.rs b/testing-framework/tools/cfgsync/src/repo.rs index 0398c95..d59295b 100644 --- a/testing-framework/tools/cfgsync/src/repo.rs +++ b/testing-framework/tools/cfgsync/src/repo.rs @@ -9,14 +9,16 @@ use testing_framework_config::topology::configs::{ GeneralConfig, consensus::ConsensusParams, da::DaParams, wallet::WalletConfig, }; use tokio::{sync::oneshot::Sender, time::timeout}; +use tracing::{error, info, warn}; -use crate::{config::builder::create_node_configs, host::Host, server::CfgSyncConfig}; +use crate::{config::builder::try_create_node_configs, host::Host, server::CfgSyncConfig}; const HOST_POLLING_INTERVAL: Duration = Duration::from_secs(1); pub enum RepoResponse { Config(Box), Timeout, + Error(String), } pub struct ConfigRepo { @@ -102,12 +104,12 @@ impl ConfigRepo { .await .is_ok() { - println!("All hosts have announced their IPs"); + info!("all hosts have announced their IPs"); let mut waiting_hosts = self.waiting_hosts.lock().unwrap(); let hosts = waiting_hosts.keys().cloned().collect(); - let configs = create_node_configs( + let configs = match try_create_node_configs( &self.consensus_params, &self.da_params, &self.tracing_settings, @@ -116,14 +118,32 @@ impl ConfigRepo { self.da_ports.clone(), self.blend_ports.clone(), hosts, - ); + ) { + Ok(configs) => configs, + Err(err) => { + error!(error = %err, "failed to generate node configs"); + let message = err.to_string(); + for (_, sender) in waiting_hosts.drain() { + let _ = sender.send(RepoResponse::Error(message.clone())); + } + return; + } + }; for (host, sender) in waiting_hosts.drain() { - let config = configs.get(&host).expect("host should have a config"); - let _ = sender.send(RepoResponse::Config(Box::new(config.to_owned()))); + match configs.get(&host) { + Some(config) => { + let _ = sender.send(RepoResponse::Config(Box::new(config.to_owned()))); + } + None => { + warn!(identifier = %host.identifier, "missing config for host"); + let _ = + sender.send(RepoResponse::Error("missing config for host".to_string())); + } + } } } else { - println!("Timeout: Not all hosts announced within the time limit"); + warn!("timeout: not all hosts announced within the time limit"); let mut waiting_hosts = self.waiting_hosts.lock().unwrap(); for (_, sender) in waiting_hosts.drain() { diff --git a/testing-framework/tools/cfgsync/src/server.rs b/testing-framework/tools/cfgsync/src/server.rs index caba070..358cf38 100644 --- a/testing-framework/tools/cfgsync/src/server.rs +++ b/testing-framework/tools/cfgsync/src/server.rs @@ -190,6 +190,9 @@ async fn validator_config( (StatusCode::OK, Json(value)).into_response() } RepoResponse::Timeout => (StatusCode::REQUEST_TIMEOUT).into_response(), + RepoResponse::Error(message) => { + (StatusCode::INTERNAL_SERVER_ERROR, message).into_response() + } }, ) } @@ -233,6 +236,9 @@ async fn executor_config( (StatusCode::OK, Json(value)).into_response() } RepoResponse::Timeout => (StatusCode::REQUEST_TIMEOUT).into_response(), + RepoResponse::Error(message) => { + (StatusCode::INTERNAL_SERVER_ERROR, message).into_response() + } }, ) }