Skip to content

Commit 8588e1a

Browse files
committed
Merge branch 'mraszyk/canister-history-populate' into 'master'
[EXC-1180]: Populate canister history with canister changes This MR makes the replica record canister changes in the `CanisterHistory` struct of the canister's `SystemState`. See merge request dfinity-lab/public/ic!11955
2 parents 76e66f7 + 01a9867 commit 8588e1a

File tree

17 files changed

+1102
-63
lines changed

17 files changed

+1102
-63
lines changed

rs/execution_environment/src/canister_manager.rs

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ use ic_config::flag_status::FlagStatus;
1414
use ic_cycles_account_manager::CyclesAccountManager;
1515
use ic_error_types::{ErrorCode, RejectCode, UserError};
1616
use ic_ic00_types::{
17-
CanisterChangeOrigin, CanisterInstallMode, CanisterStatusResultV2, CanisterStatusType,
18-
InstallCodeArgs, Method as Ic00Method,
17+
CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallMode, CanisterStatusResultV2,
18+
CanisterStatusType, InstallCodeArgs, Method as Ic00Method,
1919
};
2020
use ic_interfaces::execution_environment::{
2121
CanisterOutOfCyclesError, HypervisorError, IngressHistoryWriter, SubnetAvailableMemory,
@@ -260,6 +260,12 @@ impl TryFrom<(CanisterChangeOrigin, InstallCodeArgs)> for InstallCodeContext {
260260
}
261261
}
262262

263+
/// Indicates whether `uninstall_canister` should push a canister change (with a given change origin) to canister history.
264+
pub enum AddCanisterChangeToHistory {
265+
Yes(CanisterChangeOrigin),
266+
No,
267+
}
268+
263269
/// The entity responsible for managing canisters (creation, installing, etc.)
264270
pub(crate) struct CanisterManager {
265271
hypervisor: Arc<Hypervisor>,
@@ -469,7 +475,7 @@ impl CanisterManager {
469475
/// `canister_id`.
470476
pub(crate) fn update_settings(
471477
&self,
472-
_timestamp_nanos: Time,
478+
timestamp_nanos: Time,
473479
origin: CanisterChangeOrigin,
474480
settings: CanisterSettings,
475481
canister: &mut CanisterState,
@@ -494,6 +500,8 @@ impl CanisterManager {
494500

495501
let validated_settings =
496502
ValidatedCanisterSettings::try_from((settings, self.config.max_controllers))?;
503+
let is_controllers_change =
504+
validated_settings.controller.is_some() || validated_settings.controllers.is_some();
497505

498506
let old_usage = canister.memory_usage(self.config.own_subnet_type);
499507
let old_mem = canister
@@ -538,6 +546,14 @@ impl CanisterManager {
538546
}
539547

540548
canister.system_state.canister_version += 1;
549+
if is_controllers_change {
550+
let new_controllers = canister.system_state.controllers.iter().copied().collect();
551+
canister.system_state.add_canister_change(
552+
timestamp_nanos,
553+
origin,
554+
CanisterChangeDetails::controllers_change(new_controllers),
555+
);
556+
}
541557

542558
Ok(())
543559
}
@@ -806,7 +822,12 @@ impl CanisterManager {
806822
validate_controller(canister, &sender)?
807823
}
808824

809-
let rejects = uninstall_canister(&self.log, canister, time, Some(origin));
825+
let rejects = uninstall_canister(
826+
&self.log,
827+
canister,
828+
time,
829+
AddCanisterChangeToHistory::Yes(origin),
830+
);
810831
crate::util::process_responses(
811832
rejects,
812833
state,
@@ -1220,6 +1241,19 @@ impl CanisterManager {
12201241
round_limits.compute_allocation_used = round_limits
12211242
.compute_allocation_used
12221243
.saturating_add(new_canister.scheduler_state.compute_allocation.as_percent());
1244+
1245+
let controllers = new_canister
1246+
.system_state
1247+
.controllers
1248+
.iter()
1249+
.copied()
1250+
.collect();
1251+
new_canister.system_state.add_canister_change(
1252+
state.time(),
1253+
origin,
1254+
CanisterChangeDetails::canister_creation(controllers),
1255+
);
1256+
12231257
// Add new canister to the replicated state.
12241258
state.put_canister_state(new_canister);
12251259

@@ -1541,7 +1575,7 @@ pub fn uninstall_canister(
15411575
log: &ReplicaLogger,
15421576
canister: &mut CanisterState,
15431577
time: Time,
1544-
_origin: Option<CanisterChangeOrigin>,
1578+
add_canister_change: AddCanisterChangeToHistory,
15451579
) -> Vec<Response> {
15461580
// Drop the canister's execution state.
15471581
canister.execution_state = None;
@@ -1553,6 +1587,16 @@ pub fn uninstall_canister(
15531587
canister.system_state.global_timer = CanisterTimer::Inactive;
15541588
// Increment canister version.
15551589
canister.system_state.canister_version += 1;
1590+
match add_canister_change {
1591+
AddCanisterChangeToHistory::Yes(origin) => {
1592+
canister.system_state.add_canister_change(
1593+
time,
1594+
origin,
1595+
CanisterChangeDetails::CanisterCodeUninstall,
1596+
);
1597+
}
1598+
AddCanisterChangeToHistory::No => {}
1599+
};
15561600

15571601
let mut rejects = Vec::new();
15581602
let canister_id = canister.canister_id();

rs/execution_environment/src/canister_manager/tests.rs

Lines changed: 65 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::{
22
as_num_instructions,
33
canister_manager::{
4-
uninstall_canister, CanisterManager, CanisterManagerError, CanisterMgrConfig,
5-
InstallCodeContext, StopCanisterResult,
4+
uninstall_canister, AddCanisterChangeToHistory, CanisterManager, CanisterManagerError,
5+
CanisterMgrConfig, InstallCodeContext, StopCanisterResult,
66
},
77
canister_settings::{CanisterSettings, CanisterSettingsBuilder},
88
execution_environment::as_round_instructions,
@@ -20,9 +20,9 @@ use ic_constants::SMALL_APP_SUBNET_MAX_SIZE;
2020
use ic_cycles_account_manager::CyclesAccountManager;
2121
use ic_error_types::{ErrorCode, UserError};
2222
use ic_ic00_types::{
23-
CanisterChangeOrigin, CanisterIdRecord, CanisterInstallMode, CanisterSettingsArgsBuilder,
24-
CanisterStatusType, CreateCanisterArgs, EmptyBlob, InstallCodeArgs, Method, Payload,
25-
UpdateSettingsArgs,
23+
CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, CanisterIdRecord,
24+
CanisterInstallMode, CanisterSettingsArgsBuilder, CanisterStatusType, CreateCanisterArgs,
25+
EmptyBlob, InstallCodeArgs, Method, Payload, UpdateSettingsArgs,
2626
};
2727
use ic_interfaces::{
2828
execution_environment::{
@@ -72,7 +72,7 @@ use ic_types::{
7272
use ic_wasm_types::{CanisterModule, WasmValidationError};
7373
use lazy_static::lazy_static;
7474
use maplit::{btreemap, btreeset};
75-
use std::{collections::BTreeSet, convert::TryFrom, sync::Arc};
75+
use std::{collections::BTreeSet, convert::TryFrom, mem::size_of, sync::Arc};
7676

7777
use super::InstallCodeResult;
7878
use prometheus::IntCounter;
@@ -763,8 +763,16 @@ fn upgrading_canister_makes_subnet_oversubscribed() {
763763
fn install_canister_fails_if_memory_capacity_exceeded() {
764764
let initial_cycles = Cycles::new(1_000_000_000_000_000);
765765
let mb = 1 << 20;
766+
// canister history memory usage for canister1 at the beginning of install_code
767+
let canister_history_memory_usage = size_of::<CanisterChange>() + size_of::<PrincipalId>();
766768
let memory_capacity = 1000 * mb;
767-
let memory_used = memory_capacity - 10 * mb;
769+
// canister1 memory usage before code change: `canister_history_memory_usage`
770+
// canister1 memory usage after code change: `memory_used`
771+
// => SubnetAvailableMemory decreases by `memory_used - canister_history_memory_usage`
772+
// after canister1 code change and then SubnetAvailableMemory is equal to
773+
// `memory_capacity - (memory_used - canister_history_memory_usage)`;
774+
// we want this quantity to be `10 * mb` and derive the value of `memory_used` from there.
775+
let memory_used = memory_capacity - 10 * mb + (canister_history_memory_usage as u64);
768776

769777
let wat = r#"
770778
(module
@@ -2599,8 +2607,12 @@ fn installing_a_canister_with_not_enough_memory_allocation_fails() {
25992607
.0
26002608
.unwrap();
26012609

2602-
// Give just 10 bytes of memory allocation which should result in an error.
2603-
let memory_allocation = MemoryAllocation::try_from(NumBytes::from(10)).unwrap();
2610+
// Give just 10 bytes of memory allocation on top of canister history memory usage
2611+
// at the beginning of install_code which should result in an error.
2612+
let canister_history_memory = size_of::<CanisterChange>() + size_of::<PrincipalId>();
2613+
let memory_allocation =
2614+
MemoryAllocation::try_from(NumBytes::from(canister_history_memory as u64 + 10))
2615+
.unwrap();
26042616
let res = install_code(
26052617
&canister_manager,
26062618
InstallCodeContextBuilder::default()
@@ -2645,7 +2657,12 @@ fn installing_a_canister_with_not_enough_memory_allocation_fails() {
26452657

26462658
// Attempt to re-install with low memory allocation should fail.
26472659
let instructions_before_reinstall = as_num_instructions(round_limits.instructions);
2648-
let memory_allocation = MemoryAllocation::try_from(NumBytes::from(50)).unwrap();
2660+
// Give just 50 bytes of memory allocation on top of canister history memory usage
2661+
// at the beginning of install_code which should result in an error.
2662+
let canister_history_memory = 2 * size_of::<CanisterChange>() + size_of::<PrincipalId>();
2663+
let memory_allocation =
2664+
MemoryAllocation::try_from(NumBytes::from(canister_history_memory as u64 + 50))
2665+
.unwrap();
26492666
let res = install_code(
26502667
&canister_manager,
26512668
InstallCodeContextBuilder::default()
@@ -2849,7 +2866,7 @@ fn uninstall_canister_doesnt_respond_to_responded_call_contexts() {
28492866
.with_call_context(CallContextBuilder::new().with_responded(true).build())
28502867
.build(),
28512868
mock_time(),
2852-
None,
2869+
AddCanisterChangeToHistory::No,
28532870
),
28542871
Vec::new()
28552872
);
@@ -2873,7 +2890,7 @@ fn uninstall_canister_responds_to_unresponded_call_contexts() {
28732890
)
28742891
.build(),
28752892
mock_time(),
2876-
None,
2893+
AddCanisterChangeToHistory::No,
28772894
)[0],
28782895
Response::Ingress(IngressResponse {
28792896
message_id: message_test_id(456),
@@ -3357,6 +3374,7 @@ fn install_code_preserves_system_state_and_scheduler_state() {
33573374
.sender(controller.into())
33583375
.canister_id(canister_id)
33593376
.build();
3377+
let module_hash = ctxt.wasm_module.module_hash();
33603378
let (instructions_left, res, canister) =
33613379
install_code(&canister_manager, ctxt, &mut state, &mut round_limits);
33623380
state.put_canister_state(canister.unwrap());
@@ -3367,7 +3385,8 @@ fn install_code_preserves_system_state_and_scheduler_state() {
33673385
// No heap delta.
33683386
assert_eq!(res.unwrap().heap_delta, NumBytes::from(0));
33693387

3370-
// Verify the system state is preserved except for certified data, global timer, and canister version.
3388+
// Verify the system state is preserved except for certified data, global timer,
3389+
// canister version, and canister history.
33713390
let new_state = state
33723391
.canister_state(&canister_id)
33733392
.unwrap()
@@ -3376,6 +3395,11 @@ fn install_code_preserves_system_state_and_scheduler_state() {
33763395
original_canister.system_state.certified_data = Vec::new();
33773396
original_canister.system_state.global_timer = CanisterTimer::Inactive;
33783397
original_canister.system_state.canister_version += 1;
3398+
original_canister.system_state.add_canister_change(
3399+
state.time(),
3400+
canister_change_origin_from_canister(&controller),
3401+
CanisterChangeDetails::code_deployment(CanisterInstallMode::Install, module_hash),
3402+
);
33793403
assert_eq!(new_state, original_canister.system_state);
33803404

33813405
// Verify the scheduler state is preserved.
@@ -3392,6 +3416,7 @@ fn install_code_preserves_system_state_and_scheduler_state() {
33923416
.sender(controller.into())
33933417
.canister_id(canister_id)
33943418
.build();
3419+
let module_hash = ctxt.wasm_module.module_hash();
33953420
let (instructions_left, res, canister) =
33963421
install_code(&canister_manager, ctxt, &mut state, &mut round_limits);
33973422
state.put_canister_state(canister.unwrap());
@@ -3405,7 +3430,8 @@ fn install_code_preserves_system_state_and_scheduler_state() {
34053430
// No heap delta.
34063431
assert_eq!(res.unwrap().heap_delta, NumBytes::from(0));
34073432

3408-
// Verify the system state is preserved except for certified data, global timer, and canister version.
3433+
// Verify the system state is preserved except for certified data, global timer,
3434+
// canister version, and canister history.
34093435
let new_state = state
34103436
.canister_state(&canister_id)
34113437
.unwrap()
@@ -3414,6 +3440,11 @@ fn install_code_preserves_system_state_and_scheduler_state() {
34143440
original_canister.system_state.certified_data = Vec::new();
34153441
original_canister.system_state.global_timer = CanisterTimer::Inactive;
34163442
original_canister.system_state.canister_version += 1;
3443+
original_canister.system_state.add_canister_change(
3444+
state.time(),
3445+
canister_change_origin_from_canister(&controller),
3446+
CanisterChangeDetails::code_deployment(CanisterInstallMode::Reinstall, module_hash),
3447+
);
34173448
assert_eq!(new_state, original_canister.system_state);
34183449

34193450
// Verify the scheduler state is preserved.
@@ -3436,6 +3467,7 @@ fn install_code_preserves_system_state_and_scheduler_state() {
34363467
.sender(controller.into())
34373468
.canister_id(canister_id)
34383469
.build();
3470+
let module_hash = ctxt.wasm_module.module_hash();
34393471
let (instructions_left, res, canister) =
34403472
install_code(&canister_manager, ctxt, &mut state, &mut round_limits);
34413473
state.put_canister_state(canister.unwrap());
@@ -3449,14 +3481,20 @@ fn install_code_preserves_system_state_and_scheduler_state() {
34493481
// No heap delta.
34503482
assert_eq!(res.unwrap().heap_delta, NumBytes::from(0));
34513483

3452-
// Verify the system state is preserved except for global timer and canister version.
3484+
// Verify the system state is preserved except for global timer,
3485+
// canister version, and canister history.
34533486
let new_state = state
34543487
.canister_state(&canister_id)
34553488
.unwrap()
34563489
.system_state
34573490
.clone();
34583491
original_canister.system_state.global_timer = CanisterTimer::Inactive;
34593492
original_canister.system_state.canister_version += 1;
3493+
original_canister.system_state.add_canister_change(
3494+
state.time(),
3495+
canister_change_origin_from_canister(&controller),
3496+
CanisterChangeDetails::code_deployment(CanisterInstallMode::Upgrade, module_hash),
3497+
);
34603498
assert_eq!(new_state, original_canister.system_state);
34613499

34623500
// Verify the scheduler state is preserved.
@@ -3634,9 +3672,14 @@ fn test_upgrade_when_updating_memory_allocation_via_canister_settings() {
36343672
compute_allocation_used: state.total_compute_allocation(),
36353673
};
36363674
let sender = canister_test_id(100).get();
3675+
// canister history memory usage at the beginning of attempted upgrade
3676+
let canister_history_memory = 2 * size_of::<CanisterChange>() + size_of::<PrincipalId>();
36373677
let settings = CanisterSettingsBuilder::new()
36383678
.with_memory_allocation(
3639-
MemoryAllocation::try_from(NumBytes::from(WASM_PAGE_SIZE_IN_BYTES + 100)).unwrap(),
3679+
MemoryAllocation::try_from(NumBytes::from(
3680+
WASM_PAGE_SIZE_IN_BYTES + 100 + canister_history_memory as u64,
3681+
))
3682+
.unwrap(),
36403683
)
36413684
.build();
36423685
let wat = r#"
@@ -3705,12 +3748,16 @@ fn test_upgrade_when_updating_memory_allocation_via_canister_settings() {
37053748
);
37063749
state.put_canister_state(res.2.unwrap());
37073750

3751+
// canister history memory usage at the beginning of update_settings
3752+
let canister_history_memory = 2 * size_of::<CanisterChange>() + size_of::<PrincipalId>();
37083753
// Update memory allocation to a big enough value via canister settings. The
37093754
// upgrade should succeed.
37103755
let settings = CanisterSettingsBuilder::new()
37113756
.with_memory_allocation(
3712-
MemoryAllocation::try_from(NumBytes::from(WASM_PAGE_SIZE_IN_BYTES * 2 + 100))
3713-
.unwrap(),
3757+
MemoryAllocation::try_from(NumBytes::from(
3758+
WASM_PAGE_SIZE_IN_BYTES * 2 + 100 + canister_history_memory as u64,
3759+
))
3760+
.unwrap(),
37143761
)
37153762
.build();
37163763

rs/execution_environment/src/execution/install.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ pub(crate) fn execute_install(
8686
let canister_id = helper.canister().canister_id();
8787
let layout = canister_layout(&original.canister_layout_path, &canister_id);
8888
let context_sender = context.sender();
89+
let module_hash = context.wasm_module.module_hash();
8990
let (instructions_from_compilation, result) = round.hypervisor.create_execution_state(
9091
context.wasm_module,
9192
layout.raw_path(),
@@ -105,6 +106,7 @@ pub(crate) fn execute_install(
105106
helper.clear_certified_data();
106107
helper.deactivate_global_timer();
107108
helper.bump_canister_version();
109+
helper.add_canister_change(round.time, context.origin, context.mode, module_hash.into());
108110

109111
// Stage 2: invoke the `start()` method of the Wasm module (if present).
110112
let method = WasmMethod::System(SystemMethod::CanisterStart);

rs/execution_environment/src/execution/install_code.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::path::{Path, PathBuf};
66
use ic_base_types::{CanisterId, NumBytes, PrincipalId};
77
use ic_config::flag_status::FlagStatus;
88
use ic_embedders::wasm_executor::CanisterStateChanges;
9-
use ic_ic00_types::CanisterInstallMode;
9+
use ic_ic00_types::{CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallMode};
1010
use ic_interfaces::{
1111
execution_environment::{
1212
HypervisorError, HypervisorResult, SubnetAvailableMemory, SubnetAvailableMemoryError,
@@ -23,6 +23,7 @@ use ic_types::{
2323
funds::Cycles, CanisterTimer, ComputeAllocation, Height, MemoryAllocation, NumInstructions,
2424
Time,
2525
};
26+
use ic_wasm_types::WasmHash;
2627

2728
use crate::{
2829
canister_manager::{
@@ -121,6 +122,20 @@ impl InstallCodeHelper {
121122
self.canister.system_state.canister_version += 1;
122123
}
123124

125+
pub fn add_canister_change(
126+
&mut self,
127+
timestamp_nanos: Time,
128+
origin: CanisterChangeOrigin,
129+
mode: CanisterInstallMode,
130+
module_hash: WasmHash,
131+
) {
132+
self.canister.system_state.add_canister_change(
133+
timestamp_nanos,
134+
origin,
135+
CanisterChangeDetails::code_deployment(mode, module_hash.to_slice()),
136+
);
137+
}
138+
124139
pub fn execution_parameters(&self) -> &ExecutionParameters {
125140
&self.execution_parameters
126141
}

0 commit comments

Comments
 (0)