Fix input validation, event payload consistency, and pagination semantics

apexchainx_calculator/src/calculation.rs

@@ -212,6 +212,12 @@ fn require_not_paused(env: &Env) -> Result<(), SLAError> {
     Ok(())
 }
 
+/// Maximum allowed MTTR in minutes to prevent arithmetic overflow.
+/// This conservative bound ensures that even with maximum penalty rates,
+/// the calculation cannot overflow i128. 
+/// 525,600 minutes = 365 days, well beyond any realistic outage duration.
+const MAX_MTTR_MINUTES: u32 = 525_600;
+
 /// Computes the SLA result (met/violated, reward/penalty, rating) from inputs.
 /// Pure function — no state reads or writes.
 pub fn compute_result(
@@ -222,6 +228,11 @@ pub fn compute_result(
     recorded_at: u64,
 ) -> Result<SLAResult, SLAError> {
     let threshold = cfg.threshold_minutes;
+
+    // Validate input range before computation to provide clear error messages
+    if mttr_minutes > MAX_MTTR_MINUTES {
+        return Err(SLAError::InvalidInput);
+    }
 
     if mttr_minutes > threshold {
         let overtime = (mttr_minutes - threshold) as i128;
@@ -428,6 +439,8 @@ fn publish_sla_event(env: &Env, severity: Symbol, result: &SLAResult) {
             result.mttr_minutes,
             result.threshold_minutes,
             result.amount,
+            result.config_version_hash,
+            result.recorded_at,
         ),
     );
 }

apexchainx_calculator/src/event_schema.rs

@@ -18,7 +18,7 @@
 //! - topic[2]: severity Symbol
 //! - payload:  (outage_id: Symbol, status: Symbol, payment_type: Symbol,
 //!   rating: Symbol, mttr_minutes: u32, threshold_minutes: u32,
-//!   amount: i128)
+//!   amount: i128, config_version_hash: u64, recorded_at: u64)
 //!
 //! ## set_int (`set_int`)
 //! Settlement intent emitted alongside sla_calc for backend reconciliation.

apexchainx_calculator/src/history.rs

@@ -159,7 +159,9 @@ pub fn get_history_page(env: &Env, offset: u32, limit: u32) -> Result<Vec<SLARes
 /// This is a metadata-carrying companion to [`get_history_page`]. The `items`
 /// slice is identical to what `get_history_page` returns for the same
 /// `(offset, limit)`; `total` is the full history length and `has_more` is
-/// `true` when the requested range ends before the end of history.
+/// `true` when the requested range ends before the end of history **and**
+/// `limit > 0`. When `limit == 0`, `has_more` is `false` (empty page signals
+/// end-of-history).
 ///
 /// Pagination semantics (offset-based, oldest-first, saturating
 /// `offset + limit`, empty page when `offset >= len` or `limit == 0`) are
@@ -178,17 +180,20 @@ pub fn get_history_page_with_meta(env: &Env, offset: u32, limit: u32) -> Result<
     // Saturating arithmetic mirrors `get_history_page`: clamp the end index to
     // the real history length so extreme `u32` inputs can never wrap into a
     // wrong slice. `end` also drives `has_more`: entries remain whenever the
-    // requested range stops short of the end of history.
+    // requested range stops short of the end of history and limit > 0.
     let end = offset.saturating_add(limit).min(total);
     if offset < total && limit != 0 {
         for i in offset..end {
             items.push_back(history.get(i).unwrap());
         }
     }
+    // When limit == 0, the page is empty by request, which signals end-of-history
+    // per the pagination policy. This ensures consistency with get_history_page.
+    let has_more = if limit == 0 { false } else { end < total };
     Ok(HistoryPage {
         items,
         total,
-        has_more: end < total,
+        has_more,
     })
 }
 

apexchainx_calculator/src/lib.rs

@@ -228,7 +228,7 @@ pub use crate::config_metadata::LAST_CFG_UPDATE_KEY;
 //
 // sla_calc  → (outage_id: Symbol, status: Symbol, payment_type: Symbol,
 //              rating: Symbol, mttr_minutes: u32, threshold_minutes: u32,
-//              amount: i128)
+//              amount: i128, config_version_hash: u64, recorded_at: u64)
 //   context: severity Symbol
 //
 // cfg_upd   → (threshold_minutes: u32, penalty_per_minute: i128,
@@ -512,7 +512,7 @@ pub enum SLAError {
     InvalidRewardAmount = 15,
     /// Configuration is frozen — config changes are blocked.
     ConfigFrozen = 16,
-    /// Input parameter violates documented constraints (e.g., reason too long). (#68)
+    /// Input parameter violates documented constraints (e.g., reason too long, mttr_minutes exceeds maximum). (#68)
     InvalidInput = 17,
     /// Custom severity referenced but not registered. (#93)
     SeverityNotInSet = 18,
@@ -584,8 +584,9 @@ pub struct SLAResult {
 ///
 /// The `items` slice is identical to what `get_history_page` returns for the
 /// same `(offset, limit)`; `total` is the full history length and `has_more`
-/// is `true` when the requested range ends before the end of history (i.e.
-/// more entries can be fetched by advancing `offset`).
+/// is `true` when the requested range ends before the end of history **and**
+/// `limit > 0`. When `limit == 0`, `has_more` is `false` (empty page signals
+/// end-of-history).
 #[allow(missing_docs)]
 #[contracttype]
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -595,7 +596,8 @@ pub struct HistoryPage {
     /// Total number of history entries currently stored.
     pub total: u32,
     /// Whether the requested range ends before the end of history (more
-    /// entries can be fetched by advancing `offset`).
+    /// entries can be fetched by advancing `offset`). When `limit == 0`,
+    /// this is `false` (empty page signals end-of-history).
     pub has_more: bool,
 }
 
@@ -2213,6 +2215,10 @@ impl SLACalculatorContract {
 
     /// Recalculates SLA deterministically without mutating any state or emitting events.
     /// Can be called by anyone for verification and audit purposes.
+    ///
+    /// # Input constraints
+    ///
+    /// - `mttr_minutes` must be ≤ 525,600 (365 days). Values exceeding this bound are rejected with `InvalidInput`.
     pub fn calculate_sla_view(
         env: Env,
         outage_id: Symbol,
@@ -2303,10 +2309,15 @@ impl SLACalculatorContract {
     /// | `Unauthorized` | Caller is not the operator |
     /// | `ConfigNotFound` | No configuration exists for the requested severity |
     /// | `DuplicateOutageInput` | Same `outage_id` submitted with conflicting inputs; emits a `dup_input` event carrying the stored result |
+    /// | `InvalidInput` | Input parameter violates documented constraints (e.g., mttr_minutes exceeds maximum allowed) |
     /// | `InvalidPenaltyAmount` | Penalty computation overflowed or produced a non-negative value |
     /// | `InvalidRewardAmount` | Reward computation overflowed or produced a non-positive value |
     /// Records an SLA decision for `outage_id`. Operator only.
     ///
+    /// # Input constraints
+    ///
+    /// - `mttr_minutes` must be ≤ 525,600 (365 days). Values exceeding this bound are rejected with `InvalidInput`.
+    ///
     /// # Repeated submissions for the same outage_id
     ///
     /// Anti-spam policy, applied in this order:
@@ -3163,7 +3174,8 @@ impl SLACalculatorContract {
     /// `items` slice is identical to what `get_history_page` returns for the
     /// same `(offset, limit)`; `total` is the full history length and
     /// `has_more` is `true` when the requested range ends before the end of
-    /// history.
+    /// history **and** `limit > 0`. When `limit == 0`, `has_more` is `false` to
+    /// signal end-of-history (empty page).
     ///
     /// Pagination semantics (offset-based, oldest-first, saturating
     /// `offset + limit`, empty page when `offset >= len` or `limit == 0`) are
@@ -3182,17 +3194,20 @@ impl SLACalculatorContract {
         // Saturating arithmetic mirrors `get_history_page`: clamp the end index
         // to the real history length so extreme `u32` inputs can never wrap into
         // a wrong slice. `end` also drives `has_more`: entries remain whenever
-        // the requested range stops short of the end of history.
+        // the requested range stops short of the end of history and limit > 0.
         let end = offset.saturating_add(limit).min(total);
         if offset < total && limit != 0 {
             for i in offset..end {
                 items.push_back(history.get(i).unwrap());
             }
         }
+        // When limit == 0, the page is empty by request, which signals end-of-history
+        // per the pagination policy. This ensures consistency with get_history_page.
+        let has_more = if limit == 0 { false } else { end < total };
         Ok(HistoryPage {
             items,
             total,
-            has_more: end < total,
+            has_more,
         })
     }
 

apexchainx_calculator/src/tests.rs

@@ -3136,12 +3136,30 @@ fn test_get_history_page_with_meta_zero_limit() {
         client.calculate_sla(&actors.operator, &oid, &symbol_short!("low"), &10);
     }
 
-    // `limit == 0` returns an empty page with the correct total. The cursor
-    // has not advanced past `offset`, so history still remains at offset 0.
+    // `limit == 0` returns an empty page with the correct total. Per the
+    // pagination policy, an empty page signals end-of-history, so has_more is false.
     let page = client.get_history_page_with_meta(&0, &0);
     assert_eq!(page.total, 3);
     assert_eq!(page.items.len(), 0);
-    assert!(page.has_more);
+    assert!(!page.has_more);
+
+    // Test limit == 0 at mid offset (offset = 1, total = 3)
+    let page_mid = client.get_history_page_with_meta(&1, &0);
+    assert_eq!(page_mid.total, 3);
+    assert_eq!(page_mid.items.len(), 0);
+    assert!(!page_mid.has_more);
+
+    // Test limit == 0 at end offset (offset = 3, total = 3)
+    let page_end = client.get_history_page_with_meta(&3, &0);
+    assert_eq!(page_end.total, 3);
+    assert_eq!(page_end.items.len(), 0);
+    assert!(!page_end.has_more);
+
+    // Test limit == 0 beyond end (offset = 100, total = 3)
+    let page_beyond = client.get_history_page_with_meta(&100, &0);
+    assert_eq!(page_beyond.total, 3);
+    assert_eq!(page_beyond.items.len(), 0);
+    assert!(!page_beyond.has_more);
 }
 
 #[test]

apexchainx_calculator/src/version_negotiation.rs

@@ -189,8 +189,8 @@ pub fn negotiate_contract_versions(
 /// downstream contracts should expose for version discovery.
 pub fn version_discovery_interfaces(env: &Env) -> Vec<Symbol> {
     let mut ifaces = Vec::new(env);
-    ifaces.push_back(symbol_short!("ver_info"));
-    ifaces.push_back(symbol_short!("mig_state"));
+    ifaces.push_back(Symbol::new(env, "get_version_info"));
+    ifaces.push_back(Symbol::new(env, "get_migration_state"));
     ifaces.push_back(symbol_short!("is_paused"));
     ifaces
 }
@@ -314,11 +314,33 @@ mod tests {
         let env = Env::default();
         let ifaces = version_discovery_interfaces(&env);
         assert_eq!(ifaces.len(), 3);
-        assert!(ifaces.contains(&symbol_short!("ver_info")));
-        assert!(ifaces.contains(&symbol_short!("mig_state")));
+        assert!(ifaces.contains(&Symbol::new(&env, "get_version_info")));
+        assert!(ifaces.contains(&Symbol::new(&env, "get_migration_state")));
         assert!(ifaces.contains(&symbol_short!("is_paused")));
     }
 
+    #[test]
+    fn test_version_discovery_interfaces_match_actual_methods() {
+        let env = Env::default();
+        let ifaces = version_discovery_interfaces(&env);
+
+        // Verify each discovery symbol corresponds to an actual contract method
+        // These are the exact method names exposed in the contract's public API
+        let expected_methods = [
+            Symbol::new(&env, "get_version_info"),
+            Symbol::new(&env, "get_migration_state"),
+            symbol_short!("is_paused"),
+        ];
+
+        for expected_method in expected_methods.iter() {
+            assert!(
+                ifaces.contains(expected_method),
+                "Discovery list should contain actual method: {:?}",
+                expected_method
+            );
+        }
+    }
+
     #[test]
     fn test_negotiation_info_storage_version() {
         let info = build_negotiation_info(1, 1, false);

docs/HISTORY_PAGINATION_POLICY.md

@@ -92,7 +92,7 @@ wrapped in a `HistoryPage` struct:
 pub struct HistoryPage {
     pub items: Vec<SLAResult>, // identical to get_history_page(offset, limit)
     pub total: u32,            // full history length at read time
-    pub has_more: bool,        // end = min(saturating_add(offset, limit), total) < total
+    pub has_more: bool,        // (limit > 0) && (end = min(saturating_add(offset, limit), total) < total)
 }
 ```
 
@@ -102,12 +102,14 @@ pub struct HistoryPage {
 - `total` is the full history length, so consumers no longer need a separate
   `get_history` or `get_retention_limit` call to learn the total size.
 - `has_more` is `true` exactly when the requested range ends before the end
-  of history (`end < total`). A consumer can therefore iterate with
-  `offset += items.len()` and stop when `has_more` is `false`.
+  of history (`end < total`) **and** `limit > 0`. When `limit == 0`, `has_more`
+  is `false` because the empty page signals end-of-history per the policy.
+  A consumer can therefore iterate with `offset += items.len()` and stop when
+  `has_more` is `false` or when an empty page is returned.
 - The same empty-page edge cases apply to `items`: `offset >= total` and
-  `limit == 0` both produce an empty `items`. For `limit == 0` with
-  `offset < total`, `has_more` is still `true` because the cursor has not
-  advanced past `offset`.
+  `limit == 0` both produce an empty `items`. For `limit == 0`, `has_more` is
+  `false` to maintain consistency with the "empty page as end-of-history signal"
+  policy.
 
 `get_history_page_with_meta` is read-only, performs no storage writes, emits
 no events, and never mutates history.