From 97bfaffade5e17f94c817de725fa0cd1e63eea25 Mon Sep 17 00:00:00 2001 From: Andre Popovitch Date: Thu, 7 Nov 2024 19:23:39 -0500 Subject: [PATCH] automatically trigger upgrades --- .../tests/advance_target_version.rs | 96 ++++++- rs/sns/governance/canister/canister.rs | 2 +- rs/sns/governance/canister/governance.did | 12 +- .../governance/canister/governance_test.did | 12 +- .../ic_sns_governance/pb/v1/governance.proto | 3 +- .../src/gen/ic_sns_governance.pb.v1.rs | 6 +- rs/sns/governance/src/governance.rs | 241 +++++++++++++++--- .../fail_stuck_upgrade_in_progress_tests.rs | 6 +- rs/sns/governance/src/sns_upgrade.rs | 4 +- 9 files changed, 337 insertions(+), 45 deletions(-) diff --git a/rs/nervous_system/integration_tests/tests/advance_target_version.rs b/rs/nervous_system/integration_tests/tests/advance_target_version.rs index 4c2feb9b3a8..e7e084033c9 100644 --- a/rs/nervous_system/integration_tests/tests/advance_target_version.rs +++ b/rs/nervous_system/integration_tests/tests/advance_target_version.rs @@ -253,7 +253,7 @@ async fn test_get_upgrade_journal() { expected_upgrade_journal_entries.push(sns_pb::upgrade_journal_entry::Event::TargetVersionSet( sns_pb::upgrade_journal_entry::TargetVersionSet { old_target_version: None, - new_target_version: Some(new_sns_version_2), + new_target_version: Some(new_sns_version_2.clone()), }, )); @@ -263,4 +263,98 @@ async fn test_get_upgrade_journal() { &expected_upgrade_journal_entries, ) .await; + + // Check that the target version is set to the new version. + { + let sns_pb::GetUpgradeJournalResponse { target_version, .. } = + sns::governance::get_upgrade_journal(&pocket_ic, sns.governance.canister_id).await; + + assert_eq!(target_version, Some(new_sns_version_2.clone())); + } + + await_with_timeout( + &pocket_ic, + UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS, + |pocket_ic| async { + sns::governance::get_upgrade_journal(pocket_ic, sns.governance.canister_id) + .await + .deployed_version + }, + &Some(new_sns_version_2.clone()), + ) + .await + .unwrap(); + + // Check that the deployed version is now set to the new version. + { + let sns_pb::GetUpgradeJournalResponse { + deployed_version, .. + } = sns::governance::get_upgrade_journal(&pocket_ic, sns.governance.canister_id).await; + + assert_eq!(deployed_version, Some(new_sns_version_2.clone())); + } + + // Check that the upgrade journal contains the correct entries. + { + expected_upgrade_journal_entries.push( + sns_pb::upgrade_journal_entry::Event::UpgradeStarted( + sns_pb::upgrade_journal_entry::UpgradeStarted { + current_version: Some(initial_sns_version.clone()), + expected_version: Some(new_sns_version_1.clone()), + reason: Some( + sns_pb::upgrade_journal_entry::upgrade_started::Reason::BehindTargetVersion( + sns_pb::Empty {}, + ), + ), + }, + ), + ); + + expected_upgrade_journal_entries.push( + sns_pb::upgrade_journal_entry::Event::UpgradeOutcome( + sns_pb::upgrade_journal_entry::UpgradeOutcome { + status: Some( + sns_pb::upgrade_journal_entry::upgrade_outcome::Status::Success( + sns_pb::Empty {}, + ), + ), + human_readable: None, + }, + ), + ); + + expected_upgrade_journal_entries.push( + sns_pb::upgrade_journal_entry::Event::UpgradeStarted( + sns_pb::upgrade_journal_entry::UpgradeStarted { + current_version: Some(new_sns_version_1.clone()), + expected_version: Some(new_sns_version_2.clone()), + reason: Some( + sns_pb::upgrade_journal_entry::upgrade_started::Reason::BehindTargetVersion( + sns_pb::Empty {}, + ), + ), + }, + ), + ); + + expected_upgrade_journal_entries.push( + sns_pb::upgrade_journal_entry::Event::UpgradeOutcome( + sns_pb::upgrade_journal_entry::UpgradeOutcome { + status: Some( + sns_pb::upgrade_journal_entry::upgrade_outcome::Status::Success( + sns_pb::Empty {}, + ), + ), + human_readable: None, + }, + ), + ); + + assert_upgrade_journal( + &pocket_ic, + sns.governance, + &expected_upgrade_journal_entries, + ) + .await; + } } diff --git a/rs/sns/governance/canister/canister.rs b/rs/sns/governance/canister/canister.rs index 537746b1b5f..93bd9aad184 100644 --- a/rs/sns/governance/canister/canister.rs +++ b/rs/sns/governance/canister/canister.rs @@ -473,7 +473,7 @@ fn get_running_sns_version(_: GetRunningSnsVersionRequest) -> GetRunningSnsVersi target_version: upgrade_in_progress.target_version.clone(), mark_failed_at_seconds: upgrade_in_progress.mark_failed_at_seconds, checking_upgrade_lock: upgrade_in_progress.checking_upgrade_lock, - proposal_id: upgrade_in_progress.proposal_id, + proposal_id: upgrade_in_progress.proposal_id.unwrap_or(0), }); GetRunningSnsVersionResponse { deployed_version: governance().proto.deployed_version.clone(), diff --git a/rs/sns/governance/canister/governance.did b/rs/sns/governance/canister/governance.did index 96c85afbf8e..4f9b9b11130 100644 --- a/rs/sns/governance/canister/governance.did +++ b/rs/sns/governance/canister/governance.did @@ -257,7 +257,12 @@ type GetProposalResponse = record { type GetRunningSnsVersionResponse = record { deployed_version : opt Version; - pending_version : opt UpgradeInProgress; + pending_version : opt record { + mark_failed_at_seconds : nat64; + checking_upgrade_lock : nat64; + proposal_id : nat64; + target_version : opt Version; + }; }; type GetSnsInitializationParametersResponse = record { @@ -674,14 +679,14 @@ type TransferSnsTreasuryFunds = record { type UpgradeInProgress = record { mark_failed_at_seconds : nat64; checking_upgrade_lock : nat64; - proposal_id : nat64; + proposal_id : opt nat64; target_version : opt Version; }; type PendingVersion = record { mark_failed_at_seconds : nat64; checking_upgrade_lock : nat64; - proposal_id : nat64; + proposal_id : opt nat64; target_version : opt Version; }; @@ -781,6 +786,7 @@ type GetUpgradeJournalResponse = record { upgrade_steps : opt Versions; response_timestamp_seconds : opt nat64; target_version : opt Version; + deployed_version : opt Version; upgrade_journal : opt UpgradeJournal; }; diff --git a/rs/sns/governance/canister/governance_test.did b/rs/sns/governance/canister/governance_test.did index 2d67529ed68..d29c14ddf9d 100644 --- a/rs/sns/governance/canister/governance_test.did +++ b/rs/sns/governance/canister/governance_test.did @@ -266,7 +266,12 @@ type GetProposalResponse = record { type GetRunningSnsVersionResponse = record { deployed_version : opt Version; - pending_version : opt UpgradeInProgress; + pending_version : opt record { + mark_failed_at_seconds : nat64; + checking_upgrade_lock : nat64; + proposal_id : nat64; + target_version : opt Version; + }; }; type GetSnsInitializationParametersResponse = record { @@ -688,14 +693,14 @@ type TransferSnsTreasuryFunds = record { type UpgradeInProgress = record { mark_failed_at_seconds : nat64; checking_upgrade_lock : nat64; - proposal_id : nat64; + proposal_id : opt nat64; target_version : opt Version; }; type PendingVersion = record { mark_failed_at_seconds : nat64; checking_upgrade_lock : nat64; - proposal_id : nat64; + proposal_id : opt nat64; target_version : opt Version; }; @@ -795,6 +800,7 @@ type GetUpgradeJournalResponse = record { upgrade_steps : opt Versions; response_timestamp_seconds : opt nat64; target_version : opt Version; + deployed_version : opt Version; upgrade_journal : opt UpgradeJournal; }; diff --git a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto index 8b5711d5f09..3f6b74863ce 100644 --- a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto +++ b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto @@ -1424,7 +1424,7 @@ message Governance { // allowing us to fail in case we otherwise have gotten stuck. uint64 checking_upgrade_lock = 3; // The proposal that initiated this upgrade - uint64 proposal_id = 4; + optional uint64 proposal_id = 4; } // Version SNS is in process of upgrading to. @@ -2224,6 +2224,7 @@ message GetUpgradeJournalResponse { // Currently, this field is always None, but in the "effortless SNS upgrade" // feature, it reflect the version of the SNS that the community has decided to upgrade to. Governance.Version target_version = 3; + Governance.Version deployed_version = 5; UpgradeJournal upgrade_journal = 4; } 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 270f550f11e..7b67465fdd7 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 @@ -1898,8 +1898,8 @@ pub mod governance { #[prost(uint64, tag = "3")] pub checking_upgrade_lock: u64, /// The proposal that initiated this upgrade - #[prost(uint64, tag = "4")] - pub proposal_id: u64, + #[prost(uint64, optional, tag = "4")] + pub proposal_id: ::core::option::Option, } #[derive( candid::CandidType, @@ -3569,6 +3569,8 @@ pub struct GetUpgradeJournalResponse { /// feature, it reflect the version of the SNS that the community has decided to upgrade to. #[prost(message, optional, tag = "3")] pub target_version: ::core::option::Option, + #[prost(message, optional, tag = "5")] + pub deployed_version: ::core::option::Option, #[prost(message, optional, tag = "4")] pub upgrade_journal: ::core::option::Option, } diff --git a/rs/sns/governance/src/governance.rs b/rs/sns/governance/src/governance.rs index 9d7f0ce4590..bbdef825e40 100644 --- a/rs/sns/governance/src/governance.rs +++ b/rs/sns/governance/src/governance.rs @@ -65,8 +65,8 @@ use crate::{ MAX_NUMBER_OF_PROPOSALS_WITH_BALLOTS, }, sns_upgrade::{ - get_all_sns_canisters, get_running_version, get_upgrade_params, get_wasm, SnsCanisterType, - UpgradeSnsParams, + canister_type_and_wasm_hash_for_upgrade, get_all_sns_canisters, get_canisters_to_upgrade, + get_running_version, get_upgrade_params, get_wasm, SnsCanisterType, UpgradeSnsParams, }, types::{ function_id_to_proposal_criticality, is_registered_function_id, Environment, @@ -2063,8 +2063,9 @@ impl Governance { } Action::UpgradeSnsToNextVersion(_) => { log!(INFO, "Executing UpgradeSnsToNextVersion action",); - let upgrade_sns_result = - self.perform_upgrade_to_next_sns_version(proposal_id).await; + let upgrade_sns_result = self + .perform_upgrade_to_next_sns_version_legacy(proposal_id) + .await; // If the upgrade returned `Ok(true)` that means the upgrade completed successfully // and the proposal can be marked as "executed". If the upgrade returned `Ok(false)` @@ -2545,6 +2546,33 @@ impl Governance { }) } + pub fn check_no_upgrades_in_progress(&self) -> Result<(), GovernanceError> { + let upgrade_proposals_in_progress = self.upgrade_proposals_in_progress(); + if !upgrade_proposals_in_progress.is_empty() { + return Err(GovernanceError::new_with_message( + ErrorType::ResourceExhausted, + format!( + "Another upgrade is currently in progress (proposal IDs {}). \ + Please, try again later.", + upgrade_proposals_in_progress + .into_iter() + .map(|id| id.to_string()) + .collect::>() + .join(", ") + ), + )); + } + + if self.proto.pending_version.is_some() { + return Err(GovernanceError::new_with_message( + ErrorType::ResourceExhausted, + "Upgrade lock currently acquired, not upgrading".to_string(), + )); + } + + Ok(()) + } + pub fn check_no_other_upgrades_in_progress( &self, proposal_id: u64, @@ -2577,7 +2605,7 @@ impl Governance { /// Return `Ok(true)` if the upgrade was completed successfully, return `Ok(false)` if an /// upgrade was successfully kicked-off, but its completion is pending. - async fn perform_upgrade_to_next_sns_version( + async fn perform_upgrade_to_next_sns_version_legacy( &mut self, proposal_id: u64, ) -> Result { @@ -2649,7 +2677,7 @@ impl Governance { target_version: Some(next_version), mark_failed_at_seconds: self.env.now() + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }); log!( @@ -2661,6 +2689,63 @@ impl Governance { Ok(false) } + async fn upgrade_sns_framework_canister( + &mut self, + new_wasm_hash: Vec, + canister_type_to_upgrade: SnsCanisterType, + ) -> Result { + let root_canister_id = self.proto.root_canister_id_or_panic(); + + let target_wasm = get_wasm(&*self.env, new_wasm_hash.to_vec(), canister_type_to_upgrade) + .await + .map_err(|e| { + GovernanceError::new_with_message( + ErrorType::External, + format!("Could not get wasm for upgrade: {}", e), + ) + })? + .wasm; + + let target_is_root = canister_type_to_upgrade == SnsCanisterType::Root; + + if target_is_root { + upgrade_canister_directly( + &*self.env, + root_canister_id, + target_wasm, + Encode!().unwrap(), + ) + .await?; + } else { + let canister_ids_to_upgrade = + get_canisters_to_upgrade(&*self.env, root_canister_id, canister_type_to_upgrade) + .await + .map_err(|e| { + GovernanceError::new_with_message( + ErrorType::External, + format!("Could not get list of SNS canisters from root: {}", e), + ) + })?; + for target_canister_id in canister_ids_to_upgrade { + self.upgrade_non_root_canister( + target_canister_id, + target_wasm.clone(), + Encode!().unwrap(), + CanisterInstallMode::Upgrade, + ) + .await?; + } + } + + log!( + INFO, + "Successfully kicked off upgrade for SNS canister {:?}", + canister_type_to_upgrade, + ); + + Ok(false) + } + async fn perform_transfer_sns_treasury_funds( &mut self, proposal_id: u64, // This is just to control concurrency. @@ -4699,6 +4784,8 @@ impl Governance { self.refresh_cached_upgrade_steps().await; } + self.initiate_upgrade_if_behind_target_version().await; + self.release_upgrade_periodic_task_lock(); } @@ -4752,6 +4839,96 @@ impl Governance { } } } + /// Checks if an automatic upgrade is needed and initiates it. + /// An automatic upgrade is needed if `target_version` is set to a future version on the upgrade path + async fn initiate_upgrade_if_behind_target_version(&mut self) { + if self.check_no_upgrades_in_progress().is_err() { + // An upgrade is already in progress + return; + } + + let Some(deployed_version) = self.proto.deployed_version.clone() else { + return; + }; + + let Some(target_version) = self.proto.target_version.clone() else { + return; + }; + + let Some(CachedUpgradeSteps { + upgrade_steps: Some(Versions { + versions: upgrade_steps, + }), + .. + }) = &self.proto.cached_upgrade_steps + else { + log!(ERROR, "Cached upgrade steps set to None"); + return; + }; + + // Find the current position of the deployed version + let Some(deployed_position) = upgrade_steps.iter().position(|v| v == &deployed_version) + else { + log!(ERROR, "Deployed version is not on the upgrade path"); + self.invalidate_cached_upgrade_steps(); + return; + }; + + // Find the target position of the target version + let Some(target_position) = upgrade_steps.iter().position(|v| v == &target_version) else { + log!(ERROR, "Target version is not on the upgrade path"); + return; + }; + + if target_position <= deployed_position { + // Target version is not after the deployed version + return; + } + + // since `target_position > deployed_position`, `deployed_position + 1 < upgrade_steps.len()` + let next_version = upgrade_steps[deployed_position + 1].clone(); + + let (canister_type, wasm_hash) = + match canister_type_and_wasm_hash_for_upgrade(&deployed_version, &next_version) { + Ok((canister_type, wasm_hash)) => (canister_type, wasm_hash), + + Err(err) => { + log!(ERROR, "Upgrade attempt failed: {}", err); + self.proto.target_version = None; + return; + } + }; + + self.push_to_upgrade_journal(upgrade_journal_entry::UpgradeStarted { + current_version: self.proto.deployed_version.clone(), + expected_version: Some(next_version.clone()), + reason: Some( + upgrade_journal_entry::upgrade_started::Reason::BehindTargetVersion(Empty {}), + ), + }); + + self.proto.pending_version = Some(PendingVersion { + target_version: Some(next_version.clone()), + mark_failed_at_seconds: self.env.now() + 5 * 60, + checking_upgrade_lock: 0, + proposal_id: None, + }); + + println!("Initiating upgrade to version: {:?}", next_version); + let upgrade_attempt = self + .upgrade_sns_framework_canister(wasm_hash, canister_type) + .await; + if let Err(err) = upgrade_attempt { + log!(ERROR, "Upgrade attempt failed: {}", err); + self.proto.pending_version = None; + self.proto.target_version = None; + } + } + + /// Invalidates the cached upgrade steps. + fn invalidate_cached_upgrade_steps(&mut self) { + self.proto.cached_upgrade_steps = None; + } fn release_upgrade_periodic_task_lock(&mut self) { self.upgrade_periodic_task_lock = None; @@ -4862,6 +5039,7 @@ impl Governance { upgrade_steps: cached_upgrade_steps.upgrade_steps, response_timestamp_seconds: cached_upgrade_steps.response_timestamp_seconds, target_version: self.proto.target_version.clone(), + deployed_version: self.proto.deployed_version.clone(), // TODO(NNS1-3416): Bound the size of the response. upgrade_journal: self.proto.upgrade_journal.clone(), }, @@ -4869,6 +5047,7 @@ impl Governance { upgrade_steps: None, response_timestamp_seconds: None, target_version: None, + deployed_version: self.proto.deployed_version.clone(), // TODO(NNS1-3416): Bound the size of the response. upgrade_journal: self.proto.upgrade_journal.clone(), }, @@ -5505,7 +5684,9 @@ impl Governance { )), human_readable: None, }); - self.set_proposal_execution_status(proposal_id, Ok(())); + if let Some(proposal_id) = proposal_id { + self.set_proposal_execution_status(proposal_id, Ok(())); + } self.proto.deployed_version = Some(target_version); self.proto.pending_version = None; } @@ -5539,12 +5720,14 @@ impl Governance { // an error for an UpgradeSnsToNextVersion actions failure. This unblocks further upgrade proposals. fn fail_sns_upgrade_to_next_version_proposal( &mut self, - proposal_id: u64, + proposal_id: Option, error: GovernanceError, ) { log!(ERROR, "{}", error.error_message); let result = Err(error); - self.set_proposal_execution_status(proposal_id, result); + if let Some(proposal_id) = proposal_id { + self.set_proposal_execution_status(proposal_id, result); + } self.proto.pending_version = None; } @@ -5585,7 +5768,7 @@ impl Governance { self.fail_sns_upgrade_to_next_version_proposal( pending_version.proposal_id, GovernanceError::new_with_message(ErrorType::External, error), - ); + ) } FailStuckUpgradeInProgressResponse {} @@ -7325,7 +7508,7 @@ mod tests { target_version: Some(next_version.into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); // We do not check the upgrade completion in this test because of limitations @@ -7640,7 +7823,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds, checking_upgrade_lock: 0, - proposal_id: 0, + proposal_id: Some(0), }); // Make sure Governance state is correctly set @@ -7650,7 +7833,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds, checking_upgrade_lock: 0, - proposal_id: 0, + proposal_id: Some(0), } ); assert_eq!( @@ -7738,7 +7921,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, - proposal_id: 0, + proposal_id: Some(0), }), ..basic_governance_proto() } @@ -7756,7 +7939,7 @@ mod tests { target_version: Some(next_version.into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, - proposal_id: 0, + proposal_id: Some(0), } ); assert_eq!( @@ -7833,7 +8016,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }), // we make a proposal that is already decided so that it won't execute again because // proposals to upgrade SNS's cannot execute if there's no deployed_version set on Governance state @@ -7879,7 +8062,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); assert_eq!( @@ -7974,7 +8157,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 1, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }), // we make a proposal that is already decided so that it won't execute again because // proposals to upgrade SNS's cannot execute if there's no deployed_version set on Governance state @@ -8020,7 +8203,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 1, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); assert_eq!( @@ -8037,7 +8220,7 @@ mod tests { target_version: Some(next_version.into()), mark_failed_at_seconds: now + 1, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); @@ -8110,7 +8293,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }), // we make a proposal that is already decided so that it won't execute again because // proposals to upgrade SNS's cannot execute if there's no deployed_version set on Governance state @@ -8156,7 +8339,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); assert_eq!( @@ -8258,7 +8441,7 @@ mod tests { target_version: None, mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }), // we make a proposal that is already decided so that it won't execute again because // proposals to upgrade SNS's cannot execute if there's no deployed_version set on Governance state @@ -8393,7 +8576,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }), // we make a proposal that is already decided so that it won't execute again because // proposals to upgrade SNS's cannot execute if there's no deployed_version set on Governance state @@ -8439,7 +8622,7 @@ mod tests { target_version: Some(next_version.into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); @@ -8600,7 +8783,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }), ..basic_governance_proto() } @@ -8618,7 +8801,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); assert_eq!( @@ -8678,7 +8861,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), }), ..basic_governance_proto() } @@ -8696,7 +8879,7 @@ mod tests { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, - proposal_id, + proposal_id: Some(proposal_id), } ); assert_eq!( diff --git a/rs/sns/governance/src/governance/tests/fail_stuck_upgrade_in_progress_tests.rs b/rs/sns/governance/src/governance/tests/fail_stuck_upgrade_in_progress_tests.rs index 3ac095b0561..e4fc9344cbc 100644 --- a/rs/sns/governance/src/governance/tests/fail_stuck_upgrade_in_progress_tests.rs +++ b/rs/sns/governance/src/governance/tests/fail_stuck_upgrade_in_progress_tests.rs @@ -79,7 +79,7 @@ lazy_static! { target_version: Some(SNS_VERSION_2.clone()), mark_failed_at_seconds: UPGRADE_DEADLINE_TIMESTAMP_SECONDS, checking_upgrade_lock: 10, - proposal_id: UPGRADE_PROPOSAL_ID, + proposal_id: Some(UPGRADE_PROPOSAL_ID), }), // we make a proposal that is already decided so that it won't execute again because // proposals to upgrade SNS's cannot execute if there's no deployed_version set on Governance state @@ -173,7 +173,7 @@ fn test_does_nothing_if_upgrade_attempt_not_expired() { target_version: Some(SNS_VERSION_2.clone()), mark_failed_at_seconds: UPGRADE_DEADLINE_TIMESTAMP_SECONDS, checking_upgrade_lock: 10, - proposal_id: UPGRADE_PROPOSAL_ID, + proposal_id: Some(UPGRADE_PROPOSAL_ID), }; assert_eq!( governance.proto.pending_version.clone().unwrap(), @@ -252,7 +252,7 @@ fn test_fails_proposal_and_removes_upgrade_if_upgrade_attempt_is_expired() { target_version: Some(SNS_VERSION_2.clone()), mark_failed_at_seconds: UPGRADE_DEADLINE_TIMESTAMP_SECONDS, checking_upgrade_lock: 10, - proposal_id: UPGRADE_PROPOSAL_ID, + proposal_id: Some(UPGRADE_PROPOSAL_ID), } ); assert_eq!( diff --git a/rs/sns/governance/src/sns_upgrade.rs b/rs/sns/governance/src/sns_upgrade.rs index 82ea9695d7a..c778d3638a9 100644 --- a/rs/sns/governance/src/sns_upgrade.rs +++ b/rs/sns/governance/src/sns_upgrade.rs @@ -137,7 +137,7 @@ pub(crate) async fn get_proposal_id_that_added_wasm( Ok(proposal_id) } -async fn get_canisters_to_upgrade( +pub(crate) async fn get_canisters_to_upgrade( env: &dyn Environment, root_canister_id: CanisterId, canister_type: SnsCanisterType, @@ -174,7 +174,7 @@ async fn get_canisters_to_upgrade( .collect() } -fn canister_type_and_wasm_hash_for_upgrade( +pub(crate) fn canister_type_and_wasm_hash_for_upgrade( current_version: &Version, next_version: &Version, ) -> Result<(SnsCanisterType, Vec), String> {