Revert "feat(sns): Only set the wasm_memory_limit for SNS Governanc…

…e when deploying an SNS (#1427)"

This reverts commit a4b9ab2.

rs/nervous_system/integration_tests/tests/sns_lifecycle.rs

@@ -1,4 +1,3 @@
-use crate::sns::root::get_sns_canisters_summary;
 use assert_matches::assert_matches;
 use candid::{Nat, Principal};
 use canister_test::Wasm;
@@ -28,7 +27,6 @@ use ic_sns_governance::{
     pb::v1::{self as sns_pb, NeuronPermissionType},
 };
 use ic_sns_init::distributions::MAX_DEVELOPER_DISTRIBUTION_COUNT;
-use ic_sns_root::CanisterSummary;
 use ic_sns_swap::{
     pb::v1::{
         new_sale_ticket_response, set_dapp_controllers_call_result, set_mode_call_result,
@@ -1880,47 +1878,6 @@ fn test_sns_lifecycle(
         "No archives found from get_sns_canisters_summary response: {:#?}",
         response
     );
-
-    // Check that the SNS framework canister settings are as expected
-    {
-        // get SNS canisters summary
-        let sns_canisters_summary = get_sns_canisters_summary(&pocket_ic, sns_root_canister_id);
-        fn get_wasm_memory_limit(summary: Option<CanisterSummary>) -> u64 {
-            u64::try_from(
-                summary
-                    .unwrap()
-                    .status
-                    .unwrap()
-                    .settings
-                    .wasm_memory_limit
-                    .unwrap()
-                    .0,
-            )
-            .unwrap()
-        }
-        // Governance should have a higher memory limit
-        assert_eq!(
-            get_wasm_memory_limit(sns_canisters_summary.governance),
-            4 * 1024 * 1024 * 1024,
-        );
-        // Other canisters should have a lower memory limit
-        assert_eq!(
-            get_wasm_memory_limit(sns_canisters_summary.root),
-            3 * 1024 * 1024 * 1024,
-        );
-        assert_eq!(
-            get_wasm_memory_limit(sns_canisters_summary.swap),
-            3 * 1024 * 1024 * 1024,
-        );
-        assert_eq!(
-            get_wasm_memory_limit(sns_canisters_summary.ledger),
-            3 * 1024 * 1024 * 1024,
-        );
-        assert_eq!(
-            get_wasm_memory_limit(sns_canisters_summary.index),
-            3 * 1024 * 1024 * 1024,
-        );
-    }
 }
 
 #[test]

rs/nns/constants/src/lib.rs

@@ -163,13 +163,10 @@ const NNS_GOVERNANCE_CANISTER_MEMORY_ALLOCATION_IN_BYTES: u64 = 10 * 1024 * 1024
 // The default memory allocation to set for the remaining NNS canister (1GiB)
 const NNS_DEFAULT_CANISTER_MEMORY_ALLOCATION_IN_BYTES: u64 = 1024 * 1024 * 1024;
 
-/// The current value is 4 GiB, s.t. the SNS governance canister never hits the soft memory limit.
+/// The current value is 4 GiB, s.t. the SNS framework canisters never hit the soft memory limit.
 /// This mitigates the risk that an SNS Governance canister runs out of memory and proposals cannot
 /// be passed anymore.
-pub const DEFAULT_SNS_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT: u64 = 1 << 32;
-
-/// This value is 3GiB, which will leave a comfortable buffer in the situation when a canister runs out of memory
-pub const DEFAULT_SNS_NON_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT: u64 = 3 * (1 << 30);
+pub const DEFAULT_SNS_FRAMEWORK_CANISTER_WASM_MEMORY_LIMIT: u64 = 1 << 32;
 
 /// Returns the memory allocation of the given nns canister.
 pub fn memory_allocation_of(canister_id: CanisterId) -> u64 {

rs/nns/governance/tests/governance.rs

@@ -39,7 +39,8 @@ use ic_nns_common::{
     types::UpdateIcpXdrConversionRatePayload,
 };
 use ic_nns_constants::{
-    GOVERNANCE_CANISTER_ID, LEDGER_CANISTER_ID as ICP_LEDGER_CANISTER_ID, SNS_WASM_CANISTER_ID,
+    DEFAULT_SNS_FRAMEWORK_CANISTER_WASM_MEMORY_LIMIT, GOVERNANCE_CANISTER_ID,
+    LEDGER_CANISTER_ID as ICP_LEDGER_CANISTER_ID, SNS_WASM_CANISTER_ID,
 };
 use ic_nns_governance::{
     governance::{
@@ -11774,7 +11775,7 @@ lazy_static! {
                     Some(517576), // memory_allocation
                     448076, // freezing_threshold
                     268693, // idle_cycles_burned_per_day
-                    (3.5 * (1 << 30) as f32) as u64, // wasm_memory_limit (3.5gb)
+                    DEFAULT_SNS_FRAMEWORK_CANISTER_WASM_MEMORY_LIMIT, // wasm_memory_limit
                 )),
             }),
             governance: Some(ic_sns_root::CanisterSummary {

rs/nns/sns-wasm/canister/canister.rs

@@ -15,7 +15,7 @@ use ic_nervous_system_clients::{
     canister_status::{canister_status, CanisterStatusResultV2, CanisterStatusType},
 };
 use ic_nervous_system_runtime::DfnRuntime;
-use ic_nns_constants::GOVERNANCE_CANISTER_ID;
+use ic_nns_constants::{DEFAULT_SNS_FRAMEWORK_CANISTER_WASM_MEMORY_LIMIT, GOVERNANCE_CANISTER_ID};
 use ic_nns_handler_root_interface::client::NnsRootCanisterClientImpl;
 use ic_sns_wasm::{
     canister_api::CanisterApi,
@@ -69,17 +69,18 @@ impl CanisterApi for CanisterApiImpl {
         target_subnet: SubnetId,
         controller_id: PrincipalId,
         cycles: Cycles,
-        wasm_memory_limit: u64,
     ) -> Result<CanisterId, String> {
-        let settings = CanisterSettingsArgsBuilder::new()
-            .with_controllers(vec![controller_id])
-            .with_wasm_memory_limit(wasm_memory_limit);
         let result: Result<CanisterIdRecord, _> = dfn_core::api::call_with_funds_and_cleanup(
             target_subnet.into(),
             &Method::CreateCanister.to_string(),
             candid_one,
             CreateCanisterArgs {
-                settings: Some(settings.build()),
+                settings: Some(
+                    CanisterSettingsArgsBuilder::new()
+                        .with_controllers(vec![controller_id])
+                        .with_wasm_memory_limit(DEFAULT_SNS_FRAMEWORK_CANISTER_WASM_MEMORY_LIMIT)
+                        .build(),
+                ),
                 sender_canister_version: Some(dfn_core::api::canister_version()),
             },
             Funds::new(cycles.get().try_into().unwrap()),

rs/nns/sns-wasm/src/canister_api.rs

@@ -13,7 +13,6 @@ pub trait CanisterApi {
         target_subnet: SubnetId,
         controller_id: PrincipalId,
         cycles: Cycles,
-        wasm_memory_limit: u64,
     ) -> Result<CanisterId, String>;
 
     /// Delete a canister that has been created

rs/nns/sns-wasm/src/sns_wasm.rs

@@ -27,10 +27,6 @@ use ic_cdk::api::stable::StableMemory;
 use ic_nervous_system_clients::canister_id_record::CanisterIdRecord;
 use ic_nervous_system_common::{ONE_TRILLION, SNS_CREATION_FEE};
 use ic_nervous_system_proto::pb::v1::Canister;
-use ic_nns_constants::{
-    DEFAULT_SNS_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT,
-    DEFAULT_SNS_NON_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT,
-};
 use ic_nns_constants::{GOVERNANCE_CANISTER_ID, ROOT_CANISTER_ID};
 use ic_nns_handler_root_interface::{
     client::NnsRootCanisterClient, ChangeCanisterControllersRequest,
@@ -1175,68 +1171,61 @@ where
         initial_cycles_per_canister: u64,
     ) -> Result<SnsCanisterIds, (String, Option<SnsCanisterIds>)> {
         let this_canister_id = canister_api.local_canister_id().get();
-        let new_canister = |canister_type: SnsCanisterType| {
+        let new_canister = || {
             canister_api.create_canister(
                 subnet_id,
                 this_canister_id,
                 Cycles::new(initial_cycles_per_canister.into()),
-                if canister_type == SnsCanisterType::Governance {
-                    DEFAULT_SNS_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT
-                } else {
-                    DEFAULT_SNS_NON_GOVERNANCE_CANISTER_WASM_MEMORY_LIMIT
-                },
             )
         };
 
         // Create these in order instead of join_all to get deterministic ordering for tests
-        let root = new_canister(SnsCanisterType::Root).await;
-        let governance = new_canister(SnsCanisterType::Governance).await;
-        let ledger = new_canister(SnsCanisterType::Ledger).await;
-        let swap = new_canister(SnsCanisterType::Swap).await;
-        let index = new_canister(SnsCanisterType::Index).await;
+        let canisters_attempted = vec![
+            new_canister().await,
+            new_canister().await,
+            new_canister().await,
+            new_canister().await,
+            new_canister().await,
+        ];
+        let canisters_attempted_count = canisters_attempted.len();
 
-        let (root, governance, ledger, swap, index) = match (root, governance, ledger, swap, index)
-        {
-            (Ok(root), Ok(governance), Ok(ledger), Ok(swap), Ok(index)) => {
-                (root, governance, ledger, swap, index)
-            }
-            (root, governance, ledger, swap, index) => {
-                let canisters_to_delete = SnsCanisterIds {
-                    root: root.ok().map(|canister_id| canister_id.get()),
-                    governance: governance.ok().map(|canister_id| canister_id.get()),
-                    ledger: ledger.ok().map(|canister_id| canister_id.get()),
-                    swap: swap.ok().map(|canister_id| canister_id.get()),
-                    index: index.ok().map(|canister_id| canister_id.get()),
-                };
-                let problem_canisters = vec![
-                    canisters_to_delete.root.is_none().then_some("Root"),
-                    canisters_to_delete
-                        .governance
-                        .is_none()
-                        .then_some("Governance"),
-                    canisters_to_delete.ledger.is_none().then_some("Ledger"),
-                    canisters_to_delete.swap.is_none().then_some("Swap"),
-                    canisters_to_delete.index.is_none().then_some("Index"),
-                ]
-                .into_iter()
-                .flatten()
-                .collect::<Vec<_>>();
-                return Err((
-                    format!(
-                        "Could not create some canisters: {}",
-                        problem_canisters.join(", ")
-                    ),
-                    Some(canisters_to_delete),
-                ));
-            }
-        };
+        let mut canisters_created = canisters_attempted
+            .into_iter()
+            .flatten()
+            .collect::<Vec<_>>();
+
+        let canisters_created_count = canisters_created.len();
+
+        if canisters_created_count < canisters_attempted_count {
+            let next = |c: &mut Vec<CanisterId>| {
+                if !c.is_empty() {
+                    Some(c.remove(0).get())
+                } else {
+                    None
+                }
+            };
+            let canisters_to_delete = SnsCanisterIds {
+                root: next(&mut canisters_created),
+                governance: next(&mut canisters_created),
+                ledger: next(&mut canisters_created),
+                swap: next(&mut canisters_created),
+                index: next(&mut canisters_created),
+            };
+            return Err((
+                format!(
+                    "Could not create needed canisters. Only created {} but 5 needed.",
+                    canisters_created_count
+                ),
+                Some(canisters_to_delete),
+            ));
+        }
 
         Ok(SnsCanisterIds {
-            root: Some(root.get()),
-            governance: Some(governance.get()),
-            ledger: Some(ledger.get()),
-            swap: Some(swap.get()),
-            index: Some(index.get()),
+            root: Some(canisters_created.remove(0).get()),
+            governance: Some(canisters_created.remove(0).get()),
+            ledger: Some(canisters_created.remove(0).get()),
+            swap: Some(canisters_created.remove(0).get()),
+            index: Some(canisters_created.remove(0).get()),
         })
     }
 
@@ -2042,7 +2031,6 @@ mod test {
             _target_subnet: SubnetId,
             _controller_id: PrincipalId,
             _cycles: Cycles,
-            _wasm_memory_limit: u64,
         ) -> Result<CanisterId, String> {
             let mut errors = self.errors_on_create_canister.lock().unwrap();
             if errors.len() > 0 {
@@ -3278,10 +3266,10 @@ mod test {
 
         let subnet_id = subnet_test_id(1);
 
-        let governance_id = canister_test_id(1);
-        let ledger_id = canister_test_id(2);
-        let swap_id = canister_test_id(3);
-        let index_id = canister_test_id(4);
+        let root_id = canister_test_id(1);
+        let governance_id = canister_test_id(2);
+        let ledger_id = canister_test_id(3);
+        let swap_id = canister_test_id(4);
 
         let sns_init_payload = SnsInitPayload {
             dapp_canisters: None,
@@ -3296,21 +3284,22 @@ mod test {
             true,
             vec![],
             vec![],
-            vec![governance_id, ledger_id, swap_id, index_id],
+            vec![root_id, governance_id, ledger_id, swap_id],
             vec![],
             vec![],
             vec![],
             DeployNewSnsResponse {
                 canisters: Some(SnsCanisterIds {
-                    root: None,
+                    root: Some(root_id.get()),
                     ledger: Some(ledger_id.get()),
                     governance: Some(governance_id.get()),
                     swap: Some(swap_id.get()),
-                    index: Some(index_id.get()),
+                    index: None,
                 }),
                 subnet_id: Some(subnet_id.get()),
                 error: Some(SnsWasmError {
-                    message: "Could not create some canisters: Root".to_string(),
+                    message: "Could not create needed canisters. Only created 4 but 5 needed."
+                        .to_string(),
                 }),
                 dapp_canisters_transfer_result: Some(DappCanistersTransferResult {
                     restored_dapp_canisters: vec![],