diff --git a/rs/canonical_state/tests/hash_tree.rs b/rs/canonical_state/tests/hash_tree.rs index 1c58f9b92fe..8d9788fe9c1 100644 --- a/rs/canonical_state/tests/hash_tree.rs +++ b/rs/canonical_state/tests/hash_tree.rs @@ -197,8 +197,8 @@ fn error_code_change_guard() { // the name. assert_eq!( [ - 168, 93, 103, 26, 25, 115, 47, 212, 112, 195, 250, 97, 212, 98, 80, 98, 182, 207, 148, - 106, 249, 63, 178, 91, 255, 255, 186, 84, 195, 222, 206, 80 + 49, 160, 44, 253, 61, 181, 131, 134, 148, 115, 119, 247, 83, 108, 123, 1, 163, 212, + 137, 11, 99, 122, 199, 16, 89, 105, 87, 234, 200, 211, 76, 96 ], hasher.finish() ); diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 2d539342d0a..2f700563acf 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -2249,6 +2249,13 @@ pub(crate) enum CanisterManagerError { requested: Cycles, limit: Cycles, }, + // TODO(RUN-1001): Use this error type after successful rollout of the next + // replica version. + #[allow(dead_code)] + ReservedCyclesLimitIsTooLow { + cycles: Cycles, + limit: Cycles, + }, WasmChunkStoreError { message: String, }, @@ -2306,6 +2313,7 @@ impl AsErrorHelp for CanisterManagerError { | CanisterManagerError::InsufficientCyclesInMemoryGrow { .. } | CanisterManagerError::ReservedCyclesLimitExceededInMemoryAllocation { .. } | CanisterManagerError::ReservedCyclesLimitExceededInMemoryGrow { .. } + | CanisterManagerError::ReservedCyclesLimitIsTooLow { .. } | CanisterManagerError::WasmChunkStoreError { .. } | CanisterManagerError::CanisterSnapshotNotFound { .. } | CanisterManagerError::CanisterHeapDeltaRateLimited { .. } @@ -2553,6 +2561,16 @@ impl From for UserError { ), ) } + ReservedCyclesLimitIsTooLow { cycles, limit } => { + Self::new( + ErrorCode::ReservedCyclesLimitIsTooLow, + format!( + "Cannot set the reserved cycles limit {} below the reserved cycles balance of \ + the canister {}.{additional_help}", + limit, cycles, + ), + ) + } WasmChunkStoreError { message } => { Self::new( ErrorCode::CanisterContractViolation, diff --git a/rs/execution_environment/src/canister_settings.rs b/rs/execution_environment/src/canister_settings.rs index 8d3cf9c5613..316fc856fa1 100644 --- a/rs/execution_environment/src/canister_settings.rs +++ b/rs/execution_environment/src/canister_settings.rs @@ -519,43 +519,47 @@ pub(crate) fn validate_canister_settings( } } - let reservation_cycles = if new_memory_bytes <= old_memory_bytes { - Cycles::zero() + let allocated_bytes = if new_memory_bytes > old_memory_bytes { + new_memory_bytes - old_memory_bytes } else { - let allocated_bytes = new_memory_bytes - old_memory_bytes; - let reservation_cycles = cycles_account_manager.storage_reservation_cycles( - allocated_bytes, - subnet_memory_saturation, - subnet_size, - ); - 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, - }, - ); - } - } - // Note that this check does not include the freezing threshold to be - // consistent with the `reserve_cycles()` function, which moves - // cycles between the main and reserved balances without checking - // the freezing threshold. - if canister_cycles_balance < reservation_cycles { - return Err(CanisterManagerError::InsufficientCyclesInMemoryAllocation { - memory_allocation: new_memory_allocation, - available: canister_cycles_balance, - threshold: reservation_cycles, - }); - } - reservation_cycles + NumBytes::new(0) }; + let reservation_cycles = cycles_account_manager.storage_reservation_cycles( + allocated_bytes, + subnet_memory_saturation, + subnet_size, + ); + let reserved_balance_limit = settings + .reserved_cycles_limit() + .or(canister_reserved_balance_limit); + + if let Some(limit) = reserved_balance_limit { + // TODO(RUN-1001): return `ReservedCyclesLimitIsTooLow` once + // the replica with that error type rolls out successfully. + if canister_reserved_balance + reservation_cycles > limit { + return Err( + CanisterManagerError::ReservedCyclesLimitExceededInMemoryAllocation { + memory_allocation: new_memory_allocation, + requested: canister_reserved_balance + reservation_cycles, + limit, + }, + ); + } + } + + // Note that this check does not include the freezing threshold to be + // consistent with the `reserve_cycles()` function, which moves + // cycles between the main and reserved balances without checking + // the freezing threshold. + if canister_cycles_balance < reservation_cycles { + return Err(CanisterManagerError::InsufficientCyclesInMemoryAllocation { + memory_allocation: new_memory_allocation, + available: canister_cycles_balance, + threshold: reservation_cycles, + }); + } + Ok(ValidatedCanisterSettings { controller: settings.controller(), controllers: settings.controllers(), diff --git a/rs/execution_environment/src/history.rs b/rs/execution_environment/src/history.rs index 6af46d6c3ea..0b666e37401 100644 --- a/rs/execution_environment/src/history.rs +++ b/rs/execution_environment/src/history.rs @@ -330,6 +330,9 @@ fn dashboard_label_value_from(code: ErrorCode) -> &'static str { ReservedCyclesLimitExceededInMemoryGrow => { "Canister cannot grow memory due to its reserved cycles limit" } + ReservedCyclesLimitIsTooLow => { + "Canister cannot set the reserved cycles limit below the reserved cycles balance" + } InsufficientCyclesInMessageMemoryGrow => { "Canister does not have enough cycles to grow message memory" } diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index 13db548cb83..c5ab0ff67e5 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -6670,6 +6670,92 @@ 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 + ); +} + #[test] fn upgrade_with_skip_pre_upgrade_preserves_stable_memory() { let mut test: ExecutionTest = ExecutionTestBuilder::new().build(); diff --git a/rs/protobuf/def/state/ingress/v1/ingress.proto b/rs/protobuf/def/state/ingress/v1/ingress.proto index a16689b4684..3e4dcf0921c 100644 --- a/rs/protobuf/def/state/ingress/v1/ingress.proto +++ b/rs/protobuf/def/state/ingress/v1/ingress.proto @@ -93,6 +93,7 @@ enum ErrorCode { ERROR_CODE_CANISTER_WASM_MODULE_NOT_FOUND = 537; ERROR_CODE_CANISTER_ALREADY_INSTALLED = 538; ERROR_CODE_CANISTER_WASM_MEMORY_LIMIT_EXCEEDED = 539; + ERROR_CODE_RESERVED_CYCLES_LIMIT_IS_TOO_LOW = 540; } message IngressStatusFailed { diff --git a/rs/protobuf/src/gen/state/state.ingress.v1.rs b/rs/protobuf/src/gen/state/state.ingress.v1.rs index 68eb2312989..7c659b73ba0 100644 --- a/rs/protobuf/src/gen/state/state.ingress.v1.rs +++ b/rs/protobuf/src/gen/state/state.ingress.v1.rs @@ -203,6 +203,7 @@ pub enum ErrorCode { CanisterWasmModuleNotFound = 537, CanisterAlreadyInstalled = 538, CanisterWasmMemoryLimitExceeded = 539, + ReservedCyclesLimitIsTooLow = 540, } impl ErrorCode { /// String value of the enum field names used in the ProtoBuf definition. @@ -294,6 +295,7 @@ impl ErrorCode { ErrorCode::CanisterWasmMemoryLimitExceeded => { "ERROR_CODE_CANISTER_WASM_MEMORY_LIMIT_EXCEEDED" } + ErrorCode::ReservedCyclesLimitIsTooLow => "ERROR_CODE_RESERVED_CYCLES_LIMIT_IS_TOO_LOW", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -382,6 +384,9 @@ impl ErrorCode { "ERROR_CODE_CANISTER_WASM_MEMORY_LIMIT_EXCEEDED" => { Some(Self::CanisterWasmMemoryLimitExceeded) } + "ERROR_CODE_RESERVED_CYCLES_LIMIT_IS_TOO_LOW" => { + Some(Self::ReservedCyclesLimitIsTooLow) + } _ => None, } } diff --git a/rs/protobuf/src/gen/types/state.ingress.v1.rs b/rs/protobuf/src/gen/types/state.ingress.v1.rs index 68eb2312989..7c659b73ba0 100644 --- a/rs/protobuf/src/gen/types/state.ingress.v1.rs +++ b/rs/protobuf/src/gen/types/state.ingress.v1.rs @@ -203,6 +203,7 @@ pub enum ErrorCode { CanisterWasmModuleNotFound = 537, CanisterAlreadyInstalled = 538, CanisterWasmMemoryLimitExceeded = 539, + ReservedCyclesLimitIsTooLow = 540, } impl ErrorCode { /// String value of the enum field names used in the ProtoBuf definition. @@ -294,6 +295,7 @@ impl ErrorCode { ErrorCode::CanisterWasmMemoryLimitExceeded => { "ERROR_CODE_CANISTER_WASM_MEMORY_LIMIT_EXCEEDED" } + ErrorCode::ReservedCyclesLimitIsTooLow => "ERROR_CODE_RESERVED_CYCLES_LIMIT_IS_TOO_LOW", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -382,6 +384,9 @@ impl ErrorCode { "ERROR_CODE_CANISTER_WASM_MEMORY_LIMIT_EXCEEDED" => { Some(Self::CanisterWasmMemoryLimitExceeded) } + "ERROR_CODE_RESERVED_CYCLES_LIMIT_IS_TOO_LOW" => { + Some(Self::ReservedCyclesLimitIsTooLow) + } _ => None, } } diff --git a/rs/types/error_types/src/lib.rs b/rs/types/error_types/src/lib.rs index 6184f5b0a61..b32fe1a0e2d 100644 --- a/rs/types/error_types/src/lib.rs +++ b/rs/types/error_types/src/lib.rs @@ -150,6 +150,7 @@ impl From for RejectCode { InsufficientCyclesInMemoryGrow => CanisterError, ReservedCyclesLimitExceededInMemoryAllocation => CanisterError, ReservedCyclesLimitExceededInMemoryGrow => CanisterError, + ReservedCyclesLimitIsTooLow => CanisterError, InsufficientCyclesInMessageMemoryGrow => CanisterError, CanisterMethodNotFound => CanisterError, CanisterWasmModuleNotFound => CanisterError, @@ -229,6 +230,7 @@ pub enum ErrorCode { CanisterWasmModuleNotFound = 537, CanisterAlreadyInstalled = 538, CanisterWasmMemoryLimitExceeded = 539, + ReservedCyclesLimitIsTooLow = 540, } impl TryFrom for ErrorCode { @@ -325,6 +327,9 @@ impl TryFrom for ErrorCode { ErrorCodeProto::CanisterWasmMemoryLimitExceeded => { Ok(ErrorCode::CanisterWasmMemoryLimitExceeded) } + ErrorCodeProto::ReservedCyclesLimitIsTooLow => { + Ok(ErrorCode::ReservedCyclesLimitIsTooLow) + } } } } @@ -412,6 +417,7 @@ impl From for ErrorCodeProto { ErrorCode::CanisterWasmMemoryLimitExceeded => { ErrorCodeProto::CanisterWasmMemoryLimitExceeded } + ErrorCode::ReservedCyclesLimitIsTooLow => ErrorCodeProto::ReservedCyclesLimitIsTooLow, } } } @@ -528,6 +534,7 @@ impl UserError { | ErrorCode::InsufficientCyclesInMemoryGrow | ErrorCode::ReservedCyclesLimitExceededInMemoryAllocation | ErrorCode::ReservedCyclesLimitExceededInMemoryGrow + | ErrorCode::ReservedCyclesLimitIsTooLow | ErrorCode::InsufficientCyclesInMessageMemoryGrow | ErrorCode::CanisterSnapshotNotFound | ErrorCode::CanisterHeapDeltaRateLimited @@ -589,7 +596,7 @@ mod tests { 402, 403, 404, 405, 406, 407, 408, 502, 503, 504, 505, 506, 507, 508, 509, 510, 511, 512, 513, 514, 517, 520, 521, 522, 524, 525, 526, 527, 528, 529, 530, 531, 532, - 533, 534, 535, 536, 537, 538, 539 + 533, 534, 535, 536, 537, 538, 539, 540, ] ); }