From 959a5ebd1fa677f9d44ac8939b02811810df0b39 Mon Sep 17 00:00:00 2001 From: Andre Popovitch Date: Tue, 26 Nov 2024 19:26:17 -0600 Subject: [PATCH] refactor(sns): Move upgrade-steps-related functions to their own file (#2836) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR moves `temporarily_lock_refresh_cached_upgrade_steps`, `should_refresh_cached_upgrade_steps`, and `should_refresh_cached_upgrade_steps` into the already existing `cached_upgrade_steps.rs` file. The benefit of this is improved organization – pretty much all the upgrade-steps-related stuff is now in one small file. [Next PR →](https://github.com/dfinity/ic/pull/2839) ## Note We don't do this very often in SNS governance, but I think it would be a good habit to get into. These giant files we have are intimidating and make it hard to find what you're looking for. Another case where we do this currently is in `upgrade_journal.rs` --- rs/sns/governance/src/cached_upgrade_steps.rs | 84 +++++++++++++++++++ rs/sns/governance/src/governance.rs | 77 ----------------- 2 files changed, 84 insertions(+), 77 deletions(-) diff --git a/rs/sns/governance/src/cached_upgrade_steps.rs b/rs/sns/governance/src/cached_upgrade_steps.rs index a29ce530659..2c706789393 100644 --- a/rs/sns/governance/src/cached_upgrade_steps.rs +++ b/rs/sns/governance/src/cached_upgrade_steps.rs @@ -1,7 +1,12 @@ +use crate::governance::Governance; +use crate::governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS; +use crate::logs::ERROR; use crate::pb::v1::governance::CachedUpgradeSteps as CachedUpgradeStepsPb; use crate::pb::v1::governance::Version; use crate::pb::v1::governance::Versions; +use crate::pb::v1::upgrade_journal_entry; use crate::sns_upgrade::SnsCanisterType; +use ic_canister_log::log; #[derive(Clone, Debug, PartialEq)] pub(crate) struct CachedUpgradeSteps { @@ -284,3 +289,82 @@ impl CachedUpgradeSteps { Ok(()) } } + +impl Governance { + pub fn temporarily_lock_refresh_cached_upgrade_steps(&mut self) { + let upgrade_steps = + self.proto + .cached_upgrade_steps + .get_or_insert_with(|| CachedUpgradeStepsPb { + requested_timestamp_seconds: Some(self.env.now()), + ..Default::default() + }); + upgrade_steps.requested_timestamp_seconds = Some(self.env.now()); + } + + pub fn should_refresh_cached_upgrade_steps(&mut self) -> bool { + let now = self.env.now(); + + if let Some(ref cached_upgrade_steps) = self.proto.cached_upgrade_steps { + let requested_timestamp_seconds = cached_upgrade_steps + .requested_timestamp_seconds + .unwrap_or(0); + if now - requested_timestamp_seconds < UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS { + return false; + } + } + + true + } + + /// Refreshes the cached_upgrade_steps field + pub async fn refresh_cached_upgrade_steps(&mut self) { + let Some(deployed_version) = self.proto.deployed_version.as_ref() else { + log!( + ERROR, + "Cannot refresh cached_upgrade_steps: deployed_version not set." + ); + return; + }; + let sns_governance_canister_id = self.env.canister_id().get(); + + let upgrade_steps = crate::sns_upgrade::get_upgrade_steps( + &*self.env, + deployed_version.clone(), + sns_governance_canister_id, + ) + .await; + + let upgrade_steps = match upgrade_steps { + Ok(upgrade_steps) => upgrade_steps, + Err(err) => { + log!( + ERROR, + "Cannot refresh cached_upgrade_steps: call to SNS-W failed: {}", + err + ); + return; + } + }; + let upgrade_steps = Versions { + versions: upgrade_steps, + }; + + // Ensure `cached_upgrade_steps` is initialized + let cached_upgrade_steps = self + .proto + .cached_upgrade_steps + .get_or_insert_with(Default::default); + + // Update `response_timestamp_seconds` + cached_upgrade_steps.response_timestamp_seconds = Some(self.env.now()); + + // Refresh the upgrade steps if they have changed + if cached_upgrade_steps.upgrade_steps != Some(upgrade_steps.clone()) { + cached_upgrade_steps.upgrade_steps = Some(upgrade_steps.clone()); + self.push_to_upgrade_journal(upgrade_journal_entry::UpgradeStepsRefreshed::new( + upgrade_steps.versions, + )); + } + } +} diff --git a/rs/sns/governance/src/governance.rs b/rs/sns/governance/src/governance.rs index 953af2437a8..e82342dfbaf 100644 --- a/rs/sns/governance/src/governance.rs +++ b/rs/sns/governance/src/governance.rs @@ -5056,83 +5056,6 @@ impl Governance { self.upgrade_periodic_task_lock = None; } - pub fn temporarily_lock_refresh_cached_upgrade_steps(&mut self) { - let upgrade_steps = - self.proto - .cached_upgrade_steps - .get_or_insert_with(|| CachedUpgradeStepsPb { - requested_timestamp_seconds: Some(self.env.now()), - ..Default::default() - }); - upgrade_steps.requested_timestamp_seconds = Some(self.env.now()); - } - - pub fn should_refresh_cached_upgrade_steps(&mut self) -> bool { - let now = self.env.now(); - - if let Some(ref cached_upgrade_steps) = self.proto.cached_upgrade_steps { - let requested_timestamp_seconds = cached_upgrade_steps - .requested_timestamp_seconds - .unwrap_or(0); - if now - requested_timestamp_seconds < UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS { - return false; - } - } - - true - } - - /// Refreshes the cached_upgrade_steps field - pub async fn refresh_cached_upgrade_steps(&mut self) { - let Some(deployed_version) = self.proto.deployed_version.as_ref() else { - log!( - ERROR, - "Cannot refresh cached_upgrade_steps: deployed_version not set." - ); - return; - }; - let sns_governance_canister_id = self.env.canister_id().get(); - - let upgrade_steps = crate::sns_upgrade::get_upgrade_steps( - &*self.env, - deployed_version.clone(), - sns_governance_canister_id, - ) - .await; - - let upgrade_steps = match upgrade_steps { - Ok(upgrade_steps) => upgrade_steps, - Err(err) => { - log!( - ERROR, - "Cannot refresh cached_upgrade_steps: call to SNS-W failed: {}", - err - ); - return; - } - }; - let upgrade_steps = Versions { - versions: upgrade_steps, - }; - - // Ensure `cached_upgrade_steps` is initialized - let cached_upgrade_steps = self - .proto - .cached_upgrade_steps - .get_or_insert_with(Default::default); - - // Update `response_timestamp_seconds` - cached_upgrade_steps.response_timestamp_seconds = Some(self.env.now()); - - // Refresh the upgrade steps if they have changed - if cached_upgrade_steps.upgrade_steps != Some(upgrade_steps.clone()) { - cached_upgrade_steps.upgrade_steps = Some(upgrade_steps.clone()); - self.push_to_upgrade_journal(upgrade_journal_entry::UpgradeStepsRefreshed::new( - upgrade_steps.versions, - )); - } - } - pub fn get_upgrade_journal(&self) -> GetUpgradeJournalResponse { let cached_upgrade_steps = self.proto.cached_upgrade_steps.clone(); match cached_upgrade_steps {