diff --git a/rs/sns/governance/canister/canister.rs b/rs/sns/governance/canister/canister.rs index 1a8cd91e4b2..5537262f8e3 100644 --- a/rs/sns/governance/canister/canister.rs +++ b/rs/sns/governance/canister/canister.rs @@ -40,7 +40,8 @@ use ic_sns_governance::{ }, logs::{ERROR, INFO}, pb::v1::{ - ClaimSwapNeuronsRequest, ClaimSwapNeuronsResponse, FailStuckUpgradeInProgressRequest, + get_running_sns_version_response::UpgradeInProgress, ClaimSwapNeuronsRequest, + ClaimSwapNeuronsResponse, FailStuckUpgradeInProgressRequest, FailStuckUpgradeInProgressResponse, GetMaturityModulationRequest, GetMaturityModulationResponse, GetMetadataRequest, GetMetadataResponse, GetMode, GetModeResponse, GetNeuron, GetNeuronResponse, GetProposal, GetProposalResponse, @@ -467,9 +468,16 @@ async fn get_root_canister_status(_: ()) -> CanisterStatusResultV2 { #[query] fn get_running_sns_version(_: GetRunningSnsVersionRequest) -> GetRunningSnsVersionResponse { log!(INFO, "get_running_sns_version"); + let upgrade_in_progress = governance().proto.pending_version.clone(); + let upgrade_in_progress = upgrade_in_progress.map(|upgrade_in_progress| UpgradeInProgress { + 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, + }); GetRunningSnsVersionResponse { deployed_version: governance().proto.deployed_version.clone(), - pending_version: governance().proto.pending_version.clone(), + pending_version: upgrade_in_progress, } } diff --git a/rs/sns/governance/canister/governance.did b/rs/sns/governance/canister/governance.did index f8eb4598af3..96c85afbf8e 100644 --- a/rs/sns/governance/canister/governance.did +++ b/rs/sns/governance/canister/governance.did @@ -282,7 +282,7 @@ type Governance = record { cached_upgrade_steps : opt CachedUpgradeSteps; sns_initialization_parameters : text; latest_reward_event : opt RewardEvent; - pending_version : opt UpgradeInProgress; + pending_version : opt PendingVersion; swap_canister_id : opt principal; ledger_canister_id : opt principal; proposals : vec record { nat64; ProposalData }; @@ -678,6 +678,13 @@ type UpgradeInProgress = record { target_version : opt Version; }; +type PendingVersion = record { + mark_failed_at_seconds : nat64; + checking_upgrade_lock : nat64; + proposal_id : nat64; + target_version : opt Version; +}; + type UpgradeSnsControlledCanister = record { new_canister_wasm : blob; mode : opt int32; diff --git a/rs/sns/governance/canister/governance_test.did b/rs/sns/governance/canister/governance_test.did index b0f18dc8b9a..2d67529ed68 100644 --- a/rs/sns/governance/canister/governance_test.did +++ b/rs/sns/governance/canister/governance_test.did @@ -291,7 +291,7 @@ type Governance = record { cached_upgrade_steps : opt CachedUpgradeSteps; sns_initialization_parameters : text; latest_reward_event : opt RewardEvent; - pending_version : opt UpgradeInProgress; + pending_version : opt PendingVersion; swap_canister_id : opt principal; ledger_canister_id : opt principal; proposals : vec record { nat64; ProposalData }; @@ -692,6 +692,13 @@ type UpgradeInProgress = record { target_version : opt Version; }; +type PendingVersion = record { + mark_failed_at_seconds : nat64; + checking_upgrade_lock : nat64; + proposal_id : nat64; + target_version : opt Version; +}; + type UpgradeSnsControlledCanister = record { new_canister_wasm : blob; mode : opt int32; 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 96c7b0c2dab..8b5711d5f09 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 @@ -1415,7 +1415,7 @@ message Governance { Version deployed_version = 23; // An upgrade in progress, defined as a version target and a time at which it is considered failed. - message UpgradeInProgress { + message PendingVersion { // Version to be upgraded to Version target_version = 1; // Seconds since UNIX epoch to mark this as a failed version if not in sync with current version @@ -1428,7 +1428,7 @@ message Governance { } // Version SNS is in process of upgrading to. - UpgradeInProgress pending_version = 24; + PendingVersion pending_version = 24; Version target_version = 30; // True if the run_periodic_tasks function is currently finalizing disburse maturity, meaning @@ -1501,11 +1501,25 @@ message GetRunningSnsVersionRequest {} // Response with the SNS's currently running version and any upgrades // that are in progress. +// GetUpgradeJournal is a superior API to this one that should message GetRunningSnsVersionResponse { // The currently deployed version of the SNS. Governance.Version deployed_version = 1; // The upgrade in progress, if any. - Governance.UpgradeInProgress pending_version = 2; + UpgradeInProgress pending_version = 2; + + // The same as PendingVersion (stored in the governance proto). They are separated to make it easy to change one without changing the other. + message UpgradeInProgress { + // Version to be upgraded to + Governance.Version target_version = 1; + // Seconds since UNIX epoch to mark this as a failed version if not in sync with current version + uint64 mark_failed_at_seconds = 2; + // Lock to avoid checking over and over again. Also, it is a counter for how many times we have attempted to check, + // 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; + } } // Request to fail an upgrade proposal that is Adopted but not Executed or 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 c067a3877b0..270f550f11e 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 @@ -1636,7 +1636,7 @@ pub struct Governance { pub deployed_version: ::core::option::Option, /// Version SNS is in process of upgrading to. #[prost(message, optional, tag = "24")] - pub pending_version: ::core::option::Option, + pub pending_version: ::core::option::Option, #[prost(message, optional, tag = "30")] pub target_version: ::core::option::Option, /// True if the run_periodic_tasks function is currently finalizing disburse maturity, meaning @@ -1886,7 +1886,7 @@ pub mod governance { PartialEq, ::prost::Message, )] - pub struct UpgradeInProgress { + pub struct PendingVersion { /// Version to be upgraded to #[prost(message, optional, tag = "1")] pub target_version: ::core::option::Option, @@ -2065,6 +2065,7 @@ pub struct GetSnsInitializationParametersResponse { pub struct GetRunningSnsVersionRequest {} /// Response with the SNS's currently running version and any upgrades /// that are in progress. +/// GetUpgradeJournal is a superior API to this one that should #[derive( candid::CandidType, candid::Deserialize, @@ -2079,7 +2080,35 @@ pub struct GetRunningSnsVersionResponse { pub deployed_version: ::core::option::Option, /// The upgrade in progress, if any. #[prost(message, optional, tag = "2")] - pub pending_version: ::core::option::Option, + pub pending_version: + ::core::option::Option, +} +/// Nested message and enum types in `GetRunningSnsVersionResponse`. +pub mod get_running_sns_version_response { + /// The same as PendingVersion (stored in the governance proto). They are separated to make it easy to change one without changing the other. + #[derive( + candid::CandidType, + candid::Deserialize, + comparable::Comparable, + Clone, + PartialEq, + ::prost::Message, + )] + pub struct UpgradeInProgress { + /// Version to be upgraded to + #[prost(message, optional, tag = "1")] + pub target_version: ::core::option::Option, + /// Seconds since UNIX epoch to mark this as a failed version if not in sync with current version + #[prost(uint64, tag = "2")] + pub mark_failed_at_seconds: u64, + /// Lock to avoid checking over and over again. Also, it is a counter for how many times we have attempted to check, + /// allowing us to fail in case we otherwise have gotten stuck. + #[prost(uint64, tag = "3")] + pub checking_upgrade_lock: u64, + /// The proposal that initiated this upgrade + #[prost(uint64, tag = "4")] + pub proposal_id: u64, + } } /// Request to fail an upgrade proposal that is Adopted but not Executed or /// Failed if it is past the time when it should have been marked as failed. diff --git a/rs/sns/governance/src/governance.rs b/rs/sns/governance/src/governance.rs index 33775b55c6a..b25b8f2f3ec 100644 --- a/rs/sns/governance/src/governance.rs +++ b/rs/sns/governance/src/governance.rs @@ -20,8 +20,8 @@ use crate::{ governance::{ self, neuron_in_flight_command::{self, Command as InFlightCommand}, - CachedUpgradeSteps, MaturityModulation, NeuronInFlightCommand, SnsMetadata, - UpgradeInProgress, Version, Versions, + CachedUpgradeSteps, MaturityModulation, NeuronInFlightCommand, PendingVersion, + SnsMetadata, Version, Versions, }, governance_error::ErrorType, manage_neuron::{ @@ -2599,7 +2599,7 @@ impl Governance { // A canister upgrade has been successfully kicked-off. Set the pending upgrade-in-progress // field so that Governance's run_periodic_tasks logic can check on the status of // this upgrade. - self.proto.pending_version = Some(UpgradeInProgress { + self.proto.pending_version = Some(PendingVersion { target_version: Some(next_version), mark_failed_at_seconds: self.env.now() + 5 * 60, checking_upgrade_lock: 0, @@ -7303,7 +7303,7 @@ mod tests { assert_required_calls(); assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -7618,7 +7618,7 @@ mod tests { // Now set the pending_version in Governance such that the period_task to check upgrade // status is triggered. let mark_failed_at_seconds = governance.env.now() + ONE_DAY_SECONDS; - governance.proto.pending_version = Some(UpgradeInProgress { + governance.proto.pending_version = Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds, checking_upgrade_lock: 0, @@ -7628,7 +7628,7 @@ mod tests { // Make sure Governance state is correctly set assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds, checking_upgrade_lock: 0, @@ -7716,7 +7716,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: Some(current_version.clone().into()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, @@ -7734,7 +7734,7 @@ mod tests { assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, @@ -7811,7 +7811,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: Some(current_version.clone().into()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -7857,7 +7857,7 @@ mod tests { assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -7952,7 +7952,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: Some(current_version.clone().into()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 1, checking_upgrade_lock: 0, @@ -7998,7 +7998,7 @@ mod tests { assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 1, checking_upgrade_lock: 0, @@ -8015,7 +8015,7 @@ mod tests { // We still have pending version assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.into()), mark_failed_at_seconds: now + 1, checking_upgrade_lock: 0, @@ -8088,7 +8088,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: Some(current_version.clone().into()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, @@ -8134,7 +8134,7 @@ mod tests { assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now - 1, checking_upgrade_lock: 0, @@ -8235,7 +8235,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: Some(current_version.into()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { // This should be impossible due to how it's set, but is the condition of this test target_version: None, mark_failed_at_seconds: now - 1, @@ -8371,7 +8371,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: None, - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -8417,7 +8417,7 @@ mod tests { assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -8578,7 +8578,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: Some(current_version.clone().into()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -8596,7 +8596,7 @@ mod tests { assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -8656,7 +8656,7 @@ mod tests { GovernanceProto { root_canister_id: Some(root_canister_id.get()), deployed_version: Some(current_version.clone().into()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, @@ -8674,7 +8674,7 @@ mod tests { assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(next_version.clone().into()), mark_failed_at_seconds: now + 5 * 60, checking_upgrade_lock: 0, 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 1371b525953..3ac095b0561 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 @@ -10,7 +10,7 @@ use crate::{ }, pb::v1::{ get_proposal_response, - governance::{UpgradeInProgress, Version}, + governance::{PendingVersion, Version}, governance_error::ErrorType, proposal::Action, Ballot, FailStuckUpgradeInProgressRequest, FailStuckUpgradeInProgressResponse, GetProposal, @@ -75,7 +75,7 @@ lazy_static! { GovernanceProto { root_canister_id: Some(PrincipalId::from(*TEST_ROOT_CANISTER_ID)), deployed_version: Some(SNS_VERSION_1.clone()), - pending_version: Some(UpgradeInProgress { + pending_version: Some(PendingVersion { target_version: Some(SNS_VERSION_2.clone()), mark_failed_at_seconds: UPGRADE_DEADLINE_TIMESTAMP_SECONDS, checking_upgrade_lock: 10, @@ -169,7 +169,7 @@ fn test_does_nothing_if_upgrade_attempt_not_expired() { // inspect them here to make sure that any expected changes are // real, not just because the world was (accidentally) already the // way we expected them afterwards. - let expected_upgrade_in_progress = UpgradeInProgress { + let expected_upgrade_in_progress = PendingVersion { target_version: Some(SNS_VERSION_2.clone()), mark_failed_at_seconds: UPGRADE_DEADLINE_TIMESTAMP_SECONDS, checking_upgrade_lock: 10, @@ -248,7 +248,7 @@ fn test_fails_proposal_and_removes_upgrade_if_upgrade_attempt_is_expired() { // way we expected them afterwards. assert_eq!( governance.proto.pending_version.clone().unwrap(), - UpgradeInProgress { + PendingVersion { target_version: Some(SNS_VERSION_2.clone()), mark_failed_at_seconds: UPGRADE_DEADLINE_TIMESTAMP_SECONDS, checking_upgrade_lock: 10,