From 7d5b9981ec17ea9f36a012960be29fa8fe10acf9 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Tue, 5 Nov 2024 17:02:32 +0100 Subject: [PATCH] feat(sns): SNS Governance upgrades its Swap (#2300) This PR makes SNS Governance actually upgrade its Swap. This is a step towards making Swaps proper SNS canisters. < [Previous PR](https://github.com/dfinity/ic/pull/2286) | --- .../src/pocket_ic_helpers.rs | 136 +++++++++--------- .../tests/sns_release_qualification.rs | 11 +- rs/sns/governance/src/governance.rs | 17 --- 3 files changed, 77 insertions(+), 87 deletions(-) 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 03497a5e2b0..aa2f9c48b0a 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -1123,11 +1123,25 @@ pub mod sns { use super::*; - pub async fn upgrade_sns_to_next_version_and_assert_change( + #[derive(Clone, Debug, PartialEq)] + pub enum SnsUpgradeError { + CanisterVersionMismatch { + canister_type: SnsCanisterType, + canister_version_from_sns_pov: Vec, + canister_version_from_ic00_pov: Vec, + is_pre_upgrade: bool, + }, + TargetCanisterVersionUnchanged { + pre_upgrade_canister_version: Vec, + post_upgrade_canister_version: Vec, + }, + } + + pub async fn try_upgrade_sns_to_next_version( pocket_ic: &PocketIc, sns_root_canister_id: PrincipalId, expected_type_to_change: SnsCanisterType, - ) { + ) -> Result<(), SnsUpgradeError> { let response = root::list_sns_canisters(pocket_ic, sns_root_canister_id).await; let ListSnsCanistersResponse { root: Some(sns_root_canister_id_1), @@ -1149,10 +1163,7 @@ pub mod sns { SnsCanisterType::Root => (sns_root_canister_id, sns_governance_canister_id), SnsCanisterType::Governance => (sns_governance_canister_id, sns_root_canister_id), SnsCanisterType::Ledger => (sns_ledger_canister_id, sns_root_canister_id), - SnsCanisterType::Swap => { - // The Swap canister is special in that it is controlled by the NNS, not SNS. - (swap_canister_id, ROOT_CANISTER_ID.get()) - } + SnsCanisterType::Swap => (swap_canister_id, sns_root_canister_id), SnsCanisterType::Archive => { let archive_canister_id = archives.last().expect( "Testing Archive canister upgrade requires some Archive canisters \ @@ -1173,26 +1184,26 @@ pub mod sns { .unwrap(); // Check that we get the same version from the management canister and from the SNS. - let pre_upgrade_running_canister_version = { - let running_version_for_canister = extract_sns_canister_version( + let pre_upgrade_canister_version = { + let canister_version_from_sns_pov = extract_sns_canister_version( pre_upgrade_running_version.clone(), expected_type_to_change, ); - let module_hash = pocket_ic + let canister_version_from_ic00_pov = pocket_ic .canister_status(canister_id.into(), Some(controller_id.into())) .await .unwrap() .module_hash .unwrap(); - assert_eq!( - running_version_for_canister, - module_hash, - "pre_upgrade: running_version_for_canister of type {} ({}) != module_hash ({})", - expected_type_to_change.as_str_name(), - fmt_bytes(&running_version_for_canister), - fmt_bytes(&module_hash), - ); - running_version_for_canister + if canister_version_from_sns_pov != canister_version_from_ic00_pov { + return Err(SnsUpgradeError::CanisterVersionMismatch { + canister_type: expected_type_to_change, + canister_version_from_sns_pov, + canister_version_from_ic00_pov, + is_pre_upgrade: true, + }); + } + canister_version_from_sns_pov }; governance::propose_to_upgrade_sns_to_next_version_and_wait( @@ -1206,70 +1217,57 @@ pub mod sns { pocket_ic.tick().await; } - let post_upgrade_running_version = + let post_upgrade_version = governance::get_running_sns_version(pocket_ic, sns_governance_canister_id) .await .deployed_version .unwrap(); - if expected_type_to_change == SnsCanisterType::Swap { - let wasm = nns::sns_wasm::get_wasm( - pocket_ic, - post_upgrade_running_version.swap_wasm_hash.clone(), - ) - .await - .wasm; - nns::governance::propose_and_wait( - pocket_ic, - MakeProposalRequest { - title: Some("Upgrade Swap from NNS Governance".to_string()), - summary: "".to_string(), - url: "".to_string(), - action: Some(ProposalActionRequest::InstallCode(InstallCodeRequest { - canister_id: Some(swap_canister_id), - install_mode: Some(CanisterInstallMode::Upgrade as i32), - wasm_module: Some(wasm), - arg: Some(vec![]), - skip_stopping_before_installing: None, - })), - }, - ) - .await - .expect("Proposal did not execute successfully"); - - for _ in 0..10 { - pocket_ic.tick().await; - pocket_ic.advance_time(Duration::from_secs(10)).await; - } - } - // Check that we get the same version from the management canister and from the SNS. - let post_upgrade_running_version_for_canister = { - let running_version_for_canister = - extract_sns_canister_version(post_upgrade_running_version, expected_type_to_change); - let module_hash = pocket_ic + let post_upgrade_canister_version = { + let canister_version_from_sns_pov = + extract_sns_canister_version(post_upgrade_version, expected_type_to_change); + let canister_version_from_ic00_pov = pocket_ic .canister_status(canister_id.into(), Some(controller_id.into())) .await .unwrap() .module_hash .unwrap(); - assert_eq!( - running_version_for_canister, - module_hash, - "post_upgrade: running_version_for_canister of type {} ({}) != module_hash ({})", - expected_type_to_change.as_str_name(), - fmt_bytes(&running_version_for_canister), - fmt_bytes(&module_hash), - ); - running_version_for_canister + if canister_version_from_sns_pov != canister_version_from_ic00_pov { + println!( + "pre_upgrade_canister_version = {:?}", + pre_upgrade_canister_version + ); + return Err(SnsUpgradeError::CanisterVersionMismatch { + canister_type: expected_type_to_change, + canister_version_from_sns_pov, + canister_version_from_ic00_pov, + is_pre_upgrade: false, + }); + } + canister_version_from_sns_pov }; - assert_ne!( - pre_upgrade_running_canister_version, - post_upgrade_running_version_for_canister, - "pre_upgrade_running_canister_version == post_upgrade_running_version_for_canister == {}", - fmt_bytes(&pre_upgrade_running_canister_version), - ); + if pre_upgrade_canister_version == post_upgrade_canister_version { + return Err(SnsUpgradeError::TargetCanisterVersionUnchanged { + pre_upgrade_canister_version, + post_upgrade_canister_version, + }); + } + + Ok(()) + } + + pub async fn upgrade_sns_to_next_version_and_assert_change( + pocket_ic: &PocketIc, + sns_root_canister_id: PrincipalId, + expected_type_to_change: SnsCanisterType, + ) { + try_upgrade_sns_to_next_version(pocket_ic, sns_root_canister_id, expected_type_to_change) + .await + .unwrap_or_else(|err| { + panic!("Upgrading {:?} failed: {:#?}", expected_type_to_change, err) + }); } pub mod governance { diff --git a/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs b/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs index ab65f83cc2e..da0af24f6c0 100644 --- a/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs +++ b/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs @@ -31,7 +31,7 @@ use ic_sns_wasm::pb::v1::SnsCanisterType; /// Note: FI canisters are considered fully tested elsewhere, and have stable APIs. /// Deployment tests -// + #[tokio::test] async fn test_deployment_all_upgrades() { test_sns_deployment( @@ -82,6 +82,15 @@ async fn test_deployment_swap_upgrade() { } /// Upgrade Tests + +// TODO[NNS1-3433]: Enable this test after the SNS Governance canister published to SNS-W on mainnet +// TODO[NNS1-3433]: starts upgrading its Swap. +#[ignore] +#[tokio::test] +async fn test_upgrade_swap() { + test_sns_upgrade(vec![SnsCanisterType::Swap]).await; +} + #[tokio::test] async fn test_upgrade_sns_gov_root() { test_sns_upgrade(vec![SnsCanisterType::Root, SnsCanisterType::Governance]).await; diff --git a/rs/sns/governance/src/governance.rs b/rs/sns/governance/src/governance.rs index 2a02e637f20..33775b55c6a 100644 --- a/rs/sns/governance/src/governance.rs +++ b/rs/sns/governance/src/governance.rs @@ -2564,23 +2564,6 @@ impl Governance { ), }); - // SNS Swap is controlled by NNS Governance, so this SNS instance cannot upgrade it. - // Simply set `deployed_version` to `next_version` version so that other SNS upgrades can - // be executed, and let the Swap upgrade occur externally (e.g. by someone submitting an - // NNS proposal). - if canister_type_to_upgrade == SnsCanisterType::Swap { - self.proto.deployed_version = Some(next_version); - self.push_to_upgrade_journal(upgrade_journal_entry::UpgradeOutcome { - status: Some(upgrade_journal_entry::upgrade_outcome::Status::Success( - Empty {}, - )), - human_readable: Some( - "Swap upgrades are no-ops, upgrade automatically succeeded".to_string(), - ), - }); - return Ok(true); - } - let target_wasm = get_wasm(&*self.env, new_wasm_hash.to_vec(), canister_type_to_upgrade) .await .map_err(|e| {