diff --git a/contracts/teachlink/src/access_control.rs b/contracts/teachlink/src/access_control.rs index f1a086ff..753d8a5a 100644 --- a/contracts/teachlink/src/access_control.rs +++ b/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> = 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> = env .storage() diff --git a/contracts/teachlink/src/audit.rs b/contracts/teachlink/src/audit.rs index bd238210..952cbd91 100644 --- a/contracts/teachlink/src/audit.rs +++ b/contracts/teachlink/src/audit.rs @@ -341,7 +341,7 @@ impl AuditManager { env, &admin, crate::types::AccessRole::AuditManager, - ); + )?; let audit_records: Map = env .storage() diff --git a/contracts/teachlink/src/bridge.rs b/contracts/teachlink/src/bridge.rs index f9c5aa7c..0ee2502c 100644 --- a/contracts/teachlink/src/bridge.rs +++ b/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); diff --git a/contracts/teachlink/src/emergency.rs b/contracts/teachlink/src/emergency.rs index 1f1a8965..94bfbf0e 100644 --- a/contracts/teachlink/src/emergency.rs +++ b/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 = env .storage() @@ -162,7 +162,7 @@ impl EmergencyManager { env, &resumer, crate::types::AccessRole::EmergencyManager, - ); + )?; let mut paused_chains: Map = env .storage() @@ -338,7 +338,7 @@ impl EmergencyManager { env, &resetter, crate::types::AccessRole::EmergencyManager, - ); + )?; let mut circuit_breakers: Map = env .storage() @@ -458,7 +458,7 @@ impl EmergencyManager { env, &updater, crate::types::AccessRole::EmergencyManager, - ); + )?; let mut circuit_breakers: Map = env .storage() diff --git a/contracts/teachlink/src/errors.rs b/contracts/teachlink/src/errors.rs index f425222f..31002f78 100644 --- a/contracts/teachlink/src/errors.rs +++ b/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, } diff --git a/contracts/teachlink/src/governance.rs b/contracts/teachlink/src/governance.rs index b37f7aee..8004db32 100644 --- a/contracts/teachlink/src/governance.rs +++ b/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 = 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 = 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(()) } } diff --git a/contracts/teachlink/src/lib.rs b/contracts/teachlink/src/lib.rs index 66d50fb0..7ca02f3e 100644 --- a/contracts/teachlink/src/lib.rs +++ b/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) diff --git a/contracts/teachlink/src/validation_tests.rs b/contracts/teachlink/src/validation_tests.rs index c7ef8570..9e5c080b 100644 --- a/contracts/teachlink/src/validation_tests.rs +++ b/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]