chore:(PRO-259) Add additional Validation Checks (#209)

* chore/Warn when 'All' is used for allowed_spl_paid_tokens

Adds a warning if 'All' is set for allowed_spl_paid_tokens in the config, alerting users to potential volatility and security risks. Updates tests to verify the warning is triggered.

* Update coverage badge [skip ci]

* wip: Add signers config validation to CLI and core

Introduces optional signers configuration validation to CLI commands and RPC server startup. Adds `SignerValidator` for detailed validation and warnings, updates `ConfigValidator` to support signers config, and enforces non-zero weights for weighted strategy. Improves error and warning reporting for signer configuration issues.

* Update coverage badge [skip ci]

* refactor

* Update coverage badge [skip ci]

* chore: Increase test setup timeout to 90 seconds

Extended the timeout for test setup from 30,000ms to 90,000ms to allow more time for airdrops and token initialization in integration tests.

* chore: add strategy display impl

---------

Co-authored-by: github-actions <github-actions@github.com>

Author: amilz

.github/badges/coverage.json

@@ -1 +1 @@
-{"schemaVersion": 1, "label": "coverage", "message": "86.2%", "color": "green"}
+{"schemaVersion": 1, "label": "coverage", "message": "86.3%", "color": "green"}

crates/cli/src/main.rs

@@ -43,9 +43,17 @@ enum Commands {
 #[derive(Subcommand)]
 enum ConfigCommands {
     /// Validate configuration file (fast, no RPC calls)
-    Validate,
+    Validate {
+        /// Path to signers configuration file (optional)
+        #[arg(long)]
+        signers_config: Option<std::path::PathBuf>,
+    },
     /// Validate configuration file with RPC validation (slower but more thorough)
-    ValidateWithRpc,
+    ValidateWithRpc {
+        /// Path to signers configuration file (optional)
+        #[arg(long)]
+        signers_config: Option<std::path::PathBuf>,
+    },
 }
 
 #[derive(Subcommand)]
@@ -116,22 +124,47 @@ async fn main() -> Result<(), KoraError> {
     match cli.command {
         Some(Commands::Config { config_command }) => {
             match config_command {
-                ConfigCommands::Validate => {
-                    let _ = ConfigValidator::validate_with_result(rpc_client.as_ref(), true).await;
+                ConfigCommands::Validate { signers_config } => {
+                    let _ = ConfigValidator::validate_with_result_and_signers(
+                        rpc_client.as_ref(),
+                        true,
+                        signers_config.as_ref(),
+                    )
+                    .await;
                 }
-                ConfigCommands::ValidateWithRpc => {
-                    let _ = ConfigValidator::validate_with_result(rpc_client.as_ref(), false).await;
+                ConfigCommands::ValidateWithRpc { signers_config } => {
+                    let _ = ConfigValidator::validate_with_result_and_signers(
+                        rpc_client.as_ref(),
+                        false,
+                        signers_config.as_ref(),
+                    )
+                    .await;
                 }
             }
             std::process::exit(0);
         }
         Some(Commands::Rpc { rpc_command }) => {
             match rpc_command {
                 RpcCommands::Start { rpc_args } => {
-                    // Validate config before starting server
-                    if let Err(e) = ConfigValidator::validate(rpc_client.as_ref()).await {
-                        print_error(&format!("Config validation failed: {e}"));
-                        std::process::exit(1);
+                    // Validate config and signers before starting server
+                    match ConfigValidator::validate_with_result_and_signers(
+                        rpc_client.as_ref(),
+                        true,
+                        rpc_args.signers_config.as_ref(),
+                    )
+                    .await
+                    {
+                        Err(errors) => {
+                            for e in errors {
+                                print_error(&format!("Validation error: {e}"));
+                            }
+                            std::process::exit(1);
+                        }
+                        Ok(warnings) => {
+                            for w in warnings {
+                                println!("Warning: {w}");
+                            }
+                        }
                     }
 
                     setup_logging(&rpc_args.logging_format);

crates/lib/src/signer/config.rs

@@ -10,7 +10,7 @@ use crate::{
     },
 };
 use serde::{Deserialize, Serialize};
-use std::{fs, path::Path};
+use std::{fmt, fs, path::Path};
 
 /// Configuration for a pool of signers
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -38,6 +38,17 @@ pub enum SelectionStrategy {
     Weighted,
 }
 
+impl fmt::Display for SelectionStrategy {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let s = match self {
+            SelectionStrategy::RoundRobin => "round_robin",
+            SelectionStrategy::Random => "random",
+            SelectionStrategy::Weighted => "weighted",
+        };
+        write!(f, "{s}")
+    }
+}
+
 fn default_strategy() -> SelectionStrategy {
     SelectionStrategy::RoundRobin
 }
@@ -99,18 +110,31 @@ impl SignerPoolConfig {
 
     /// Validate the signer pool configuration
     pub fn validate_signer_config(&self) -> Result<(), KoraError> {
-        if self.signers.is_empty() {
-            return Err(KoraError::ValidationError(
-                "At least one signer must be configured".to_string(),
-            ));
-        }
+        // Validate that at least one signer is configured
+        self.validate_signer_not_empty()?;
 
         // Validate each signer configuration
         for (index, signer) in self.signers.iter().enumerate() {
             signer.validate_individual_signer_config(index)?;
         }
 
-        // Check for duplicate names
+        self.validate_signer_names()?;
+
+        self.validate_strategy_weights()?;
+
+        Ok(())
+    }
+
+    pub fn validate_signer_not_empty(&self) -> Result<(), KoraError> {
+        if self.signers.is_empty() {
+            return Err(KoraError::ValidationError(
+                "At least one signer must be configured".to_string(),
+            ));
+        }
+        Ok(())
+    }
+
+    pub fn validate_signer_names(&self) -> Result<(), KoraError> {
         let mut names = std::collections::HashSet::new();
         for signer in &self.signers {
             if !names.insert(&signer.name) {
@@ -120,7 +144,22 @@ impl SignerPoolConfig {
                 )));
             }
         }
+        Ok(())
+    }
 
+    pub fn validate_strategy_weights(&self) -> Result<(), KoraError> {
+        if matches!(self.signer_pool.strategy, SelectionStrategy::Weighted) {
+            for signer in &self.signers {
+                if let Some(weight) = signer.weight {
+                    if weight == 0 {
+                        return Err(KoraError::ValidationError(format!(
+                            "Signer '{}' has weight of 0 in weighted strategy",
+                            signer.name
+                        )));
+                    }
+                }
+            }
+        }
         Ok(())
     }
 }
@@ -145,7 +184,7 @@ impl SignerConfig {
     }
 
     /// Validate an individual signer configuration
-    fn validate_individual_signer_config(&self, index: usize) -> Result<(), KoraError> {
+    pub fn validate_individual_signer_config(&self, index: usize) -> Result<(), KoraError> {
         if self.name.is_empty() {
             return Err(KoraError::ValidationError(format!(
                 "Signer at index {index} must have a non-empty name"
@@ -230,6 +269,7 @@ weight = 2
         };
 
         assert!(config.validate_signer_config().is_ok());
+        assert!(config.validate_strategy_weights().is_ok());
     }
 
     #[test]

crates/lib/src/validator/config_validator.rs

@@ -1,13 +1,17 @@
-use std::str::FromStr;
+use std::{path::Path, str::FromStr};
 
 use crate::{
     admin::token_util::find_missing_atas,
-    config::Token2022Config,
+    config::{SplTokenConfig, Token2022Config},
     fee::price::PriceModel,
     oracle::PriceSource,
+    signer::SignerPoolConfig,
     state::get_config,
     token::{spl_token_2022_util, token::TokenUtil},
-    validator::account_validator::{validate_account, AccountType},
+    validator::{
+        account_validator::{validate_account, AccountType},
+        signer_validator::SignerValidator,
+    },
     KoraError,
 };
 use solana_client::nonblocking::rpc_client::RpcClient;
@@ -41,6 +45,14 @@ impl ConfigValidator {
     pub async fn validate_with_result(
         rpc_client: &RpcClient,
         skip_rpc_validation: bool,
+    ) -> Result<Vec<String>, Vec<String>> {
+        Self::validate_with_result_and_signers(rpc_client, skip_rpc_validation, None::<&Path>).await
+    }
+
+    pub async fn validate_with_result_and_signers<P: AsRef<Path>>(
+        rpc_client: &RpcClient,
+        skip_rpc_validation: bool,
+        signers_config_path: Option<P>,
     ) -> Result<Vec<String>, Vec<String>> {
         let mut errors = Vec::new();
         let mut warnings = Vec::new();
@@ -119,6 +131,15 @@ impl ConfigValidator {
             errors.push(format!("Invalid spl paid token address: {e}"));
         }
 
+        // Warn if using "All" for allowed_spl_paid_tokens
+        if matches!(config.validation.allowed_spl_paid_tokens, SplTokenConfig::All) {
+            warnings.push(
+                "⚠️  Using 'All' for allowed_spl_paid_tokens - this accepts ANY SPL token for payment. \
+                Consider using an explicit allowlist to reduce volatility risk and protect against \
+                potentially malicious or worthless tokens being used for fees.".to_string()
+            );
+        }
+
         // Validate disallowed accounts
         if let Err(e) = TokenUtil::check_valid_tokens(&config.validation.disallowed_accounts) {
             errors.push(format!("Invalid disallowed account address: {e}"));
@@ -236,6 +257,23 @@ impl ConfigValidator {
             }
         }
 
+        // Validate signers configuration if provided
+        if let Some(path) = signers_config_path {
+            match SignerPoolConfig::load_config(path.as_ref()) {
+                Ok(signer_config) => {
+                    let (signer_warnings, signer_errors) =
+                        SignerValidator::validate_with_result(&signer_config);
+                    warnings.extend(signer_warnings);
+                    errors.extend(signer_errors);
+                }
+                Err(e) => {
+                    errors.push(format!("Failed to load signers config: {e}"));
+                }
+            }
+        } else {
+            println!("ℹ️  Signers configuration not validated. Include --signers-config path/to/signers.toml to validate signers");
+        }
+
         // Output results
         println!("=== Configuration Validation ===");
         if errors.is_empty() {
@@ -684,6 +722,11 @@ mod tests {
 
         let result = ConfigValidator::validate_with_result(&rpc_client, true).await;
         assert!(result.is_ok());
+
+        // Check that it warns about using "All" for allowed_spl_paid_tokens
+        let warnings = result.unwrap();
+        assert!(warnings.iter().any(|w| w.contains("Using 'All' for allowed_spl_paid_tokens")));
+        assert!(warnings.iter().any(|w| w.contains("volatility risk")));
     }
 
     #[tokio::test]

crates/lib/src/validator/mod.rs

@@ -1,3 +1,4 @@
 pub mod account_validator;
 pub mod config_validator;
+pub mod signer_validator;
 pub mod transaction_validator;