Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sns): Create test-only function that can be used to advance the target version directly (rather than through a proposal) #1743

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,10 @@ pub fn propose_to_set_network_economics_and_wait(

pub fn add_wasms_to_sns_wasm(
pocket_ic: &PocketIc,
with_mainnet_ledger_wasms: bool,
with_mainnet_sns_canister_wasms: bool,
) -> Result<BTreeMap<SnsCanisterType, (ProposalInfo, SnsWasm)>, String> {
let (root_wasm, governance_wasm, swap_wasm, index_wasm, ledger_wasm, archive_wasm) =
if with_mainnet_ledger_wasms {
if with_mainnet_sns_canister_wasms {
(
ensure_sns_wasm_gzipped(build_mainnet_root_sns_wasm()),
ensure_sns_wasm_gzipped(build_mainnet_governance_sns_wasm()),
Expand Down
13 changes: 11 additions & 2 deletions rs/sns/governance/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ use ic_nervous_system_runtime::DfnRuntime;
use ic_nns_constants::LEDGER_CANISTER_ID as NNS_LEDGER_CANISTER_ID;
#[cfg(feature = "test")]
use ic_sns_governance::pb::v1::{
AddMaturityRequest, AddMaturityResponse, GovernanceError, MintTokensRequest,
MintTokensResponse, Neuron,
AddMaturityRequest, AddMaturityResponse, AdvanceTargetVersionRequest,
AdvanceTargetVersionResponse, GovernanceError, MintTokensRequest, MintTokensResponse, Neuron,
};
use ic_sns_governance::{
governance::{
Expand Down Expand Up @@ -524,6 +524,7 @@ fn get_running_sns_version_(_: GetRunningSnsVersionRequest) -> GetRunningSnsVers
GetRunningSnsVersionResponse {
deployed_version: governance().proto.deployed_version.clone(),
pending_version: governance().proto.pending_version.clone(),
target_version: governance().proto.target_version.clone(),
}
}

Expand Down Expand Up @@ -726,6 +727,14 @@ fn add_maturity_(request: AddMaturityRequest) -> AddMaturityResponse {
governance_mut().add_maturity(request)
}

#[cfg(feature = "test")]
#[export_name = "canister_update advance_target_version"]
fn advance_target_version(request: AdvanceTargetVersionRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why didn't this function end up in rs/sns/governance/canister/governance_test.did?

Copy link
Contributor Author

@anchpop anchpop Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, seems like it's not enforced by the candid type enforcement in CI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the candid type enforecment seems to specifically reject it, not sure what's going on here

over(candid_one, |request: AdvanceTargetVersionRequest| {
AdvanceTargetVersionResponse {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not implement this function in the same PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the plan, however the scope of this PR has unfortunately grown and I may have to start splitting it up before merging it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's keep this PR just for bumping the target in testing.

Note that it should still only be possible to advance the target using new_target, not go back to an older version (I recommend implementing that logic in this PR as well).

})
}

/// Mints tokens for testing
#[cfg(feature = "test")]
#[export_name = "canister_update mint_tokens"]
Expand Down
7 changes: 7 additions & 0 deletions rs/sns/governance/canister/governance.did
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,17 @@ type GetProposalResponse = record {
type GetRunningSnsVersionResponse = record {
deployed_version : opt Version;
pending_version : opt UpgradeInProgress;
target_version : opt Version;
};

type GetSnsInitializationParametersResponse = record {
sns_initialization_parameters : text;
};

type CachedUpgradeSteps = record {
upgrade_steps : vec Version;
};

type Governance = record {
root_canister_id : opt principal;
id_to_nervous_system_functions : vec record { nat64; NervousSystemFunction };
Expand All @@ -273,6 +278,8 @@ type Governance = record {
parameters : opt NervousSystemParameters;
is_finalizing_disburse_maturity : opt bool;
deployed_version : opt Version;
target_version : opt Version;
cached_upgrade_steps : opt CachedUpgradeSteps;
sns_initialization_parameters : text;
latest_reward_event : opt RewardEvent;
pending_version : opt UpgradeInProgress;
Expand Down
7 changes: 7 additions & 0 deletions rs/sns/governance/canister/governance_test.did
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,17 @@ type GetProposalResponse = record {
type GetRunningSnsVersionResponse = record {
deployed_version : opt Version;
pending_version : opt UpgradeInProgress;
target_version : opt Version;
};

type GetSnsInitializationParametersResponse = record {
sns_initialization_parameters : text;
};

type CachedUpgradeSteps = record {
upgrade_steps : vec Version;
};

type Governance = record {
root_canister_id : opt principal;
id_to_nervous_system_functions : vec record { nat64; NervousSystemFunction };
Expand All @@ -282,6 +287,8 @@ type Governance = record {
parameters : opt NervousSystemParameters;
is_finalizing_disburse_maturity : opt bool;
deployed_version : opt Version;
target_version : opt Version;
cached_upgrade_steps : opt CachedUpgradeSteps;
sns_initialization_parameters : text;
latest_reward_event : opt RewardEvent;
pending_version : opt UpgradeInProgress;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,7 @@ message RewardEvent {
optional uint64 total_available_e8s_equivalent = 8;
}


// The representation of the whole governance system, containing all
// information about the governance system that must be kept
// across upgrades of the governance system, i.e. kept in stable memory.
Expand Down Expand Up @@ -1449,6 +1450,15 @@ message Governance {

reserved "migrated_root_wasm_memory_limit";
reserved 27;

Governance.Version target_version = 28;

message CachedUpgradeSteps {
repeated Governance.Version upgrade_steps = 1;
optional uint64 requested_update_at_timestamp_seconds = 2;
optional uint64 received_update_at_timestamp_seconds = 3;
}
CachedUpgradeSteps cached_upgrade_steps = 29;
}

// Request message for 'get_metadata'.
Expand Down Expand Up @@ -1480,6 +1490,8 @@ message GetRunningSnsVersionResponse {
Governance.Version deployed_version = 1;
// The upgrade in progress, if any.
Governance.UpgradeInProgress pending_version = 2;
// The target version that the SNS is in the process of upgrading to.
Governance.Version target_version = 3;
}

// Request to fail an upgrade proposal that is Adopted but not Executed or
Expand Down Expand Up @@ -2138,3 +2150,11 @@ message Account {
// subaccount (all bytes set to 0) is used.
optional Subaccount subaccount = 2;
}

message AdvanceTargetVersionRequest {

}

message AdvanceTargetVersionResponse {

}
43 changes: 43 additions & 0 deletions rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,10 @@ pub struct Governance {
pub is_finalizing_disburse_maturity: ::core::option::Option<bool>,
#[prost(message, optional, tag = "26")]
pub maturity_modulation: ::core::option::Option<governance::MaturityModulation>,
#[prost(message, optional, tag = "28")]
pub target_version: ::core::option::Option<governance::Version>,
#[prost(message, optional, tag = "29")]
pub cached_upgrade_steps: ::core::option::Option<governance::CachedUpgradeSteps>,
}
/// Nested message and enum types in `Governance`.
pub mod governance {
Expand Down Expand Up @@ -1904,6 +1908,22 @@ pub mod governance {
#[prost(uint64, optional, tag = "2")]
pub updated_at_timestamp_seconds: ::core::option::Option<u64>,
}
#[derive(
candid::CandidType,
candid::Deserialize,
comparable::Comparable,
Clone,
PartialEq,
::prost::Message,
)]
pub struct CachedUpgradeSteps {
#[prost(message, repeated, tag = "1")]
pub upgrade_steps: ::prost::alloc::vec::Vec<Version>,
#[prost(uint64, optional, tag = "2")]
pub requested_update_at_timestamp_seconds: ::core::option::Option<u64>,
#[prost(uint64, optional, tag = "3")]
pub received_update_at_timestamp_seconds: ::core::option::Option<u64>,
}
#[derive(
candid::CandidType,
candid::Deserialize,
Expand Down Expand Up @@ -2034,6 +2054,9 @@ pub struct GetRunningSnsVersionResponse {
/// The upgrade in progress, if any.
#[prost(message, optional, tag = "2")]
pub pending_version: ::core::option::Option<governance::UpgradeInProgress>,
/// The target version that the SNS is in the process of upgrading to.
#[prost(message, optional, tag = "3")]
pub target_version: ::core::option::Option<governance::Version>,
}
/// 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.
Expand Down Expand Up @@ -3330,6 +3353,26 @@ pub struct Account {
#[prost(message, optional, tag = "2")]
pub subaccount: ::core::option::Option<Subaccount>,
}
#[derive(
candid::CandidType,
candid::Deserialize,
comparable::Comparable,
Clone,
Copy,
PartialEq,
::prost::Message,
)]
pub struct AdvanceTargetVersionRequest {}
#[derive(
candid::CandidType,
candid::Deserialize,
comparable::Comparable,
Clone,
Copy,
PartialEq,
::prost::Message,
)]
pub struct AdvanceTargetVersionResponse {}
/// The different types of neuron permissions, i.e., privileges to modify a neuron,
/// that principals can have.
#[derive(
Expand Down
40 changes: 39 additions & 1 deletion rs/sns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
governance::{
self,
neuron_in_flight_command::{self, Command as InFlightCommand},
MaturityModulation, NeuronInFlightCommand, SnsMetadata, UpgradeInProgress, Version,
MaturityModulation, NeuronInFlightCommand, SnsMetadata, UpgradeInProgress, Version, CachedUpgradeSteps
},
governance_error::ErrorType,
manage_neuron::{
Expand Down Expand Up @@ -4632,6 +4632,44 @@ impl Governance {
self.maybe_move_staked_maturity();

self.maybe_gc();

self.maybe_update_cached_upgrade_steps().await;
}

async fn maybe_update_cached_upgrade_steps(&mut self) {
let update_interval = 5 * 60; // 5 minutes // TODO: move to a constant
let should_update = match self.proto.cached_upgrade_steps.clone() {
Some(cached_upgrade_steps) => {
let requested_update_at_timestamp_seconds = cached_upgrade_steps.requested_update_at_timestamp_seconds.unwrap_or(0);
let received_update_at_timestamp_seconds = cached_upgrade_steps.received_update_at_timestamp_seconds.unwrap_or(0);
let last_update = requested_update_at_timestamp_seconds.max(received_update_at_timestamp_seconds);
self.env.now() - last_update > update_interval
}
None => {
true
}
};

if should_update {
let current_version = self.proto.deployed_version_or_panic();
let requested_update_at_timestamp_seconds = self.env.now();
self.proto.cached_upgrade_steps = Some(CachedUpgradeSteps {
requested_update_at_timestamp_seconds: Some(requested_update_at_timestamp_seconds),
..self.proto.cached_upgrade_steps.clone().unwrap_or_default()
});
match crate::sns_upgrade::get_upgrade_path(&*self.env, current_version, self.env.canister_id().get()).await {
Ok(upgrade_path) => {
self.proto.cached_upgrade_steps = Some(CachedUpgradeSteps {
upgrade_steps: upgrade_path,
requested_update_at_timestamp_seconds: Some(requested_update_at_timestamp_seconds),
received_update_at_timestamp_seconds: Some(self.env.now()),
});
}
Err(err) => {
log!(ERROR, "Failed to get upgrade path: {}", err);
}
}
}
}

fn should_update_maturity_modulation(&self) -> bool {
Expand Down
2 changes: 2 additions & 0 deletions rs/sns/governance/src/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2489,6 +2489,8 @@ mod tests {
pending_version: None,
is_finalizing_disburse_maturity: None,
maturity_modulation: None,
target_version: None,
cached_upgrade_steps: None,
}
}

Expand Down
51 changes: 24 additions & 27 deletions rs/sns/governance/src/request_impls.rs
Original file line number Diff line number Diff line change
@@ -1,82 +1,79 @@
use ic_nervous_system_clients::Request;

use crate::pb::v1::{
ClaimSwapNeuronsRequest, ClaimSwapNeuronsResponse, FailStuckUpgradeInProgressRequest,
FailStuckUpgradeInProgressResponse, GetMaturityModulationRequest,
GetMaturityModulationResponse, GetMetadataRequest, GetMetadataResponse, GetMode,
GetModeResponse, GetNeuronResponse, GetProposalResponse, GetRunningSnsVersionResponse,
GetSnsInitializationParametersRequest, GetSnsInitializationParametersResponse,
ListNeuronsResponse, ListProposalsResponse, ManageNeuronResponse,
};

impl Request for ClaimSwapNeuronsRequest {
type Response = ClaimSwapNeuronsResponse;
impl Request for crate::pb::v1::ClaimSwapNeuronsRequest {
type Response = crate::pb::v1::ClaimSwapNeuronsResponse;
const METHOD: &'static str = "claim_swap_neurons";
const UPDATE: bool = true;
}

impl Request for FailStuckUpgradeInProgressRequest {
type Response = FailStuckUpgradeInProgressResponse;
impl Request for crate::pb::v1::FailStuckUpgradeInProgressRequest {
type Response = crate::pb::v1::FailStuckUpgradeInProgressResponse;
const METHOD: &'static str = "fail_stuck_upgrade_in_progress";
const UPDATE: bool = true;
}

impl Request for GetMaturityModulationRequest {
type Response = GetMaturityModulationResponse;
impl Request for crate::pb::v1::GetMaturityModulationRequest {
type Response = crate::pb::v1::GetMaturityModulationResponse;
const METHOD: &'static str = "get_maturity_modulation";
const UPDATE: bool = false;
}

impl Request for GetMetadataRequest {
type Response = GetMetadataResponse;
impl Request for crate::pb::v1::GetMetadataRequest {
type Response = crate::pb::v1::GetMetadataResponse;
const METHOD: &'static str = "get_metadata";
const UPDATE: bool = false;
}

impl Request for GetSnsInitializationParametersRequest {
type Response = GetSnsInitializationParametersResponse;
impl Request for crate::pb::v1::GetSnsInitializationParametersRequest {
type Response = crate::pb::v1::GetSnsInitializationParametersResponse;
const METHOD: &'static str = "get_sns_initialization_parameters";
const UPDATE: bool = false;
}

impl Request for GetMode {
type Response = GetModeResponse;
impl Request for crate::pb::v1::GetMode {
type Response = crate::pb::v1::GetModeResponse;
const METHOD: &'static str = "get_mode";
const UPDATE: bool = false;
}

impl Request for crate::pb::v1::GetNeuron {
type Response = GetNeuronResponse;
type Response = crate::pb::v1::GetNeuronResponse;
const METHOD: &'static str = "get_neuron";
const UPDATE: bool = false;
}

impl Request for crate::pb::v1::GetProposal {
type Response = GetProposalResponse;
type Response = crate::pb::v1::GetProposalResponse;
const METHOD: &'static str = "get_proposal";
const UPDATE: bool = false;
}

impl Request for crate::pb::v1::ListNeurons {
type Response = ListNeuronsResponse;
type Response = crate::pb::v1::ListNeuronsResponse;
const METHOD: &'static str = "list_neurons";
const UPDATE: bool = false;
}

impl Request for crate::pb::v1::ListProposals {
type Response = ListProposalsResponse;
type Response = crate::pb::v1::ListProposalsResponse;
const METHOD: &'static str = "list_proposals";
const UPDATE: bool = false;
}

impl Request for crate::pb::v1::ManageNeuron {
type Response = ManageNeuronResponse;
type Response = crate::pb::v1::ManageNeuronResponse;
const METHOD: &'static str = "manage_neuron";
const UPDATE: bool = true;
}

impl Request for crate::pb::v1::GetRunningSnsVersionRequest {
type Response = GetRunningSnsVersionResponse;
type Response = crate::pb::v1::GetRunningSnsVersionResponse;
const METHOD: &'static str = "get_running_sns_version";
const UPDATE: bool = false;
}

impl Request for crate::pb::v1::AdvanceTargetVersionRequest {
type Response = crate::pb::v1::AdvanceTargetVersionResponse;
const METHOD: &'static str = "advance_target_version";
const UPDATE: bool = true;
}
Loading
Loading