From 634d82305383b6bfae00ed2ba666f33352d38d92 Mon Sep 17 00:00:00 2001 From: Yggdrasil Date: Wed, 14 Oct 2026 04:38:36 -0400 Subject: [PATCH 1/2] =?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 dcf0cccd76b166ed65ad7897b384d7021c16a17d Mon Sep 17 00:00:00 2001 From: yggdrasilworld Date: Fri, 16 Oct 2026 00:51:22 -0400 Subject: [PATCH 2/2] 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 59d14178..dfd233d0 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, @@ -369,8 +370,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 +436,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 +2153,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 764e279d..548dd1f9 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 _}, @@ -200,8 +201,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); @@ -1037,7 +1038,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); }