fix(milestones): enforce issue_id uniqueness across milestones

README.md

@@ -249,8 +249,25 @@ fn get_contribution(env, milestone_id: u64, index: u32) -> Result<Contribution,
   `docs/milestones-crowdfunding-design.md` for the full design reasoning.
 - `allocate`: admin-only. Reserves a slice of `remaining_budget` for a
   specific `issue_id`. Over-allocating past what's left is rejected
-  (`OverAllocation`); allocating an issue twice is rejected
-  (`IssueAlreadyAllocated`).
+  (`OverAllocation`); allocating an issue twice within the same milestone
+  is rejected (`IssueAlreadyAllocated`).
+
+  **An `issue_id` can be allocated in at most one milestone at a time.**
+  `allocations` and `IssueStatus(milestone_id, issue_id)` are both scoped
+  to a single milestone, so on their own they could not stop `allocate(1,
+  555, x)` and `allocate(2, 555, y)` from both succeeding and both being
+  released — one merged PR paid for twice. A contract-instance-wide
+  registry, `GlobalIssueClaim(issue_id) -> milestone_id`, is written on
+  every successful `allocate`; an `allocate` naming an `issue_id` already
+  claimed by a *different* milestone is rejected with
+  `IssueClaimedByOtherMilestone`, a distinct error so callers can tell
+  "this milestone already has it" from "another milestone already has it".
+  The claim is released by `deallocate` when that lands, so removing an
+  allocation frees the issue for legitimate reallocation rather than
+  leaving a permanent false "already claimed". This is scoped to this
+  contract only; the same `issue_id` being funded through
+  `mergefi-escrow` as well remains the accepted, backend-mitigated gap
+  described under "Cross-contract double-funding" above.
 - `release_issue`: admin-only, same split/fee mechanics as escrow's
   `release`, but draws from the issue's pre-reserved allocation rather
   than a fresh deposit. Rejects double release (`IssueAlreadyReleased`).

contracts/milestones/src/error.rs

@@ -17,4 +17,8 @@ pub enum Error {
     InvalidFee = 11,
     MilestoneClosed = 12,
     TooManySponsors = 13,
+    /// Already allocated in a *different* milestone, as opposed to
+    /// `IssueAlreadyAllocated`, which means "already allocated in the
+    /// milestone you are calling against".
+    IssueClaimedByOtherMilestone = 14,
 }

contracts/milestones/src/lib.rs

@@ -173,6 +173,18 @@ impl MilestonesContract {
     /// `issue_id`. Rejects if the issue is already allocated, the milestone
     /// is closed, or `amount` exceeds the remaining (unallocated) budget.
     ///
+    /// An `issue_id` can be allocated in at most one milestone at a time.
+    /// `milestone.allocations` and `DataKey::IssueStatus(milestone_id,
+    /// issue_id)` are both scoped to the single `Milestone` record loaded
+    /// here, so neither can see that another milestone already committed
+    /// budget to the same GitHub issue — which let `allocate(1, 555, x)`
+    /// and `allocate(2, 555, y)` both succeed and both be released, paying
+    /// twice for one merged PR. `DataKey::GlobalIssueClaim(issue_id)`
+    /// closes that: an `allocate` from a *different* `milestone_id` is
+    /// rejected with `IssueClaimedByOtherMilestone`, leaving
+    /// `IssueAlreadyAllocated` to mean a repeat allocation within the
+    /// *same* milestone.
+    ///
     /// Note: this contract has no visibility into `mergefi-escrow` —
     /// nothing here stops the same `issue_id` from also being funded via
     /// `escrow::fund` as a standalone bounty. See README "Why three
@@ -198,6 +210,18 @@ impl MilestonesContract {
         if milestone.allocations.contains_key(issue_id) {
             return Err(Error::IssueAlreadyAllocated);
         }
+
+        // Checked after the per-milestone guard above, so a repeat
+        // allocation within this same milestone keeps reporting
+        // `IssueAlreadyAllocated` and only a genuine cross-milestone
+        // collision reaches this branch.
+        let ckey = DataKey::GlobalIssueClaim(issue_id);
+        if let Some(claimed_by) = env.storage().persistent().get::<DataKey, u64>(&ckey) {
+            if claimed_by != milestone_id {
+                return Err(Error::IssueClaimedByOtherMilestone);
+            }
+        }
+
         if amount > milestone.remaining_budget {
             return Err(Error::OverAllocation);
         }
@@ -213,6 +237,9 @@ impl MilestonesContract {
             .set(&skey, &IssueStatus::Allocated);
         extend_ttl(&env, &skey);
 
+        env.storage().persistent().set(&ckey, &milestone_id);
+        extend_ttl(&env, &ckey);
+
         Ok(())
     }
 

contracts/milestones/src/test.rs

@@ -559,3 +559,43 @@ fn test_get_contribution_enumerates_each_contributor() {
     let err = client.try_get_contribution(&57u64, &2u32);
     assert_eq!(err, Err(Ok(Error::MilestoneNotFound)));
 }
+
+#[test]
+fn test_allocate_rejects_issue_already_claimed_by_different_milestone() {
+    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, &20_000i128);
+
+    client.create_milestone(&70u64, &sponsor, &token_addr, &10_000i128);
+    client.create_milestone(&71u64, &sponsor, &token_addr, &10_000i128);
+
+    client.allocate(&70u64, &555u64, &4_000i128);
+
+    // Same GitHub issue, different milestone: rejected by the global claim
+    // registry even though milestone 71's own allocations have never seen
+    // issue 555.
+    let err = client.try_allocate(&71u64, &555u64, &3_000i128);
+    assert_eq!(err, Err(Ok(Error::IssueClaimedByOtherMilestone)));
+
+    // The rejected call left milestone 71 untouched, so there is no second
+    // allocation for `release_issue` to pay out.
+    let milestone_b = client.get_milestone(&71u64);
+    assert_eq!(milestone_b.remaining_budget, 10_000i128);
+    assert_eq!(milestone_b.allocations.len(), 0);
+    assert_eq!(
+        client.try_get_issue_status(&71u64, &555u64),
+        Err(Ok(Error::IssueNotAllocated))
+    );
+
+    // A repeat allocation inside the *same* milestone still reports the
+    // pre-existing error, so the two collisions stay distinguishable.
+    assert_eq!(
+        client.try_allocate(&70u64, &555u64, &1_000i128),
+        Err(Ok(Error::IssueAlreadyAllocated))
+    );
+}

contracts/milestones/src/types.rs

@@ -62,6 +62,13 @@ pub enum DataKey {
     Milestone(u64),
     IssueStatus(u64, u64),  // (milestone_id, issue_id)
     Contribution(u64, u32), // (milestone_id, contribution_index)
+    /// Contract-instance-wide claim registry: `issue_id -> milestone_id`.
+    /// The one key here scoped by `issue_id` alone, which is what lets
+    /// `allocate` see that a *different* milestone already committed budget
+    /// to the same GitHub issue. To be cleared by `deallocate` when that
+    /// lands, so a removed allocation frees the issue for reallocation
+    /// instead of leaving a permanent false "already claimed".
+    GlobalIssueClaim(u64), // issue_id
 }
 
 impl mergefi_common::AdminKey for DataKey {