feat(attestation_engine): verifier-allowlist-abuse-cases-rate-limits-…

contracts/attestation_engine/src/lib.rs

@@ -198,11 +198,22 @@ impl AttestationEngineContract {
     // Verifier Whitelist Management
     // ========================================================================
 
-    /// Add a verifier to the whitelist
+    /// Add a verifier to the allowlist.
     ///
     /// # Arguments
     /// * `caller` - Must be admin
     /// * `verifier` - Address to add as authorized verifier
+    ///
+    /// # Errors
+    /// * `NotInitialized` – contract not initialized
+    /// * `Unauthorized` – caller is not admin
+    ///
+    /// # Security Notes
+    /// - Rate-limited per caller via `RateLimiter` (configurable via `set_rate_limit`
+    ///   using function symbol `"add_verif"`).
+    /// - Duplicate adds are idempotent: emits `VerifAddAbuse` audit event and returns
+    ///   `Ok(())` without modifying state, so call patterns are visible on-chain
+    ///   without disrupting operation.
     pub fn add_verifier(
         e: Env,
         caller: Address,
@@ -221,23 +232,53 @@ impl AttestationEngineContract {
             return Err(AttestationError::Unauthorized);
         }
 
-        // Add verifier to whitelist
+        // Rate-limit allowlist mutations per caller (panics if limit exceeded)
+        let fn_symbol = Symbol::new(&e, "add_verif");
+        RateLimiter::check(&e, &caller, &fn_symbol);
+
+        // Abuse case: duplicate add — emit audit event and return idempotently
+        let already_listed: bool = e
+            .storage()
+            .instance()
+            .get(&DataKey::Verifier(verifier.clone()))
+            .unwrap_or(false);
+        if already_listed {
+            e.events().publish(
+                (Symbol::new(&e, "VerifAddAbuse"),),
+                (caller, verifier, e.ledger().timestamp()),
+            );
+            return Ok(());
+        }
+
+        // Add verifier to allowlist
         e.storage()
             .instance()
             .set(&DataKey::Verifier(verifier.clone()), &true);
 
-        // Emit event
-        e.events()
-            .publish((Symbol::new(&e, "VerifierAdded"),), (verifier,));
+        // Emit audit event with caller and timestamp
+        e.events().publish(
+            (Symbol::new(&e, "VerifierAdded"),),
+            (caller, verifier, e.ledger().timestamp()),
+        );
 
         Ok(())
     }
 
-    /// Remove a verifier from the whitelist
+    /// Remove a verifier from the allowlist.
     ///
     /// # Arguments
     /// * `caller` - Must be admin
     /// * `verifier` - Address to remove from authorized verifiers
+    ///
+    /// # Errors
+    /// * `NotInitialized` – contract not initialized
+    /// * `Unauthorized` – caller is not admin
+    ///
+    /// # Security Notes
+    /// - Rate-limited per caller via `RateLimiter` (configurable via `set_rate_limit`
+    ///   using function symbol `"rm_verif"`).
+    /// - Removing an address not in the allowlist is idempotent: emits `VerifRmAbuse`
+    ///   audit event and returns `Ok(())` without modifying state.
     pub fn remove_verifier(
         e: Env,
         caller: Address,
@@ -256,14 +297,34 @@ impl AttestationEngineContract {
             return Err(AttestationError::Unauthorized);
         }
 
-        // Remove verifier from whitelist
+        // Rate-limit allowlist mutations per caller (panics if limit exceeded)
+        let fn_symbol = Symbol::new(&e, "rm_verif");
+        RateLimiter::check(&e, &caller, &fn_symbol);
+
+        // Abuse case: remove of non-existent verifier — emit audit event and return idempotently
+        let is_listed: bool = e
+            .storage()
+            .instance()
+            .get(&DataKey::Verifier(verifier.clone()))
+            .unwrap_or(false);
+        if !is_listed {
+            e.events().publish(
+                (Symbol::new(&e, "VerifRmAbuse"),),
+                (caller, verifier, e.ledger().timestamp()),
+            );
+            return Ok(());
+        }
+
+        // Remove verifier from allowlist
         e.storage()
             .instance()
             .remove(&DataKey::Verifier(verifier.clone()));
 
-        // Emit event
-        e.events()
-            .publish((Symbol::new(&e, "VerifierRemoved"),), (verifier,));
+        // Emit audit event with caller and timestamp
+        e.events().publish(
+            (Symbol::new(&e, "VerifierRemoved"),),
+            (caller, verifier, e.ledger().timestamp()),
+        );
 
         Ok(())
     }

contracts/attestation_engine/src/tests.rs

@@ -1,7 +1,7 @@
 #![cfg(test)]
 
 use super::*;
-use soroban_sdk::{testutils::Address as _, Address, Env, Map, String};
+use soroban_sdk::{testutils::Address as _, testutils::Ledger as _, Address, Env, Map, String, Symbol};
 
 #[test]
 fn test_initialize_and_getters() {
@@ -662,3 +662,239 @@ fn test_record_drawdown_exceeds_max_loss_records_violation() {
     assert_eq!(violation_type, String::from_str(&e, "max_loss_exceeded"));
     assert_eq!(severity, String::from_str(&e, "high"));
 }
+
+// ============================================
+// Verifier Allowlist Abuse Cases
+// ============================================
+
+#[test]
+fn test_add_verifier_success() {
+    let e = Env::default();
+    e.mock_all_auths();
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+    let verifier = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+    });
+
+    let result = e.as_contract(&contract_id, || {
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone())
+    });
+    assert_eq!(result, Ok(()));
+
+    let is_listed = e.as_contract(&contract_id, || {
+        AttestationEngineContract::is_verifier(e.clone(), verifier.clone())
+    });
+    assert!(is_listed, "Verifier should be listed after add");
+}
+
+#[test]
+fn test_add_verifier_duplicate_is_idempotent() {
+    let e = Env::default();
+    e.mock_all_auths();
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+    let verifier = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+    });
+
+    // First add — normal path
+    let r1 = e.as_contract(&contract_id, || {
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone())
+    });
+    assert_eq!(r1, Ok(()));
+
+    // Second add — abuse path: idempotent, emits VerifAddAbuse event
+    let r2 = e.as_contract(&contract_id, || {
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone())
+    });
+    assert_eq!(r2, Ok(()));
+
+    // Verifier must still be listed
+    let still_listed = e.as_contract(&contract_id, || {
+        AttestationEngineContract::is_verifier(e.clone(), verifier.clone())
+    });
+    assert!(still_listed, "Verifier should remain listed after duplicate add");
+}
+
+#[test]
+fn test_add_verifier_unauthorized() {
+    let e = Env::default();
+    e.mock_all_auths();
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+    let non_admin = Address::generate(&e);
+    let verifier = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+    });
+
+    let result = e.as_contract(&contract_id, || {
+        AttestationEngineContract::add_verifier(e.clone(), non_admin.clone(), verifier.clone())
+    });
+    assert_eq!(result, Err(AttestationError::Unauthorized));
+
+    // Verifier must not have been added
+    let is_listed = e.as_contract(&contract_id, || {
+        AttestationEngineContract::is_verifier(e.clone(), verifier.clone())
+    });
+    assert!(!is_listed, "Verifier must not be listed after unauthorized add attempt");
+}
+
+#[test]
+fn test_remove_verifier_success() {
+    let e = Env::default();
+    e.mock_all_auths();
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+    let verifier = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone()).unwrap();
+    });
+
+    let result = e.as_contract(&contract_id, || {
+        AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier.clone())
+    });
+    assert_eq!(result, Ok(()));
+
+    let is_listed = e.as_contract(&contract_id, || {
+        AttestationEngineContract::is_verifier(e.clone(), verifier.clone())
+    });
+    assert!(!is_listed, "Verifier should not be listed after remove");
+}
+
+#[test]
+fn test_remove_verifier_not_listed_is_idempotent() {
+    let e = Env::default();
+    e.mock_all_auths();
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+    let verifier = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+    });
+
+    // verifier was never added; remove is idempotent, emits VerifRmAbuse event
+    let result = e.as_contract(&contract_id, || {
+        AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier.clone())
+    });
+    assert_eq!(result, Ok(()));
+
+    let is_listed = e.as_contract(&contract_id, || {
+        AttestationEngineContract::is_verifier(e.clone(), verifier.clone())
+    });
+    assert!(!is_listed, "Verifier should remain unlisted after no-op remove");
+}
+
+#[test]
+fn test_remove_verifier_unauthorized() {
+    let e = Env::default();
+    e.mock_all_auths();
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+    let non_admin = Address::generate(&e);
+    let verifier = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone()).unwrap();
+    });
+
+    let result = e.as_contract(&contract_id, || {
+        AttestationEngineContract::remove_verifier(e.clone(), non_admin.clone(), verifier.clone())
+    });
+    assert_eq!(result, Err(AttestationError::Unauthorized));
+
+    // Verifier must still be listed
+    let still_listed = e.as_contract(&contract_id, || {
+        AttestationEngineContract::is_verifier(e.clone(), verifier.clone())
+    });
+    assert!(still_listed, "Verifier must remain listed after unauthorized remove attempt");
+}
+
+#[test]
+#[should_panic(expected = "Rate limit exceeded")]
+fn test_add_verifier_rate_limit_exceeded() {
+    let e = Env::default();
+    e.mock_all_auths();
+    e.ledger().with_mut(|l| l.timestamp = 1000);
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+        // 1 add_verifier allowed per 3600-second window
+        AttestationEngineContract::set_rate_limit(
+            e.clone(),
+            admin.clone(),
+            Symbol::new(&e, "add_verif"),
+            3600u64,
+            1u32,
+        )
+        .unwrap();
+    });
+
+    let verifier1 = Address::generate(&e);
+    let verifier2 = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        // First call — within limit
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier1.clone())
+            .unwrap();
+        // Second call — exceeds limit, must panic
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier2.clone())
+            .unwrap();
+    });
+}
+
+#[test]
+#[should_panic(expected = "Rate limit exceeded")]
+fn test_remove_verifier_rate_limit_exceeded() {
+    let e = Env::default();
+    e.mock_all_auths();
+    e.ledger().with_mut(|l| l.timestamp = 1000);
+    let contract_id = e.register_contract(None, AttestationEngineContract);
+    let admin = Address::generate(&e);
+    let core = Address::generate(&e);
+    let verifier1 = Address::generate(&e);
+    let verifier2 = Address::generate(&e);
+
+    e.as_contract(&contract_id, || {
+        AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap();
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier1.clone()).unwrap();
+        AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier2.clone()).unwrap();
+        // 1 remove_verifier allowed per 3600-second window
+        AttestationEngineContract::set_rate_limit(
+            e.clone(),
+            admin.clone(),
+            Symbol::new(&e, "rm_verif"),
+            3600u64,
+            1u32,
+        )
+        .unwrap();
+    });
+
+    e.as_contract(&contract_id, || {
+        // First remove — within limit
+        AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier1.clone())
+            .unwrap();
+        // Second remove — exceeds limit, must panic
+        AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier2.clone())
+            .unwrap();
+    });
+}

contracts/shared_utils/src/lib.rs

@@ -30,23 +30,14 @@ pub mod validation;
 #[cfg(test)]
 mod tests;
 
-// Re-export commonly used items (explicit only to avoid E0252 glob clashes)
+// Re-export all public items from each utility module
 pub use access_control::AccessControl;
 pub use batch::{
     BatchConfig, BatchDataKey, BatchError, BatchMode, BatchOperationReport, BatchProcessor,
     BatchResultString, BatchResultVoid, DetailedBatchError, RollbackHelper, StateSnapshot,
 };
 pub use emergency::EmergencyControl;
-pub use error_codes::{category, code, emit_error_event, message_for_code};
-pub use errors::ErrorHelper;
-pub use events::Events;
 pub use fees;
-pub use math::SafeMath;
-pub use pausable::Pausable;
-pub use rate_limiting::RateLimiter;
-pub use storage::Storage;
-pub use time::TimeUtils;
-pub use validation::Validation;
 pub use error_codes::*;
 pub use errors::*;
 pub use events::*;