fix: return ArithmeticOverflow instead of panicking on i128 overflow

contracts/common/src/lib.rs

@@ -161,7 +161,11 @@ where
     let balance_before = token_client.balance(contract_addr);
     operation();
     let balance_after = token_client.balance(contract_addr);
-    balance_after - balance_before
+    // A malicious or deflationary token could leave `balance_after <
+    // balance_before`; a plain `-` would panic (abort) under
+    // `overflow-checks = true`. Saturate at 0: callers already reject
+    // `actual_received <= 0` with `InvalidAmount`.
+    balance_after.saturating_sub(balance_before)
 }
 
 /// Extends a persistent entry's TTL to (approximately) survive until

contracts/common/src/split.rs

@@ -38,6 +38,10 @@ pub enum SplitError {
     /// A recipient address equals the contract's own address — a self-payout
     /// that would strand funds in the contract with no recovery path.
     SelfPayout,
+    /// Checked i128 arithmetic overflowed. Returned instead of panicking
+    /// (release profile sets `overflow-checks = true` + `panic = "abort"`,
+    /// so an unchecked op would abort the whole transaction).
+    Overflow,
 }
 
 /// Validates that basis-point splits sum to exactly 10000 and computes the
@@ -54,7 +58,9 @@ pub fn compute_split(
 
     let mut bps_sum: i128 = 0;
     for (_, bps) in recipients.iter() {
-        bps_sum += bps as i128;
+        bps_sum = bps_sum
+            .checked_add(bps as i128)
+            .ok_or(SplitError::Overflow)?;
     }
     if bps_sum != BPS_DENOMINATOR {
         return Err(SplitError::InvalidSplit);
@@ -71,18 +77,23 @@ pub fn compute_split(
         }
     }
 
-    let fee = total * (fee_bps as i128) / BPS_DENOMINATOR;
-    let distributable = total - fee;
+    let fee = (total
+        .checked_mul(fee_bps as i128)
+        .ok_or(SplitError::Overflow)?)
+    / BPS_DENOMINATOR;
+    let distributable = total.checked_sub(fee).ok_or(SplitError::Overflow)?;
 
     let mut shares: Vec<(Address, i128)> = Vec::new(env);
     let mut order: Vec<(u32, i128, Address)> = Vec::new(env);
     let mut allocated: i128 = 0;
 
     for (recipient, bps) in recipients.iter() {
-        let numerator = distributable * (bps as i128);
+        let numerator = distributable
+            .checked_mul(bps as i128)
+            .ok_or(SplitError::Overflow)?;
         let share = numerator / BPS_DENOMINATOR;
         let remainder = numerator % BPS_DENOMINATOR;
-        allocated += share;
+        allocated = allocated.checked_add(share).ok_or(SplitError::Overflow)?;
         shares.push_back((recipient.clone(), share));
         order.push_back((order.len(), remainder, recipient));
     }
@@ -94,13 +105,15 @@ pub fn compute_split(
     // each award only consumes the selected entry and never changes any
     // other entry's remainder. `dust` is at most `recipients.len() - 1`, so
     // the first `dust` sorted entries always exist.
-    let dust = distributable - allocated;
+    let dust = distributable
+        .checked_sub(allocated)
+        .ok_or(SplitError::Overflow)?;
     if dust > 0 {
         sort_remainders_desc(&mut order);
         for k in 0..dust as u32 {
             let (index, _, _) = order.get(k).unwrap();
             let (recipient, share) = shares.get(index).unwrap();
-            shares.set(index, (recipient, share + 1));
+            shares.set(index, (recipient, share.checked_add(1).ok_or(SplitError::Overflow)?));
         }
     }
 
@@ -122,7 +135,9 @@ fn remainder_order_less(a: &(u32, i128, Address), b: &(u32, i128, Address)) -> b
 fn sift_down_remainder_order(order: &mut Vec<(u32, i128, Address)>, start: u32, end: u32) {
     let mut root = start;
     loop {
-        let mut child = 2 * root + 1;
+        // `order.len()` is bounded by MAX_SPONSORS (20); saturating math
+        // cannot wrap here but avoids any debug/release overflow panic.
+        let mut child = root.saturating_mul(2).saturating_add(1);
         if child >= end {
             break;
         }

contracts/escrow/src/error.rs

@@ -29,4 +29,8 @@ pub enum Error {
     /// A recipient address equals the contract's own address — a self-payout
     /// that would strand funds in the contract with no recovery path.
     SelfPayout = 20,
+    /// Checked i128 arithmetic overflowed (e.g. contribution total near
+    /// `i128::MAX`). Returned instead of panicking under release
+    /// `overflow-checks = true` + `panic = "abort"`.
+    ArithmeticOverflow = 21,
 }

contracts/escrow/src/lib.rs

@@ -261,7 +261,10 @@ mod contract {
             let contribution_key = DataKey::Contribution(issue_id, index);
             let mut contribution: Contribution =
                 env.storage().persistent().get(&contribution_key).ok_or(Error::ContributionNotFound)?;
-            contribution.amount += actual_received;
+            contribution.amount = contribution
+                .amount
+                .checked_add(actual_received)
+                .ok_or(Error::ArithmeticOverflow)?;
             contribution.timestamp = env.ledger().timestamp();
             env.storage()
                 .persistent()
@@ -281,7 +284,10 @@ mod contract {
             escrow.contributor_count += 1;
         }
 
-        escrow.amount += actual_received;
+        escrow.amount = escrow
+            .amount
+            .checked_add(actual_received)
+            .ok_or(Error::ArithmeticOverflow)?;
         env.storage().persistent().set(&key, &escrow);
         extend_ttl(&env, &key);
         extend_instance_ttl(&env);
@@ -324,6 +330,7 @@ mod contract {
             .map_err(|e| match e {
                 mergefi_common::SplitError::InvalidSplit => Error::InvalidSplit,
                 mergefi_common::SplitError::SelfPayout => Error::SelfPayout,
+                mergefi_common::SplitError::Overflow => Error::ArithmeticOverflow,
             })?;
         let treasury: Address = mergefi_common::require_treasury::<DataKey>(&env).unwrap();
         let token_client = token::Client::new(&env, &escrow.token);
@@ -375,7 +382,7 @@ mod contract {
         }
 
         let now = env.ledger().timestamp();
-        if now < escrow.deadline + GRACE_PERIOD {
+        if now < escrow.deadline.saturating_add(GRACE_PERIOD) {
             // Not yet expired + grace period: only the admin may force an early refund.
             let admin = require_admin(&env)?;
             admin.require_auth();

contracts/maintenance-pool/src/error.rs

@@ -27,4 +27,8 @@ pub enum Error {
     DepositCountOverflow = 14,
     /// The pool already holds `MAX_DEPOSITS` deposits (issue #94).
     TooManyDeposits = 15,
+    /// Checked i128 arithmetic overflowed (e.g. `balance` / monotonic
+    /// `total_deposited` near `i128::MAX`). Returned instead of panicking
+    /// under release `overflow-checks = true` + `panic = "abort"`.
+    ArithmeticOverflow = 16,
 }

contracts/maintenance-pool/src/lib.rs

@@ -138,8 +138,14 @@ mod contract {
         let token_client = token::Client::new(&env, &token);
         token_client.transfer(&sponsor, env.current_contract_address(), &amount);
 
-        pool.balance += amount;
-        pool.total_deposited += amount;
+        pool.balance = pool
+            .balance
+            .checked_add(amount)
+            .ok_or(Error::ArithmeticOverflow)?;
+        pool.total_deposited = pool
+            .total_deposited
+            .checked_add(amount)
+            .ok_or(Error::ArithmeticOverflow)?;
         let index = pool.deposit_count;
         pool.deposit_count = pool
             .deposit_count
@@ -200,15 +206,26 @@ mod contract {
 
         let fee_bps: u32 = mergefi_common::get_fee_bps::<DataKey>(&env)
             .ok_or(Error::NotInitialized)?;
-        let fee = amount * (fee_bps as i128) / BPS_DENOMINATOR;
-        let payout = amount - fee;
+        let fee = amount
+            .checked_mul(fee_bps as i128)
+            .ok_or(Error::ArithmeticOverflow)?
+            / BPS_DENOMINATOR;
+        let payout = amount
+            .checked_sub(fee)
+            .ok_or(Error::ArithmeticOverflow)?;
 
         let treasury: Address = mergefi_common::require_treasury::<DataKey>(&env).unwrap();
         let token_client = token::Client::new(&env, &pool.token);
         let contract_address = env.current_contract_address();
 
-        pool.balance -= amount;
-        pool.total_withdrawn += amount;
+        pool.balance = pool
+            .balance
+            .checked_sub(amount)
+            .ok_or(Error::ArithmeticOverflow)?;
+        pool.total_withdrawn = pool
+            .total_withdrawn
+            .checked_add(amount)
+            .ok_or(Error::ArithmeticOverflow)?;
         pool.last_withdraw_at = env.ledger().timestamp();
         env.storage().persistent().set(&pkey, &pool);
         extend_ttl(&env, &pkey);
@@ -289,7 +306,10 @@ mod contract {
         let token_client = token::Client::new(&env, &pool.token);
         token_client.transfer(&env.current_contract_address(), &sponsor, &deposit.amount);
 
-        pool.balance -= deposit.amount;
+        pool.balance = pool
+            .balance
+            .checked_sub(deposit.amount)
+            .ok_or(Error::ArithmeticOverflow)?;
         env.storage().persistent().set(&pkey, &pool);
         extend_ttl(&env, &pkey);
         extend_instance_ttl(&env);

contracts/milestones/src/error.rs

@@ -33,4 +33,8 @@ pub enum Error {
     /// A recipient address equals the contract's own address — a self-payout
     /// that would strand funds in the contract with no recovery path.
     SelfPayout = 20,
+    /// Checked i128 arithmetic overflowed (e.g. budget total near
+    /// `i128::MAX`). Returned instead of panicking under release
+    /// `overflow-checks = true` + `panic = "abort"`.
+    ArithmeticOverflow = 21,
 }

contracts/milestones/src/lib.rs

@@ -251,7 +251,10 @@ mod contract {
                 .persistent()
                 .get(&contribution_key)
                 .ok_or(Error::ContributionNotFound)?;
-            contribution.amount += actual_received;
+            contribution.amount = contribution
+                .amount
+                .checked_add(actual_received)
+                .ok_or(Error::ArithmeticOverflow)?;
             contribution.timestamp = env.ledger().timestamp();
             env.storage()
                 .persistent()
@@ -274,8 +277,14 @@ mod contract {
         // New funds arrive unallocated: the pool's total *and* its
         // unallocated remainder both grow by exactly the contribution, so
         // a later proportional refund treats them like any other share.
-        milestone.total_budget += actual_received;
-        milestone.remaining_budget += actual_received;
+        milestone.total_budget = milestone
+            .total_budget
+            .checked_add(actual_received)
+            .ok_or(Error::ArithmeticOverflow)?;
+        milestone.remaining_budget = milestone
+            .remaining_budget
+            .checked_add(actual_received)
+            .ok_or(Error::ArithmeticOverflow)?;
         env.storage().persistent().set(&mkey, &milestone);
         extend_ttl(&env, &mkey);
 
@@ -328,7 +337,10 @@ mod contract {
             return Err(Error::OverAllocation);
         }
 
-        milestone.remaining_budget -= amount;
+        milestone.remaining_budget = milestone
+            .remaining_budget
+            .checked_sub(amount)
+            .ok_or(Error::ArithmeticOverflow)?;
         milestone.allocations.set(issue_id, amount);
         env.storage().persistent().set(&mkey, &milestone);
         extend_ttl(&env, &mkey);
@@ -402,6 +414,7 @@ mod contract {
             .map_err(|e| match e {
                 mergefi_common::SplitError::InvalidSplit => Error::InvalidSplit,
                 mergefi_common::SplitError::SelfPayout => Error::SelfPayout,
+                mergefi_common::SplitError::Overflow => Error::ArithmeticOverflow,
             })?;
         let treasury: Address = mergefi_common::require_treasury::<DataKey>(&env).unwrap();
         let token_client = token::Client::new(&env, &milestone.token);
@@ -511,7 +524,10 @@ mod contract {
             .get(issue_id)
             .ok_or(Error::IssueNotAllocatedForDeallocate)?;
 
-        milestone.remaining_budget += amount;
+        milestone.remaining_budget = milestone
+            .remaining_budget
+            .checked_add(amount)
+            .ok_or(Error::ArithmeticOverflow)?;
         milestone.allocations.remove(issue_id);
 
         if milestone.closed && milestone.remaining_budget > 0 {
@@ -593,7 +609,7 @@ mod contract {
         }
 
         let now = env.ledger().timestamp();
-        if now < milestone.deadline + GRACE_PERIOD {
+        if now < milestone.deadline.saturating_add(GRACE_PERIOD) {
             return Err(Error::DeadlineNotPassed);
         }
 
@@ -857,21 +873,33 @@ fn refund_remaining_budget(
             .persistent()
             .get(&contribution_key)
             .ok_or(Error::MilestoneNotFound)?;
-        let numerator = remaining * contribution.amount;
+        let numerator = remaining
+            .checked_mul(contribution.amount)
+            .ok_or(Error::ArithmeticOverflow)?;
         let share = numerator / milestone.total_budget;
         let remainder = numerator % milestone.total_budget;
-        allocated += share;
+        allocated = allocated
+            .checked_add(share)
+            .ok_or(Error::ArithmeticOverflow)?;
         shares.push_back((contribution.sponsor.clone(), share));
         order.push_back((order.len(), remainder, contribution.sponsor));
     }
 
-    let dust = remaining - allocated;
+    let dust = remaining
+        .checked_sub(allocated)
+        .ok_or(Error::ArithmeticOverflow)?;
     if dust > 0 {
         mergefi_common::sort_remainders_desc(&mut order);
         for k in 0..dust as u32 {
             let (index, _, _) = order.get(k).unwrap();
             let (recipient, share) = shares.get(index).unwrap();
-            shares.set(index, (recipient, share + 1));
+            shares.set(
+                index,
+                (
+                    recipient,
+                    share.checked_add(1).ok_or(Error::ArithmeticOverflow)?,
+                ),
+            );
         }
     }