feat: restore review timestamps and stabilize tests

dongle-smartcontract/src/admin_manager.rs

@@ -137,6 +137,7 @@ impl AdminManager {
 mod tests {
     use crate::DongleContract;
     use crate::DongleContractClient;
+    use crate::errors::ContractError;
     use soroban_sdk::{testutils::Address as _, Address, Env};
 
     #[test]
@@ -177,9 +178,9 @@ mod tests {
         let new_admin = Address::generate(&env);
 
         client.mock_all_auths().initialize(&admin);
-        client.mock_all_auths().add_admin(&non_admin, &new_admin);
+        let result = client.mock_all_auths().try_add_admin(&non_admin, &new_admin);
 
-        // The new admin should not be added
+        assert_eq!(result, Err(Ok(ContractError::AdminOnly)));
         assert!(!client.is_admin(&new_admin));
     }
 
@@ -207,9 +208,9 @@ mod tests {
         let admin = Address::generate(&env);
 
         client.mock_all_auths().initialize(&admin);
-        client.mock_all_auths().remove_admin(&admin, &admin);
+        let result = client.mock_all_auths().try_remove_admin(&admin, &admin);
 
-        // Should still be admin
+        assert_eq!(result, Err(Ok(ContractError::CannotRemoveLastAdmin)));
         assert!(client.is_admin(&admin));
     }
 
@@ -224,9 +225,9 @@ mod tests {
 
         client.mock_all_auths().initialize(&admin);
         client.mock_all_auths().add_admin(&admin, &another_admin);
-        client.mock_all_auths().remove_admin(&admin, &non_admin);
+        let result = client.mock_all_auths().try_remove_admin(&admin, &non_admin);
 
-        // another_admin should still be there
+        assert_eq!(result, Err(Ok(ContractError::AdminNotFound)));
         assert!(client.is_admin(&another_admin));
     }
 

dongle-smartcontract/src/errors.rs

@@ -1,70 +1,65 @@
- use soroban_sdk::contracterror;
+use soroban_sdk::contracterror;
 
 /// Error types for the Dongle smart contract
 #[contracterror]
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 #[repr(u32)]
 pub enum ContractError {
-    AlreadyReviewed = 19
     /// Project not found
     ProjectNotFound = 1,
-
     /// Unauthorized access - caller is not the owner
     Unauthorized = 2,
-
     /// Project already exists
     ProjectAlreadyExists = 3,
-
     /// Invalid rating - must be between 1 and 5
     InvalidRating = 4,
-
     /// Review not found
     ReviewNotFound = 5,
-
     /// Review already exists
     ReviewAlreadyExists = 6,
-
     /// Verification record not found
     VerificationNotFound = 7,
-
     /// Invalid verification status transition
     InvalidStatusTransition = 8,
-
     /// Only admin can perform this action
     AdminOnly = 9,
-
     /// Invalid fee amount
     InvalidFeeAmount = 10,
-
     /// Insufficient fee paid
     InsufficientFee = 11,
-
     /// Invalid project data - missing required fields
     InvalidProjectData = 12,
-
     /// Project name too long
     ProjectNameTooLong = 13,
-
     /// Project description too long
     ProjectDescriptionTooLong = 14,
-
     /// Invalid project category
     InvalidProjectCategory = 15,
-
     /// Verification already processed
     VerificationAlreadyProcessed = 16,
-
     /// Cannot review own project
     CannotReviewOwnProject = 17,
-
     /// Fee configuration not set
     FeeConfigNotSet = 18,
-
     /// Treasury address not set
     TreasuryNotSet = 19,
-
     /// Review already deleted
     ReviewAlreadyDeleted = 20,
-    NotReviewer = 22,
+    /// Invalid project name format
     InvalidProjectNameFormat = 21,
-}
+    /// Not the reviewer
+    NotReviewer = 22,
+    /// Already reviewed
+    AlreadyReviewed = 23,
+    /// Cannot remove last admin
+    CannotRemoveLastAdmin = 24,
+    /// Admin not found
+    AdminNotFound = 25,
+    /// Record not found (generic)
+    RecordNotFound = 26,
+    /// Duplicate review error variant (sometimes used separately)
+    DuplicateReview = 27,
+}
+
+// Legacy alias to avoid breaking any code that uses `Error` directly
+pub type Error = ContractError;

dongle-smartcontract/src/events.rs

@@ -9,12 +9,15 @@ pub fn publish_review_event(
     reviewer: Address,
     action: ReviewAction,
     comment_cid: Option<String>,
+    created_at: u64,
+    updated_at: u64,
 ) {
     let event_data = ReviewEventData {
         project_id,
         reviewer: reviewer.clone(),
         action: action.clone(),
-        timestamp: env.ledger().timestamp(),
+        created_at,
+        updated_at,
         comment_cid,
     };
 

dongle-smartcontract/src/fee_manager.rs

@@ -9,8 +9,8 @@ use soroban_sdk::{Address, Env};
 
 pub struct FeeManager;
 
-#[allow(dead_code)]
 impl FeeManager {
+    /// Configure fees for the contract (admin only)
     pub fn set_fee(
         env: &Env,
         admin: Address,
@@ -19,21 +19,8 @@ impl FeeManager {
         treasury: Address,
     ) -> Result<(), ContractError> {
         admin.require_auth();
-
-        // Verify admin privileges
         AdminManager::require_admin(env, &admin)?;
 
-        // Authorization check
-        let stored_admin: Address = env
-            .storage()
-            .persistent()
-            .get(&StorageKey::Admin)
-            .ok_or(ContractError::Unauthorized)?;
-        if admin != stored_admin {
-            return Err(ContractError::Unauthorized);
-        }
-        admin.require_auth();
-
         let config = FeeConfig {
             token,
             verification_fee: amount,
@@ -45,24 +32,25 @@ impl FeeManager {
         env.storage()
             .persistent()
             .set(&StorageKey::Treasury, &treasury);
+
         publish_fee_set_event(env, amount, 0);
         Ok(())
     }
 
+    /// Pay the verification fee for a project
     pub fn pay_fee(
         env: &Env,
         payer: Address,
         project_id: u64,
-
-      token: Option<Address>,
+        token: Option<Address>,
     ) -> Result<(), ContractError> {
         payer.require_auth();
 
         let config = Self::get_fee_config(env)?;
         let treasury: Address = env
             .storage()
             .persistent()
-            .get(&DataKey::Treasury)
+            .get(&StorageKey::Treasury)
             .ok_or(ContractError::TreasuryNotSet)?;
 
         if config.token != token {
@@ -71,93 +59,66 @@ impl FeeManager {
 
         let amount = config.verification_fee;
         if amount > 0 {
-            if let Some(token_address) = config.token {
-                let client = soroban_sdk::token::Client::new(env, &token_address);
-                client.transfer(&payer, &treasury, &(amount as i128));
-            } else {
-                // For native token, we use the same token client since it's standardized
-                // Assuming the contract environment has access to the native asset if token is None
-                // In Soroban, native asset is also a token. 
-                // However, the FeeConfig doesn't store the native asset address if None.
-                // We'll require the caller to pass the correct token address if it's not None.
-                // If config.token is None, it means the contract isn't fully configured for native payments yet 
-                // or we need a standard way to get the native asset address.
-                return Err(ContractError::FeeConfigNotSet);
-            }
+            let token_address = config.token.ok_or(ContractError::FeeConfigNotSet)?;
+            let client = soroban_sdk::token::Client::new(env, &token_address);
+            client.transfer(&payer, &treasury, &(amount as i128));
         }
 
         env.storage()
             .persistent()
-            .set(&DataKey::FeePaidForProject(project_id), &true);
-
-        crate::events::publish_fee_paid_event(env, project_id, amount);
+            .set(&StorageKey::FeePaidForProject(project_id), &true);
 
+        publish_fee_paid_event(env, project_id, amount);
         Ok(())
     }
 
+    /// Check if the fee has been paid for a project
     pub fn is_fee_paid(env: &Env, project_id: u64) -> bool {
         env.storage()
             .persistent()
-            .get(&DataKey::FeePaidForProject(project_id))
+            .get(&StorageKey::FeePaidForProject(project_id))
             .unwrap_or(false)
     }
 
+    /// Consume the fee payment (used during verification request)
     pub fn consume_fee_payment(env: &Env, project_id: u64) -> Result<(), ContractError> {
         if !Self::is_fee_paid(env, project_id) {
             return Err(ContractError::InsufficientFee);
         }
         env.storage()
             .persistent()
-            .remove(&DataKey::FeePaidForProject(project_id));
-        Ok(())
-    }
-
-        env.storage()
-            .persistent()
-            .set(&StorageKey::FeePaidForProject(project_id), &true);
-
-        publish_fee_paid_event(env, project_id, config.verification_fee);
+            .remove(&StorageKey::FeePaidForProject(project_id));
         Ok(())
     }
 
+    /// Get current fee configuration
     pub fn get_fee_config(env: &Env) -> Result<FeeConfig, ContractError> {
         env.storage()
             .persistent()
             .get(&StorageKey::FeeConfig)
             .ok_or(ContractError::FeeConfigNotSet)
     }
 
+    /// Set the treasury address (admin only)
     pub fn set_treasury(env: &Env, admin: Address, treasury: Address) -> Result<(), ContractError> {
-        let stored_admin: Address = env
-            .storage()
-            .persistent()
-            .get(&StorageKey::Admin)
-            .ok_or(ContractError::Unauthorized)?;
-        if admin != stored_admin {
-            return Err(ContractError::Unauthorized);
-        }
         admin.require_auth();
+        AdminManager::require_admin(env, &admin)?;
 
         env.storage()
             .persistent()
             .set(&StorageKey::Treasury, &treasury);
         Ok(())
     }
 
+    /// Get the current treasury address
     pub fn get_treasury(env: &Env) -> Result<Address, ContractError> {
         env.storage()
             .persistent()
             .get(&StorageKey::Treasury)
-            .ok_or(ContractError::FeeConfigNotSet) // Reusing error or could use a new one
-    }
-
-    pub fn is_fee_paid(env: &Env, project_id: u64) -> bool {
-        env.storage()
-            .persistent()
-            .get(&StorageKey::FeePaidForProject(project_id))
-            .unwrap_or(false)
+            .ok_or(ContractError::TreasuryNotSet)
     }
 
+    /// Get fee for a specific operation
     #[allow(dead_code)]
     pub fn get_operation_fee(env: &Env, operation_type: &str) -> Result<u128, ContractError> {
         let config = Self::get_fee_config(env)?;

dongle-smartcontract/src/lib.rs

@@ -2,15 +2,15 @@
 
 mod admin_manager;
 mod constants;
-mod errors;
+pub mod errors;
 mod events;
 mod fee_manager;
 mod project_registry;
 mod rating_calculator;
 mod review_registry;
 mod storage_keys;
 mod verification_registry;
-mod types;
+pub mod types;
 
 #[cfg(test)]
 mod test;

dongle-smartcontract/src/project_registry.rs

@@ -145,7 +145,7 @@ impl ProjectRegistry {
         env.storage()
             .persistent()
             .get(&StorageKey::OwnerProjects(owner.clone()))
-            .unwrap_or_else(|| Vec::new(env))
+            .unwrap_or_else(|| Vec::<u64>::new(env))
             .len()
     }
 

dongle-smartcontract/src/registration_tests.rs

@@ -31,7 +31,7 @@ fn test_register_project_success() {
         metadata_cid: None,
     };
 
-    let id = client.register_project(&params).unwrap();
+    let id = client.register_project(&params);
 
     assert_eq!(id, 1);
 
@@ -60,7 +60,7 @@ fn test_register_duplicate_project_fails() {
     };
 
     // Register first project
-    client.register_project(&params).unwrap();
+    client.register_project(&params);
 
     // Attempt to register another project with the same name
     let result = client.try_register_project(&params);
@@ -87,7 +87,7 @@ fn test_register_different_projects_success() {
         logo_cid: None,
         metadata_cid: None,
     };
-    let id1 = client.register_project(&params1).unwrap();
+    let id1 = client.register_project(&params1);
     assert_eq!(id1, 1);
 
     let params2 = ProjectRegistrationParams {
@@ -99,6 +99,6 @@ fn test_register_different_projects_success() {
         logo_cid: None,
         metadata_cid: None,
     };
-    let id2 = client.register_project(&params2).unwrap();
+    let id2 = client.register_project(&params2);
     assert_eq!(id2, 2);
 }