fix(split): reject duplicate recipients and 0-bps entries

contracts/common/src/split.rs

@@ -57,7 +57,19 @@ pub fn compute_split(
     }
 
     let mut bps_sum: i128 = 0;
-    for (_, bps) in recipients.iter() {
+    for (i, (recipient, bps)) in recipients.iter().enumerate() {
+        // #540 — a 0-bps recipient is a no-op slot the bps_sum check alone
+        // wouldn't catch; reject it.
+        if bps == 0 {
+            return Err(SplitError::InvalidSplit);
+        }
+        // #540 — a duplicate recipient would be paid twice / collapse
+        // silently; reject repeats.
+        for (other, _) in recipients.iter().skip(i + 1) {
+            if other == recipient {
+                return Err(SplitError::InvalidSplit);
+            }
+        }
         bps_sum = bps_sum
             .checked_add(bps as i128)
             .ok_or(SplitError::Overflow)?;

contracts/escrow/src/error.rs

@@ -33,4 +33,8 @@ pub enum Error {
     /// `i128::MAX`). Returned instead of panicking under release
     /// `overflow-checks = true` + `panic = "abort"`.
     ArithmeticOverflow = 21,
+    /// A contribution arrived after the escrow's deadline — the escrow is
+    /// refundable once deadline + grace elapse, so new funds would be
+    /// stranded or defeat a pending refund (issue #568).
+    DeadlinePassed = 22,
 }

contracts/escrow/src/lib.rs

@@ -222,6 +222,12 @@ mod contract {
             EscrowStatus::Refunded => return Err(Error::AlreadyRefunded),
             EscrowStatus::Funded => {}
         }
+        // A past-deadline escrow is refundable once the grace window elapses
+        // — accepting new funds into it would strand them or defeat a
+        // pending refund (#568).
+        if env.ledger().timestamp() >= escrow.deadline {
+            return Err(Error::DeadlinePassed);
+        }
 
         let max_sponsors: u32 = env
             .storage()

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,
@@ -52,6 +53,21 @@ fn test_getters_return_not_initialized_before_initialize() {
     assert_eq!(client.try_get_max_sponsors(), Err(Ok(Error::NotInitialized)));
 }
 
+#[test]
+fn test_initialize_rejects_treasury_equal_to_contract() {
+    // #516 — InvalidTreasury: the treasury must not be the contract itself.
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(EscrowContract, ());
+    let client = EscrowContractClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    let oracle = Address::generate(&env);
+
+    let result = client.try_initialize(&admin, &oracle, &contract_id, &500u32, &None);
+    assert_eq!(result, Err(Ok(Error::InvalidTreasury)));
+}
+
+
 #[test]
 fn test_initialize_rejects_fee_bps_above_10000() {
     let env = Env::default();
@@ -369,8 +385,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 +451,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 +2168,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);
@@ -2217,14 +2234,15 @@ fn test_release_with_duplicate_recipient_addresses() {
         &None,
     );
 
-    // Same address twice with 5000 bps each — must produce two separate
-    // transfers summing to the full 10000-bps entitlement.
+    // #540 — duplicate recipients are now rejected outright: silently paying
+    // the same address twice hides a malformed split behind a doubled transfer.
     let recipients = vec![&env, (alice.clone(), 5_000u32), (alice.clone(), 5_000u32)];
-    client.release(&200u64, &recipients);
+    let err = client.try_release(&200u64, &recipients);
+    assert_eq!(err, Err(Ok(Error::InvalidSplit)));
 
-    let distributable = 950_0000000i128; // after 5% fee
-    assert_eq!(token_client.balance(&alice), distributable);
-    assert_eq!(token_client.balance(&treasury), 50_0000000i128);
+    // Nothing moved.
+    assert_eq!(token_client.balance(&alice), 0);
+    assert_eq!(token_client.balance(&treasury), 0);
 }
 
 #[test]

contracts/maintenance-pool/src/lib.rs

@@ -299,19 +299,41 @@ mod contract {
             return Err(Error::InactivityWindowNotElapsed);
         }
 
-        if deposit.amount > pool.balance {
+        // #534 — refund the sponsor's pro-rata share of the REMAINING
+        // balance, not the full original deposit. The pool is fungible and
+        // withdraw pays rewards from it, so a full refund would let an early
+        // reclaimer drain later depositors' share of the spending.
+        //   reclaimable = deposit.amount * pool.balance / total_deposited
+        let reclaimable = if pool.total_deposited > 0 {
+            deposit
+                .amount
+                .checked_mul(pool.balance)
+                .ok_or(Error::ArithmeticOverflow)?
+                .checked_div(pool.total_deposited)
+                .ok_or(Error::ArithmeticOverflow)?
+        } else {
+            deposit.amount
+        };
+        if reclaimable <= 0 {
             return Err(Error::InsufficientBalance);
         }
 
         let token_client = token::Client::new(&env, &pool.token);
-        token_client.transfer(&env.current_contract_address(), &sponsor, &deposit.amount);
+        token_client.transfer(&env.current_contract_address(), &sponsor, &reclaimable);
 
         pool.balance = pool
             .balance
+            .checked_sub(reclaimable)
+            .ok_or(Error::ArithmeticOverflow)?;
+        pool.total_deposited = pool
+            .total_deposited
             .checked_sub(deposit.amount)
             .ok_or(Error::ArithmeticOverflow)?;
         env.storage().persistent().set(&pkey, &pool);
         extend_ttl(&env, &pkey);
+        // Clear the deposit record — otherwise the same deposit could be
+        // reclaimed twice.
+        env.storage().persistent().remove(&dkey);
         extend_instance_ttl(&env);
 
         Ok(())