From f040350513236a4781df303cf362b9d1c2f38c7a Mon Sep 17 00:00:00 2001 From: Maksym Arutyunyan <103510076+maksymar@users.noreply.github.com> Date: Fri, 9 Aug 2024 17:01:55 +0200 Subject: [PATCH] feat: [EXC-1675] migrate replica to no-op LogVisibilityV2 (#768) This PR migrates replica to using a no-op `LogVisibilityV2` instead of `LogVisibility`. The difference between the two is in `AllowedViewers` enum variant which is currently ignored, so it's expected for V2 to behave the same way as V1. --- .../src/canister_settings.rs | 16 +-- .../src/execution_environment/tests.rs | 14 +-- rs/execution_environment/src/query_handler.rs | 23 ++++- .../tests/canister_logging.rs | 72 +++++++++----- rs/nns/cmc/src/lib.rs | 97 ++++++++++++++++++- rs/nns/cmc/src/main.rs | 16 +-- .../src/cycles_minting_canister.rs | 14 +-- rs/replica_tests/tests/canister_lifecycle.rs | 6 +- rs/replicated_state/src/canister_state.rs | 4 +- .../src/canister_state/system_state.rs | 8 +- .../src/manage_dapp_canister_settings.rs | 12 +-- rs/state_layout/src/state_layout.rs | 18 ++-- rs/state_layout/src/state_layout/tests.rs | 5 +- .../execution_environment/src/lib.rs | 4 +- rs/test_utilities/state/src/lib.rs | 10 +- rs/types/management_canister_types/src/lib.rs | 28 +++--- 16 files changed, 244 insertions(+), 103 deletions(-) diff --git a/rs/execution_environment/src/canister_settings.rs b/rs/execution_environment/src/canister_settings.rs index a446b8e6b14..7dfd825d08f 100644 --- a/rs/execution_environment/src/canister_settings.rs +++ b/rs/execution_environment/src/canister_settings.rs @@ -2,7 +2,7 @@ use ic_base_types::{NumBytes, NumSeconds}; use ic_cycles_account_manager::{CyclesAccountManager, ResourceSaturation}; use ic_error_types::{ErrorCode, UserError}; use ic_interfaces::execution_environment::SubnetAvailableMemory; -use ic_management_canister_types::{CanisterSettingsArgs, LogVisibility}; +use ic_management_canister_types::{CanisterSettingsArgs, LogVisibilityV2}; use ic_types::{ ComputeAllocation, Cycles, InvalidComputeAllocationError, InvalidMemoryAllocationError, MemoryAllocation, PrincipalId, @@ -25,7 +25,7 @@ pub(crate) struct CanisterSettings { pub(crate) wasm_memory_threshold: Option, pub(crate) freezing_threshold: Option, pub(crate) reserved_cycles_limit: Option, - pub(crate) log_visibility: Option, + pub(crate) log_visibility: Option, pub(crate) wasm_memory_limit: Option, } @@ -37,7 +37,7 @@ impl CanisterSettings { wasm_memory_threshold: Option, freezing_threshold: Option, reserved_cycles_limit: Option, - log_visibility: Option, + log_visibility: Option, wasm_memory_limit: Option, ) -> Self { Self { @@ -76,7 +76,7 @@ impl CanisterSettings { self.reserved_cycles_limit } - pub fn log_visibility(&self) -> Option<&LogVisibility> { + pub fn log_visibility(&self) -> Option<&LogVisibilityV2> { self.log_visibility.as_ref() } @@ -179,7 +179,7 @@ pub(crate) struct CanisterSettingsBuilder { wasm_memory_threshold: Option, freezing_threshold: Option, reserved_cycles_limit: Option, - log_visibility: Option, + log_visibility: Option, wasm_memory_limit: Option, } @@ -253,7 +253,7 @@ impl CanisterSettingsBuilder { } } - pub fn with_log_visibility(self, log_visibility: LogVisibility) -> Self { + pub fn with_log_visibility(self, log_visibility: LogVisibilityV2) -> Self { Self { log_visibility: Some(log_visibility), ..self @@ -348,7 +348,7 @@ pub(crate) struct ValidatedCanisterSettings { freezing_threshold: Option, reserved_cycles_limit: Option, reservation_cycles: Cycles, - log_visibility: Option, + log_visibility: Option, wasm_memory_limit: Option, } @@ -381,7 +381,7 @@ impl ValidatedCanisterSettings { self.reservation_cycles } - pub fn log_visibility(&self) -> Option<&LogVisibility> { + pub fn log_visibility(&self) -> Option<&LogVisibilityV2> { self.log_visibility.as_ref() } diff --git a/rs/execution_environment/src/execution_environment/tests.rs b/rs/execution_environment/src/execution_environment/tests.rs index c2237988d72..39e1611f080 100644 --- a/rs/execution_environment/src/execution_environment/tests.rs +++ b/rs/execution_environment/src/execution_environment/tests.rs @@ -7,7 +7,7 @@ use ic_management_canister_types::{ self as ic00, BitcoinGetUtxosArgs, BitcoinNetwork, BoundedHttpHeaders, CanisterChange, CanisterHttpRequestArgs, CanisterIdRecord, CanisterStatusResultV2, CanisterStatusType, DerivationPath, EcdsaCurve, EcdsaKeyId, EmptyBlob, FetchCanisterLogsRequest, HttpMethod, - LogVisibility, MasterPublicKeyId, Method, Payload as Ic00Payload, + LogVisibilityV2, MasterPublicKeyId, Method, Payload as Ic00Payload, ProvisionalCreateCanisterWithCyclesArgs, ProvisionalTopUpCanisterArgs, SchnorrAlgorithm, SchnorrKeyId, TakeCanisterSnapshotArgs, TransformContext, TransformFunc, IC_00, }; @@ -3524,7 +3524,7 @@ fn test_canister_settings_log_visibility_default_controllers() { // Assert. assert_eq!( canister_status.settings().log_visibility(), - &LogVisibility::Controllers + &LogVisibilityV2::Controllers ); } @@ -3537,7 +3537,7 @@ fn test_canister_settings_log_visibility_create_with_settings() { .create_canister_with_settings( Cycles::new(1_000_000_000), ic00::CanisterSettingsArgsBuilder::new() - .with_log_visibility(LogVisibility::Public) + .with_log_visibility(LogVisibilityV2::Public) .build(), ) .unwrap(); @@ -3546,7 +3546,7 @@ fn test_canister_settings_log_visibility_create_with_settings() { // Assert. assert_eq!( canister_status.settings().log_visibility(), - &LogVisibility::Public + &LogVisibilityV2::Public ); } @@ -3556,14 +3556,14 @@ fn test_canister_settings_log_visibility_set_to_public() { let mut test = ExecutionTestBuilder::new().build(); let canister_id = test.create_canister(Cycles::new(1_000_000_000)); // Act. - test.set_log_visibility(canister_id, LogVisibility::Public) + test.set_log_visibility(canister_id, LogVisibilityV2::Public) .unwrap(); let result = test.canister_status(canister_id); let canister_status = CanisterStatusResultV2::decode(&get_reply(result)).unwrap(); // Assert. assert_eq!( canister_status.settings().log_visibility(), - &LogVisibility::Public + &LogVisibilityV2::Public ); } @@ -3574,7 +3574,7 @@ fn test_fetch_canister_logs_should_accept_ingress_message() { let mut test = ExecutionTestBuilder::new().build(); let canister_id = test.universal_canister().unwrap(); let not_a_controller = user_test_id(42); - test.set_log_visibility(canister_id, LogVisibility::Public) + test.set_log_visibility(canister_id, LogVisibilityV2::Public) .unwrap(); // Act. test.set_user_id(not_a_controller); diff --git a/rs/execution_environment/src/query_handler.rs b/rs/execution_environment/src/query_handler.rs index 30e4b2d395f..99c4060f574 100644 --- a/rs/execution_environment/src/query_handler.rs +++ b/rs/execution_environment/src/query_handler.rs @@ -50,7 +50,7 @@ use tower::{util::BoxCloneService, Service}; pub(crate) use self::query_scheduler::{QueryScheduler, QuerySchedulerFlag}; use ic_management_canister_types::{ - FetchCanisterLogsRequest, FetchCanisterLogsResponse, LogVisibility, Payload, QueryMethod, + FetchCanisterLogsRequest, FetchCanisterLogsResponse, LogVisibilityV2, Payload, QueryMethod, }; /// Convert an object into CBOR binary. @@ -282,6 +282,10 @@ impl InternalHttpQueryHandler { } } +// TODO(EXC-1678): remove after release. +/// Feature flag to enable/disable allowed viewers for canister log visibility. +const ALLOWED_VIEWERS_ENABLED: bool = false; + fn fetch_canister_logs( sender: PrincipalId, state: &ReplicatedState, @@ -295,10 +299,19 @@ fn fetch_canister_logs( ) })?; - match canister.log_visibility() { - LogVisibility::Public => Ok(()), - LogVisibility::Controllers if canister.controllers().contains(&sender) => Ok(()), - LogVisibility::Controllers => Err(UserError::new( + let log_visibility = match canister.log_visibility() { + // If the feature is disabled override `AllowedViewers` with default value. + LogVisibilityV2::AllowedViewers(_) if !ALLOWED_VIEWERS_ENABLED => { + &LogVisibilityV2::default() + } + other => other, + }; + match log_visibility { + LogVisibilityV2::Public => Ok(()), + LogVisibilityV2::Controllers if canister.controllers().contains(&sender) => Ok(()), + LogVisibilityV2::AllowedViewers(principals) if principals.get().contains(&sender) => Ok(()), + LogVisibilityV2::AllowedViewers(_) if canister.controllers().contains(&sender) => Ok(()), + LogVisibilityV2::AllowedViewers(_) | LogVisibilityV2::Controllers => Err(UserError::new( ErrorCode::CanisterRejectedMessage, format!( "Caller {} is not allowed to query ic00 method {}", diff --git a/rs/execution_environment/tests/canister_logging.rs b/rs/execution_environment/tests/canister_logging.rs index 4ffa2119345..13342e94479 100644 --- a/rs/execution_environment/tests/canister_logging.rs +++ b/rs/execution_environment/tests/canister_logging.rs @@ -1,9 +1,9 @@ use ic_config::execution_environment::Config as ExecutionConfig; use ic_config::subnet_config::SubnetConfig; use ic_management_canister_types::{ - self as ic00, CanisterIdRecord, CanisterInstallMode, CanisterLogRecord, CanisterSettingsArgs, - CanisterSettingsArgsBuilder, DataSize, EmptyBlob, FetchCanisterLogsRequest, - FetchCanisterLogsResponse, LogVisibility, Payload, + self as ic00, BoundedAllowedViewers, CanisterIdRecord, CanisterInstallMode, CanisterLogRecord, + CanisterSettingsArgs, CanisterSettingsArgsBuilder, DataSize, EmptyBlob, + FetchCanisterLogsRequest, FetchCanisterLogsResponse, LogVisibilityV2, Payload, }; use ic_registry_subnet_type::SubnetType; use ic_state_machine_tests::{ @@ -79,7 +79,7 @@ fn setup_with_controller(wasm: Vec) -> (StateMachine, CanisterId, PrincipalI let controller = PrincipalId::new_user_test_id(42); let (env, canister_id) = setup_and_install_wasm( CanisterSettingsArgsBuilder::new() - .with_log_visibility(LogVisibility::Controllers) + .with_log_visibility(LogVisibilityV2::Controllers) .with_controllers(vec![controller]) .build(), wasm, @@ -111,7 +111,7 @@ fn fetch_canister_logs( fn test_fetch_canister_logs_via_submit_ingress() { let (env, canister_id) = setup_and_install_wasm( CanisterSettingsArgsBuilder::new() - .with_log_visibility(LogVisibility::Public) + .with_log_visibility(LogVisibilityV2::Public) .build(), wat_canister().build_wasm(), ); @@ -135,7 +135,7 @@ fn test_fetch_canister_logs_via_execute_ingress() { // Test fetch_canister_logs API call results. let (env, canister_id) = setup_and_install_wasm( CanisterSettingsArgsBuilder::new() - .with_log_visibility(LogVisibility::Public) + .with_log_visibility(LogVisibilityV2::Public) .build(), wat_canister().build_wasm(), ); @@ -159,7 +159,7 @@ fn test_fetch_canister_logs_via_query_call() { // Test fetch_canister_logs API call results. let (env, canister_id) = setup_and_install_wasm( CanisterSettingsArgsBuilder::new() - .with_log_visibility(LogVisibility::Public) + .with_log_visibility(LogVisibilityV2::Public) .build(), wat_canister().build_wasm(), ); @@ -230,37 +230,63 @@ fn test_fetch_canister_logs_via_composite_query_call() { #[test] fn test_log_visibility_of_fetch_canister_logs() { // Test combinations of log_visibility and sender for fetch_canister_logs API call. - let controller = PrincipalId::new_user_test_id(27); - let not_a_controller = PrincipalId::new_user_test_id(42); + let controller = PrincipalId::new_user_test_id(1); + let not_a_controller = PrincipalId::new_user_test_id(2); + let allowed_viewer = PrincipalId::new_user_test_id(3); + let not_allowed_viewer = PrincipalId::new_user_test_id(4); + let allowed_viewers = BoundedAllowedViewers::new(vec![allowed_viewer]); let ok = Ok(WasmResult::Reply( FetchCanisterLogsResponse { canister_log_records: vec![], } .encode(), )); - let error = Err(UserError::new( - ErrorCode::CanisterRejectedMessage, - format!( - "Caller {not_a_controller} is not allowed to query ic00 method fetch_canister_logs" - ), - )); + fn not_allowed_error(caller: &PrincipalId) -> Result { + Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!("Caller {caller} is not allowed to query ic00 method fetch_canister_logs"), + )) + } let test_cases = vec![ // (log_visibility, sender, expected_result) - (LogVisibility::Public, controller, ok.clone()), - (LogVisibility::Public, not_a_controller, ok.clone()), - (LogVisibility::Controllers, controller, ok), - (LogVisibility::Controllers, not_a_controller, error), + (LogVisibilityV2::Public, controller, ok.clone()), + (LogVisibilityV2::Public, not_a_controller, ok.clone()), + (LogVisibilityV2::Controllers, controller, ok.clone()), + ( + LogVisibilityV2::Controllers, + not_a_controller, + not_allowed_error(¬_a_controller), + ), + ( + LogVisibilityV2::AllowedViewers(allowed_viewers.clone()), + allowed_viewer, + // TODO(EXC-1675): when disabled works as for controllers, change to ok when enabled. + not_allowed_error(&allowed_viewer), + ), + ( + LogVisibilityV2::AllowedViewers(allowed_viewers.clone()), + not_allowed_viewer, + not_allowed_error(¬_allowed_viewer), + ), + ( + LogVisibilityV2::AllowedViewers(allowed_viewers), + controller, + ok, + ), ]; for (log_visibility, sender, expected_result) in test_cases { let (env, canister_id) = setup_and_install_wasm( CanisterSettingsArgsBuilder::new() - .with_log_visibility(log_visibility) + .with_log_visibility(log_visibility.clone()) .with_controllers(vec![controller]) .build(), wat_canister().build_wasm(), ); let actual_result = fetch_canister_logs(&env, sender, canister_id); - assert_eq!(actual_result, expected_result); + assert_eq!( + actual_result, expected_result, + "Failed for log_visibility: {log_visibility:?}, sender: {sender}" + ); } } @@ -730,7 +756,7 @@ fn test_logging_debug_print_persists_over_upgrade() { fn test_logging_trap_at_install_start() { let (env, canister_id) = setup( CanisterSettingsArgsBuilder::new() - .with_log_visibility(LogVisibility::Public) + .with_log_visibility(LogVisibilityV2::Public) .build(), ); env.advance_time(TIME_STEP); @@ -763,7 +789,7 @@ fn test_logging_trap_at_install_start() { fn test_logging_trap_at_install_init() { let (env, canister_id) = setup( CanisterSettingsArgsBuilder::new() - .with_log_visibility(LogVisibility::Public) + .with_log_visibility(LogVisibilityV2::Public) .build(), ); env.advance_time(TIME_STEP); diff --git a/rs/nns/cmc/src/lib.rs b/rs/nns/cmc/src/lib.rs index 719a031ffa4..ee893f35876 100644 --- a/rs/nns/cmc/src/lib.rs +++ b/rs/nns/cmc/src/lib.rs @@ -1,5 +1,8 @@ use candid::{CandidType, Nat}; -use ic_management_canister_types::CanisterSettingsArgs; +// TODO(EXC-1687): remove temporary alias `Ic00CanisterSettingsArgs`. +use ic_management_canister_types::{ + BoundedControllers, CanisterSettingsArgs as Ic00CanisterSettingsArgs, LogVisibilityV2, +}; use ic_nns_common::types::UpdateIcpXdrConversionRatePayload; use ic_types::{CanisterId, Cycles, PrincipalId, SubnetId}; use ic_xrc_types::ExchangeRate; @@ -65,6 +68,98 @@ pub struct NotifyTopUp { pub canister_id: CanisterId, } +// TODO(EXC-1670): remove after migration to `LogVisibilityV2`. +/// Log visibility for a canister. +/// ```text +/// variant { +/// controllers; +/// public; +/// } +/// ``` +#[derive(Default, Clone, CandidType, Deserialize, Debug, PartialEq, Eq)] +pub enum LogVisibility { + #[default] + #[serde(rename = "controllers")] + Controllers = 1, + #[serde(rename = "public")] + Public = 2, +} + +impl From for LogVisibilityV2 { + fn from(log_visibility: LogVisibility) -> Self { + match log_visibility { + LogVisibility::Controllers => Self::Controllers, + LogVisibility::Public => Self::Public, + } + } +} + +impl From for LogVisibility { + fn from(log_visibility: LogVisibilityV2) -> Self { + match log_visibility { + LogVisibilityV2::Controllers => Self::Controllers, + LogVisibilityV2::Public => Self::Public, + LogVisibilityV2::AllowedViewers(_) => Self::default(), + } + } +} + +// TODO(EXC-1687): remove temporary copy of management canister types. +// It was added to overcome dependency on `LogVisibility` while +// management canister already migrated to `LogVisibilityV2`. +/// Struct used for encoding/decoding +/// `(record { +/// controllers: opt vec principal; +/// compute_allocation: opt nat; +/// memory_allocation: opt nat; +/// freezing_threshold: opt nat; +/// reserved_cycles_limit: opt nat; +/// log_visibility : opt log_visibility; +/// wasm_memory_limit: opt nat; +/// wasm_memory_threshold: opt nat; +/// })` +#[derive(Default, Clone, CandidType, Deserialize, Debug, PartialEq, Eq)] +pub struct CanisterSettingsArgs { + pub controllers: Option, + pub compute_allocation: Option, + pub memory_allocation: Option, + pub freezing_threshold: Option, + pub reserved_cycles_limit: Option, + pub log_visibility: Option, + pub wasm_memory_limit: Option, + pub wasm_memory_threshold: Option, +} + +impl From for Ic00CanisterSettingsArgs { + fn from(settings: CanisterSettingsArgs) -> Self { + Ic00CanisterSettingsArgs { + controllers: settings.controllers, + compute_allocation: settings.compute_allocation, + memory_allocation: settings.memory_allocation, + freezing_threshold: settings.freezing_threshold, + reserved_cycles_limit: settings.reserved_cycles_limit, + log_visibility: settings.log_visibility.map(LogVisibilityV2::from), + wasm_memory_limit: settings.wasm_memory_limit, + wasm_memory_threshold: settings.wasm_memory_threshold, + } + } +} + +impl From for CanisterSettingsArgs { + fn from(settings: Ic00CanisterSettingsArgs) -> Self { + CanisterSettingsArgs { + controllers: settings.controllers, + compute_allocation: settings.compute_allocation, + memory_allocation: settings.memory_allocation, + freezing_threshold: settings.freezing_threshold, + reserved_cycles_limit: settings.reserved_cycles_limit, + log_visibility: settings.log_visibility.map(LogVisibility::from), + wasm_memory_limit: settings.wasm_memory_limit, + wasm_memory_threshold: settings.wasm_memory_threshold, + } + } +} + /// Argument taken by create canister notification endpoint #[derive(Deserialize, CandidType, Clone, Debug, PartialEq, Eq)] pub struct NotifyCreateCanister { diff --git a/rs/nns/cmc/src/main.rs b/rs/nns/cmc/src/main.rs index bcaa0af40ee..201f6ee4f7b 100644 --- a/rs/nns/cmc/src/main.rs +++ b/rs/nns/cmc/src/main.rs @@ -17,9 +17,11 @@ use ic_crypto_tree_hash::{ }; use ic_ledger_core::block::BlockType; use ic_ledger_core::tokens::CheckedSub; +// TODO(EXC-1687): remove temporary aliases `Ic00CanisterSettingsArgs` and `Ic00CanisterSettingsArgsBuilder`. use ic_management_canister_types::{ - BoundedVec, CanisterIdRecord, CanisterSettingsArgs, CanisterSettingsArgsBuilder, - CreateCanisterArgs, Method, IC_00, + BoundedVec, CanisterIdRecord, CanisterSettingsArgs as Ic00CanisterSettingsArgs, + CanisterSettingsArgsBuilder as Ic00CanisterSettingsArgsBuilder, CreateCanisterArgs, Method, + IC_00, }; use ic_nervous_system_common::NNS_DAPP_BACKEND_CANISTER_ID; use ic_nervous_system_governance::maturity_modulation::{ @@ -2176,9 +2178,11 @@ async fn do_create_canister( settings }) .unwrap_or_else(|| { - CanisterSettingsArgsBuilder::new() - .with_controllers(vec![controller_id]) - .build() + CanisterSettingsArgs::from( + Ic00CanisterSettingsArgsBuilder::new() + .with_controllers(vec![controller_id]) + .build(), + ) }); for subnet_id in subnets { @@ -2187,7 +2191,7 @@ async fn do_create_canister( &Method::CreateCanister.to_string(), dfn_candid::candid_one, CreateCanisterArgs { - settings: Some(canister_settings.clone()), + settings: Some(Ic00CanisterSettingsArgs::from(canister_settings.clone())), sender_canister_version: Some(dfn_core::api::canister_version()), }, dfn_core::api::Funds::new(cycles.get().try_into().unwrap()), diff --git a/rs/nns/integration_tests/src/cycles_minting_canister.rs b/rs/nns/integration_tests/src/cycles_minting_canister.rs index 6717579bc5a..63c8239040b 100644 --- a/rs/nns/integration_tests/src/cycles_minting_canister.rs +++ b/rs/nns/integration_tests/src/cycles_minting_canister.rs @@ -1,7 +1,7 @@ use candid::{Decode, Encode}; use canister_test::Canister; use cycles_minting_canister::{ - ChangeSubnetTypeAssignmentArgs, CreateCanister, CreateCanisterError, + CanisterSettingsArgs, ChangeSubnetTypeAssignmentArgs, CreateCanister, CreateCanisterError, IcpXdrConversionRateCertifiedResponse, NotifyCreateCanister, NotifyError, NotifyMintCyclesArg, NotifyMintCyclesSuccess, SubnetListWithType, SubnetTypesToSubnetsResponse, UpdateSubnetTypeArgs, BAD_REQUEST_CYCLES_PENALTY, CYCLES_LEDGER_CANISTER_ID, @@ -11,8 +11,10 @@ use dfn_candid::candid_one; use dfn_protobuf::protobuf; use ic_canister_client_sender::Sender; use ic_ledger_core::tokens::{CheckedAdd, CheckedSub}; +// TODO(EXC-1687): remove temporary alias `Ic00CanisterSettingsArgs`. use ic_management_canister_types::{ - CanisterIdRecord, CanisterSettingsArgs, CanisterSettingsArgsBuilder, CanisterStatusResultV2, + CanisterIdRecord, CanisterSettingsArgs as Ic00CanisterSettingsArgs, + CanisterSettingsArgsBuilder, CanisterStatusResultV2, }; use ic_nervous_system_common_test_keys::{ TEST_NEURON_1_ID, TEST_NEURON_1_OWNER_KEYPAIR, TEST_USER1_KEYPAIR, TEST_USER1_PRINCIPAL, @@ -663,7 +665,7 @@ fn send_transfer(env: &StateMachine, arg: &TransferArgs) -> Result, + settings: Option, ) -> CanisterId { let transfer_args = TransferArgs { memo: MEMO_CREATE_CANISTER, @@ -685,7 +687,7 @@ fn notify_create_canister( controller: *TEST_USER1_PRINCIPAL, subnet_type: None, subnet_selection: None, - settings, + settings: settings.map(CanisterSettingsArgs::from), }; if let WasmResult::Reply(res) = state_machine @@ -766,13 +768,13 @@ fn cycles_ledger_balance_of(state_machine: &StateMachine, account: Account) -> u fn cmc_create_canister_with_cycles( state_machine: &StateMachine, universal_canister: CanisterId, - settings: Option, + settings: Option, subnet_type: Option, cycles: u128, ) -> Result { #[allow(deprecated)] let create_args = Encode!(&CreateCanister { - settings, + settings: settings.map(CanisterSettingsArgs::from), subnet_type, subnet_selection: None, }) diff --git a/rs/replica_tests/tests/canister_lifecycle.rs b/rs/replica_tests/tests/canister_lifecycle.rs index 32c37a15090..da90d7e9191 100644 --- a/rs/replica_tests/tests/canister_lifecycle.rs +++ b/rs/replica_tests/tests/canister_lifecycle.rs @@ -6,7 +6,7 @@ use ic_error_types::{ErrorCode, RejectCode}; use ic_management_canister_types::{ self as ic00, CanisterChange, CanisterIdRecord, CanisterInstallMode, CanisterSettingsArgsBuilder, CanisterStatusResultV2, CanisterStatusType, EmptyBlob, - InstallCodeArgs, LogVisibility, Method, Payload, UpdateSettingsArgs, IC_00, + InstallCodeArgs, Method, Payload, UpdateSettingsArgs, IC_00, }; use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_replica_tests as utils; @@ -712,7 +712,7 @@ fn can_get_canister_information() { None, 2592000, Some(5_000_000_000_000u128), - LogVisibility::default(), + Default::default(), 0u128, 0u128, 0u128, @@ -770,7 +770,7 @@ fn can_get_canister_information() { None, 259200, None, - LogVisibility::default(), + Default::default(), 0u128, 0u128, 0u128, diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index 22e4e084d20..455e20932da 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -8,7 +8,7 @@ use crate::canister_state::queues::CanisterOutputQueuesIterator; use crate::canister_state::system_state::{CanisterStatus, ExecutionTask, SystemState}; use crate::{InputQueueType, StateError}; pub use execution_state::{EmbedderCache, ExecutionState, ExportedFunctions, Global}; -use ic_management_canister_types::{CanisterStatusType, LogVisibility}; +use ic_management_canister_types::{CanisterStatusType, LogVisibilityV2}; use ic_registry_subnet_type::SubnetType; use ic_types::batch::TotalQueryStats; use ic_types::methods::SystemMethod; @@ -153,7 +153,7 @@ impl CanisterState { &self.system_state.controllers } - pub fn log_visibility(&self) -> &LogVisibility { + pub fn log_visibility(&self) -> &LogVisibilityV2 { &self.system_state.log_visibility } diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index ca3dadcc181..f6f670976df 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -12,7 +12,7 @@ pub use call_context_manager::{CallContext, CallContextAction, CallContextManage use ic_base_types::NumSeconds; use ic_logger::{error, ReplicaLogger}; use ic_management_canister_types::{ - CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, LogVisibility, + CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, LogVisibilityV2, }; use ic_protobuf::proxy::{try_from_option_field, ProxyDecodeError}; use ic_protobuf::state::canister_state_bits::v1 as pb; @@ -343,7 +343,7 @@ pub struct SystemState { pub wasm_chunk_store: WasmChunkStore, /// Log visibility of the canister. - pub log_visibility: LogVisibility, + pub log_visibility: LogVisibilityV2, /// Log records of the canister. pub canister_log: CanisterLog, @@ -722,7 +722,7 @@ impl SystemState { canister_version: 0, canister_history: CanisterHistory::default(), wasm_chunk_store, - log_visibility: LogVisibility::default(), + log_visibility: Default::default(), canister_log: Default::default(), wasm_memory_limit: None, next_snapshot_id: 0, @@ -750,7 +750,7 @@ impl SystemState { canister_history: CanisterHistory, wasm_chunk_store_data: PageMap, wasm_chunk_store_metadata: WasmChunkStoreMetadata, - log_visibility: LogVisibility, + log_visibility: LogVisibilityV2, canister_log: CanisterLog, wasm_memory_limit: Option, next_snapshot_id: u64, diff --git a/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs b/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs index b179664a3ee..ffe7bd42f30 100644 --- a/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs +++ b/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs @@ -60,7 +60,7 @@ fn test_manage_dapp_canister_settings_successful() { .with_memory_allocation(1 << 30) .with_freezing_threshold(100_000) .with_reserved_cycles_limit(1_000_000_000_000) - .with_log_visibility(ic_management_canister_types::LogVisibility::Public) + .with_log_visibility(ic_management_canister_types::LogVisibilityV2::Public) .with_wasm_memory_limit(1_000_000_000) .build(), ), @@ -105,7 +105,7 @@ fn test_manage_dapp_canister_settings_successful() { Some(1 << 30), 100_000, Some(1_000_000_000_000), - ic_management_canister_types::LogVisibility::Public, + ic_management_canister_types::LogVisibilityV2::Public, Some(1_000_000_000), ), ); @@ -158,7 +158,7 @@ fn test_manage_dapp_canister_settings_successful() { Some(0), 0, Some(0), - ic_management_canister_types::LogVisibility::Controllers, + ic_management_canister_types::LogVisibilityV2::Controllers, Some(2_000_000_000), ), ); @@ -199,7 +199,7 @@ fn test_manage_dapp_canister_settings_failure() { .with_freezing_threshold(100_000) .with_reserved_cycles_limit(1_000_000_000_000) .with_wasm_memory_limit(1_000_000_000) - .with_log_visibility(ic_management_canister_types::LogVisibility::Public) + .with_log_visibility(ic_management_canister_types::LogVisibilityV2::Public) .build(), ), ); @@ -235,7 +235,7 @@ fn test_manage_dapp_canister_settings_failure() { Some(1 << 30), 100_000, Some(1_000_000_000_000), - ic_management_canister_types::LogVisibility::Public, + ic_management_canister_types::LogVisibilityV2::Public, Some(1_000_000_000), ), ); @@ -308,7 +308,7 @@ fn test_manage_dapp_canister_settings_failure() { Some(1 << 30), 100_000, Some(1_000_000_000_000), - ic_management_canister_types::LogVisibility::Public, + ic_management_canister_types::LogVisibilityV2::Public, Some(1_000_000_000), ), ); diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index 70ea7f724e0..1fcf1686ae3 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -170,7 +170,7 @@ pub struct CanisterStateBits { pub canister_history: CanisterHistory, pub wasm_chunk_store_metadata: WasmChunkStoreMetadata, pub total_query_stats: TotalQueryStats, - pub log_visibility: LogVisibility, + pub log_visibility: LogVisibilityV2, pub canister_log: CanisterLog, pub wasm_memory_limit: Option, pub next_snapshot_id: u64, @@ -2164,12 +2164,12 @@ impl From for pb_canister_state_bits::CanisterStateBits { canister_history: Some((&item.canister_history).into()), wasm_chunk_store_metadata: Some((&item.wasm_chunk_store_metadata).into()), total_query_stats: Some((&item.total_query_stats).into()), - log_visibility: pb_canister_state_bits::LogVisibility::from(&item.log_visibility) - .into(), - log_visibility_v2: pb_canister_state_bits::LogVisibilityV2::from( - &LogVisibilityV2::from(item.log_visibility), - ) + log_visibility: pb_canister_state_bits::LogVisibility::from(&LogVisibility::from( + item.log_visibility.clone(), + )) .into(), + log_visibility_v2: pb_canister_state_bits::LogVisibilityV2::from(&item.log_visibility) + .into(), canister_log_records: item .canister_log .records() @@ -2252,7 +2252,7 @@ impl TryFrom for CanisterStateBits { // TODO(EXC-1670): remove after migration to `pb_canister_state_bits::LogVisibilityV2`. // First try to decode `log_visibility_v2` and if it fails, fallback to `log_visibility`. // This should populate `allowed_viewers` correctly with the list of principals. - let log_visibility: LogVisibility = match try_from_option_field::< + let log_visibility: LogVisibilityV2 = match try_from_option_field::< pb_canister_state_bits::LogVisibilityV2, LogVisibilityV2, _, @@ -2260,7 +2260,7 @@ impl TryFrom for CanisterStateBits { value.log_visibility_v2, "CanisterStateBits::log_visibility_v2", ) { - Ok(log_visibility_v2) => LogVisibility::from(log_visibility_v2), + Ok(log_visibility_v2) => log_visibility_v2, Err(_) => { let pb_log_visibility = pb_canister_state_bits::LogVisibility::try_from( value.log_visibility, @@ -2272,7 +2272,7 @@ impl TryFrom for CanisterStateBits { value.log_visibility ), })?; - LogVisibility::from(pb_log_visibility) + LogVisibilityV2::from(LogVisibility::from(pb_log_visibility)) } }; diff --git a/rs/state_layout/src/state_layout/tests.rs b/rs/state_layout/src/state_layout/tests.rs index 23b50986049..138baacbe79 100644 --- a/rs/state_layout/src/state_layout/tests.rs +++ b/rs/state_layout/src/state_layout/tests.rs @@ -1,8 +1,7 @@ use super::*; use ic_management_canister_types::{ - CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallMode, - LogVisibility, IC_00, + CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallMode, IC_00, }; use ic_replicated_state::{ canister_state::system_state::CanisterHistory, @@ -54,7 +53,7 @@ fn default_canister_state_bits() -> CanisterStateBits { canister_history: CanisterHistory::default(), wasm_chunk_store_metadata: WasmChunkStoreMetadata::default(), total_query_stats: TotalQueryStats::default(), - log_visibility: LogVisibility::default(), + log_visibility: Default::default(), canister_log: Default::default(), wasm_memory_limit: None, next_snapshot_id: 0, diff --git a/rs/test_utilities/execution_environment/src/lib.rs b/rs/test_utilities/execution_environment/src/lib.rs index c61b4a89b5d..53560c24c2c 100644 --- a/rs/test_utilities/execution_environment/src/lib.rs +++ b/rs/test_utilities/execution_environment/src/lib.rs @@ -29,7 +29,7 @@ use ic_logger::{replica_logger::no_op_logger, ReplicaLogger}; use ic_management_canister_types::{ CanisterIdRecord, CanisterInstallMode, CanisterInstallModeV2, CanisterSettingsArgs, CanisterSettingsArgsBuilder, CanisterStatusType, CanisterUpgradeOptions, EmptyBlob, - InstallCodeArgs, InstallCodeArgsV2, LogVisibility, MasterPublicKeyId, Method, Payload, + InstallCodeArgs, InstallCodeArgsV2, LogVisibilityV2, MasterPublicKeyId, Method, Payload, ProvisionalCreateCanisterWithCyclesArgs, UpdateSettingsArgs, }; use ic_metrics::MetricsRegistry; @@ -690,7 +690,7 @@ impl ExecutionTest { pub fn set_log_visibility( &mut self, canister_id: CanisterId, - log_visibility: LogVisibility, + log_visibility: LogVisibilityV2, ) -> Result { let payload = UpdateSettingsArgs { canister_id: canister_id.into(), diff --git a/rs/test_utilities/state/src/lib.rs b/rs/test_utilities/state/src/lib.rs index e322eb2c720..1fba770b3d3 100644 --- a/rs/test_utilities/state/src/lib.rs +++ b/rs/test_utilities/state/src/lib.rs @@ -1,8 +1,8 @@ use ic_base_types::NumSeconds; use ic_btc_replica_types::BitcoinAdapterRequestWrapper; use ic_management_canister_types::{ - CanisterStatusType, EcdsaCurve, EcdsaKeyId, LogVisibility, MasterPublicKeyId, SchnorrAlgorithm, - SchnorrKeyId, + CanisterStatusType, EcdsaCurve, EcdsaKeyId, LogVisibilityV2, MasterPublicKeyId, + SchnorrAlgorithm, SchnorrKeyId, }; use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; use ic_registry_subnet_features::SubnetFeatures; @@ -221,7 +221,7 @@ pub struct CanisterStateBuilder { inputs: Vec, time_of_last_allocation_charge: Time, certified_data: Vec, - log_visibility: LogVisibility, + log_visibility: LogVisibilityV2, } impl CanisterStateBuilder { @@ -310,7 +310,7 @@ impl CanisterStateBuilder { self } - pub fn with_log_visibility(mut self, log_visibility: LogVisibility) -> Self { + pub fn with_log_visibility(mut self, log_visibility: LogVisibilityV2) -> Self { self.log_visibility = log_visibility; self } @@ -427,7 +427,7 @@ impl Default for CanisterStateBuilder { inputs: Vec::default(), time_of_last_allocation_charge: UNIX_EPOCH, certified_data: vec![], - log_visibility: LogVisibility::default(), + log_visibility: Default::default(), } } } diff --git a/rs/types/management_canister_types/src/lib.rs b/rs/types/management_canister_types/src/lib.rs index 02db79f6450..40c70aee365 100644 --- a/rs/types/management_canister_types/src/lib.rs +++ b/rs/types/management_canister_types/src/lib.rs @@ -866,13 +866,15 @@ pub enum LogVisibilityV2 { // When decoding, if `AllowedViewers` is encountered, it is changed to a default value // to maintain compatibility with the old `LogVisibility`. impl Payload<'_> for LogVisibilityV2 { - fn decode(blob: &'_ [u8]) -> Result { - match Decode!([decoder_config()]; blob, Self).map_err(candid_error_to_user_error) { - // Fall back to the default value. - Ok(Self::AllowedViewers(_)) => Ok(Self::default()), - Ok(log_visibility_v2) => Ok(log_visibility_v2), - Err(err) => Err(err), + fn decode(blob: &[u8]) -> Result { + let decoded = + Decode!([decoder_config()]; blob, Self).map_err(candid_error_to_user_error)?; + + if matches!(decoded, Self::AllowedViewers(_)) { + return Ok(Self::default()); // Fall back to the default value. } + + Ok(decoded) } } @@ -1022,7 +1024,7 @@ pub struct DefiniteCanisterSettingsArgs { memory_allocation: candid::Nat, freezing_threshold: candid::Nat, reserved_cycles_limit: candid::Nat, - log_visibility: LogVisibility, + log_visibility: LogVisibilityV2, wasm_memory_limit: candid::Nat, } @@ -1034,7 +1036,7 @@ impl DefiniteCanisterSettingsArgs { memory_allocation: Option, freezing_threshold: u64, reserved_cycles_limit: Option, - log_visibility: LogVisibility, + log_visibility: LogVisibilityV2, wasm_memory_limit: Option, ) -> Self { let memory_allocation = candid::Nat::from(memory_allocation.unwrap_or(0)); @@ -1060,7 +1062,7 @@ impl DefiniteCanisterSettingsArgs { self.reserved_cycles_limit.clone() } - pub fn log_visibility(&self) -> &LogVisibility { + pub fn log_visibility(&self) -> &LogVisibilityV2 { &self.log_visibility } } @@ -1178,7 +1180,7 @@ impl CanisterStatusResultV2 { memory_allocation: Option, freezing_threshold: u64, reserved_cycles_limit: Option, - log_visibility: LogVisibility, + log_visibility: LogVisibilityV2, idle_cycles_burned_per_day: u128, reserved_cycles: u128, query_num_calls: u128, @@ -1862,7 +1864,7 @@ pub struct CanisterSettingsArgs { pub memory_allocation: Option, pub freezing_threshold: Option, pub reserved_cycles_limit: Option, - pub log_visibility: Option, + pub log_visibility: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, } @@ -1893,7 +1895,7 @@ pub struct CanisterSettingsArgsBuilder { memory_allocation: Option, freezing_threshold: Option, reserved_cycles_limit: Option, - log_visibility: Option, + log_visibility: Option, wasm_memory_limit: Option, wasm_memory_threshold: Option, } @@ -1976,7 +1978,7 @@ impl CanisterSettingsArgsBuilder { } /// Sets the log visibility. - pub fn with_log_visibility(self, log_visibility: LogVisibility) -> Self { + pub fn with_log_visibility(self, log_visibility: LogVisibilityV2) -> Self { Self { log_visibility: Some(log_visibility), ..self