diff --git a/Cargo.lock b/Cargo.lock index f100680a3b7..fffd02201ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9993,6 +9993,7 @@ dependencies = [ "ic-xrc-types", "icp-ledger", "icrc-ledger-types", + "itertools 0.12.1", "lazy_static", "lifeline", "maplit", diff --git a/rs/nervous_system/integration_tests/BUILD.bazel b/rs/nervous_system/integration_tests/BUILD.bazel index b0b91c93f81..39f034b70c5 100644 --- a/rs/nervous_system/integration_tests/BUILD.bazel +++ b/rs/nervous_system/integration_tests/BUILD.bazel @@ -29,6 +29,7 @@ BASE_DEPENDENCIES = [ "@crate_index//:assert_matches", "@crate_index//:candid", "@crate_index//:futures", + "@crate_index//:itertools", "@crate_index//:lazy_static", "@crate_index//:prost", "@crate_index//:rust_decimal", diff --git a/rs/nervous_system/integration_tests/Cargo.toml b/rs/nervous_system/integration_tests/Cargo.toml index 8bd1c87eedf..35e0c69236a 100644 --- a/rs/nervous_system/integration_tests/Cargo.toml +++ b/rs/nervous_system/integration_tests/Cargo.toml @@ -28,6 +28,7 @@ ic-sns-root = { path = "../../sns/root" } ic-sns-swap = { path = "../../sns/swap" } icp-ledger = { path = "../../ledger_suite/icp" } icrc-ledger-types = { path = "../../../packages/icrc-ledger-types" } +itertools = { workspace = true } lazy_static = { workspace = true } lifeline = { path = "../../nns/handlers/lifeline/impl" } pocket-ic = { path = "../../../packages/pocket-ic" } diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index fb410b3b6d4..56e13402956 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -58,6 +58,8 @@ use icrc_ledger_types::icrc1::{ account::Account, transfer::{TransferArg, TransferError}, }; +use itertools::EitherOrBoth; +use itertools::Itertools; use maplit::btreemap; use pocket_ic::{ management_canister::CanisterSettings, nonblocking::PocketIc, ErrorCode, PocketIcBuilder, @@ -703,6 +705,59 @@ pub async fn upgrade_nns_canister_to_tip_of_master_or_panic( ); } +/// Advances time by up to `timeout_seconds` seconds and `timeout_seconds` tickets (1 tick = 1 second). +/// Each tick, it observes the state using the provided `observe` function. +/// If the observed state matches the `expected` state, it returns `Ok(())`. +/// If the timeout is reached, it returns an error. +/// +/// Example: +/// ``` +/// let upgrade_journal_interval_seconds = 60 * 60; +/// await_with_timeout( +/// &pocket_ic, +/// upgrade_journal_interval_seconds, +/// |pocket_ic| async { +/// sns::governance::get_upgrade_journal(pocket_ic, sns.governance.canister_id) +/// .await +/// .upgrade_steps +/// .unwrap() +/// .versions +/// }, +/// &vec![initial_sns_version.clone()], +/// ) +/// .await +/// .unwrap(); +/// ``` +pub async fn await_with_timeout<'a, T, F, Fut>( + pocket_ic: &'a PocketIc, + timeout_seconds: u64, + observe: F, + expected: &T, +) -> Result<(), String> +where + T: std::cmp::PartialEq + std::fmt::Debug, + F: Fn(&'a PocketIc) -> Fut, + Fut: std::future::Future, +{ + let mut counter = 0; + loop { + pocket_ic.advance_time(Duration::from_secs(1)).await; + pocket_ic.tick().await; + + let observed = observe(pocket_ic).await; + if observed == *expected { + return Ok(()); + } + if counter == timeout_seconds { + return Err(format!( + "Observed state: {:?}\n!= Expected state {:?}\nafter {} seconds / rounds", + observed, expected, timeout_seconds, + )); + } + counter += 1; + } +} + pub mod nns { use super::*; pub mod governance { @@ -1469,7 +1524,8 @@ pub mod sns { Decode!(&result, sns_pb::ListNeuronsResponse).unwrap() } - /// Searches for the ID and controller principal of an SNS neuron that can submit proposals. + /// Searches for the ID and controller principal of an SNS neuron that can submit proposals, + /// i.e., a neuron whose `dissolve_delay_seconds` is greater that or equal 6 months. pub async fn find_neuron_with_majority_voting_power( pocket_ic: &PocketIc, canister_id: PrincipalId, @@ -1514,14 +1570,49 @@ pub mod sns { Decode!(&result, sns_pb::NervousSystemParameters).unwrap() } + pub async fn propose_to_advance_sns_target_version( + pocket_ic: &PocketIc, + sns_governance_canister_id: PrincipalId, + ) -> Result { + // Get an ID of an SNS neuron that can submit proposals. We rely on the fact that this + // neuron either holds the majority of the voting power or the follow graph is set up + // s.t. when this neuron submits a proposal, that proposal gets through without the need + // for any voting. + let (sns_neuron_id, sns_neuron_principal_id) = + sns::governance::find_neuron_with_majority_voting_power( + pocket_ic, + sns_governance_canister_id, + ) + .await + .expect("cannot find SNS neuron with dissolve delay over 6 months."); + + sns::governance::propose_and_wait( + pocket_ic, + sns_governance_canister_id, + sns_neuron_principal_id, + sns_neuron_id.clone(), + sns_pb::Proposal { + title: "Advance SNS target version.".to_string(), + summary: "".to_string(), + url: "".to_string(), + action: Some(sns_pb::proposal::Action::AdvanceSnsTargetVersion( + sns_pb::AdvanceSnsTargetVersion { new_target: None }, + )), + }, + ) + .await + .map_err(|err| err.to_string()) + } + // Upgrade; one canister at a time. pub async fn propose_to_upgrade_sns_to_next_version_and_wait( pocket_ic: &PocketIc, sns_governance_canister_id: PrincipalId, ) { - // Get an ID of an SNS neuron that can submit proposals. We rely on the fact that this neuron - // either holds the majority of the voting power or the follow graph is set up s.t. when this - // neuron submits a proposal, that proposal gets through without the need for any voting. + // Get an ID of an SNS neuron that can submit proposals. We rely on the fact that this + // neuron either holds the majority of the voting power or the follow graph is set up + // s.t. when this neuron submits a proposal, that proposal gets through without the need + // for any voting. let (sns_neuron_id, sns_neuron_principal_id) = find_neuron_with_majority_voting_power(pocket_ic, sns_governance_canister_id) .await @@ -1626,6 +1717,47 @@ pub mod sns { .await .unwrap() } + + /// Verifies that the upgrade journal has the expected entries. + pub async fn assert_upgrade_journal( + pocket_ic: &PocketIc, + sns_governance_canister_id: PrincipalId, + expected_entries: &[sns_pb::upgrade_journal_entry::Event], + ) { + let sns_pb::GetUpgradeJournalResponse { + upgrade_journal, .. + } = sns::governance::get_upgrade_journal(pocket_ic, sns_governance_canister_id).await; + + let upgrade_journal = upgrade_journal.unwrap().entries; + + for (index, either_or_both) in upgrade_journal + .iter() + .zip_longest(expected_entries.iter()) + .enumerate() + { + let (actual, expected) = match either_or_both { + EitherOrBoth::Both(actual, expected) => (actual, expected), + EitherOrBoth::Left(actual) => panic!( + "Observed an unexpected journal entry at index {}: {:?}", + index, actual + ), + EitherOrBoth::Right(expected) => panic!( + "Did not observe an expected entry at index {}: {:?}", + index, expected + ), + }; + assert!(actual.timestamp_seconds.is_some()); + assert_eq!( + &actual + .event + .clone() + .map(|event| event.redact_human_readable()), + &Some(expected.clone().redact_human_readable()), + "Upgrade journal entry at index {} does not match", + index + ); + } + } } pub mod index_ng { diff --git a/rs/nervous_system/integration_tests/tests/advance_target_version.rs b/rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs similarity index 81% rename from rs/nervous_system/integration_tests/tests/advance_target_version.rs rename to rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs index eea43207655..6cdc20fbdde 100644 --- a/rs/nervous_system/integration_tests/tests/advance_target_version.rs +++ b/rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs @@ -1,5 +1,4 @@ -use ic_nervous_system_agent::sns::governance::GovernanceCanister; -use ic_nervous_system_integration_tests::pocket_ic_helpers::sns; +use ic_nervous_system_integration_tests::pocket_ic_helpers::{await_with_timeout, sns}; use ic_nervous_system_integration_tests::{ create_service_nervous_system_builder::CreateServiceNervousSystemBuilder, pocket_ic_helpers::{ @@ -15,75 +14,8 @@ use ic_sns_governance::{ }; use ic_sns_swap::pb::v1::Lifecycle; use ic_sns_wasm::pb::v1::SnsCanisterType; -use pocket_ic::nonblocking::PocketIc; use pocket_ic::PocketIcBuilder; use std::collections::BTreeMap; -use std::time::Duration; - -/// Verifies that the upgrade journal has the expected entries. -async fn assert_upgrade_journal( - pocket_ic: &PocketIc, - governance: GovernanceCanister, - expected_entries: &[sns_pb::upgrade_journal_entry::Event], -) { - let sns_pb::GetUpgradeJournalResponse { - upgrade_journal, .. - } = sns::governance::get_upgrade_journal(pocket_ic, governance.canister_id).await; - - let upgrade_journal = upgrade_journal.unwrap().entries; - assert_eq!(upgrade_journal.len(), expected_entries.len()); - - for (index, (actual, expected)) in upgrade_journal - .iter() - .zip(expected_entries.iter()) - .enumerate() - { - assert!(actual.timestamp_seconds.is_some()); - assert_eq!( - &actual - .event - .clone() - .map(|event| event.redact_human_readable()), - &Some(expected.clone().redact_human_readable()), - "Upgrade journal entry at index {} does not match", - index - ); - } -} - -/// Advances time by up to `timeout_seconds` seconds and `timeout_seconds` tickets (1 tick = 1 second). -/// Each tick, it observes the state using the provided `observe` function. -/// If the observed state matches the `expected` state, it returns `Ok(())`. -/// If the timeout is reached, it returns an error. -async fn await_with_timeout<'a, T, F, Fut>( - pocket_ic: &'a PocketIc, - timeout_seconds: u64, - observe: F, - expected: &T, -) -> Result<(), String> -where - T: std::cmp::PartialEq + std::fmt::Debug, - F: Fn(&'a PocketIc) -> Fut, - Fut: std::future::Future, -{ - let mut counter = 0; - loop { - pocket_ic.advance_time(Duration::from_secs(1)).await; - pocket_ic.tick().await; - - let observed = observe(pocket_ic).await; - if observed == *expected { - return Ok(()); - } - if counter == timeout_seconds { - return Err(format!( - "Observed state: {:?}\n!= Expected state {:?}\nafter {} seconds / rounds", - observed, expected, timeout_seconds, - )); - } - counter += 1; - } -} #[tokio::test] async fn test_get_upgrade_journal() { @@ -139,9 +71,9 @@ async fn test_get_upgrade_journal() { UpgradeStepsRefreshed::new(vec![initial_sns_version.clone()]), )); - assert_upgrade_journal( + sns::governance::assert_upgrade_journal( &pocket_ic, - sns.governance, + sns.governance.canister_id, &expected_upgrade_journal_entries, ) .await; @@ -228,30 +160,27 @@ async fn test_get_upgrade_journal() { ]), )); - assert_upgrade_journal( + sns::governance::assert_upgrade_journal( &pocket_ic, - sns.governance, + sns.governance.canister_id, &expected_upgrade_journal_entries, ) .await; } - // State 3: Advance the target version. - sns::governance::advance_target_version( - &pocket_ic, - sns.governance.canister_id, - new_sns_version_2.clone(), - ) - .await; + // State 3: Advance the target version via proposal. + sns::governance::propose_to_advance_sns_target_version(&pocket_ic, sns.governance.canister_id) + .await + .unwrap(); expected_upgrade_journal_entries.push(Event::TargetVersionSet(TargetVersionSet::new( None, Some(new_sns_version_2.clone()), ))); - assert_upgrade_journal( + sns::governance::assert_upgrade_journal( &pocket_ic, - sns.governance, + sns.governance.canister_id, &expected_upgrade_journal_entries, ) .await; @@ -318,9 +247,9 @@ async fn test_get_upgrade_journal() { ), ); - assert_upgrade_journal( + sns::governance::assert_upgrade_journal( &pocket_ic, - sns.governance, + sns.governance.canister_id, &expected_upgrade_journal_entries, ) .await; diff --git a/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs b/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs index af421192b6a..e159bfe22a5 100644 --- a/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs +++ b/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs @@ -1903,6 +1903,8 @@ pub mod governance { candid::CandidType, candid::Deserialize, comparable::Comparable, + Eq, + std::hash::Hash, serde::Serialize, Clone, PartialEq, diff --git a/rs/sns/governance/protobuf_generator/src/lib.rs b/rs/sns/governance/protobuf_generator/src/lib.rs index 5becbedcadc..9bb51a21592 100644 --- a/rs/sns/governance/protobuf_generator/src/lib.rs +++ b/rs/sns/governance/protobuf_generator/src/lib.rs @@ -44,6 +44,10 @@ pub fn generate_prost_files(proto: ProtoPaths<'_>, out: &Path) { "ic_sns_governance.pb.v1.NeuronId", "#[derive(Eq, std::hash::Hash)]", ); + config.type_attribute( + "ic_sns_governance.pb.v1.Governance.Version", + "#[derive(Eq, std::hash::Hash)]", + ); let mut apply_attribute = |attribute, type_names| { for type_name in type_names { diff --git a/rs/sns/governance/src/cached_upgrade_steps.rs b/rs/sns/governance/src/cached_upgrade_steps.rs new file mode 100644 index 00000000000..a29ce530659 --- /dev/null +++ b/rs/sns/governance/src/cached_upgrade_steps.rs @@ -0,0 +1,286 @@ +use crate::pb::v1::governance::CachedUpgradeSteps as CachedUpgradeStepsPb; +use crate::pb::v1::governance::Version; +use crate::pb::v1::governance::Versions; +use crate::sns_upgrade::SnsCanisterType; + +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct CachedUpgradeSteps { + current_version: Version, + subsequent_versions: Vec, + + response_timestamp_seconds: u64, + pub requested_timestamp_seconds: u64, +} + +impl From for CachedUpgradeStepsPb { + fn from(src: CachedUpgradeSteps) -> Self { + let CachedUpgradeSteps { + current_version, + subsequent_versions, + requested_timestamp_seconds, + response_timestamp_seconds, + } = src; + + let upgrade_steps = { + let mut versions = vec![current_version]; + versions.extend(subsequent_versions); + Some(Versions { versions }) + }; + + Self { + upgrade_steps, + requested_timestamp_seconds: Some(requested_timestamp_seconds), + response_timestamp_seconds: Some(response_timestamp_seconds), + } + } +} + +impl TryFrom<&CachedUpgradeStepsPb> for CachedUpgradeSteps { + type Error = String; + + fn try_from(src: &CachedUpgradeStepsPb) -> Result { + let CachedUpgradeStepsPb { + upgrade_steps: Some(upgrade_steps), + // requested_timestamp_seconds is not guaranteed to be <= response_timestamp_seconds, + // when requested_timestamp_seconds > response_timestamp_seconds it indicates a request + // for upgrade steps is in-flight. Thus we don't validate the relationship between them. + requested_timestamp_seconds, + response_timestamp_seconds, + } = src + else { + return Err("Cannot interpret CachedUpgradeSteps; \ + please specify the required field upgrade_steps" + .to_string()); + }; + + let Some((current_version, subsequent_versions)) = upgrade_steps.versions.split_first() + else { + return Err( + "Cannot interpret CachedUpgradeSteps: upgrade_steps must not be empty.".to_string(), + ); + }; + + let requested_timestamp_seconds = requested_timestamp_seconds.unwrap_or_default(); + + let response_timestamp_seconds = response_timestamp_seconds.unwrap_or_default(); + + Ok(Self { + current_version: current_version.clone(), + subsequent_versions: subsequent_versions.to_vec(), + requested_timestamp_seconds, + response_timestamp_seconds, + }) + } +} + +/// Formats the first 3 bytes of a hash as a hexadecimal string. Corresponds to 6 ascii symbols. +pub fn format_short_hash(hash: &[u8]) -> String { + hash.iter() + .take(3) + .map(|b| format!("{:02x}", b)) + .collect::>() + .join("") +} + +/// Formats the 32 bytes of a hash as a hexadecimal string. Corresponds to 64 ascii symbols. +pub fn format_full_hash(hash: &[u8]) -> String { + hash.iter() + .map(|b| format!("{:02x}", b)) + .collect::>() + .join("") +} + +pub fn render_two_versions_as_markdown_table( + current_version: &Version, + target_version: &Version, +) -> String { + let long_line = "-".repeat(64); + let current_column_label = format!("{:<64}", "Current version's module hash"); + let target_column_label = format!("{:<64}", "New target version's module hash"); + format!( + "| Canister | {current_column_label} | {target_column_label} |\n\ + |------------|-{long_line}-|-{long_line}-|\n\ + | Root | {} | {} |\n\ + | Governance | {} | {} |\n\ + | Swap | {} | {} |\n\ + | Index | {} | {} |\n\ + | Ledger | {} | {} |\n\ + | Archive | {} | {} |", + format_full_hash(¤t_version.root_wasm_hash[..]), + format_full_hash(&target_version.root_wasm_hash[..]), + format_full_hash(¤t_version.governance_wasm_hash[..]), + format_full_hash(&target_version.governance_wasm_hash[..]), + format_full_hash(¤t_version.swap_wasm_hash[..]), + format_full_hash(&target_version.swap_wasm_hash[..]), + format_full_hash(¤t_version.index_wasm_hash[..]), + format_full_hash(&target_version.index_wasm_hash[..]), + format_full_hash(¤t_version.ledger_wasm_hash[..]), + format_full_hash(&target_version.ledger_wasm_hash[..]), + format_full_hash(¤t_version.archive_wasm_hash[..]), + format_full_hash(&target_version.archive_wasm_hash[..]), + ) +} + +impl std::fmt::Display for SnsCanisterType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let value = match self { + SnsCanisterType::Unspecified => "Unspecified", + SnsCanisterType::Root => "Root", + SnsCanisterType::Governance => "Governance", + SnsCanisterType::Swap => "Swap", + SnsCanisterType::Index => "Index", + SnsCanisterType::Ledger => "Ledger", + SnsCanisterType::Archive => "Archive", + }; + write!(f, "{}", value) + } +} + +fn render_sns_canister_change( + canister_changes: Vec<(SnsCanisterType, Vec /*wasm hash*/)>, +) -> String { + canister_changes + .into_iter() + .map(|(sns_canister_type, wasm_hash)| { + format!( + "{} @ {}", + sns_canister_type, + format_full_hash(&wasm_hash[..]) + ) + }) + .collect::>() + .join(",") +} + +/// Formats the version as a Markdown table row. +fn render_markdown_row(index: usize, version: &Version, canister_changes: &str) -> String { + format!( + "| {:>4} | {} | {} | {} | {} | {} | {} | {} |", + index, + format_short_hash(&version.root_wasm_hash[..]), + format_short_hash(&version.governance_wasm_hash[..]), + format_short_hash(&version.swap_wasm_hash[..]), + format_short_hash(&version.index_wasm_hash[..]), + format_short_hash(&version.ledger_wasm_hash[..]), + format_short_hash(&version.archive_wasm_hash[..]), + canister_changes, + ) +} + +impl std::fmt::Display for CachedUpgradeSteps { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + writeln!( + f, + "| Step | Root | Governance | Swap | Index | Ledger | Archive | Changes |\n\ + |------|------|------------|------|-------|--------|---------|---------|" + )?; + writeln!( + f, + "{}", + render_markdown_row(0, &self.current_version, "Current version") + )?; + + let mut previous_version = &self.current_version; + for (index, version) in self.subsequent_versions.iter().enumerate() { + let changes = previous_version.changes_against(version); + let changes = render_sns_canister_change(changes); + + // Index 0 corresponds to `current_version`. + let index = index.saturating_add(1); + + writeln!(f, "{}", render_markdown_row(index, version, &changes))?; + + previous_version = version; + } + + Ok(()) + } +} + +impl CachedUpgradeSteps { + pub fn last(&self) -> &Version { + self.subsequent_versions + .last() + .unwrap_or(&self.current_version) + } + + pub fn contains(&self, version: &Version) -> bool { + &self.current_version == version || self.subsequent_versions.contains(version) + } + + pub fn contains_in_order(&self, left: &Version, right: &Version) -> Result { + if !self.contains(left) { + return Err(format!("{:?} does not contain {:?}", self, left)); + } + if !self.contains(right) { + return Err(format!("{:?} does not contain {:?}", self, right)); + } + + // Check if we have `current_version` -> ... -> `left` -> `right` -> ... + let upgrade_steps_starting_from_left = self.clone().take_from(left)?; + + let contains_in_order = upgrade_steps_starting_from_left.contains(right); + + Ok(contains_in_order) + } + + pub fn current(&self) -> &Version { + &self.current_version + } + + pub fn is_current(&self, version: &Version) -> bool { + version == self.current() + } + + /// Returns whether there are no pending upgrades. + pub fn is_empty(&self) -> bool { + self.subsequent_versions.is_empty() + } + + /// Returns a new instance of `Self` starting with `version` in the `Ok` result + /// or `Err` if `!self.contains(version)`. + pub fn take_from(self, new_current_version: &Version) -> Result { + if self.is_current(new_current_version) { + return Ok(self); + } + + let Self { + current_version: _current_version, + subsequent_versions, + response_timestamp_seconds, + requested_timestamp_seconds, + } = self; + + let mut subsequent_versions = subsequent_versions.into_iter(); + while let Some(current_version) = subsequent_versions.next() { + if new_current_version == ¤t_version { + return Ok(Self { + current_version, + subsequent_versions: subsequent_versions.collect(), + response_timestamp_seconds, + requested_timestamp_seconds, + }); + } + } + + Err(format!( + "Cannot take_from {} that is not one of the cached upgrade steps.", + new_current_version + )) + } + + /// Approximate time at which this cache was valid. + pub fn approximate_time_of_validity_timestamp_seconds(&self) -> u64 { + self.response_timestamp_seconds + } + + pub fn validate_new_target_version(&self, new_target: &Version) -> Result<(), String> { + if !self.contains(new_target) { + return Err("new_target_version must be among the upgrade steps.".to_string()); + } + if self.is_current(new_target) { + return Err("new_target_version must differ from the current version.".to_string()); + } + Ok(()) + } +} diff --git a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs index af421192b6a..e159bfe22a5 100644 --- a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs +++ b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs @@ -1903,6 +1903,8 @@ pub mod governance { candid::CandidType, candid::Deserialize, comparable::Comparable, + Eq, + std::hash::Hash, serde::Serialize, Clone, PartialEq, diff --git a/rs/sns/governance/src/governance.rs b/rs/sns/governance/src/governance.rs index dc28bef9eab..9ab3546701a 100644 --- a/rs/sns/governance/src/governance.rs +++ b/rs/sns/governance/src/governance.rs @@ -1,3 +1,4 @@ +use crate::cached_upgrade_steps::CachedUpgradeSteps; use crate::{ canister_control::{ get_canister_id, perform_execute_generic_nervous_system_function_call, @@ -20,8 +21,8 @@ use crate::{ governance::{ self, neuron_in_flight_command::{self, Command as InFlightCommand}, - CachedUpgradeSteps, MaturityModulation, NeuronInFlightCommand, PendingVersion, - SnsMetadata, Version, Versions, + CachedUpgradeSteps as CachedUpgradeStepsPb, MaturityModulation, + NeuronInFlightCommand, PendingVersion, SnsMetadata, Version, Versions, }, governance_error::ErrorType, manage_neuron::{ @@ -433,12 +434,32 @@ impl GovernanceProto { result } + pub(crate) fn cached_upgrade_steps_or_err(&self) -> Result { + let Some(cached_upgrade_steps) = &self.cached_upgrade_steps else { + return Err( + "Internal error: GovernanceProto.cached_upgrade_steps must be specified." + .to_string(), + ); + }; + + let cached_upgrade_steps = CachedUpgradeSteps::try_from(cached_upgrade_steps) + .map_err(|err| format!("Internal error: {}", err))?; + + Ok(cached_upgrade_steps) + } + + pub fn deployed_version_or_err(&self) -> Result { + if let Some(deployed_version) = &self.deployed_version { + Ok(deployed_version.clone()) + } else { + Err("GovernanceProto.deployed_version is not set.".to_string()) + } + } + /// Gets the current deployed version of the SNS or panics. pub fn deployed_version_or_panic(&self) -> Version { - self.deployed_version - .as_ref() - .cloned() - .expect("No version set in Governance.") + self.deployed_version_or_err() + .expect("GovernanceProto.deployed_version must be set.") } /// Returns 0 if maturity modulation is disabled (per @@ -2128,11 +2149,13 @@ impl Governance { self.perform_manage_dapp_canister_settings(manage_dapp_canister_settings) .await } - // TODO[NNS1-3434]: Implement `AdvanceSnsTargetVersion` proposals. - Action::AdvanceSnsTargetVersion(_) => Err(GovernanceError::new_with_message( - ErrorType::InvalidProposal, - "AdvanceSnsTargetVersion proposals are not implemented yet.".to_string(), - )), + Action::AdvanceSnsTargetVersion(_) => { + get_action_auxiliary(&self.proto.proposals, ProposalId { id: proposal_id }) + .and_then(|action_auxiliary| { + action_auxiliary.unwrap_advance_sns_target_version_or_err() + }) + .and_then(|new_target| self.perform_advance_target_version(new_target)) + } // This should not be possible, because Proposal validation is performed when // a proposal is first made. Action::Unspecified(_) => Err(GovernanceError::new_with_message( @@ -3024,6 +3047,32 @@ impl Governance { ) } + fn perform_advance_target_version( + &mut self, + new_target: Version, + ) -> Result<(), GovernanceError> { + // TODO[NNS1-3365]: Enable the AdvanceSnsTargetVersionFeature. + self.check_test_features_enabled(); + + let cached_upgrade_steps = self + .proto + .cached_upgrade_steps_or_err() + .map_err(|err| GovernanceError::new_with_message(ErrorType::NotFound, err))?; + + cached_upgrade_steps + .validate_new_target_version(&new_target) + .map_err(|err| GovernanceError::new_with_message(ErrorType::InvalidProposal, err))?; + + self.push_to_upgrade_journal(upgrade_journal_entry::TargetVersionSet::new( + self.proto.target_version.clone(), + Some(new_target.clone()), + )); + + self.proto.target_version = Some(new_target); + + Ok(()) + } + // Returns an option with the NervousSystemParameters fn nervous_system_parameters(&self) -> Option<&NervousSystemParameters> { self.proto.parameters.as_ref() @@ -4847,7 +4896,7 @@ impl Governance { return; }; - let Some(CachedUpgradeSteps { + let Some(CachedUpgradeStepsPb { upgrade_steps: Some(Versions { versions: upgrade_steps, }), @@ -4963,7 +5012,7 @@ impl Governance { let upgrade_steps = self.proto .cached_upgrade_steps - .get_or_insert_with(|| CachedUpgradeSteps { + .get_or_insert_with(|| CachedUpgradeStepsPb { requested_timestamp_seconds: Some(self.env.now()), ..Default::default() }); @@ -5058,16 +5107,20 @@ impl Governance { } } + // This is a test-only function, so panicking should be okay. pub fn advance_target_version( &mut self, request: AdvanceTargetVersionRequest, ) -> AdvanceTargetVersionResponse { - self.push_to_upgrade_journal(upgrade_journal_entry::TargetVersionSet::new( - self.proto.target_version.clone(), - request.target_version.clone(), - )); + let AdvanceTargetVersionRequest { + target_version: Some(target_version), + } = request + else { + panic!("AdvanceTargetVersionRequest.target_version must be specified."); + }; - self.proto.target_version = request.target_version; + self.perform_advance_target_version(target_version) + .expect("Cannot perform perform_advance_target_version"); AdvanceTargetVersionResponse {} } diff --git a/rs/sns/governance/src/lib.rs b/rs/sns/governance/src/lib.rs index 96ac60ff94a..779bef2cdee 100644 --- a/rs/sns/governance/src/lib.rs +++ b/rs/sns/governance/src/lib.rs @@ -1,6 +1,7 @@ use crate::pb::v1::Subaccount as SubaccountProto; use std::{convert::TryInto, fmt::Debug}; +mod cached_upgrade_steps; pub mod canister_control; pub mod governance; pub mod init; diff --git a/rs/sns/governance/src/proposal.rs b/rs/sns/governance/src/proposal.rs index 31eaebbe997..02e7f6f2aa1 100644 --- a/rs/sns/governance/src/proposal.rs +++ b/rs/sns/governance/src/proposal.rs @@ -1,3 +1,6 @@ +use crate::cached_upgrade_steps::render_two_versions_as_markdown_table; +use crate::cached_upgrade_steps::CachedUpgradeSteps; +use crate::pb::v1::AdvanceSnsTargetVersion; use crate::{ canister_control::perform_execute_generic_nervous_system_function_validate_and_render_call, governance::{ @@ -169,7 +172,7 @@ pub(crate) fn get_action_auxiliary( }) } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub(crate) enum ActionAuxiliary { TransferSnsTreasuryFunds(Valuation), MintSnsTokens(Valuation), @@ -192,6 +195,21 @@ impl ActionAuxiliary { )), } } + + pub fn unwrap_advance_sns_target_version_or_err(self) -> Result { + match self { + Self::AdvanceSnsTargetVersion(new_target) => Ok(new_target), + + wrong => Err(GovernanceError::new_with_message( + ErrorType::InconsistentInternalData, + format!( + "Missing supporting information. Specifically, \ + no new target version: {:#?}", + wrong, + ), + )), + } + } } /// Most proposal actions have no auxiliary data. In those cases, we would have @@ -466,9 +484,17 @@ pub(crate) async fn validate_and_render_action( proposal::Action::ManageDappCanisterSettings(manage_dapp_canister_settings) => { validate_and_render_manage_dapp_canister_settings(manage_dapp_canister_settings) } - proposal::Action::AdvanceSnsTargetVersion(_) => { - // TODO[NNS1-3434]: Implement Action::AdvanceSnsTargetVersion - return Err("Action::AdvanceSnsTargetVersion is not implemented yet.".to_string()); + proposal::Action::AdvanceSnsTargetVersion(advance_sns_target_version) => { + let deployed_version = governance_proto.deployed_version_or_err()?; + let cached_upgrade_steps = governance_proto.cached_upgrade_steps_or_err()?; + let upgrade_steps = cached_upgrade_steps.take_from(&deployed_version)?; + + return validate_and_render_advance_sns_target_version_proposal( + env.canister_id(), + upgrade_steps, + governance_proto.target_version.clone(), + advance_sns_target_version, + ); } } .map(|rendering| (rendering, ActionAuxiliary::None)) @@ -1696,6 +1722,83 @@ fn validate_and_render_manage_dapp_canister_settings( } } +/// Attempts to validate an `AdvanceSnsTargetVersion` action and render its human-readable text. +/// Invalidates the action in the following cases: +/// - There are no pending upgrades. +/// - `new_target` is equal to `current_version`. +/// - `new_target` comes before `current_target_version` along the `upgrade_steps`. +/// +/// Details: +/// 1. Validates the action's `new_target` field, if it is `Some(new_target)`. +/// 2. Identifies the `new_target`, either based on the above, or using `upgrade_steps`. +/// 3. Renders the Markdown proposal description. +/// 4. Returns the rendering and the identified `new_target` (guaranteed to be `Some`) +/// as `ActionAuxiliary`. This returned `new_target` should be used for executing this action, +/// assuming the proposal will be adopted. +fn validate_and_render_advance_sns_target_version_proposal( + sns_governance_canister_id: CanisterId, + upgrade_steps: CachedUpgradeSteps, + current_target_version: Option, + advance_sns_target_version: &AdvanceSnsTargetVersion, +) -> Result<(String, ActionAuxiliary), String> { + if upgrade_steps.is_empty() { + return Err( + "Cannot advance SNS target version: there are no pending upgrades.".to_string(), + ); + } + + let new_target = if let Some(new_target) = &advance_sns_target_version.new_target { + let new_target = Version::try_from(new_target.clone()).map_err(|err| { + format!( + "Cannot validate and render AdvanceSnsTargetVersion proposal: {}", + err + ) + })?; + + upgrade_steps.validate_new_target_version(&new_target)?; + + new_target + } else { + upgrade_steps.last().clone() + }; + + if let Some(current_target_version) = current_target_version { + let new_target_is_not_ahead_of_current_target = + upgrade_steps.contains_in_order(&new_target, ¤t_target_version)?; + if new_target_is_not_ahead_of_current_target { + return Err(format!( + "SNS target already set to version {}.", + current_target_version + )); + } + } + + let valid_timestamp_seconds = upgrade_steps.approximate_time_of_validity_timestamp_seconds(); + + let current_target_versions_render = + render_two_versions_as_markdown_table(upgrade_steps.current(), &new_target); + + let upgrade_journal_url_render = format!( + "https://{}.raw.icp0.io/journal/json", + sns_governance_canister_id, + ); + + let render = format!( + "# Proposal to advance SNS target version\n\n\ + {current_target_versions_render}\n\n\ + ### Upgrade steps\n\n\ + {upgrade_steps}\n\n\ + ### Monitoring the upgrade process\n\n\ + Please note: the upgrade steps above (valid around timestamp {valid_timestamp_seconds} \ + seconds) might change during this proposal's voting period. Such changes are unlikely and \ + are subject to NNS community's approval.\n\n\ + The **upgrade journal** provides up-to-date information on this SNS's upgrade process:\n\n\ + {upgrade_journal_url_render}" + ); + + Ok((render, ActionAuxiliary::AdvanceSnsTargetVersion(new_target))) +} + impl ProposalData { /// Returns the proposal's decision status. See [ProposalDecisionStatus] in the SNS's /// proto for more information. @@ -2472,6 +2575,9 @@ mod treasury_tests; #[cfg(test)] mod minting_tests; +#[cfg(test)] +mod advance_sns_target_version; + #[cfg(test)] mod tests { use super::*; diff --git a/rs/sns/governance/src/proposal/advance_sns_target_version.rs b/rs/sns/governance/src/proposal/advance_sns_target_version.rs new file mode 100644 index 00000000000..e689cdee996 --- /dev/null +++ b/rs/sns/governance/src/proposal/advance_sns_target_version.rs @@ -0,0 +1,353 @@ +use super::*; +use crate::pb::v1::governance::Versions; +use crate::pb::v1::governance::{CachedUpgradeSteps as CachedUpgradeStepsPb, Mode as ModePb}; +use crate::pb::v1::Governance as GovernancePb; +use crate::types::test_helpers::NativeEnvironment; +use futures::FutureExt; +use ic_test_utilities_types::ids::canister_test_id; + +fn sns_version_for_tests() -> Version { + Version { + root_wasm_hash: vec![ + 73, 94, 49, 55, 11, 20, 250, 97, 199, 107, 209, 72, 60, 159, 155, 166, 103, 51, 121, + 62, 226, 150, 62, 142, 68, 162, 49, 67, 106, 96, 188, 198, + ], + governance_wasm_hash: vec![ + 63, 235, 143, 247, 180, 127, 83, 218, 131, 35, 94, 76, 104, 103, 107, 182, 219, 84, + 223, 30, 98, 223, 54, 129, 222, 148, 37, 173, 92, 244, 59, 229, + ], + swap_wasm_hash: vec![ + 59, 180, 144, 209, 151, 184, 207, 46, 125, 153, 72, 188, 181, 209, 252, 70, 116, 122, + 131, 82, 148, 179, 255, 228, 123, 136, 45, 191, 165, 132, 85, 95, + ], + index_wasm_hash: vec![ + 8, 174, 80, 66, 200, 228, 19, 113, 109, 4, 160, 141, 184, 134, 184, 198, 176, 27, 182, + 16, 184, 25, 124, 219, 224, 82, 197, 149, 56, 185, 36, 240, + ], + ledger_wasm_hash: vec![ + 232, 148, 47, 86, 249, 67, 155, 137, 177, 59, 216, 3, 127, 53, 113, 38, 226, 79, 30, + 121, 50, 207, 3, 1, 130, 67, 52, 117, 5, 149, 159, 212, + ], + archive_wasm_hash: vec![ + 92, 89, 92, 42, 220, 127, 109, 153, 113, 41, 143, 238, 47, 166, 102, 146, 151, 17, 231, + 51, 65, 25, 42, 183, 8, 4, 199, 131, 160, 238, 224, 63, + ], + } +} + +fn standard_governance_proto_for_tests(deployed_version: Option) -> GovernancePb { + GovernancePb { + root_canister_id: Some(PrincipalId::from(canister_test_id(500))), + ledger_canister_id: Some(PrincipalId::from(canister_test_id(502))), + swap_canister_id: Some(PrincipalId::from(canister_test_id(503))), + + sns_metadata: None, + sns_initialization_parameters: "".to_string(), + parameters: Some(NervousSystemParameters::with_default_values()), + id_to_nervous_system_functions: BTreeMap::new(), + + neurons: Default::default(), + proposals: Default::default(), + + latest_reward_event: None, + in_flight_commands: Default::default(), + genesis_timestamp_seconds: 0, + metrics: None, + mode: ModePb::Normal.into(), + deployed_version, + pending_version: None, + is_finalizing_disburse_maturity: None, + maturity_modulation: None, + target_version: None, + timers: None, + upgrade_journal: None, + cached_upgrade_steps: None, + } +} + +#[test] +fn test_validate_and_render_advance_target_version_action() { + // Prepare the world. + let pre_deployed_version = sns_version_for_tests(); + let deployed_version = Version { + root_wasm_hash: vec![ + 67, 28, 179, 51, 254, 179, 247, 98, 247, 66, 176, 222, 165, 135, 69, 99, 58, 42, 44, + 164, 16, 117, 233, 147, 49, 131, 216, 80, 180, 221, 178, 89, + ], + ..pre_deployed_version.clone() + }; + let intermediate_version = Version { + governance_wasm_hash: vec![ + 131, 31, 108, 253, 195, 85, 209, 50, 78, 217, 59, 177, 168, 212, 177, 246, 163, 237, + 165, 7, 14, 89, 228, 112, 205, 253, 15, 45, 53, 222, 138, 136, + ], + ..deployed_version.clone() + }; + let expected_target_version = Version { + swap_wasm_hash: vec![ + 131, 19, 172, 34, 210, 239, 10, 12, 18, 144, 168, 91, 71, 242, 53, 207, 162, 76, 162, + 201, 109, 9, 91, 141, 190, 213, 80, 36, 131, 185, 205, 24, + ], + ..intermediate_version.clone() + }; + + // Smoke check: Make sure all versions are different + let versions = vec![ + pre_deployed_version, + deployed_version.clone(), + intermediate_version.clone(), + expected_target_version.clone(), + ]; + assert!( + versions.iter().collect::>().len() == versions.len(), + "Duplicates!" + ); + + let mut governance_proto = standard_governance_proto_for_tests(Some(deployed_version)); + governance_proto.cached_upgrade_steps = Some(CachedUpgradeStepsPb { + upgrade_steps: Some(Versions { versions }), + ..Default::default() + }); + let env = NativeEnvironment::new(Some(canister_test_id(501))); + + // Run code under test. + { + // Experiment A: Advance the target to the intermediate version. + let action = Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(intermediate_version.clone())), + }); + + let (_, action_auxiliary) = + validate_and_render_action(&Some(action), &env, &governance_proto, vec![]) + .now_or_never() + .unwrap() + .unwrap(); + + // Inspect the observed results. + assert_eq!( + action_auxiliary, + ActionAuxiliary::AdvanceSnsTargetVersion(intermediate_version) + ); + } + + // Experiments B, C: Advance the target to the latest available version (either implicitly + // or explicitly). + for action in [ + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { new_target: None }), + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(expected_target_version.clone())), + }), + ] { + let expected_target_version = expected_target_version.clone(); + + let (actual_text, action_auxiliary) = + validate_and_render_action(&Some(action), &env, &governance_proto, vec![]) + .now_or_never() + .unwrap() + .unwrap(); + + // Inspect the observed results. + assert_eq!( + action_auxiliary, + ActionAuxiliary::AdvanceSnsTargetVersion(expected_target_version) + ); + // Notice that there are only 3 expected upgrade steps (pre_deployed_version is gone). + assert_eq!( + actual_text, + r#"# Proposal to advance SNS target version + +| Canister | Current version's module hash | New target version's module hash | +|------------|------------------------------------------------------------------|------------------------------------------------------------------| +| Root | 431cb333feb3f762f742b0dea58745633a2a2ca41075e9933183d850b4ddb259 | 431cb333feb3f762f742b0dea58745633a2a2ca41075e9933183d850b4ddb259 | +| Governance | 3feb8ff7b47f53da83235e4c68676bb6db54df1e62df3681de9425ad5cf43be5 | 831f6cfdc355d1324ed93bb1a8d4b1f6a3eda5070e59e470cdfd0f2d35de8a88 | +| Swap | 3bb490d197b8cf2e7d9948bcb5d1fc46747a835294b3ffe47b882dbfa584555f | 8313ac22d2ef0a0c1290a85b47f235cfa24ca2c96d095b8dbed5502483b9cd18 | +| Index | 08ae5042c8e413716d04a08db886b8c6b01bb610b8197cdbe052c59538b924f0 | 08ae5042c8e413716d04a08db886b8c6b01bb610b8197cdbe052c59538b924f0 | +| Ledger | e8942f56f9439b89b13bd8037f357126e24f1e7932cf03018243347505959fd4 | e8942f56f9439b89b13bd8037f357126e24f1e7932cf03018243347505959fd4 | +| Archive | 5c595c2adc7f6d9971298fee2fa666929711e73341192ab70804c783a0eee03f | 5c595c2adc7f6d9971298fee2fa666929711e73341192ab70804c783a0eee03f | + +### Upgrade steps + +| Step | Root | Governance | Swap | Index | Ledger | Archive | Changes | +|------|------|------------|------|-------|--------|---------|---------| +| 0 | 431cb3 | 3feb8f | 3bb490 | 08ae50 | e8942f | 5c595c | Current version | +| 1 | 431cb3 | 831f6c | 3bb490 | 08ae50 | e8942f | 5c595c | Governance @ 831f6cfdc355d1324ed93bb1a8d4b1f6a3eda5070e59e470cdfd0f2d35de8a88 | +| 2 | 431cb3 | 831f6c | 8313ac | 08ae50 | e8942f | 5c595c | Swap @ 8313ac22d2ef0a0c1290a85b47f235cfa24ca2c96d095b8dbed5502483b9cd18 | + + +### Monitoring the upgrade process + +Please note: the upgrade steps above (valid around timestamp 0 seconds) might change during this proposal's voting period. Such changes are unlikely and are subject to NNS community's approval. + +The **upgrade journal** provides up-to-date information on this SNS's upgrade process: + +https://qys37-7yaaa-aaaaa-aah2q-cai.raw.icp0.io/journal/json"#, + ); + } +} + +#[test] +fn test_no_pending_upgrades() { + // Prepare the world. + let pre_deployed_version = sns_version_for_tests(); + let deployed_version = Version { + root_wasm_hash: vec![ + 67, 28, 179, 51, 254, 179, 247, 98, 247, 66, 176, 222, 165, 135, 69, 99, 58, 42, 44, + 164, 16, 117, 233, 147, 49, 131, 216, 80, 180, 221, 178, 89, + ], + ..pre_deployed_version.clone() + }; + let non_existent_version = Version { + root_wasm_hash: vec![ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, + 25, 26, 27, 28, 29, 30, + ], + ..deployed_version.clone() + }; + + // Smoke check: Make sure all versions are different + let versions = vec![pre_deployed_version.clone(), deployed_version.clone()]; + assert!( + versions.iter().collect::>().len() == versions.len(), + "Duplicates!" + ); + + let mut governance_proto = standard_governance_proto_for_tests(Some(deployed_version.clone())); + governance_proto.cached_upgrade_steps = Some(CachedUpgradeStepsPb { + upgrade_steps: Some(Versions { versions }), + ..Default::default() + }); + let env = NativeEnvironment::new(Some(canister_test_id(501))); + + // Run code under test. + for action in [ + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { new_target: None }), + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(deployed_version)), + }), + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(pre_deployed_version)), + }), + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(non_existent_version)), + }), + ] { + let err = validate_and_render_action(&Some(action), &env, &governance_proto, vec![]) + .now_or_never() + .unwrap() + .unwrap_err(); + + // Inspect the observed results. + assert_eq!( + err, + "Cannot advance SNS target version: there are no pending upgrades." + ); + } +} + +#[test] +fn test_invalid_new_targets() { + // Prepare the world. + let deployed_version = sns_version_for_tests(); + let next_version = Version { + root_wasm_hash: vec![ + 67, 28, 179, 51, 254, 179, 247, 98, 247, 66, 176, 222, 165, 135, 69, 99, 58, 42, 44, + 164, 16, 117, 233, 147, 49, 131, 216, 80, 180, 221, 178, 89, + ], + ..deployed_version.clone() + }; + let next_next_version = Version { + index_wasm_hash: vec![ + 103, 181, 240, 191, 18, 142, 128, 26, 223, 74, 149, 158, 162, 108, 60, 156, 160, 205, + 57, 153, 64, 225, 105, 162, 106, 46, 178, 55, 137, 154, 148, 221, + ], + ..deployed_version.clone() + }; + let non_existent_version = Version { + root_wasm_hash: vec![ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, + 25, 26, 27, 28, 29, 30, + ], + ..deployed_version.clone() + }; + + // Smoke check: Make sure all versions are different + let versions = vec![ + deployed_version.clone(), + next_version.clone(), + next_next_version.clone(), + ]; + assert!( + versions.iter().collect::>().len() == versions.len(), + "Duplicates!" + ); + + // Run code under test. + + for (label, current_target_version, action, expected_result) in [ + ( + "Scenario A: `new_target` is equal to `deployed_version`.", + None, + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(deployed_version.clone())), + }), + Err("new_target_version must differ from the current version.".to_string()), + ), + ( + "Scenario B: `new_target` is not a known version.", + None, + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(non_existent_version)), + }), + Err("new_target_version must be among the upgrade steps.".to_string()), + ), + ( + "Scenario C: `new_target` is equal to `current_target_version`.", + Some(next_version.clone()), + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(next_version.clone())), + }), + Err(format!( + "SNS target already set to version {}.", + next_version + )), + ), + ( + "Scenario D: `new_target` is behind `current_target_version`.", + Some(next_next_version.clone()), + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(next_version.clone())), + }), + Err(format!( + "SNS target already set to version {}.", + next_next_version + )), + ), + ( + "Scenario E: `new_target` is ahead of `current_target_version`.", + Some(next_version), + Action::AdvanceSnsTargetVersion(AdvanceSnsTargetVersion { + new_target: Some(SnsVersion::from(next_next_version.clone())), + }), + Ok(ActionAuxiliary::AdvanceSnsTargetVersion(next_next_version)), + ), + ] { + let mut governance_proto = + standard_governance_proto_for_tests(Some(deployed_version.clone())); + governance_proto.target_version = current_target_version; + governance_proto.cached_upgrade_steps = Some(CachedUpgradeStepsPb { + upgrade_steps: Some(Versions { + versions: versions.clone(), + }), + ..Default::default() + }); + let env = NativeEnvironment::new(Some(canister_test_id(501))); + + let result = validate_and_render_action(&Some(action), &env, &governance_proto, vec![]) + .now_or_never() + .unwrap() + .map(|(_, action_auxiliary)| action_auxiliary); + + // Inspect the observed results. + assert_eq!(result, expected_result, "{}", label); + } +} diff --git a/rs/sns/governance/src/sns_upgrade.rs b/rs/sns/governance/src/sns_upgrade.rs index 392e31d48fa..e99e7b94cc8 100644 --- a/rs/sns/governance/src/sns_upgrade.rs +++ b/rs/sns/governance/src/sns_upgrade.rs @@ -416,6 +416,7 @@ impl Version { differences } + pub(crate) fn version_has_expected_hashes( &self, expected_hashes: &[(SnsCanisterType, Vec /* wasm hash*/)],