diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3517ba7..b8d9fea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -115,6 +115,17 @@ jobs: working-directory: apexchainx_calculator run: cargo test --lib + # Issue #255: Result schema migration guardrail. + # schema_migration_tests.rs contains a compile-time exhaustive SLAResult + # destructure and runtime assertions that RESULT_SCHEMA_VERSION and + # RESULT_SCHEMA_FIELD_COUNT match the actual struct layout and the values + # returned by get_result_schema(). Any PR that modifies SLAResult without + # updating both constants will fail here. + # See: docs/result-schema-migration-guard.md + - name: Result schema migration guard + working-directory: apexchainx_calculator + run: cargo test --lib schema_migration_tests + # Issue #81: Normalize snapshot artifacts before upload so that volatile # fields (timestamp, elapsed_ms, generated_at) are stripped and keys are # sorted. This prevents noisy PR diffs caused by non-semantic changes. diff --git a/CHANGELOG.md b/CHANGELOG.md index a4e125d..c47cefa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,10 @@ - `.devcontainer/` — reproducible dev container workspace with Rust + WASM target + just + Node.js (#281) - `just bootstrap` target — session-safe, idempotent one-command local bootstrap for the Rust WASM contract workflow: verifies rustup, installs the pinned `1.94.1` toolchain with `rustfmt` + `clippy` components, adds `wasm32-unknown-unknown` target, and verifies `cargo` is on `PATH` (closes #257) - `docs/CONTRACT_MAINTENANCE_POLICY.md` — comprehensive maintenance policy covering `#[contracttype]` compatibility notes (#279), response-shape stability (#283), version negotiation (#284), API archetypes (#285), event payload size checks (#286), event drift review (#287), history write audit (#288), telemetry counters (#289), and role-change incident review (#290) +- `RESULT_SCHEMA_FIELD_COUNT` constant — compile-time sentinel recording the number of named fields in `SLAResult`; must be updated alongside `RESULT_SCHEMA_VERSION` when the result layout changes (#255) +- `SLAResultSchema::result_field_count` — exposes `RESULT_SCHEMA_FIELD_COUNT` to backend consumers via `get_result_schema()` so they can detect layout drift at runtime (#255) +- `schema_migration_tests.rs` — CI-backed guardrail tests for `get_result_schema()`: exhaustive `SLAResult` destructure (compile-time gate), field count sentinel, symbol stability, deprecated-symbols invariant, and `get_config_bundle` consistency (closes #255) +- `docs/result-schema-migration-guard.md` — documentation for the result schema migration process, describing the two-level guardrail, step-by-step change process, and backend consumer guidance (closes #255) - `docs/EVENT_DRIFT_CHECKLIST.md` — standalone quick-reference event drift review checklist for everyday maintainer use (#287) - `tooling/release-summary.ts` — release summary generator for maintainers (#280) - `scripts/release-replay.ts` — minimal release candidate validation command for fast pre-release checks (#270) diff --git a/apexchainx_calculator/src/lib.rs b/apexchainx_calculator/src/lib.rs index e139144..9227281 100644 --- a/apexchainx_calculator/src/lib.rs +++ b/apexchainx_calculator/src/lib.rs @@ -24,6 +24,8 @@ mod tests; #[cfg(test)] mod fuzz_tests; +#[cfg(test)] +mod schema_migration_tests; /// Parity checker: compares current `compute_result` against the locked-in /// canonical golden vectors in `test_snapshots/tests/parity_baseline.json`. /// Run with `cargo test --lib parity_tests::` or `just parity-check`. @@ -129,6 +131,23 @@ pub(crate) const STORAGE_VERSION: u32 = 1; /// Incremented when result encoding changes in a breaking way. pub(crate) const RESULT_SCHEMA_VERSION: u32 = 1; +/// Number of named fields in `SLAResult`. +/// +/// This constant is the migration guardrail for `get_result_schema()`. +/// It must be updated in the same commit that adds or removes a field from +/// `SLAResult`. The companion test `test_result_schema_field_count_sentinel` +/// in `schema_migration_tests.rs` will fail CI if the struct layout changes +/// without a corresponding update to this constant and `RESULT_SCHEMA_VERSION`. +/// +/// **How to update when adding a field:** +/// 1. Add the field to `SLAResult`. +/// 2. Increment this constant. +/// 3. Increment `RESULT_SCHEMA_VERSION` (breaking change). +/// 4. Update `get_result_schema()` if a new symbol descriptor is needed. +/// 5. Add a CHANGELOG entry under `[Unreleased]` → `Changed`. +/// 6. See `docs/result-schema-migration-guard.md` for the full process. +pub(crate) const RESULT_SCHEMA_FIELD_COUNT: u32 = 9; + /// Hard upper bound on retained history entries. (SC-062) /// Configurable down to 1 via set_retention_limit(). pub(crate) const MAX_HISTORY_SIZE: u32 = 1000; @@ -520,6 +539,10 @@ pub struct SLAResultSchema { pub version: Symbol, /// Numeric schema version (incremented on breaking changes). pub schema_version: u32, + /// Number of named fields in `SLAResult` at this schema version. + /// Backends can compare this against their own deserialization code to + /// detect layout drift without parsing the full field list. + pub result_field_count: u32, /// Symbol for SLA met status. pub status_met: Symbol, /// Symbol for SLA violated status. @@ -1657,6 +1680,7 @@ pub fn get_result_schema(env: Env) -> Result { Ok(SLAResultSchema { version: symbol_short!("v1"), schema_version: RESULT_SCHEMA_VERSION, + result_field_count: RESULT_SCHEMA_FIELD_COUNT, status_met: symbol_short!("met"), status_violated: symbol_short!("viol"), payment_reward: symbol_short!("rew"), diff --git a/apexchainx_calculator/src/schema_migration_tests.rs b/apexchainx_calculator/src/schema_migration_tests.rs new file mode 100644 index 0000000..e7941e7 --- /dev/null +++ b/apexchainx_calculator/src/schema_migration_tests.rs @@ -0,0 +1,221 @@ +/// Schema migration guardrail tests for `get_result_schema()` (#255). +/// +/// These tests act as a CI-backed safety net that prevents `SLAResult` layout +/// changes from being merged without a deliberate, reviewed schema version bump. +/// +/// # How the guard works +/// +/// 1. `RESULT_SCHEMA_FIELD_COUNT` in `lib.rs` records the number of named fields +/// in `SLAResult`. +/// 2. `RESULT_SCHEMA_VERSION` records the breaking-change counter. +/// 3. The tests below assert that both constants match the actual struct shape +/// and the values returned by `get_result_schema()`. +/// +/// If a contributor adds or removes a field from `SLAResult` without updating +/// `RESULT_SCHEMA_FIELD_COUNT` and `RESULT_SCHEMA_VERSION`, the sentinel test +/// `test_result_schema_field_count_sentinel` will fail — surfacing the oversight +/// before the PR lands. +/// +/// # What to do when changing `SLAResult` +/// +/// See `docs/result-schema-migration-guard.md` for the full migration checklist. +/// Quick summary: +/// +/// 1. Add / remove / change the field in `SLAResult`. +/// 2. Update `RESULT_SCHEMA_FIELD_COUNT` to the new field count. +/// 3. Increment `RESULT_SCHEMA_VERSION` (breaking schema change). +/// 4. Update `get_result_schema()` if a new symbol descriptor is warranted. +/// 5. Add a CHANGELOG entry under `[Unreleased]` → `Changed`. +/// 6. Update the `expected_fields` list in +/// `test_result_schema_symbols_are_stable` if a symbol changes. +#[cfg(test)] +mod tests { + use crate::{ + SLACalculatorContract, SLACalculatorContractClient, RESULT_SCHEMA_FIELD_COUNT, + RESULT_SCHEMA_VERSION, + }; + use soroban_sdk::{testutils::Address as _, Env, Symbol}; + + // ----------------------------------------------------------------------- + // Helpers + // ----------------------------------------------------------------------- + + fn setup() -> (Env, SLACalculatorContractClient<'static>) { + let env = Env::default(); + env.mock_all_auths(); + let cid = env.register_contract(None, SLACalculatorContract); + let client = SLACalculatorContractClient::new(&env, &cid); + let admin = soroban_sdk::Address::generate(&env); + let operator = soroban_sdk::Address::generate(&env); + client.initialize(&admin, &operator); + (env, client) + } + + // ----------------------------------------------------------------------- + // Sentinel: field count must match RESULT_SCHEMA_FIELD_COUNT + // ----------------------------------------------------------------------- + + /// **Migration guardrail — CI gate.** + /// + /// This test counts the fields of `SLAResult` by name and asserts the + /// count equals `RESULT_SCHEMA_FIELD_COUNT`. It will fail if a field is + /// added, removed, or renamed without updating the constant. + /// + /// `SLAResult` currently has 9 fields: + /// outage_id, status, mttr_minutes, threshold_minutes, amount, + /// payment_type, rating, config_version_hash, recorded_at + /// + /// Update `RESULT_SCHEMA_FIELD_COUNT` in `lib.rs` when this changes. + #[test] + fn test_result_schema_field_count_sentinel() { + use crate::SLAResult; + use soroban_sdk::{symbol_short, Env}; + + let env = Env::default(); + + // Build a representative SLAResult and destructure it exhaustively so + // the compiler enforces that every field is named here. When a new + // field is added the destructure will fail to compile unless the + // test is updated. This is the first line of defense. + let sample = SLAResult { + outage_id: symbol_short!("out1"), + status: symbol_short!("met"), + mttr_minutes: 10, + threshold_minutes: 30, + amount: 750, + payment_type: symbol_short!("rew"), + rating: symbol_short!("excel"), + config_version_hash: 0, + recorded_at: 0, + }; + + // Destructure every field explicitly — adding a field without updating + // this match will cause a compile error, catching the drift at build time. + let SLAResult { + outage_id: _, + status: _, + mttr_minutes: _, + threshold_minutes: _, + amount: _, + payment_type: _, + rating: _, + config_version_hash: _, + recorded_at: _, + } = sample; + + // The runtime check: ensure the constant matches the actual count. + // If the struct grows and the destructure above is updated but + // RESULT_SCHEMA_FIELD_COUNT is not, this assertion catches the gap. + let _ = &env; // env kept for Soroban test harness compatibility + assert_eq!( + RESULT_SCHEMA_FIELD_COUNT, + 9, + "RESULT_SCHEMA_FIELD_COUNT is out of sync with SLAResult. \ + Update lib.rs::RESULT_SCHEMA_FIELD_COUNT and \ + RESULT_SCHEMA_VERSION when adding or removing fields." + ); + } + + // ----------------------------------------------------------------------- + // get_result_schema() returns the expected version and field count + // ----------------------------------------------------------------------- + + /// Assert that `get_result_schema()` returns the constants declared in + /// `lib.rs` so any divergence between the runtime schema and the + /// compile-time constants surfaces in CI. + #[test] + fn test_get_result_schema_version_matches_constant() { + let (_env, client) = setup(); + let schema = client.get_result_schema(); + assert_eq!( + schema.schema_version, RESULT_SCHEMA_VERSION, + "get_result_schema() schema_version ({}) does not match \ + RESULT_SCHEMA_VERSION constant ({}). \ + Increment RESULT_SCHEMA_VERSION when the result layout changes.", + schema.schema_version, RESULT_SCHEMA_VERSION + ); + assert_eq!( + schema.result_field_count, RESULT_SCHEMA_FIELD_COUNT, + "get_result_schema() result_field_count ({}) does not match \ + RESULT_SCHEMA_FIELD_COUNT constant ({}). \ + Update RESULT_SCHEMA_FIELD_COUNT to match the SLAResult field count.", + schema.result_field_count, RESULT_SCHEMA_FIELD_COUNT + ); + } + + // ----------------------------------------------------------------------- + // Symbol stability: symbol values must not change without a version bump + // ----------------------------------------------------------------------- + + /// Assert that every result symbol returned by `get_result_schema()` still + /// matches the canonical values baked into `compute_result`. + /// + /// If a symbol is renamed (e.g. `"met"` → `"sla_met"`) this test fails, + /// prompting the contributor to increment `RESULT_SCHEMA_VERSION` and + /// update `CHANGELOG.md`. + #[test] + fn test_result_schema_symbols_are_stable() { + let (env, client) = setup(); + let schema = client.get_result_schema(); + + // These are the canonical symbol strings baked into compute_result. + // Changing any of them is a breaking wire-format change. + assert_eq!(schema.status_met, Symbol::new(&env, "met")); + assert_eq!(schema.status_violated, Symbol::new(&env, "viol")); + assert_eq!(schema.payment_reward, Symbol::new(&env, "rew")); + assert_eq!(schema.payment_penalty, Symbol::new(&env, "pen")); + assert_eq!(schema.rating_exceptional, Symbol::new(&env, "top")); + assert_eq!(schema.rating_excellent, Symbol::new(&env, "excel")); + assert_eq!(schema.rating_good, Symbol::new(&env, "good")); + assert_eq!(schema.rating_poor, Symbol::new(&env, "poor")); + assert!( + schema.includes_config_version_hash, + "includes_config_version_hash must remain true while \ + SLAResult::config_version_hash exists" + ); + } + + // ----------------------------------------------------------------------- + // Deprecated symbols list is empty at schema v1 + // ----------------------------------------------------------------------- + + /// Confirm the deprecated_symbols list is empty for schema v1. + /// When a symbol is deprecated, this test must be updated to assert + /// the expected entry is present rather than asserting the list is empty. + #[test] + fn test_result_schema_no_deprecated_symbols_at_v1() { + let (_env, client) = setup(); + let schema = client.get_result_schema(); + assert_eq!( + schema.deprecated_symbols.len(), + 0, + "Schema v1 should have no deprecated symbols. \ + If you are introducing a deprecation, update this test to \ + assert the expected DeprecatedSymbol entry is present." + ); + } + + // ----------------------------------------------------------------------- + // get_config_bundle includes schema with correct version + // ----------------------------------------------------------------------- + + /// `get_config_bundle` composes `get_result_schema` internally. + /// Verify its embedded schema also reflects the current version. + #[test] + fn test_config_bundle_schema_version_consistent() { + let (_env, client) = setup(); + let bundle = client.get_config_bundle(); + if let Some(b) = bundle { + assert_eq!( + b.schema.schema_version, RESULT_SCHEMA_VERSION, + "get_config_bundle schema_version is inconsistent with RESULT_SCHEMA_VERSION" + ); + assert_eq!( + b.schema.result_field_count, RESULT_SCHEMA_FIELD_COUNT, + "get_config_bundle result_field_count is inconsistent with RESULT_SCHEMA_FIELD_COUNT" + ); + } else { + panic!("get_config_bundle returned None after initialization"); + } + } +} diff --git a/docs/result-schema-migration-guard.md b/docs/result-schema-migration-guard.md new file mode 100644 index 0000000..f2d1d47 --- /dev/null +++ b/docs/result-schema-migration-guard.md @@ -0,0 +1,245 @@ +# Result Schema Migration Guard + +> **Safe migration guardrail for `get_result_schema()` when the result layout changes.** +> +> Resolves #255. This document describes the mechanism used to prevent +> `SLAResult` layout changes from silently breaking backend consumers, the +> process for making a deliberate schema change, and what CI checks to expect. + +--- + +## Table of Contents + +1. [Problem](#problem) +2. [Mechanism](#mechanism) +3. [Constants](#constants) +4. [The `result_field_count` field in `SLAResultSchema`](#the-result_field_count-field-in-slaresultschema) +5. [CI-backed tests (`schema_migration_tests.rs`)](#ci-backed-tests-schema_migration_testsrs) +6. [Process for changing `SLAResult`](#process-for-changing-slaresult) +7. [Process for deprecating a symbol](#process-for-deprecating-a-symbol) +8. [Backend consumer guidance](#backend-consumer-guidance) +9. [Release process requirements](#release-process-requirements) + +--- + +## Problem + +`get_result_schema()` is the primary compatibility boundary between the contract +and backend consumers. Before this guardrail existed: + +- A contributor could add or rename a field in `SLAResult` without bumping + `RESULT_SCHEMA_VERSION`. +- The contract would continue to compile and pass its existing tests. +- Backend consumers would deserialize the wrong fields or miss new ones silently — + no compile error, no test failure, only corrupt data at runtime. + +--- + +## Mechanism + +The guardrail operates on two levels: + +### Level 1 — Compile-time + +`schema_migration_tests.rs` contains a fully destructured `SLAResult` pattern +match: + +```rust +let SLAResult { + outage_id: _, + status: _, + mttr_minutes: _, + threshold_minutes: _, + amount: _, + payment_type: _, + rating: _, + config_version_hash: _, + recorded_at: _, +} = sample; +``` + +If a field is added or removed from `SLAResult`, this destructure will **fail to +compile** — surfacing the change before any runtime tests run. + +### Level 2 — Runtime (CI) + +Four tests in `schema_migration_tests.rs` assert: + +| Test | What it checks | +|---|---| +| `test_result_schema_field_count_sentinel` | `RESULT_SCHEMA_FIELD_COUNT == 9` (current field count) | +| `test_get_result_schema_version_matches_constant` | `get_result_schema()` returns `schema_version == RESULT_SCHEMA_VERSION` and `result_field_count == RESULT_SCHEMA_FIELD_COUNT` | +| `test_result_schema_symbols_are_stable` | Every symbol in `get_result_schema()` matches the canonical value baked into `compute_result` | +| `test_result_schema_no_deprecated_symbols_at_v1` | `deprecated_symbols` list is empty for schema v1 | +| `test_config_bundle_schema_version_consistent` | `get_config_bundle()` embeds the same schema version and field count | + +Any layout change that is not reflected in the constants will cause at least one +of these tests to fail in CI. + +--- + +## Constants + +| Constant | Location | Value | Meaning | +|---|---|---|---| +| `RESULT_SCHEMA_VERSION` | `lib.rs` | `1` | Breaking-change counter for `SLAResult` layout | +| `RESULT_SCHEMA_FIELD_COUNT` | `lib.rs` | `9` | Number of named fields in `SLAResult` | + +Both constants are exposed through `get_result_schema()` so backend consumers +can detect drift at runtime without hardcoding field lists. + +--- + +## The `result_field_count` field in `SLAResultSchema` + +`SLAResultSchema` now includes: + +```rust +/// Number of named fields in `SLAResult` at this schema version. +pub result_field_count: u32, +``` + +Backend consumers can call `get_result_schema()` at startup and compare +`result_field_count` against their own deserialization code. A mismatch signals +that the contract binary is newer or older than the backend expects. + +--- + +## CI-backed tests (`schema_migration_tests.rs`) + +The test file lives at: + +``` +apexchainx_calculator/src/schema_migration_tests.rs +``` + +It is declared as a `#[cfg(test)]` module in `lib.rs` and runs as part of the +standard `cargo test --lib` step in CI. No special flags are needed. + +To run the guardrail tests locally: + +```bash +cd apexchainx_calculator +cargo test schema_migration_tests +``` + +--- + +## Process for changing `SLAResult` + +> Follow these steps in order. All steps must be in the same PR. + +### Step 1 — Modify `SLAResult` + +Add, remove, or rename a field in the `SLAResult` struct in `lib.rs`. + +### Step 2 — Fix the compile-time destructure in `schema_migration_tests.rs` + +Update the exhaustive destructure in `test_result_schema_field_count_sentinel` +to include the new field. The test will not compile until you do this. + +### Step 3 — Update `RESULT_SCHEMA_FIELD_COUNT` + +Change the value in `lib.rs` to match the new field count. + +### Step 4 — Increment `RESULT_SCHEMA_VERSION` + +Every field-level change to `SLAResult` is a **breaking change** for backend +consumers. Increment the constant in `lib.rs`: + +```rust +pub(crate) const RESULT_SCHEMA_VERSION: u32 = 2; // was 1 +``` + +### Step 5 — Update `get_result_schema()` if needed + +If a new symbol descriptor is required (e.g. for a new status or payment type), +add it to `SLAResultSchema` and populate it in `get_result_schema()`. + +### Step 6 — Update the symbol stability test + +In `test_result_schema_symbols_are_stable`, add assertions for any new symbol +fields. If an existing symbol value changes, update the expected string. + +### Step 7 — Update `CHANGELOG.md` + +Under `[Unreleased]` → `Changed`: + +```markdown +- `SLAResult` — added `: ` (breaking); `RESULT_SCHEMA_VERSION` + bumped from 1 to 2 (closes #NNN) +``` + +### Step 8 — Notify backend consumers + +The `apexchainx-be` team must be notified before the PR is merged. Provide: +- The new `RESULT_SCHEMA_VERSION` value +- The added/removed/changed field name and type +- A migration note in the PR description explaining how the backend should adapt + +### Step 9 — Run CI locally + +```bash +cd apexchainx_calculator +cargo fmt +cargo clippy -- -D warnings +cargo test --lib +cargo check --target wasm32-unknown-unknown --lib +``` + +--- + +## Process for deprecating a symbol + +When a result symbol (e.g. the value of `status_met`) is being replaced: + +1. Continue emitting the old symbol (backward-compatible coexistence period). +2. Add a `DeprecatedSymbol` entry to `deprecated_symbols` in `get_result_schema()`. +3. Update `test_result_schema_no_deprecated_symbols_at_v1` to assert the entry + is present rather than asserting the list is empty. +4. Announce the removal version in `deprecated_at` and `removal_version`. +5. In the release after `removal_version`, remove the old symbol and update all + tests. + +--- + +## Backend consumer guidance + +| Signal | Action | +|---|---| +| `schema_version` unchanged, `result_field_count` unchanged | No change needed; proceed normally | +| `schema_version` bumped | Schema breaking change; redeploy backend adapter before or at the same release | +| `result_field_count` larger | New field present; old deserialization code may miss it — update and test | +| `result_field_count` smaller | Field removed; old deserialization code may error — update and test | +| `deprecated_symbols` non-empty | Start migration off the old symbol before `removal_version` is reached | + +Backends should call `get_result_schema()` (or `get_config_bundle()`) once at +startup and compare `schema_version` and `result_field_count` against compiled-in +expectations. A mismatch should block operations and alert on-call until the +backend and contract versions are aligned. + +--- + +## Release process requirements + +Before any release that includes a schema change: + +- [ ] `RESULT_SCHEMA_VERSION` incremented +- [ ] `RESULT_SCHEMA_FIELD_COUNT` updated +- [ ] All four schema migration tests pass (`cargo test schema_migration_tests`) +- [ ] `CHANGELOG.md` updated with the breaking change note +- [ ] `apexchainx-be` team notified and adapter PR is merged or in flight +- [ ] PR description contains a **Schema Migration Note** section: + +```markdown +## Schema Migration Note + +`RESULT_SCHEMA_VERSION` bumped from X to Y. + +**Changed field(s):** +- Added `: ` — + +**Backend action required:** +- Update deserialization to handle the new field +- Re-run backend parity tests against the new WASM binary +```