Validate InvoiceOptions ranges at invoice creation (#690–#693)

contracts/split/src/events.rs

@@ -182,12 +182,6 @@ pub fn invoice_expired(env: &Env, invoice_id: u64, deadline: u64, funded: i128,
             invoice_id,
         ),
         (deadline, funded, creator.clone()),
-/// Data: (deadline, funded)
-pub fn invoice_expired(env: &Env, invoice_id: u64, deadline: u64, funded: i128) {
-    let event_seq = next_seq(env, invoice_id);
-    env.events().publish(
-        (symbol_short!("split"), symbol_short!("expired"), invoice_id),
-        (deadline, funded, event_seq),
     );
 }
 

contracts/split/src/lib.rs

@@ -71,8 +71,6 @@ mod storage_keys;
 
 mod migrations;
 
-mod validation;
-
 use error::ContractError;
 use validation::assert_valid_bps;
 use soroban_sdk::crypto::bls12_381::{Fr, G1Affine};
@@ -5418,7 +5416,12 @@ impl SplitContract {
             );
         }
         assert!(bonus_pool >= 0, "bonus_pool must be non-negative");
-        assert_valid_bps(penalty_bps).expect("penalty_bps must be ≤ 10000");
+        // Issue #690: `penalty_bps` is a fraction of a late payment; a value
+        // above 100% would make every late payment exceed its principal. Reject
+        // it before any storage is written.
+        if let Err(e) = assert_valid_bps(penalty_bps) {
+            env.panic_with_error(e);
+        }
         assert!(min_funding_bps <= 10_000, "min_funding_bps must be ≤ 10000");
         assert_valid_bps(tax_bps).expect("tax_bps must be ≤ 10000");
         assert_valid_bps(insurance_premium_bps).expect("insurance_premium_bps must be ≤ 10000");
@@ -5514,16 +5517,40 @@ impl SplitContract {
             let _ = load_invoice(env, prereq_id);
         }
 
+        // Issue #693: a multi-signature release gate that requires more approvals
+        // than there are co-signers (or requires approvals with no co-signers at
+        // all) can never be satisfied, permanently locking the invoice. Reject
+        // such a configuration at creation time rather than at release time.
+        if required_signatures > co_signers.len() {
+            env.panic_with_error(ContractError::InvalidAmount);
+        }
+        if !co_signers.is_empty() && required_signatures == 0 {
+            env.panic_with_error(ContractError::InvalidAmount);
+        }
+
+        // Issue #691: when a graduated release schedule is supplied, its
+        // per-tranche basis points must cover exactly 100% so that all funds are
+        // eventually releasable. An empty schedule (release-all-at-once) is fine.
         if !tranches.is_empty() {
-            let total_bps: u32 = tranches.iter().map(|t| t.basis_points).fold(0u32, |a, b| a.saturating_add(b));
-            validation::assert_bps_total(total_bps)
-                .expect("tranches must sum to 10000 basis points");
+            let total_bps: u32 = tranches
+                .iter()
+                .map(|t| t.basis_points)
+                .fold(0u32, |a, b| a.saturating_add(b));
+            if let Err(e) = validation::assert_bps_total(total_bps) {
+                env.panic_with_error(e);
+            }
         }
 
+        // Issue #692: a non-empty staged-release schedule must sum to exactly
+        // 100% (10 000 bps); otherwise the final stage is silently truncated and
+        // residual funds are left unreachable. An empty schedule is skipped.
         if !release_stages.is_empty() {
-            let total_bps: u32 = release_stages.iter().fold(0u32, |a, b| a.saturating_add(b));
-            validation::assert_bps_total(total_bps)
-                .expect("release_stages must sum to 10000 basis points");
+            let total_bps: u32 = release_stages
+                .iter()
+                .fold(0u32, |a, b| a.saturating_add(b));
+            if let Err(e) = validation::assert_bps_total(total_bps) {
+                env.panic_with_error(e);
+            }
         }
         let milestones = milestones.unwrap_or_else(|| Vec::new(env));
         validate_milestones(env, &milestones);

contracts/split/src/test.rs

@@ -5,7 +5,6 @@ use soroban_sdk::{
     testutils::{Address as _, Events as _, Ledger},
     token::{Client as TokenClient, StellarAssetClient},
     Address, Bytes, BytesN, Env, String, Symbol, TryFromVal, Val, Vec,
-    Address, Bytes, BytesN, Env, String, Symbol, Vec,
 };
 use types::InvoiceOptions;
 
@@ -3196,7 +3195,6 @@ fn test_stage_release_not_fully_funded_panics() {
 }
 
 #[test]
-#[should_panic(expected = "release_stages must sum to 10000 basis points")]
 fn test_create_invoice_invalid_release_stages_panics() {
     let (env, contract_id, token_id) = setup_initialized();
     let c = client(&env, &contract_id);
@@ -3219,7 +3217,8 @@ fn test_create_invoice_invalid_release_stages_panics() {
     let mut opts = default_options(&env);
     opts.release_stages = stages;
 
-    c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    let res = c.try_create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert_eq!(res, Err(Ok(ContractError::InvalidRatioSum.into())));
 }
 
 // ---------------------------------------------------------------------------
@@ -8362,3 +8361,207 @@ fn test_get_invoice_status_not_found() {
     let result = c.try_get_invoice_status(&999);
     assert!(result.is_err());
 }
+
+// ---------------------------------------------------------------------------
+// Issues #690-#693 — InvoiceOptions validation at invoice creation
+// ---------------------------------------------------------------------------
+
+/// Build a two-recipient / two-amount pair for the validation tests. Two
+/// recipients keeps these tests clear of the minimum-recipient-count gate.
+fn two_recipients(env: &Env) -> (Vec<Address>, Vec<i128>) {
+    let mut recipients = Vec::new(env);
+    recipients.push_back(Address::generate(env));
+    recipients.push_back(Address::generate(env));
+    let mut amounts = Vec::new(env);
+    amounts.push_back(600_i128);
+    amounts.push_back(400_i128);
+    (recipients, amounts)
+}
+
+// --- Issue #690: penalty_bps must not exceed 10 000 ---
+
+#[test]
+fn test_create_invoice_penalty_bps_over_max_rejected() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    let mut opts = default_options(&env);
+    opts.penalty_bps = Some(10_001);
+
+    let res = c.try_create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert_eq!(res, Err(Ok(ContractError::InvalidAmount.into())));
+}
+
+#[test]
+fn test_create_invoice_penalty_bps_at_max_ok() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    let mut opts = default_options(&env);
+    opts.penalty_bps = Some(10_000);
+
+    let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert!(id >= 1);
+}
+
+// --- Issue #691: non-empty tranches must sum to exactly 10 000 bps ---
+
+#[test]
+fn test_create_invoice_tranches_sum_not_total_rejected() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    // 4_000 + 5_999 = 9_999, one basis point short of 10_000.
+    let mut tranches = Vec::new(&env);
+    tranches.push_back(Tranche { timestamp: 2_000, basis_points: 4_000 });
+    tranches.push_back(Tranche { timestamp: 3_000, basis_points: 5_999 });
+
+    let mut opts = default_options(&env);
+    opts.tranches = tranches;
+
+    let res = c.try_create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert_eq!(res, Err(Ok(ContractError::InvalidRatioSum.into())));
+}
+
+#[test]
+fn test_create_invoice_tranches_sum_exact_total_ok() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    let mut tranches = Vec::new(&env);
+    tranches.push_back(Tranche { timestamp: 2_000, basis_points: 4_000 });
+    tranches.push_back(Tranche { timestamp: 3_000, basis_points: 6_000 });
+
+    let mut opts = default_options(&env);
+    opts.tranches = tranches;
+
+    let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert!(id >= 1);
+}
+
+// --- Issue #692: non-empty release_stages must sum to exactly 10 000 bps ---
+
+#[test]
+fn test_create_invoice_release_stages_sum_not_total_rejected() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    // 5_000 + 4_500 = 9_500, short of 10_000.
+    let mut stages: Vec<u32> = Vec::new(&env);
+    stages.push_back(5_000u32);
+    stages.push_back(4_500u32);
+
+    let mut opts = default_options(&env);
+    opts.release_stages = stages;
+
+    let res = c.try_create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert_eq!(res, Err(Ok(ContractError::InvalidRatioSum.into())));
+}
+
+#[test]
+fn test_create_invoice_release_stages_three_stages_sum_total_ok() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    let mut stages: Vec<u32> = Vec::new(&env);
+    stages.push_back(3_000u32);
+    stages.push_back(3_000u32);
+    stages.push_back(4_000u32);
+
+    let mut opts = default_options(&env);
+    opts.release_stages = stages;
+
+    let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert!(id >= 1);
+}
+
+// --- Issue #693: required_signatures must be in 1..=co_signers.len() ---
+
+#[test]
+fn test_create_invoice_required_signatures_exceeds_cosigners_rejected() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    // 2-of-3 requested, but only one co-signer supplied.
+    let mut co_signers = Vec::new(&env);
+    co_signers.push_back(Address::generate(&env));
+
+    let mut opts = default_options(&env);
+    opts.co_signers = co_signers;
+    opts.required_signatures = 2;
+
+    let res = c.try_create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert_eq!(res, Err(Ok(ContractError::InvalidAmount.into())));
+}
+
+#[test]
+fn test_create_invoice_required_signatures_zero_with_cosigners_rejected() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    let mut co_signers = Vec::new(&env);
+    co_signers.push_back(Address::generate(&env));
+    co_signers.push_back(Address::generate(&env));
+
+    let mut opts = default_options(&env);
+    opts.co_signers = co_signers;
+    opts.required_signatures = 0;
+
+    let res = c.try_create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert_eq!(res, Err(Ok(ContractError::InvalidAmount.into())));
+}
+
+#[test]
+fn test_create_invoice_valid_multisig_setup_ok() {
+    let (env, contract_id, token_id) = setup_initialized();
+    let c = client(&env, &contract_id);
+    env.ledger().set_timestamp(1_000);
+
+    let creator = Address::generate(&env);
+    let (recipients, amounts) = two_recipients(&env);
+
+    // Valid 2-of-3.
+    let mut co_signers = Vec::new(&env);
+    co_signers.push_back(Address::generate(&env));
+    co_signers.push_back(Address::generate(&env));
+    co_signers.push_back(Address::generate(&env));
+
+    let mut opts = default_options(&env);
+    opts.co_signers = co_signers;
+    opts.required_signatures = 2;
+
+    let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts);
+    assert!(id >= 1);
+}

contracts/split/src/validation.rs

@@ -97,6 +97,20 @@ pub fn assert_bps_total(total: u32) -> Result<(), ContractError> {
     Ok(())
 }
 
+/// Reject a single basis-point value that exceeds 100% (`BASIS_POINTS_TOTAL` =
+/// 10 000). Used for standalone rate fields such as `penalty_bps`, `tax_bps`
+/// and `insurance_premium_bps` where the value must be a fraction of a whole,
+/// not a sum that covers it.
+///
+/// # Errors
+/// Returns `Err(ContractError::InvalidAmount)` when `bps > 10_000`.
+pub fn assert_valid_bps(bps: u32) -> Result<(), ContractError> {
+    if bps > BASIS_POINTS_TOTAL {
+        return Err(ContractError::InvalidAmount);
+    }
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;