refactor: drop AddCanisterChangeToHistory enum (#7860)

This PR drops the `AddCanisterChangeToHistory` enum by inlining the
canister history update to `CanisterManager::uninstall_code`.

The motivation for this change is to decouple `RoundLimits` update due
to uninstalling code and due to updating canister history: those steps
will be performed separately (in a follow-up PR) when taking a canister
snapshot and uninstalling its code atomically (and thus setting
`RoundLimits` to `None` would conveniently skip the `RoundLimits` update
in the helper function `uninstall_canister`).

Author: mraszyk

rs/execution_environment/src/canister_manager.rs

@@ -966,16 +966,26 @@ impl CanisterManager {
             canister,
             Some(round_limits),
             time,
-            AddCanisterChangeToHistory::Yes(origin),
             Arc::clone(&self.fd_factory),
         );
+
+        let available_execution_memory_change = canister.add_canister_change(
+            time,
+            origin,
+            CanisterChangeDetails::CanisterCodeUninstall,
+        );
+        round_limits
+            .subnet_available_memory
+            .update_execution_memory_unchecked(available_execution_memory_change);
+
         crate::util::process_responses(
             rejects,
             state,
             Arc::clone(&self.ingress_history_writer),
             self.log.clone(),
             canister_not_found_error,
         );
+
         Ok(())
     }
 
@@ -3109,9 +3119,8 @@ fn get_response_size(kind: &CanisterSnapshotDataKind) -> Result<u64, CanisterMan
 pub fn uninstall_canister(
     log: &ReplicaLogger,
     canister: &mut CanisterState,
-    mut round_limits: Option<&mut RoundLimits>,
+    round_limits: Option<&mut RoundLimits>,
     time: Time,
-    add_canister_change: AddCanisterChangeToHistory,
     fd_factory: Arc<dyn PageAllocatorFileDescriptor>,
 ) -> Vec<Response> {
     let old_allocated_bytes = canister.memory_allocated_bytes();
@@ -3136,7 +3145,7 @@ pub fn uninstall_canister(
     let new_allocated_bytes = canister.memory_allocated_bytes();
     debug_assert!(new_allocated_bytes <= old_allocated_bytes);
 
-    if let Some(round_limits) = round_limits.as_deref_mut() {
+    if let Some(round_limits) = round_limits {
         let deallocated_bytes = old_allocated_bytes.saturating_sub(&new_allocated_bytes);
         round_limits.subnet_available_memory.increment(
             deallocated_bytes,
@@ -3145,22 +3154,6 @@ pub fn uninstall_canister(
         );
     }
 
-    match add_canister_change {
-        AddCanisterChangeToHistory::Yes(origin) => {
-            let available_execution_memory_change = canister.add_canister_change(
-                time,
-                origin,
-                CanisterChangeDetails::CanisterCodeUninstall,
-            );
-            if let Some(round_limits) = round_limits {
-                round_limits
-                    .subnet_available_memory
-                    .update_execution_memory_unchecked(available_execution_memory_change);
-            }
-        }
-        AddCanisterChangeToHistory::No => {}
-    };
-
     let canister_id = canister.canister_id();
 
     canister

rs/execution_environment/src/canister_manager/tests.rs

@@ -1,9 +1,9 @@
 use crate::{
     IngressHistoryWriterImpl, RoundLimits, as_num_instructions,
     canister_manager::{
-        AddCanisterChangeToHistory, CanisterManager, CanisterManagerError, CanisterMgrConfig,
-        DtsInstallCodeResult, InstallCodeContext, MAX_SLICE_SIZE_BYTES, StopCanisterResult,
-        WasmSource, uninstall_canister,
+        CanisterManager, CanisterManagerError, CanisterMgrConfig, DtsInstallCodeResult,
+        InstallCodeContext, MAX_SLICE_SIZE_BYTES, StopCanisterResult, WasmSource,
+        uninstall_canister,
     },
     canister_settings::CanisterSettings,
     execution_environment::{CompilationCostHandling, RoundCounters, as_round_instructions},
@@ -2286,7 +2286,6 @@ fn uninstall_canister_doesnt_respond_to_responded_call_contexts() {
                 .build(),
             None,
             UNIX_EPOCH,
-            AddCanisterChangeToHistory::No,
             Arc::new(TestPageAllocatorFileDescriptorImpl),
         ),
         Vec::new()
@@ -2313,7 +2312,6 @@ fn uninstall_canister_responds_to_unresponded_call_contexts() {
                 .build(),
             None,
             UNIX_EPOCH,
-            AddCanisterChangeToHistory::No,
             Arc::new(TestPageAllocatorFileDescriptorImpl),
         )[0],
         Response::Ingress(IngressResponse {

rs/execution_environment/src/canister_manager/types.rs

@@ -316,12 +316,6 @@ impl TryFrom<(CanisterChangeOrigin, InstallCodeArgsV2)> for InstallCodeContext {
     }
 }
 
-/// Indicates whether `uninstall_canister` should push a canister change (with a given change origin) to canister history.
-pub enum AddCanisterChangeToHistory {
-    Yes(CanisterChangeOrigin),
-    No,
-}
-
 pub(crate) struct UploadChunkResult {
     pub(crate) reply: UploadChunkReply,
     pub(crate) heap_delta_increase: NumBytes,

rs/execution_environment/src/scheduler.rs

@@ -1,5 +1,5 @@
 use crate::{
-    canister_manager::{types::AddCanisterChangeToHistory, uninstall_canister},
+    canister_manager::uninstall_canister,
     execution_environment::{
         ExecuteCanisterResult, ExecutionEnvironment, RoundInstructions, RoundLimits,
         as_num_instructions, as_round_instructions, execute_canister,
@@ -934,7 +934,6 @@ impl SchedulerImpl {
                         canister,
                         None, /* we're at the end of a round so no need to update round limits */
                         state_time,
-                        AddCanisterChangeToHistory::No,
                         Arc::clone(&self.fd_factory),
                     ));
                     canister.scheduler_state.compute_allocation = ComputeAllocation::zero();