From b9e851921a03e5bdd58bd9510fbc8c1cb9c20511 Mon Sep 17 00:00:00 2001 From: gideononiru Date: Sun, 30 Aug 2026 08:26:40 +0100 Subject: [PATCH 1/2] fix: block release_issue on a closed milestone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #5 cancel_milestone / cancel_milestone_after_deadline only refund remaining_budget (the unallocated portion) to the sponsor — amounts already reserved via allocate() live in milestone.allocations and are not part of that refund. release_issue checked IssueStatus (Allocated/Released) but never checked milestone.closed, so an issue left in IssueStatus::Allocated when the milestone closed could still be released afterward: real token amounts the sponsor's refund had already implicitly treated as settled would leave the contract on a later release_issue call. Error::MilestoneClosed already existed and is already used by contribute/allocate/cancel_milestone/cancel_milestone_after_deadline — release_issue was the one call site missing it. Added the same check as the very first thing release_issue does after loading the milestone, before touching IssueStatus or moving any funds. This doesn't strand the allocated amount: deallocate() (added alongside this fix) is the admin's path to reclaim an Allocated, not-yet-Released issue's amount back into remaining_budget — either before cancelling (recommended) or, if an issue was left allocated through a cancel, after the fact via deallocate + a fresh create_milestone/contribute cycle if the funds still need to reach their intended recipient. --- contracts/milestones/src/lib.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/contracts/milestones/src/lib.rs b/contracts/milestones/src/lib.rs index fcddf6d..a441ff1 100644 --- a/contracts/milestones/src/lib.rs +++ b/contracts/milestones/src/lib.rs @@ -308,6 +308,20 @@ impl MilestonesContract { .get(&mkey) .ok_or(Error::MilestoneNotFound)?; + // #5: cancel_milestone/cancel_milestone_after_deadline only refund + // remaining_budget (the *unallocated* portion) — amounts already + // reserved via allocate() are not included and are not refunded, on + // the assumption that a closed milestone can't pay them out later + // either. Without this check that assumption was false: an issue + // left in IssueStatus::Allocated when the milestone closed could + // still be released afterward, silently paying out funds the + // sponsor's refund had already implicitly accounted as settled. + // (`deallocate` is the intended way to reclaim an allocation before + // closing, exactly to avoid needing this path.) + if milestone.closed { + return Err(Error::MilestoneClosed); + } + let skey = DataKey::IssueStatus(milestone_id, issue_id); let status: IssueStatus = env .storage() From eea6873136e19506f30981b27c0259f582e4caca Mon Sep 17 00:00:00 2001 From: gideononiru Date: Sun, 30 Aug 2026 08:26:48 +0100 Subject: [PATCH 2/2] test: cover release_issue rejecting an allocated issue after cancel Part of #5 Regression test for the fix in the previous commit: allocates an issue, cancels the milestone (only the unallocated remainder is refunded to the sponsor), then asserts release_issue on the still- Allocated issue is rejected with MilestoneClosed and the allocated amount stays untouched in the contract (not paid to the maintainer, not silently lost) rather than being released post-cancellation. --- contracts/milestones/src/test.rs | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/contracts/milestones/src/test.rs b/contracts/milestones/src/test.rs index 4a7fa92..75a7379 100644 --- a/contracts/milestones/src/test.rs +++ b/contracts/milestones/src/test.rs @@ -1201,6 +1201,45 @@ fn test_state_machine_cancel_milestone_preserves_issue_statuses() { assert_eq!(client.get_issue_status(&903u64, &9032u64), IssueStatus::Allocated); } +#[test] +fn test_release_issue_rejects_allocated_issue_after_cancel() { + // #5: cancel_milestone only refunds remaining_budget (the *unallocated* + // portion) to the sponsor — the 3_000 already reserved for issue 9032 + // via allocate() is not part of that refund. Before this fix, + // release_issue never checked milestone.closed, so that still-Allocated + // issue could be released *after* cancellation, paying out funds the + // sponsor's refund had already implicitly treated as settled. + 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_000i128); + + client.create_milestone(&950u64, &sponsor, &token_addr, &10_000i128, &1_000u64); + client.allocate(&950u64, &9501u64, &3_000i128); + + // remaining_budget (7_000) is refunded to the sponsor; the 3_000 + // allocated to 9501 stays in the contract, still Allocated. + client.cancel_milestone(&950u64); + assert!(client.get_milestone(&950u64).closed); + assert_eq!(client.get_issue_status(&950u64, &9501u64), IssueStatus::Allocated); + assert_eq!(token_client.balance(&sponsor), 7_000i128); + + let maintainer = Address::generate(&env); + let result = + client.try_release_issue(&950u64, &9501u64, &vec![&env, (maintainer.clone(), 10_000u32)]); + assert_eq!(result, Err(Ok(Error::MilestoneClosed))); + + // The 3_000 is still sitting in the contract, unpaid — deallocate is the + // documented path to reclaim it (rather than release_issue silently + // paying it out post-cancellation). + assert_eq!(token_client.balance(&maintainer), 0i128); + assert_eq!(token_client.balance(&client.address), 3_000i128); +} + #[test] fn test_state_machine_cancel_milestone_rejects_double_cancel() { let env = Env::default();