EventPublisher 2-topic vs 3-topic layout violation

apexchainx_calculator/src/config.rs

@@ -64,6 +64,24 @@ pub fn get_config(env: &Env, severity: Symbol) -> Result<SLAConfig, SLAError> {
 }
 
 /// Returns a deterministic backend-friendly snapshot of all canonical config values.
+///
+/// # Canonical Config Endpoint
+///
+/// This is the **canonical endpoint** for reading configuration data. It returns
+/// entries in a guaranteed canonical severity order (critical → high → medium → low)
+/// with typed `SLAConfigEntry` structs, making it suitable for:
+/// - Backend consumers that need stable ordering
+/// - Serialization and diffing logic
+/// - Config bundle generation
+///
+/// # When to Use list_configs Instead
+///
+/// Use `list_configs` only if you need:
+/// - Raw map access for low-level inspection
+/// - Direct iteration over the underlying storage map
+///
+/// Note that `list_configs` does not guarantee any ordering and returns raw
+/// `SLAConfig` values without the typed entry wrapper.
 pub fn get_config_snapshot(env: &Env) -> Result<SLAConfigSnapshot, SLAError> {
     crate::SLACalculatorContract::check_version(env)?;
 
@@ -81,6 +99,23 @@ pub fn get_config_snapshot(env: &Env) -> Result<SLAConfigSnapshot, SLAError> {
 }
 
 /// Returns the full map of severity-to-config entries.
+///
+/// # Raw/Low-Level Config Endpoint
+///
+/// This is a **raw endpoint** that returns the underlying storage map directly.
+/// It is provided for low-level inspection and debugging purposes.
+///
+/// **Important caveats:**
+/// - Does **not** guarantee any ordering (map-internal ordering is SDK-dependent)
+/// - Returns raw `SLAConfig` values without the typed entry wrapper
+/// - Not suitable for consumers that need stable ordering across SDK versions
+///
+/// # Canonical Endpoint
+///
+/// For most use cases, use `get_config_snapshot` instead, which:
+/// - Guarantees canonical severity order (critical → high → medium → low)
+/// - Returns typed `SLAConfigEntry` structs with severity labels
+/// - Is stable across SDK versions
 pub fn list_configs(env: &Env) -> Result<Map<Symbol, SLAConfig>, SLAError> {
     crate::SLACalculatorContract::check_version(env)?;
     env.storage()

apexchainx_calculator/src/event.rs

@@ -1,64 +1,15 @@
-//! Structured event payloads emitted by the contract.
-
-use soroban_sdk::{contracttype, Env, Symbol};
-
-/// Structured event payload for calculation execution events.
-///
-/// This struct is the canonical event shape emitted when a business-logic
-/// calculation completes. It is defined here rather than in `calculation.rs`
-/// so that the event schema and the computation logic can evolve independently.
-#[allow(missing_docs)]
-#[contracttype]
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct CalculationExecutedEventV1 {
-    /// Input key associated with the calculation (e.g. outage_id).
-    pub input_key: Symbol,
-    /// Input value for the calculation.
-    pub input_value: i128,
-    /// Computed result value.
-    pub result_value: i128,
-    /// Ledger timestamp at calculation time.
-    pub timestamp: u64,
-}
-
-/// A stateless event publisher that owns no state and only emits events.
-///
-/// # Boundary between business logic and side effects
-///
-/// `EventPublisher` is intentionally decoupled from computation functions
-/// like [`crate::calculation::compute_result`]. The business logic returns
-/// a pure result, and callers (e.g. `calculate_sla`) decide whether and
-/// when to publish events. This separation:
-///
-/// - Makes business logic testable without needing an event assertion harness.
-/// - Keeps event schemas versioned independently from computation rules.
-/// - Allows the same computation to be used in view-only (`calculate_sla_view`)
-///   and mutating (`calculate_sla`) paths without conditional event logic.
-///
-/// Events are always published through this struct to ensure consistent
-/// topic layout and payload formatting.
-pub struct EventPublisher;
-
-impl EventPublisher {
-    /// Publishes a calculation execution event with strict field ordering.
-    ///
-    /// Topic layout: `(topic, input_key)` for efficient filtering.
-    /// Payload: `CalculationExecutedEventV1` with all fields in canonical order.
-    pub fn publish_calculation_executed(
-        env: &Env,
-        topic: Symbol,
-        input_key: Symbol,
-        input_value: i128,
-        result_value: i128,
-        timestamp: u64,
-    ) {
-        let payload = CalculationExecutedEventV1 {
-            input_key: input_key.clone(),
-            input_value,
-            result_value,
-            timestamp,
-        };
-
-        env.events().publish((topic, input_key), payload);
-    }
-}
+//! Event publication module.
+//!
+//! This module is reserved for event publication helpers. All event schemas
+//! and topic layouts are defined in `event_schema.rs`, which is the canonical
+//! source of truth for event structure.
+//!
+//! # Topic Layout Convention
+//!
+//! All events follow the 3-topic layout defined in `event_schema.rs`:
+//! - topic[0] = event name (Symbol constant)
+//! - topic[1] = event version ("v1")
+//! - topic[2] = event-specific context (severity, caller address, etc.)
+//!
+//! Event publication should use direct `env.events().publish()` calls with
+//! the topic tuple `(EVENT_NAME, EVENT_VERSION, context)` to ensure consistency.

apexchainx_calculator/src/lib.rs

@@ -488,11 +488,26 @@ pub enum SLAError {
     /// | `outage_id` exists **and** the config version hash is **unchanged** **but** the inputs **differ** | **DuplicateOutageInput** error — the caller submitted contradictory data for the same outage under the same config |
     /// | `outage_id` exists **and** the config version hash **changed** | Treated as a **fresh calculation** — the config update invalidates the previous entry, so the new result is appended to history |
     ///
+    /// # Severity-Blind Detection
+    ///
+    /// The duplicate detection is **severity-blind**: it compares only `mttr_minutes`
+    /// and `threshold_minutes` (via the config hash), not the severity argument.
+    /// This means that if two severities have identical configuration parameters
+    /// (e.g., both high and medium configured with threshold 30 / penalty 50 / reward 750),
+    /// resubmitting the same outage under a different severity with the same MTTR is
+    /// treated as an idempotent replay, not a conflict.
+    ///
+    /// **Rationale:** The stored `SLAResult` does not carry a severity field, so
+    /// the contract cannot distinguish severity-only changes from true replays.
+    /// Adding severity to the result schema requires a breaking migration. Until
+    /// that migration is implemented, the contract treats severity as a routing
+    /// parameter rather than a data dimension for duplicate detection.
+    ///
     /// # Consumer guidance
     ///
     /// Backend callers that receive this error should:
-    /// 1. Check whether the submitted `mttr_minutes` or severity level was
-    ///    entered incorrectly (typo, stale measurement).
+    /// 1. Check whether the submitted `mttr_minutes` was entered incorrectly
+    ///    (typo, stale measurement).
     /// 2. If the previous calculation was incorrect, the admin must call
     ///    `prune_history` to remove the conflicting entry before
     ///    re-submitting with corrected values — or wait for a config
@@ -3405,12 +3420,39 @@ impl SLACalculatorContract {
     /// This function intentionally bypasses `check_version` (like
     /// `get_version_info` and `get_migration_state`) so it remains callable
     /// even when the contract is in a pre-migration or pre-init state.
+    ///
+    /// # Readiness Definition
+    ///
+    /// The healthcheck returns `ready: true` only when the contract is:
+    /// - Initialized (storage version matches expected version)
+    /// - Has an admin (not permanently renounced)
+    ///
+    /// This definition focuses on operational readiness for governance functions.
+    /// Pause/freeze states are not included in the readiness check to keep the
+    /// probe simple; operators should use `get_contract_state_fingerprint` for
+    /// full state visibility.
+    ///
+    /// # Status Vocabulary
+    ///
+    /// - `noinit`: Contract has never been initialized
+    /// - `migrate`: Storage version mismatch, migration required
+    /// - `noadmin`: Admin has been permanently renounced (governance-dead)
+    /// - `ok`: Contract is operational and has an admin
     pub fn healthcheck(env: Env) -> HealthcheckResult {
         let stored_version: Option<u32> = env.storage().instance().get(&STORAGE_VERSION_KEY);
+        let admin_renounced: Option<bool> = env.storage().instance().get(&ADMIN_RENOUNCED_KEY);
+
         let (ready, status) = match stored_version {
             None => (false, symbol_short!("noinit")),
             Some(v) if v != STORAGE_VERSION => (false, symbol_short!("migrate")),
-            Some(_) => (true, symbol_short!("ok")),
+            Some(_) => {
+                // Check if admin has been permanently renounced
+                if admin_renounced == Some(true) {
+                    (false, symbol_short!("noadmin"))
+                } else {
+                    (true, symbol_short!("ok"))
+                }
+            }
         };
         HealthcheckResult {
             ready,

apexchainx_calculator/src/tests.rs

@@ -6705,6 +6705,34 @@ fn test_duplicate_same_config_with_different_mttr_still_panics() {
     client.calculate_sla(&actors.operator, &outage_id, &symbol_short!("high"), &20);
 }
 
+#[test]
+fn test_duplicate_detection_is_severity_blind() {
+    // When two severities have identical configs, resubmitting the same outage
+    // under a different severity with the same MTTR is treated as an idempotent replay.
+    let (_env, client, actors) = setup();
+    let outage_id = symbol_short!("SEV_BLIND");
+
+    // Configure high and medium with identical parameters
+    client.set_config(&actors.admin, &symbol_short!("high"), &30, &50, &750);
+    client.set_config(&actors.admin, &symbol_short!("medium"), &30, &50, &750);
+
+    // Submit under high severity
+    let r1 = client.calculate_sla(&actors.operator, &outage_id, &symbol_short!("high"), &10);
+
+    // Resubmit under medium severity with same MTTR - should be idempotent replay
+    let r2 = client.calculate_sla(&actors.operator, &outage_id, &symbol_short!("medium"), &10);
+
+    // Both should return the same result (from the first submission)
+    assert_eq!(r1.config_version_hash, r2.config_version_hash);
+    assert_eq!(r1.mttr_minutes, r2.mttr_minutes);
+    assert_eq!(r1.threshold_minutes, r2.threshold_minutes);
+    assert_eq!(r1.amount, r2.amount);
+
+    // Only one entry in history (severity-blind detection)
+    assert_eq!(client.get_history().len(), 1);
+    assert_eq!(client.get_stats().total_calculations, 1);
+}
+
 #[test]
 fn test_255_prune_reduces_history_to_keep_latest() {
     // After prune_history(keep=3), exactly 3 entries remain (the most recent).
@@ -7699,6 +7727,23 @@ fn test_healthcheck_returns_not_ready_on_version_mismatch() {
     assert_eq!(hc.status, symbol_short!("migrate"));
 }
 
+#[test]
+fn test_healthcheck_returns_not_ready_after_renounce_admin() {
+    let (_env, client, actors) = setup();
+    // Contract should be ready initially
+    let hc_before = client.healthcheck();
+    assert!(hc_before.ready);
+    assert_eq!(hc_before.status, symbol_short!("ok"));
+
+    // Renounce admin
+    client.renounce_admin(&actors.admin);
+
+    // Healthcheck should now report not ready with noadmin status
+    let hc_after = client.healthcheck();
+    assert!(!hc_after.ready);
+    assert_eq!(hc_after.status, symbol_short!("noadmin"));
+}
+
 #[test]
 fn test_healthcheck_is_deterministic() {
     let (_env, client, _actors) = setup();

apexchainx_calculator/src/topic_stability_tests.rs

@@ -311,4 +311,43 @@ mod topic_stability_tests {
         }
         panic!("set_int event not found");
     }
+
+    // ── All events must have exactly 3 topics ─────────────────────────
+
+    #[test]
+    fn test_all_events_have_exactly_three_topics() {
+        let env = Env::default();
+        let (admin, operator, client) = setup(&env);
+        let new_admin = Address::generate(&env);
+        let new_op = Address::generate(&env);
+
+        // Trigger all event types to ensure comprehensive coverage
+        client.calculate_sla(
+            &operator,
+            &symbol_short!("ALL3"),
+            &symbol_short!("critical"),
+            &5,
+        );
+        client.set_config(&admin, &symbol_short!("critical"), &20, &200, &1000);
+        client.pause(&admin);
+        client.unpause(&admin);
+        client.propose_admin(&admin, &new_admin);
+        client.cancel_admin_proposal(&admin);
+        client.propose_operator(&admin, &new_op);
+        client.cancel_operator_proposal(&admin);
+        client.set_operator(&admin, &new_op);
+        client.freeze_config(&admin);
+        client.unfreeze_config(&admin);
+        client.set_retention_limit(&admin, &50);
+
+        let events = env.events().all();
+        for i in 0..events.len() {
+            let (_, topics, _) = events.get(i).unwrap();
+            assert_eq!(
+                topics.len(), 3,
+                "Every event must have exactly 3 topics (name, version, context), found {} topics",
+                topics.len()
+            );
+        }
+    }
 }