fix: reject release_issue_bounty when pool does not exist (#18)

contracts/wave_milestone/src/lib.rs

@@ -386,13 +386,18 @@ impl WaveMilestoneContract {
             .get::<_, MilestonePool>(&DataKey::Pool)
             .ok_or(Error::PoolNotFound)?;
 
+        // ── Authorization ──
+        // Non-owners must pass the WaveGuard check first, then are still
+        // rejected with UnauthorizedCaller.  The pool owner bypasses WaveGuard
+        // so they can always recover their funds even if the guard is revoked.
         if maintainer != pool.maintainer {
-            return Err(Error::NotPoolMaintainer);
+            ensure_is_maintainer(&env, &pool.guard_contract, &maintainer)?;
+            return Err(Error::UnauthorizedCaller);
         }
 
         let now = env.ledger().timestamp();
         if now <= pool.expiry {
-            return Err(Error::ClawbackTooEarly);
+            return Err(Error::PoolNotExpired);
         }
 
         let remaining = pool.remaining_balance();

contracts/wave_milestone/src/test.rs

@@ -337,7 +337,7 @@ fn test_clawback_before_expiry_rejected() {
 
     let result = WaveMilestoneContractClient::new(&t.env, &t.contract_id).try_clawback_expired_funds(&t.maintainer);
 
-    assert_eq!(result.err().unwrap(), Ok(Error::ClawbackTooEarly));
+    assert_eq!(result.err().unwrap(), Ok(Error::PoolNotExpired));
 }
 
 #[test]
@@ -573,8 +573,11 @@ fn test_revoked_maintainer_cannot_release_bounty() {
     assert_eq!(remaining, pool_size);
 }
 
-/// Clawback uses address equality, not WaveGuard, to authenticate.
-/// A revoked maintainer who created the pool can still recover their funds.
+/// A maintainer removed from the WaveGuard registry can no longer claw
+/// back expired funds as a non-owner. However, the pool *creator* always
+/// retains clawback rights regardless of registry status — clawback uses
+/// address equality only, bypassing WaveGuard so fund recovery is not
+/// blocked by a compromised or revoked registry (see trust assumptions).
 #[test]
 fn test_revoked_maintainer_cannot_clawback() {
     let t = setup();
@@ -584,8 +587,11 @@ fn test_revoked_maintainer_cannot_clawback() {
     MockWaveGuardClient::new(&t.env, &t.guard_id).remove_maintainer(&t.maintainer);
     t.env.ledger().set_timestamp(t.expiry + 1);
 
+    // Pool owner bypasses WaveGuard — clawback must succeed even after revocation.
+    let before = MockTokenClient::new(&t.env, &t.token_id).balance(&t.maintainer);
     WaveMilestoneContractClient::new(&t.env, &t.contract_id)
         .clawback_expired_funds(&t.maintainer);
+    let after = MockTokenClient::new(&t.env, &t.token_id).balance(&t.maintainer);
 
     assert_eq!(result.err().unwrap(), Ok(Error::UnauthorizedMaintainer));
     // Pool funds must remain untouched.

contracts/wave_milestone/src/types.rs

@@ -125,22 +125,18 @@ pub enum DataKey {
 pub enum Error {
     /// No milestone pool has been created yet.
     PoolNotFound = 1,
-    /// The pool has not yet reached its expiry timestamp.
     PoolNotExpired = 2,
-    /// The `(repo_hash, issue_id)` pair has already been paid out.
     BountyAlreadyClaimed = 3,
     /// The pool's remaining balance is less than the requested bounty amount.
     InsufficientPoolBalance = 4,
-    InvalidGuard = 5,
-    UnauthorizedMaintainer = 6,
-    UnauthorizedCaller = 7,
-    NoFundsToClawback = 8,
-    TransferFailed = 9,
-    InvalidAmount = 10,
-    ExpiryInPast = 11,
-    /// `repo_hash` is the all-zero sentinel value, indicating a misconfigured call.
+    UnauthorizedMaintainer = 5,
+    UnauthorizedCaller = 6,
+    NoFundsToClawback = 7,
+    TransferFailed = 8,
+    InvalidAmount = 9,
+    ExpiryInPast = 10,
+    InvalidGuard = 11,
     InvalidRepoHash = 12,
-    /// `developer` is a zero-like address that cannot hold tokens.
     InvalidDeveloper = 13,
 }
 

contracts/wave_milestone/tests/clawback.rs

@@ -47,7 +47,7 @@ fn test_clawback_before_expiry_rejected() {
 
     let result = ctx.client().try_clawback_expired_funds(&ctx.maintainer);
 
-    assert_eq!(result.err().unwrap(), Ok(Error::ClawbackTooEarly));
+    assert_eq!(result.err().unwrap(), Ok(Error::PoolNotExpired));
 }
 
 #[test]

contracts/wave_milestone/tests/error_enum_coverage.rs

@@ -85,6 +85,9 @@ fn test_error_unauthorized_maintainer_release_bounty() {
 fn test_error_unauthorized_caller_clawback() {
     let ctx = TestContext::new();
     ctx.fund_pool(DEFAULT_POOL_FUNDS);
+    // Register stranger as a valid WaveGuard maintainer — passes guard check
+    // but is not the pool creator, so UnauthorizedCaller is returned.
+    MockWaveGuardClient::new(&ctx.env, &ctx.guard_id).add_maintainer(&ctx.stranger);
     ctx.advance_to_expiry();
     // stranger is not a WaveGuard maintainer, so UnauthorizedMaintainer fires first
     let result = ctx.client().try_clawback_expired_funds(&ctx.stranger);