fix: EXC-1889 Hook condition should be checked after every mgmt canister call

rs/execution_environment/src/canister_manager.rs

@@ -643,9 +643,6 @@ impl CanisterManager {
         if let Some(wasm_memory_limit) = settings.wasm_memory_limit() {
             canister.system_state.wasm_memory_limit = Some(wasm_memory_limit);
         }
-        // Change of `wasm_memory_limit` or `new_wasm_memory_threshold` or `memory_allocation`,
-        // can influence the satisfaction of the condition for `low_wasm_memory` hook.
-        canister.update_on_low_wasm_memory_hook_condition();
     }
 
     /// Tries to apply the requested settings on the canister identified by

rs/execution_environment/src/canister_manager/tests.rs

@@ -8103,118 +8103,88 @@ fn helper_update_settings_updates_hook_status(
     updated_memory_state: MemoryState,
     execute_hook_after_install: bool,
 ) {
-    with_setup(|canister_manager, mut state, subnet_id| {
-        let mut round_limits = RoundLimits {
-            instructions: as_round_instructions(EXECUTION_PARAMETERS.instruction_limits.message()),
-            subnet_available_memory: (*MAX_SUBNET_AVAILABLE_MEMORY),
-            subnet_available_callbacks: SUBNET_CALLBACK_SOFT_LIMIT as i64,
-            compute_allocation_used: state.total_compute_allocation(),
-        };
+    let mut test = ExecutionTestBuilder::new().build();
 
-        let wat = format!("(module (memory {used_wasm_memory_pages}))");
+    let mut initial_settings = CanisterSettingsArgsBuilder::new();
 
-        let wasm = wat::parse_str(wat).unwrap();
+    if let Some(MemoryAllocation::Reserved(memory_allocation)) =
+        initial_memory_state.memory_allocation
+    {
+        initial_settings = initial_settings.with_memory_allocation(memory_allocation.get());
+    }
 
-        let sender = canister_test_id(100).get();
+    if let Some(wasm_memory_limit) = initial_memory_state.wasm_memory_limit {
+        initial_settings = initial_settings.with_wasm_memory_limit(wasm_memory_limit.get());
+    }
 
-        let initial_settings = CanisterSettings {
-            wasm_memory_limit: initial_memory_state.wasm_memory_limit,
-            wasm_memory_threshold: initial_memory_state.wasm_memory_threshold,
-            memory_allocation: initial_memory_state.memory_allocation,
-            ..Default::default()
-        };
+    if let Some(wasm_memory_threshold) = initial_memory_state.wasm_memory_threshold {
+        initial_settings = initial_settings.with_wasm_memory_threshold(wasm_memory_threshold.get());
+    }
 
-        let canister_id = canister_manager
-            .create_canister(
-                canister_change_origin_from_principal(&sender),
-                subnet_id,
-                *INITIAL_CYCLES,
-                initial_settings,
-                MAX_NUMBER_OF_CANISTERS,
-                &mut state,
-                SMALL_APP_SUBNET_MAX_SIZE,
-                &mut round_limits,
-                ResourceSaturation::default(),
-                &no_op_counter(),
-            )
-            .0
-            .unwrap();
+    let canister_id = test
+        .create_canister_with_settings(*INITIAL_CYCLES, initial_settings.build())
+        .unwrap();
 
-        let res = install_code(
-            &canister_manager,
-            InstallCodeContext {
-                origin: canister_change_origin_from_principal(&sender),
-                canister_id,
-                wasm_source: WasmSource::CanisterModule(CanisterModule::new(wasm)),
-                arg: vec![],
-                compute_allocation: None,
-                memory_allocation: None,
-                mode: CanisterInstallModeV2::Install,
-            },
-            &mut state,
-            &mut round_limits,
-        );
-        assert!(res.1.is_ok(), "{:?}", res.1);
+    let wat = format!("(module (memory {used_wasm_memory_pages}))");
 
-        let mut c_state = res.2.unwrap();
+    let wasm = wat::parse_str(wat).unwrap();
 
-        if execute_hook_after_install {
-            c_state.system_state.task_queue.pop_front();
-        }
+    test.install_canister(canister_id, wasm).unwrap();
 
-        assert_eq!(
-            c_state.system_state.task_queue.peek_hook_status(),
-            initial_memory_state.hook_status
-        );
+    if execute_hook_after_install {
+        test.canister_state_mut(canister_id)
+            .system_state
+            .task_queue
+            .pop_front();
+    }
 
-        state.put_canister_state(c_state);
+    assert_eq!(
+        test.canister_state_mut(canister_id)
+            .system_state
+            .task_queue
+            .peek_hook_status(),
+        initial_memory_state.hook_status
+    );
 
-        let mut settings = CanisterSettingsBuilder::new();
+    let mut settings = CanisterSettingsArgsBuilder::new();
 
-        if updated_memory_state.wasm_memory_threshold != initial_memory_state.wasm_memory_threshold
-        {
-            if let Some(updated_wasm_memory_threshold) = updated_memory_state.wasm_memory_threshold
-            {
-                settings = settings.with_wasm_memory_threshold(updated_wasm_memory_threshold);
-            }
+    if updated_memory_state.wasm_memory_threshold != initial_memory_state.wasm_memory_threshold {
+        if let Some(updated_wasm_memory_threshold) = updated_memory_state.wasm_memory_threshold {
+            settings = settings.with_wasm_memory_threshold(updated_wasm_memory_threshold.get());
         }
+    }
 
-        if updated_memory_state.wasm_memory_limit != initial_memory_state.wasm_memory_limit {
-            if let Some(updated_wasm_memory_limit) = updated_memory_state.wasm_memory_limit {
-                settings = settings.with_wasm_memory_limit(updated_wasm_memory_limit);
-            }
+    if updated_memory_state.wasm_memory_limit != initial_memory_state.wasm_memory_limit {
+        if let Some(updated_wasm_memory_limit) = updated_memory_state.wasm_memory_limit {
+            settings = settings.with_wasm_memory_limit(updated_wasm_memory_limit.get());
         }
+    }
 
-        if updated_memory_state.memory_allocation != initial_memory_state.memory_allocation {
-            if let Some(updated_memory_allocation) = updated_memory_state.memory_allocation {
-                settings = settings.with_memory_allocation(updated_memory_allocation);
-            }
+    if updated_memory_state.memory_allocation != initial_memory_state.memory_allocation {
+        if let Some(MemoryAllocation::Reserved(updated_memory_allocation)) =
+            updated_memory_state.memory_allocation
+        {
+            settings = settings.with_memory_allocation(updated_memory_allocation.get());
         }
+    }
 
-        let canister = state.canister_state_mut(&canister_id).unwrap();
+    let payload = UpdateSettingsArgs {
+        canister_id: canister_id.get(),
+        settings: settings.build(),
+        sender_canister_version: None,
+    }
+    .encode();
 
-        assert!(canister_manager
-            .update_settings(
-                Time::from_nanos_since_unix_epoch(777),
-                canister_change_origin_from_principal(&sender),
-                settings.build(),
-                canister,
-                &mut round_limits,
-                ResourceSaturation::default(),
-                SMALL_APP_SUBNET_MAX_SIZE,
-            )
-            .is_ok());
+    test.subnet_message(Method::UpdateSettings, payload)
+        .unwrap();
 
-        assert_eq!(
-            state
-                .canister_state_mut(&canister_id)
-                .unwrap()
-                .system_state
-                .task_queue
-                .peek_hook_status(),
-            updated_memory_state.hook_status
-        );
-    })
+    assert_eq!(
+        test.canister_state_mut(canister_id)
+            .system_state
+            .task_queue
+            .peek_hook_status(),
+        updated_memory_state.hook_status
+    );
 }
 
 #[test]