From 2a6b650173e3afb8e66996aa0635e9fd79fb8566 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Wed, 29 Jan 2025 10:57:59 +0000 Subject: [PATCH 01/10] first --- rs/nervous_system/agent/src/lib.rs | 14 - .../src/pocket_ic_helpers.rs | 316 ++++++++++++------ 2 files changed, 207 insertions(+), 123 deletions(-) diff --git a/rs/nervous_system/agent/src/lib.rs b/rs/nervous_system/agent/src/lib.rs index 47855ef2263..730b744782a 100644 --- a/rs/nervous_system/agent/src/lib.rs +++ b/rs/nervous_system/agent/src/lib.rs @@ -27,20 +27,6 @@ pub trait Request: Send { type Response: CandidType + DeserializeOwned; } -// impl Request for R { -// fn method(&self) -> &'static str { -// Self::METHOD -// } -// fn update(&self) -> bool { -// Self::UPDATE -// } -// fn payload(&self) -> Result, candid::Error> { -// candid::encode_one(self) -// } - -// type Response = ::Response; -// } - pub trait CallCanisters: sealed::Sealed { type Error: Display + Send + std::error::Error + 'static; fn call( diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index 98f1775e71a..e8209ba84ff 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -314,6 +314,203 @@ pub async fn add_wasms_to_sns_wasm( }) } +/// A builder for fine-tuning and installing the NNS canister suite in PocketIc. +pub struct NnsInstaller { + mainnet_nns_canister_versions: Option, + neurons_fund_hotkeys: Vec, + custom_initial_registry_mutations: Option>, + initial_balances: Vec<(AccountIdentifier, Tokens)>, +} + +impl NnsInstaller { + pub fn new() -> Self { + Self { + // Enforce an explicit decision. + mainnet_nns_canister_versions: None, + neurons_fund_hotkeys: vec![], + custom_initial_registry_mutations: None, + initial_balances: vec![], + } + } + + /// Requests the mainnet Wasm versions for all NNS canisters being installed. + pub fn with_mainnet_nns_canister_versions(&mut self) -> &mut Self { + self.mainnet_nns_canister_versions = Some(true); + self + } + + /// Requests tip-of-this-branch Wasm versions for all NNS canisters being installed. + pub fn with_tip_nns_canister_versions(&mut self) -> &mut Self { + self.mainnet_nns_canister_versions = Some(false); + self + } + + /// Requests that the NNS Governance is initialized with a Neurons' Fund neuron controlled + /// by `neurons_fund_hotkeys`. + pub fn with_neurons_fund_hotkeys( + &mut self, + neurons_fund_hotkeys: Vec, + ) -> &mut Self { + self.neurons_fund_hotkeys = neurons_fund_hotkeys; + self + } + + /// Requests that the NNS Registry is initialized with `custom_initial_registry_mutations`. + /// + /// The custom mutations must result in an invariant-compliant Registry state. + /// + /// Without this specification, default initial Registry mutations will be used, which are + /// guaranteed to be compliant. + pub fn with_custom_initial_registry_mutations( + &mut self, + custom_initial_registry_mutations: Vec, + ) -> &mut Self { + self.custom_initial_registry_mutations = Some(custom_initial_registry_mutations); + self + } + + /// Requests `initial_balances` as a Vec of `(test_user_icp_ledger_account, + /// test_user_icp_ledger_initial_balance)` pairs, representing some initial ICP balances. + pub fn with_initial_balances( + &mut self, + initial_balances: Vec<(AccountIdentifier, Tokens)>, + ) -> &mut Self { + self.initial_balances = initial_balances; + self + } + + /// Installs the NNS canister suite. + /// + /// Ensures that there is a whale neuron with `TEST_NEURON_1_ID`. + /// + /// Requires that the PocketIC instance has an NNS and an SNS subnet. + /// + /// Returns the list of `controller_principal_id`s of pre-configured NNS neurons. + pub async fn install(self, pocket_ic: &PocketIc) -> Vec { + let with_mainnet_canister_versions = self + .mainnet_nns_canister_versions + .expect("Please explicitly request either mainnet or tip-of-the-branch NNS version."); + + let topology = pocket_ic.topology().await; + + let sns_subnet_id = topology.get_sns().expect("No SNS subnet found"); + let sns_subnet_id = SubnetId::from(PrincipalId::from(sns_subnet_id)); + + let mut nns_init_payload_builder = NnsInitPayloadsBuilder::new(); + + if let Some(custom_initial_registry_mutations) = self.custom_initial_registry_mutations { + nns_init_payload_builder.with_initial_mutations(custom_initial_registry_mutations); + } else { + nns_init_payload_builder.with_initial_invariant_compliant_mutations(); + } + + let maturity_equivalent_icp_e8s = 1_500_000 * E8; + nns_init_payload_builder + .with_test_neurons_fund_neurons_with_hotkeys( + self.neurons_fund_hotkeys, + maturity_equivalent_icp_e8s, + ) + .with_sns_dedicated_subnets(vec![sns_subnet_id]) + .with_sns_wasm_access_controls(true); + + for (test_user_icp_ledger_account, test_user_icp_ledger_initial_balance) in + self.initial_balances + { + nns_init_payload_builder.with_ledger_account( + test_user_icp_ledger_account, + test_user_icp_ledger_initial_balance, + ); + } + + let nns_init_payload = nns_init_payload_builder.build(); + + let (governance_wasm, ledger_wasm, root_wasm, lifeline_wasm, sns_wasm_wasm, registry_wasm) = + if with_mainnet_canister_versions { + ( + build_mainnet_governance_wasm(), + build_mainnet_ledger_wasm(), + build_mainnet_root_wasm(), + build_mainnet_lifeline_wasm(), + build_mainnet_sns_wasms_wasm(), + build_mainnet_registry_wasm(), + ) + } else { + ( + build_governance_wasm(), + build_ledger_wasm(), + build_root_wasm(), + build_lifeline_wasm(), + build_sns_wasms_wasm(), + build_registry_wasm(), + ) + }; + + install_canister( + pocket_ic, + "ICP Ledger", + LEDGER_CANISTER_ID, + Encode!(&nns_init_payload.ledger).unwrap(), + ledger_wasm, + Some(ROOT_CANISTER_ID.get()), + ) + .await; + install_canister( + pocket_ic, + "NNS Root", + ROOT_CANISTER_ID, + Encode!(&nns_init_payload.root).unwrap(), + root_wasm, + Some(LIFELINE_CANISTER_ID.get()), + ) + .await; + install_canister( + pocket_ic, + "NNS Governance", + GOVERNANCE_CANISTER_ID, + nns_init_payload.governance.encode_to_vec(), + governance_wasm, + Some(ROOT_CANISTER_ID.get()), + ) + .await; + install_canister( + pocket_ic, + "Lifeline", + LIFELINE_CANISTER_ID, + Encode!(&nns_init_payload.lifeline).unwrap(), + lifeline_wasm, + Some(ROOT_CANISTER_ID.get()), + ) + .await; + install_canister( + pocket_ic, + "NNS SNS-W", + SNS_WASM_CANISTER_ID, + Encode!(&nns_init_payload.sns_wasms).unwrap(), + sns_wasm_wasm, + Some(ROOT_CANISTER_ID.get()), + ) + .await; + install_canister( + pocket_ic, + "Registry", + REGISTRY_CANISTER_ID, + Encode!(&nns_init_payload.registry).unwrap(), + registry_wasm, + Some(ROOT_CANISTER_ID.get()), + ) + .await; + + let nns_neurons = nns_init_payload + .governance + .neurons + .values() + .map(|neuron| neuron.controller.unwrap()) + .collect(); + + nns_neurons + } +} + /// Installs the NNS canisters, ensuring that there is a whale neuron with `TEST_NEURON_1_ID`. /// Requires PocketIC to have at least an NNS and an SNS subnet. /// @@ -335,122 +532,23 @@ pub async fn install_nns_canisters( custom_initial_registry_mutations: Option>, neurons_fund_hotkeys: Vec, ) -> Vec { - let topology = pocket_ic.topology().await; + let mut nns_installer = NnsInstaller::new(); - let sns_subnet_id = topology.get_sns().expect("No SNS subnet found"); - let sns_subnet_id = PrincipalId::from(sns_subnet_id); - let sns_subnet_id = SubnetId::from(sns_subnet_id); - println!("sns_subnet_id = {:?}", sns_subnet_id); + nns_installer + .with_initial_balances(initial_balances) + .with_neurons_fund_hotkeys(neurons_fund_hotkeys); - let mut nns_init_payload_builder = NnsInitPayloadsBuilder::new(); - - if let Some(custom_initial_registry_mutations) = custom_initial_registry_mutations { - nns_init_payload_builder.with_initial_mutations(custom_initial_registry_mutations); + if with_mainnet_nns_canister_versions { + nns_installer.with_mainnet_nns_canister_versions(); } else { - nns_init_payload_builder.with_initial_invariant_compliant_mutations(); + nns_installer.with_tip_nns_canister_versions(); } - let maturity_equivalent_icp_e8s = 1_500_000 * E8; - nns_init_payload_builder - .with_test_neurons_fund_neurons_with_hotkeys( - neurons_fund_hotkeys, - maturity_equivalent_icp_e8s, - ) - .with_sns_dedicated_subnets(vec![sns_subnet_id]) - .with_sns_wasm_access_controls(true); - for (test_user_icp_ledger_account, test_user_icp_ledger_initial_balance) in initial_balances { - nns_init_payload_builder.with_ledger_account( - test_user_icp_ledger_account, - test_user_icp_ledger_initial_balance, - ); + if let Some(custom_initial_registry_mutations) = custom_initial_registry_mutations { + nns_installer.with_custom_initial_registry_mutations(custom_initial_registry_mutations); } - let nns_init_payload = nns_init_payload_builder.build(); - - let (governance_wasm, ledger_wasm, root_wasm, lifeline_wasm, sns_wasm_wasm, registry_wasm) = - if with_mainnet_nns_canister_versions { - ( - build_mainnet_governance_wasm(), - build_mainnet_ledger_wasm(), - build_mainnet_root_wasm(), - build_mainnet_lifeline_wasm(), - build_mainnet_sns_wasms_wasm(), - build_mainnet_registry_wasm(), - ) - } else { - ( - build_governance_wasm(), - build_ledger_wasm(), - build_root_wasm(), - build_lifeline_wasm(), - build_sns_wasms_wasm(), - build_registry_wasm(), - ) - }; - - install_canister( - pocket_ic, - "ICP Ledger", - LEDGER_CANISTER_ID, - Encode!(&nns_init_payload.ledger).unwrap(), - ledger_wasm, - Some(ROOT_CANISTER_ID.get()), - ) - .await; - install_canister( - pocket_ic, - "NNS Root", - ROOT_CANISTER_ID, - Encode!(&nns_init_payload.root).unwrap(), - root_wasm, - Some(LIFELINE_CANISTER_ID.get()), - ) - .await; - install_canister( - pocket_ic, - "NNS Governance", - GOVERNANCE_CANISTER_ID, - nns_init_payload.governance.encode_to_vec(), - governance_wasm, - Some(ROOT_CANISTER_ID.get()), - ) - .await; - install_canister( - pocket_ic, - "Lifeline", - LIFELINE_CANISTER_ID, - Encode!(&nns_init_payload.lifeline).unwrap(), - lifeline_wasm, - Some(ROOT_CANISTER_ID.get()), - ) - .await; - install_canister( - pocket_ic, - "NNS SNS-W", - SNS_WASM_CANISTER_ID, - Encode!(&nns_init_payload.sns_wasms).unwrap(), - sns_wasm_wasm, - Some(ROOT_CANISTER_ID.get()), - ) - .await; - install_canister( - pocket_ic, - "Registry", - REGISTRY_CANISTER_ID, - Encode!(&nns_init_payload.registry).unwrap(), - registry_wasm, - Some(ROOT_CANISTER_ID.get()), - ) - .await; - - let nns_neurons = nns_init_payload - .governance - .neurons - .values() - .map(|neuron| neuron.controller.unwrap()) - .collect(); - - nns_neurons + nns_installer.install(pocket_ic).await } #[derive(Copy, Clone, Debug)] From 60df3b9109e7e91c5724225ae4d0746e9f13f9c0 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Wed, 29 Jan 2025 11:02:58 +0000 Subject: [PATCH 02/10] second --- .../integration_tests/src/pocket_ic_helpers.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index e8209ba84ff..d5bc13891a8 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -322,8 +322,14 @@ pub struct NnsInstaller { initial_balances: Vec<(AccountIdentifier, Tokens)>, } +impl Default for NnsInstaller { + fn default() -> Self { + Self::new() + } +} + impl NnsInstaller { - pub fn new() -> Self { + fn new() -> Self { Self { // Enforce an explicit decision. mainnet_nns_canister_versions: None, @@ -532,7 +538,7 @@ pub async fn install_nns_canisters( custom_initial_registry_mutations: Option>, neurons_fund_hotkeys: Vec, ) -> Vec { - let mut nns_installer = NnsInstaller::new(); + let mut nns_installer = NnsInstaller::default(); nns_installer .with_initial_balances(initial_balances) From b521875f7bede8bf95e171fa4b53ee6b0608ccb0 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Wed, 29 Jan 2025 12:23:39 +0000 Subject: [PATCH 03/10] first --- .../src/pocket_ic_helpers.rs | 68 ++++++++++++++++++- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index d5bc13891a8..933ec98e901 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -320,6 +320,7 @@ pub struct NnsInstaller { neurons_fund_hotkeys: Vec, custom_initial_registry_mutations: Option>, initial_balances: Vec<(AccountIdentifier, Tokens)>, + with_cycles_ledger: bool, } impl Default for NnsInstaller { @@ -336,6 +337,7 @@ impl NnsInstaller { neurons_fund_hotkeys: vec![], custom_initial_registry_mutations: None, initial_balances: vec![], + with_cycles_ledger: false, } } @@ -385,6 +387,11 @@ impl NnsInstaller { self } + pub fn with_cycles_ledger(&mut self) -> &mut Self { + self.with_cycles_ledger = true; + self + } + /// Installs the NNS canister suite. /// /// Ensures that there is a whale neuron with `TEST_NEURON_1_ID`. @@ -506,14 +513,69 @@ impl NnsInstaller { ) .await; - let nns_neurons = nns_init_payload + if self.with_cycles_ledger { + cycles_ledger::install(pocket_ic).await; + } + + nns_init_payload .governance .neurons .values() .map(|neuron| neuron.controller.unwrap()) - .collect(); + .collect() + } +} + +pub mod cycles_ledger { + use super::install_canister; + use candid::{CandidType, Encode, Principal}; + use canister_test::Wasm; + use ic_nns_constants::{CYCLES_LEDGER_CANISTER_ID, ROOT_CANISTER_ID}; + use pocket_ic::nonblocking::PocketIc; + + #[derive(Clone, Eq, PartialEq, Hash, Debug, CandidType)] + struct CyclesLedgerInitArgs { + pub index_id: Option, + pub max_blocks_per_request: u64, + } - nns_neurons + /// Argument taken by the Cycles Ledger canister. + /// + /// See (https://github.com/dfinity/cycles-ledger/tree/main/cycles-ledger + /// + /// ```candid + /// (variant { + /// Init = record { + /// index_id : opt principal; + /// max_blocks_per_request : nat64; + /// } + /// }) + /// ``` + #[derive(Clone, Eq, PartialEq, Hash, Debug, CandidType)] + enum CyclesLedgerArgs { + Init(CyclesLedgerInitArgs), + } + + pub async fn install(pocket_ic: &PocketIc) { + let features = &[]; + let cycles_ledger_wasm = + Wasm::from_location_specified_by_env_var("cycles_ledger", features).unwrap(); + + let arg = Encode!(&CyclesLedgerArgs::Init(CyclesLedgerInitArgs { + index_id: None, + max_blocks_per_request: 1000, + })) + .unwrap(); + + install_canister( + pocket_ic, + "Cycles Ledger", + CYCLES_LEDGER_CANISTER_ID, + arg, + cycles_ledger_wasm, + Some(ROOT_CANISTER_ID.get()), + ) + .await; } } From 080d590bad5650d2c7d3ac62fc29778a82417fb6 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Wed, 29 Jan 2025 13:19:50 +0000 Subject: [PATCH 04/10] second --- .../integration_tests/src/pocket_ic_helpers.rs | 6 +++--- ...upgrade_sns_controlled_canister_with_large_wasm.rs | 11 +++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index 933ec98e901..06c89d43e83 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -536,7 +536,7 @@ pub mod cycles_ledger { #[derive(Clone, Eq, PartialEq, Hash, Debug, CandidType)] struct CyclesLedgerInitArgs { pub index_id: Option, - pub max_blocks_per_request: u64, + pub max_transactions_per_request: u64, } /// Argument taken by the Cycles Ledger canister. @@ -547,7 +547,7 @@ pub mod cycles_ledger { /// (variant { /// Init = record { /// index_id : opt principal; - /// max_blocks_per_request : nat64; + /// max_transactions_per_request : nat64; /// } /// }) /// ``` @@ -563,7 +563,7 @@ pub mod cycles_ledger { let arg = Encode!(&CyclesLedgerArgs::Init(CyclesLedgerInitArgs { index_id: None, - max_blocks_per_request: 1000, + max_transactions_per_request: 1000, })) .unwrap(); diff --git a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs index 751238dc165..789ed6a694e 100644 --- a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs +++ b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs @@ -4,11 +4,11 @@ use candid::Principal; use canister_test::Wasm; use ic_management_canister_types::CanisterInstallMode; use ic_nervous_system_integration_tests::pocket_ic_helpers::{ - await_with_timeout, install_canister_on_subnet, nns, sns, + await_with_timeout, install_canister_on_subnet, nns, sns, NnsInstaller, }; use ic_nervous_system_integration_tests::{ create_service_nervous_system_builder::CreateServiceNervousSystemBuilder, - pocket_ic_helpers::{add_wasms_to_sns_wasm, install_nns_canisters}, + pocket_ic_helpers::add_wasms_to_sns_wasm, }; use ic_nns_constants::ROOT_CANISTER_ID; use ic_nns_test_utils::common::modify_wasm_bytes; @@ -107,14 +107,17 @@ async fn run_test(store_same_as_target: bool) { let pocket_ic = PocketIcBuilder::new() .with_nns_subnet() .with_sns_subnet() + .with_ii_subnet() .with_application_subnet() .build_async() .await; // Install the NNS canisters. { - let with_mainnet_nns_canisters = false; - install_nns_canisters(&pocket_ic, vec![], with_mainnet_nns_canisters, None, vec![]).await; + let mut nns_installer = NnsInstaller::default(); + nns_installer.with_tip_nns_canister_versions(); + nns_installer.with_cycles_ledger(); + nns_installer.install(&pocket_ic).await; } // Publish SNS Wasms to SNS-W. From 4c243383023e0230c629312c1897f4e5e42da460 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Thu, 30 Jan 2025 16:04:13 +0000 Subject: [PATCH 05/10] save --- Cargo.lock | 5 + rs/nervous_system/agent/BUILD.bazel | 2 + rs/nervous_system/agent/Cargo.toml | 2 + rs/nervous_system/agent/src/agent_impl.rs | 95 +++++++++- rs/nervous_system/agent/src/lib.rs | 22 ++- rs/nervous_system/agent/src/pocketic_impl.rs | 45 ++++- rs/nervous_system/agent/src/sns/governance.rs | 59 +++--- rs/nervous_system/agent/src/sns/root.rs | 66 ++++--- .../integration_tests/BUILD.bazel | 2 + .../integration_tests/Cargo.toml | 3 +- .../src/pocket_ic_helpers.rs | 22 ++- ...sns_controlled_canister_with_large_wasm.rs | 139 +++++++------- rs/sns/cli/BUILD.bazel | 1 + rs/sns/cli/Cargo.toml | 1 + rs/sns/cli/src/main.rs | 4 +- .../src/upgrade_sns_controlled_canister.rs | 170 +++++++++--------- 16 files changed, 403 insertions(+), 235 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d8516e738e..9054489b196 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10111,9 +10111,11 @@ dependencies = [ "ic-sns-root", "ic-sns-swap", "ic-sns-wasm", + "itertools 0.12.1", "pocket-ic", "registry-canister", "serde", + "serde_cbor", "tempfile", "thiserror 2.0.11", "tokio", @@ -10362,6 +10364,7 @@ dependencies = [ "ic-registry-routing-table", "ic-registry-subnet-type", "ic-registry-transport", + "ic-sns-cli", "ic-sns-governance", "ic-sns-governance-api", "ic-sns-init", @@ -10389,6 +10392,7 @@ dependencies = [ "rustc-hash 1.1.0", "serde", "tokio", + "url", "xrc-mock", ] @@ -12206,6 +12210,7 @@ dependencies = [ "tempfile", "thiserror 2.0.11", "tokio", + "url", ] [[package]] diff --git a/rs/nervous_system/agent/BUILD.bazel b/rs/nervous_system/agent/BUILD.bazel index 213f2899b3a..b4d3f3435b0 100644 --- a/rs/nervous_system/agent/BUILD.bazel +++ b/rs/nervous_system/agent/BUILD.bazel @@ -19,7 +19,9 @@ DEPENDENCIES = [ "@crate_index//:anyhow", "@crate_index//:candid", "@crate_index//:ic-agent", + "@crate_index//:itertools", "@crate_index//:serde", + "@crate_index//:serde_cbor", "@crate_index//:tempfile", "@crate_index//:thiserror", "@crate_index//:tokio", diff --git a/rs/nervous_system/agent/Cargo.toml b/rs/nervous_system/agent/Cargo.toml index 2cebb55f884..5a80705b0e0 100644 --- a/rs/nervous_system/agent/Cargo.toml +++ b/rs/nervous_system/agent/Cargo.toml @@ -23,7 +23,9 @@ pocket-ic = { path = "../../../packages/pocket-ic" } registry-canister = { path = "../../registry/canister" } ic-sns-root = { path = "../../sns/root" } ic-sns-swap = { path = "../../sns/swap" } +itertools = { workspace = true } serde = { workspace = true } +serde_cbor = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } diff --git a/rs/nervous_system/agent/src/agent_impl.rs b/rs/nervous_system/agent/src/agent_impl.rs index 586c7c2cbf5..53f2a86dce4 100644 --- a/rs/nervous_system/agent/src/agent_impl.rs +++ b/rs/nervous_system/agent/src/agent_impl.rs @@ -1,24 +1,31 @@ -use crate::Request; +use crate::CallCanisters; +use crate::{CanisterInfo, Request}; use candid::Principal; use ic_agent::Agent; +use itertools::{Either, Itertools}; +use serde_cbor::Value; +use std::collections::BTreeSet; use thiserror::Error; -use crate::CallCanisters; - #[derive(Error, Debug)] pub enum AgentCallError { + #[error("agent identity error: {0}")] + Identity(String), #[error("agent error: {0}")] Agent(#[from] ic_agent::AgentError), #[error("canister request could not be encoded: {0}")] CandidEncode(candid::Error), #[error("canister did not respond with the expected response type: {0}")] CandidDecode(candid::Error), + #[error("invalid canister controllers: {0}")] + CanisterControllers(String), } impl crate::sealed::Sealed for Agent {} impl CallCanisters for Agent { type Error = AgentCallError; + async fn call( &self, canister_id: impl Into + Send, @@ -50,4 +57,86 @@ impl CallCanisters for Agent { candid::decode_one(response.as_slice()).map_err(AgentCallError::CandidDecode)?; Ok(response) } + + async fn canister_info( + &self, + canister_id: impl Into + Send, + ) -> Result { + let canister_id = canister_id.into(); + + let read_state_result = self + .read_state_canister_info(canister_id, "module_hash") + .await; + + let module_hash = match read_state_result { + Ok(module_hash) => Some(module_hash), + Err(ic_agent::AgentError::LookupPathAbsent(_)) => { + // https://internetcomputer.org/docs/current/references/ic-interface-spec#state-tree-canister-information + None + } + Err(err) => { + return Err(Self::Error::Agent(err)); + } + }; + + let controllers_blob = self + .read_state_canister_info(canister_id, "controllers") + .await + .map_err(AgentCallError::Agent)?; + + let cbor: Value = serde_cbor::from_slice(&controllers_blob).map_err(|err| { + Self::Error::CanisterControllers(format!("Failed decoding CBOR data: {:?}", err)) + })?; + + let Value::Array(controllers) = cbor else { + return Err(Self::Error::CanisterControllers(format!( + "Expected controllers to be an array, but got {:?}", + cbor + ))); + }; + + let (controllers, errors): (Vec<_>, Vec<_>) = + controllers.into_iter().partition_map(|value| { + let Value::Bytes(bytes) = value else { + let err = format!( + "Expected canister controller to be of type bytes, got {:?}", + value + ); + return Either::Right(err); + }; + match Principal::try_from(&bytes) { + Err(err) => { + let err = + format!("Cannot interpret canister controller principal: {}", err); + Either::Right(err) + } + Ok(principal) => Either::Left(principal), + } + }); + + if !errors.is_empty() { + return Err(Self::Error::CanisterControllers(format!( + "\n - {}", + errors.join("\n - ") + ))); + } + + let unique_controllers = BTreeSet::from_iter(controllers.iter().copied()); + + if unique_controllers.len() != controllers.len() { + return Err(Self::Error::CanisterControllers(format!( + "Canister controllers have duplicates: {}", + controllers.into_iter().join(", ") + ))); + } + + Ok(CanisterInfo { + module_hash, + controllers: unique_controllers, + }) + } + + fn caller(&self) -> Result { + self.get_principal().map_err(Self::Error::Identity) + } } diff --git a/rs/nervous_system/agent/src/lib.rs b/rs/nervous_system/agent/src/lib.rs index 730b744782a..79433904a1e 100644 --- a/rs/nervous_system/agent/src/lib.rs +++ b/rs/nervous_system/agent/src/lib.rs @@ -1,3 +1,8 @@ +use candid::{CandidType, Principal}; +use serde::de::DeserializeOwned; +use std::collections::BTreeSet; +use std::fmt::Display; + pub mod agent_impl; pub mod management_canister; pub mod nns; @@ -5,10 +10,6 @@ mod null_request; pub mod pocketic_impl; pub mod sns; -use candid::{CandidType, Principal}; -use serde::de::DeserializeOwned; -use std::fmt::Display; - // This is used to "seal" the CallCanisters trait so that it cannot be implemented outside of this crate. // This is useful because it means we can modify the trait in the future without worrying about // breaking backwards compatibility with implementations outside of this crate. @@ -27,11 +28,24 @@ pub trait Request: Send { type Response: CandidType + DeserializeOwned; } +pub struct CanisterInfo { + pub module_hash: Option>, + pub controllers: BTreeSet, +} + pub trait CallCanisters: sealed::Sealed { type Error: Display + Send + std::error::Error + 'static; + + fn caller(&self) -> Result; + fn call( &self, canister_id: impl Into + Send, request: R, ) -> impl std::future::Future> + Send; + + fn canister_info( + &self, + canister_id: impl Into + Send, + ) -> impl std::future::Future> + Send; } diff --git a/rs/nervous_system/agent/src/pocketic_impl.rs b/rs/nervous_system/agent/src/pocketic_impl.rs index b1da05081a1..b936a06c644 100644 --- a/rs/nervous_system/agent/src/pocketic_impl.rs +++ b/rs/nervous_system/agent/src/pocketic_impl.rs @@ -1,10 +1,10 @@ use crate::Request; +use crate::{CallCanisters, CanisterInfo}; use candid::Principal; -use pocket_ic::nonblocking::PocketIc; +use pocket_ic::management_canister::DefiniteCanisterSettings; +use pocket_ic::{management_canister::CanisterStatusResult, nonblocking::PocketIc}; use thiserror::Error; -use crate::CallCanisters; - /// A wrapper around PocketIc that specifies a sender for the requests. /// The name is an analogy for `ic_agent::Agent`, since each `ic_agent::Agent` specifies a sender. pub struct PocketIcAgent<'a> { @@ -54,6 +54,32 @@ impl CallCanisters for PocketIcAgent<'_> { candid::decode_one(response.as_slice()).map_err(PocketIcCallError::CandidDecode) } + + async fn canister_info( + &self, + canister_id: impl Into + Send, + ) -> Result { + let canister_id = canister_id.into(); + + let CanisterStatusResult { + module_hash, + settings: DefiniteCanisterSettings { controllers, .. }, + .. + } = self + .pocket_ic + .canister_status(canister_id, Some(self.sender)) + .await + .map_err(PocketIcCallError::PocketIc)?; + + Ok(CanisterInfo { + module_hash, + controllers: controllers.into_iter().collect(), + }) + } + + fn caller(&self) -> Result { + Ok(self.sender) + } } impl CallCanisters for PocketIc { @@ -67,4 +93,17 @@ impl CallCanisters for PocketIc { .call(canister_id, request) .await } + + async fn canister_info( + &self, + canister_id: impl Into + Send, + ) -> Result { + PocketIcAgent::new(self, Principal::anonymous()) + .canister_info(canister_id) + .await + } + + fn caller(&self) -> Result { + Ok(Principal::anonymous()) + } } diff --git a/rs/nervous_system/agent/src/sns/governance.rs b/rs/nervous_system/agent/src/sns/governance.rs index 27ef610fad7..37e9165a24d 100644 --- a/rs/nervous_system/agent/src/sns/governance.rs +++ b/rs/nervous_system/agent/src/sns/governance.rs @@ -6,7 +6,7 @@ use ic_sns_governance_api::pb::v1::{ ManageNeuron, ManageNeuronResponse, NervousSystemParameters, NeuronId, Proposal, ProposalId, }; use serde::{Deserialize, Serialize}; -use std::error::Error; +use thiserror::Error; pub mod requests; @@ -15,14 +15,38 @@ pub struct GovernanceCanister { pub canister_id: PrincipalId, } -#[derive(Debug, thiserror::Error)] -pub enum SubmitProposalError { - #[error("Failed to call SNS Governance")] - CallGovernanceError(#[source] C), - #[error("SNS Governance returned an error")] - GovernanceError(#[source] GovernanceError), - #[error("SNS Governance did not confirm that the proposal was made: {0:?}")] - ProposalNotMade(ManageNeuronResponse), +pub struct SubmittedProposal { + pub proposal_id: ProposalId, +} + +#[derive(Debug, Error)] +pub enum ProposalSubmissionError { + #[error("SNS Governance returned an error: {0:?}")] + GovernanceError(GovernanceError), + #[error("SNS Governance did not confirm that the proposal was made.")] + NoConfirmation, +} + +impl TryFrom for SubmittedProposal { + type Error = ProposalSubmissionError; + + fn try_from(response: ManageNeuronResponse) -> Result { + let proposal_id = match response.command { + Some(manage_neuron_response::Command::MakeProposal( + manage_neuron_response::MakeProposalResponse { + proposal_id: Some(proposal_id), + }, + )) => proposal_id, + Some(manage_neuron_response::Command::Error(err)) => { + return Err(ProposalSubmissionError::GovernanceError(err)); + } + _ => { + return Err(ProposalSubmissionError::NoConfirmation); + } + }; + + Ok(Self { proposal_id }) + } } impl GovernanceCanister { @@ -73,27 +97,16 @@ impl GovernanceCanister { agent: &C, neuron_id: NeuronId, proposal: Proposal, - ) -> Result> { + ) -> Result { let response = self .manage_neuron( agent, neuron_id, manage_neuron::Command::MakeProposal(proposal), ) - .await - .map_err(SubmitProposalError::CallGovernanceError)?; + .await?; - match response.command { - Some(manage_neuron_response::Command::MakeProposal( - manage_neuron_response::MakeProposalResponse { - proposal_id: Some(proposal_id), - }, - )) => Ok(proposal_id), - Some(manage_neuron_response::Command::Error(e)) => { - Err(SubmitProposalError::GovernanceError(e)) - } - _ => Err(SubmitProposalError::ProposalNotMade(response)), - } + Ok(response) } } diff --git a/rs/nervous_system/agent/src/sns/root.rs b/rs/nervous_system/agent/src/sns/root.rs index 1c992bbe055..6555e1bcc3b 100644 --- a/rs/nervous_system/agent/src/sns/root.rs +++ b/rs/nervous_system/agent/src/sns/root.rs @@ -16,20 +16,41 @@ pub struct RootCanister { pub canister_id: PrincipalId, } -#[allow(clippy::large_enum_variant)] -#[derive(Debug, thiserror::Error)] -pub enum ListSnsCanistersError { - #[error("SNS root canister did not return canister IDs for all canisters - this should never happen")] - SnsRootDidNotReturnAllCanisterIds(ListSnsCanistersResponse), - #[error("Failed to call SNS root canister")] - CallFailed(#[from] E), -} - pub struct SnsCanisters { pub sns: Sns, pub dapps: Vec, } +impl TryFrom for SnsCanisters { + type Error = String; + + fn try_from(src: ListSnsCanistersResponse) -> Result { + let ListSnsCanistersResponse { + root: Some(sns_root_canister_id), + governance: Some(sns_governance_canister_id), + ledger: Some(sns_ledger_canister_id), + swap: Some(swap_canister_id), + index: Some(index_canister_id), + archives, + dapps, + } = src + else { + return Err(format!("Some SNS canisters were missing: {:?}", src)); + }; + + let sns = Sns { + root: RootCanister::new(sns_root_canister_id), + governance: GovernanceCanister::new(sns_governance_canister_id), + ledger: LedgerCanister::new(sns_ledger_canister_id), + swap: SwapCanister::new(swap_canister_id), + index: IndexCanister::new(index_canister_id), + archive: archives.into_iter().map(ArchiveCanister::new).collect(), + }; + + Ok(Self { sns, dapps }) + } +} + impl RootCanister { pub fn new(canister_id: impl Into) -> Self { let canister_id = canister_id.into(); @@ -53,34 +74,11 @@ impl RootCanister { pub async fn list_sns_canisters( &self, agent: &C, - ) -> Result> { + ) -> Result { let response = agent .call(self.canister_id, ListSnsCanistersRequest {}) .await?; - let ListSnsCanistersResponse { - root: Some(sns_root_canister_id), - governance: Some(sns_governance_canister_id), - ledger: Some(sns_ledger_canister_id), - swap: Some(swap_canister_id), - index: Some(index_canister_id), - archives, - dapps, - } = response - else { - return Err(ListSnsCanistersError::SnsRootDidNotReturnAllCanisterIds( - response, - )); - }; - - let sns = Sns { - root: RootCanister::new(sns_root_canister_id), - governance: GovernanceCanister::new(sns_governance_canister_id), - ledger: LedgerCanister::new(sns_ledger_canister_id), - swap: SwapCanister::new(swap_canister_id), - index: IndexCanister::new(index_canister_id), - archive: archives.into_iter().map(ArchiveCanister::new).collect(), - }; - Ok(SnsCanisters { sns, dapps }) + Ok(response) } } diff --git a/rs/nervous_system/integration_tests/BUILD.bazel b/rs/nervous_system/integration_tests/BUILD.bazel index be5af458a6c..c4285b3ac4d 100644 --- a/rs/nervous_system/integration_tests/BUILD.bazel +++ b/rs/nervous_system/integration_tests/BUILD.bazel @@ -21,6 +21,7 @@ BASE_DEPENDENCIES = [ "//rs/nns/sns-wasm", "//rs/sns/governance", "//rs/sns/governance/api", + "//rs/sns/cli", "//rs/sns/init", "//rs/sns/root", "//rs/sns/swap", @@ -35,6 +36,7 @@ BASE_DEPENDENCIES = [ "@crate_index//:prost", "@crate_index//:rust_decimal", "@crate_index//:tokio", + "@crate_index//:url", ] + select({ "@rules_rust//rust/platform:wasm32-unknown-unknown": [], "//conditions:default": [ diff --git a/rs/nervous_system/integration_tests/Cargo.toml b/rs/nervous_system/integration_tests/Cargo.toml index 75529f7c0ba..0408b98765e 100644 --- a/rs/nervous_system/integration_tests/Cargo.toml +++ b/rs/nervous_system/integration_tests/Cargo.toml @@ -25,6 +25,7 @@ ic-nns-governance = { path = "../../nns/governance" } ic-nns-governance-api = { path = "../../nns/governance/api" } ic-sns-governance = { path = "../../sns/governance" } ic-sns-governance-api = { path = "../../sns/governance/api" } +ic-sns-cli = { path = "../../sns/cli" } ic-sns-root = { path = "../../sns/root" } ic-sns-swap = { path = "../../sns/swap" } icp-ledger = { path = "../../ledger_suite/icp" } @@ -37,7 +38,7 @@ prost = { workspace = true } rust_decimal = "1.36.0" rust_decimal_macros = "1.36.0" tokio = { workspace = true } - +url = { workspace = true } # Dependencies required to compile the tests. [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index 06c89d43e83..ad473a1c558 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -1421,6 +1421,7 @@ pub mod nns { pub mod sns { use super::*; + use ic_nervous_system_agent::sns::root::SnsCanisters; #[derive(Clone, Debug, PartialEq)] pub enum SnsUpgradeError { @@ -1442,7 +1443,10 @@ pub mod sns { expected_type_to_change: SnsCanisterType, ) -> Result<(), SnsUpgradeError> { // Ensure that we are working with knowledge of the latest archive canisters (if there are any). - let sns = sns.root.list_sns_canisters(pocket_ic).await.unwrap().sns; + let sns = { + let response = sns.root.list_sns_canisters(pocket_ic).await.unwrap(); + SnsCanisters::try_from(response).unwrap().sns + }; let (canister_id, controller_id) = match expected_type_to_change { SnsCanisterType::Root => (sns.root.canister_id, sns.governance.canister_id), @@ -1551,7 +1555,9 @@ pub mod sns { use super::*; use assert_matches::assert_matches; use ic_crypto_sha2::Sha256; - use ic_nervous_system_agent::sns::governance::{GovernanceCanister, SubmitProposalError}; + use ic_nervous_system_agent::sns::governance::{ + GovernanceCanister, ProposalSubmissionError, SubmittedProposal, + }; use ic_sns_governance_api::pb::v1::{ get_neuron_response, neuron::DissolveState, @@ -1641,11 +1647,15 @@ pub mod sns { ) -> Result { let agent = PocketIcAgent::new(pocket_ic, sender); let governance = GovernanceCanister::new(canister_id); - let proposal_id = governance + + let response = governance .submit_proposal(&agent, neuron_id, proposal) .await - .map_err(|err| match err { - SubmitProposalError::GovernanceError(e) => e, + .unwrap(); + + let SubmittedProposal { proposal_id } = + SubmittedProposal::try_from(response).map_err(|err| match err { + ProposalSubmissionError::GovernanceError(e) => e, e => panic!("Unexpected error: {e}"), })?; @@ -1653,7 +1663,7 @@ pub mod sns { } /// This function assumes that the proposal submission succeeded (and panics otherwise). - async fn wait_for_proposal_execution( + pub async fn wait_for_proposal_execution( pocket_ic: &PocketIc, canister_id: PrincipalId, proposal_id: sns_pb::ProposalId, diff --git a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs index 789ed6a694e..a55365906f5 100644 --- a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs +++ b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs @@ -1,8 +1,10 @@ -use std::collections::BTreeSet; - use candid::Principal; use canister_test::Wasm; -use ic_management_canister_types::CanisterInstallMode; +use ic_nervous_system_agent::nns::registry::get_subnet_for_canister; +use ic_nervous_system_agent::pocketic_impl::PocketIcAgent; +use ic_nervous_system_integration_tests::pocket_ic_helpers::sns::governance::{ + find_neuron_with_majority_voting_power, wait_for_proposal_execution, +}; use ic_nervous_system_integration_tests::pocket_ic_helpers::{ await_with_timeout, install_canister_on_subnet, nns, sns, NnsInstaller, }; @@ -12,38 +14,32 @@ use ic_nervous_system_integration_tests::{ }; use ic_nns_constants::ROOT_CANISTER_ID; use ic_nns_test_utils::common::modify_wasm_bytes; -use ic_sns_governance_api::pb::v1::{ChunkedCanisterWasm, UpgradeSnsControlledCanister}; +use ic_sns_cli::neuron_id_to_candid_subaccount::ParsedSnsNeuron; +use ic_sns_cli::upgrade_sns_controlled_canister::{ + self, UpgradeSnsControlledCanisterArgs, UpgradeSnsControlledCanisterInfo, +}; use ic_sns_swap::pb::v1::Lifecycle; use pocket_ic::nonblocking::PocketIc; use pocket_ic::PocketIcBuilder; +use std::collections::BTreeSet; +use std::path::PathBuf; +use url::Url; const MIN_INSTALL_CHUNKED_CODE_TIME_SECONDS: u64 = 20; const MAX_INSTALL_CHUNKED_CODE_TIME_SECONDS: u64 = 5 * 60; const CHUNK_SIZE: usize = 1024 * 1024; // 1 MiB -// TODO: Figure out how to best support uploading chunks into the target itself, which has -// SNS Root as the controller but not SNS Governance. -// -// #[tokio::test] -// async fn test_store_same_as_target() { -// let store_same_as_target = true; -// run_test(store_same_as_target).await; -// } - -#[tokio::test] -async fn test_store_different_from_target() { - let store_same_as_target = false; - run_test(store_same_as_target).await; -} - -fn very_large_wasm_bytes() -> Vec { +fn very_large_wasm_path() -> PathBuf { let image_classification_canister_wasm_path = std::env::var("IMAGE_CLASSIFICATION_CANISTER_WASM_PATH") .expect("Please ensure that this Bazel test target correctly specifies env and data."); - let wasm_path = std::path::PathBuf::from(image_classification_canister_wasm_path); + PathBuf::from(image_classification_canister_wasm_path) +} +fn very_large_wasm_bytes() -> Vec { + let wasm_path = very_large_wasm_path(); std::fs::read(&wasm_path).expect("Failed to read WASM file") } @@ -102,7 +98,8 @@ async fn upload_wasm_as_chunks( uploaded_chunk_hashes } -async fn run_test(store_same_as_target: bool) { +#[tokio::test] +async fn test_store_different_from_target() { // 1. Prepare the world let pocket_ic = PocketIcBuilder::new() .with_nns_subnet() @@ -129,10 +126,24 @@ async fn run_test(store_same_as_target: bool) { }; // Install a dapp canister. - let original_wasm = Wasm::from_bytes(very_large_wasm_bytes()); + let original_wasm = { + // Modify the Wasm upfront, as we then upgrade to the (unmodified) Wasm on the file system. + let wasm_bytes = very_large_wasm_bytes(); + let wasm_bytes = modify_wasm_bytes(&wasm_bytes, 42); + Wasm::from_bytes(&wasm_bytes[..]) + }; let original_wasm_hash = original_wasm.sha256_hash(); let app_subnet = pocket_ic.topology().await.get_app_subnets()[0]; + let ii_subnet = pocket_ic.topology().await.get_ii().unwrap(); + assert_ne!(app_subnet, ii_subnet); + + let root_subnet = get_subnet_for_canister(&pocket_ic, ROOT_CANISTER_ID.into()) + .await + .unwrap(); + + println!("root_subnet = {}", root_subnet); + panic!("boo"); let target_canister_id = install_canister_on_subnet( &pocket_ic, @@ -174,59 +185,47 @@ async fn run_test(store_same_as_target: bool) { sns }; - let store_canister_id = if store_same_as_target { - target_canister_id - } else { - install_canister_on_subnet( - &pocket_ic, - app_subnet, - vec![], - None, - vec![sns.root.canister_id], + // Get an ID of an SNS neuron that can submit proposals. We rely on the fact that this + // neuron either holds the majority of the voting power or the follow graph is set up + // s.t. when this neuron submits a proposal, that proposal gets through without the need + // for any voting. + let (sns_neuron_id, _) = + find_neuron_with_majority_voting_power(&pocket_ic, sns.governance.canister_id) + .await + .expect("cannot find SNS neuron with dissolve delay over 6 months."); + + let cli_arg = UpgradeSnsControlledCanisterArgs { + sns_neuron_id: Some(ParsedSnsNeuron(sns_neuron_id)), + target_canister_id, + wasm_path: very_large_wasm_path().clone(), + candid_arg: None, + proposal_url: Url::try_from( + "https://github.com/dfinity/examples/tree/master/rust/image-classification", ) - .await + .unwrap(), + summary: "Upgrade Image Classification canister.".to_string(), }; - let new_wasm = { - let new_wasm_bytes = modify_wasm_bytes(&original_wasm.bytes(), 42); - Wasm::from_bytes(&new_wasm_bytes[..]) + // 2. Submit the upgrade proposal. + let pocket_ic_agent = PocketIcAgent { + pocket_ic: &pocket_ic, + sender: sns.root.canister_id.into(), }; - let new_wasm_hash = new_wasm.sha256_hash(); - - // Smoke test - assert_ne!(new_wasm_hash, original_wasm_hash); + let UpgradeSnsControlledCanisterInfo { + wasm_module_hash, + proposal_id, + } = upgrade_sns_controlled_canister::exec(cli_arg, &pocket_ic_agent) + .await + .unwrap(); - // WASM with 15_843_866 bytes (`image-classification.wasm.gz`) is split into 1 MiB chunks. - let num_chunks_expected = 16; + let proposal_id = proposal_id.unwrap(); - let chunk_hashes_list = upload_wasm_as_chunks( - &pocket_ic, - sns.root.canister_id.into(), - store_canister_id.into(), - new_wasm, - num_chunks_expected, - ) - .await; - - // 2. Run code under test. - sns::governance::propose_to_upgrade_sns_controlled_canister_and_wait( - &pocket_ic, - sns.governance.canister_id, - UpgradeSnsControlledCanister { - canister_id: Some(target_canister_id.get()), - new_canister_wasm: vec![], - canister_upgrade_arg: None, - mode: Some(CanisterInstallMode::Upgrade as i32), - chunked_canister_wasm: Some(ChunkedCanisterWasm { - wasm_module_hash: new_wasm_hash.clone().to_vec(), - store_canister_id: Some(store_canister_id.get()), - chunk_hashes_list, - }), - }, - ) - .await; + // 3. Await proposal execution. + wait_for_proposal_execution(&pocket_ic, sns.governance.canister_id, proposal_id) + .await + .unwrap(); - // 3. Inspect the resulting state. + // 4. Inspect the resulting state. await_with_timeout( &pocket_ic, MIN_INSTALL_CHUNKED_CODE_TIME_SECONDS..MAX_INSTALL_CHUNKED_CODE_TIME_SECONDS, @@ -238,7 +237,7 @@ async fn run_test(store_same_as_target: bool) { .expect("canister status must be available") .module_hash }, - &Some(new_wasm_hash.to_vec()), + &Some(wasm_module_hash), ) .await .unwrap(); diff --git a/rs/sns/cli/BUILD.bazel b/rs/sns/cli/BUILD.bazel index a9e61b92a96..e615c0c8c9b 100644 --- a/rs/sns/cli/BUILD.bazel +++ b/rs/sns/cli/BUILD.bazel @@ -41,6 +41,7 @@ DEPENDENCIES = [ "@crate_index//:tempfile", "@crate_index//:thiserror", "@crate_index//:tokio", + "@crate_index//:url", ] MACRO_DEPENDENCIES = [] diff --git a/rs/sns/cli/Cargo.toml b/rs/sns/cli/Cargo.toml index a9f00aed991..ed4d14d76b6 100644 --- a/rs/sns/cli/Cargo.toml +++ b/rs/sns/cli/Cargo.toml @@ -50,6 +50,7 @@ serde_yaml = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } +url = { workspace = true } [dev-dependencies] lazy_static = { workspace = true } diff --git a/rs/sns/cli/src/main.rs b/rs/sns/cli/src/main.rs index 5a2ff962f9f..8dd5dbd0d99 100644 --- a/rs/sns/cli/src/main.rs +++ b/rs/sns/cli/src/main.rs @@ -30,7 +30,9 @@ async fn main() -> Result<()> { SubCommand::List(args) => list::exec(args, &agent).await, SubCommand::Health(args) => health::exec(args, &agent).await, SubCommand::UpgradeSnsControlledCanister(args) => { - upgrade_sns_controlled_canister::exec(args, &agent).await + upgrade_sns_controlled_canister::exec(args, &agent) + .await + .map_err(|err| anyhow::anyhow!(err)) } } } diff --git a/rs/sns/cli/src/upgrade_sns_controlled_canister.rs b/rs/sns/cli/src/upgrade_sns_controlled_canister.rs index 624d66b39ca..111406c1007 100644 --- a/rs/sns/cli/src/upgrade_sns_controlled_canister.rs +++ b/rs/sns/cli/src/upgrade_sns_controlled_canister.rs @@ -1,5 +1,5 @@ use crate::neuron_id_to_candid_subaccount::ParsedSnsNeuron; -use anyhow::{bail, Context, Result}; +use anyhow::Result; use candid::{CandidType, Encode, Nat, Principal}; use candid_utils::{ printing, @@ -7,28 +7,27 @@ use candid_utils::{ }; use clap::Parser; use cycles_minting_canister::{CanisterSettingsArgs, CreateCanister, SubnetSelection}; -use ic_agent::{export::reqwest::Url, Agent}; use ic_base_types::{CanisterId, PrincipalId}; use ic_management_canister_types::{BoundedVec, CanisterInstallMode}; use ic_nervous_system_agent::{ management_canister, nns, - sns::{self, root::SnsCanisters}, - CallCanisters, Request, + sns::{self, governance::SubmittedProposal, root::SnsCanisters}, + CallCanisters, CanisterInfo, Request, }; use ic_nns_constants::CYCLES_LEDGER_CANISTER_ID; use ic_sns_governance_api::pb::v1::{ - proposal::Action, ChunkedCanisterWasm, Proposal, UpgradeSnsControlledCanister, + proposal::Action, ChunkedCanisterWasm, Proposal, ProposalId, UpgradeSnsControlledCanister, }; use ic_wasm::{metadata, utils::parse_wasm}; use itertools::{Either, Itertools}; use serde::Deserialize; -use serde_cbor::Value; use std::{ collections::BTreeSet, fs::File, io::{Read, Write}, - path::PathBuf, + path::{Path, PathBuf}, }; +use thiserror::Error; const RAW_WASM_HEADER: [u8; 4] = [0, 0x61, 0x73, 0x6d]; const GZIPPED_WASM_HEADER: [u8; 3] = [0x1f, 0x8b, 0x08]; @@ -46,27 +45,27 @@ pub struct UpgradeSnsControlledCanisterArgs { /// /// If not specified, the proposal payload will be printed at the end. #[clap(long)] - sns_neuron_id: Option, + pub sns_neuron_id: Option, /// ID of the target canister to be upgraded. #[clap(long)] - target_canister_id: CanisterId, + pub target_canister_id: CanisterId, /// Path to a ICP WASM module file (may be gzipped). #[clap(long)] - wasm_path: PathBuf, + pub wasm_path: PathBuf, /// Upgrade argument for the Candid service. #[clap(long)] - candid_arg: Option, + pub candid_arg: Option, /// URL (starting with https://) of a web page with a public announcement of this upgrade. #[clap(long)] - proposal_url: Url, + pub proposal_url: url::Url, /// Human-readable text explaining why this upgrade is being done (may be markdown). #[clap(long)] - summary: String, + pub summary: String, } pub struct Wasm { @@ -84,8 +83,8 @@ impl Wasm { self.module_hash } - pub fn path(&self) -> String { - self.path.display().to_string() + pub fn path(&self) -> &Path { + &self.path } } @@ -179,8 +178,8 @@ pub fn validate_candid_arg_for_wasm(wasm: &Wasm, args: Option) -> Result ⚠️ Please consider adding it as follows:\n\ cargo install ic-wasm\n\ ic-wasm -o augmented-{} {} metadata -v public candid:service -f service.did", - wasm.path(), - wasm.path(), + wasm.path().display(), + wasm.path().display(), ); // Proceed with whatever argument the user has specified without validation. @@ -193,68 +192,11 @@ pub fn validate_candid_arg_for_wasm(wasm: &Wasm, args: Option) -> Result Ok(canister_arg) } -/// Checks if `canister_id` is an ID of a canister that exists and has some Wasm code installed. -/// -/// In the Ok result, returns a tuple with the following components: -/// 1. Set of controllers. -/// 2. Module hash of the installed code. -/// -/// See https://internetcomputer.org/docs/current/references/ic-interface-spec#state-tree-canister-information -/// -/// This function is analogous to `dfx canister info`. -pub async fn fetch_canister_info( - agent: &Agent, - canister_id: CanisterId, -) -> Result<(BTreeSet, Vec)> { - let module_hash = agent - .read_state_canister_info(canister_id.get().0, "module_hash") - .await - .context("Cannot read target canister's module hash.")?; - - let controllers_blob = agent - .read_state_canister_info(canister_id.get().0, "controllers") - .await - .context("Cannot read canister controllers.")?; - - let cbor: Value = serde_cbor::from_slice(&controllers_blob) - .expect("Invalid cbor data for controller controllers."); - - let Value::Array(controllers) = cbor else { - bail!("Expected controllers to be an array, but got {cbor:?}"); - }; - - let (controllers, errors): (BTreeSet<_>, Vec<_>) = - controllers.into_iter().partition_map(|value| { - let Value::Bytes(bytes) = value else { - let err = - format!("Expected canister controller to be of type bytes, got {value:?}",); - return Either::Right(err); - }; - match Principal::try_from(&bytes) { - Err(err) => { - let err = format!("Cannot interpret canister controller principal: {err}"); - Either::Right(err) - } - Ok(principal) => Either::Left(PrincipalId(principal)), - } - }); - - if !errors.is_empty() { - let err = format!( - "Problems with canister controllers:\n - {}", - errors.join("\n - ") - ); - bail!(err); - } - - Ok((controllers, module_hash)) -} - /// Attempts to create an empty canister on the same subnet as `next_to`. /// /// Returns the ID of the newly created canister in the Ok result. -pub async fn create_canister_next_to( - agent: &Agent, +pub async fn create_canister_next_to( + agent: &C, next_to: CanisterId, controllers: Vec, cycles_amount: u128, @@ -294,9 +236,29 @@ pub async fn create_canister_next_to( CanisterId::try_from_principal_id(canister_id).map_err(|err| anyhow::anyhow!(err)) } -pub async fn exec(args: UpgradeSnsControlledCanisterArgs, agent: &Agent) -> Result<()> { - // Prepare. +#[derive(Debug, Error)] +pub enum UpgradeSnsControlledCanisterError { + #[error("agent interaction failed: {0}")] + Agent(AgentError), + #[error("observed bad state: {0}")] + Client(String), +} + +impl From for UpgradeSnsControlledCanisterError { + fn from(err: E) -> Self { + Self::Agent(err) + } +} +pub struct UpgradeSnsControlledCanisterInfo { + pub wasm_module_hash: Vec, + pub proposal_id: Option, +} + +pub async fn exec( + args: UpgradeSnsControlledCanisterArgs, + agent: &C, +) -> Result> { let UpgradeSnsControlledCanisterArgs { sns_neuron_id, target_canister_id, @@ -306,14 +268,27 @@ pub async fn exec(args: UpgradeSnsControlledCanisterArgs, agent: &Agent) -> Resu summary, } = args; - let caller_principal = PrincipalId(agent.get_principal().map_err(|err| anyhow::anyhow!(err))?); + let caller_principal = PrincipalId(agent.caller()?); print!("Getting target canister info ... "); std::io::stdout().flush().unwrap(); - let (target_controllers, current_module_hash) = - fetch_canister_info(agent, target_canister_id).await?; + let CanisterInfo { + module_hash: Some(current_module_hash), + controllers: target_controllers, + } = agent.canister_info(target_canister_id).await? + else { + return Err(UpgradeSnsControlledCanisterError::Client(format!( + "Before upgrading, please install the target, e.g.: {}", + suggested_install_command(&wasm_path, &candid_arg) + ))); + }; println!("✔️"); + let target_controllers = target_controllers + .into_iter() + .map(PrincipalId) + .collect::>(); + print!("Finding the SNS controlling this target canister ... "); std::io::stdout().flush().unwrap(); let sns = { @@ -362,15 +337,17 @@ pub async fn exec(args: UpgradeSnsControlledCanisterArgs, agent: &Agent) -> Resu let root_canister = sns::root::RootCanister { canister_id }; - let SnsCanisters { sns, dapps } = root_canister.list_sns_canisters(agent).await?; + let response = root_canister.list_sns_canisters(agent).await?; + let SnsCanisters { sns, dapps } = SnsCanisters::try_from(response) + .map_err(UpgradeSnsControlledCanisterError::Client)?; // Check that the target is indeed controlled by this SNS. if !BTreeSet::from_iter(&dapps[..]).contains(&target_canister_id.get()) { - bail!( + return Err(UpgradeSnsControlledCanisterError::Client(format!( "{} is not one of the canisters controlled by the SNS with Root canister {}", target_canister_id.get(), root_canister.canister_id, - ); + ))); } Some(sns) @@ -473,10 +450,12 @@ pub async fn exec(args: UpgradeSnsControlledCanisterArgs, agent: &Agent) -> Resu )), }; - if let Some(sns_neuron_id) = sns_neuron_id { - let proposal_id = sns_governance + let proposal_id = if let Some(sns_neuron_id) = sns_neuron_id { + let manage_neuron_response = sns_governance .submit_proposal(agent, sns_neuron_id.0, proposal) .await?; + let SubmittedProposal { proposal_id } = SubmittedProposal::try_from(manage_neuron_response) + .map_err(|err| UpgradeSnsControlledCanisterError::Client(err.to_string()))?; println!("✔️"); let proposal_url = format!( @@ -487,13 +466,20 @@ pub async fn exec(args: UpgradeSnsControlledCanisterArgs, agent: &Agent) -> Resu "Successfully proposed to upgrade SNS-controlled canister, see details here:\n\ {proposal_url}", ); + + Some(proposal_id) } else { println!("✔️"); let proposal_str = printing::pretty(&proposal).unwrap(); println!("{proposal_str}"); - } - Ok(()) + None + }; + + Ok(UpgradeSnsControlledCanisterInfo { + wasm_module_hash: wasm.module_hash().to_vec(), + proposal_id, + }) } pub type BlockIndex = Nat; @@ -597,11 +583,15 @@ fn format_full_hash(hash: &[u8]) -> String { .join("") } -fn suggested_install_command(wasm_path_str: &str, candid_arg: &Option) -> String { +fn suggested_install_command(wasm_path_str: &Path, candid_arg: &Option) -> String { let arg_suggestion = if let Some(candid_arg) = candid_arg { format!(" --argument '{}'", candid_arg) } else { "".to_string() }; - format!("dfx canister install --mode auto --wasm {wasm_path_str} CANISTER_NAME{arg_suggestion}") + format!( + "dfx canister install --mode auto --wasm {} CANISTER_NAME{}", + wasm_path_str.display().to_string(), + arg_suggestion, + ) } From 5c97ccff650fcac4380d105068307e1573c25246 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Fri, 31 Jan 2025 12:12:16 +0000 Subject: [PATCH 06/10] Finally, it works --- Cargo.lock | 3 + packages/pocket-ic/src/nonblocking.rs | 2 +- rs/nervous_system/agent/src/pocketic_impl.rs | 78 ++++- .../integration_tests/BUILD.bazel | 4 + .../integration_tests/Cargo.toml | 3 + .../src/pocket_ic_helpers.rs | 272 +++++++++++++++--- ...sns_controlled_canister_with_large_wasm.rs | 116 +++----- rs/nns/test_utils/src/common.rs | 5 + .../src/upgrade_sns_controlled_canister.rs | 2 +- 9 files changed, 355 insertions(+), 130 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9054489b196..18ef354fbe7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10341,6 +10341,7 @@ dependencies = [ "ic-icrc1", "ic-icrc1-index-ng", "ic-icrc1-tokens-u64", + "ic-interfaces-registry", "ic-ledger-core", "ic-management-canister-types", "ic-nervous-system-agent", @@ -10361,6 +10362,7 @@ dependencies = [ "ic-nns-test-utils-macros", "ic-protobuf", "ic-registry-keys", + "ic-registry-proto-data-provider", "ic-registry-routing-table", "ic-registry-subnet-type", "ic-registry-transport", @@ -10391,6 +10393,7 @@ dependencies = [ "rust_decimal_macros", "rustc-hash 1.1.0", "serde", + "tempfile", "tokio", "url", "xrc-mock", diff --git a/packages/pocket-ic/src/nonblocking.rs b/packages/pocket-ic/src/nonblocking.rs index 763574b4651..9cf76974d94 100644 --- a/packages/pocket-ic/src/nonblocking.rs +++ b/packages/pocket-ic/src/nonblocking.rs @@ -1529,7 +1529,7 @@ impl PocketIc { result.into() } - pub(crate) async fn update_call_with_effective_principal( + pub async fn update_call_with_effective_principal( &self, canister_id: CanisterId, effective_principal: RawEffectivePrincipal, diff --git a/rs/nervous_system/agent/src/pocketic_impl.rs b/rs/nervous_system/agent/src/pocketic_impl.rs index b936a06c644..a021b8075ca 100644 --- a/rs/nervous_system/agent/src/pocketic_impl.rs +++ b/rs/nervous_system/agent/src/pocketic_impl.rs @@ -1,6 +1,8 @@ +use crate::management_canister::requests::{StoredChunksArgs, UploadChunkArgs}; use crate::Request; use crate::{CallCanisters, CanisterInfo}; use candid::Principal; +use pocket_ic::common::rest::RawEffectivePrincipal; use pocket_ic::management_canister::DefiniteCanisterSettings; use pocket_ic::{management_canister::CanisterStatusResult, nonblocking::PocketIc}; use thiserror::Error; @@ -23,6 +25,10 @@ impl<'a> PocketIcAgent<'a> { pub enum PocketIcCallError { #[error("pocket_ic error: {0}")] PocketIc(pocket_ic::RejectResponse), + #[error("retrieving canister info is not implemented for canister without controllers, such as this one.")] + BlackHole, + #[error("pocket_ic failed to find the subnet of canister {0}")] + CanisterSubnetNotFound(Principal), #[error("canister request could not be encoded: {0}")] CandidEncode(candid::Error), #[error("canister did not respond with the expected response type: {0}")] @@ -32,6 +38,63 @@ pub enum PocketIcCallError { impl crate::sealed::Sealed for PocketIc {} impl crate::sealed::Sealed for PocketIcAgent<'_> {} +impl PocketIcAgent<'_> { + async fn get_subnet( + &self, + canister_id: Principal, + ) -> Result { + let Some(subnet_id) = self.pocket_ic.get_subnet(canister_id.into()).await else { + return Err(PocketIcCallError::CanisterSubnetNotFound(canister_id)); + }; + + Ok(RawEffectivePrincipal::CanisterId( + subnet_id.as_slice().to_vec(), + )) + } + + async fn management_canister_call( + &self, + request: R, + ) -> Result { + let payload = request.payload().map_err(PocketIcCallError::CandidEncode)?; + + let canister_id = match request.method() { + "upload_chunk" => { + candid::decode_one::(payload.as_slice()) + .map_err(PocketIcCallError::CandidDecode)? + .canister_id + } + "stored_chunks" => { + candid::decode_one::(payload.as_slice()) + .map_err(PocketIcCallError::CandidDecode)? + .canister_id + } + mathod_name => { + unimplemented!( + "PocketIcAgent does not currently implement IC00.{}", + mathod_name + ); + } + }; + + let effective_principal = self.get_subnet(canister_id).await?; + + let response = self + .pocket_ic + .update_call_with_effective_principal( + Principal::management_canister(), + effective_principal, + self.sender, + request.method(), + payload, + ) + .await + .map_err(PocketIcCallError::PocketIc)?; + + candid::decode_one(response.as_slice()).map_err(PocketIcCallError::CandidDecode) + } +} + impl CallCanisters for PocketIcAgent<'_> { type Error = PocketIcCallError; async fn call( @@ -40,6 +103,13 @@ impl CallCanisters for PocketIcAgent<'_> { request: R, ) -> Result { let canister_id = canister_id.into(); + + // Currently, PocketIc does not route calls to aaaaa-aa, saying that it does not belong + // to any subnet. So we need to explicitly treat this case. + if canister_id == Principal::management_canister() { + return self.management_canister_call(request).await; + } + let request_bytes = request.payload().map_err(PocketIcCallError::CandidEncode)?; let response = if request.update() { self.pocket_ic @@ -61,13 +131,19 @@ impl CallCanisters for PocketIcAgent<'_> { ) -> Result { let canister_id = canister_id.into(); + let controllers = self.pocket_ic.get_controllers(canister_id).await; + + let Some(controller) = controllers.into_iter().last() else { + return Err(Self::Error::BlackHole); + }; + let CanisterStatusResult { module_hash, settings: DefiniteCanisterSettings { controllers, .. }, .. } = self .pocket_ic - .canister_status(canister_id, Some(self.sender)) + .canister_status(canister_id, Some(controller)) .await .map_err(PocketIcCallError::PocketIc)?; diff --git a/rs/nervous_system/integration_tests/BUILD.bazel b/rs/nervous_system/integration_tests/BUILD.bazel index c4285b3ac4d..2ce0239df0e 100644 --- a/rs/nervous_system/integration_tests/BUILD.bazel +++ b/rs/nervous_system/integration_tests/BUILD.bazel @@ -6,6 +6,7 @@ package(default_visibility = ["//visibility:public"]) # See rs/nervous_system/feature_test.md BASE_DEPENDENCIES = [ # Keep sorted. + "//rs/interfaces/registry", "//rs/ledger_suite/common/ledger_core", "//rs/ledger_suite/icp:icp_ledger", "//rs/ledger_suite/icrc1", @@ -16,9 +17,11 @@ BASE_DEPENDENCIES = [ "//rs/nervous_system/common", "//rs/nervous_system/proto", "//rs/nervous_system/root", + "//rs/nns/cmc", "//rs/nns/common", "//rs/nns/governance/api", "//rs/nns/sns-wasm", + "//rs/registry/proto_data_provider", "//rs/sns/governance", "//rs/sns/governance/api", "//rs/sns/cli", @@ -35,6 +38,7 @@ BASE_DEPENDENCIES = [ "@crate_index//:lazy_static", "@crate_index//:prost", "@crate_index//:rust_decimal", + "@crate_index//:tempfile", "@crate_index//:tokio", "@crate_index//:url", ] + select({ diff --git a/rs/nervous_system/integration_tests/Cargo.toml b/rs/nervous_system/integration_tests/Cargo.toml index 0408b98765e..c6147a5066b 100644 --- a/rs/nervous_system/integration_tests/Cargo.toml +++ b/rs/nervous_system/integration_tests/Cargo.toml @@ -13,6 +13,7 @@ candid = { workspace = true } cycles-minting-canister = { path = "../../nns/cmc" } futures = { workspace = true } ic-base-types = { path = "../../types/base_types" } +ic-interfaces-registry = { path = "../../interfaces/registry" } ic-ledger-core = { path = "../../ledger_suite/common/ledger_core" } ic-nervous-system-agent = { path = "../agent" } ic-nervous-system-clients = { path = "../clients" } @@ -23,6 +24,7 @@ ic-nervous-system-runtime = { path = "../runtime" } ic-nns-common = { path = "../../nns/common" } ic-nns-governance = { path = "../../nns/governance" } ic-nns-governance-api = { path = "../../nns/governance/api" } +ic-registry-proto-data-provider = { path = "../../registry/proto_data_provider" } ic-sns-governance = { path = "../../sns/governance" } ic-sns-governance-api = { path = "../../sns/governance/api" } ic-sns-cli = { path = "../../sns/cli" } @@ -37,6 +39,7 @@ pocket-ic = { path = "../../../packages/pocket-ic" } prost = { workspace = true } rust_decimal = "1.36.0" rust_decimal_macros = "1.36.0" +tempfile = { workspace = true } tokio = { workspace = true } url = { workspace = true } diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index ad473a1c558..41e0e4190d0 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -2,6 +2,7 @@ use candid::{Decode, Encode, Nat, Principal}; use canister_test::Wasm; use futures::{stream, StreamExt}; use ic_base_types::{CanisterId, PrincipalId, SubnetId}; +use ic_interfaces_registry::{RegistryDataProvider, ZERO_REGISTRY_VERSION}; use ic_ledger_core::Tokens; use ic_nervous_system_agent::{ pocketic_impl::{PocketIcAgent, PocketIcCallError}, @@ -12,8 +13,9 @@ use ic_nervous_system_common::{E8, ONE_DAY_SECONDS}; use ic_nervous_system_common_test_keys::{TEST_NEURON_1_ID, TEST_NEURON_1_OWNER_PRINCIPAL}; use ic_nns_common::pb::v1::{NeuronId, ProposalId}; use ic_nns_constants::{ - self, ALL_NNS_CANISTER_IDS, GOVERNANCE_CANISTER_ID, LEDGER_CANISTER_ID, LIFELINE_CANISTER_ID, - REGISTRY_CANISTER_ID, ROOT_CANISTER_ID, SNS_WASM_CANISTER_ID, + self, ALL_NNS_CANISTER_IDS, CYCLES_MINTING_CANISTER_ID, GOVERNANCE_CANISTER_ID, + LEDGER_CANISTER_ID, LIFELINE_CANISTER_ID, REGISTRY_CANISTER_ID, ROOT_CANISTER_ID, + SNS_WASM_CANISTER_ID, }; use ic_nns_governance_api::pb::v1::{ install_code::CanisterInstallMode, manage_neuron_response, CreateServiceNervousSystem, @@ -24,10 +26,11 @@ use ic_nns_governance_api::pb::v1::{ }; use ic_nns_test_utils::{ common::{ - build_governance_wasm, build_ledger_wasm, build_lifeline_wasm, - build_mainnet_governance_wasm, build_mainnet_ledger_wasm, build_mainnet_lifeline_wasm, - build_mainnet_registry_wasm, build_mainnet_root_wasm, build_mainnet_sns_wasms_wasm, - build_registry_wasm, build_root_wasm, build_sns_wasms_wasm, NnsInitPayloadsBuilder, + build_cmc_wasm, build_governance_wasm, build_ledger_wasm, build_lifeline_wasm, + build_mainnet_cmc_wasm, build_mainnet_governance_wasm, build_mainnet_ledger_wasm, + build_mainnet_lifeline_wasm, build_mainnet_registry_wasm, build_mainnet_root_wasm, + build_mainnet_sns_wasms_wasm, build_registry_wasm, build_root_wasm, build_sns_wasms_wasm, + NnsInitPayloadsBuilder, }, sns_wasm::{ build_archive_sns_wasm, build_governance_sns_wasm, build_index_ng_sns_wasm, @@ -37,7 +40,10 @@ use ic_nns_test_utils::{ build_swap_sns_wasm, ensure_sns_wasm_gzipped, }, }; -use ic_registry_transport::pb::v1::RegistryAtomicMutateRequest; +use ic_registry_proto_data_provider::ProtoRegistryDataProvider; +use ic_registry_transport::pb::v1::{ + registry_mutation, RegistryAtomicMutateRequest, RegistryMutation, +}; use ic_sns_governance_api::pb::v1::{ self as sns_pb, governance::Version, AdvanceTargetVersionRequest, AdvanceTargetVersionResponse, }; @@ -69,7 +75,7 @@ use pocket_ic::{ }; use prost::Message; use rust_decimal::prelude::ToPrimitive; -use std::{collections::BTreeMap, fmt::Write, ops::Range, time::Duration}; +use std::{collections::BTreeMap, fmt::Write, ops::Range, path::Path, time::Duration}; pub const STARTING_CYCLES_PER_CANISTER: u128 = 2_000_000_000_000_000; @@ -318,8 +324,9 @@ pub async fn add_wasms_to_sns_wasm( pub struct NnsInstaller { mainnet_nns_canister_versions: Option, neurons_fund_hotkeys: Vec, - custom_initial_registry_mutations: Option>, + custom_registry_mutations: Option>, initial_balances: Vec<(AccountIdentifier, Tokens)>, + with_cycles_minting_canister: bool, with_cycles_ledger: bool, } @@ -335,8 +342,9 @@ impl NnsInstaller { // Enforce an explicit decision. mainnet_nns_canister_versions: None, neurons_fund_hotkeys: vec![], - custom_initial_registry_mutations: None, + custom_registry_mutations: None, initial_balances: vec![], + with_cycles_minting_canister: false, with_cycles_ledger: false, } } @@ -348,7 +356,7 @@ impl NnsInstaller { } /// Requests tip-of-this-branch Wasm versions for all NNS canisters being installed. - pub fn with_tip_nns_canister_versions(&mut self) -> &mut Self { + pub fn with_current_nns_canister_versions(&mut self) -> &mut Self { self.mainnet_nns_canister_versions = Some(false); self } @@ -363,23 +371,23 @@ impl NnsInstaller { self } - /// Requests that the NNS Registry is initialized with `custom_initial_registry_mutations`. + /// Requests that the NNS Registry is initialized with `custom_registry_mutations`. /// /// The custom mutations must result in an invariant-compliant Registry state. /// /// Without this specification, default initial Registry mutations will be used, which are /// guaranteed to be compliant. - pub fn with_custom_initial_registry_mutations( + pub fn with_custom_registry_mutations( &mut self, - custom_initial_registry_mutations: Vec, + custom_registry_mutations: Vec, ) -> &mut Self { - self.custom_initial_registry_mutations = Some(custom_initial_registry_mutations); + self.custom_registry_mutations = Some(custom_registry_mutations); self } /// Requests `initial_balances` as a Vec of `(test_user_icp_ledger_account, /// test_user_icp_ledger_initial_balance)` pairs, representing some initial ICP balances. - pub fn with_initial_balances( + pub fn with_ledger_balances( &mut self, initial_balances: Vec<(AccountIdentifier, Tokens)>, ) -> &mut Self { @@ -387,6 +395,11 @@ impl NnsInstaller { self } + pub fn with_cycles_minting_canister(&mut self) -> &mut Self { + self.with_cycles_minting_canister = true; + self + } + pub fn with_cycles_ledger(&mut self) -> &mut Self { self.with_cycles_ledger = true; self @@ -411,8 +424,8 @@ impl NnsInstaller { let mut nns_init_payload_builder = NnsInitPayloadsBuilder::new(); - if let Some(custom_initial_registry_mutations) = self.custom_initial_registry_mutations { - nns_init_payload_builder.with_initial_mutations(custom_initial_registry_mutations); + if let Some(custom_registry_mutations) = self.custom_registry_mutations { + nns_init_payload_builder.with_initial_mutations(custom_registry_mutations); } else { nns_init_payload_builder.with_initial_invariant_compliant_mutations(); } @@ -513,6 +526,39 @@ impl NnsInstaller { ) .await; + if self.with_cycles_minting_canister { + let cycles_minting_wasm = if with_mainnet_canister_versions { + build_mainnet_cmc_wasm() + } else { + build_cmc_wasm() + }; + install_canister( + pocket_ic, + "Cycles Minting", + CYCLES_MINTING_CANISTER_ID, + Encode!(&nns_init_payload.cycles_minting).unwrap(), + cycles_minting_wasm, + Some(ROOT_CANISTER_ID.get()), + ) + .await; + + // Authorize canister creation on application subnets. + let application_subnets = pocket_ic + .topology() + .await + .subnet_configs + .keys() + .map(|subnet_id| SubnetId::from(PrincipalId(*subnet_id))) + .collect(); + nns::cmc::set_authorized_subnetwork_list( + pocket_ic, + GOVERNANCE_CANISTER_ID.get(), + None, + application_subnets, + ) + .await; + } + if self.with_cycles_ledger { cycles_ledger::install(pocket_ic).await; } @@ -526,11 +572,48 @@ impl NnsInstaller { } } +pub fn load_registry_mutations>(path: P) -> RegistryAtomicMutateRequest { + let registry_data_provider = ProtoRegistryDataProvider::load_from_file(path.as_ref()); + let updates = registry_data_provider + .get_updates_since(ZERO_REGISTRY_VERSION) + .unwrap(); + + let mutations = updates + .into_iter() + .map(|update| { + let mut mutation = RegistryMutation::default(); + + let typ = if update.value.is_none() { + registry_mutation::Type::Delete + } else { + registry_mutation::Type::Insert + }; + mutation.set_mutation_type(typ); + mutation.key = update.key.as_bytes().to_vec(); + mutation.value = update.value.unwrap_or_default(); + mutation + }) + .collect::>(); + + RegistryAtomicMutateRequest { + mutations, + ..Default::default() + } +} + pub mod cycles_ledger { - use super::install_canister; + use super::{install_canister, nns}; use candid::{CandidType, Encode, Principal}; use canister_test::Wasm; - use ic_nns_constants::{CYCLES_LEDGER_CANISTER_ID, ROOT_CANISTER_ID}; + use cycles_minting_canister::{NotifyMintCyclesSuccess, MEMO_MINT_CYCLES}; + use ic_base_types::PrincipalId; + use ic_nns_constants::{ + CYCLES_LEDGER_CANISTER_ID, CYCLES_MINTING_CANISTER_ID, ROOT_CANISTER_ID, + }; + use icp_ledger::{ + account_identifier::Subaccount, AccountIdentifier, Tokens, TransferArgs, + DEFAULT_TRANSFER_FEE, + }; use pocket_ic::nonblocking::PocketIc; #[derive(Clone, Eq, PartialEq, Hash, Debug, CandidType)] @@ -577,6 +660,44 @@ pub mod cycles_ledger { ) .await; } + + pub async fn mint_icp_and_convert_to_cycles( + pocket_ic: &PocketIc, + beneficiary: PrincipalId, + amount: Tokens, + ) { + nns::ledger::mint_icp( + pocket_ic, + AccountIdentifier::new(beneficiary, None), + amount.saturating_add(DEFAULT_TRANSFER_FEE), + None, + ) + .await; + + let beneficiary_cmc_account = AccountIdentifier::new( + CYCLES_MINTING_CANISTER_ID.into(), + Some(Subaccount::from(&beneficiary)), + ); + + let transfer_args = TransferArgs { + memo: MEMO_MINT_CYCLES, + amount, + fee: Tokens::from_e8s(10_000), + from_subaccount: None, + to: beneficiary_cmc_account.to_address(), + created_at_time: None, + }; + + let block_index = nns::ledger::transfer(&pocket_ic, beneficiary, transfer_args) + .await + .unwrap(); + + let NotifyMintCyclesSuccess { + block_index: _, + minted: _, + balance: _, + } = nns::cmc::notify_mint_cycles(pocket_ic, beneficiary, None, None, block_index).await; + } } /// Installs the NNS canisters, ensuring that there is a whale neuron with `TEST_NEURON_1_ID`. @@ -587,7 +708,7 @@ pub mod cycles_ledger { /// (or, therwise, tip-of-this-branch) WASM versions should be installed. /// 2. `initial_balances` is a `Vec` of `(test_user_icp_ledger_account, /// test_user_icp_ledger_initial_balance)` pairs, representing some initial ICP balances. -/// 3. `custom_initial_registry_mutations` are custom mutations for the inital Registry. These +/// 3. `custom_registry_mutations` are custom mutations for the inital Registry. These /// should mutations should comply with Registry invariants, otherwise this function will fail. /// 4. `maturity_equivalent_icp_e8s` - hotkeys of the 1st NNS (Neurons' Fund-participating) neuron. /// @@ -597,23 +718,23 @@ pub async fn install_nns_canisters( pocket_ic: &PocketIc, initial_balances: Vec<(AccountIdentifier, Tokens)>, with_mainnet_nns_canister_versions: bool, - custom_initial_registry_mutations: Option>, + custom_registry_mutations: Option>, neurons_fund_hotkeys: Vec, ) -> Vec { let mut nns_installer = NnsInstaller::default(); nns_installer - .with_initial_balances(initial_balances) + .with_ledger_balances(initial_balances) .with_neurons_fund_hotkeys(neurons_fund_hotkeys); if with_mainnet_nns_canister_versions { nns_installer.with_mainnet_nns_canister_versions(); } else { - nns_installer.with_tip_nns_canister_versions(); + nns_installer.with_current_nns_canister_versions(); } - if let Some(custom_initial_registry_mutations) = custom_initial_registry_mutations { - nns_installer.with_custom_initial_registry_mutations(custom_initial_registry_mutations); + if let Some(custom_registry_mutations) = custom_registry_mutations { + nns_installer.with_custom_registry_mutations(custom_registry_mutations); } nns_installer.install(pocket_ic).await @@ -1247,14 +1368,32 @@ pub mod nns { Decode!(&result, Tokens).unwrap() } + pub async fn transfer( + pocket_ic: &PocketIc, + sender: PrincipalId, + args: TransferArgs, + ) -> Result { + let result = pocket_ic + .update_call( + LEDGER_CANISTER_ID.into(), + sender.into(), + "transfer", + Encode!(&args).unwrap(), + ) + .await + .unwrap(); + + Decode!(&result, Result).unwrap() + } + // Test method to mint ICP to a principal pub async fn mint_icp( pocket_ic: &PocketIc, destination: AccountIdentifier, amount: Tokens, - ) { - // Construct request. - let transfer_request = TransferArgs { + memo: Option, + ) -> u64 { + let args = TransferArgs { to: destination.to_address(), // An overwhelmingly large number, but not so large as to cause serious risk of // addition overflow. @@ -1263,25 +1402,14 @@ pub mod nns { // Non-Operative // ------------- fee: Tokens::ZERO, // Because we are minting. - memo: Memo(0), + memo: memo.unwrap_or_else(|| Memo(0)), from_subaccount: None, created_at_time: None, }; - // Call ledger. - let result = pocket_ic - .update_call( - LEDGER_CANISTER_ID.into(), - GOVERNANCE_CANISTER_ID.get().0, - "transfer", - Encode!(&transfer_request).unwrap(), - ) - .await; - // Assert result is ok. - match result { - Ok(_reply) => (), // Ok, - _ => panic!("{:?}", result), - } + let result = transfer(pocket_ic, GOVERNANCE_CANISTER_ID.into(), args).await; + + result.unwrap() } } @@ -1417,6 +1545,61 @@ pub mod nns { version } } + + pub mod cmc { + use super::*; + use cycles_minting_canister::{ + NotifyMintCyclesArg, NotifyMintCyclesResult, NotifyMintCyclesSuccess, + SetAuthorizedSubnetworkListArgs, + }; + use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; + use icrc_ledger_types::icrc1::account::Subaccount; + + pub async fn set_authorized_subnetwork_list( + pocket_ic: &PocketIc, + sender: PrincipalId, + who: Option, + subnets: Vec, + ) { + let result = pocket_ic + .update_call( + CYCLES_MINTING_CANISTER_ID.into(), + sender.into(), + "set_authorized_subnetwork_list", + Encode!(&SetAuthorizedSubnetworkListArgs { who, subnets }).unwrap(), + ) + .await + .unwrap(); + + Decode!(&result, ()).unwrap(); + } + + pub async fn notify_mint_cycles( + pocket_ic: &PocketIc, + sender: PrincipalId, + deposit_memo: Option>, + to_subaccount: Option, + block_index: u64, + ) -> NotifyMintCyclesSuccess { + let result = pocket_ic + .update_call( + CYCLES_MINTING_CANISTER_ID.into(), + sender.into(), + "notify_mint_cycles", + Encode!(&NotifyMintCyclesArg { + block_index, + to_subaccount, + deposit_memo + }) + .unwrap(), + ) + .await + .unwrap(); + + let result = Decode!(&result, NotifyMintCyclesResult).unwrap(); + result.unwrap() + } + } } pub mod sns { @@ -3034,6 +3217,7 @@ pub mod sns { pocket_ic, AccountIdentifier::new(participant_id, None), amount.saturating_add(DEFAULT_TRANSFER_FEE), + None, ) .await; participate_in_swap( diff --git a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs index a55365906f5..fc563562792 100644 --- a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs +++ b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs @@ -1,12 +1,11 @@ -use candid::Principal; use canister_test::Wasm; -use ic_nervous_system_agent::nns::registry::get_subnet_for_canister; use ic_nervous_system_agent::pocketic_impl::PocketIcAgent; use ic_nervous_system_integration_tests::pocket_ic_helpers::sns::governance::{ find_neuron_with_majority_voting_power, wait_for_proposal_execution, }; use ic_nervous_system_integration_tests::pocket_ic_helpers::{ - await_with_timeout, install_canister_on_subnet, nns, sns, NnsInstaller, + await_with_timeout, cycles_ledger, install_canister_on_subnet, load_registry_mutations, nns, + sns, NnsInstaller, }; use ic_nervous_system_integration_tests::{ create_service_nervous_system_builder::CreateServiceNervousSystemBuilder, @@ -19,17 +18,15 @@ use ic_sns_cli::upgrade_sns_controlled_canister::{ self, UpgradeSnsControlledCanisterArgs, UpgradeSnsControlledCanisterInfo, }; use ic_sns_swap::pb::v1::Lifecycle; -use pocket_ic::nonblocking::PocketIc; +use icp_ledger::Tokens; use pocket_ic::PocketIcBuilder; -use std::collections::BTreeSet; use std::path::PathBuf; +use tempfile::TempDir; use url::Url; const MIN_INSTALL_CHUNKED_CODE_TIME_SECONDS: u64 = 20; const MAX_INSTALL_CHUNKED_CODE_TIME_SECONDS: u64 = 5 * 60; -const CHUNK_SIZE: usize = 1024 * 1024; // 1 MiB - fn very_large_wasm_path() -> PathBuf { let image_classification_canister_wasm_path = std::env::var("IMAGE_CLASSIFICATION_CANISTER_WASM_PATH") @@ -43,65 +40,14 @@ fn very_large_wasm_bytes() -> Vec { std::fs::read(&wasm_path).expect("Failed to read WASM file") } -fn format_full_hash(hash: &[u8]) -> String { - hash.iter() - .map(|b| format!("{:02x}", b)) - .collect::>() - .join("") -} - -/// Uploads `wasm` into the store canister, one [`CHUNK_SIZE`]-sized chunk at a time. -/// -/// Returns the vector of uploaded chunk hashes. -async fn upload_wasm_as_chunks( - pocket_ic: &PocketIc, - store_controller_id: Principal, - store_canister_id: Principal, - wasm: Wasm, - num_chunks_expected: usize, -) -> Vec> { - let sender = Some(store_controller_id); - - let mut uploaded_chunk_hashes = Vec::new(); - - for chunk in wasm.bytes().chunks(CHUNK_SIZE) { - let uploaded_chunk_hash = pocket_ic - .upload_chunk(store_canister_id, sender, chunk.to_vec()) - .await - .unwrap(); - - uploaded_chunk_hashes.push(uploaded_chunk_hash); - } - - // Smoke test - { - let stored_chunk_hashes = pocket_ic - .stored_chunks(store_canister_id, sender) - .await - .unwrap() - .into_iter() - .map(|hash| format_full_hash(&hash[..])) - .collect::>(); - - let stored_chunk_hashes = BTreeSet::from_iter(stored_chunk_hashes.iter()); - - let uploaded_chunk_hashes = uploaded_chunk_hashes - .iter() - .map(|hash| format_full_hash(&hash[..])) - .collect::>(); - let uploaded_chunk_hashes = BTreeSet::from_iter(uploaded_chunk_hashes.iter()); - - assert!(uploaded_chunk_hashes.is_subset(&stored_chunk_hashes)); - assert_eq!(uploaded_chunk_hashes.len(), num_chunks_expected); - } - - uploaded_chunk_hashes -} - #[tokio::test] -async fn test_store_different_from_target() { +async fn upgrade_sns_controlled_canister_with_large_wasm() { // 1. Prepare the world + let state_dir = TempDir::new().unwrap(); + let state_dir = state_dir.path().to_path_buf(); + let pocket_ic = PocketIcBuilder::new() + .with_state_dir(state_dir.clone()) .with_nns_subnet() .with_sns_subnet() .with_ii_subnet() @@ -111,19 +57,22 @@ async fn test_store_different_from_target() { // Install the NNS canisters. { + let registry_proto_path = state_dir.join("registry.proto"); + let initial_mutations = load_registry_mutations(registry_proto_path); + let mut nns_installer = NnsInstaller::default(); - nns_installer.with_tip_nns_canister_versions(); + nns_installer.with_current_nns_canister_versions(); + nns_installer.with_cycles_minting_canister(); nns_installer.with_cycles_ledger(); + nns_installer.with_custom_registry_mutations(vec![initial_mutations]); nns_installer.install(&pocket_ic).await; } // Publish SNS Wasms to SNS-W. - { - let with_mainnet_sns_canisters = false; - add_wasms_to_sns_wasm(&pocket_ic, with_mainnet_sns_canisters) - .await - .unwrap(); - }; + let with_mainnet_sns_canisters = false; + add_wasms_to_sns_wasm(&pocket_ic, with_mainnet_sns_canisters) + .await + .unwrap(); // Install a dapp canister. let original_wasm = { @@ -132,19 +81,9 @@ async fn test_store_different_from_target() { let wasm_bytes = modify_wasm_bytes(&wasm_bytes, 42); Wasm::from_bytes(&wasm_bytes[..]) }; - let original_wasm_hash = original_wasm.sha256_hash(); + let original_wasm_hash = original_wasm.sha256_hash().to_vec(); let app_subnet = pocket_ic.topology().await.get_app_subnets()[0]; - let ii_subnet = pocket_ic.topology().await.get_ii().unwrap(); - assert_ne!(app_subnet, ii_subnet); - - let root_subnet = get_subnet_for_canister(&pocket_ic, ROOT_CANISTER_ID.into()) - .await - .unwrap(); - - println!("root_subnet = {}", root_subnet); - panic!("boo"); - let target_canister_id = install_canister_on_subnet( &pocket_ic, app_subnet, @@ -175,6 +114,7 @@ async fn test_store_different_from_target() { sns::swap::await_swap_lifecycle(&pocket_ic, sns.swap.canister_id, Lifecycle::Open) .await .unwrap(); + sns::swap::smoke_test_participate_and_finalize( &pocket_ic, sns.swap.canister_id, @@ -189,11 +129,17 @@ async fn test_store_different_from_target() { // neuron either holds the majority of the voting power or the follow graph is set up // s.t. when this neuron submits a proposal, that proposal gets through without the need // for any voting. - let (sns_neuron_id, _) = + let (sns_neuron_id, sender) = find_neuron_with_majority_voting_power(&pocket_ic, sns.governance.canister_id) .await .expect("cannot find SNS neuron with dissolve delay over 6 months."); + // Ensure the user controlling the SNS neuron can also create a canister with enough cycles + // so that it can host the large Wasm. To that end, the GM grants this user 10 ICP, for + // the sake of testing, out of thin air. + let icp = Tokens::from_tokens(10).unwrap(); + cycles_ledger::mint_icp_and_convert_to_cycles(&pocket_ic, sender, icp).await; + let cli_arg = UpgradeSnsControlledCanisterArgs { sns_neuron_id: Some(ParsedSnsNeuron(sns_neuron_id)), target_canister_id, @@ -207,9 +153,10 @@ async fn test_store_different_from_target() { }; // 2. Submit the upgrade proposal. + let pocket_ic_agent = PocketIcAgent { pocket_ic: &pocket_ic, - sender: sns.root.canister_id.into(), + sender: sender.into(), }; let UpgradeSnsControlledCanisterInfo { wasm_module_hash, @@ -218,6 +165,9 @@ async fn test_store_different_from_target() { .await .unwrap(); + // Smoke test. + assert_ne!(wasm_module_hash, original_wasm_hash); + let proposal_id = proposal_id.unwrap(); // 3. Await proposal execution. diff --git a/rs/nns/test_utils/src/common.rs b/rs/nns/test_utils/src/common.rs index 73bed5ef809..a7fc7624f55 100644 --- a/rs/nns/test_utils/src/common.rs +++ b/rs/nns/test_utils/src/common.rs @@ -344,6 +344,11 @@ pub fn build_cmc_wasm() -> Wasm { let features = []; Project::cargo_bin_maybe_from_env("cycles-minting-canister", &features) } +/// Build mainnet Wasm for NNS CMC +pub fn build_mainnet_cmc_wasm() -> Wasm { + let features = []; + Project::cargo_bin_maybe_from_env("mainnet-cycles-minting-canister", &features) +} /// Build Wasm for NNS Lifeline canister pub fn build_lifeline_wasm() -> Wasm { Wasm::from_location_specified_by_env_var("lifeline_canister", &[]) diff --git a/rs/sns/cli/src/upgrade_sns_controlled_canister.rs b/rs/sns/cli/src/upgrade_sns_controlled_canister.rs index 111406c1007..8476660ef48 100644 --- a/rs/sns/cli/src/upgrade_sns_controlled_canister.rs +++ b/rs/sns/cli/src/upgrade_sns_controlled_canister.rs @@ -36,7 +36,7 @@ const GZIPPED_WASM_HEADER: [u8; 3] = [0x1f, 0x8b, 0x08]; // The cycle fee for create request is 0.1T cycles. pub const CANISTER_CREATE_FEE: u128 = 100_000_000_000_u128; -pub const STORE_CANISTER_INITIAL_CYCLES_BALANCE: u128 = 500_000_000_000_u128; // 0.5T +pub const STORE_CANISTER_INITIAL_CYCLES_BALANCE: u128 = 5_000_000_000_000_u128; // 5T /// The arguments used to configure the upgrade_sns_controlled_canister command. #[derive(Debug, Parser)] From 86e4af5e05b450b30b847517ba6cb34aa72def37 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Fri, 31 Jan 2025 12:25:58 +0000 Subject: [PATCH 07/10] lint --- rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index 41e0e4190d0..e96ed630c83 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -688,7 +688,7 @@ pub mod cycles_ledger { created_at_time: None, }; - let block_index = nns::ledger::transfer(&pocket_ic, beneficiary, transfer_args) + let block_index = nns::ledger::transfer(pocket_ic, beneficiary, transfer_args) .await .unwrap(); @@ -1402,7 +1402,7 @@ pub mod nns { // Non-Operative // ------------- fee: Tokens::ZERO, // Because we are minting. - memo: memo.unwrap_or_else(|| Memo(0)), + memo: memo.unwrap_or(Memo(0)), from_subaccount: None, created_at_time: None, }; From 238d1399219e1361fd305aed600fa61b890d94ad Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Fri, 31 Jan 2025 12:36:40 +0000 Subject: [PATCH 08/10] Fix types in SNS CLI and add PocketIc changelog --- packages/pocket-ic/CHANGELOG.md | 2 ++ packages/pocket-ic/src/lib.rs | 2 +- rs/sns/cli/src/main.rs | 9 ++++++--- rs/sns/cli/src/upgrade_sns_controlled_canister.rs | 4 ++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/pocket-ic/CHANGELOG.md b/packages/pocket-ic/CHANGELOG.md index 1f73264f836..e426dc3ef75 100644 --- a/packages/pocket-ic/CHANGELOG.md +++ b/packages/pocket-ic/CHANGELOG.md @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The function `PocketIc::await_call_no_ticks` to await the status of an update call (submitted through an ingress message) becoming known without triggering round execution (round execution must be triggered separarely, e.g., on a "live" instance or by separate PocketIC library calls). - The function `PocketIc::set_certified_time` to set the current certified time on all subnets of the PocketIC instance. +- The function `PocketIc::update_call_with_effective_principal` is made public. It is helpful, e.g., for +modeling management canister calls that need to be routed to the right subnet using effective principals. ### Changed - The response types `pocket_ic::WasmResult`, `pocket_ic::UserError`, and `pocket_ic::CallError` are replaced by a single reject response type `pocket_ic::RejectResponse`. diff --git a/packages/pocket-ic/src/lib.rs b/packages/pocket-ic/src/lib.rs index d7483a4692e..d5becb13a94 100644 --- a/packages/pocket-ic/src/lib.rs +++ b/packages/pocket-ic/src/lib.rs @@ -1129,7 +1129,7 @@ impl PocketIc { runtime.block_on(async { self.pocket_ic.get_subnet_metrics(subnet_id).await }) } - fn update_call_with_effective_principal( + pub fn update_call_with_effective_principal( &self, canister_id: CanisterId, effective_principal: RawEffectivePrincipal, diff --git a/rs/sns/cli/src/main.rs b/rs/sns/cli/src/main.rs index 8dd5dbd0d99..2637deea223 100644 --- a/rs/sns/cli/src/main.rs +++ b/rs/sns/cli/src/main.rs @@ -30,9 +30,12 @@ async fn main() -> Result<()> { SubCommand::List(args) => list::exec(args, &agent).await, SubCommand::Health(args) => health::exec(args, &agent).await, SubCommand::UpgradeSnsControlledCanister(args) => { - upgrade_sns_controlled_canister::exec(args, &agent) - .await - .map_err(|err| anyhow::anyhow!(err)) + match upgrade_sns_controlled_canister::exec(args, &agent).await { + Ok(_) => Ok(()), + Err(err) => { + bail!("{}", err); + } + } } } } diff --git a/rs/sns/cli/src/upgrade_sns_controlled_canister.rs b/rs/sns/cli/src/upgrade_sns_controlled_canister.rs index 8476660ef48..8bd55cafeb7 100644 --- a/rs/sns/cli/src/upgrade_sns_controlled_canister.rs +++ b/rs/sns/cli/src/upgrade_sns_controlled_canister.rs @@ -419,7 +419,7 @@ pub async fn exec( let Some(sns) = &sns else { unimplemented!( "Direct canister upgrades are not implemented yet. Please use DFX:\n{}", - suggested_install_command(&wasm.path(), &candid_arg) + suggested_install_command(wasm.path(), &candid_arg) ); }; @@ -591,7 +591,7 @@ fn suggested_install_command(wasm_path_str: &Path, candid_arg: &Option) }; format!( "dfx canister install --mode auto --wasm {} CANISTER_NAME{}", - wasm_path_str.display().to_string(), + wasm_path_str.display(), arg_suggestion, ) } From 6224e8750752581aef9ab3896937fe513e8ec630 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Fri, 31 Jan 2025 13:45:49 +0000 Subject: [PATCH 09/10] clippy --- rs/nervous_system/agent/src/pocketic_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/nervous_system/agent/src/pocketic_impl.rs b/rs/nervous_system/agent/src/pocketic_impl.rs index a021b8075ca..d349c66c242 100644 --- a/rs/nervous_system/agent/src/pocketic_impl.rs +++ b/rs/nervous_system/agent/src/pocketic_impl.rs @@ -43,7 +43,7 @@ impl PocketIcAgent<'_> { &self, canister_id: Principal, ) -> Result { - let Some(subnet_id) = self.pocket_ic.get_subnet(canister_id.into()).await else { + let Some(subnet_id) = self.pocket_ic.get_subnet(canister_id).await else { return Err(PocketIcCallError::CanisterSubnetNotFound(canister_id)); }; From 1f06e443d14a9a362b784e5d69dfa3f0f47061b9 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Fri, 31 Jan 2025 16:01:42 +0000 Subject: [PATCH 10/10] Address nice to have review comments from the previous PR. And make bug happy --- .../integration_tests/BUILD.bazel | 2 +- .../src/pocket_ic_helpers.rs | 24 ++++--------------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/rs/nervous_system/integration_tests/BUILD.bazel b/rs/nervous_system/integration_tests/BUILD.bazel index 2ce0239df0e..111a3dee5ed 100644 --- a/rs/nervous_system/integration_tests/BUILD.bazel +++ b/rs/nervous_system/integration_tests/BUILD.bazel @@ -22,9 +22,9 @@ BASE_DEPENDENCIES = [ "//rs/nns/governance/api", "//rs/nns/sns-wasm", "//rs/registry/proto_data_provider", + "//rs/sns/cli", "//rs/sns/governance", "//rs/sns/governance/api", - "//rs/sns/cli", "//rs/sns/init", "//rs/sns/root", "//rs/sns/swap", diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index e96ed630c83..656e2b904d6 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -321,6 +321,7 @@ pub async fn add_wasms_to_sns_wasm( } /// A builder for fine-tuning and installing the NNS canister suite in PocketIc. +#[derive(Default)] pub struct NnsInstaller { mainnet_nns_canister_versions: Option, neurons_fund_hotkeys: Vec, @@ -330,25 +331,7 @@ pub struct NnsInstaller { with_cycles_ledger: bool, } -impl Default for NnsInstaller { - fn default() -> Self { - Self::new() - } -} - impl NnsInstaller { - fn new() -> Self { - Self { - // Enforce an explicit decision. - mainnet_nns_canister_versions: None, - neurons_fund_hotkeys: vec![], - custom_registry_mutations: None, - initial_balances: vec![], - with_cycles_minting_canister: false, - with_cycles_ledger: false, - } - } - /// Requests the mainnet Wasm versions for all NNS canisters being installed. pub fn with_mainnet_nns_canister_versions(&mut self) -> &mut Self { self.mainnet_nns_canister_versions = Some(true); @@ -361,8 +344,9 @@ impl NnsInstaller { self } - /// Requests that the NNS Governance is initialized with a Neurons' Fund neuron controlled - /// by `neurons_fund_hotkeys`. + /// Requests that the NNS Governance is initialized with a Neurons' Fund neuron with hotkeys + /// taken from `neurons_fund_hotkeys`. Hotkeys are principals that can control the neuron + /// in various ways, but generally less powerful than the neuron controller. pub fn with_neurons_fund_hotkeys( &mut self, neurons_fund_hotkeys: Vec,