Skip to content

Commit

Permalink
[hotfix]: Prevent lowering Cycles reservation limit below existing
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
adambratschikaye authored and DFINITYManu committed Jun 24, 2024
1 parent e3fca54 commit 9c006a5
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 1 deletion.
17 changes: 16 additions & 1 deletion rs/execution_environment/src/canister_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
87 changes: 87 additions & 0 deletions rs/execution_environment/src/hypervisor/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 9c006a5

Please sign in to comment.