fix: prevent negative merchant amounts in fee split

settlement_contract/src/admin.rs

@@ -505,6 +505,9 @@ impl SettlementContract {
             if new_rule.platform_fee_bps < MIN_FEE_BPS || new_rule.network_fee_bps < MIN_FEE_BPS {
                 panic_with_error!(env, SettlementError::InvalidFeeBps);
             }
+            if new_rule.platform_fee_bps + new_rule.network_fee_bps > BPS_DENOMINATOR {
+                panic_with_error!(env, SettlementError::InvalidFeeBps);
+            }
             if new_rule.settlement_delay_ledger > MAX_SETTLEMENT_DELAY_LEDGER {
                 panic_with_error!(env, SettlementError::InvalidSettlementDelay);
             }

settlement_contract/src/errors.rs

@@ -73,6 +73,9 @@ pub enum SettlementError {
     AmountTooSmall = 313,
     AmountZero = 314,
     AmountNegative = 315,
+    /// The rounded platform and network fees exceed the gross payment amount.
+    /// Raised by `calculate_split` before a `FeeSplit` or `PaymentRecord` is produced.
+    SplitExceedsAmount = 316,
 }
 
 const _: () = {
@@ -121,4 +124,5 @@ const _: () = {
     assert!(SettlementError::AmountTooSmall as u32 >= error_codes::SETTLEMENT_RANGE_START);
     assert!(SettlementError::AmountZero as u32 >= error_codes::SETTLEMENT_RANGE_START);
     assert!(SettlementError::AmountNegative as u32 >= error_codes::SETTLEMENT_RANGE_START);
+    assert!(SettlementError::SplitExceedsAmount as u32 >= error_codes::SETTLEMENT_RANGE_START);
 };

settlement_contract/src/payments.rs

@@ -11,15 +11,6 @@ use crate::{
 };
 
 /// Computes the platform, network, and merchant fee amounts for an amount using ceil-based rounding.
-///
-/// # Known edge case: negative merchant amount
-///
-/// Ceiling rounding of both fees independently can make
-/// `platform_fee_amount + network_fee_amount > amount` for small gross amounts
-/// (e.g. `amount = 1`, `platform_fee_bps = 5000`, `network_fee_bps = 5000`),
-/// which yields a **negative** `merchant_amount`. This is intentional with the
-/// current rounding policy (fees are never under-collected); callers must treat
-/// a negative merchant payout as a known, documented outcome rather than a bug.
 fn calculate_split(env: &Env, amount: i128, rule: &SettlementRule) -> FeeSplit {
     let denom = BPS_DENOMINATOR as i128;
     let platform_bps = rule.platform_bps();
@@ -42,11 +33,16 @@ fn calculate_split(env: &Env, amount: i128, rule: &SettlementRule) -> FeeSplit {
     let platform_fee_amount = platform_bps.calculate_fee_ceil(amount);
     let network_fee_amount = network_bps.calculate_fee_ceil(amount);
 
-    // The merchant amount is calculated as the subtraction remainder of the gross amount minus all rounded-up fees.
-    // This ensures the sum of the split amounts (platform fee + network fee + merchant share) always equals the gross amount.
-    // Consequence: The merchant absorbs all rounding dust. For very small gross amounts with high/extreme fee percentages,
-    // the sum of rounded-up fees can exceed the gross amount, resulting in a negative merchant payout.
-    let merchant_amount = amount - platform_fee_amount - network_fee_amount;
+    let total_fee_amount = platform_fee_amount
+        .checked_add(network_fee_amount)
+        .unwrap_or_else(|| panic_with_error!(env, SettlementError::AmountOverflow));
+    if total_fee_amount > amount {
+        panic_with_error!(env, SettlementError::SplitExceedsAmount);
+    }
+
+    let merchant_amount = amount
+        .checked_sub(total_fee_amount)
+        .unwrap_or_else(|| panic_with_error!(env, SettlementError::AmountOverflow));
     FeeSplit {
         gross_amount: amount,
         platform_fee_amount,

settlement_contract/src/settlement.rs

@@ -98,6 +98,9 @@ impl SettlementContract {
             if new_rule.platform_fee_bps < MIN_FEE_BPS || new_rule.network_fee_bps < MIN_FEE_BPS {
                 panic_with_error!(&env, SettlementError::InvalidFeeBps);
             }
+            if new_rule.platform_fee_bps + new_rule.network_fee_bps > BPS_DENOMINATOR {
+                panic_with_error!(&env, SettlementError::InvalidFeeBps);
+            }
             if new_rule.settlement_delay_ledger > MAX_SETTLEMENT_DELAY_LEDGER {
                 panic_with_error!(&env, SettlementError::InvalidSettlementDelay);
             }

settlement_contract/src/tests/conformity_tests.rs

@@ -60,7 +60,7 @@ fn governance_codes() -> [(&'static str, u32); 19] {
     ]
 }
 
-fn settlement_codes() -> [(&'static str, u32); 24] {
+fn settlement_codes() -> [(&'static str, u32); 30] {
     [
         (
             "AlreadyInitialized",
@@ -122,6 +122,21 @@ fn settlement_codes() -> [(&'static str, u32); 24] {
         ),
         ("InvalidGovernance", SettlementError::InvalidGovernance as u32),
         ("AmountOverflow", SettlementError::AmountOverflow as u32),
+        (
+            "GovernanceCallFailed",
+            SettlementError::GovernanceCallFailed as u32,
+        ),
+        (
+            "FeeExceedsGovernanceConfig",
+            SettlementError::FeeExceedsGovernanceConfig as u32,
+        ),
+        ("AmountTooSmall", SettlementError::AmountTooSmall as u32),
+        ("AmountZero", SettlementError::AmountZero as u32),
+        ("AmountNegative", SettlementError::AmountNegative as u32),
+        (
+            "SplitExceedsAmount",
+            SettlementError::SplitExceedsAmount as u32,
+        ),
     ]
 }
 

settlement_contract/src/tests/mod.rs

@@ -6,6 +6,7 @@
 
 pub mod admin_tests;
 pub mod conformity_tests;
+pub mod payment_tests;
 
 use crate::*;
 use soroban_sdk::testutils::Address as _;

settlement_contract/src/tests/payment_tests.rs

@@ -0,0 +1,145 @@
+//! Tests for payment reference storage and fee-split calculation.
+
+extern crate std;
+
+use crate::*;
+use soroban_sdk::testutils::Ledger;
+use soroban_sdk::{Address, BytesN, Env};
+
+use super::setup;
+
+fn rule(platform_fee_bps: u32, network_fee_bps: u32) -> SettlementRule {
+    SettlementRule {
+        platform_fee_bps,
+        network_fee_bps,
+        settlement_delay_ledger: 0,
+        auto_settle: false,
+    }
+}
+
+fn payment_reference(env: &Env, byte: u8) -> BytesN<32> {
+    BytesN::from_array(env, &[byte; 32])
+}
+
+fn registered_merchant_with_rule(
+    client: &SettlementContractClient<'static>,
+    admins: &soroban_sdk::Vec<Address>,
+    merchant: &Address,
+    settlement_rule: SettlementRule,
+) {
+    client.register_merchant(admins, merchant);
+    client.set_settlement_rule(admins, merchant, &settlement_rule);
+}
+
+#[test]
+fn calculate_fee_split_preserves_valid_combined_fees() {
+    let (_env, client, admins, merchant) = setup();
+    registered_merchant_with_rule(&client, &admins, &merchant, rule(250, 50));
+
+    let split = client.calculate_fee_split(&merchant, &10_000);
+
+    assert_eq!(split.gross_amount, 10_000);
+    assert_eq!(split.platform_fee_amount, 250);
+    assert_eq!(split.network_fee_amount, 50);
+    assert_eq!(split.merchant_amount, 9_700);
+}
+
+#[test]
+fn calculate_fee_split_allows_exact_100_percent_when_rounding_fits() {
+    let (_env, client, admins, merchant) = setup();
+    registered_merchant_with_rule(&client, &admins, &merchant, rule(5_000, 5_000));
+
+    let split = client.calculate_fee_split(&merchant, &100);
+
+    assert_eq!(split.gross_amount, 100);
+    assert_eq!(split.platform_fee_amount, 50);
+    assert_eq!(split.network_fee_amount, 50);
+    assert_eq!(split.merchant_amount, 0);
+}
+
+#[test]
+#[should_panic(expected = "Error(Contract, #316)")]
+fn calculate_fee_split_rejects_ceil_rounded_fees_above_amount() {
+    let (_env, client, admins, merchant) = setup();
+    registered_merchant_with_rule(&client, &admins, &merchant, rule(5_000, 5_000));
+
+    client.calculate_fee_split(&merchant, &101);
+}
+
+#[test]
+#[should_panic(expected = "Error(Contract, #316)")]
+fn calculate_fee_split_rejects_minimal_amount_when_rounding_exceeds_gross() {
+    let (_env, client, admins, merchant) = setup();
+    registered_merchant_with_rule(&client, &admins, &merchant, rule(5_000, 5_000));
+
+    client.calculate_fee_split(&merchant, &1);
+}
+
+#[test]
+#[should_panic(expected = "Error(Contract, #314)")]
+fn calculate_fee_split_rejects_zero_amount_before_split_calculation() {
+    let (_env, client, admins, merchant) = setup();
+    registered_merchant_with_rule(&client, &admins, &merchant, rule(250, 50));
+
+    client.calculate_fee_split(&merchant, &0);
+}
+
+#[test]
+fn store_payment_reference_rejects_ceil_rounded_split_without_storing_record() {
+    let (env, client, admins, merchant) = setup();
+    registered_merchant_with_rule(&client, &admins, &merchant, rule(5_000, 5_000));
+    let reference = payment_reference(&env, 1);
+
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        client.store_payment_reference(&merchant, &reference, &101);
+    }));
+
+    assert!(result.is_err());
+    assert!(client.get_payment_reference(&reference).is_none());
+}
+
+#[test]
+#[should_panic(expected = "Error(Contract, #4)")]
+fn set_default_rule_rejects_fee_sum_over_100_percent() {
+    let (_env, client, admins, _merchant) = setup();
+
+    client.set_default_rule(&admins, &rule(6_000, 6_000));
+}
+
+#[test]
+fn scheduled_set_default_rule_rejects_fee_sum_over_100_percent() {
+    let (env, client, admins, _merchant) = setup();
+    let admin = admins.get(0).unwrap();
+    let invalid_rule = rule(6_000, 6_000);
+    let operation = Operation::SetDefaultRule(invalid_rule);
+
+    client.schedule(&admin, &operation, &DEFAULT_TIMELOCK_DELAY_SECONDS);
+    env.ledger()
+        .with_mut(|ledger| ledger.timestamp += DEFAULT_TIMELOCK_DELAY_SECONDS);
+
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        client.execute(&operation);
+    }));
+
+    assert!(result.is_err());
+    assert!(client.get_default_rule().is_none());
+}
+
+#[test]
+fn store_payment_reference_keeps_valid_payment_storage_unchanged() {
+    let (env, client, admins, merchant) = setup();
+    registered_merchant_with_rule(&client, &admins, &merchant, rule(250, 50));
+    let reference = payment_reference(&env, 2);
+
+    let split = client.store_payment_reference(&merchant, &reference, &10_000);
+    let record = client.get_payment_reference(&reference).unwrap();
+
+    assert_eq!(split.gross_amount, 10_000);
+    assert_eq!(split.platform_fee_amount, 250);
+    assert_eq!(split.network_fee_amount, 50);
+    assert_eq!(split.merchant_amount, 9_700);
+    assert_eq!(record.amount, 10_000);
+    assert_eq!(record.platform_fee_amount, 250);
+    assert_eq!(record.network_fee_amount, 50);
+    assert_eq!(record.merchant_amount, 9_700);
+}