fix(compute_split): reject self-payouts that strand funds in the cont…

contracts/common/src/split.rs

@@ -33,6 +33,9 @@ pub enum SplitError {
     /// `recipients` was empty, or its basis-point shares don't sum to
     /// exactly `BPS_DENOMINATOR` (10000 = 100%).
     InvalidSplit,
+    /// A recipient address equals the contract's own address — a self-payout
+    /// that would strand funds in the contract with no recovery path.
+    SelfPayout,
 }
 
 /// Validates that basis-point splits sum to exactly 10000 and computes the
@@ -55,6 +58,17 @@ pub fn compute_split(
         return Err(SplitError::InvalidSplit);
     }
 
+    // Reject self-payouts: a recipient equal to the contract's own address
+    // would strand funds — the status is marked Paid/Released before the
+    // transfer loop runs, so a self-transfer leaves tokens in the contract
+    // with no future release or refund path to recover them.
+    let contract_address = env.current_contract_address();
+    for (recipient, _) in recipients.iter() {
+        if recipient == contract_address {
+            return Err(SplitError::SelfPayout);
+        }
+    }
+
     let fee = total * (fee_bps as i128) / BPS_DENOMINATOR;
     let distributable = total - fee;
 

contracts/common/src/test_fuzz.rs

@@ -11,6 +11,13 @@ mod tests {
 
     use crate::BPS_DENOMINATOR;
     use proptest::prelude::*;
+    use soroban_sdk::{contract, contractimpl};
+
+    #[contract]
+    pub struct DummyContract;
+
+    #[contractimpl]
+    impl DummyContract {}
 
     /// Simplified compute_split for testing (mirrors the contract logic)
     fn compute_split_test(total: i128, recipients: &[u32], fee_bps: u32) -> Vec<i128> {
@@ -254,16 +261,19 @@ mod tests {
         use soroban_sdk::{Address, Env, Vec as SorobanVec};
 
         let env = Env::default();
+        let contract_id = env.register(DummyContract, ());
         let recipients: SorobanVec<(Address, u32)> = SorobanVec::new(&env);
 
         for fee_bps in [0u32, 250, 10_000] {
-            let result = compute_split(&env, 1_000_000, fee_bps, &recipients);
+            let result = env.as_contract(&contract_id, || {
+                compute_split(&env, 1_000_000, fee_bps, &recipients)
+            });
             assert!(matches!(result, Err(SplitError::InvalidSplit)));
         }
 
         // Also rejected for a zero total — emptiness alone is invalid.
         assert!(matches!(
-            compute_split(&env, 0, 0, &recipients),
+            env.as_contract(&contract_id, || compute_split(&env, 0, 0, &recipients)),
             Err(SplitError::InvalidSplit)
         ));
     }
@@ -277,10 +287,15 @@ mod tests {
         use soroban_sdk::{testutils::Address as _, Address, Env, Vec as SorobanVec};
 
         let env = Env::default();
+        let contract_id = env.register(DummyContract, ());
         let recipient = Address::generate(&env);
         let recipients = SorobanVec::from_array(&env, [(recipient.clone(), 10_000u32)]);
 
-        let payouts = compute_split(&env, 1_000_000, 250, &recipients).unwrap();
+        let payouts = env
+            .as_contract(&contract_id, || {
+                compute_split(&env, 1_000_000, 250, &recipients)
+            })
+            .unwrap();
         assert_eq!(payouts.fee, 25_000);
         assert_eq!(payouts.shares.len(), 1);
         assert_eq!(payouts.shares.get(0).unwrap(), (recipient, 975_000));

contracts/escrow/src/error.rs

@@ -26,4 +26,7 @@ pub enum Error {
     InvalidTreasury = 17,
     /// Contract is paused and this operation is not allowed (issue #14).
     ContractPaused = 19,
+    /// A recipient address equals the contract's own address — a self-payout
+    /// that would strand funds in the contract with no recovery path.
+    SelfPayout = 20,
 }

contracts/escrow/src/lib.rs

@@ -312,7 +312,10 @@ impl EscrowContract {
         let fee_bps: u32 = mergefi_common::get_fee_bps::<DataKey>(&env)
             .ok_or(Error::NotInitialized)?;
         let payouts = mergefi_common::compute_split(&env, escrow.amount, fee_bps, &recipients)
-            .map_err(|_| Error::InvalidSplit)?;
+            .map_err(|e| match e {
+                mergefi_common::SplitError::InvalidSplit => Error::InvalidSplit,
+                mergefi_common::SplitError::SelfPayout => Error::SelfPayout,
+            })?;
         let treasury: Address = mergefi_common::require_treasury::<DataKey>(&env).unwrap();
         let token_client = token::Client::new(&env, &escrow.token);
         let contract_address = env.current_contract_address();
@@ -677,6 +680,8 @@ impl EscrowContract {
         env.storage().instance().set(&DataKey::Treasury, &new_treasury);
         extend_instance_ttl(&env);
         Ok(())
+    }
+
     pub fn get_version(env: Env) -> u32 {
         env.storage().instance().get(&DataKey::Version).unwrap_or(0)
     }

contracts/escrow/src/test.rs

@@ -2195,3 +2195,103 @@ fn test_extend_deadline_returns_contribution_not_found_when_archived() {
     assert_eq!(err, Err(Ok(Error::ContributionNotFound)));
 }
 
+#[test]
+fn test_release_with_duplicate_recipient_addresses() {
+    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);
+
+    let alice = Address::generate(&env);
+
+    client.fund(
+        &200u64,
+        &sponsor,
+        &token_addr,
+        &10_000_000_000i128,
+        &1_000u64,
+        &None,
+    );
+
+    // Same address twice with 5000 bps each — must produce two separate
+    // transfers summing to the full 10000-bps entitlement.
+    let recipients = vec![&env, (alice.clone(), 5_000u32), (alice.clone(), 5_000u32)];
+    client.release(&200u64, &recipients);
+
+    let distributable = 950_0000000i128; // after 5% fee
+    assert_eq!(token_client.balance(&alice), distributable);
+    assert_eq!(token_client.balance(&treasury), 50_0000000i128);
+}
+
+#[test]
+fn test_release_with_treasury_as_recipient() {
+    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);
+
+    let alice = Address::generate(&env);
+
+    client.fund(
+        &201u64,
+        &sponsor,
+        &token_addr,
+        &10_000_000_000i128,
+        &1_000u64,
+        &None,
+    );
+
+    // Treasury as one of the recipients — fee transfer and recipient transfer
+    // are separate calls; treasury must receive fee + share without double-counting.
+    let recipients = vec![&env, (alice.clone(), 5_000u32), (treasury.clone(), 5_000u32)];
+    client.release(&201u64, &recipients);
+
+    let distributable = 950_0000000i128; // after 5% fee
+    let alice_expected = distributable * 5000 / 10000;
+    let treasury_expected = 50_0000000i128 + (distributable - alice_expected);
+    assert_eq!(token_client.balance(&alice), alice_expected);
+    assert_eq!(token_client.balance(&treasury), treasury_expected);
+}
+
+#[test]
+fn test_release_rejects_self_payout() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let (_contract_id, _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.fund(
+        &202u64,
+        &sponsor,
+        &token_addr,
+        &10_000_000_000i128,
+        &1_000u64,
+        &None,
+    );
+
+    // Recipient equal to the contract's own address must be rejected.
+    let recipients = vec![
+        &env,
+        (client.address.clone(), 5_000u32),
+        (Address::generate(&env), 5_000u32),
+    ];
+    let err = client.try_release(&202u64, &recipients);
+    assert_eq!(err, Err(Ok(Error::SelfPayout)));
+
+    // Escrow must remain Funded (not Paid) so the funds are not stranded.
+    let escrow = client.get_escrow(&202u64);
+    assert_eq!(escrow.status, crate::types::EscrowStatus::Funded);
+}
+

contracts/milestones/src/error.rs

@@ -30,4 +30,7 @@ pub enum Error {
     ContractPaused = 18,
     /// The contribution index is out of range for an existing milestone (issue #256).
     ContributionNotFound = 19,
+    /// A recipient address equals the contract's own address — a self-payout
+    /// that would strand funds in the contract with no recovery path.
+    SelfPayout = 20,
 }

contracts/milestones/src/lib.rs

@@ -384,7 +384,10 @@ impl MilestonesContract {
         let fee_bps: u32 = mergefi_common::get_fee_bps::<DataKey>(&env)
             .ok_or(Error::NotInitialized)?;
         let payouts = mergefi_common::compute_split(&env, amount, fee_bps, &recipients)
-            .map_err(|_| Error::InvalidSplit)?;
+            .map_err(|e| match e {
+                mergefi_common::SplitError::InvalidSplit => Error::InvalidSplit,
+                mergefi_common::SplitError::SelfPayout => Error::SelfPayout,
+            })?;
         let treasury: Address = mergefi_common::require_treasury::<DataKey>(&env).unwrap();
         let token_client = token::Client::new(&env, &milestone.token);
         let contract_address = env.current_contract_address();

contracts/milestones/src/test.rs

@@ -1806,3 +1806,109 @@ fn test_cancel_milestone_after_deadline_with_partial_allocation() {
     // Sponsor should receive full refund of remaining 40% (no fee on refunds)
     assert_eq!(token_client.balance(&sponsor), 4_000_000_000i128);
 }
+
+#[test]
+fn test_release_issue_with_duplicate_recipient_addresses() {
+    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);
+
+    let alice = Address::generate(&env);
+
+    client.create_milestone(&2u64, &sponsor, &token_addr, &10_000_000_000i128, &1_000u64);
+    client.allocate(&2u64, &201u64, &10_000_000_000i128);
+
+    // Same address twice with 5000 bps each — must produce two separate
+    // transfers summing to the full 10000-bps entitlement.
+    let recipients = vec![&env, (alice.clone(), 5_000u32), (alice.clone(), 5_000u32)];
+    client.release_issue(&2u64, &201u64, &recipients);
+
+    let distributable = 950_0000000i128; // after 5% fee
+    assert_eq!(token_client.balance(&alice), distributable);
+    assert_eq!(token_client.balance(&treasury), 50_0000000i128);
+}
+
+#[test]
+fn test_release_issue_with_treasury_as_recipient() {
+    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);
+
+    let alice = Address::generate(&env);
+
+    client.create_milestone(&2u64, &sponsor, &token_addr, &10_000_000_000i128, &1_000u64);
+    client.allocate(&2u64, &202u64, &10_000_000_000i128);
+
+    // Treasury as one of the recipients — fee transfer and recipient transfer
+    // are separate calls; treasury must receive fee + share without double-counting.
+    let recipients = vec![&env, (alice.clone(), 5_000u32), (treasury.clone(), 5_000u32)];
+    client.release_issue(&2u64, &202u64, &recipients);
+
+    let distributable = 950_0000000i128; // after 5% fee
+    let alice_expected = distributable * 5000 / 10000;
+    let treasury_expected = 50_0000000i128 + (distributable - alice_expected);
+    assert_eq!(token_client.balance(&alice), alice_expected);
+    assert_eq!(token_client.balance(&treasury), treasury_expected);
+}
+
+#[test]
+fn test_release_issue_rejects_self_payout() {
+    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(&2u64, &sponsor, &token_addr, &10_000_000_000i128, &1_000u64);
+    client.allocate(&2u64, &203u64, &10_000_000_000i128);
+
+    // Recipient equal to the contract's own address must be rejected.
+    let recipients = vec![
+        &env,
+        (client.address.clone(), 5_000u32),
+        (Address::generate(&env), 5_000u32),
+    ];
+    let err = client.try_release_issue(&2u64, &203u64, &recipients);
+    assert_eq!(err, Err(Ok(Error::SelfPayout)));
+
+    // Issue must remain Allocated (not Released) so the funds are not stranded.
+    assert_eq!(
+        client.get_issue_status(&2u64, &203u64),
+        crate::types::IssueStatus::Allocated
+    );
+}
+
+#[test]
+fn test_release_issue_with_admin_as_recipient() {
+    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(&2u64, &sponsor, &token_addr, &10_000_000_000i128, &1_000u64);
+    client.allocate(&2u64, &204u64, &10_000_000_000i128);
+
+    // Admin is a regular address — must be a valid recipient.
+    let recipients = vec![&env, (admin.clone(), 10_000u32)];
+    client.release_issue(&2u64, &204u64, &recipients);
+
+    let distributable = 950_0000000i128; // after 5% fee
+    assert_eq!(token_client.balance(&admin), distributable);
+}