diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index b2fe796..351561d 100644 --- a/contracts/split/src/lib.rs +++ b/contracts/split/src/lib.rs @@ -68,6 +68,8 @@ mod storage_keys; mod migrations; +mod validation; + use error::ContractError; use soroban_sdk::crypto::bls12_381::{Fr, G1Affine}; use soroban_sdk::xdr::ToXdr; @@ -2169,29 +2171,39 @@ fn is_paused(env: &Env) -> bool { }) } -fn require_not_paused(env: &Env) { - migrations::require_schema_current(env); - assert!(!is_paused(env), "contract is paused"); - // Issue #297: also check circuit breaker +/// Issue #626: Reusable pause guard. +/// +/// Returns `Err(ContractError::ContractPaused)` when: +/// - the instance-level `Paused` flag is `true`, or +/// - the circuit-breaker persistent flag (issue #297) is `true`. +/// +/// Does **not** check the schema version; callers that also need a version +/// guard should call `migrations::require_schema_current` first (see +/// `require_not_paused` below). +fn assert_not_paused(env: &Env) -> Result<(), ContractError> { + if is_paused(env) { + return Err(ContractError::ContractPaused); + } + // Issue #297: also check circuit breaker. let cb_active: bool = env .storage() .persistent() .get(&circuit_breaker_key()) .unwrap_or(false); - assert!(!cb_active, "ContractPaused"); + if cb_active { + return Err(ContractError::ContractPaused); + } + Ok(()) +} + +fn require_not_paused(env: &Env) { + migrations::require_schema_current(env); + assert_not_paused(env).expect("contract is paused"); } fn check_not_paused(env: &Env) { migrations::require_schema_current(env); - if is_paused(env) { - panic!("ContractPaused"); - } - let cb_active: bool = env - .storage() - .persistent() - .get(&circuit_breaker_key()) - .unwrap_or(false); - if cb_active { + if assert_not_paused(env).is_err() { panic!("ContractPaused"); } } @@ -4231,8 +4243,9 @@ impl SplitContract { admin.require_auth(); assert!(!fee_recipients.is_empty(), "fee_recipients must not be empty"); - let sum: u32 = fee_recipients.iter().map(|r| r.basis_points).sum(); - assert!(sum == 10_000, "fee_recipients basis points must sum to 10000"); + let sum: u32 = fee_recipients.iter().map(|r| r.basis_points).fold(0u32, |a, b| a.saturating_add(b)); + validation::assert_bps_total(sum) + .expect("fee_recipients basis points must sum to 10000"); env.storage() .instance() @@ -5315,19 +5328,15 @@ impl SplitContract { } if !tranches.is_empty() { - let total_bps: u32 = tranches.iter().map(|t| t.basis_points).sum(); - assert!( - total_bps == 10_000, - "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)); + validation::assert_bps_total(total_bps) + .expect("tranches must sum to 10000 basis points"); } if !release_stages.is_empty() { - let total_bps: u32 = release_stages.iter().sum(); - assert!( - total_bps == 10_000, - "release_stages must sum to 10000 basis points" - ); + 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 milestones = milestones.unwrap_or_else(|| Vec::new(env)); validate_milestones(env, &milestones); @@ -15047,8 +15056,9 @@ impl SplitContract { ); // Ratios must sum to exactly 10 000 bps. - let ratio_sum: u32 = ratios.iter().sum(); - assert!(ratio_sum == 10_000, "ratios must sum to 10000 basis points"); + let ratio_sum: u32 = ratios.iter().fold(0u32, |a, b| a.saturating_add(b)); + validation::assert_bps_total(ratio_sum) + .expect("ratios must sum to 10000 basis points"); // Assign the next template ID for this creator. let next_id: u64 = env diff --git a/contracts/split/src/storage_keys.rs b/contracts/split/src/storage_keys.rs index e6f22bf..c6c9bfd 100644 --- a/contracts/split/src/storage_keys.rs +++ b/contracts/split/src/storage_keys.rs @@ -44,50 +44,114 @@ use soroban_sdk::{contracttype, symbol_short, Address, Env, IntoVal, Symbol, Try #[contracttype] #[derive(Clone)] pub enum StorageKey { + // --- Admin --- + /// Primary super-admin address. Admin, + /// Role map: Address → AdminRole for RBAC. Admins, + /// Pending admin address for two-step admin transfer. + PendingAdmin, + /// Governance contract address. + GovernanceContract, + /// Registered factory contracts. + Factories, + + // --- Pause / Circuit breaker --- + /// Global pause flag (true = all write entry-points blocked). Paused, + /// Set of individual function names that are selectively paused. PausedFns, - Treasury, - UsdcToken, + /// Circuit-breaker active flag (issue #297). + CircuitBreaker, + /// Human-readable reason string set when the circuit breaker fires. + CircuitBreakerReason, + + // --- Fees --- + /// One-time invoice creation fee (in stroops / token units). CreationFee, + /// Platform fee in basis points charged on release. PlatformFeeBps, + /// Addresses exempt from the platform fee. PlatformFeeWaiverList, + /// Addresses exempt from the creation fee. CreatorFeeWaiver, - Counter, + /// Tiered fee schedule (Vec). + FeeTiers, + /// Underlying protocol / network fee (issue #559 extension). + ProtocolFee, + + // --- Tokens / Treasury --- + /// Default USDC token contract address. + UsdcToken, + /// Primary treasury address for fee collection. + Treasury, + /// DEX router contract used for token swaps. + DexContract, + + // --- Invoice limits / config --- + /// Global cap on the number of active payers per invoice. GlobalPayerLimit, + /// Rolling-window length (in ledgers) for the global payer rate limit. GlobalPayerWindow, - StreamContract, - CreatorWhitelist, - Compliance, - KycContract, + /// Generic monotonic counter used by various features. + Counter, + /// Maximum cancel-rate threshold in basis points. + MaxCancelBps, + /// Minimum invoice-volume before a platform milestone is triggered. + PlatformVolThresh, + /// Last platform-volume milestone that was recorded. + PlatformVolMile, + /// Per-creator volume threshold for milestone events. + CreatorVolThresh, + /// Number of ledgers after which a released/refunded invoice is archived. + ArchiveAfterLedgers, + + // --- Contract config --- + /// Timelock duration in seconds for governance-gated actions. + TimelockSecs, + /// Monotonically increasing counter for timelock action IDs. + TimelockActionCounter, + /// Rate-limit cap: max calls per window for protected entry-points. RateLimit, + /// Rolling-window length (in ledgers) for rate-limit tracking. RateWindow, - MaxCancelBps, + + // --- Integrations --- + /// Streaming payment contract address. + StreamContract, + /// Receipt-NFT factory contract address. ReceiptFactory, + /// Dashboard analytics contract address. DashboardContract, + /// NFT gate contract address (token-gated access). NftGate, - TimelockSecs, - TimelockActionCounter, - FeeTiers, - PendingAdmin, - GovernanceContract, - Factories, - DexContract, + /// KYC / compliance oracle contract address. + KycContract, + /// Compliance module contract address. + Compliance, + /// Creator allowlist for restricted-deployment mode. + CreatorWhitelist, + + // --- Stats --- + /// Cumulative count of all invoices ever created. TotalInvoices, + /// Cumulative payment volume across all invoices. TotalVolume, + /// Cumulative amount successfully released to recipients. TotalReleased, + /// Cumulative amount refunded to payers. TotalRefunded, + /// Counter of treasury-group invoices. TreasuryGroupCounter, + + // --- Upgrade / versioning --- + /// Deployed contract schema/version number. ContractVersion, - ArchiveAfterLedgers, - CircuitBreaker, - CircuitBreakerReason, - PlatformVolThresh, - PlatformVolMile, - CreatorVolThresh, + /// Pending WASM-upgrade proposal hash and metadata. UpgradeProposal, - ProtocolFee, + + // --- Reentrancy --- + /// Reentrancy guard flag (stored in temporary storage; cleared each tx). ReentrancyGuard, } diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index 7d491be..1ceb0d4 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -1220,7 +1220,7 @@ impl Invoice { let bytes = &compact.data; // Unpack status (1 byte) - let status_byte = bytes.get(0).unwrap(); + let status_byte = bytes.get(0).expect("from_compact: byte 0 (status) missing"); let status = match status_byte { 0 => InvoiceStatus::Pending, 1 => InvoiceStatus::Released, @@ -1237,14 +1237,18 @@ impl Invoice { // Unpack funded (16 bytes) let mut funded_bytes = [0u8; 16]; for (i, byte) in funded_bytes.iter_mut().enumerate() { - *byte = bytes.get((1 + i) as u32).unwrap(); + *byte = bytes + .get((1 + i) as u32) + .expect("from_compact: funded byte missing"); } let funded = i128::from_be_bytes(funded_bytes); // Unpack deadline (8 bytes) let mut deadline_bytes = [0u8; 8]; for (i, byte) in deadline_bytes.iter_mut().enumerate() { - *byte = bytes.get((17 + i) as u32).unwrap(); + *byte = bytes + .get((17 + i) as u32) + .expect("from_compact: deadline byte missing"); } let deadline = u64::from_be_bytes(deadline_bytes); diff --git a/contracts/split/src/validation.rs b/contracts/split/src/validation.rs index e0c16da..08588c9 100644 --- a/contracts/split/src/validation.rs +++ b/contracts/split/src/validation.rs @@ -64,6 +64,37 @@ pub fn assert_recipients_have_trustlines( Ok(()) } +/// Issue #623: Verify that the sum of `values` equals exactly `BASIS_POINTS_TOTAL` (10 000). +/// +/// Used wherever a slice of basis-point weights must cover 100% of a whole: +/// split ratios, release stages, fee recipients, etc. +/// +/// # Errors +/// Returns `Err(ContractError::InvalidRatioSum)` when `values.iter().sum::() != 10_000`. +/// +/// # Examples +/// ``` +/// assert!(assert_bps_sum(&[5_000u32, 5_000]).is_ok()); +/// assert!(assert_bps_sum(&[3_000u32, 3_000]).is_err()); +/// ``` +pub const BASIS_POINTS_TOTAL: u32 = 10_000; + +pub fn assert_bps_sum(values: &[u32]) -> Result<(), ContractError> { + let sum: u32 = values.iter().copied().fold(0u32, |acc, v| acc.saturating_add(v)); + assert_bps_total(sum) +} + +/// Variant of [`assert_bps_sum`] for call sites where the sum has already +/// been computed (e.g. from a soroban `Vec` iterator in a `no_std` context). +/// +/// Returns `Err(ContractError::InvalidRatioSum)` when `total != 10_000`. +pub fn assert_bps_total(total: u32) -> Result<(), ContractError> { + if total != BASIS_POINTS_TOTAL { + return Err(ContractError::InvalidRatioSum); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -103,4 +134,33 @@ mod tests { let v: Vec
= Vec::new(&env); assert!(assert_unique_recipients(&env, &v.to_vec()).is_ok()); } + + // --- assert_bps_sum (issue #623) --- + + #[test] + fn bps_sum_equals_10000_passes() { + assert!(assert_bps_sum(&[5_000u32, 5_000]).is_ok()); + assert!(assert_bps_sum(&[3_000u32, 3_000, 4_000]).is_ok()); + assert!(assert_bps_sum(&[10_000u32]).is_ok()); + } + + #[test] + fn bps_sum_not_10000_fails() { + assert_eq!( + assert_bps_sum(&[3_000u32, 3_000]), + Err(ContractError::InvalidRatioSum) + ); + assert_eq!( + assert_bps_sum(&[0u32]), + Err(ContractError::InvalidRatioSum) + ); + assert_eq!( + assert_bps_sum(&[10_001u32]), + Err(ContractError::InvalidRatioSum) + ); + assert_eq!( + assert_bps_sum(&[]), + Err(ContractError::InvalidRatioSum) + ); + } }