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

fix: commands with --all parameter skip remote canisters #4016

Merged
merged 12 commits into from
Nov 26, 2024
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ You can use the `--replica` flag already to write scripts that anticipate that c
An extension can define one or more project templates for `dfx new` to use.
These can be new templates or replace the built-in project templates.

### fix: all commands with --all parameter skip remote canisters

This affects the following commands:
- `dfx canister delete`
- `dfx canister deposit-cycles`
- `dfx canister start`
- `dfx canister status`
- `dfx canister stop`
- `dfx canister uninstall-code`
- `dfx canister update-settings`
- `dfx ledger fabricate-cycles`

# 0.24.3

### feat: Bitcoin support in PocketIC
Expand Down
7 changes: 7 additions & 0 deletions e2e/tests-dfx/cycles-ledger.bash
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,13 @@ current_time_nanoseconds() {
assert_eq "2399699700000 cycles."
assert_command dfx canister status e2e_project_backend
assert_contains "Balance: 3_100_002_100_000 Cycles"

# deposit-cycles --all skips remote canisters
jq '.canisters.remote.remote.id.local="rdmx6-jaaaa-aaaaa-aaadq-cai"' dfx.json | sponge dfx.json
assert_command dfx canister deposit-cycles 10000 --all --identity bob
assert_contains "Skipping canister 'remote' because it is remote for network 'local'"
assert_contains "Depositing 10000 cycles onto e2e_project_backend"
assert_not_contains "Depositing 10000 cycles onto remote"
}

@test "top-up deduplication" {
Expand Down
28 changes: 27 additions & 1 deletion e2e/tests-dfx/remote.bash
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ teardown() {
assert_match "Canister 'remote' is a remote canister on network 'actuallylocal', and cannot be installed from here."
}

@test "canister create --all, canister install --all skip remote canisters" {
@test "all commands with --all skip remote canisters" {
install_asset remote/actual
dfx_start
setup_actuallylocal_shared_network
Expand Down Expand Up @@ -201,6 +201,32 @@ teardown() {
assert_command jq .remote canister_ids.json
assert_eq "null"

assert_command dfx ledger fabricate-cycles --all --t 100 --network actuallylocal
assert_contains "Skipping canister 'remote' because it is remote for network 'actuallylocal'"

assert_command dfx canister status --all --network actuallylocal
assert_contains "Skipping canister 'remote' because it is remote for network 'actuallylocal'"

assert_command dfx canister update-settings --log-visibility public --all --network actuallylocal
assert_contains "Skipping canister 'remote' because it is remote for network 'actuallylocal'"

assert_command dfx canister stop --all --network actuallylocal
assert_contains "Skipping canister 'remote' because it is remote for network 'actuallylocal'"

assert_command dfx canister start --all --network actuallylocal
assert_contains "Skipping canister 'remote' because it is remote for network 'actuallylocal'"

# have to stop to uninstall
assert_command dfx canister stop --all --network actuallylocal

assert_command dfx canister uninstall-code --all --network actuallylocal
assert_contains "Skipping canister 'remote' because it is remote for network 'actuallylocal'"

assert_command dfx build --all --network actuallylocal

assert_command dfx canister delete --all --network actuallylocal
assert_contains "Skipping canister 'remote' because it is remote for network 'actuallylocal'"

# Assert frontend declarations are actually created
dfx generate
assert_file_exists "src/declarations/remote/remote.did"
Expand Down
12 changes: 2 additions & 10 deletions src/dfx/src/commands/canister/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::lib::ic_attributes::{
get_compute_allocation, get_freezing_threshold, get_log_visibility, get_memory_allocation,
get_reserved_cycles_limit, get_wasm_memory_limit, CanisterSettings,
};
use crate::lib::operations::canister::create_canister;
use crate::lib::operations::canister::{create_canister, skip_remote_canister};
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::clap::parsers::{
compute_allocation_parser, freezing_threshold_parser, log_visibility_parser,
Expand Down Expand Up @@ -245,15 +245,7 @@ pub async fn exec(
if pull_canisters_in_config.contains_key(canister_name) {
continue;
}
let canister_is_remote =
config_interface.is_remote_canister(canister_name, &network.name)?;
if canister_is_remote {
info!(
env.get_logger(),
"Skipping canister '{canister_name}' because it is remote for network '{}'",
&network.name,
);

if skip_remote_canister(env, canister_name)? {
continue;
}
let specified_id = config_interface.get_specified_id(canister_name)?;
Expand Down
5 changes: 4 additions & 1 deletion src/dfx/src/commands/canister/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::lib::error::DfxResult;
use crate::lib::ic_attributes::CanisterSettings;
use crate::lib::operations::canister;
use crate::lib::operations::canister::{
deposit_cycles, start_canister, stop_canister, update_settings,
deposit_cycles, skip_remote_canister, start_canister, stop_canister, update_settings,
};
use crate::lib::operations::cycles_ledger::wallet_deposit_to_cycles_ledger;
use crate::lib::root_key::fetch_root_key_if_needed;
Expand Down Expand Up @@ -352,6 +352,9 @@ pub async fn exec(
} else if opts.all {
if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if skip_remote_canister(env, canister)? {
continue;
}
delete_canister(
env,
canister,
Expand Down
5 changes: 5 additions & 0 deletions src/dfx/src/commands/canister/deposit_cycles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::time::{SystemTime, UNIX_EPOCH};
use crate::lib::error::DfxResult;
use crate::lib::identity::wallet::get_or_create_wallet_canister;
use crate::lib::operations::canister;
use crate::lib::operations::canister::skip_remote_canister;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::lib::{environment::Environment, operations::cycles_ledger};
use crate::util::clap::parsers::{cycle_amount_parser, icrc_subaccount_parser};
Expand Down Expand Up @@ -147,6 +148,10 @@ pub async fn exec(

if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if skip_remote_canister(env, canister)? {
continue;
}

deposit_cycles(
env,
canister,
Expand Down
10 changes: 2 additions & 8 deletions src/dfx/src/commands/canister/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::util::clap::install_mode::{InstallModeHint, InstallModeOpt};
use dfx_core::canister::{install_canister_wasm, install_mode_to_prompt};
use dfx_core::identity::CallSender;

use crate::lib::operations::canister::skip_remote_canister;
use anyhow::bail;
use candid::Principal;
use clap::Parser;
Expand Down Expand Up @@ -187,21 +188,14 @@ pub async fn exec(
} else if opts.all {
// Install all canisters.
let config = env.get_config_or_anyhow()?;
let config_interface = config.get_config();
let env_file = config.get_output_env_file(opts.output_env_file)?;
let pull_canisters_in_config = get_pull_canisters_in_config(env)?;
if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if pull_canisters_in_config.contains_key(canister) {
continue;
}
if config_interface.is_remote_canister(canister, &network.name)? {
info!(
env.get_logger(),
"Skipping canister '{}' because it is remote for network '{}'",
canister,
&network.name,
);
if skip_remote_canister(env, canister)? {
continue;
}

Expand Down
6 changes: 6 additions & 0 deletions src/dfx/src/commands/canister/start.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::operations::canister;
use crate::lib::operations::canister::skip_remote_canister;
use crate::lib::root_key::fetch_root_key_if_needed;
use candid::Principal;
use clap::Parser;
Expand Down Expand Up @@ -51,8 +52,13 @@ pub async fn exec(
start_canister(env, canister, call_sender).await
} else if opts.all {
let config = env.get_config_or_anyhow()?;

if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if skip_remote_canister(env, canister)? {
continue;
}

start_canister(env, canister, call_sender).await?;
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/dfx/src/commands/canister/status.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::operations::canister;
use crate::lib::operations::canister::skip_remote_canister;
use crate::lib::root_key::fetch_root_key_if_needed;
use candid::Principal;
use clap::Parser;
Expand Down Expand Up @@ -95,8 +96,13 @@ pub async fn exec(
canister_status(env, canister, call_sender).await
} else if opts.all {
let config = env.get_config_or_anyhow()?;

if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if skip_remote_canister(env, canister)? {
continue;
}

canister_status(env, canister, call_sender).await?;
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/dfx/src/commands/canister/stop.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::operations::canister;
use crate::lib::operations::canister::skip_remote_canister;
use crate::lib::root_key::fetch_root_key_if_needed;
use candid::Principal;
use clap::Parser;
Expand Down Expand Up @@ -52,8 +53,13 @@ pub async fn exec(
stop_canister(env, canister, call_sender).await
} else if opts.all {
let config = env.get_config_or_anyhow()?;

if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if skip_remote_canister(env, canister)? {
continue;
}

stop_canister(env, canister, call_sender).await?;
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/dfx/src/commands/canister/uninstall_code.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::operations::canister;
use crate::lib::operations::canister::skip_remote_canister;
use crate::lib::root_key::fetch_root_key_if_needed;
use candid::Principal;
use clap::Parser;
Expand Down Expand Up @@ -56,6 +57,9 @@ pub async fn exec(

if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if skip_remote_canister(env, canister)? {
continue;
}
uninstall_code(env, canister, call_sender).await?;
}
}
Expand Down
8 changes: 7 additions & 1 deletion src/dfx/src/commands/canister/update_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use crate::lib::ic_attributes::{
get_compute_allocation, get_freezing_threshold, get_log_visibility, get_memory_allocation,
get_reserved_cycles_limit, get_wasm_memory_limit, CanisterSettings,
};
use crate::lib::operations::canister::{get_canister_status, update_settings};
use crate::lib::operations::canister::{
get_canister_status, skip_remote_canister, update_settings,
};
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::clap::parsers::{
compute_allocation_parser, freezing_threshold_parser, memory_allocation_parser,
Expand Down Expand Up @@ -220,8 +222,12 @@ pub async fn exec(
// Update all canister settings.
let config = env.get_config_or_anyhow()?;
let config_interface = config.get_config();

if let Some(canisters) = &config_interface.canisters {
for canister_name in canisters.keys() {
if skip_remote_canister(env, canister_name)? {
continue;
}
let mut controllers = controllers.clone();
let canister_id = canister_id_store.get(canister_name)?;
let compute_allocation = get_compute_allocation(
Expand Down
6 changes: 6 additions & 0 deletions src/dfx/src/commands/ledger/fabricate_cycles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::nns_types::icpts::ICPTs;
use crate::lib::operations::canister;
use crate::lib::operations::canister::skip_remote_canister;
use crate::lib::root_key::fetch_root_key_or_anyhow;
use crate::util::clap::parsers::{cycle_amount_parser, e8s_parser, trillion_cycle_amount_parser};
use crate::util::currency_conversion::as_cycles_with_current_exchange_rate;
Expand Down Expand Up @@ -120,8 +121,13 @@ pub async fn exec(env: &dyn Environment, opts: FabricateCyclesOpts) -> DfxResult
deposit_minted_cycles(env, canister, &CallSender::SelectedId, cycles).await
} else if opts.all {
let config = env.get_config_or_anyhow()?;

if let Some(canisters) = &config.get_config().canisters {
for canister in canisters.keys() {
if skip_remote_canister(env, canister)? {
continue;
}

deposit_minted_cycles(env, canister, &CallSender::SelectedId, cycles).await?;
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/dfx/src/lib/operations/canister/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ pub(crate) mod create_canister;
pub(crate) mod deploy_canisters;
pub(crate) mod install_canister;
pub mod motoko_playground;
mod skip_remote_canister;

pub use create_canister::create_canister;
use ic_utils::interfaces::management_canister::Snapshot;
pub use install_canister::install_wallet;
pub use skip_remote_canister::skip_remote_canister;

use crate::lib::canister_info::CanisterInfo;
use crate::lib::environment::Environment;
Expand Down
17 changes: 17 additions & 0 deletions src/dfx/src/lib/operations/canister/skip_remote_canister.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use slog::info;

pub fn skip_remote_canister(env: &dyn Environment, canister: &str) -> DfxResult<bool> {
let config = env.get_config_or_anyhow()?;
let config_interface = config.get_config();
let network = env.get_network_descriptor();
let canister_is_remote = config_interface.is_remote_canister(canister, &network.name)?;
if canister_is_remote {
info!(
env.get_logger(),
"Skipping canister '{canister}' because it is remote for network '{}'", &network.name,
);
}
Ok(canister_is_remote)
}
Loading