feature:Refactor validation and introduce granular error handling

dongle-smartcontract/src/fee_manager.rs

@@ -53,7 +53,7 @@ impl FeeManager {
             .ok_or(ContractError::TreasuryNotSet)?;
 
         if config.token != token {
-            return Err(ContractError::InvalidProjectData);
+            return Err(ContractError::InvalidFeeToken);
         }
 
         let amount = config.verification_fee;

dongle-smartcontract/src/project_registry.rs

@@ -162,13 +162,22 @@ impl ProjectRegistry {
             }
             project.category = value;
         }
+
+        // Validate and update website
         if let Some(value) = params.website {
+            Utils::validate_website_url(&value)?;
             project.website = value;
         }
+
+        // Validate and update logo_cid
         if let Some(value) = params.logo_cid {
+            Utils::validate_logo_cid(&value)?;
             project.logo_cid = value;
         }
+
+        // Validate and update metadata_cid
         if let Some(value) = params.metadata_cid {
+            Utils::validate_metadata_cid(&value)?;
             project.metadata_cid = value;
         }
 
@@ -367,48 +376,12 @@ mod tests {
     use crate::errors::ContractError;
     use soroban_sdk::{Env, String};
 
-    // Validation function only used in tests
-    fn validate_project_data(
-        name: &String,
-        _description: &String,
-        _category: &String,
-    ) -> Result<(), ContractError> {
-        extern crate alloc;
-        use alloc::string::ToString;
-
-        let name_str = name.to_string();
-
-        // 1. Validate Non-empty and not only whitespace
-        if name_str.trim().is_empty() {
-            return Err(ContractError::InvalidProjectData);
-        }
-
-        // 2. Validate max length using the CONSTANT
-        let max_len = crate::constants::MAX_NAME_LEN;
-        if name_str.len() > max_len {
-            return Err(ContractError::ProjectNameTooLong);
-        }
-
-        // 3. Validate alphanumeric, underscore, hyphen
-        for c in name_str.chars() {
-            if !c.is_ascii_alphanumeric() && c != '_' && c != '-' {
-                return Err(ContractError::InvalidProjectNameFormat);
-            }
-        }
-
-        Ok(())
-    }
-
     #[test]
     fn test_valid_project_name() {
         let env = Env::default();
         let name = String::from_str(&env, "Valid-Project_Name123");
 
-        let result = validate_project_data(
-            &name,
-            &String::from_str(&env, "Desc"),
-            &String::from_str(&env, "Cat"),
-        );
+        let result = Utils::validate_project_name(&name);
         assert!(result.is_ok());
     }
 
@@ -417,24 +390,16 @@ mod tests {
         let env = Env::default();
         let name = String::from_str(&env, "   ");
 
-        let result = validate_project_data(
-            &name,
-            &String::from_str(&env, "Desc"),
-            &String::from_str(&env, "Cat"),
-        );
-        assert_eq!(result, Err(ContractError::InvalidProjectData));
+        let result = Utils::validate_project_name(&name);
+        assert_eq!(result, Err(ContractError::ProjectNameEmpty));
     }
 
     #[test]
     fn test_invalid_characters_in_name() {
         let env = Env::default();
         let name = String::from_str(&env, "My Project *");
 
-        let result = validate_project_data(
-            &name,
-            &String::from_str(&env, "Desc"),
-            &String::from_str(&env, "Cat"),
-        );
+        let result = Utils::validate_project_name(&name);
         assert_eq!(result, Err(ContractError::InvalidProjectNameFormat));
     }
 
@@ -444,11 +409,7 @@ mod tests {
         // 51 characters
         let name = String::from_str(&env, "ThisProjectNameIsWayTooLongAndExceedsTheFiftyCharL1");
 
-        let result = validate_project_data(
-            &name,
-            &String::from_str(&env, "Desc"),
-            &String::from_str(&env, "Cat"),
-        );
+        let result = Utils::validate_project_name(&name);
         assert_eq!(result, Err(ContractError::ProjectNameTooLong));
     }
 

dongle-smartcontract/src/review_registry.rs

@@ -34,9 +34,11 @@ impl ReviewRegistry {
         // Validation phase
         reviewer.require_auth();
 
-        if !(RATING_MIN..=RATING_MAX).contains(&rating) {
-            return Err(ContractError::InvalidRating);
-        }
+        // Validate rating
+        Utils::validate_rating(rating)?;
+
+        // Validate comment CID if provided
+        Utils::validate_comment_cid(&comment_cid)?;
 
         let review_key = StorageKey::Review(project_id, reviewer.clone());
         if env.storage().persistent().has(&review_key) {
@@ -146,9 +148,11 @@ impl ReviewRegistry {
         // Validation phase
         reviewer.require_auth();
 
-        if !(RATING_MIN..=RATING_MAX).contains(&rating) {
-            return Err(ContractError::InvalidRating);
-        }
+        // Validate rating
+        Utils::validate_rating(rating)?;
+
+        // Validate comment CID if provided
+        Utils::validate_comment_cid(&comment_cid)?;
 
         let review_key = StorageKey::Review(project_id, reviewer.clone());
         let mut review: Review = env

dongle-smartcontract/src/utils.rs

@@ -1,3 +1,7 @@
+use crate::constants::{
+    MAX_CATEGORY_LEN, MAX_CID_LEN, MAX_DESCRIPTION_LEN, MAX_NAME_LEN, MAX_WEBSITE_LEN,
+    MIN_STRING_LEN, RATING_MAX, RATING_MIN,
+};
 use crate::errors::ContractError;
 use crate::storage_keys::StorageKey;
 use soroban_sdk::{Address, Env, String};
@@ -25,19 +29,152 @@ impl Utils {
         Ok(())
     }
 
+    /// Validate project name: non-empty, trimmed, max length, alphanumeric/underscore/hyphen
+    pub fn validate_project_name(name: &String) -> Result<(), ContractError> {
+        extern crate alloc;
+        use alloc::string::ToString;
+
+        let name_str = name.to_string();
+        let trimmed = name_str.trim();
+
+        // Check non-empty after trim
+        if trimmed.is_empty() {
+            return Err(ContractError::ProjectNameEmpty);
+        }
+
+        // Check max length
+        if trimmed.len() > MAX_NAME_LEN {
+            return Err(ContractError::ProjectNameTooLong);
+        }
+
+        // Check valid characters: alphanumeric, underscore, hyphen
+        for c in trimmed.chars() {
+            if !c.is_ascii_alphanumeric() && c != '_' && c != '-' {
+                return Err(ContractError::InvalidProjectNameFormat);
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Validate project description: non-empty, trimmed, max length
+    pub fn validate_project_description(description: &String) -> Result<(), ContractError> {
+        extern crate alloc;
+        use alloc::string::ToString;
+
+        let desc_str = description.to_string();
+        let trimmed = desc_str.trim();
+
+        if trimmed.is_empty() {
+            return Err(ContractError::ProjectDescriptionEmpty);
+        }
+
+        if trimmed.len() > MAX_DESCRIPTION_LEN {
+            return Err(ContractError::InvalidProjectData);
+        }
+
+        Ok(())
+    }
+
+    /// Validate project category: non-empty, trimmed, max length
+    pub fn validate_project_category(category: &String) -> Result<(), ContractError> {
+        extern crate alloc;
+        use alloc::string::ToString;
+
+        let cat_str = category.to_string();
+        let trimmed = cat_str.trim();
+
+        if trimmed.is_empty() {
+            return Err(ContractError::ProjectCategoryEmpty);
+        }
+
+        if trimmed.len() > MAX_CATEGORY_LEN {
+            return Err(ContractError::InvalidProjectData);
+        }
+
+        Ok(())
+    }
+
+    /// Validate website URL: optional, but if provided, check length and basic format
+    pub fn validate_website_url(website: &Option<String>) -> Result<(), ContractError> {
+        if let Some(url) = website {
+            if url.len() > MAX_WEBSITE_LEN {
+                return Err(ContractError::InvalidWebsiteUrl);
+            }
+            // Basic URL validation - should start with http:// or https://
+            extern crate alloc;
+            use alloc::string::ToString;
+            let url_str = url.to_string();
+            if !url_str.starts_with("http://") && !url_str.starts_with("https://") {
+                return Err(ContractError::InvalidWebsiteUrl);
+            }
+        }
+        Ok(())
+    }
+
+    /// Validate IPFS CID: optional, but if provided, check length and basic format
+    pub fn validate_ipfs_cid(cid: &Option<String>) -> Result<(), ContractError> {
+        if let Some(cid_val) = cid {
+            if cid_val.is_empty() {
+                return Err(ContractError::InvalidIpfsCid);
+            }
+            if cid_val.len() > MAX_CID_LEN {
+                return Err(ContractError::InvalidIpfsCid);
+            }
+            // Basic IPFS CID validation - should be reasonable length
+            if cid_val.len() < 10 {
+                return Err(ContractError::InvalidIpfsCid);
+            }
+        }
+        Ok(())
+    }
+
+    /// Validate logo CID (same as IPFS CID)
+    pub fn validate_logo_cid(logo_cid: &Option<String>) -> Result<(), ContractError> {
+        Self::validate_ipfs_cid(logo_cid)
+    }
+
+    /// Validate metadata CID (same as IPFS CID)
+    pub fn validate_metadata_cid(metadata_cid: &Option<String>) -> Result<(), ContractError> {
+        Self::validate_ipfs_cid(metadata_cid)
+    }
+
+    /// Validate comment CID (same as IPFS CID)
+    pub fn validate_comment_cid(comment_cid: &Option<String>) -> Result<(), ContractError> {
+        Self::validate_ipfs_cid(comment_cid)
+    }
+
+    /// Validate evidence CID: required, non-empty, valid IPFS CID
+    pub fn validate_evidence_cid(evidence_cid: &String) -> Result<(), ContractError> {
+        if evidence_cid.is_empty() {
+            return Err(ContractError::InvalidEvidenceCid);
+        }
+        if evidence_cid.len() > MAX_CID_LEN {
+            return Err(ContractError::InvalidEvidenceCid);
+        }
+        if evidence_cid.len() < 10 {
+            return Err(ContractError::InvalidEvidenceCid);
+        }
+        Ok(())
+    }
+
+    /// Validate rating: must be between RATING_MIN and RATING_MAX
+    pub fn validate_rating(rating: u32) -> Result<(), ContractError> {
+        if !(RATING_MIN..=RATING_MAX).contains(&rating) {
+            return Err(ContractError::InvalidRating);
+        }
+        Ok(())
+    }
+
+    /// Legacy function - kept for backward compatibility but deprecated
     pub fn validate_string_length(
-        value: &String,
-        min_length: u32,
-        max_length: u32,
+        _value: &String,
+        _min_length: u32,
+        _max_length: u32,
         _field_name: &str,
     ) -> Result<(), ContractError> {
-        let length = value.len();
-
-        if length < min_length || length > max_length {
-            Err(ContractError::InvalidProjectData)
-        } else {
-            Ok(())
-        }
+        // This function is deprecated - use specific validation functions instead
+        Err(ContractError::InvalidProjectData)
     }
 
     pub fn is_valid_ipfs_cid(cid: &String) -> bool {
@@ -64,9 +201,12 @@ impl Utils {
         bytes[0] == b'b'
     }
 
+    /// Legacy function - kept for backward compatibility
     pub fn is_valid_url(_url: &String) -> bool {
-        true
+        // Use validate_website_url instead
+        false
     }
+}
 
     pub fn sanitize_string(input: &String) -> String {
         input.clone()

dongle-smartcontract/src/verification_registry.rs

@@ -281,10 +281,7 @@ impl VerificationRegistry {
     }
 
     pub fn validate_evidence_cid(evidence_cid: &String) -> Result<(), ContractError> {
-        if evidence_cid.is_empty() {
-            return Err(ContractError::InvalidProjectData);
-        }
-        Ok(())
+        Utils::validate_evidence_cid(evidence_cid)
     }
 
     #[allow(dead_code)]