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

CHANGELOG.md

@@ -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

e2e/tests-dfx/cycles-ledger.bash

@@ -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" {

e2e/tests-dfx/remote.bash

@@ -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
@@ -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"

src/dfx/src/commands/canister/create.rs

@@ -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,
@@ -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)?;

src/dfx/src/commands/canister/delete.rs

@@ -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;
@@ -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,

src/dfx/src/commands/canister/deposit_cycles.rs

@@ -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};
@@ -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,

src/dfx/src/commands/canister/install.rs

@@ -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;
@@ -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;
                 }
 

src/dfx/src/commands/canister/start.rs

@@ -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;
@@ -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?;
             }
         }

src/dfx/src/commands/canister/status.rs

@@ -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;
@@ -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?;
             }
         }

src/dfx/src/commands/canister/stop.rs

@@ -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;
@@ -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?;
             }
         }

src/dfx/src/commands/canister/uninstall_code.rs

@@ -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;
@@ -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?;
             }
         }

src/dfx/src/commands/canister/update_settings.rs

@@ -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,
@@ -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(

src/dfx/src/commands/ledger/fabricate_cycles.rs

@@ -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;
@@ -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?;
             }
         }

src/dfx/src/lib/operations/canister/mod.rs

@@ -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;

src/dfx/src/lib/operations/canister/skip_remote_canister.rs

@@ -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)
+}