Skip to content

Commit

Permalink
chore(sns): Remove wasm_memory_limit migration code (#1729)
Browse files Browse the repository at this point in the history
## Problem

In #1540 and #1521, we added code for performing a migration of
wasm_memory_limit for all SNS canisters other than SNS Governance. That
code is no longer necessary now that it has been released.

## Solution

Remove that code. A few changes to tests are still useful and have been
left in.
  • Loading branch information
anchpop authored Sep 28, 2024
1 parent 6cb46aa commit db5901a
Show file tree
Hide file tree
Showing 14 changed files with 4 additions and 261 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion rs/sns/governance/canister/governance.did
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ type Governance = record {
sns_metadata : opt ManageSnsMetadata;
neurons : vec record { text; Neuron };
genesis_timestamp_seconds : nat64;
migrated_root_wasm_memory_limit : opt bool;
};

type GovernanceCachedMetrics = record {
Expand Down
1 change: 0 additions & 1 deletion rs/sns/governance/canister/governance_test.did
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ type Governance = record {
sns_metadata : opt ManageSnsMetadata;
neurons : vec record { text; Neuron };
genesis_timestamp_seconds : nat64;
migrated_root_wasm_memory_limit : opt bool;
};

type GovernanceCachedMetrics = record {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1447,7 +1447,8 @@ message Governance {

MaturityModulation maturity_modulation = 26;

optional bool migrated_root_wasm_memory_limit = 27;
reserved "migrated_root_wasm_memory_limit";
reserved 27;
}

// Request message for 'get_metadata'.
Expand Down
31 changes: 0 additions & 31 deletions rs/sns/governance/src/canister_control.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::governance::Governance;
use crate::{
governance::log_prefix,
logs::{ERROR, INFO},
Expand All @@ -16,7 +15,6 @@ use ic_canister_log::log;
use ic_nervous_system_clients::{
canister_id_record::CanisterIdRecord,
canister_status::{CanisterStatusResultFromManagementCanister, CanisterStatusType},
update_settings::{CanisterSettings, UpdateSettings},
};
use std::convert::TryFrom;

Expand Down Expand Up @@ -322,32 +320,3 @@ pub async fn perform_execute_generic_nervous_system_function_call(
}
}
}

pub async fn update_root_canister_settings(
governance: &Governance,
settings: CanisterSettings,
) -> Result<(), GovernanceError> {
let update_settings_args = UpdateSettings {
canister_id: PrincipalId::from(governance.proto.root_canister_id_or_panic()),
settings,
// allowed to be None
sender_canister_version: None,
};
governance
.env
.call_canister(
ic_management_canister_types::IC_00,
"update_settings",
Encode!(&update_settings_args).expect("Unable to encode update_settings args."),
)
.await
.map(|_reply| ())
.map_err(|err| {
let err = GovernanceError::new_with_message(
ErrorType::External,
format!("Failed to update settings of the root canister: {:?}", err),
);
log!(ERROR, "{}{:?}", log_prefix(), err);
err
})
}
2 changes: 0 additions & 2 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,8 +1644,6 @@ 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(bool, optional, tag = "27")]
pub migrated_root_wasm_memory_limit: ::core::option::Option<bool>,
}
/// Nested message and enum types in `Governance`.
pub mod governance {
Expand Down
36 changes: 2 additions & 34 deletions rs/sns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::pb::v1::{
use crate::{
canister_control::{
get_canister_id, perform_execute_generic_nervous_system_function_call,
update_root_canister_settings, upgrade_canister_directly,
upgrade_canister_directly,
},
logs::{ERROR, INFO},
neuron::{
Expand Down Expand Up @@ -82,7 +82,6 @@ use ic_management_canister_types::{
CanisterChangeDetails, CanisterInfoRequest, CanisterInfoResponse, CanisterInstallMode,
};
use ic_nervous_system_clients::ledger_client::ICRC1Ledger;
use ic_nervous_system_clients::update_settings::CanisterSettings;
use ic_nervous_system_collections_union_multi_map::UnionMultiMap;
use ic_nervous_system_common::{
cmc::CMC,
Expand All @@ -95,10 +94,7 @@ use ic_nervous_system_governance::maturity_modulation::{
};
use ic_nervous_system_lock::acquire;
use ic_nervous_system_root::change_canister::ChangeCanisterRequest;
use ic_nns_constants::{
DEFAULT_SNS_NON_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT,
LEDGER_CANISTER_ID as NNS_LEDGER_CANISTER_ID,
};
use ic_nns_constants::LEDGER_CANISTER_ID as NNS_LEDGER_CANISTER_ID;
use ic_protobuf::types::v1::CanisterInstallMode as CanisterInstallModeProto;
use ic_sns_governance_proposal_criticality::ProposalCriticality;
use ic_sns_governance_token_valuation::Valuation;
Expand Down Expand Up @@ -4601,32 +4597,6 @@ impl Governance {
true
}

async fn maybe_migrate_root_wasm_memory_limit(&mut self) {
if self
.proto
.migrated_root_wasm_memory_limit
.unwrap_or_default()
{
return;
}

// Set migrated_root_wasm_memory_limit to Some(true) so this doesn't run again
self.proto.migrated_root_wasm_memory_limit = Some(true);

let settings = CanisterSettings {
wasm_memory_limit: Some(candid::Nat::from(
DEFAULT_SNS_NON_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT,
)),
..Default::default()
};

// Set root settings
match update_root_canister_settings(self, settings).await {
Ok(_) => (),
Err(e) => log!(ERROR, "Failed to update root canister settings: {}", e),
}
}

/// Runs periodic tasks that are not directly triggered by user input.
pub async fn heartbeat(&mut self) {
self.process_proposals();
Expand Down Expand Up @@ -4664,8 +4634,6 @@ impl Governance {
self.maybe_move_staked_maturity();

self.maybe_gc();

self.maybe_migrate_root_wasm_memory_limit().await;
}

fn should_update_maturity_modulation(&self) -> bool {
Expand Down
1 change: 0 additions & 1 deletion rs/sns/governance/src/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2489,7 +2489,6 @@ mod tests {
pending_version: None,
is_finalizing_disburse_maturity: None,
maturity_modulation: None,
migrated_root_wasm_memory_limit: Some(true),
}
}

Expand Down
6 changes: 0 additions & 6 deletions rs/sns/governance/tests/fixtures/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,6 @@ impl Default for GovernanceCanisterFixtureBuilder {
current_basis_points: Some(0),
updated_at_timestamp_seconds: Some(1),
}),
migrated_root_wasm_memory_limit: Some(true),
..Default::default()
},
sns_ledger_transforms: Vec::default(),
Expand Down Expand Up @@ -1076,11 +1075,6 @@ impl GovernanceCanisterFixtureBuilder {
// Set up the canister fixture with our neuron.
self.add_neuron(neuron)
}

pub fn set_migrated_root_wasm_memory_limit(mut self, value: bool) -> Self {
self.governance.migrated_root_wasm_memory_limit = Some(value);
self
}
}

#[macro_export]
Expand Down
42 changes: 0 additions & 42 deletions rs/sns/governance/tests/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::fixtures::{
GovernanceCanisterFixtureBuilder, NeuronBuilder, TargetLedger,
};
use assert_matches::assert_matches;
use fixtures::environment_fixture::CanisterCallReply;
use ic_base_types::{CanisterId, PrincipalId};
use ic_nervous_system_common::{E8, ONE_DAY_SECONDS, ONE_MONTH_SECONDS};
use ic_nervous_system_common_test_keys::{
Expand Down Expand Up @@ -3018,44 +3017,3 @@ fn test_deregister_dapp_has_higher_voting_thresholds() {
Percentage::from_basis_points(2000)
);
}

#[test]
fn test_updates_root_settings() {
let mut gov = {
// Set up the test environment migrated_root_wasm_memory_limit set to Some(false);
let gov_fixture_builder =
GovernanceCanisterFixtureBuilder::new().set_migrated_root_wasm_memory_limit(false);
let gov = gov_fixture_builder.create();
// The heartbeat will call the management canister to update the settings,
// so we need to mock the reply
gov.environment_fixture
.environment_fixture_state
.lock()
.unwrap()
.mocked_canister_replies
.push(CanisterCallReply::Response(Vec::new()));
gov
};

gov.heartbeat();
assert!(gov
.governance
.proto
.migrated_root_wasm_memory_limit
.unwrap());
}

#[test]
#[should_panic(
expected = "Expected there to be a mocked canister reply on the stack for method `update_settings`"
)]
fn test_updates_root_settings_calls_management_canister() {
let mut gov = {
// Set up the test environment migrated_root_wasm_memory_limit set to Some(false);
let gov_fixture_builder =
GovernanceCanisterFixtureBuilder::new().set_migrated_root_wasm_memory_limit(false);
gov_fixture_builder.create()
};

gov.heartbeat(); // should panic
}
1 change: 0 additions & 1 deletion rs/sns/root/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ DEPENDENCIES = [
"//rs/nervous_system/common",
"//rs/nervous_system/root",
"//rs/nervous_system/runtime",
"//rs/nns/constants",
"//rs/rust_canisters/canister_log",
"//rs/rust_canisters/http_types",
"//rs/sns/swap", # TODO[NNS1-2282]
Expand Down
1 change: 0 additions & 1 deletion rs/sns/root/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ ic-nervous-system-common = { path = "../../nervous_system/common" }
ic-nervous-system-common-build-metadata = { path = "../../nervous_system/common/build_metadata" }
ic-nervous-system-root = { path = "../../nervous_system/root" }
ic-nervous-system-runtime = { path = "../../nervous_system/runtime" }
ic-nns-constants = { path = "../../nns/constants" }
ic-sns-swap = { path = "../swap" }
icrc-ledger-types = { path = "../../../packages/icrc-ledger-types" }
prost = { workspace = true }
Expand Down
10 changes: 0 additions & 10 deletions rs/sns/root/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,6 @@ fn canister_post_upgrade() {
);
canister_init(state);

// Kick off migration one minute after upgrade
let duration = Duration::from_secs(60);
ic_cdk_timers::set_timer(duration, || {
ic_cdk::spawn(async {
let management_canister_client =
ManagementCanisterClientImpl::<CanisterRuntime>::new(None);
SnsRootCanister::migrate_canister_settings(&STATE, &management_canister_client).await;
})
});

log!(INFO, "canister_post_upgrade: Done!");
}

Expand Down
Loading

0 comments on commit db5901a

Please sign in to comment.