From 634d82305383b6bfae00ed2ba666f33352d38d92 Mon Sep 17 00:00:00 2001 From: Yggdrasil Date: Wed, 14 Oct 2026 04:38:36 -0400 Subject: [PATCH 1/3] =?UTF-8?q?fix(milestones):=20deallocate=20stays=20ope?= =?UTF-8?q?n=20while=20paused=20=E2=80=94=20allocated=20funds=20were=20fro?= =?UTF-8?q?zen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pause() was meant to block NEW risky ops (allocate/release/contribute) but keep recovery paths open — its own doc says so. deallocate was pause- blocked though, so during a pause a sponsor couldn't un-allocate funds back to remaining_budget (or refund on a closed milestone) — allocated funds were frozen. Deallocate is a recovery path; it no longer checks is_paused. Resolves #530 Tests: updated test_deallocate_blocked_while_paused + the pause-coverage test to assert the recovery-path semantics. Note: couldn't run cargo test — the workspace needs rustc 1.95 (installed: 1.92); CI verifies. --- contracts/milestones/src/lib.rs | 11 +++++------ contracts/milestones/src/test.rs | 14 ++++++++------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/contracts/milestones/src/lib.rs b/contracts/milestones/src/lib.rs index 90c53d76..61100ede 100644 --- a/contracts/milestones/src/lib.rs +++ b/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); diff --git a/contracts/milestones/src/test.rs b/contracts/milestones/src/test.rs index 981ea64d..764e279d 100644 --- a/contracts/milestones/src/test.rs +++ b/contracts/milestones/src/test.rs @@ -424,7 +424,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 +439,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,8 +1029,8 @@ 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); From e848100152b7ae6330a40318ec2e91115058d351 Mon Sep 17 00:00:00 2001 From: Yggdrasil Date: Wed, 14 Oct 2026 04:44:07 -0400 Subject: [PATCH 2/3] test: cover the untested error-paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - escrow #516: initialize rejects treasury == contract (InvalidTreasury) - milestones #518: same InvalidTreasury; #519: deallocate an unallocated issue (IssueNotAllocatedForDeallocate); #525: get_oracle returns the configured oracle - maintenance-pool #521: InvalidTreasury; #522: reclaim_deposit rejects a different sponsor (NotDepositSponsor) Note on #517/#520/#523/#524 (ArithmeticOverflow + DepositCountOverflow): those checked_add paths are defensive — the token's i128 supply bounds escrow.amount / contribution.amount / deposit-count, so a caller can't push them past overflow through the public API. No honest test can trigger them; flagged here rather than writing a fake one. --- contracts/escrow/src/test.rs | 15 +++++++++ contracts/maintenance-pool/src/test.rs | 34 ++++++++++++++++++++ contracts/milestones/src/test.rs | 44 ++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 59d14178..2353f23d 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -52,6 +52,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(); diff --git a/contracts/maintenance-pool/src/test.rs b/contracts/maintenance-pool/src/test.rs index 1b0c34ce..f47ab9af 100644 --- a/contracts/maintenance-pool/src/test.rs +++ b/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(); diff --git a/contracts/milestones/src/test.rs b/contracts/milestones/src/test.rs index 764e279d..5df73962 100644 --- a/contracts/milestones/src/test.rs +++ b/contracts/milestones/src/test.rs @@ -44,6 +44,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(); From d85e6c7dfd872ab0aa628f022d19d91c8a0b51fb Mon Sep 17 00:00:00 2001 From: yggdrasilworld Date: Fri, 16 Oct 2026 00:53:22 -0400 Subject: [PATCH 3/3] test: contract-path imports + pause/pool expectations --- contracts/escrow/src/test.rs | 10 ++++++---- contracts/milestones/src/test.rs | 7 ++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 2353f23d..a67f4eab 100644 --- a/contracts/escrow/src/test.rs +++ b/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, @@ -384,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); @@ -450,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); @@ -2167,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); diff --git a/contracts/milestones/src/test.rs b/contracts/milestones/src/test.rs index 5df73962..4ec8365d 100644 --- a/contracts/milestones/src/test.rs +++ b/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 _}, @@ -244,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); @@ -1081,7 +1082,7 @@ fn test_pause_blocks_commitment_paths_but_allows_cancel() { 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); }