cfgsync: add typed config build errors and validation

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<Vec<u16>>,
     hosts: Vec<Host>,
 ) -> HashMap<Host, GeneralConfig> {
+    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<Vec<[u8; 32]>>,
+    da_ports: Option<Vec<u16>>,
+    blend_ports: Option<Vec<u16>>,
+    hosts: Vec<Host>,
+) -> Result<HashMap<Host, GeneralConfig>, 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<Vec<Multiaddr>> = 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]>>) -> Vec<[u8; 32]> {
@@ -172,10 +244,13 @@ fn generate_ids(count: usize, ids: Option<Vec<[u8; 32]>>) -> Vec<[u8; 32]> {
     })
 }
 
-fn resolve_da_ports(count: usize, da_ports: Option<Vec<u16>>) -> Vec<u16> {
-    da_ports.unwrap_or_else(|| {
+fn resolve_da_ports(
+    count: usize,
+    da_ports: Option<Vec<u16>>,
+) -> Result<Vec<u16>, 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<BaseConfigs, NodeConfigBuildError> {
+    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<GeneralApiConfig> {
+fn build_api_configs(hosts: &[Host]) -> Result<Vec<GeneralApiConfig>, 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::<Vec<_>>()
+        })
+        .collect()
 }
 
-fn build_peer_ids(ids: &[[u8; 32]]) -> Vec<PeerId> {
+fn build_peer_ids(ids: &[[u8; 32]]) -> Result<Vec<PeerId>, 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()
 }

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<Vec<ProviderInfo>, 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<ProviderInfo> {
-    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")
 }

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<GeneralConfig>),
     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");
+                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() {

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()
+            }
         },
     )
 }