Millicentogalanya/revora contracts

src/lib.rs

@@ -2309,10 +2309,12 @@ impl RevoraRevenueShare {
             }
         }
 
+        // Use bounded read for event snapshots to avoid unbounded payloads
+        // Cap at MAX_PAGE_LIMIT (20) to prevent gas spikes from large blacklists
         let blacklist = if event_only {
             Vec::new(&env)
         } else {
-            Self::get_blacklist(env.clone(), issuer.clone(), namespace.clone(), token.clone())
+            Self::get_blacklist_page(env.clone(), issuer.clone(), namespace.clone(), token.clone(), 0, MAX_PAGE_LIMIT).0
         };
 
         let mut actual_override = false;
@@ -3223,6 +3225,16 @@ impl RevoraRevenueShare {
 
     /// Return all blacklisted addresses for an offering.
     /// Ordering: by insertion order, deterministic and stable across calls (#38).
+    ///
+    /// ## Legacy/Bounded Warning
+    ///
+    /// This method returns the entire blacklist in a single call, which can exceed gas limits
+    /// for large lists. It is retained for backward compatibility but should be avoided in
+    /// production code. Use `get_blacklist_page` instead for pagination with deterministic cursors.
+    ///
+    /// The blacklist size is bounded by MAX_BLACKLIST_SIZE (200) per offering, so this method
+    /// will never return more than 200 addresses. However, for off-chain tooling and event
+    /// processing, the paginated form is preferred to avoid gas spikes.
     pub fn get_blacklist(
         env: Env,
         issuer: Address,
@@ -3238,7 +3250,31 @@ impl RevoraRevenueShare {
     }
 
     /// Return a page of blacklisted addresses for an offering.
-    /// Limit capped at MAX_PAGE_LIMIT (20).
+    ///
+    /// ## Pagination Behavior
+    ///
+    /// - `start`: Zero-based cursor position in the insertion-ordered blacklist
+    /// - `limit`: Maximum number of addresses to return (capped at MAX_PAGE_LIMIT = 20)
+    /// - Returns: (page of addresses, next_cursor)
+    ///   - `next_cursor = Some(n)` indicates more data is available at position `n`
+    ///   - `next_cursor = None` indicates end of list
+    ///
+    /// The cursor is deterministic and stable: it corresponds to the index in the
+    /// insertion-ordered blacklist. Pagination preserves insertion order (#38).
+    ///
+    /// ## Usage Pattern
+    ///
+    /// ```ignore
+    /// let mut cursor = 0;
+    /// loop {
+    ///     let (page, next) = get_blacklist_page(env, issuer, ns, token, cursor, 20);
+    ///     // process page...
+    ///     match next {
+    ///         Some(n) => cursor = n,
+    ///         None => break,
+    ///     }
+    /// }
+    /// ```
     pub fn get_blacklist_page(
         env: Env,
         issuer: Address,
@@ -3960,6 +3996,17 @@ impl RevoraRevenueShare {
     /// Guarantees:
     /// - Overflow-resistant arithmetic without panic.
     /// - Result is clamped to [min(0, amount), max(0, amount)] to avoid over-distribution.
+    ///
+    /// ## Decomposition Bound
+    ///
+    /// The function decomposes `amount` as `amount = q * 10_000 + r` where:
+    /// - `q = amount / 10_000` (quotient)
+    /// - `r = amount % 10_000` (remainder, bounded to `|r| < 10_000`)
+    ///
+    /// This ensures:
+    /// - `|r * bps| < 10_000 * 10_000 = 10^8` (well within i128 range)
+    /// - The remainder product uses `checked_mul` with saturating fallback for defense-in-depth
+    /// - Even if the bound assumption is violated by refactors, saturation prevents overflow
     pub fn compute_share(
         _env: Env,
         amount: i128,
@@ -3976,6 +4023,7 @@ impl RevoraRevenueShare {
         // Decompose `amount` to avoid `amount * bps` overflow:
         // amount = q * 10_000 + r, so (amount * bps) / 10_000 = q * bps + (r * bps) / 10_000.
         // `r` is bounded to (-10_000, 10_000), so `r * bps` is always safe in i128.
+        // Defense-in-depth: use checked_mul with saturating fallback to guard against refactors.
         let q = amount / 10_000;
         let r = amount % 10_000;
         let bps = revenue_share_bps as i128;
@@ -3987,7 +4035,13 @@ impl RevoraRevenueShare {
             }
         });
 
-        let remainder_product = r * bps;
+        let remainder_product = r.checked_mul(bps).unwrap_or_else(|| {
+            if (r >= 0 && bps >= 0) || (r < 0 && bps < 0) {
+                i128::MAX
+            } else {
+                i128::MIN
+            }
+        });
         let remainder_share = match mode {
             RoundingMode::Truncation => remainder_product / 10_000,
             RoundingMode::RoundHalfUp => {

src/test_compute_share_invariants.rs

@@ -16,6 +16,7 @@
 //! - `r * bps` fits in i128 because `|r| < 10_000` and `bps ≤ 10_000`,
 //!   so `|r * bps| < 10_000 * 10_000 = 10^8` — well within i128 range.
 //! - `q * bps` uses `checked_mul` with a saturating fallback, so it never wraps.
+//! - `r * bps` now also uses `checked_mul` with saturating fallback for defense-in-depth.
 //! - The final `checked_add` also saturates rather than wrapping.
 //! - A final clamp to `[min(0, amount), max(0, amount)]` enforces the bounds
 //!   invariant even if saturation produced an out-of-range intermediate.
@@ -522,11 +523,86 @@ fn compute_share_extreme_negative_roundhalfup_midpoint() {
     let amount = i128::MIN + 10001;
     let result = c.compute_share(&amount, &5000, &RoundingMode::RoundHalfUp);
     assert_bounds(result, amount, "Extreme negative with bps=5000");
-    
+
     // Verify RoundHalfUp vs Truncation behavior
     let trunc = c.compute_share(&amount, &5000, &RoundingMode::Truncation);
     let round = c.compute_share(&amount, &5000, &RoundingMode::RoundHalfUp);
     // For negative: RoundHalfUp should be <= Truncation (more negative when rounding)
     assert!(round <= trunc);
 }
 
+// ═══════════════════════════════════════════════════════════════════════════════
+// INVARIANT: Remainder product bound and checked_mul defense-in-depth
+// ═══════════════════════════════════════════════════════════════════════════════
+
+#[test]
+fn remainder_product_bound_holds_for_all_bps() {
+    // Explicit invariant test: |r| < 10_000 and bps <= 10_000 ensures |r * bps| < 10^8
+    // This test verifies the decomposition bound assumption used in compute_share
+    let (_env, c) = client();
+
+    // Test with amounts that produce various remainders
+    let test_amounts = [
+        1_i128,
+        9_999,
+        10_000,
+        10_001,
+        19_999,
+        20_000,
+        100_000,
+        1_000_000,
+        i128::MAX / 10_000 * 10_000 + 9_999, // Max remainder
+        i128::MIN / 10_000 * 10_000 - 9_999,   // Min remainder
+    ];
+
+    let bps_values = [1_u32, 100, 1_000, 5_000, 9_999, 10_000];
+
+    for &amount in &test_amounts {
+        for &bps in &bps_values {
+            let result_trunc = c.compute_share(&amount, &bps, &RoundingMode::Truncation);
+            let result_round = c.compute_share(&amount, &bps, &RoundingMode::RoundHalfUp);
+
+            // Verify bounds invariant
+            assert_bounds(result_trunc, amount, &format!("Truncation amount={amount} bps={bps}"));
+            assert_bounds(result_round, amount, &format!("RoundHalfUp amount={amount} bps={bps}"));
+
+            // Verify that the result is consistent with the decomposition formula
+            // amount = q * 10_000 + r, share = q * bps + (r * bps) / 10_000
+            let q = amount / 10_000;
+            let r = amount % 10_000;
+            let bps_i128 = bps as i128;
+
+            // The remainder product should be safe
+            let remainder_product = r * bps_i128;
+            assert!(
+                remainder_product.abs() < 10_000 * 10_000,
+                "Remainder product {remainder_product} exceeds bound for r={r}, bps={bps}"
+            );
+        }
+    }
+}
+
+#[test]
+fn checked_mul_defense_in_depth_prevents_overflow() {
+    // Verify that even if the bound assumption were violated, checked_mul prevents overflow
+    // This is a defense-in-depth test to ensure the saturating fallback works correctly
+    let (_env, c) = client();
+
+    // Test with extreme values that would be problematic without checked_mul
+    // The decomposition ensures |r| < 10_000, but we test the saturating fallback path
+    let extreme_amounts = [
+        i128::MAX,
+        i128::MIN,
+        i128::MAX - 1,
+        i128::MIN + 1,
+    ];
+
+    for &amount in &extreme_amounts {
+        for &bps in [1_u32, 5_000, 10_000] {
+            let result = c.compute_share(&amount, &bps, &RoundingMode::Truncation);
+            // Should never panic and should always satisfy bounds
+            assert_bounds(result, amount, &format!("Extreme amount={amount} bps={bps}"));
+        }
+    }
+}
+