fix(errors): standardize error handling patterns across modules

contracts/teachlink/src/access_control.rs

@@ -34,11 +34,12 @@ impl AccessControlManager {
         }
     }
 
-    /// Enforce a role check, panicking if unauthorized
-    pub fn check_role(env: &Env, address: &Address, role: AccessRole) {
+    /// Enforce a role check, returning an error if unauthorized
+    pub fn check_role(env: &Env, address: &Address, role: AccessRole) -> Result<(), BridgeError> {
         if !Self::has_role(env, address, role) {
-            panic!("Unauthorized: Missing required role");
+            return Err(BridgeError::Unauthorized);
         }
+        Ok(())
     }
 
     /// Grant a role to an address (Admin only)
@@ -50,7 +51,7 @@ impl AccessControlManager {
     ) -> Result<(), BridgeError> {
         caller.require_auth();
         // Only Admin can grant roles
-        Self::check_role(env, &caller, AccessRole::Admin);
+        Self::check_role(env, &caller, AccessRole::Admin)?;
 
         let mut roles: Map<Address, Vec<AccessRole>> = env
             .storage()
@@ -89,7 +90,7 @@ impl AccessControlManager {
         role: AccessRole,
     ) -> Result<(), BridgeError> {
         caller.require_auth();
-        Self::check_role(env, &caller, AccessRole::Admin);
+        Self::check_role(env, &caller, AccessRole::Admin)?;
 
         let mut roles: Map<Address, Vec<AccessRole>> = env
             .storage()

contracts/teachlink/src/audit.rs

@@ -341,7 +341,7 @@ impl AuditManager {
             env,
             &admin,
             crate::types::AccessRole::AuditManager,
-        );
+        )?;
 
         let audit_records: Map<u64, AuditRecord> = env
             .storage()

contracts/teachlink/src/bridge.rs

@@ -75,11 +75,8 @@ impl Bridge {
             .set_fee_recipient(&fee_recipient)
             .map_err(|_| BridgeError::StorageError)?;
 
-        // Initialize nonce to 0
-        repo.transactions
-            .get_current_nonce()
-            .map_err(|_| BridgeError::StorageError)
-            .ok();
+        // Nonce initializes lazily on first use; ignore if absent
+        let _ = repo.transactions.get_current_nonce();
 
         Ok(())
     }
@@ -122,7 +119,10 @@ impl Bridge {
 
             // Apply bridge fee if configured
             let fee = repo.config.get_bridge_fee().unwrap_or(0);
-            let fee_recipient = repo.config.get_fee_recipient().unwrap();
+            let fee_recipient = repo
+                .config
+                .get_fee_recipient()
+                .map_err(|_| BridgeError::NotInitialized)?;
             let amount_after_fee = if fee > 0 && fee < amount {
                 amount - fee
             } else {
@@ -470,7 +470,7 @@ impl Bridge {
             env,
             &admin,
             crate::types::AccessRole::ValidatorManager,
-        );
+        )?;
 
         repo.validators
             .add_validator(&validator)
@@ -508,7 +508,7 @@ impl Bridge {
             env,
             &admin,
             crate::types::AccessRole::ValidatorManager,
-        );
+        )?;
 
         repo.validators
             .remove_validator(&validator)
@@ -546,7 +546,7 @@ impl Bridge {
             env,
             &admin,
             crate::types::AccessRole::BridgeOperator,
-        );
+        )?;
 
         repo.chains
             .add_chain(chain_id)
@@ -577,7 +577,7 @@ impl Bridge {
             env,
             &admin,
             crate::types::AccessRole::BridgeOperator,
-        );
+        )?;
 
         repo.chains
             .remove_chain(chain_id)
@@ -607,7 +607,7 @@ impl Bridge {
             env,
             &admin,
             crate::types::AccessRole::Admin,
-        );
+        )?;
 
         if fee < 0 {
             return Err(BridgeError::FeeCannotBeNegative);
@@ -645,7 +645,7 @@ impl Bridge {
             env,
             &admin,
             crate::types::AccessRole::Admin,
-        );
+        )?;
 
         repo.config
             .set_fee_recipient(&fee_recipient)
@@ -682,7 +682,7 @@ impl Bridge {
             env,
             &admin,
             crate::types::AccessRole::Admin,
-        );
+        )?;
 
         if min_validators == 0 {
             return Err(BridgeError::MinimumValidatorsMustBeAtLeastOne);

contracts/teachlink/src/emergency.rs

@@ -28,7 +28,7 @@ impl EmergencyManager {
             env,
             &pauser,
             crate::types::AccessRole::EmergencyManager,
-        );
+        )?;
 
         // Check if already paused
         let emergency_state: EmergencyState = env
@@ -77,7 +77,7 @@ impl EmergencyManager {
             env,
             &resumer,
             crate::types::AccessRole::EmergencyManager,
-        );
+        )?;
 
         // Check if paused
         let mut emergency_state: EmergencyState = env
@@ -125,7 +125,7 @@ impl EmergencyManager {
             env,
             &pauser,
             crate::types::AccessRole::EmergencyManager,
-        );
+        )?;
 
         let mut paused_chains: Map<u32, bool> = env
             .storage()
@@ -162,7 +162,7 @@ impl EmergencyManager {
             env,
             &resumer,
             crate::types::AccessRole::EmergencyManager,
-        );
+        )?;
 
         let mut paused_chains: Map<u32, bool> = env
             .storage()
@@ -338,7 +338,7 @@ impl EmergencyManager {
             env,
             &resetter,
             crate::types::AccessRole::EmergencyManager,
-        );
+        )?;
 
         let mut circuit_breakers: Map<u32, CircuitBreaker> = env
             .storage()
@@ -458,7 +458,7 @@ impl EmergencyManager {
             env,
             &updater,
             crate::types::AccessRole::EmergencyManager,
-        );
+        )?;
 
         let mut circuit_breakers: Map<u32, CircuitBreaker> = env
             .storage()

contracts/teachlink/src/errors.rs

@@ -149,12 +149,28 @@ pub enum MobilePlatformError {
 }
 
 /// Common errors that can be used across modules
+///
+/// Error codes are in the range 500–504.
 #[contracterror]
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum CommonError {
-    Unauthorized = 400,
-    InvalidInput = 401,
-    InsufficientBalance = 402,
-    TransferFailed = 403,
-    StorageError = 404,
+    Unauthorized = 500,
+    InvalidInput = 501,
+    InsufficientBalance = 502,
+    TransferFailed = 503,
+    StorageError = 504,
+}
+
+/// Governance module errors
+///
+/// Error codes are in the range 600–605.
+#[contracterror]
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum GovernanceError {
+    ProposalsNotInitialized = 600,
+    GovernanceProposalNotFound = 601,
+    VotingPeriodEnded = 602,
+    GovernanceProposalNotActive = 603,
+    AlreadyVoted = 604,
+    VotingStillInProgress = 605,
 }

contracts/teachlink/src/governance.rs

@@ -1,4 +1,5 @@
-use soroban_sdk::{contracttype, Address, BytesN, Env, Symbol, Vec, map, symbol_short};
+use crate::errors::GovernanceError;
+use soroban_sdk::{contracttype, map, symbol_short, Address, BytesN, Env, Symbol, Vec};
 
 #[contracttype]
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -41,7 +42,7 @@ impl GovernanceManager {
         proposer.require_auth();
 
         // In a full implementation, we would verify the proposer's TEACH balance here
-        
+
         let mut count: u64 = env.storage().persistent().get(&PROP_CNT).unwrap_or(0);
         count += 1;
 
@@ -82,29 +83,30 @@ impl GovernanceManager {
         proposal_id: u64,
         support: bool,
         weight: i128,
-    ) {
+    ) -> Result<(), GovernanceError> {
         voter.require_auth();
 
         let mut proposals: soroban_sdk::Map<u64, Proposal> = env
             .storage()
             .persistent()
             .get(&PROPOSALS)
-            .expect("Proposals not initialized");
+            .ok_or(GovernanceError::ProposalsNotInitialized)?;
 
-        let mut proposal = proposals.get(proposal_id).expect("Proposal not found");
+        let mut proposal = proposals
+            .get(proposal_id)
+            .ok_or(GovernanceError::GovernanceProposalNotFound)?;
 
-        // Validation
         if env.ledger().timestamp() > proposal.end_time {
-            panic!("Voting period ended");
+            return Err(GovernanceError::VotingPeriodEnded);
         }
         if proposal.status != ProposalStatus::Active {
-            panic!("Proposal not active");
+            return Err(GovernanceError::GovernanceProposalNotActive);
         }
 
         // Check for double voting using a composite key
         let vote_key = (symbol_short!("votes"), proposal_id, voter.clone());
         if env.storage().persistent().has(&vote_key) {
-            panic!("Already voted");
+            return Err(GovernanceError::AlreadyVoted);
         }
 
         // Update Tally
@@ -122,20 +124,24 @@ impl GovernanceManager {
             (symbol_short!("gov"), symbol_short!("vote")),
             (proposal_id, voter, weight, support),
         );
+
+        Ok(())
     }
 
     /// Finalizes a proposal after the voting period has ended.
-    pub fn finalize_proposal(env: &Env, proposal_id: u64) {
+    pub fn finalize_proposal(env: &Env, proposal_id: u64) -> Result<(), GovernanceError> {
         let mut proposals: soroban_sdk::Map<u64, Proposal> = env
             .storage()
             .persistent()
             .get(&PROPOSALS)
-            .expect("Proposals not initialized");
+            .ok_or(GovernanceError::ProposalsNotInitialized)?;
 
-        let mut proposal = proposals.get(proposal_id).expect("Proposal not found");
+        let mut proposal = proposals
+            .get(proposal_id)
+            .ok_or(GovernanceError::GovernanceProposalNotFound)?;
 
         if env.ledger().timestamp() <= proposal.end_time {
-            panic!("Voting still in progress");
+            return Err(GovernanceError::VotingStillInProgress);
         }
 
         if proposal.for_votes > proposal.against_votes {
@@ -151,5 +157,7 @@ impl GovernanceManager {
             (symbol_short!("gov"), symbol_short!("prop_end")),
             (proposal_id, proposal.status),
         );
+
+        Ok(())
     }
 }

contracts/teachlink/src/lib.rs

@@ -177,7 +177,7 @@ pub use crate::types::{
 pub use assessment::{
     Assessment, AssessmentSettings, AssessmentSubmission, Question, QuestionType,
 };
-pub use errors::{BridgeError, EscrowError, MobilePlatformError, RewardsError};
+pub use errors::{BridgeError, EscrowError, GovernanceError, MobilePlatformError, RewardsError};
 pub use repository::{
     BridgeRepository, EscrowAggregateRepository, GenericCounterRepository, GenericMapRepository,
     SingleValueRepository, StorageError,
@@ -261,23 +261,23 @@ impl TeachLinkBridge {
     // ========== Admin Functions ==========
 
     /// Add a validator (admin only)
-    pub fn add_validator(env: Env, validator: Address) {
-        let _ = bridge::Bridge::add_validator(&env, validator);
+    pub fn add_validator(env: Env, validator: Address) -> Result<(), BridgeError> {
+        bridge::Bridge::add_validator(&env, validator)
     }
 
     /// Remove a validator (admin only)
-    pub fn remove_validator(env: Env, validator: Address) {
-        let _ = bridge::Bridge::remove_validator(&env, validator);
+    pub fn remove_validator(env: Env, validator: Address) -> Result<(), BridgeError> {
+        bridge::Bridge::remove_validator(&env, validator)
     }
 
     /// Add a supported destination chain (admin only)
-    pub fn add_supported_chain(env: Env, chain_id: u32) {
-        let _ = bridge::Bridge::add_supported_chain(&env, chain_id);
+    pub fn add_supported_chain(env: Env, chain_id: u32) -> Result<(), BridgeError> {
+        bridge::Bridge::add_supported_chain(&env, chain_id)
     }
 
     /// Remove a supported destination chain (admin only)
-    pub fn remove_supported_chain(env: Env, chain_id: u32) {
-        let _ = bridge::Bridge::remove_supported_chain(&env, chain_id);
+    pub fn remove_supported_chain(env: Env, chain_id: u32) -> Result<(), BridgeError> {
+        bridge::Bridge::remove_supported_chain(&env, chain_id)
     }
 
     /// Set bridge fee (admin only)
@@ -286,8 +286,8 @@ impl TeachLinkBridge {
     }
 
     /// Set fee recipient (admin only)
-    pub fn set_fee_recipient(env: Env, fee_recipient: Address) {
-        let _ = bridge::Bridge::set_fee_recipient(&env, fee_recipient);
+    pub fn set_fee_recipient(env: Env, fee_recipient: Address) -> Result<(), BridgeError> {
+        bridge::Bridge::set_fee_recipient(&env, fee_recipient)
     }
 
     /// Set minimum validators (admin only)

contracts/teachlink/src/validation_tests.rs

@@ -791,16 +791,15 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "Unauthorized: Missing required role")]
     fn test_rbac_unauthorized_fails() {
         use crate::access_control::AccessControlManager;
+        use crate::errors::BridgeError;
         use crate::types::AccessRole;
         let env = Env::default();
         let user = Address::generate(&env);
 
-        // Mock auth for user but they don't have the role
-        user.require_auth();
-        AccessControlManager::check_role(&env, &user, AccessRole::BridgeOperator);
+        let result = AccessControlManager::check_role(&env, &user, AccessRole::BridgeOperator);
+        assert_eq!(result, Err(BridgeError::Unauthorized));
     }
 
     #[test]