diff --git a/rs/nns/governance/src/proposals/install_code.rs b/rs/nns/governance/src/proposals/install_code.rs index d36b708d3d7..65b8975a09d 100644 --- a/rs/nns/governance/src/proposals/install_code.rs +++ b/rs/nns/governance/src/proposals/install_code.rs @@ -202,9 +202,9 @@ mod tests { }; let is_invalid_proposal_with_keywords = |install_code: InstallCode, keywords: Vec<&str>| { - let error = install_code - .validate() - .expect_err("Expecting validation error for {install_code:?} but got Ok(())"); + let error = install_code.validate().expect_err(&format!( + "Expecting validation error for {install_code:?} but got Ok(())" + )); assert_eq!(error.error_type, ErrorType::InvalidProposal as i32); for keyword in keywords { let error_message = error.error_message.to_lowercase(); diff --git a/rs/nns/governance/src/proposals/stop_or_start_canister.rs b/rs/nns/governance/src/proposals/stop_or_start_canister.rs index 9794081e3c8..51c39b474c7 100644 --- a/rs/nns/governance/src/proposals/stop_or_start_canister.rs +++ b/rs/nns/governance/src/proposals/stop_or_start_canister.rs @@ -133,9 +133,9 @@ mod tests { let is_invalid_proposal_with_keywords = |stop_or_start_canister: StopOrStartCanister, keywords: Vec<&str>| { - let error = stop_or_start_canister.validate().expect_err( - "Expecting validation error for {stop_or_start_canister:?} but got Ok(())", - ); + let error = stop_or_start_canister.validate().expect_err(&format!( + "Expecting validation error for {stop_or_start_canister:?} but got Ok(())" + )); assert_eq!(error.error_type, ErrorType::InvalidProposal as i32); for keyword in keywords { let error_message = error.error_message.to_lowercase(); diff --git a/rs/nns/governance/src/proposals/update_canister_settings.rs b/rs/nns/governance/src/proposals/update_canister_settings.rs index 25920b5acab..7e310a1c0f2 100644 --- a/rs/nns/governance/src/proposals/update_canister_settings.rs +++ b/rs/nns/governance/src/proposals/update_canister_settings.rs @@ -12,7 +12,7 @@ use ic_base_types::CanisterId; use ic_nervous_system_clients::update_settings::{ CanisterSettings as RootCanisterSettings, LogVisibility as RootLogVisibility, }; -use ic_nns_constants::ROOT_CANISTER_ID; +use ic_nns_constants::{LIFELINE_CANISTER_ID, ROOT_CANISTER_ID}; use ic_nns_handler_root_interface::UpdateCanisterSettingsRequest; impl UpdateCanisterSettings { @@ -37,11 +37,6 @@ impl UpdateCanisterSettings { .ok_or(invalid_proposal_error("Canister ID is required"))?; let canister_id = CanisterId::try_from(canister_principal_id) .map_err(|_| invalid_proposal_error("Invalid canister ID"))?; - if canister_id == ROOT_CANISTER_ID { - return Err(invalid_proposal_error( - "Updating root canister settings is not supported yet.", - )); - } Ok(canister_id) } @@ -107,18 +102,29 @@ impl UpdateCanisterSettings { impl CallCanister for UpdateCanisterSettings { fn canister_and_function(&self) -> Result<(CanisterId, &str), GovernanceError> { - Ok((ROOT_CANISTER_ID, "update_canister_settings")) + let canister_id = self.valid_canister_id()?; + if canister_id == ROOT_CANISTER_ID { + Ok((LIFELINE_CANISTER_ID, "update_root_settings")) + } else { + Ok((ROOT_CANISTER_ID, "update_canister_settings")) + } } fn payload(&self) -> Result, GovernanceError> { - let canister_id = self.valid_canister_id()?.get(); + let canister_id = self.valid_canister_id()?; let settings = self.valid_canister_settings()?; - let update_settings = UpdateCanisterSettingsRequest { - canister_id, - settings, - }; - Encode!(&update_settings) - .map_err(|err| invalid_proposal_error(&format!("Failed to encode payload: {err}"))) + + if canister_id == ROOT_CANISTER_ID { + Encode!(&settings) + .map_err(|err| invalid_proposal_error(&format!("Failed to encode payload: {err}"))) + } else { + let update_settings = UpdateCanisterSettingsRequest { + canister_id: canister_id.get(), + settings, + }; + Encode!(&update_settings) + .map_err(|err| invalid_proposal_error(&format!("Failed to encode payload: {err}"))) + } } } @@ -129,8 +135,13 @@ mod tests { use crate::pb::v1::governance_error::ErrorType; #[cfg(feature = "test")] use crate::pb::v1::update_canister_settings::{CanisterSettings, Controllers}; - + #[cfg(feature = "test")] + use candid::Decode; + #[cfg(feature = "test")] + use ic_base_types::CanisterId; use ic_nns_constants::LEDGER_CANISTER_ID; + #[cfg(feature = "test")] + use ic_nns_constants::{GOVERNANCE_CANISTER_ID, SNS_WASM_CANISTER_ID}; #[cfg(not(feature = "test"))] #[test] @@ -163,9 +174,9 @@ mod tests { let is_invalid_proposal_with_keywords = |update_canister_settings: UpdateCanisterSettings, keywords: Vec<&str>| { - let error = update_canister_settings - .validate() - .expect_err("Expecting validation error for {install_code:?} but got Ok(())"); + let error = update_canister_settings.validate().expect_err(&format!( + "Expecting validation error for {update_canister_settings:?} but got Ok(())" + )); assert_eq!(error.error_type, ErrorType::InvalidProposal as i32); for keyword in keywords { let error_message = error.error_message.to_lowercase(); @@ -223,22 +234,11 @@ mod tests { }, vec!["invalid log visibility", "0"], ); - - is_invalid_proposal_with_keywords( - UpdateCanisterSettings { - canister_id: Some(ROOT_CANISTER_ID.get()), - ..valid_update_canister_settings.clone() - }, - vec!["root canister", "not supported"], - ); } #[cfg(feature = "test")] #[test] fn test_update_ledger_canister_settings() { - use candid::Decode; - use ic_nns_constants::GOVERNANCE_CANISTER_ID; - let update_ledger_canister_settings = UpdateCanisterSettings { canister_id: Some(LEDGER_CANISTER_ID.get()), // The value of the settings are arbitrary and do not have any meaning. @@ -288,10 +288,54 @@ mod tests { #[cfg(feature = "test")] #[test] - fn test_update_canister_settings_topic_mapping() { - use ic_base_types::CanisterId; - use ic_nns_constants::SNS_WASM_CANISTER_ID; + fn test_update_root_canister_settings() { + let update_root_canister_settings = UpdateCanisterSettings { + canister_id: Some(ROOT_CANISTER_ID.get()), + // The value of the settings are arbitrary and do not have any meaning. + settings: Some(CanisterSettings { + controllers: Some(Controllers { + controllers: vec![LIFELINE_CANISTER_ID.get()], + }), + memory_allocation: Some(1 << 32), + wasm_memory_limit: Some(1 << 31), + compute_allocation: Some(10), + freezing_threshold: Some(100), + log_visibility: Some(LogVisibility::Public as i32), + }), + }; + assert_eq!(update_root_canister_settings.validate(), Ok(())); + assert_eq!( + update_root_canister_settings.valid_topic(), + Ok(Topic::ProtocolCanisterManagement) + ); + assert_eq!( + update_root_canister_settings.canister_and_function(), + Ok((LIFELINE_CANISTER_ID, "update_root_settings")) + ); + + let decoded_payload = Decode!( + &update_root_canister_settings.payload().unwrap(), + RootCanisterSettings + ) + .unwrap(); + assert_eq!( + decoded_payload, + RootCanisterSettings { + controllers: Some(vec![LIFELINE_CANISTER_ID.get()]), + memory_allocation: Some(Nat::from(1u64 << 32)), + wasm_memory_limit: Some(Nat::from(1u64 << 31)), + compute_allocation: Some(Nat::from(10u64)), + freezing_threshold: Some(Nat::from(100u64)), + log_visibility: Some(RootLogVisibility::Public), + reserved_cycles_limit: None, + } + ); + } + + #[cfg(feature = "test")] + #[test] + fn test_update_canister_settings_topic_mapping() { let test_cases = vec![ (LEDGER_CANISTER_ID, Topic::ProtocolCanisterManagement), (SNS_WASM_CANISTER_ID, Topic::ServiceNervousSystemManagement), diff --git a/rs/nns/handlers/lifeline/impl/lifeline.did b/rs/nns/handlers/lifeline/impl/lifeline.did index 8b4185e75f5..a591df91ee6 100644 --- a/rs/nns/handlers/lifeline/impl/lifeline.did +++ b/rs/nns/handlers/lifeline/impl/lifeline.did @@ -1,17 +1,25 @@ -type CanisterStatusResult = record { - status : CanisterStatusType; - memory_size : nat; - cycles : nat; - settings : DefiniteCanisterSettings; - module_hash : opt vec nat8; +type UpgradeRootProposalPayload = record { + module_arg: blob; + stop_upgrade_start: bool; + wasm_module: blob; }; -type CanisterStatusType = variant { stopped; stopping; running }; -type DefiniteCanisterSettings = record { controllers : vec principal }; +type HardResetRootToVersionPayload = record { + wasm_module: blob; + init_arg: blob; +}; +type CanisterSettings = record { + controllers: opt vec principal; + compute_allocation: opt nat; + memory_allocation: opt nat; + freezing_threshold: opt nat; + reserved_cycles_limit: opt nat; + wasm_memory_limit: opt nat; + log_visibility: opt LogVisibility; +}; +type LogVisibility = variant { controllers; public }; + service : { - upgrade_root: - (record { - module_arg: blob; - stop_upgrade_start: bool; - wasm_module: blob; - }) -> (); + upgrade_root: (UpgradeRootProposalPayload) -> (); + hard_reset_root_to_version: (HardResetRootToVersionPayload) -> (); + upgrade_root_settings: (CanisterSettings) -> (); } diff --git a/rs/nns/handlers/lifeline/impl/lifeline.mo b/rs/nns/handlers/lifeline/impl/lifeline.mo index 08c39aa6de1..07129722c29 100644 --- a/rs/nns/handlers/lifeline/impl/lifeline.mo +++ b/rs/nns/handlers/lifeline/impl/lifeline.mo @@ -8,8 +8,30 @@ actor { private let governanceCanister : Principal = Prim.principalOfActor Governance; private let root : Principal = Prim.principalOfActor Root; - type UpgradeRootProposalPayload = { wasm_module : Blob; module_arg : Blob; stop_upgrade_start : Bool }; - type HardResetRootToVersionPayload = { wasm_module : Blob; init_arg : Blob; }; + type UpgradeRootProposalPayload = { + wasm_module : Blob; + module_arg : Blob; + stop_upgrade_start : Bool; + }; + + type HardResetRootToVersionPayload = { + wasm_module : Blob; + init_arg : Blob; + }; + + type CanisterIdRecord = { canister_id : Principal }; + + type LogVisibility = {#controllers; #public_}; + + type CanisterSettings = { + controllers : ?[Principal]; + compute_allocation: ?Nat; + memory_allocation: ?Nat; + freezing_threshold: ?Nat; + reserved_cycles_limit: ?Nat; + wasm_memory_limit: ?Nat; + log_visibility: ?LogVisibility; + }; // IC00 is the management canister. We rely on it for the four // fundamental methods as listed below. @@ -22,7 +44,11 @@ actor { } -> async (); start_canister : CanisterIdRecord -> async (); stop_canister : CanisterIdRecord -> async (); - uninstall_code : CanisterIdRecord -> async () + uninstall_code : CanisterIdRecord -> async (); + update_settings: { + canister_id: Principal; + settings: CanisterSettings; + } -> async (); }; public shared ({caller}) func upgrade_root(pl : UpgradeRootProposalPayload) : async () { @@ -72,10 +98,16 @@ actor { debug { Prim.debugPrint "hard_reset_root: finished installing" }; }; - type CanisterIdRecord = { canister_id : Principal }; + public shared ({caller}) func update_root_settings(settings: CanisterSettings) : async () { + assert caller == governanceCanister; - type DefiniteCanisterSettings = { - controllers : [Principal]; + debug { Prim.debugPrint ("update_root_settings: about to update settings") }; + + await ic00.update_settings({ + canister_id = root; + settings = settings; + }); + debug { Prim.debugPrint ("update_root_settings: finished updating settings") }; }; } diff --git a/rs/nns/integration_tests/src/update_canister_settings.rs b/rs/nns/integration_tests/src/update_canister_settings.rs index 02afec5ff37..88a59876eed 100644 --- a/rs/nns/integration_tests/src/update_canister_settings.rs +++ b/rs/nns/integration_tests/src/update_canister_settings.rs @@ -1,7 +1,7 @@ use candid::Nat; use ic_base_types::{CanisterId, PrincipalId}; use ic_nervous_system_clients::canister_status::{DefiniteCanisterSettings, LogVisibility}; -use ic_nns_constants::{REGISTRY_CANISTER_ID, ROOT_CANISTER_ID}; +use ic_nns_constants::{LIFELINE_CANISTER_ID, REGISTRY_CANISTER_ID, ROOT_CANISTER_ID}; use ic_nns_governance_api::pb::v1::{ manage_neuron_response::Command, proposal::Action, @@ -19,8 +19,10 @@ use ic_nns_test_utils::{ }, }; -#[test] -fn test_update_canister_settings() { +fn test_update_canister_settings_proposal( + target_canister_id: CanisterId, + controller_canister_id: CanisterId, +) { // Step 1: Set up the NNS canisters and get the neuron. let state_machine = state_machine_builder_for_nns_tests().build(); let nns_init_payloads = NnsInitPayloadsBuilder::new().with_test_neurons().build(); @@ -28,7 +30,10 @@ fn test_update_canister_settings() { let n1 = get_neuron_1(); // Step 2: Define the target settings and make sure they are different from the current ones. - let target_controllers = vec![ROOT_CANISTER_ID.get(), PrincipalId::new_user_test_id(1)]; + let target_controllers = vec![ + controller_canister_id.get(), + PrincipalId::new_user_test_id(1), + ]; let target_memory_allocation = 1u64 << 33; let target_compute_allocation = 10u64; let target_freezing_threshold = 100_000u64; @@ -37,8 +42,8 @@ fn test_update_canister_settings() { let canister_settings = || -> DefiniteCanisterSettings { get_canister_status( &state_machine, - ROOT_CANISTER_ID.get(), - REGISTRY_CANISTER_ID, + controller_canister_id.get(), + target_canister_id, CanisterId::ic_00(), ) .unwrap() @@ -73,7 +78,7 @@ fn test_update_canister_settings() { &Proposal { title: Some("Update canister settings".to_string()), action: Some(Action::UpdateCanisterSettings(UpdateCanisterSettings { - canister_id: Some(REGISTRY_CANISTER_ID.get()), + canister_id: Some(target_canister_id.get()), settings: Some(CanisterSettings { controllers: Some(Controllers { controllers: target_controllers.clone(), @@ -115,3 +120,13 @@ fn test_update_canister_settings() { ); assert_eq!(updated_settings.log_visibility, target_log_visibility); } + +#[test] +fn test_update_canister_settings_proposal_non_root() { + test_update_canister_settings_proposal(REGISTRY_CANISTER_ID, ROOT_CANISTER_ID); +} + +#[test] +fn test_update_canister_settings_proposal_root() { + test_update_canister_settings_proposal(ROOT_CANISTER_ID, LIFELINE_CANISTER_ID); +}