Refactor code structure for improved readability and maintainability

contracts/escrow/src/lib.rs

@@ -98,6 +98,7 @@ pub struct ContractConfig {
 pub enum DataKey {
     Job(u64),
     JobLock(u64),
+    Config,
     Admin,
     AgentJudge,
     JobRegistry,
@@ -106,43 +107,6 @@ pub enum DataKey {
     FeeBps,
     MultisigConfig(u64), // Per-job multisig configuration
     UpgradeAdmin,
-    Treasury,
-    FeeBps,
-}
-
-#[contracttype]
-#[derive(Clone)]
-pub struct FeeConfigUpdatedEvent {
-    pub treasury: Address,
-    pub fee_bps: u32,
-    pub updated_at: u64,
-}
-
-#[contracttype]
-#[derive(Clone)]
-pub struct LockupUpdatedEvent {
-    pub job_id: u64,
-    pub expires_at: u64,
-    pub updated_at: u64,
-}
-
-#[contracttype]
-#[derive(Clone)]
-pub struct EmergencySweepEvent {
-    pub job_id: u64,
-    pub admin: Address,
-    pub rescue_address: Address,
-    pub amount: i128,
-    pub swept_at: u64,
-}
-
-#[contracttype]
-#[derive(Clone)]
-pub struct MilestonesAmendedEvent {
-    pub job_id: u64,
-    pub milestone_count: u32,
-    pub remaining_amount: i128,
-    pub amended_at: u64,
 }
 
 #[contracttype]
@@ -182,7 +146,19 @@ pub enum EscrowError {
     NoPendingMilestones = 8,
     JobRegistrySyncFailed = 9,
     UpgradeUnauthorized = 10,
-    ReentrantCall = 11,
+    InvalidStateTransition = 11,
+    ReentrancyDetected = 12,
+    MultisigRequired = 13,
+    InsufficientSignatures = 14,
+    AlreadySigned = 15,
+    ArithmeticError = 16,
+    UpgradeAdminAlreadySet = 17,
+    UpgradeAdminNotSet = 18,
+    ArithmeticOverflow = 19,
+    DisputeResolutionExpired = 20,
+    FeeTooHigh = 21,
+    NothingToSweep = 22,
+    ReentrantCall = 23,
 }
 
 /// Maximum platform fee, in basis points (100% = 10_000 bps).
@@ -343,6 +319,10 @@ fn enter_reentrancy_guard(env: &Env) -> ReentrancyGuard<'_> {
     ReentrancyGuard { env }
 }
 
+fn exit_reentrancy_guard(env: &Env) {
+    env.storage().instance().remove(&DataKey::Locked);
+}
+
 fn checked_i128_add(lhs: i128, rhs: i128) -> Result<i128, EscrowError> {
     lhs.checked_add(rhs).ok_or(EscrowError::InvalidInput)
 }
@@ -390,16 +370,27 @@ impl EscrowContract {
         env.storage().temporary().remove(&lock_key);
     }
 
-fn checked_sub_i128(
-    env: &Env,
-    a: i128,
-    b: i128,
-) -> Result<i128, EscrowError> {
-    a.checked_sub(b).ok_or_else(|| {
-        log!(env, "checked_sub_i128 underflow: {} - {}", a, b);
-        EscrowError::InvalidInput
-    })
-}
+    fn checked_sub_i128(env: &Env, a: i128, b: i128) -> Result<i128, EscrowError> {
+        a.checked_sub(b).ok_or_else(|| {
+            log!(env, "checked_sub_i128 underflow: {} - {}", a, b);
+            EscrowError::InvalidInput
+        })
+    }
+
+    fn checked_add_i128(env: &Env, a: i128, b: i128) -> Result<i128, EscrowError> {
+        a.checked_add(b).ok_or_else(|| {
+            log!(env, "checked_add_i128 overflow: {} + {}", a, b);
+            EscrowError::ArithmeticOverflow
+        })
+    }
+
+    fn assert_not_same_ledger_as_funding(env: &Env, job: &EscrowJob) -> Result<(), EscrowError> {
+        if job.funded_ledger_seq != 0 && env.ledger().sequence() == job.funded_ledger_seq {
+            return Err(EscrowError::InvalidState);
+        }
+        Ok(())
+    }
+
     fn sync_dispute_to_job_registry(env: &Env, job_id: u64) -> Result<(), EscrowError> {
         Self::bump_instance_ttl(env);
         let Some(registry_contract) = env
@@ -645,19 +636,15 @@ fn checked_sub_i128(
             return Err(EscrowError::InvalidInput);
         }
         let now: u64 = env.ledger().timestamp();
-let expires_duration = 30u64
-    .checked_mul(24)
-    .and_then(|h| h.checked_mul(60))
-    .and_then(|m| m.checked_mul(60))
-let expires_duration = 30u64
-    .checked_mul(24)
-    .and_then(|h| h.checked_mul(60))
-    .and_then(|m| m.checked_mul(60))
-    .ok_or(EscrowError::ArithmeticError)?;
-
-let expires_at = now
-    .checked_add(expires_duration)
-    .ok_or(EscrowError::ArithmeticError)?;
+        let expires_duration = 30u64
+            .checked_mul(24)
+            .and_then(|h| h.checked_mul(60))
+            .and_then(|m| m.checked_mul(60))
+            .ok_or(EscrowError::ArithmeticError)?;
+
+        let expires_at = now
+            .checked_add(expires_duration)
+            .ok_or(EscrowError::ArithmeticError)?;
 
         let job = EscrowJob {
             client: client.clone(),
@@ -884,21 +871,19 @@ let expires_at = now
             .ok_or(EscrowError::JobNotFound)?;
         Self::bump_job_ttl(&env, &key);
 
-Self::assert_not_same_ledger_as_funding(&env, &job)?;
+        Self::assert_not_same_ledger_as_funding(&env, &job)?;
 
-if !(job.status == EscrowStatus::Funded
-    || job.status == EscrowStatus::WorkInProgress)
-{
-    return Err(EscrowError::InvalidState);
-}
+        if !(job.status == EscrowStatus::Funded || job.status == EscrowStatus::WorkInProgress) {
+            return Err(EscrowError::InvalidState);
+        }
 
-if caller != job.client {
-    return Err(EscrowError::Unauthorized);
-}
+        if caller != job.client {
+            return Err(EscrowError::Unauthorized);
+        }
 
-if milestone_index >= job.milestones.len() {
-    return Err(EscrowError::InvalidInput);
-}
+        if milestone_index >= job.milestones.len() {
+            return Err(EscrowError::InvalidInput);
+        }
 
         let mut milestone = job
             .milestones
@@ -940,6 +925,8 @@ if milestone_index >= job.milestones.len() {
         env.storage().persistent().set(&key, &job);
         Self::bump_job_ttl(&env, &key);
         Self::exit_job_lock(&env, lock_key);
+
+        Ok(())
     }
 
     /// Either party opens a dispute, locking remaining funds.
@@ -1002,13 +989,11 @@ if milestone_index >= job.milestones.len() {
         }
 
         // 3. Job must still be active
-Self::assert_not_same_ledger_as_funding(&env, &job)?;
+        Self::assert_not_same_ledger_as_funding(&env, &job)?;
 
-if !(job.status == EscrowStatus::Funded
-    || job.status == EscrowStatus::WorkInProgress)
-{
-    return Err(EscrowError::InvalidState);
-}
+        if !(job.status == EscrowStatus::Funded || job.status == EscrowStatus::WorkInProgress) {
+            return Err(EscrowError::InvalidState);
+        }
 
         // 4. Prevent dispute if all funds are already released
         if job.released_amount >= job.total_amount {
@@ -1139,14 +1124,16 @@ if !(job.status == EscrowStatus::Funded
         env.storage().persistent().set(&key, &job);
         Self::bump_job_ttl(&env, &key);
         Self::exit_job_lock(&env, lock_key);
+
+        Ok(())
     }
 
     /// Client recoups funds if freelancer never responded or deadline has passed.
     pub fn refund(env: Env, job_id: u64, client: Address) -> Result<(), EscrowError> {
         client.require_auth();
 
         let key = DataKey::Job(job_id);
-        let mut job: EscrowJob = env
+        let job: EscrowJob = env
             .storage()
             .persistent()
             .get(&key)
@@ -1375,10 +1362,10 @@ if !(job.status == EscrowStatus::Funded
             job_id,
             remaining
         );
-env.storage().persistent().set(&key, &job);
-Self::bump_job_ttl(&env, &key);
+        env.storage().persistent().set(&key, &job);
+        Self::bump_job_ttl(&env, &key);
 
-exit_reentrancy_guard(&env);
+        exit_reentrancy_guard(&env);
         env.events().publish(
             ("escrow", "DisputeExpired"),
             DisputeExpiredEvent {
@@ -1435,18 +1422,6 @@ exit_reentrancy_guard(&env);
         Ok((config.admin, config.agent_judge, job_registry))
     }
 
-    /// Read-only helper exposing unreleased escrow balance for a job.
-    pub fn get_remaining_balance(env: Env, job_id: u64) -> Result<i128, EscrowError> {
-        let key = DataKey::Job(job_id);
-        let job: EscrowJob = env
-            .storage()
-            .persistent()
-            .get(&key)
-            .ok_or(EscrowError::JobNotFound)?;
-        Self::bump_job_ttl(&env, &key);
-        Self::checked_sub_i128(&env, job.total_amount, job.released_amount)
-    }
-
     /// Configure multisig for a job. Only callable by client during Setup phase.
     pub fn configure_multisig(
         env: Env,
@@ -1598,8 +1573,7 @@ exit_reentrancy_guard(&env);
             .instance()
             .get(&DataKey::Config)
             .ok_or(EscrowError::NotInitialized)?;
-        let admin = config.admin;
-        admin.require_auth();
+        config.admin.require_auth();
 
         if fee_bps > MAX_FEE_BPS {
             return Err(EscrowError::FeeTooHigh);
@@ -1623,7 +1597,7 @@ exit_reentrancy_guard(&env);
 
     /// Returns the active platform fee in basis points (0 when unset).
     pub fn get_fee_bps(env: Env) -> u32 {
-        Self::fee_bps(&env)
+        env.storage().instance().get(&DataKey::FeeBps).unwrap_or(0)
     }
 
     /// Returns the configured treasury address, if any.
@@ -1639,7 +1613,10 @@ exit_reentrancy_guard(&env);
     }
 
     fn payout_with_fee(env: &Env, job: &EscrowJob, amount: i128) {
-        let treasury = env.storage().instance().get::<_, Address>(&DataKey::Treasury);
+        let treasury = env
+            .storage()
+            .instance()
+            .get::<_, Address>(&DataKey::Treasury);
         let fee_bps = Self::fee_bps(env);
         let fee_amount = if fee_bps > 0 {
             amount
@@ -1658,7 +1635,11 @@ exit_reentrancy_guard(&env);
             }
         }
         if payout_amount > 0 {
-            token_client.transfer(&env.current_contract_address(), &job.freelancer, &payout_amount);
+            token_client.transfer(
+                &env.current_contract_address(),
+                &job.freelancer,
+                &payout_amount,
+            );
         }
     }
 
@@ -1744,8 +1725,7 @@ exit_reentrancy_guard(&env);
             .instance()
             .get(&DataKey::Config)
             .ok_or(EscrowError::NotInitialized)?;
-        let admin = config.admin;
-        admin.require_auth();
+        config.admin.require_auth();
 
         let key = DataKey::Job(job_id);
         let mut job: EscrowJob = env
@@ -1781,7 +1761,7 @@ exit_reentrancy_guard(&env);
             ("escrow", "EmergencySweep"),
             EmergencySweepEvent {
                 job_id,
-                admin,
+                admin: config.admin,
                 rescue_address,
                 amount: remaining,
                 swept_at: env.ledger().timestamp(),
@@ -1873,56 +1853,6 @@ exit_reentrancy_guard(&env);
 
         Ok(())
     }
-
-    fn fee_bps(env: &Env) -> u32 {
-        env.storage().instance().get(&DataKey::FeeBps).unwrap_or(0)
-    }
-
-    fn payout_with_fee(
-        env: &Env,
-        _job_id: u64,
-        job: &EscrowJob,
-        amount: i128,
-    ) {
-        let treasury_opt: Option<Address> = env.storage().instance().get(&DataKey::Treasury);
-        let fee_bps = Self::fee_bps(env);
-        let token_client = token::Client::new(env, &job.token);
-
-        if let Some(treasury) = treasury_opt {
-            if fee_bps > 0 && fee_bps <= 10_000 {
-                // fee_amount = amount * fee_bps / 10_000
-                let fee_amount = amount
-                    .checked_mul(fee_bps as i128)
-                    .unwrap()
-                    .checked_div(10_000)
-                    .unwrap();
-                let freelancer_amount = amount.checked_sub(fee_amount).unwrap();
-
-                if fee_amount > 0 {
-                    token_client.transfer(
-                        &env.current_contract_address(),
-                        &treasury,
-                        &fee_amount,
-                    );
-                }
-                if freelancer_amount > 0 {
-                    token_client.transfer(
-                        &env.current_contract_address(),
-                        &job.freelancer,
-                        &freelancer_amount,
-                    );
-                }
-                return;
-            }
-        }
-
-        // Default: no fee or fee is 0 or treasury not configured
-        token_client.transfer(
-            &env.current_contract_address(),
-            &job.freelancer,
-            &amount,
-        );
-    }
 }
 
 #[cfg(test)]