From 9c006a50d364edf1403ef50b24c3be39dba8a5f6 Mon Sep 17 00:00:00 2001
From: Adam Bratschi-Kaye <adam.bratschikaye@dfinity.org>
Date: Mon, 24 Jun 2024 11:45:44 +0000
Subject: [PATCH] [hotfix]: Prevent lowering Cycles reservation limit below
 existing

If a canister has it's settings updated to decrease it's reservation
limit below the amount of cycles already reserved then we should fail
the update.
---
 .../src/canister_settings.rs                  | 17 +++-
 .../src/hypervisor/tests.rs                   | 87 +++++++++++++++++++
 2 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/rs/execution_environment/src/canister_settings.rs b/rs/execution_environment/src/canister_settings.rs
index 8d3cf9c5613..53a79618ae5 100644
--- a/rs/execution_environment/src/canister_settings.rs
+++ b/rs/execution_environment/src/canister_settings.rs
@@ -520,7 +520,22 @@ pub(crate) fn validate_canister_settings(
     }
 
     let reservation_cycles = if new_memory_bytes <= old_memory_bytes {
-        Cycles::zero()
+        let reservation_cycles = Cycles::zero();
+        let reserved_balance_limit = settings
+            .reserved_cycles_limit()
+            .or(canister_reserved_balance_limit);
+        if let Some(limit) = reserved_balance_limit {
+            if canister_reserved_balance + reservation_cycles > limit {
+                return Err(
+                    CanisterManagerError::ReservedCyclesLimitExceededInMemoryAllocation {
+                        memory_allocation: new_memory_allocation,
+                        requested: canister_reserved_balance + reservation_cycles,
+                        limit,
+                    },
+                );
+            }
+        }
+        reservation_cycles
     } else {
         let allocated_bytes = new_memory_bytes - old_memory_bytes;
         let reservation_cycles = cycles_account_manager.storage_reservation_cycles(
diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs
index f5a9fa04f33..b92517a67a3 100644
--- a/rs/execution_environment/src/hypervisor/tests.rs
+++ b/rs/execution_environment/src/hypervisor/tests.rs
@@ -6665,6 +6665,93 @@ fn wasm_memory_grow_reserves_cycles() {
     assert!(balance_before - balance_after > reserved_cycles);
 }
 
+#[test]
+fn set_reserved_cycles_limit_below_existing_fails() {
+    const CYCLES: Cycles = Cycles::new(20_000_000_000_000);
+    const CAPACITY: u64 = 1_000_000_000;
+    const THRESHOLD: u64 = 500_000_000;
+
+    let mut test = ExecutionTestBuilder::new()
+        .with_subnet_execution_memory(CAPACITY as i64)
+        .with_subnet_memory_threshold(THRESHOLD as i64)
+        .with_subnet_memory_reservation(0)
+        .build();
+
+    let wat = r#"
+        (module
+            (import "ic0" "msg_reply" (func $msg_reply))
+            (import "ic0" "msg_reply_data_append"
+                (func $msg_reply_data_append (param i32 i32)))
+            (func $update
+                ;; 7500 Wasm pages is close to 500MB.
+                (if (i32.eq (memory.grow (i32.const 7500)) (i32.const -1))
+                  (then (unreachable))
+                )
+                (call $msg_reply)
+            )
+            (memory $memory 1)
+            (export "canister_update update" (func $update))
+        )"#;
+
+    let wasm = wat::parse_str(wat).unwrap();
+
+    let canister_id = test.canister_from_cycles_and_binary(CYCLES, wasm).unwrap();
+
+    test.update_freezing_threshold(canister_id, NumSeconds::new(0))
+        .unwrap();
+    test.canister_update_reserved_cycles_limit(canister_id, CYCLES)
+        .unwrap();
+
+    let balance_before = test.canister_state(canister_id).system_state.balance();
+    let result = test.ingress(canister_id, "update", vec![]).unwrap();
+    assert_eq!(result, WasmResult::Reply(vec![]));
+    let balance_after = test.canister_state(canister_id).system_state.balance();
+
+    assert_eq!(
+        test.canister_state(canister_id)
+            .system_state
+            .reserved_balance(),
+        Cycles::zero()
+    );
+    // Message execution fee is an order of a few million cycles.
+    assert!(balance_before - balance_after < Cycles::new(1_000_000_000));
+
+    let subnet_memory_usage =
+        CAPACITY - test.subnet_available_memory().get_execution_memory() as u64;
+    let memory_usage_before = test.canister_state(canister_id).execution_memory_usage();
+    let balance_before = test.canister_state(canister_id).system_state.balance();
+    let result = test.ingress(canister_id, "update", vec![]).unwrap();
+    assert_eq!(result, WasmResult::Reply(vec![]));
+    let balance_after = test.canister_state(canister_id).system_state.balance();
+    let memory_usage_after = test.canister_state(canister_id).execution_memory_usage();
+
+    let reserved_cycles = test
+        .canister_state(canister_id)
+        .system_state
+        .reserved_balance();
+
+    assert_eq!(
+        reserved_cycles,
+        test.cycles_account_manager().storage_reservation_cycles(
+            memory_usage_after - memory_usage_before,
+            &ResourceSaturation::new(subnet_memory_usage, THRESHOLD, CAPACITY),
+            test.subnet_size(),
+        )
+    );
+
+    assert!(balance_before - balance_after > reserved_cycles);
+
+    let err = test
+        .canister_update_reserved_cycles_limit(canister_id, Cycles::from(reserved_cycles.get() - 1))
+        .unwrap_err();
+
+    assert_eq!(
+        err.code(),
+        ErrorCode::ReservedCyclesLimitExceededInMemoryAllocation
+    );
+    assert!(err.description().contains("reserved cycles limit"));
+}
+
 #[test]
 fn upgrade_with_skip_pre_upgrade_preserves_stable_memory() {
     let mut test: ExecutionTest = ExecutionTestBuilder::new().build();