diff --git a/contracts/split/src/calc.rs b/contracts/split/src/calc.rs index 719c77d..e30dde8 100644 --- a/contracts/split/src/calc.rs +++ b/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
) { *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 { + 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 // ----------------------------------------------------------------------- diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index c2868a3..3619bd7 100644 --- a/contracts/split/src/events.rs +++ b/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); + } +} diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index 98feb2b..f5e1624 100644 --- a/contracts/split/src/lib.rs +++ b/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
= 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() { diff --git a/contracts/split/src/stats.rs b/contracts/split/src/stats.rs index a05d0d2..36d8455 100644 --- a/contracts/split/src/stats.rs +++ b/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 { - let (current_invoices, current_volume, current_recipients_paid) = get_stats(env); +) -> Result { + 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 { pub fn recipients_paid(env: &Env, count: u64) -> Result { 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); + } +} diff --git a/contracts/split/src/storage_keys.rs b/contracts/split/src/storage_keys.rs index 9b13fde..8117f0c 100644 --- a/contracts/split/src/storage_keys.rs +++ b/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) +} + diff --git a/contracts/split/src/test.rs b/contracts/split/src/test.rs index 84ce332..ca0e831 100644 --- a/contracts/split/src/test.rs +++ b/contracts/split/src/test.rs @@ -82,62 +82,12 @@ fn default_options(env: &Env) -> InvoiceOptions { ratios: Vec::new(env), cosigners: None, cosigner_threshold: None, - ext: types::InvoiceOptions2 { - target_usd_cents: None, - payment_token: None, - release_delay_ledgers: None, - metadata_hash: None, - payment_cooldown_secs: None, - max_payments_per_window: None, - payment_window_secs: None, - oracle: None, - oracle_asset_pair_base: None, - oracle_asset_pair_quote: None, - min_payer_rep: None, - payment_open_at: None, - payment_close_at: None, - milestones: None, - recipient_max_payouts: None, - release_condition_hash: None, - recipient_whitelist_enabled: false, - escrow_hold_period: None, - overfunding_policy: types::OverfundingPolicy::Cap, - early_bird_window_ledgers: 0, - early_bird_fee_bps: 0, - creator_fee_bps: 0, - early_bird_fee_credit: 0, - ratio_denominator: 10_000, - }, + ext: types::InvoiceOptions2::default(), } } fn default_options2(_env: &Env) -> InvoiceOptions2 { - InvoiceOptions2 { - target_usd_cents: None, - payment_token: None, - release_delay_ledgers: None, - metadata_hash: None, - payment_cooldown_secs: None, - max_payments_per_window: None, - payment_window_secs: None, - oracle: None, - oracle_asset_pair_base: None, - oracle_asset_pair_quote: None, - min_payer_rep: None, - payment_open_at: None, - payment_close_at: None, - milestones: None, - recipient_max_payouts: None, - release_condition_hash: None, - recipient_whitelist_enabled: false, - escrow_hold_period: None, - overfunding_policy: types::OverfundingPolicy::Cap, - early_bird_window_ledgers: 0, - early_bird_fee_bps: 0, - creator_fee_bps: 0, - early_bird_fee_credit: 0, - ratio_denominator: 10_000, - } + InvoiceOptions2::default() } fn invoice_options( @@ -187,30 +137,10 @@ fn invoice_options( cosigners: None, cosigner_threshold: None, ext: types::InvoiceOptions2 { - target_usd_cents: None, - payment_token: None, - release_delay_ledgers: None, - metadata_hash: None, payment_cooldown_secs: cooldown_secs, max_payments_per_window: max_payments, payment_window_secs: window_secs, - oracle: None, - oracle_asset_pair_base: None, - oracle_asset_pair_quote: None, - min_payer_rep: None, - payment_open_at: None, - payment_close_at: None, - milestones: None, - recipient_max_payouts: None, - release_condition_hash: None, - recipient_whitelist_enabled: false, - escrow_hold_period: None, - overfunding_policy: types::OverfundingPolicy::Cap, - early_bird_window_ledgers: 0, - early_bird_fee_bps: 0, - creator_fee_bps: 0, - early_bird_fee_credit: 0, - ratio_denominator: 10_000, + ..Default::default() }, } } diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index 36ca6a3..15521d2 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -548,6 +548,51 @@ pub struct InvoiceOptions2 { pub ratio_denominator: u64, } +impl Default for InvoiceOptions2 { + /// Returns an `InvoiceOptions2` with every optional field set to `None`, + /// every boolean to `false`, every numeric to `0`, and + /// `overfunding_policy` to [`OverfundingPolicy::Cap`] (the historical + /// behaviour). `ratio_denominator` is `10_000` to match + /// [`InvoiceExt2::default`]. + /// + /// Tests that only care about one or two fields can use this as a + /// starting point and override just those fields: + /// ``` + /// let opts = InvoiceOptions2 { + /// payment_cooldown_secs: Some(60), + /// ..Default::default() + /// }; + /// ``` + fn default() -> Self { + InvoiceOptions2 { + target_usd_cents: None, + payment_token: None, + release_delay_ledgers: None, + metadata_hash: None, + payment_cooldown_secs: None, + max_payments_per_window: None, + payment_window_secs: None, + oracle: None, + oracle_asset_pair_base: None, + oracle_asset_pair_quote: None, + min_payer_rep: None, + payment_open_at: None, + payment_close_at: None, + milestones: None, + recipient_max_payouts: None, + release_condition_hash: None, + recipient_whitelist_enabled: false, + escrow_hold_period: None, + overfunding_policy: OverfundingPolicy::Cap, + early_bird_window_ledgers: 0, + early_bird_fee_bps: 0, + creator_fee_bps: 0, + early_bird_fee_credit: 0, + ratio_denominator: 10_000, + } + } +} + /// Legacy invoice layout used by stored invoices created before the `version` /// field was added. Kept for on-chain migration so old data can be /// deserialised and re-saved in the current schema.