diff --git a/apexchainx_calculator/src/calculation.rs b/apexchainx_calculator/src/calculation.rs index 22f45e2..986c52c 100644 --- a/apexchainx_calculator/src/calculation.rs +++ b/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 { 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, ), ); } diff --git a/apexchainx_calculator/src/event_schema.rs b/apexchainx_calculator/src/event_schema.rs index 6378d9b..6fbb878 100644 --- a/apexchainx_calculator/src/event_schema.rs +++ b/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. diff --git a/apexchainx_calculator/src/history.rs b/apexchainx_calculator/src/history.rs index 7457faa..9c90e6f 100644 --- a/apexchainx_calculator/src/history.rs +++ b/apexchainx_calculator/src/history.rs @@ -159,7 +159,9 @@ pub fn get_history_page(env: &Env, offset: u32, limit: u32) -> Result 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, }) } diff --git a/apexchainx_calculator/src/lib.rs b/apexchainx_calculator/src/lib.rs index e6c885d..38bc6cc 100644 --- a/apexchainx_calculator/src/lib.rs +++ b/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, }) } diff --git a/apexchainx_calculator/src/tests.rs b/apexchainx_calculator/src/tests.rs index 32b76b9..4fb831b 100644 --- a/apexchainx_calculator/src/tests.rs +++ b/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] diff --git a/apexchainx_calculator/src/version_negotiation.rs b/apexchainx_calculator/src/version_negotiation.rs index 6764743..c7a9351 100644 --- a/apexchainx_calculator/src/version_negotiation.rs +++ b/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 { 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); diff --git a/docs/HISTORY_PAGINATION_POLICY.md b/docs/HISTORY_PAGINATION_POLICY.md index cfd59b8..af81165 100644 --- a/docs/HISTORY_PAGINATION_POLICY.md +++ b/docs/HISTORY_PAGINATION_POLICY.md @@ -92,7 +92,7 @@ wrapped in a `HistoryPage` struct: pub struct HistoryPage { pub items: Vec, // 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.