fix(#868,#869,#883,#866): overflow guard, rollback typo, concurrency invariant, self-payment check

contracts/asset_path_payment/src/lib.rs

@@ -24,6 +24,7 @@ pub enum PathPaymentError {
     PriceImpactTooHigh = 12,
     TransferFailed = 13,
     ContractPaused = 14,
+    SelfPayment = 15,
 }
 
 /// Storage keys
@@ -213,6 +214,10 @@ impl AssetPathPaymentContract {
         Self::require_not_paused(&env)?;
         from.require_auth();
 
+        if from == to {
+            return Err(PathPaymentError::SelfPayment);
+        }
+
         // Validate amounts
         if source_amount <= 0 {
             return Err(PathPaymentError::InvalidAmount);

contracts/asset_path_payment/src/test.rs

@@ -157,6 +157,28 @@ fn test_initiate_path_payment_rejects_invalid_amounts() {
     assert_eq!(result, Err(Ok(PathPaymentError::SlippageExceeded)));
 }
 
+#[test]
+fn test_initiate_path_payment_rejects_self_payment() {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let admin = Address::generate(&env);
+    let from = Address::generate(&env);
+    let source = create_token_contract(&env, &Address::generate(&env));
+    let dest = create_token_contract(&env, &Address::generate(&env));
+    let stellar = token::StellarAssetClient::new(&env, &source);
+    stellar.mint(&from, &5000);
+
+    let contract_id = env.register(AssetPathPaymentContract, ());
+    let client = AssetPathPaymentContractClient::new(&env, &contract_id);
+    client.init(&admin);
+
+    let path = Vec::new(&env);
+
+    let result = client.try_initiate_path_payment(&from, &from, &source, &dest, &100, &90, &100, &path);
+    assert_eq!(result, Err(Ok(PathPaymentError::SelfPayment)));
+}
+
 #[test]
 fn test_initiate_path_payment_escrows_tokens() {
     let env = Env::default();

contracts/bulk_payment/src/lib.rs

@@ -1091,7 +1091,7 @@ impl BulkPaymentContract {
         let status = if fail_count == 0 {
             symbol_short!("completed")
         } else if actual_success == 0 {
-            symbol_short!("rollbck")
+            symbol_short!("rollback")
         } else {
             symbol_short!("partial")
         };
@@ -1722,7 +1722,7 @@ impl BulkPaymentContract {
         let status = if fail_count == 0 {
             symbol_short!("completed")
         } else if success_count == 0 {
-            symbol_short!("rollbck")
+            symbol_short!("rollback")
         } else {
             symbol_short!("partial")
         };

contracts/bulk_payment/src/test.rs

@@ -251,7 +251,7 @@ fn test_partial_batch_skips_insufficient_funds() {
 }
 
 #[test]
-fn test_partial_batch_all_fail_status_is_rollbck() {
+fn test_partial_batch_all_fail_status_is_rollback() {
     let (env, sender, token, client) = setup();
     let mut payments: Vec<PaymentOp> = Vec::new(&env);
     payments.push_back(PaymentOp {
@@ -269,6 +269,26 @@ fn test_partial_batch_all_fail_status_is_rollbck() {
     assert_eq!(result.failures.get(0).unwrap().index, 0);
 }
 
+#[test]
+#[should_panic(expected = "Error(Contract, #7)")]
+fn test_partial_batch_overflow_returns_error() {
+    let (env, sender, token, client) = setup();
+
+    let mut payments: Vec<PaymentOp> = Vec::new(&env);
+    payments.push_back(PaymentOp {
+        recipient: Address::generate(&env),
+        amount: i128::MAX,
+        category: soroban_sdk::symbol_short!("payroll"),
+    });
+    payments.push_back(PaymentOp {
+        recipient: Address::generate(&env),
+        amount: 1,
+        category: soroban_sdk::symbol_short!("payroll"),
+    });
+
+    client.execute_batch_partial(&sender, &token, &payments, &0);
+}
+
 #[test]
 #[should_panic(expected = "Error(Contract, #4)")]
 fn test_partial_batch_empty_panics() {
@@ -1047,9 +1067,9 @@ fn test_v2_partial_invalid_recorded_as_failed() {
 }
 
 /// When ALL payments in a partial batch are invalid, the batch status is
-/// "rollbck" (no funds were pulled or held).
+/// "rollback" (no funds were pulled or held).
 #[test]
-fn test_v2_partial_all_fail_status_rollbck() {
+fn test_v2_partial_all_fail_status_rollback() {
     let (env, sender, token, client) = setup();
 
     let mut payments: Vec<PaymentOp> = Vec::new(&env);
@@ -1070,7 +1090,7 @@ fn test_v2_partial_all_fail_status_rollbck() {
     let record = client.get_batch(&batch_id);
     assert_eq!(record.success_count, 0);
     assert_eq!(record.fail_count, 2);
-    assert_eq!(record.status, soroban_sdk::symbol_short!("rollbck"));
+    assert_eq!(record.status, soroban_sdk::symbol_short!("rollback"));
 
     // Sender balance is unchanged — nothing was pulled.
     let tc = TokenClient::new(&env, &token);

contracts/milestone_escrow/src/lib.rs

@@ -342,6 +342,13 @@ impl MilestoneEscrowContract {
         Ok(())
     }
 
+    /// Release funds for an approved milestone.
+    ///
+    /// Soroban executes each transaction atomically within a single thread, so
+    /// two concurrent releases for the *same* escrow cannot interleave within a
+    /// single execution.  However, we defensively re-derive the remaining
+    /// balance from milestone state before transferring so that future changes
+    /// (e.g. batched or cross-contract calls) cannot cause a double-spend.
     pub fn release_milestone(
         e: Env,
         escrow_id: u64,
@@ -378,6 +385,16 @@ impl MilestoneEscrowContract {
             }
         }
 
+        // Invariant: re-derive remaining balance from milestone state so that
+        // concurrent or cross-contract callers cannot double-spend.
+        let remaining_from_state = record
+            .total_amount
+            .checked_sub(record.released_amount)
+            .ok_or(ContractError::InvalidAmount)?;
+        if remaining_from_state < milestone.amount {
+            return Err(ContractError::InsufficientEscrowBalance);
+        }
+
         let contract_balance =
             token::Client::new(&e, &record.token).balance(&e.current_contract_address());
         if contract_balance < milestone.amount {

contracts/milestone_escrow/src/test.rs

@@ -773,3 +773,55 @@ fn test_cancel_with_approved_milestones_recovers_full_unreleased() {
     assert_eq!(token_client.balance(&sender), 1_000_000);
     assert_eq!(token_client.balance(&beneficiary), 0);
 }
+
+// ==============================================================================
+// -- Issue #883: sequential release invariant ----------------------------------
+// ==============================================================================
+
+/// Sequential releases across milestones must never allow balances to go
+/// negative or double-spend.  The invariant check re-derives remaining balance
+/// from milestone state before each transfer.
+#[test]
+fn test_sequential_release_balances_never_negative() {
+    let (e, sender, beneficiary, verifier, token, token_client, _, client) = setup();
+    // Three milestones: [1000, 2000, 3000], total = 6000.
+    let milestones = make_milestones(&e, &[1000, 2000, 3000]);
+    let escrow_id = client.create_escrow(&sender, &beneficiary, &verifier, &token, &milestones);
+
+    // Approve all milestones on separate ledgers.
+    e.ledger().set_sequence_number(1);
+    client.approve_milestone(&escrow_id, &0);
+    e.ledger().set_sequence_number(2);
+    client.approve_milestone(&escrow_id, &1);
+    e.ledger().set_sequence_number(3);
+    client.approve_milestone(&escrow_id, &2);
+
+    // Release milestone 0.
+    e.ledger().set_sequence_number(4);
+    client.release_milestone(&escrow_id, &0);
+    let record = client.get_escrow(&escrow_id);
+    assert_eq!(record.released_amount, 1000);
+    assert!(record.is_active);
+
+    // Release milestone 1.
+    e.ledger().set_sequence_number(5);
+    client.release_milestone(&escrow_id, &1);
+    let record = client.get_escrow(&escrow_id);
+    assert_eq!(record.released_amount, 3000);
+    assert!(record.is_active);
+
+    // Release milestone 2.
+    e.ledger().set_sequence_number(6);
+    client.release_milestone(&escrow_id, &2);
+    let record = client.get_escrow(&escrow_id);
+    assert_eq!(record.released_amount, 6000);
+    assert!(!record.is_active);
+
+    // Beneficiary received exactly the sum of all milestones.
+    assert_eq!(token_client.balance(&beneficiary), 6000);
+    // Contract holds zero.
+    assert_eq!(
+        token_client.balance(&e.current_contract_address()),
+        0
+    );
+}