Skip to content

Commit

Permalink
test(nns): Clean up integration test helpers (#3812)
Browse files Browse the repository at this point in the history 
Changes:

* Some test helper methods are duplicated between
`rs/nns/test_utils/src/itest_helpers.rs` and
`rs/nns/test_utils/src/governance.rs`, and among those duplicated code
some are not used at all. Delete those unused ones.
* `change_nns_canister_by_proposal` is hostile to those use cases where
canisters are upgraded to the same wasm but different arg. The `old_wasm
!= new_wasm` doesn't seem to be very useful in general. The check is
removed in this PR
* The purpose is to switch
`//rs/tests/cross_chain:ic_xc_ledger_suite_orchestrator_test_head_nns`
to use this test helper while sending proposals to add ERC20 tokens
(which uses the same WASM but with an upgrade arg) Currently the test is
using `NnsFunction::NnsCanisterUpgrade` and we want to move away from
it.
  • Loading branch information
@jasonz-dfinity
jasonz-dfinity authored Feb 6, 2025
1 parent 90acaff commit fa83f00
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 238 deletions.
33 changes: 2 additions & 31 deletions rs/nns/test_utils/src/governance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,6 @@ pub use ic_nns_handler_lifeline_interface::{
};
use std::time::Duration;

/// Thin-wrapper around submit_proposal to handle
/// serialization/deserialization
pub async fn submit_proposal(
governance_canister: &Canister<'_>,
proposal: &MakeProposalRequest,
) -> ProposalId {
governance_canister
.update_("submit_proposal", candid_one, proposal)
.await
.unwrap()
}

/// Wraps the given nns_function_input into a proposal; sends it to the governance
/// canister; returns the proposal id or, in case of failure, a
/// `GovernanceError`.
Expand Down Expand Up @@ -311,30 +299,13 @@ async fn change_nns_canister_by_proposal(
let wasm = wasm.bytes();
let new_module_hash = &ic_crypto_sha2::Sha256::hash(&wasm);

let status: CanisterStatusResult = root
.update_(
"canister_status",
candid_one,
CanisterIdRecord::from(canister.canister_id()),
)
.await
.unwrap();
let old_module_hash = status.module_hash.unwrap();
assert_ne!(
old_module_hash.as_slice(),
new_module_hash,
"change_nns_canister_by_proposal: both module hashes prev, cur are \
the same {:?}, but they should be different for upgrade",
old_module_hash
);

let proposal = MakeProposalRequest {
title: Some("Upgrade NNS Canister".to_string()),
summary: "<proposal created by change_nns_canister_by_proposal>".to_string(),
url: "".to_string(),
action: Some(ProposalActionRequest::InstallCode(InstallCodeRequest {
canister_id: Some(canister.canister_id().get()),
wasm_module: Some(wasm.clone()),
wasm_module: Some(wasm),
install_mode: Some(how as i32),
arg: Some(arg.unwrap_or_default()),
skip_stopping_before_installing: Some(stop_before_installing),
Expand Down Expand Up @@ -391,7 +362,7 @@ async fn change_nns_canister_by_proposal(
else {
continue;
};
if status.module_hash.unwrap().as_slice() == new_module_hash
if status.module_hash.unwrap_or_default().as_slice() == new_module_hash
&& status.status == CanisterStatusType::Running
{
break;
Expand Down
207 changes: 0 additions & 207 deletions rs/nns/test_utils/src/itest_helpers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,10 @@ use canister_test::{
use cycles_minting_canister::CyclesCanisterInitPayload;
use dfn_candid::{candid_one, CandidOne};
use futures::{future::join_all, FutureExt};
use ic_base_types::CanisterId;
use ic_canister_client_sender::Sender;
use ic_config::Config;
use ic_management_canister_types::CanisterInstallMode;
use ic_nervous_system_clients::{
canister_id_record::CanisterIdRecord,
canister_status::{CanisterStatusResult, CanisterStatusType},
};
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_common::{
init::LifelineCanisterInitPayload,
types::{NeuronId, ProposalId},
Expand Down Expand Up @@ -716,207 +710,6 @@ pub async fn maybe_upgrade_to_self(canister: &mut Canister<'_>, scenario: Upgrad
}
}

fn is_gzipped_blob(blob: &[u8]) -> bool {
(blob.len() > 4)
// Has magic bytes.
&& (blob[0..2] == [0x1F, 0x8B])
}

/// Bumps the gzip timestamp of the provided gzipped Wasm.
/// Results in a functionally identical binary.
pub fn bump_gzip_timestamp(wasm: &Wasm) -> Wasm {
// wasm is gzipped and the subslice [4..8]
// is the little endian representation of a timestamp
// so we just increment that timestamp
let mut new_wasm = wasm.clone().bytes();
assert!(is_gzipped_blob(&new_wasm));
let t = u32::from_le_bytes(new_wasm[4..8].try_into().unwrap());
new_wasm[4..8].copy_from_slice(&(t + 1).to_le_bytes());
Wasm::from_bytes(new_wasm)
}

/// Perform a change on a canister by upgrading it or
/// reinstalling entirely, depending on the `how` argument.
/// Argument `wasm` is ensured to have a different
/// hash relative to the current binary.
/// In argument `arg` additional arguments can be provided
/// that serve as input to the upgrade hook or as init arguments
/// to the fresh installation.
///
/// This is an internal method.
async fn change_nns_canister_by_proposal(
how: CanisterInstallMode,
canister_id: CanisterId,
governance: &Canister<'_>,
root: &Canister<'_>,
stop_before_installing: bool,
wasm: Wasm,
arg: Option<Vec<u8>>,
) {
let wasm = wasm.bytes();
let new_module_hash = &ic_crypto_sha2::Sha256::hash(&wasm);

let status: CanisterStatusResult = root
.update_(
"canister_status",
candid_one,
CanisterIdRecord::from(canister_id),
)
.await
.unwrap();
let old_module_hash = status.module_hash.unwrap();
assert_ne!(old_module_hash.as_slice(), new_module_hash, "change_nns_canister_by_proposal: both module hashes prev, cur are the same {:?}, but they should be different for upgrade", old_module_hash);

let change_canister_request =
ChangeCanisterRequest::new(stop_before_installing, how, canister_id)
.with_memory_allocation(memory_allocation_of(canister_id))
.with_wasm(wasm);
let change_canister_request = if let Some(arg) = arg {
change_canister_request.with_arg(arg)
} else {
change_canister_request
};

// Submitting a proposal also implicitly records a vote from the proposer,
// which with TEST_NEURON_1 is enough to trigger execution.
submit_external_update_proposal(
governance,
Sender::from_keypair(&TEST_NEURON_1_OWNER_KEYPAIR),
NeuronId(TEST_NEURON_1_ID),
NnsFunction::NnsCanisterUpgrade,
change_canister_request,
"Upgrade NNS Canister".to_string(),
"<proposal created by change_nns_canister_by_proposal>".to_string(),
)
.await;

// Wait 'till the hash matches and the canister is running again.
loop {
let status: CanisterStatusResult = root
.update_(
"canister_status",
candid_one,
CanisterIdRecord::from(canister_id),
)
.await
.unwrap();
if status.module_hash.unwrap().as_slice() == new_module_hash
&& status.status == CanisterStatusType::Running
{
break;
}
}
}

/// 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.
///
/// This goes through MANY rounds of consensus, so expect it to be slow!
///
/// WARNING: this calls `execute_eligible_proposals` on the governance canister,
/// so it may have side effects!
pub async fn upgrade_nns_canister_by_proposal(
canister: &Canister<'_>,
governance: &Canister<'_>,
root: &Canister<'_>,
stop_before_installing: bool,
wasm: Wasm,
) {
change_nns_canister_by_proposal(
CanisterInstallMode::Upgrade,
canister.canister_id(),
governance,
root,
stop_before_installing,
wasm,
None,
)
.await
}

/// Upgrades an nns canister via proposal, with an argument.
pub async fn upgrade_nns_canister_with_arg_by_proposal(
canister: &Canister<'_>,
governance: &Canister<'_>,
root: &Canister<'_>,
wasm: Wasm,
arg: Vec<u8>,
) {
change_nns_canister_by_proposal(
CanisterInstallMode::Upgrade,
canister.canister_id(),
governance,
root,
false,
wasm,
Some(arg),
)
.await
}

/// Propose and execute the fresh reinstallation of the canister. Wasm
/// and initialisation arguments can be specified.
/// This should only be called in NNS integration tests, where the NNS
/// canisters have their expected IDs.
///
/// WARNING: this calls `execute_eligible_proposals` on the governance canister,
/// so it may have side effects!
pub async fn reinstall_nns_canister_by_proposal(
canister: &Canister<'_>,
governance: &Canister<'_>,
root: &Canister<'_>,
wasm: Wasm,
arg: Vec<u8>,
) {
change_nns_canister_by_proposal(
CanisterInstallMode::Reinstall,
canister.canister_id(),
governance,
root,
true,
bump_gzip_timestamp(&wasm),
Some(arg),
)
.await
}

/// Depending on the testing scenario, upgrade the given root-controlled
/// canister to itself, or do nothing. This should only be called in NNS
/// integration tests, where the NNS canisters have their expected IDs.
///
/// This goes through MANY rounds of consensus, so expect it to be slow!
///
/// WARNING: this calls `execute_eligible_proposals` on the governance canister,
/// so it may have side effects!
pub async fn maybe_upgrade_root_controlled_canister_to_self(
// nns_canisters is NOT passed by reference because of the canister to upgrade,
// for which we have a mutable borrow.
nns_canisters: NnsCanisters<'_>,
canister: &mut Canister<'_>,
stop_before_installing: bool,
scenario: UpgradeTestingScenario,
) {
if UpgradeTestingScenario::Always != scenario {
return;
}

// Copy the wasm of the canister to upgrade. We'll need it to upgrade back to
// it. To observe that the upgrade happens, we need to make the binary different
// post-upgrade.
let wasm = bump_gzip_timestamp(canister.wasm().unwrap());
let wasm_clone = wasm.clone().bytes();
upgrade_nns_canister_by_proposal(
canister,
&nns_canisters.governance,
&nns_canisters.root,
stop_before_installing,
wasm,
)
.await;
canister.set_wasm(wasm_clone);
}

const UNIVERSAL_CANISTER_YEAH_RESPONSE: &[u8] = b"It worked";
const UNIVERSAL_CANISTER_NOPE_RESPONSE: &[u8] = b"It failed";

Expand Down

0 comments on commit fa83f00

Please sign in to comment.