Skip to content

Commit

Permalink
refactor(nervous-system): Use Sns type in integration tests rather …
Browse files Browse the repository at this point in the history
…than `DeployedSns` (#1589)

Unlike `DeployedSns`, the `Sns` type has no optional fields. This means
we can remove the repetitive unwrapping in every test that deploys an
SNS. The `Sns` type also has some utility functions on it that can be
used in the integration tests once #1590 is merged
  • Loading branch information
anchpop authored Sep 25, 2024
1 parent 09e7929 commit 35153c7
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 145 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions rs/nervous_system/integration_tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package(default_visibility = ["//visibility:public"])
# See rs/nervous_system/feature_test.md
BASE_DEPENDENCIES = [
# Keep sorted.
"//rs/nervous_system/agent",
"//rs/nervous_system/clients",
"//rs/nervous_system/common",
"//rs/nervous_system/proto",
Expand Down
1 change: 1 addition & 0 deletions rs/nervous_system/integration_tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ candid = { workspace = true }
cycles-minting-canister = { path = "../../nns/cmc" }
ic-base-types = { path = "../../types/base_types" }
ic-ledger-core = { path = "../../rosetta-api/ledger_core" }
ic-nervous-system-agent = { path = "../agent" }
ic-nervous-system-clients = { path = "../clients" }
ic-nervous-system-common = { path = "../common" }
ic-nervous-system-proto = { path = "../proto" }
Expand Down
10 changes: 6 additions & 4 deletions rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use candid::{Decode, Encode, Nat, Principal};
use canister_test::Wasm;
use ic_base_types::{CanisterId, PrincipalId, SubnetId};
use ic_ledger_core::Tokens;
use ic_nervous_system_agent::sns::Sns;
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};
Expand Down Expand Up @@ -45,8 +46,8 @@ use ic_sns_swap::pb::v1::{
use ic_sns_test_utils::itest_helpers::populate_canister_ids;
use ic_sns_wasm::pb::v1::{
get_deployed_sns_by_proposal_id_response::GetDeployedSnsByProposalIdResult, AddWasmRequest,
DeployedSns, GetDeployedSnsByProposalIdRequest, GetDeployedSnsByProposalIdResponse,
SnsCanisterType, SnsWasm,
GetDeployedSnsByProposalIdRequest, GetDeployedSnsByProposalIdResponse, SnsCanisterType,
SnsWasm,
};
use icp_ledger::{AccountIdentifier, BinaryAccountBalanceArgs};
use icrc_ledger_types::icrc1::{
Expand Down Expand Up @@ -845,7 +846,7 @@ pub mod nns {
pocket_ic: &PocketIc,
create_service_nervous_system: CreateServiceNervousSystem,
sns_instance_label: &str,
) -> (DeployedSns, ProposalId) {
) -> (Sns, ProposalId) {
let proposal_info = propose_and_wait(
pocket_ic,
MakeProposalRequest {
Expand All @@ -868,7 +869,8 @@ pub mod nns {
nns_proposal_id, sns_instance_label,
);
};
(deployed_sns, nns_proposal_id)
let sns = Sns::try_from(deployed_sns).expect("Failed to convert DeployedSns to Sns");
(sns, nns_proposal_id)
}

pub fn get_network_economics_parameters(pocket_ic: &PocketIc) -> NetworkEconomics {
Expand Down
82 changes: 31 additions & 51 deletions rs/nervous_system/integration_tests/tests/sns_ledger_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use ic_nns_test_utils::sns_wasm::{
build_archive_sns_wasm, build_index_ng_sns_wasm, build_ledger_sns_wasm, build_swap_sns_wasm,
create_modified_sns_wasm,
};
use ic_sns_wasm::pb::v1::{DeployedSns, SnsCanisterType};
use ic_sns_wasm::pb::v1::SnsCanisterType;
use ic_test_utilities::universal_canister::UNIVERSAL_CANISTER_WASM;
use icrc_ledger_types::{
icrc1::{account::Account, transfer::TransferArg},
Expand Down Expand Up @@ -93,25 +93,17 @@ fn test_deploy_fresh_sns() {

// Deploy an SNS instance via proposal.
let sns_instance_label = "1";
let (deployed_sns, _) = nns::governance::propose_to_deploy_sns_and_wait(
let (sns, _) = nns::governance::propose_to_deploy_sns_and_wait(
&pocket_ic,
create_service_nervous_system,
sns_instance_label,
);
let DeployedSns {
governance_canister_id: Some(sns_governance_canister_id),
ledger_canister_id: Some(sns_ledger_canister_id),
..
} = deployed_sns
else {
panic!("Cannot find some SNS caniser IDs in {:#?}", deployed_sns);
};

// Testing the Archive canister requires that it can be spawned.
sns::ensure_archive_canister_is_spawned_or_panic(
&pocket_ic,
sns_governance_canister_id,
sns_ledger_canister_id,
sns.governance.canister_id,
sns.ledger.canister_id,
);
// TODO eventually we need to test a swap
}
Expand Down Expand Up @@ -178,32 +170,20 @@ fn test_upgrade_existing_sns() {

// Deploy an SNS instance via proposal.
let sns_instance_label = "1";
let (deployed_sns, _) = nns::governance::propose_to_deploy_sns_and_wait(
let (sns, _) = nns::governance::propose_to_deploy_sns_and_wait(
&pocket_ic,
create_service_nervous_system,
sns_instance_label,
);
let DeployedSns {
governance_canister_id: Some(sns_governance_canister_id),
root_canister_id: Some(sns_root_canister_id),
index_canister_id: Some(index_canister_id),
ledger_canister_id: Some(sns_ledger_canister_id),
..
} = deployed_sns
else {
panic!("Cannot find some SNS caniser IDs in {:#?}", deployed_sns);
};

// Testing the Archive canister requires that it can be spawned.
sns::ensure_archive_canister_is_spawned_or_panic(
&pocket_ic,
sns_governance_canister_id,
sns_ledger_canister_id,
sns.governance.canister_id,
sns.ledger.canister_id,
);

// Step 3. Upgrade SNS to the tip of master

// Step 4. Upgrade NNS Governance and SNS-W to the latest version.
// Step 3. Upgrade NNS Governance and SNS-W to the latest version.
upgrade_nns_canister_to_tip_of_master_or_panic(&pocket_ic, GOVERNANCE_CANISTER_ID);
upgrade_nns_canister_to_tip_of_master_or_panic(&pocket_ic, SNS_WASM_CANISTER_ID);

Expand Down Expand Up @@ -238,66 +218,66 @@ fn test_upgrade_existing_sns() {
// Upgrade Swap - should be non-event
sns::upgrade_sns_to_next_version_and_assert_change(
&pocket_ic,
sns_root_canister_id,
sns.root.canister_id,
SnsCanisterType::Swap,
);

// Upgrade Index-Ng
{
sns::upgrade_sns_to_next_version_and_assert_change(
&pocket_ic,
sns_root_canister_id,
sns.root.canister_id,
SnsCanisterType::Index,
);

// Index-Ng check 1: The Index canister still recognised our Ledger canitser.
assert_eq!(
sns::index_ng::ledger_id(&pocket_ic, index_canister_id),
sns_ledger_canister_id
sns::index_ng::ledger_id(&pocket_ic, sns.index.canister_id),
sns.ledger.canister_id
);

// Index-Ng check 2: Index and Ledger sync.
sns::wait_until_ledger_and_index_sync_is_completed(
&pocket_ic,
sns_ledger_canister_id,
index_canister_id,
sns.ledger.canister_id,
sns.index.canister_id,
);

// Index-Ng check 3: The same blocks can be observed via Index and Ledger.
sns::assert_ledger_index_parity(&pocket_ic, sns_ledger_canister_id, index_canister_id);
sns::assert_ledger_index_parity(&pocket_ic, sns.ledger.canister_id, sns.index.canister_id);
}

// Upgrade SNS Ledger
{
let original_total_supply_sns_e8s =
sns::ledger::icrc1_total_supply(&pocket_ic, sns_ledger_canister_id)
sns::ledger::icrc1_total_supply(&pocket_ic, sns.ledger.canister_id)
.0
.to_u64()
.unwrap();

let pre_upgrade_chain_length =
sns::ledger::get_blocks(&pocket_ic, sns_ledger_canister_id, 0_u64, 1_u64).chain_length;
sns::ledger::get_blocks(&pocket_ic, sns.ledger.canister_id, 0_u64, 1_u64).chain_length;

sns::ledger::check_blocks_or_panic(&pocket_ic, sns_ledger_canister_id);
sns::ledger::check_blocks_or_panic(&pocket_ic, sns.ledger.canister_id);

sns::upgrade_sns_to_next_version_and_assert_change(
&pocket_ic,
sns_root_canister_id,
sns.root.canister_id,
SnsCanisterType::Ledger,
);

// Ledger check 1: We get the expected state in the archive(s).
sns::ledger::check_blocks_or_panic(&pocket_ic, sns_ledger_canister_id);
sns::ledger::check_blocks_or_panic(&pocket_ic, sns.ledger.canister_id);

// Ledger check 2: We get the same number of blocks that we had before the upgrade (because
// no transactions have happened after the upgrade).
let post_upgrade_chain_length =
sns::ledger::get_blocks(&pocket_ic, sns_ledger_canister_id, 0_u64, 1_u64).chain_length;
sns::ledger::get_blocks(&pocket_ic, sns.ledger.canister_id, 0_u64, 1_u64).chain_length;
assert_eq!(post_upgrade_chain_length, pre_upgrade_chain_length);

// Ledger check 3: Total supply remains unchanged.
let total_supply_sns_e8s =
sns::ledger::icrc1_total_supply(&pocket_ic, sns_ledger_canister_id)
sns::ledger::icrc1_total_supply(&pocket_ic, sns.ledger.canister_id)
.0
.to_u64()
.unwrap();
Expand All @@ -315,8 +295,8 @@ fn test_upgrade_existing_sns() {
// Mint some tokens for the wealthy user.
let _block_height = sns::ledger::icrc1_transfer(
&pocket_ic,
sns_ledger_canister_id,
sns_governance_canister_id,
sns.ledger.canister_id,
sns.governance.canister_id,
TransferArg {
from_subaccount: None,
to: wealthy_user_account,
Expand All @@ -341,7 +321,7 @@ fn test_upgrade_existing_sns() {
};
sns::ledger::icrc2_approve(
&pocket_ic,
sns_ledger_canister_id,
sns.ledger.canister_id,
wealthy_user_principal_id,
ApproveArgs {
from_subaccount: wealthy_user_account.subaccount,
Expand All @@ -357,7 +337,7 @@ fn test_upgrade_existing_sns() {
.unwrap();
let allowance = sns::ledger::icrc2_allowance(
&pocket_ic,
sns_ledger_canister_id,
sns.ledger.canister_id,
PrincipalId::new_anonymous(),
AllowanceArgs {
account: wealthy_user_account,
Expand All @@ -367,7 +347,7 @@ fn test_upgrade_existing_sns() {
assert_eq!(allowance.allowance, Nat::from(100_000_u64));
sns::ledger::icrc2_transfer_from(
&pocket_ic,
sns_ledger_canister_id,
sns.ledger.canister_id,
spender_principal_id,
TransferFromArgs {
spender_subaccount: None,
Expand All @@ -384,16 +364,16 @@ fn test_upgrade_existing_sns() {

// Upgrade SNS Archive
{
sns::ledger::check_blocks_or_panic(&pocket_ic, sns_ledger_canister_id);
sns::ledger::check_blocks_or_panic(&pocket_ic, sns.ledger.canister_id);

sns::upgrade_sns_to_next_version_and_assert_change(
&pocket_ic,
sns_root_canister_id,
sns.root.canister_id,
SnsCanisterType::Archive,
);

// Archive check 1: We get the expected state in the archive(s).
sns::ledger::check_blocks_or_panic(&pocket_ic, sns_ledger_canister_id);
sns::ledger::check_blocks_or_panic(&pocket_ic, sns.ledger.canister_id);
}

// Publish modified versions of all the wasms and ensure we can upgrade a second time (pre-upgrade smoke test)
Expand Down Expand Up @@ -426,7 +406,7 @@ fn test_upgrade_existing_sns() {
] {
sns::upgrade_sns_to_next_version_and_assert_change(
&pocket_ic,
sns_root_canister_id,
sns.root.canister_id,
sns_canister_type,
);
}
Expand Down
Loading

0 comments on commit 35153c7

Please sign in to comment.