test: EXC-1652: Future proof canister snapshots (#1677)

Introducing small functions that requires developers to consider the
potential impact of adding or removing canister state fields on the
canister snapshot logic. This covers changes in CanisterState,
SystemState, ExecutionState, WasmBinary and SchedulerState.

rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs

@@ -13,7 +13,9 @@ use ic_management_canister_types::{
 };
 use ic_registry_subnet_type::SubnetType;
 use ic_replicated_state::{
-    canister_snapshots::SnapshotOperation, canister_state::system_state::CyclesUseCase,
+    canister_snapshots::SnapshotOperation,
+    canister_state::{execution_state::WasmBinary, system_state::CyclesUseCase},
+    CanisterState, ExecutionState, SchedulerState,
 };
 use ic_test_utilities_execution_environment::{
     get_output_messages, ExecutionTest, ExecutionTestBuilder,
@@ -28,6 +30,7 @@ use ic_types::{
 use ic_universal_canister::{call_args, wasm, UNIVERSAL_CANISTER_WASM};
 use more_asserts::assert_gt;
 use serde_bytes::ByteBuf;
+use std::borrow::Borrow;
 
 #[test]
 fn take_canister_snapshot_decode_round_trip() {
@@ -1908,3 +1911,69 @@ fn snapshot_must_include_globals() {
         .unwrap();
     assert_eq!(result, WasmResult::Reply(vec![1, 0, 0, 0]));
 }
+
+/// Early warning system / stumbling block forcing the authors of changes adding
+/// or removing canister state fields to think about and/or ask the Execution
+/// team to think about any repercussions to the canister snapshot logic.
+///
+/// If you do find yourself having to make changes to this function, it is quite
+/// possible that you have not broken anything. But there is a non-zero chance
+/// for changes to the structure of the canister state to also require changes
+/// to the canister snapshot logic or risk breaking it. Which is why this brute
+/// force check exists.
+///
+/// See `CanisterSnapshot::from_canister()` for more context.
+#[allow(dead_code)]
+fn canister_snapshot_change_guard_do_not_modify_without_reading_doc_comment() {
+    let mut test = ExecutionTestBuilder::new().build();
+    let uc = test.universal_canister().unwrap();
+    let canister_state = test.canister_state(uc).clone();
+
+    //
+    // DO NOT MODIFY WITHOUT READING DOC COMMENT!
+    //
+    let CanisterState {
+        // There is a separate test for SystemState.
+        system_state: _,
+        execution_state,
+        scheduler_state,
+    } = canister_state;
+
+    //
+    // DO NOT MODIFY WITHOUT READING DOC COMMENT!
+    //
+    let ExecutionState {
+        canister_root: _,
+        wasm_binary,
+        wasm_memory: _,
+        stable_memory: _,
+        exported_globals: _,
+        exports: _,
+        metadata: _,
+        last_executed_round: _,
+        next_scheduled_method: _,
+    } = execution_state.unwrap();
+
+    //
+    // DO NOT MODIFY WITHOUT READING DOC COMMENT!
+    //
+    let WasmBinary {
+        binary: _,
+        embedder_cache: _,
+    } = wasm_binary.borrow();
+
+    //
+    // DO NOT MODIFY WITHOUT READING DOC COMMENT!
+    //
+    let SchedulerState {
+        last_full_execution_round: _,
+        compute_allocation: _,
+        accumulated_priority: _,
+        priority_credit: _,
+        long_execution_mode: _,
+        heap_delta_debit: _,
+        install_code_debit: _,
+        time_of_last_allocation_charge: _,
+        total_query_stats: _,
+    } = scheduler_state;
+}

rs/replicated_state/src/canister_state/system_state.rs

@@ -1767,4 +1767,48 @@ pub mod testing {
             self.split_input_schedules(own_canister_id, local_canisters)
         }
     }
+
+    /// Early warning system / stumbling block forcing the authors of changes adding
+    /// or removing system state fields to think about and/or ask the Execution
+    /// team to think about any repercussions to the canister snapshot logic.
+    ///
+    /// If you do find yourself having to make changes to this function, it is quite
+    /// possible that you have not broken anything. But there is a non-zero chance
+    /// for changes to the structure of the system state to also require changes
+    /// to the canister snapshot logic or risk breaking it. Which is why this brute
+    /// force check exists.
+    ///
+    /// See `CanisterSnapshot::from_canister()` for more context.
+    #[allow(dead_code)]
+    fn canister_snapshot_change_guard_do_not_modify_without_reading_doc_comment() {
+        //
+        // DO NOT MODIFY WITHOUT READING DOC COMMENT!
+        //
+        let _system_state = SystemState {
+            controllers: Default::default(),
+            canister_id: 0.into(),
+            queues: Default::default(),
+            memory_allocation: Default::default(),
+            wasm_memory_threshold: Default::default(),
+            freeze_threshold: Default::default(),
+            status: CanisterStatus::Stopped,
+            certified_data: Default::default(),
+            canister_metrics: Default::default(),
+            cycles_balance: Default::default(),
+            ingress_induction_cycles_debit: Default::default(),
+            reserved_balance: Default::default(),
+            reserved_balance_limit: Default::default(),
+            task_queue: Default::default(),
+            global_timer: CanisterTimer::Inactive,
+            canister_version: Default::default(),
+            canister_history: Default::default(),
+            wasm_chunk_store: WasmChunkStore::new_for_testing(),
+            log_visibility: Default::default(),
+            canister_log: Default::default(),
+            wasm_memory_limit: Default::default(),
+            next_snapshot_id: Default::default(),
+            snapshots_memory_usage: Default::default(),
+            on_low_wasm_memory_hook_status: Default::default(),
+        };
+    }
 }