From c7eddc03e3b263d1ab69c80b62855f664a9058bc Mon Sep 17 00:00:00 2001 From: kulkan-IV Date: Fri, 28 Aug 2026 07:54:51 +0000 Subject: [PATCH] fix(events): add event_seq to delegate_set, revoker to delegate_revoked - #619: call next_seq inside delegate_set and publish (delegate, event_seq) so indexers can order multiple delegate changes within the same tx - #620: add revoker: &Address param to delegate_revoked and publish (revoker, ledger_sequence) for complete audit trails - #621: extract assert_invoice_pending helper to replace repeated if status != Pending { return Err(InvalidStatus) } inline checks - #622: add pub fn calc_platform_fee(funded, fee_bps) to calc.rs with checked_mul / ArithmeticOverflow guard; replace all three inline fee expressions in lib.rs with calls to the new helper; add unit tests covering normal, zero-bps, 100%-bps and overflow cases --- contracts/split/src/calc.rs | 56 ++++++++++++++++++++++++++++++++++- contracts/split/src/events.rs | 11 +++---- contracts/split/src/lib.rs | 32 +++++++++++++++----- 3 files changed, 85 insertions(+), 14 deletions(-) diff --git a/contracts/split/src/calc.rs b/contracts/split/src/calc.rs index cd8a120..ace3f9a 100644 --- a/contracts/split/src/calc.rs +++ b/contracts/split/src/calc.rs @@ -4,7 +4,31 @@ //! across recipients proportionally, ensuring every stroop is accounted for //! (i.e. `sum(result) == total` always holds). -use soroban_sdk::{Env, Vec}; +use soroban_sdk::{Address, BytesN, Env, Vec}; + +use crate::error::ContractError; + +// --------------------------------------------------------------------------- +// Platform fee calculation +// --------------------------------------------------------------------------- + +/// Calculate the platform fee for a given funded amount and fee rate in +/// basis points (1 bps = 0.01%). +/// +/// # Formula +/// `fee = funded * fee_bps / 10_000` +/// +/// Uses checked arithmetic to prevent silent overflow on large amounts. +/// +/// # Errors +/// Returns [`ContractError::ArithmeticOverflow`] if the intermediate +/// multiplication `funded * fee_bps` overflows `i128`. +pub fn calc_platform_fee(funded: i128, fee_bps: u32) -> Result { + let numerator = funded + .checked_mul(fee_bps as i128) + .ok_or(ContractError::ArithmeticOverflow)?; + Ok(numerator / 10_000) +} /// Distribute `total` among recipients according to their `ratios` out of /// `denom`, using the largest-remainder method to handle rounding. @@ -272,4 +296,34 @@ mod tests { assert_exact(&env, total, ratios, denom); } } + + // ----------------------------------------------------------------------- + // calc_platform_fee tests + // ----------------------------------------------------------------------- + + #[test] + fn test_calc_platform_fee_normal() { + // 1_000_000 funded at 250 bps (2.5%) → fee = 25_000 + let fee = calc_platform_fee(1_000_000, 250).unwrap(); + assert_eq!(fee, 25_000); + } + + #[test] + fn test_calc_platform_fee_zero_bps() { + // Zero fee rate → always zero fee regardless of funded amount + assert_eq!(calc_platform_fee(999_999_999, 0).unwrap(), 0); + } + + #[test] + fn test_calc_platform_fee_max_bps() { + // 10_000 bps = 100% → fee equals funded + assert_eq!(calc_platform_fee(500, 10_000).unwrap(), 500); + } + + #[test] + fn test_calc_platform_fee_overflow() { + // i128::MAX * any fee_bps > 0 will overflow the intermediate multiplication + let result = calc_platform_fee(i128::MAX, 1); + assert_eq!(result, Err(crate::error::ContractError::ArithmeticOverflow)); + } } diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index 46db8b6..0b27340 100644 --- a/contracts/split/src/events.rs +++ b/contracts/split/src/events.rs @@ -251,25 +251,26 @@ pub fn invoice_archived(env: &Env, invoice_id: u64) { /// Emitted when a delegate is assigned to an invoice. /// Topics: (split, delegated, invoice_id) -/// Data: delegate +/// Data: (delegate, event_seq) pub fn delegate_set(env: &Env, invoice_id: u64, delegate: &Address) { + let event_seq = next_seq(env, invoice_id); env.events().publish( ( symbol_short!("split"), symbol_short!("delegated"), invoice_id, ), - delegate.clone(), + (delegate.clone(), event_seq), ); } /// Emitted when a delegate is revoked from an invoice. /// Topics: (split, revoked, invoice_id) -/// Data: () -pub fn delegate_revoked(env: &Env, invoice_id: u64) { +/// Data: (revoker, ledger_sequence) +pub fn delegate_revoked(env: &Env, invoice_id: u64, revoker: &Address) { env.events().publish( (symbol_short!("split"), symbol_short!("revoked"), invoice_id), - (), + (revoker.clone(), env.ledger().sequence()), ); } diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index b2fe796..f4dea3a 100644 --- a/contracts/split/src/lib.rs +++ b/contracts/split/src/lib.rs @@ -54,6 +54,7 @@ const DEFAULT_INVOICE_STORAGE_QUOTA: u64 = 65_536; mod error; mod events; pub mod types; +mod calc; #[cfg(test)] mod test; @@ -69,6 +70,7 @@ mod storage_keys; mod migrations; use error::ContractError; +use calc::calc_platform_fee; use soroban_sdk::crypto::bls12_381::{Fr, G1Affine}; use soroban_sdk::xdr::ToXdr; use soroban_sdk::{ @@ -1931,6 +1933,22 @@ fn load_invoice(env: &Env, id: u64) -> Invoice { invoice } +/// Guard: return `Err(ContractError::InvalidStatus)` unless the invoice is +/// still in the `Pending` state. Calling this at the top of any mutating +/// function replaces the repeated inline pattern: +/// +/// ```rust,ignore +/// if invoice.status != InvoiceStatus::Pending { +/// return Err(ContractError::InvalidStatus); +/// } +/// ``` +fn assert_invoice_pending(invoice: &Invoice) -> Result<(), ContractError> { + if invoice.status != InvoiceStatus::Pending { + return Err(ContractError::InvalidStatus); + } + Ok(()) +} + /// Estimates the serialised size (in bytes) of an invoice's persisted /// representation (issue #425). Sums the XDR-encoded length of the three /// pieces `save_invoice` actually writes to storage (`InvoiceCore`, @@ -3001,9 +3019,7 @@ impl SplitContract { let mut invoice = load_invoice(&env, invoice_id); // Only allow withdrawal while invoice is in Pending (Open) status. - if invoice.status != InvoiceStatus::Pending { - return Err(ContractError::InvalidStatus); - } + assert_invoice_pending(&invoice)?; let contrib_key = contribution_key(invoice_id, &payer); let amount: i128 = env @@ -9009,7 +9025,7 @@ impl SplitContract { let fee = if is_waived { 0 } else { - (proportional as u128 * platform_fee_bps as u128 / 10_000u128) as i128 + calc_platform_fee(proportional, platform_fee_bps).expect("ArithmeticOverflow") }; let tax = (proportional as u128 * invoice.tax_bps as u128 / 10_000u128) as i128; let payout = proportional - fee - tax; @@ -9573,7 +9589,7 @@ impl SplitContract { let fee = if is_waived { 0 } else { - (payout_raw as u128 * platform_fee_bps as u128 / 10_000u128) as i128 + calc_platform_fee(payout_raw, platform_fee_bps).expect("ArithmeticOverflow") }; let tax = (payout_raw as u128 * invoice.tax_bps as u128 / 10_000u128) as i128; let payout = payout_raw - fee - tax; @@ -10303,8 +10319,8 @@ impl SplitContract { (amount as u128 * member_funded as u128 / member_total as u128) as i128 }; - let fee = (proportional as u128 * platform_fee_bps as u128 / 10_000u128) - as i128; + let fee = calc_platform_fee(proportional, platform_fee_bps) + .expect("ArithmeticOverflow"); let tax = (proportional as u128 * member.tax_bps as u128 / 10_000u128) as i128; let payout = proportional - fee - tax; @@ -12957,7 +12973,7 @@ impl SplitContract { env.storage().persistent().remove(&delegate_key(invoice_id)); - events::delegate_revoked(&env, invoice_id); + events::delegate_revoked(&env, invoice_id, &invoice.creator); append_audit_entry(&env, invoice_id, symbol_short!("rvk_del"), &invoice.creator); }