Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

test(nns): Stop using NnsCanisterUpgrade nns function in integration tests #3841

Merged
merged 2 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion rs/nns/integration_tests/src/canister_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ fn test_upgrade_canister(canister_id: CanisterId, canister_wasm: Wasm) {
canister_id,
modified_wasm.clone(),
vec![],
true,
);

let controller_canister_id = if canister_id == ROOT_CANISTER_ID {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ fn test_copy_inactive_neurons_to_stable_memory() {
GOVERNANCE_CANISTER_ID, // Target, i.e. the canister that we want to upgrade.
new_wasm_content, // The new code that we want the canister to start running.
module_arg,
true,
);
println!("Done proposing governance upgrade: {:?}", proposal_id);

Expand Down
1 change: 0 additions & 1 deletion rs/nns/integration_tests/src/governance_mem_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ fn governance_mem_test() {
GOVERNANCE_CANISTER_ID,
real_gov_wasm.bytes(),
module_arg,
true,
);

state_machine.tick();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ fn test_upgrade_canisters_with_golden_nns_state() {
*canister_id,
wasm_content.clone(),
module_arg.clone(),
true,
);

// Step 3: Verify result(s): In a short while, the canister should
Expand Down
20 changes: 20 additions & 0 deletions rs/nns/test_utils/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,26 @@ async fn change_nns_canister_by_proposal(
}
}

/// Installs the given root-controlled canister with the specified Wasm module and args.
pub async fn install_nns_canister_by_proposal(
canister: &Canister<'_>,
governance: &Canister<'_>,
root: &Canister<'_>,
wasm: Wasm,
arg: Option<Vec<u8>>,
) {
change_nns_canister_by_proposal(
CanisterInstallMode::Install,
canister,
governance,
root,
false,
wasm,
arg,
)
.await
}

/// Upgrade the given root-controlled canister to the specified Wasm module.
/// This should only be called in NNS integration tests, where the NNS
/// canisters have their expected IDs.
Expand Down
50 changes: 6 additions & 44 deletions rs/nns/test_utils/src/state_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use ic_nervous_system_common::{
ledger::{compute_neuron_staking_subaccount, compute_neuron_staking_subaccount_bytes},
DEFAULT_TRANSFER_FEE, ONE_DAY_SECONDS,
};
use ic_nervous_system_root::change_canister::ChangeCanisterRequest;
use ic_nns_common::pb::v1::{NeuronId, ProposalId};
use ic_nns_constants::{
canister_id_to_nns_canister_name, memory_allocation_of, CYCLES_LEDGER_CANISTER_ID,
Expand Down Expand Up @@ -51,7 +50,6 @@ use ic_nns_governance_api::pb::v1::{
ManageNeuronResponse, MonthlyNodeProviderRewards, NetworkEconomics, NnsFunction,
ProposalActionRequest, ProposalInfo, RewardNodeProviders, Topic, Vote,
};
use ic_nns_handler_lifeline_interface::UpgradeRootProposal;
use ic_nns_handler_root::init::RootCanisterInitPayload;
use ic_registry_transport::pb::v1::{
RegistryGetChangesSinceRequest, RegistryGetChangesSinceResponse,
Expand Down Expand Up @@ -1051,54 +1049,18 @@ pub fn nns_propose_upgrade_nns_canister(
target_canister_id: CanisterId,
wasm_module: Vec<u8>,
module_arg: Vec<u8>,
use_proposal_action: bool,
) -> ProposalId {
let action = if use_proposal_action {
Some(ProposalActionRequest::InstallCode(InstallCodeRequest {
let target_canister_name = canister_id_to_nns_canister_name(target_canister_id);

let proposal = MakeProposalRequest {
title: Some(format!("Upgrade {}", target_canister_name)),
action: Some(ProposalActionRequest::InstallCode(InstallCodeRequest {
canister_id: Some(target_canister_id.get()),
install_mode: Some(3),
wasm_module: Some(wasm_module),
arg: Some(module_arg),
skip_stopping_before_installing: None,
}))
} else if target_canister_id != ROOT_CANISTER_ID {
let payload = ChangeCanisterRequest::new(
true, // stop_before_installing,
CanisterInstallMode::Upgrade,
target_canister_id,
)
.with_memory_allocation(memory_allocation_of(target_canister_id))
.with_wasm(wasm_module);

let payload = Encode!(&payload).unwrap();

Some(ProposalActionRequest::ExecuteNnsFunction(
ExecuteNnsFunction {
nns_function: NnsFunction::NnsCanisterUpgrade as i32,
payload,
},
))
} else {
let payload = UpgradeRootProposal {
wasm_module,
module_arg,
stop_upgrade_start: true,
};
let payload = Encode!(&payload).unwrap();

Some(ProposalActionRequest::ExecuteNnsFunction(
ExecuteNnsFunction {
nns_function: NnsFunction::NnsRootUpgrade as i32,
payload,
},
))
};

let target_canister_name = canister_id_to_nns_canister_name(target_canister_id);

let proposal = MakeProposalRequest {
title: Some(format!("Upgrade {}", target_canister_name)),
action,
})),
..Default::default()
};

Expand Down
65 changes: 17 additions & 48 deletions rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,18 @@ use ic_ledger_suite_orchestrator::candid::{
AddErc20Arg, Erc20Contract, InitArg, LedgerInitArg, ManagedCanisterIds, OrchestratorArg,
UpgradeArg,
};
use ic_management_canister_types_private::CanisterInstallMode;
use ic_nervous_system_clients::canister_status::CanisterStatusResult;
use ic_nervous_system_common_test_keys::{TEST_NEURON_1_ID, TEST_NEURON_1_OWNER_KEYPAIR};
use ic_nervous_system_root::change_canister::ChangeCanisterRequest;
use ic_nns_constants::{GOVERNANCE_CANISTER_ID, ROOT_CANISTER_ID};
use ic_nns_test_utils::governance::submit_external_update_proposal;
use ic_nns_test_utils::governance::{
install_nns_canister_by_proposal, upgrade_nns_canister_with_args_by_proposal,
};
use ic_registry_subnet_type::SubnetType;
use ic_system_test_driver::driver::group::SystemTestGroup;
use ic_system_test_driver::driver::ic::{InternetComputer, Subnet};
use ic_system_test_driver::driver::test_env::TestEnv;
use ic_system_test_driver::driver::test_env_api::{
get_dependency_path, HasPublicApiUrl, HasTopologySnapshot, IcNodeContainer, NnsCustomizations,
};
use ic_system_test_driver::nns::vote_and_execute_proposal;
use ic_system_test_driver::systest;
use ic_system_test_driver::util::{block_on, runtime_from_url};
use ic_wasm_types::CanisterModule;
Expand Down Expand Up @@ -188,10 +186,7 @@ async fn install_nns_controlled_canister<'a>(
canister_wasm: CanisterModule,
canister_init_payload: Vec<u8>,
) -> Canister<'a> {
use ic_canister_client::Sender;
use ic_nervous_system_clients::canister_status::CanisterStatusType;
use ic_nns_common::types::{NeuronId, ProposalId};
use ic_nns_governance_api::pb::v1::{NnsFunction, ProposalStatus};

let canister = application_subnet_runtime
.create_canister(Some(u128::MAX))
Expand All @@ -214,26 +209,16 @@ async fn install_nns_controlled_canister<'a>(
ROOT_CANISTER_ID
);

let new_module_hash = canister_wasm.module_hash().to_vec();
let wasm = canister_wasm.as_slice().to_vec();
let proposal_payload =
ChangeCanisterRequest::new(true, CanisterInstallMode::Install, canister.canister_id())
.with_wasm(wasm)
.with_arg(canister_init_payload);

let proposal_id: ProposalId = submit_external_update_proposal(
let new_module_hash = canister_wasm.module_hash();
let wasm = Wasm::from_bytes(canister_wasm.as_slice());
install_nns_canister_by_proposal(
&canister,
governance_canister,
Sender::from_keypair(&TEST_NEURON_1_OWNER_KEYPAIR),
NeuronId(TEST_NEURON_1_ID),
NnsFunction::NnsCanisterUpgrade,
proposal_payload,
"Install Canister".to_string(),
"<proposal created by install_nns_controlled_canister>".to_string(),
root_canister,
wasm,
Some(canister_init_payload),
)
.await;

let proposal_result = vote_and_execute_proposal(governance_canister, proposal_id).await;
assert_eq!(proposal_result.status, ProposalStatus::Executed as i32);
info!(
logger,
"Installed WASM to {} via NNS proposal",
Expand All @@ -242,7 +227,7 @@ async fn install_nns_controlled_canister<'a>(

status_of_nns_controlled_canister_satisfy(logger, root_canister, &canister, |status| {
status.status == CanisterStatusType::Running
&& status.module_hash.as_deref() == Some(new_module_hash.as_ref())
&& status.module_hash.as_deref() == Some(new_module_hash.as_slice())
})
.await;

Expand All @@ -259,33 +244,17 @@ async fn upgrade_ledger_suite_orchestrator_by_nns_proposal(
orchestrator: &LedgerOrchestratorCanister<'_>,
upgrade_arg: OrchestratorArg,
) {
use ic_canister_client::Sender;
use ic_nervous_system_clients::canister_status::CanisterStatusType;
use ic_nns_common::types::{NeuronId, ProposalId};
use ic_nns_governance_api::pb::v1::{NnsFunction, ProposalStatus};

let wasm = canister_wasm.as_slice().to_vec();
let proposal_payload = ChangeCanisterRequest::new(
true,
CanisterInstallMode::Upgrade,
orchestrator.as_ref().canister_id(),
)
.with_wasm(wasm)
.with_arg(Encode!(&upgrade_arg).unwrap());

let proposal_id: ProposalId = submit_external_update_proposal(
upgrade_nns_canister_with_args_by_proposal(
orchestrator.as_ref(),
governance_canister,
Sender::from_keypair(&TEST_NEURON_1_OWNER_KEYPAIR),
NeuronId(TEST_NEURON_1_ID),
NnsFunction::NnsCanisterUpgrade,
proposal_payload,
"Upgrade LSO".to_string(),
"<proposal created by upgrade_lso_by_nns_proposal>".to_string(),
root_canister,
true,
Wasm::from_bytes(canister_wasm.as_slice()),
Encode!(&upgrade_arg).unwrap(),
)
.await;

let proposal_result = vote_and_execute_proposal(governance_canister, proposal_id).await;
assert_eq!(proposal_result.status, ProposalStatus::Executed as i32);
info!(
logger,
"Upgrade ledger suite orchestrator {:?} via NNS proposal", upgrade_arg
Expand Down