refactor: [EXC-1752] Remove the parts of old CanisterStateBits after migration (#4335)

This is follow-up on #2254. The plan
is to merge this PR after the cutoff for this week's release.

---------

Co-authored-by: Dimitris Sarlis <dimitrios.sarlis@dfinity.org>

Author: dragoljub-duric

rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto

@@ -409,8 +409,7 @@ message CanisterStateBits {
   uint64 install_code_debit = 29;
   // Contains tasks that need to be executed before processing any input of the
   // canister.
-  // This field will be deprecated after EXC-1752.
-  repeated ExecutionTask task_queue = 30;
+  reserved 30;
   // Time of last charge for resource allocations.
   google.protobuf.UInt64Value time_of_last_allocation_charge_nanos = 31;
   // Postponed charges that are not applied to `cycles_balance` yet.
@@ -451,8 +450,7 @@ message CanisterStateBits {
   int64 priority_credit = 48;
   LongExecutionMode long_execution_mode = 49;
   optional uint64 wasm_memory_threshold = 50;
-  // This field will be deprecated after EXC-1752.
-  optional OnLowWasmMemoryHookStatus on_low_wasm_memory_hook_status = 53;
+  reserved 53;
   // Contains tasks that need to be executed before processing any input of the
   // canister.
   TaskQueue tasks = 54;

rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs

@@ -602,11 +602,6 @@ pub struct CanisterStateBits {
     /// tracked for the purposes of rate limiting the install_code messages.
     #[prost(uint64, tag = "29")]
     pub install_code_debit: u64,
-    /// Contains tasks that need to be executed before processing any input of the
-    /// canister.
-    /// This field will be deprecated after EXC-1752.
-    #[prost(message, repeated, tag = "30")]
-    pub task_queue: ::prost::alloc::vec::Vec<ExecutionTask>,
     /// Time of last charge for resource allocations.
     #[prost(message, optional, tag = "31")]
     pub time_of_last_allocation_charge_nanos: ::core::option::Option<u64>,
@@ -662,9 +657,6 @@ pub struct CanisterStateBits {
     pub long_execution_mode: i32,
     #[prost(uint64, optional, tag = "50")]
     pub wasm_memory_threshold: ::core::option::Option<u64>,
-    /// This field will be deprecated after EXC-1752.
-    #[prost(enumeration = "OnLowWasmMemoryHookStatus", optional, tag = "53")]
-    pub on_low_wasm_memory_hook_status: ::core::option::Option<i32>,
     /// Contains tasks that need to be executed before processing any input of the
     /// canister.
     #[prost(message, optional, tag = "54")]

rs/replicated_state/src/canister_state/system_state/task_queue.rs

@@ -28,32 +28,6 @@ pub struct TaskQueue {
 }
 
 impl TaskQueue {
-    pub fn from_checkpoint(
-        queue: VecDeque<ExecutionTask>,
-        on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus,
-    ) -> Self {
-        let mut mut_queue = queue;
-
-        // Extraction of paused_or_aborted_task from queue will be removed in the follow-up EXC-1752 when
-        // we introduce CanisterStateBits version of TaskQueue, so the conversion will be implicit.
-        let paused_or_aborted_task = match mut_queue.front() {
-            Some(ExecutionTask::AbortedInstallCode { .. })
-            | Some(ExecutionTask::PausedExecution { .. })
-            | Some(ExecutionTask::PausedInstallCode(_))
-            | Some(ExecutionTask::AbortedExecution { .. }) => mut_queue.pop_front(),
-            Some(ExecutionTask::OnLowWasmMemory)
-            | Some(ExecutionTask::Heartbeat)
-            | Some(ExecutionTask::GlobalTimer)
-            | None => None,
-        };
-
-        Self {
-            paused_or_aborted_task,
-            on_low_wasm_memory_hook_status,
-            queue: mut_queue,
-        }
-    }
-
     pub fn front(&self) -> Option<&ExecutionTask> {
         self.paused_or_aborted_task.as_ref().or_else(|| {
             if self.on_low_wasm_memory_hook_status.is_ready() {
@@ -123,20 +97,11 @@ impl TaskQueue {
             }
     }
 
-    /// peek_hook_status will be removed in the follow-up EXC-1752.
+    /// This function is used only in tests.
     pub fn peek_hook_status(&self) -> OnLowWasmMemoryHookStatus {
         self.on_low_wasm_memory_hook_status
     }
 
-    /// get_queue will be removed in the follow-up EXC-1752.
-    pub fn get_queue(&self) -> VecDeque<ExecutionTask> {
-        let mut queue = self.queue.clone();
-        if let Some(task) = self.paused_or_aborted_task.as_ref() {
-            queue.push_front(task.clone());
-        }
-        queue
-    }
-
     /// `check_dts_invariants` should only be called after round execution.
     ///
     /// It checks that the following properties are satisfied:

rs/state_layout/src/state_layout.rs

@@ -18,7 +18,7 @@ use ic_replicated_state::{
         },
     },
     page_map::{Shard, StorageLayout, StorageResult},
-    CallContextManager, CanisterStatus, ExecutionTask, ExportedFunctions, NumWasmPages,
+    CallContextManager, CanisterStatus, ExportedFunctions, NumWasmPages,
 };
 use ic_sys::{fs::sync_path, mmap::ScopedMmap};
 use ic_types::{
@@ -29,7 +29,7 @@ use ic_types::{
 use ic_utils::thread::maybe_parallel_map;
 use ic_wasm_types::{CanisterModule, WasmHash};
 use prometheus::{Histogram, IntCounterVec};
-use std::collections::{BTreeMap, BTreeSet, VecDeque};
+use std::collections::{BTreeMap, BTreeSet};
 use std::convert::{identity, From, TryFrom, TryInto};
 use std::ffi::OsStr;
 use std::fs::OpenOptions;
@@ -2385,12 +2385,6 @@ impl From<CanisterStateBits> for pb_canister_state_bits::CanisterStateBits {
             heap_delta_debit: item.heap_delta_debit.get(),
             install_code_debit: item.install_code_debit.get(),
             time_of_last_allocation_charge_nanos: Some(item.time_of_last_allocation_charge_nanos),
-            task_queue: item
-                .task_queue
-                .get_queue()
-                .iter()
-                .map(|v| v.into())
-                .collect(),
             global_timer_nanos: item.global_timer_nanos,
             canister_version: item.canister_version,
             consumed_cycles_by_use_cases: item
@@ -2418,12 +2412,6 @@ impl From<CanisterStateBits> for pb_canister_state_bits::CanisterStateBits {
             wasm_memory_limit: item.wasm_memory_limit.map(|v| v.get()),
             next_snapshot_id: item.next_snapshot_id,
             snapshots_memory_usage: item.snapshots_memory_usage.get(),
-            on_low_wasm_memory_hook_status: Some(
-                pb_canister_state_bits::OnLowWasmMemoryHookStatus::from(
-                    &item.task_queue.peek_hook_status(),
-                )
-                .into(),
-            ),
             tasks: Some((&item.task_queue).into()),
         }
     }
@@ -2479,40 +2467,10 @@ impl TryFrom<pb_canister_state_bits::CanisterStateBits> for CanisterStateBits {
             );
         }
 
-        let pb_task_queue: Result<pb_canister_state_bits::TaskQueue, ProxyDecodeError> =
-            try_from_option_field(value.tasks, "CanisterStateBits::tasks");
+        let tasks: pb_canister_state_bits::TaskQueue =
+            try_from_option_field(value.tasks, "CanisterStateBits::tasks").unwrap_or_default();
 
-        let task_queue = match pb_task_queue {
-            Ok(tasks) => TaskQueue::try_from(tasks)?,
-            Err(_) => {
-                let task_queue: VecDeque<ExecutionTask> = value
-                    .task_queue
-                    .into_iter()
-                    .map(|v| v.try_into())
-                    .collect::<Result<VecDeque<_>, _>>()?;
-                if task_queue.len() > 1 {
-                    return Err(ProxyDecodeError::Other(format!(
-                        "Expecting at most one task queue entry. Found {:?}",
-                        task_queue
-                    )));
-                }
-
-                let on_low_wasm_memory_hook_status: Option<
-                    pb_canister_state_bits::OnLowWasmMemoryHookStatus,
-                > = value.on_low_wasm_memory_hook_status.map(|v| {
-                    pb_canister_state_bits::OnLowWasmMemoryHookStatus::try_from(v)
-                        .unwrap_or_default()
-                });
-                TaskQueue::from_checkpoint(
-                    task_queue,
-                    try_from_option_field(
-                        on_low_wasm_memory_hook_status,
-                        "CanisterStateBits::on_low_wasm_memory_hook_status",
-                    )
-                    .unwrap_or_default(),
-                )
-            }
-        };
+        let task_queue = TaskQueue::try_from(tasks)?;
 
         Ok(Self {
             controllers,

rs/state_layout/src/state_layout/tests.rs

@@ -4,6 +4,7 @@ use ic_management_canister_types_private::{
     CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallMode, IC_00,
 };
 use ic_replicated_state::canister_state::system_state::PausedExecutionId;
+use ic_replicated_state::ExecutionTask;
 use ic_replicated_state::{
     canister_state::system_state::CanisterHistory,
     metadata_state::subnet_call_context_manager::InstallCodeCallId, page_map::Shard, NumWasmPages,

rs/system_api/tests/system_api.rs

@@ -1456,7 +1456,7 @@ fn helper_test_on_low_wasm_memory(
     let mut state_builder = SystemStateBuilder::default()
         .wasm_memory_threshold(wasm_memory_threshold)
         .wasm_memory_limit(wasm_memory_limit)
-        .on_low_wasm_memory_hook_status(start_status)
+        .empty_task_queue_with_on_low_wasm_memory_hook_status(start_status)
         .initial_cycles(Cycles::from(10_000_000_000_000_000u128));
 
     if let Some(memory_allocation) = memory_allocation {

rs/test_utilities/state/src/lib.rs

@@ -467,12 +467,30 @@ impl SystemStateBuilder {
         self
     }
 
-    pub fn on_low_wasm_memory_hook_status(
+    pub fn empty_task_queue_with_on_low_wasm_memory_hook_status(
         mut self,
         on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus,
     ) -> Self {
-        self.system_state.task_queue =
-            TaskQueue::from_checkpoint(VecDeque::new(), on_low_wasm_memory_hook_status);
+        self.system_state.task_queue = TaskQueue::default();
+        match on_low_wasm_memory_hook_status {
+            // Default hook status is `ConditionNotSatisfied`.
+            OnLowWasmMemoryHookStatus::ConditionNotSatisfied => (),
+            // To make hook status `Ready`, we should enqueue `ExecutionTask::OnLowWasmMemory`.
+            OnLowWasmMemoryHookStatus::Ready => self
+                .system_state
+                .task_queue
+                .enqueue(ic_replicated_state::ExecutionTask::OnLowWasmMemory),
+            // To make hook status `Executed`, we should enqueue `ExecutionTask::OnLowWasmMemory`,
+            // followed by `pop_front()`, which from the standpoint of `TaskQueue` is equivalent to
+            // executing task.
+            OnLowWasmMemoryHookStatus::Executed => {
+                self.system_state
+                    .task_queue
+                    .enqueue(ic_replicated_state::ExecutionTask::OnLowWasmMemory);
+                self.system_state.task_queue.pop_front();
+            }
+        };
+
         self
     }