fix: resolve issues #705 #706 #707 #708

contracts/split/src/calc.rs

@@ -34,7 +34,7 @@
 
 #[allow(unused_imports)]
 use crate::types::BASIS_POINTS_TOTAL;
-use soroban_sdk::{Env, Vec};
+use soroban_sdk::{Address, BytesN, Env, Vec};
 
 use crate::error::ContractError;
 
@@ -185,6 +185,67 @@ pub fn sort_recipients(env: &Env, recipients: &mut Vec<Address>) {
     *recipients = sorted;
 }
 
+// ---------------------------------------------------------------------------
+// Issue #705: Invoice funding completion helper
+// ---------------------------------------------------------------------------
+
+/// Compute the funding completion of an invoice in basis points.
+///
+/// Returns `funded * 10_000 / total`, clamped to `[0, 10_000]`.
+///
+/// # Edge cases
+/// * Returns `0` when `total <= 0` (nothing to fund).
+/// * Returns `0` when `funded <= 0`.
+/// * Returns `10_000` when `funded >= total` (fully funded or overfunded).
+///
+/// # Examples
+/// ```
+/// assert_eq!(funding_bps(500, 1000), 5_000);  // 50%
+/// assert_eq!(funding_bps(1000, 1000), 10_000); // 100%
+/// assert_eq!(funding_bps(1500, 1000), 10_000); // overfunded → clamped
+/// assert_eq!(funding_bps(0, 1000), 0);          // nothing paid
+/// assert_eq!(funding_bps(500, 0), 0);            // invalid total
+/// ```
+pub fn funding_bps(funded: i128, total: i128) -> u32 {
+    if total <= 0 || funded <= 0 {
+        return 0;
+    }
+    if funded >= total {
+        return 10_000;
+    }
+    // funded < total, both positive — safe to cast to u128 and divide.
+    let bps = (funded as u128 * 10_000u128) / (total as u128);
+    // bps is in [0, 9_999] since funded < total; the clamp is a safeguard.
+    bps.min(10_000) as u32
+}
+
+// ---------------------------------------------------------------------------
+// Issue #705: Platform fee computation helper
+// ---------------------------------------------------------------------------
+
+/// Compute the platform fee for a given funded amount and fee rate.
+///
+/// Returns `funded * fee_bps / 10_000`, using checked arithmetic to prevent
+/// overflow on very large amounts.
+///
+/// # Arguments
+/// * `funded`   – gross collected amount (stroops); must be ≥ 0.
+/// * `fee_bps`  – platform fee rate in basis points (0 – 10 000).
+///
+/// # Errors
+/// Returns [`ContractError::ArithmeticOverflow`] when `funded * fee_bps`
+/// overflows `i128` (i.e. `funded` is close to `i128::MAX` and `fee_bps > 0`).
+pub fn calc_platform_fee(funded: i128, fee_bps: u32) -> Result<i128, ContractError> {
+    if fee_bps == 0 || funded == 0 {
+        return Ok(0);
+    }
+    let fee = (funded as i128)
+        .checked_mul(fee_bps as i128)
+        .ok_or(ContractError::ArithmeticOverflow)?
+        / 10_000;
+    Ok(fee)
+}
+
 // ---------------------------------------------------------------------------
 // Unit tests
 // ---------------------------------------------------------------------------
@@ -345,6 +406,53 @@ mod tests {
         }
     }
 
+    // -----------------------------------------------------------------------
+    // funding_bps tests
+    // -----------------------------------------------------------------------
+
+    #[test]
+    fn test_funding_bps_partial() {
+        // 500 funded out of 1000 total → 50% → 5_000 bps
+        assert_eq!(funding_bps(500, 1_000), 5_000);
+    }
+
+    #[test]
+    fn test_funding_bps_full() {
+        // Exactly fully funded → 100% → 10_000 bps
+        assert_eq!(funding_bps(1_000, 1_000), 10_000);
+    }
+
+    #[test]
+    fn test_funding_bps_overfunded() {
+        // Overfunded → clamped to 10_000
+        assert_eq!(funding_bps(1_500, 1_000), 10_000);
+    }
+
+    #[test]
+    fn test_funding_bps_zero_funded() {
+        // Nothing paid yet → 0
+        assert_eq!(funding_bps(0, 1_000), 0);
+    }
+
+    #[test]
+    fn test_funding_bps_zero_total() {
+        // Invalid total → 0 to avoid divide-by-zero
+        assert_eq!(funding_bps(500, 0), 0);
+    }
+
+    #[test]
+    fn test_funding_bps_negative_total() {
+        assert_eq!(funding_bps(500, -1), 0);
+    }
+
+    #[test]
+    fn test_funding_bps_one_stroop_below_full() {
+        // funded = total - 1 → result must be < 10_000
+        let bps = funding_bps(999, 1_000);
+        assert!(bps < 10_000);
+        assert!(bps > 9_980); // should be ~9_990
+    }
+
     // -----------------------------------------------------------------------
     // calc_platform_fee tests
     // -----------------------------------------------------------------------

contracts/split/src/events.rs

@@ -26,6 +26,7 @@ use crate::types::{DisputeOutcome, FeeSplit, InvoicePhase, InvoiceStatus, RepSco
 //! symbol exceeds the short-macro length limit or must be constructed
 //! dynamically.
 
+use crate::storage_keys::ev_seq_key;
 use crate::types::{DisputeOutcome, FeeSplit, InvoiceStatus, RepScore, TimelockAction};
 use soroban_sdk::{contracttype, symbol_short, Address, BytesN, Env, String, Vec};
 
@@ -36,7 +37,7 @@ use soroban_sdk::{contracttype, symbol_short, Address, BytesN, Env, String, Vec}
 /// Fetch and increment the per-invoice event sequence counter.
 /// Lives in `storage::temporary` so it resets between transactions.
 fn next_seq(env: &Env, invoice_id: u64) -> u64 {
-    let key = (symbol_short!("ev_seq"), invoice_id);
+    let key = ev_seq_key(invoice_id);
     let seq: u64 = env.storage().temporary().get(&key).unwrap_or(0) + 1;
     env.storage().temporary().set(&key, &seq);
     seq
@@ -1853,3 +1854,43 @@ pub fn admin_transfer_completed(env: &Env, new_admin: &Address) {
         new_admin.clone(),
     );
 }
+
+// ---------------------------------------------------------------------------
+// Unit tests for the per-invoice event sequence counter (issue #708)
+// ---------------------------------------------------------------------------
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use soroban_sdk::Env;
+
+    /// `next_seq` returns 1 on first call and increments on each subsequent
+    /// call for the same invoice ID.
+    #[test]
+    fn test_next_seq_increments_per_invoice() {
+        let env = Env::default();
+        assert_eq!(next_seq(&env, 1), 1);
+        assert_eq!(next_seq(&env, 1), 2);
+        assert_eq!(next_seq(&env, 1), 3);
+    }
+
+    /// Sequences for different invoice IDs are independent — incrementing the
+    /// counter for invoice A must not affect invoice B's counter.
+    #[test]
+    fn test_next_seq_independent_for_different_invoice_ids() {
+        let env = Env::default();
+
+        // Advance invoice 10 twice.
+        assert_eq!(next_seq(&env, 10), 1);
+        assert_eq!(next_seq(&env, 10), 2);
+
+        // Invoice 20 should still start at 1.
+        assert_eq!(next_seq(&env, 20), 1);
+
+        // Invoice 10 continues independently from where it left off.
+        assert_eq!(next_seq(&env, 10), 3);
+
+        // Invoice 20 is still at 2 after one more call.
+        assert_eq!(next_seq(&env, 20), 2);
+    }
+}

contracts/split/src/lib.rs

@@ -56,6 +56,8 @@ mod error;
 mod events;
 pub mod types;
 mod validation;
+mod calc;
+mod stats;
 
 #[cfg(test)]
 mod test;
@@ -73,6 +75,7 @@ mod migrations;
 
 use error::ContractError;
 use validation::assert_valid_bps;
+use calc::{calc_platform_fee, funding_bps};
 use soroban_sdk::crypto::bls12_381::{Fr, G1Affine};
 use soroban_sdk::xdr::ToXdr;
 use soroban_sdk::{
@@ -2499,7 +2502,7 @@ fn check_and_emit_funding_checkpoints(env: &Env, invoice_id: u64, funded: i128,
         return;
     }
 
-    let progress_bps = (funded.saturating_mul(10_000)) / total;
+    let progress_bps = funding_bps(funded, total) as i128;
     let last_emitted: u32 = env
         .storage()
         .persistent()
@@ -4124,7 +4127,7 @@ impl SplitContract {
         let total: i128 = invoice.amounts.iter().sum();
         let cumulative_contributed: i128 = env.storage().persistent()
             .get(&cumulative_contributed_key(invoice_id)).unwrap_or(0);
-        let completion_bps: u32 = if total > 0 { ((invoice.funded * 10_000) / total) as u32 } else { 0 };
+        let completion_bps: u32 = funding_bps(invoice.funded, total);
         let mut unique_payers: Vec<Address> = Vec::new(&env);
         for payment in invoice.payments.iter() {
             if !unique_payers.contains(&payment.payer) { unique_payers.push_back(payment.payer); }
@@ -10906,7 +10909,7 @@ impl SplitContract {
         let total: i128 = invoice.amounts.iter().sum();
         assert!(total > 0, "invoice total must be positive");
 
-        let funded_bps = (invoice.funded as u128 * 10_000u128 / total as u128) as u32;
+        let funded_bps = funding_bps(invoice.funded, total);
 
         // Evaluate rules in order; execute first match.
         for rule in invoice.auto_resolve_rules.clone().iter() {

contracts/split/src/stats.rs

@@ -6,7 +6,11 @@ use crate::error::ContractError;
 ///
 /// The counters are stored in instance storage, so they do not require a
 /// persistent-storage TTL and survive independently of individual invoices.
-pub type Stats = (u64, i128, u64);
+pub struct ProtocolStats {
+    pub total_invoices: u64,
+    pub total_volume: i128,
+    pub total_recipients_paid: u64,
+}
 
 const TOTAL_INVOICES: &str = "stats_total_invoices";
 const TOTAL_VOLUME: &str = "stats_total_volume";
@@ -40,7 +44,7 @@ pub fn get_stats(env: &Env) -> Stats {
         storage
             .get(&total_recipients_paid_key(env))
             .unwrap_or(0u64),
-    )
+    }
 }
 
 /// Applies a statistics delta atomically.
@@ -63,16 +67,16 @@ pub fn increment(
     invoices: u64,
     volume: i128,
     recipients_paid: u64,
-) -> Result<Stats, ContractError> {
-    let (current_invoices, current_volume, current_recipients_paid) = get_stats(env);
+) -> Result<ProtocolStats, ContractError> {
+    let current = get_stats(env);
 
-    let next_invoices = current_invoices
+    let next_invoices = current.total_invoices
         .checked_add(invoices)
         .ok_or(ContractError::StatsOverflow)?;
-    let next_volume = current_volume
+    let next_volume = current.total_volume
         .checked_add(volume)
         .ok_or(ContractError::StatsOverflow)?;
-    let next_recipients_paid = current_recipients_paid
+    let next_recipients_paid = current.total_recipients_paid
         .checked_add(recipients_paid)
         .ok_or(ContractError::StatsOverflow)?;
 
@@ -91,7 +95,11 @@ pub fn increment(
         ),
     );
 
-    Ok((next_invoices, next_volume, next_recipients_paid))
+    Ok(ProtocolStats {
+        total_invoices: next_invoices,
+        total_volume: next_volume,
+        total_recipients_paid: next_recipients_paid,
+    })
 }
 
 /// Records one newly created invoice.
@@ -129,3 +137,62 @@ pub fn volume_added(env: &Env, amount: i128) -> Result<Stats, ContractError> {
 pub fn recipients_paid(env: &Env, count: u64) -> Result<Stats, ContractError> {
     increment(env, 0, 0, count)
 }
+
+// ---------------------------------------------------------------------------
+// Unit tests
+// ---------------------------------------------------------------------------
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use soroban_sdk::Env;
+
+    #[test]
+    fn test_get_stats_initial_state() {
+        let env = Env::default();
+        let stats = get_stats(&env);
+        assert_eq!(stats.total_invoices, 0);
+        assert_eq!(stats.total_volume, 0);
+        assert_eq!(stats.total_recipients_paid, 0);
+    }
+
+    #[test]
+    fn test_invoice_created_increments_counter() {
+        let env = Env::default();
+        invoice_created(&env).unwrap();
+        let stats = get_stats(&env);
+        assert_eq!(stats.total_invoices, 1);
+        assert_eq!(stats.total_volume, 0);
+        assert_eq!(stats.total_recipients_paid, 0);
+    }
+
+    #[test]
+    fn test_volume_added() {
+        let env = Env::default();
+        volume_added(&env, 1_000).unwrap();
+        let stats = get_stats(&env);
+        assert_eq!(stats.total_invoices, 0);
+        assert_eq!(stats.total_volume, 1_000);
+        assert_eq!(stats.total_recipients_paid, 0);
+    }
+
+    #[test]
+    fn test_recipients_paid_increments() {
+        let env = Env::default();
+        recipients_paid(&env, 3).unwrap();
+        let stats = get_stats(&env);
+        assert_eq!(stats.total_invoices, 0);
+        assert_eq!(stats.total_volume, 0);
+        assert_eq!(stats.total_recipients_paid, 3);
+    }
+
+    #[test]
+    fn test_increment_uses_named_fields() {
+        let env = Env::default();
+        let result = increment(&env, 2, 500, 4).unwrap();
+        // Named fields — not positional
+        assert_eq!(result.total_invoices, 2);
+        assert_eq!(result.total_volume, 500);
+        assert_eq!(result.total_recipients_paid, 4);
+    }
+}

contracts/split/src/storage_keys.rs

@@ -213,6 +213,9 @@ pub enum InvoiceKey {
     GroupTreasury(u64),
     TimelockAction(u64),
     PayoutCheckpoint(u64),
+    /// Per-invoice event sequence counter — typed replacement for the former
+    /// `(symbol_short!("ev_seq"), invoice_id)` inline key (issue #708).
+    EvSeq(u64),
 }
 
 // ---------------------------------------------------------------------------
@@ -386,6 +389,7 @@ mod tests {
             InvoiceKey::RecipientsList(id), InvoiceKey::AmountsList(id),
             InvoiceKey::PaidFlags(id), InvoiceKey::MilestoneFlags(id),
             InvoiceKey::ArchiveMarker(id), InvoiceKey::CreatedLedger(id),
+            InvoiceKey::EvSeq(id),
         ];
         for i in 0..keys.len() {
             for j in (i + 1)..keys.len() {
@@ -510,3 +514,15 @@ pub fn tombstone_key(invoice_id: u64) -> (Symbol, u64) {
     (symbol_short!("tombstone"), invoice_id)
 }
 
+// ---------------------------------------------------------------------------
+// Issue #708: Per-invoice event sequence counter (typed key)
+// ---------------------------------------------------------------------------
+
+/// Per-invoice event sequence counter — temporary storage.
+///
+/// Returns the [`InvoiceKey::EvSeq`] variant for `invoice_id`, replacing the
+/// old inline `(symbol_short!("ev_seq"), invoice_id)` tuple.
+pub fn ev_seq_key(invoice_id: u64) -> InvoiceKey {
+    InvoiceKey::EvSeq(invoice_id)
+}
+