From 706c5a22a2c8c76ca9ddf3fad4fb3c7773596428 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Thu, 23 Jul 2026 15:16:04 +0000 Subject: [PATCH 1/3] Added a new NNS proposal type: UpdateStandardEngineReplicaVersion. --- rs/nns/governance/api/src/types.rs | 12 ++ rs/nns/governance/canister/governance.did | 9 + .../ic_nns_governance/pb/v1/governance.proto | 10 ++ .../src/gen/ic_nns_governance.pb.v1.rs | 24 ++- rs/nns/governance/src/governance.rs | 12 ++ rs/nns/governance/src/pb/conversions/mod.rs | 26 +++ .../governance/src/pb/proposal_conversions.rs | 3 + rs/nns/governance/src/proposals/mod.rs | 19 +- .../update_standard_engine_replica_version.rs | 122 +++++++++++++ ...e_standard_engine_replica_version_tests.rs | 163 ++++++++++++++++++ rs/nns/governance/unreleased_changelog.md | 3 + 11 files changed, 400 insertions(+), 3 deletions(-) create mode 100644 rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs create mode 100644 rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs diff --git a/rs/nns/governance/api/src/types.rs b/rs/nns/governance/api/src/types.rs index 30260295753b..955e309c115e 100644 --- a/rs/nns/governance/api/src/types.rs +++ b/rs/nns/governance/api/src/types.rs @@ -693,6 +693,8 @@ pub mod proposal { LoadCanisterSnapshot(super::LoadCanisterSnapshot), /// Create a canister in a (possibly non-NNS) subnet and install code into it. CreateCanisterAndInstallCode(super::CreateCanisterAndInstallCode), + /// Change what replica version(s) are run by Cloud Engines. + UpdateStandardEngineReplicaVersion(super::UpdateStandardEngineReplicaVersion), } } /// Empty message to use in oneof fields that represent empty @@ -1454,6 +1456,7 @@ pub enum ProposalActionRequest { TakeCanisterSnapshot(TakeCanisterSnapshot), LoadCanisterSnapshot(LoadCanisterSnapshot), CreateCanisterAndInstallCode(CreateCanisterAndInstallCodeRequest), + UpdateStandardEngineReplicaVersion(UpdateStandardEngineReplicaVersion), } #[derive( @@ -2843,6 +2846,15 @@ pub struct BlessAlternativeGuestOsVersion { pub base_guest_launch_measurements: Option, } +#[derive( + candid::CandidType, candid::Deserialize, serde::Serialize, Clone, PartialEq, Debug, Default, +)] +pub struct UpdateStandardEngineReplicaVersion { + pub new_replica_version_id: Option, + pub old_replica_version_id: Option, + pub deployment_progress: Option, +} + /// See also the definition of GuestLaunchMeasurements (plural!) in /// rs/protobuf/def/registry/replica_version/v1/replica_version.proto #[derive( diff --git a/rs/nns/governance/canister/governance.did b/rs/nns/governance/canister/governance.did index a026b77afda1..e3123a42fd04 100644 --- a/rs/nns/governance/canister/governance.did +++ b/rs/nns/governance/canister/governance.did @@ -25,6 +25,7 @@ type Action = variant { TakeCanisterSnapshot : TakeCanisterSnapshot; LoadCanisterSnapshot : LoadCanisterSnapshot; CreateCanisterAndInstallCode : CreateCanisterAndInstallCode; + UpdateStandardEngineReplicaVersion : UpdateStandardEngineReplicaVersion; }; type AddHotKey = record { @@ -1096,6 +1097,7 @@ type ProposalActionRequest = variant { TakeCanisterSnapshot : TakeCanisterSnapshot; LoadCanisterSnapshot : LoadCanisterSnapshot; CreateCanisterAndInstallCode : CreateCanisterAndInstallCodeRequest; + UpdateStandardEngineReplicaVersion : UpdateStandardEngineReplicaVersion; }; // Creates a rented subnet from a rental request (in the Subnet Rental @@ -1151,6 +1153,13 @@ type BlessAlternativeGuestOsVersion = record { // (Here, we refer to the version being replaced as the "base" version.) base_guest_launch_measurements : opt GuestLaunchMeasurements; }; +// Changes what replica version(s) are run by Cloud Engines. +type UpdateStandardEngineReplicaVersion = record { + new_replica_version_id : opt text; + old_replica_version_id : opt text; + deployment_progress : opt float64; +}; + type GuestLaunchMeasurements = record { guest_launch_measurements : opt vec GuestLaunchMeasurement; }; diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index 8e3e8b0be40d..5c831cdb79ac 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -730,6 +730,8 @@ message Proposal { LoadCanisterSnapshot load_canister_snapshot = 33; // Create a canister in a (possibly non-NNS) subnet and install code into it. CreateCanisterAndInstallCode create_canister_and_install_code = 34; + // Change what replica version(s) are run by Cloud Engines. + UpdateStandardEngineReplicaVersion update_standard_engine_replica_version = 35; } } @@ -2010,6 +2012,14 @@ message BlessAlternativeGuestOsVersion { registry.replica_version.v1.GuestLaunchMeasurements base_guest_launch_measurements = 3; } +// Changes what replica version(s) are run by Cloud Engines. See Registry's +// do_update_standard_engine_replica_version for what changes are allowed. +message UpdateStandardEngineReplicaVersion { + string new_replica_version_id = 1; + string old_replica_version_id = 2; + double deployment_progress = 3; +} + message LoadCanisterSnapshot { // The ID of the canister to load the snapshot into. ic_base_types.pb.v1.PrincipalId canister_id = 1; diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index af9369bd6752..4f774e2f082b 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -466,7 +466,7 @@ pub struct Proposal { /// take. #[prost( oneof = "proposal::Action", - tags = "10, 12, 13, 14, 15, 16, 17, 18, 19, 21, 29, 22, 23, 24, 25, 26, 27, 28, 31, 32, 33, 34" + tags = "10, 12, 13, 14, 15, 16, 17, 18, 19, 21, 29, 22, 23, 24, 25, 26, 27, 28, 31, 32, 33, 34, 35" )] pub action: ::core::option::Option, } @@ -585,6 +585,9 @@ pub mod proposal { /// Create a canister in a (possibly non-NNS) subnet and install code into it. #[prost(message, tag = "34")] CreateCanisterAndInstallCode(super::CreateCanisterAndInstallCode), + /// Change what replica version(s) are run by Cloud Engines. + #[prost(message, tag = "35")] + UpdateStandardEngineReplicaVersion(super::UpdateStandardEngineReplicaVersion), } } /// Take a canister snapshot. @@ -3159,6 +3162,25 @@ pub struct BlessAlternativeGuestOsVersion { ::ic_protobuf::registry::replica_version::v1::GuestLaunchMeasurements, >, } +/// Changes what replica version(s) are run by Cloud Engines. See Registry's +/// do_update_standard_engine_replica_version for what changes are allowed. +#[derive( + candid::CandidType, + candid::Deserialize, + serde::Serialize, + comparable::Comparable, + Clone, + PartialEq, + ::prost::Message, +)] +pub struct UpdateStandardEngineReplicaVersion { + #[prost(string, tag = "1")] + pub new_replica_version_id: ::prost::alloc::string::String, + #[prost(string, tag = "2")] + pub old_replica_version_id: ::prost::alloc::string::String, + #[prost(double, tag = "3")] + pub deployment_progress: f64, +} #[derive( candid::CandidType, candid::Deserialize, diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index 920b03cf1cb5..6fc411d57930 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -541,6 +541,9 @@ impl Action { Action::TakeCanisterSnapshot(_) => "ACTION_TAKE_CANISTER_SNAPSHOT", Action::LoadCanisterSnapshot(_) => "ACTION_LOAD_CANISTER_SNAPSHOT", Action::CreateCanisterAndInstallCode(_) => "ACTION_CREATE_CANISTER_AND_INSTALL_CODE", + Action::UpdateStandardEngineReplicaVersion(_) => { + "ACTION_UPDATE_STANDARD_ENGINE_REPLICA_VERSION" + } } } } @@ -4282,6 +4285,12 @@ impl Governance { self.perform_call_canister(pid, create_canister_and_install_code) .await; } + ValidProposalAction::UpdateStandardEngineReplicaVersion( + update_standard_engine_replica_version, + ) => { + self.perform_call_canister(pid, update_standard_engine_replica_version) + .await; + } } } @@ -4920,6 +4929,9 @@ impl Governance { ValidProposalAction::CreateCanisterAndInstallCode(create_canister_and_install_code) => { create_canister_and_install_code.validate() } + ValidProposalAction::UpdateStandardEngineReplicaVersion( + update_standard_engine_replica_version, + ) => update_standard_engine_replica_version.validate(), } } diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index c0ecae923268..c98debec1378 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -469,6 +469,9 @@ impl From for pb::proposal::Action { api::proposal::Action::CreateCanisterAndInstallCode(v) => { pb::proposal::Action::CreateCanisterAndInstallCode(v.into()) } + api::proposal::Action::UpdateStandardEngineReplicaVersion(v) => { + pb::proposal::Action::UpdateStandardEngineReplicaVersion(v.into()) + } } } } @@ -530,6 +533,9 @@ impl From for pb::proposal::Action { api::ProposalActionRequest::CreateCanisterAndInstallCode(v) => { pb::proposal::Action::CreateCanisterAndInstallCode(v.into()) } + api::ProposalActionRequest::UpdateStandardEngineReplicaVersion(v) => { + pb::proposal::Action::UpdateStandardEngineReplicaVersion(v.into()) + } } } } @@ -2831,6 +2837,26 @@ impl From for pb::BlessAlternativeGuestOsVe } } +impl From for api::UpdateStandardEngineReplicaVersion { + fn from(item: pb::UpdateStandardEngineReplicaVersion) -> Self { + Self { + new_replica_version_id: Some(item.new_replica_version_id), + old_replica_version_id: Some(item.old_replica_version_id), + deployment_progress: Some(item.deployment_progress), + } + } +} + +impl From for pb::UpdateStandardEngineReplicaVersion { + fn from(item: api::UpdateStandardEngineReplicaVersion) -> Self { + Self { + new_replica_version_id: item.new_replica_version_id.unwrap_or_default(), + old_replica_version_id: item.old_replica_version_id.unwrap_or_default(), + deployment_progress: item.deployment_progress.unwrap_or_default(), + } + } +} + impl From for api::LoadCanisterSnapshot { fn from(item: pb::LoadCanisterSnapshot) -> Self { Self { diff --git a/rs/nns/governance/src/pb/proposal_conversions.rs b/rs/nns/governance/src/pb/proposal_conversions.rs index ff34ce9da023..79fd1aa8294f 100644 --- a/rs/nns/governance/src/pb/proposal_conversions.rs +++ b/rs/nns/governance/src/pb/proposal_conversions.rs @@ -273,6 +273,9 @@ fn convert_action( pb::proposal::Action::LoadCanisterSnapshot(v) => { api::proposal::Action::LoadCanisterSnapshot(v.clone().into()) } + pb::proposal::Action::UpdateStandardEngineReplicaVersion(v) => { + api::proposal::Action::UpdateStandardEngineReplicaVersion(v.clone().into()) + } // The action types with potentially large fields need to be converted in a way that avoids // cloning the action first. diff --git a/rs/nns/governance/src/proposals/mod.rs b/rs/nns/governance/src/proposals/mod.rs index ab3e45c2be9a..b591549aef21 100644 --- a/rs/nns/governance/src/proposals/mod.rs +++ b/rs/nns/governance/src/proposals/mod.rs @@ -5,8 +5,8 @@ use crate::{ CreateServiceNervousSystem, DeregisterKnownNeuron, GovernanceError, InstallCode, KnownNeuron, LoadCanisterSnapshot, ManageNeuron, Motion, NetworkEconomics, ProposalData, RewardNodeProvider, RewardNodeProviders, SelfDescribingProposalAction, StopOrStartCanister, - TakeCanisterSnapshot, Topic, UpdateCanisterSettings, Vote, governance_error::ErrorType, - proposal::Action, + TakeCanisterSnapshot, Topic, UpdateCanisterSettings, UpdateStandardEngineReplicaVersion, + Vote, governance_error::ErrorType, proposal::Action, }, proposals::{ add_or_remove_node_provider::ValidAddOrRemoveNodeProvider, @@ -36,6 +36,7 @@ pub mod self_describing; pub mod stop_or_start_canister; pub mod take_canister_snapshot; pub mod update_canister_settings; +pub mod update_standard_engine_replica_version; pub mod wasm_module; mod decode_candid_args_to_self_describing_value; @@ -64,6 +65,7 @@ pub(crate) enum ValidProposalAction { TakeCanisterSnapshot(TakeCanisterSnapshot), LoadCanisterSnapshot(LoadCanisterSnapshot), CreateCanisterAndInstallCode(CreateCanisterAndInstallCode), + UpdateStandardEngineReplicaVersion(UpdateStandardEngineReplicaVersion), } impl TryFrom> for ValidProposalAction { @@ -133,6 +135,11 @@ impl TryFrom> for ValidProposalAction { Action::CreateCanisterAndInstallCode(create_canister_and_install_code) => Ok( ValidProposalAction::CreateCanisterAndInstallCode(create_canister_and_install_code), ), + Action::UpdateStandardEngineReplicaVersion(update_standard_engine_replica_version) => { + Ok(ValidProposalAction::UpdateStandardEngineReplicaVersion( + update_standard_engine_replica_version, + )) + } // Obsolete actions Action::SetDefaultFollowees(_) => Err(GovernanceError::new_with_message( @@ -186,6 +193,9 @@ impl ValidProposalAction { ValidProposalAction::CreateCanisterAndInstallCode(create_canister_and_install_code) => { create_canister_and_install_code.valid_topic()? } + ValidProposalAction::UpdateStandardEngineReplicaVersion( + update_standard_engine_replica_version, + ) => update_standard_engine_replica_version.valid_topic(), }; Ok(topic) } @@ -282,6 +292,11 @@ impl ValidProposalAction { create_canister_and_install_code.abridge(), )) } + ValidProposalAction::UpdateStandardEngineReplicaVersion( + update_standard_engine_replica_version, + ) => Ok(SelfDescribingProposalAction::from( + update_standard_engine_replica_version.clone(), + )), ValidProposalAction::RewardNodeProvider(reward_node_provider) => Ok( SelfDescribingProposalAction::from(reward_node_provider.clone()), ), diff --git a/rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs b/rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs new file mode 100644 index 000000000000..1da77d40b0dd --- /dev/null +++ b/rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs @@ -0,0 +1,122 @@ +use crate::{ + pb::v1::{GovernanceError, SelfDescribingValue, Topic, UpdateStandardEngineReplicaVersion}, + proposals::{ + call_canister::CallCanister, + invalid_proposal_error, + self_describing::{DocumentedAction, ValueBuilder}, + }, +}; + +use candid::Encode; +use ic_base_types::CanisterId; +use ic_nervous_system_ids::is_potential_full_git_commit_id; +use ic_nns_constants::REGISTRY_CANISTER_ID; +use registry_canister::mutations::do_update_standard_engine_replica_version::UpdateStandardEngineReplicaVersionPayload; + +impl UpdateStandardEngineReplicaVersion { + /// Passing this validation does NOT guarantee that Registry will accept the + /// change. E.g. if at the time of proposal execution (or creation), one of + /// the versions is not elected, then, no changes will be made in Registry. + pub fn validate(&self) -> Result<(), GovernanceError> { + let Self { + new_replica_version_id, + old_replica_version_id, + deployment_progress, + } = self; + + // Replica version IDs must be plausible full git commit IDs. + if !is_potential_full_git_commit_id(new_replica_version_id) { + return Err(invalid_proposal_error(&format!( + "new_replica_version_id is not a 40 character hexidecimal string (it was {:?})", + new_replica_version_id, + ))); + } + if !is_potential_full_git_commit_id(old_replica_version_id) { + return Err(invalid_proposal_error(&format!( + "old_replica_version_id is not a 40 character hexidecimal string (it was {:?})", + old_replica_version_id, + ))); + } + + // Replica version IDs must differ. + if old_replica_version_id == new_replica_version_id { + return Err(invalid_proposal_error(&format!( + "new_replica_version_id and old_replica_version_id must not be equal (both were {:?})", + new_replica_version_id, + ))); + } + + // deployment_progress must be in the closed interval [0.0, 1.0]. + if !(0.0..=1.0).contains(deployment_progress) { + return Err(invalid_proposal_error(&format!( + "deployment_progress must be in the closed interval [0.0, 1.0], but got {}", + deployment_progress, + ))); + } + + Ok(()) + } + + pub fn valid_topic(&self) -> Topic { + Topic::IcOsVersionDeployment + } +} + +impl CallCanister for UpdateStandardEngineReplicaVersion { + type Reply = (); + + fn canister_and_function(&self) -> Result<(CanisterId, &str), GovernanceError> { + Ok(( + REGISTRY_CANISTER_ID, + "update_standard_engine_replica_version", + )) + } + + fn payload(&self) -> Result, GovernanceError> { + let payload = UpdateStandardEngineReplicaVersionPayload::from(self.clone()); + + Encode!(&payload) + .map_err(|err| invalid_proposal_error(&format!("Failed to encode payload: {err}"))) + } +} + +impl From for UpdateStandardEngineReplicaVersionPayload { + fn from(original: UpdateStandardEngineReplicaVersion) -> Self { + let UpdateStandardEngineReplicaVersion { + new_replica_version_id, + old_replica_version_id, + deployment_progress, + } = original; + + Self { + new_replica_version_id, + old_replica_version_id, + deployment_progress, + } + } +} + +impl DocumentedAction for UpdateStandardEngineReplicaVersion { + const NAME: &'static str = "Update Standard Engine Replica Version"; + const DESCRIPTION: &'static str = "Change what replica version(s) are run by Cloud Engines."; +} + +impl From for SelfDescribingValue { + fn from(original: UpdateStandardEngineReplicaVersion) -> Self { + let UpdateStandardEngineReplicaVersion { + new_replica_version_id, + old_replica_version_id, + deployment_progress, + } = original; + + ValueBuilder::new() + .add_field("new_replica_version_id", new_replica_version_id) + .add_field("old_replica_version_id", old_replica_version_id) + .add_field("deployment_progress", deployment_progress.to_string()) + .build() + } +} + +#[cfg(test)] +#[path = "./update_standard_engine_replica_version_tests.rs"] +mod tests; diff --git a/rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs b/rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs new file mode 100644 index 000000000000..b00e888dc4b6 --- /dev/null +++ b/rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs @@ -0,0 +1,163 @@ +use super::*; + +use crate::{ + pb::v1::{ + SelfDescribingValue as SelfDescribingValuePb, UpdateStandardEngineReplicaVersion, + governance_error::ErrorType, proposal::Action, + }, + proposals::ValidProposalAction, +}; + +use candid::Decode; +use ic_nns_governance_api::SelfDescribingValue; +use lazy_static::lazy_static; +use maplit::hashmap; + +lazy_static! { + static ref VALID_UPDATE: UpdateStandardEngineReplicaVersion = + UpdateStandardEngineReplicaVersion { + new_replica_version_id: "1234567890".repeat(4), + old_replica_version_id: "abcd".repeat(10), + deployment_progress: 0.1, + }; +} + +#[test] +fn test_valid_update_standard_engine_replica_version() { + assert_eq!(VALID_UPDATE.validate(), Ok(())); +} + +#[track_caller] +fn assert_invalid_update(update: UpdateStandardEngineReplicaVersion, keywords: &[&str]) { + let error = update.validate().unwrap_err(); + assert_eq!(error.error_type, ErrorType::InvalidProposal as i32); + + for keyword in keywords { + let error_message = error.error_message.to_lowercase(); + assert!( + error_message.contains(keyword), + "{keyword} not found in {error_message:#?}" + ); + } +} + +#[test] +fn test_invalid_update_standard_engine_replica_version() { + // Reject garbage replica version IDs. + assert_invalid_update( + UpdateStandardEngineReplicaVersion { + new_replica_version_id: "g@rbage".to_string(), + ..VALID_UPDATE.clone() + }, + &["new_replica_version_id", "40", "hexidecimal"], + ); + assert_invalid_update( + UpdateStandardEngineReplicaVersion { + old_replica_version_id: "not_a_git_commit_id".to_string(), + ..VALID_UPDATE.clone() + }, + &["old_replica_version_id", "40", "hexidecimal"], + ); + + // Replica versions must differ. + assert_invalid_update( + UpdateStandardEngineReplicaVersion { + old_replica_version_id: VALID_UPDATE.new_replica_version_id.clone(), + ..VALID_UPDATE.clone() + }, + &["new_replica_version_id", "old_replica_version_id", "equal"], + ); + + // deployment_progress out of range. + assert_invalid_update( + UpdateStandardEngineReplicaVersion { + deployment_progress: 1.1, + ..VALID_UPDATE.clone() + }, + &["deployment_progress", "[0.0, 1.0]"], + ); + assert_invalid_update( + UpdateStandardEngineReplicaVersion { + deployment_progress: -0.1, + ..VALID_UPDATE.clone() + }, + &["deployment_progress", "[0.0, 1.0]"], + ); +} + +#[test] +fn test_update_standard_engine_replica_version_boundary_progress_values_are_valid() { + assert_eq!( + UpdateStandardEngineReplicaVersion { + deployment_progress: 0.0, + ..VALID_UPDATE.clone() + } + .validate(), + Ok(()) + ); + + assert_eq!( + UpdateStandardEngineReplicaVersion { + deployment_progress: 1.0, + ..VALID_UPDATE.clone() + } + .validate(), + Ok(()) + ); +} + +#[test] +fn test_update_standard_engine_replica_version_topic_and_dispatch() { + assert_eq!(VALID_UPDATE.valid_topic(), Topic::IcOsVersionDeployment); + assert_eq!( + VALID_UPDATE.canister_and_function(), + Ok(( + REGISTRY_CANISTER_ID, + "update_standard_engine_replica_version" + )) + ); + + let decoded_payload = Decode!( + &VALID_UPDATE.payload().unwrap(), + UpdateStandardEngineReplicaVersionPayload + ) + .unwrap(); + assert_eq!( + decoded_payload, + UpdateStandardEngineReplicaVersionPayload { + new_replica_version_id: VALID_UPDATE.new_replica_version_id.clone(), + old_replica_version_id: VALID_UPDATE.old_replica_version_id.clone(), + deployment_progress: VALID_UPDATE.deployment_progress, + } + ); +} + +#[test] +fn test_update_standard_engine_replica_version_to_self_describing() { + let value = SelfDescribingValue::from(SelfDescribingValuePb::from(VALID_UPDATE.clone())); + + assert_eq!( + value, + SelfDescribingValue::Map(hashmap! { + "new_replica_version_id".to_string() => + SelfDescribingValue::from(VALID_UPDATE.new_replica_version_id.as_str()), + "old_replica_version_id".to_string() => + SelfDescribingValue::from(VALID_UPDATE.old_replica_version_id.as_str()), + "deployment_progress".to_string() => + SelfDescribingValue::from(VALID_UPDATE.deployment_progress.to_string().as_str()), + }) + ); +} + +#[test] +fn test_valid_proposal_action_conversion() { + let action = ValidProposalAction::try_from(Some(Action::UpdateStandardEngineReplicaVersion( + VALID_UPDATE.clone(), + ))) + .unwrap(); + + assert_eq!( + action, + ValidProposalAction::UpdateStandardEngineReplicaVersion(VALID_UPDATE.clone()) + ); +} diff --git a/rs/nns/governance/unreleased_changelog.md b/rs/nns/governance/unreleased_changelog.md index 94126a0ff421..51f56748c20e 100644 --- a/rs/nns/governance/unreleased_changelog.md +++ b/rs/nns/governance/unreleased_changelog.md @@ -9,6 +9,9 @@ on the process that this file is part of, see ## Added +* Added a new proposal type: `UpdateStandardEngineReplicaVersion`. Change what + replica version(s) are run by Cloud Engines. + ## Changed ## Deprecated From 29e05f1c82a5b6ebad85b6833f8026f5a9d7118a Mon Sep 17 00:00:00 2001 From: daniel-wong-dfinity-org-twin Date: Thu, 23 Jul 2026 17:58:03 +0200 Subject: [PATCH 2/3] It's spelled "hexadecimal" not "hexidecimal". Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/proposals/update_standard_engine_replica_version.rs | 4 ++-- .../proposals/update_standard_engine_replica_version_tests.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs b/rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs index 1da77d40b0dd..4cf152e7798d 100644 --- a/rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs +++ b/rs/nns/governance/src/proposals/update_standard_engine_replica_version.rs @@ -27,13 +27,13 @@ impl UpdateStandardEngineReplicaVersion { // Replica version IDs must be plausible full git commit IDs. if !is_potential_full_git_commit_id(new_replica_version_id) { return Err(invalid_proposal_error(&format!( - "new_replica_version_id is not a 40 character hexidecimal string (it was {:?})", + "new_replica_version_id is not a 40-character hexadecimal string (it was {:?})", new_replica_version_id, ))); } if !is_potential_full_git_commit_id(old_replica_version_id) { return Err(invalid_proposal_error(&format!( - "old_replica_version_id is not a 40 character hexidecimal string (it was {:?})", + "old_replica_version_id is not a 40-character hexadecimal string (it was {:?})", old_replica_version_id, ))); } diff --git a/rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs b/rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs index b00e888dc4b6..154d18fb5060 100644 --- a/rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs +++ b/rs/nns/governance/src/proposals/update_standard_engine_replica_version_tests.rs @@ -49,14 +49,14 @@ fn test_invalid_update_standard_engine_replica_version() { new_replica_version_id: "g@rbage".to_string(), ..VALID_UPDATE.clone() }, - &["new_replica_version_id", "40", "hexidecimal"], + &["new_replica_version_id", "40", "hexadecimal"], ); assert_invalid_update( UpdateStandardEngineReplicaVersion { old_replica_version_id: "not_a_git_commit_id".to_string(), ..VALID_UPDATE.clone() }, - &["old_replica_version_id", "40", "hexidecimal"], + &["old_replica_version_id", "40", "hexadecimal"], ); // Replica versions must differ. From 72fc571d04dac7efe8d0ed482d7c39295f0dce24 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Thu, 23 Jul 2026 18:47:07 +0000 Subject: [PATCH 3/3] Reject proposals where deployment_progress is not explicitly specified. --- rs/nns/governance/src/pb/conversions/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index c98debec1378..78a7208d5859 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -2850,9 +2850,16 @@ impl From for api::UpdateStandardEngineR impl From for pb::UpdateStandardEngineReplicaVersion { fn from(item: api::UpdateStandardEngineReplicaVersion) -> Self { Self { + // These are string fields. Therefore, if no value is supplied, + // unwrap_or_default returns an empty string, which will be rejected + // later (during proposal creation time), because we require that + // these fields have the shape of a git commit ID. new_replica_version_id: item.new_replica_version_id.unwrap_or_default(), old_replica_version_id: item.old_replica_version_id.unwrap_or_default(), - deployment_progress: item.deployment_progress.unwrap_or_default(), + // -1.0 is a "poison" value. That way, we do not make the unfounded + // assumption that the user intended that deployment_progress be set + // to 0.0. + deployment_progress: item.deployment_progress.unwrap_or(-1.0), } } }