diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index 99816e8..f525150 100644 --- a/contracts/split/src/events.rs +++ b/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), ); } diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index 874d3bf..b42facb 100644 --- a/contracts/split/src/lib.rs +++ b/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); diff --git a/contracts/split/src/test.rs b/contracts/split/src/test.rs index 498ea3b..be19887 100644 --- a/contracts/split/src/test.rs +++ b/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
, Vec) { + 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 = 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 = 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); +} diff --git a/contracts/split/src/validation.rs b/contracts/split/src/validation.rs index f90a3ea..63eb412 100644 --- a/contracts/split/src/validation.rs +++ b/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::*;