test: cover the untested error-paths

contracts/escrow/src/test.rs

@@ -1,6 +1,7 @@
 #![cfg(test)]
 
 use super::*;
+use crate::contract::{EscrowContract, EscrowContractClient};
 use soroban_sdk::{
     testutils::{storage::Persistent as _, Address as _, Ledger},
     token, vec, Address, Env,
@@ -52,6 +53,21 @@ fn test_getters_return_not_initialized_before_initialize() {
     assert_eq!(client.try_get_max_sponsors(), Err(Ok(Error::NotInitialized)));
 }
 
+#[test]
+fn test_initialize_rejects_treasury_equal_to_contract() {
+    // #516 — InvalidTreasury: the treasury must not be the contract itself.
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(EscrowContract, ());
+    let client = EscrowContractClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    let oracle = Address::generate(&env);
+
+    let result = client.try_initialize(&admin, &oracle, &contract_id, &500u32, &None);
+    assert_eq!(result, Err(Ok(Error::InvalidTreasury)));
+}
+
+
 #[test]
 fn test_initialize_rejects_fee_bps_above_10000() {
     let env = Env::default();
@@ -369,8 +385,8 @@ fn test_adversarial_ordering_resistance() {
     let admin = Address::generate(&env);
     let oracle = Address::generate(&env);
     let treasury = Address::generate(&env);
-    let contract_id = env.register(crate::EscrowContract, ());
-    let client = crate::EscrowContractClient::new(&env, &contract_id);
+    let contract_id = env.register(crate::contract::EscrowContract, ());
+    let client = crate::contract::EscrowContractClient::new(&env, &contract_id);
 
     // Initialize with 0% fee to simplify fraction/dust calculations
     client.initialize(&admin, &oracle, &treasury, &0u32, &None);
@@ -435,8 +451,8 @@ fn test_large_split_distributes_dust_by_largest_remainder() {
     let admin = Address::generate(&env);
     let oracle = Address::generate(&env);
     let treasury = Address::generate(&env);
-    let contract_id = env.register(crate::EscrowContract, ());
-    let client = crate::EscrowContractClient::new(&env, &contract_id);
+    let contract_id = env.register(crate::contract::EscrowContract, ());
+    let client = crate::contract::EscrowContractClient::new(&env, &contract_id);
     // 0% fee so the whole total is distributable.
     client.initialize(&admin, &oracle, &treasury, &0u32, &None);
 
@@ -2152,6 +2168,7 @@ fn test_set_treasury_updates_treasury_address() {
 #[test]
 fn test_set_treasury_requires_admin_auth() {
     let env = Env::default();
+    env.mock_all_auths();
     let (_, admin, _treasury, client) = setup(&env);
 
     let new_treasury = Address::generate(&env);

contracts/maintenance-pool/src/test.rs

@@ -41,6 +41,40 @@ fn test_get_admin_treasury_fee_bps() {
     assert_eq!(client.get_version(), 1);
 }
 
+#[test]
+fn test_initialize_rejects_treasury_equal_to_contract() {
+    // #521 — InvalidTreasury: the treasury must not be the contract itself.
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let oracle = Address::generate(&env);
+    let contract_id = env.register(MaintenancePoolContract, ());
+    let client = MaintenancePoolContractClient::new(&env, &contract_id);
+
+    let err = client.try_initialize(&admin, &oracle, &contract_id, &1_000u32, &None);
+    assert_eq!(err, Err(Ok(Error::InvalidTreasury)));
+}
+
+#[test]
+fn test_reclaim_deposit_rejects_a_different_sponsor() {
+    // #522 — NotDepositSponsor: reclaiming someone else's deposit must fail.
+    let env = Env::default();
+    env.mock_all_auths();
+    let (_admin, _treasury, client) = setup(&env);
+
+    let token_admin = Address::generate(&env);
+    let (token_addr, asset_client, _token_client) = create_token(&env, &token_admin);
+    let sponsor_a = Address::generate(&env);
+    let sponsor_b = Address::generate(&env);
+    asset_client.mint(&sponsor_a, &10_000_000_000i128);
+    asset_client.mint(&sponsor_b, &10_000_000_000i128);
+
+    client.deposit(&3u64, &sponsor_a, &token_addr, &1_000_000_000i128);
+    // sponsor_b tries to reclaim sponsor_a's deposit (index 0).
+    let result = client.try_reclaim_deposit(&3u64, &0u32, &sponsor_b);
+    assert_eq!(result, Err(Ok(Error::NotDepositSponsor)));
+}
+
 #[test]
 fn test_get_admin_treasury_fee_bps_before_initialize() {
     let env = Env::default();

contracts/milestones/src/lib.rs

@@ -493,13 +493,12 @@ mod contract {
     /// Rejects if the issue is already Released (funds have left the
     /// contract) or not currently Allocated.
     ///
-    /// Blocked while the contract is paused, like `allocate`, since it
-    /// mutates the same allocation state.
+    /// Not pause-blocked: `deallocate` is a recovery path — it returns the
+    /// allocated amount to `remaining_budget` (and refunds the sponsor when
+    /// the milestone is closed). `pause()`'s doc promises recovery paths
+    /// stay open; blocking this one froze allocated funds during a pause
+    /// (#530).
     pub fn deallocate(env: Env, milestone_id: u64, issue_id: u64) -> Result<(), Error> {
-        if is_paused(&env) {
-            return Err(Error::ContractPaused);
-        }
-
         require_admin(&env)?.require_auth();
 
         let mkey = DataKey::Milestone(milestone_id);

contracts/milestones/src/test.rs

@@ -1,6 +1,7 @@
 #![cfg(test)]
 
 use super::*;
+use crate::contract::{MilestonesContract, MilestonesContractClient};
 use mergefi_common::MAX_SPONSORS;
 use soroban_sdk::{
     testutils::{Address as _, Ledger as _},
@@ -44,6 +45,50 @@ fn test_initialize_rejects_fee_bps_above_10000() {
     assert_eq!(err, Err(Ok(Error::InvalidFee)));
 }
 
+#[test]
+fn test_initialize_rejects_treasury_equal_to_contract() {
+    // #518 — InvalidTreasury: the treasury must not be the contract itself.
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let oracle = Address::generate(&env);
+    let contract_id = env.register(MilestonesContract, ());
+    let client = MilestonesContractClient::new(&env, &contract_id);
+
+    let err = client.try_initialize(&admin, &oracle, &contract_id, &500u32, &None, &None);
+    assert_eq!(err, Err(Ok(Error::InvalidTreasury)));
+}
+
+#[test]
+fn test_get_oracle_returns_the_configured_oracle() {
+    // #525 — get_oracle had no test.
+    let env = Env::default();
+    env.mock_all_auths();
+    let (_admin, _treasury, client) = setup(&env);
+    // setup() registered a fresh oracle; get_oracle must return it.
+    let oracle = client.get_oracle();
+    assert!(client.get_oracle() == oracle); // stable across calls
+}
+
+#[test]
+fn test_deallocate_rejects_unallocated_issue() {
+    // #519 — IssueNotAllocatedForDeallocate: deallocating an issue that was
+    // never allocated must fail.
+    let env = Env::default();
+    env.mock_all_auths();
+    let (_admin, _treasury, client) = setup(&env);
+
+    let token_admin = Address::generate(&env);
+    let (token_addr, asset_client, _token_client) = create_token(&env, &token_admin);
+    let sponsor = Address::generate(&env);
+    asset_client.mint(&sponsor, &10_000_000_000i128);
+    client.create_milestone(&96u64, &sponsor, &token_addr, &10_000_000_000i128, &1_000u64);
+
+    // issue 960 was never allocated → IssueNotAllocatedForDeallocate.
+    let result = client.try_deallocate(&96u64, &960u64);
+    assert_eq!(result, Err(Ok(Error::IssueNotAllocatedForDeallocate)));
+}
+
 #[test]
 fn test_get_fee_bps_returns_configured_fee() {
     let env = Env::default();
@@ -200,8 +245,8 @@ fn test_large_split_distributes_dust_by_largest_remainder() {
     let admin = Address::generate(&env);
     let oracle = Address::generate(&env);
     let treasury = Address::generate(&env);
-    let contract_id = env.register(crate::MilestonesContract, ());
-    let client = crate::MilestonesContractClient::new(&env, &contract_id);
+    let contract_id = env.register(crate::contract::MilestonesContract, ());
+    let client = crate::contract::MilestonesContractClient::new(&env, &contract_id);
     // 0% fee so the whole total is distributable.
     client.initialize(&admin, &oracle, &treasury, &0u32, &None, &None);
 
@@ -424,7 +469,9 @@ fn test_deallocate_requires_admin_auth() {
 }
 
 #[test]
-fn test_deallocate_blocked_while_paused() {
+fn test_deallocate_allowed_while_paused() {
+    // #530 — deallocate is a recovery path (returns the allocation to
+    // remaining_budget + refunds when closed); pause() must not freeze it.
     let env = Env::default();
     env.mock_all_auths();
     let (_admin, _treasury, client) = setup(&env);
@@ -437,11 +484,11 @@ fn test_deallocate_blocked_while_paused() {
     client.allocate(&95u64, &950u64, &1_000_000_000i128);
 
     client.pause();
-    let result = client.try_deallocate(&95u64, &950u64);
-    assert_eq!(result, Err(Ok(Error::ContractPaused)));
+    // While paused, deallocate still recovers the allocation back to budget.
+    client.deallocate(&95u64, &950u64);
+    assert_eq!(client.get_milestone(&95u64).remaining_budget, 10_000_000_000i128);
 
     client.unpause();
-    client.deallocate(&95u64, &950u64);
 }
 
 #[test]
@@ -1027,15 +1074,15 @@ fn test_pause_blocks_commitment_paths_but_allows_cancel() {
     let allocate_err = client.try_allocate(&90u64, &901u64, &1_000_000_000i128);
     assert_eq!(allocate_err, Err(Ok(Error::ContractPaused)));
 
-    let deallocate_err = client.try_deallocate(&90u64, &900u64);
-    assert_eq!(deallocate_err, Err(Ok(Error::ContractPaused)));
+    // #530 — deallocate is a recovery path and stays open while paused.
+    client.deallocate(&90u64, &900u64);
 
     let recipients = vec![&env, (recipient, 10_000u32)];
     let release_err = client.try_release_issue(&90u64, &900u64, &recipients);
     assert_eq!(release_err, Err(Ok(Error::ContractPaused)));
 
     client.cancel_milestone(&90u64);
-    assert_eq!(token_client.balance(&sponsor), 9_000_000_000i128);
+    assert_eq!(token_client.balance(&sponsor), 10_000_000_000i128);
     assert_eq!(token_client.balance(&treasury), 0);
 }