Skip to content

Commit

Permalink
Add with_suspend_all_spinners and use it for ask_for_consent
Browse files Browse the repository at this point in the history
  • Loading branch information
adamspofford-dfinity committed Feb 10, 2025
1 parent bb305d2 commit 63e296d
Show file tree
Hide file tree
Showing 11 changed files with 73 additions and 44 deletions.
10 changes: 6 additions & 4 deletions src/dfx-core/src/canister/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::{
cli::ask_for_consent,
error::canister::{CanisterBuilderError, CanisterInstallError},
error::{
canister::{CanisterBuilderError, CanisterInstallError},
cli::UserConsent,
},
identity::CallSender,
};
use candid::Principal;
Expand Down Expand Up @@ -52,10 +54,10 @@ pub async fn install_canister_wasm(
mode: InstallMode,
call_sender: &CallSender,
wasm_module: Vec<u8>,
skip_consent: bool,
ask_for_consent: impl FnOnce(&str) -> Result<(), UserConsent>,
) -> Result<(), CanisterInstallError> {
let mgr = ManagementCanister::create(agent);
if !skip_consent && mode == InstallMode::Reinstall {
if mode == InstallMode::Reinstall {
let msg = if let Some(name) = canister_name {
format!("You are about to reinstall the {name} canister")
} else {
Expand Down
17 changes: 0 additions & 17 deletions src/dfx-core/src/cli/mod.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/dfx-core/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
pub mod canister;
pub mod cli;
pub mod config;
pub mod error;
pub mod extension;
Expand Down
10 changes: 5 additions & 5 deletions src/dfx/src/commands/canister/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ use crate::lib::operations::canister::{
use crate::lib::operations::cycles_ledger::wallet_deposit_to_cycles_ledger;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::assets::wallet_wasm;
use crate::util::blob_from_arguments;
use crate::util::clap::parsers::{cycle_amount_parser, icrc_subaccount_parser};
use crate::util::{ask_for_consent, blob_from_arguments};
use anyhow::{bail, Context};
use candid::Principal;
use clap::Parser;
use dfx_core::canister::build_wallet_canister;
use dfx_core::cli::ask_for_consent;
use dfx_core::identity::wallet::wallet_canister_id;
use dfx_core::identity::CallSender;
use fn_error_context::context;
Expand Down Expand Up @@ -177,9 +176,10 @@ async fn delete_canister(
// Determine how many cycles we can withdraw.
let status = canister::get_canister_status(env, canister_id, call_sender).await?;
if status.status != CanisterStatus::Stopped && !skip_confirmation {
ask_for_consent(&format!(
"Canister {canister} has not been stopped. Delete anyway?"
))?;
ask_for_consent(
env,
&format!("Canister {canister} has not been stopped. Delete anyway?"),
)?;
}
let agent = env.get_agent();
let mgr = ManagementCanister::create(agent);
Expand Down
10 changes: 8 additions & 2 deletions src/dfx/src/commands/canister/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::operations::canister::install_canister::install_canister;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::blob_from_arguments;
use crate::util::clap::argument_from_cli::ArgumentFromCliLongOpt;
use crate::util::clap::install_mode::{InstallModeHint, InstallModeOpt};
use crate::util::{ask_for_consent, blob_from_arguments};
use dfx_core::canister::{
install_canister_wasm, install_mode_to_past_tense, install_mode_to_present_tense,
};
Expand Down Expand Up @@ -122,7 +122,13 @@ pub async fn exec(
mode,
call_sender,
wasm_module,
opts.yes,
|message| {
if opts.yes {
Ok(())
} else {
ask_for_consent(env, message)
}
},
)
.await?;
spinner.finish_and_clear();
Expand Down
4 changes: 2 additions & 2 deletions src/dfx/src/commands/canister/update_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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::ask_for_consent;
use crate::util::clap::parsers::{
compute_allocation_parser, freezing_threshold_parser, memory_allocation_parser,
reserved_cycles_limit_parser, wasm_memory_limit_parser,
Expand All @@ -19,7 +20,6 @@ use byte_unit::Byte;
use candid::Principal as CanisterId;
use candid::Principal;
use clap::{ArgAction, Parser};
use dfx_core::cli::ask_for_consent;
use dfx_core::error::identity::InstantiateIdentityFromNameError::GetIdentityPrincipalFailed;
use dfx_core::identity::CallSender;
use fn_error_context::context;
Expand Down Expand Up @@ -146,7 +146,7 @@ pub async fn exec(
fetch_root_key_if_needed(env).await?;

if !opts.yes && user_is_removing_themselves_as_controller(env, call_sender, &opts)? {
ask_for_consent("You are trying to remove yourself as a controller of this canister. This may leave this canister un-upgradeable.")?
ask_for_consent(env, "You are trying to remove yourself as a controller of this canister. This may leave this canister un-upgradeable.")?
}

let controllers: Option<DfxResult<Vec<_>>> = opts.set_controller.as_ref().map(|controllers| {
Expand Down
5 changes: 2 additions & 3 deletions src/dfx/src/commands/wallet/redeem_faucet_coupon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::identity::wallet::set_wallet_id;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::{format_as_trillions, pretty_thousand_separators};
use crate::util::{ask_for_consent, format_as_trillions, pretty_thousand_separators};
use anyhow::{anyhow, bail, Context};
use candid::{encode_args, Decode, Principal};
use clap::Parser;
use dfx_core::cli::ask_for_consent;
use slog::{info, warn};

pub const DEFAULT_FAUCET_PRINCIPAL: Principal =
Expand Down Expand Up @@ -74,7 +73,7 @@ pub async fn exec(env: &dyn Environment, opts: RedeemFaucetCouponOpts) -> DfxRes
// identity has no wallet yet - faucet will provide one
_ => {
if !opts.yes {
ask_for_consent("`dfx cycles` is now recommended instead of `dfx wallet`. Are you sure you want to create a new cycles wallet anyway?")?;
ask_for_consent(env, "`dfx cycles` is now recommended instead of `dfx wallet`. Are you sure you want to create a new cycles wallet anyway?")?;
}
let identity = env
.get_selected_identity()
Expand Down
4 changes: 2 additions & 2 deletions src/dfx/src/lib/canister_logs/log_visibility.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::util::ask_for_consent;
use crate::util::clap::parsers::{log_visibility_parser, principal_parser};
use anyhow::anyhow;
use candid::Principal;
use clap::{ArgAction, Args};
use dfx_core::cli::ask_for_consent;
use ic_utils::interfaces::management_canister::{LogVisibility, StatusCallResult};

#[derive(Args, Clone, Debug, Default)]
Expand Down Expand Up @@ -104,7 +104,7 @@ impl LogVisibilityOpt {
if let Some(added) = self.add_log_viewer.as_ref() {
if let Some(LogVisibility::Public) = current_visibility {
let msg = "Current log is public to everyone. Adding log reviewers will make the log only visible to the reviewers.";
ask_for_consent(msg)?;
ask_for_consent(env, msg)?;
}

for principal in added {
Expand Down
12 changes: 12 additions & 0 deletions src/dfx/src/lib/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub trait Environment {
fn get_logger(&self) -> &slog::Logger;
fn get_verbose_level(&self) -> i64;
fn new_spinner(&self, message: Cow<'static, str>) -> ProgressBar;
fn with_suspend_all_spinners(&self, f: Box<dyn FnOnce() + '_>); // box needed for dyn Environment
fn new_progress(&self, message: &str) -> ProgressBar;

fn new_identity_manager(&self) -> Result<IdentityManager, NewIdentityManagerError> {
Expand Down Expand Up @@ -255,6 +256,10 @@ impl Environment for EnvironmentImpl {
}
}

fn with_suspend_all_spinners(&self, f: Box<dyn FnOnce() + '_>) {
self.spinners.suspend(f);
}

fn new_progress(&self, _message: &str) -> ProgressBar {
ProgressBar::discard()
}
Expand Down Expand Up @@ -415,6 +420,10 @@ impl<'a> Environment for AgentEnvironment<'a> {
self.backend.new_spinner(message)
}

fn with_suspend_all_spinners(&self, f: Box<dyn FnOnce() + '_>) {
self.backend.with_suspend_all_spinners(f);
}

fn new_progress(&self, message: &str) -> ProgressBar {
self.backend.new_progress(message)
}
Expand Down Expand Up @@ -535,6 +544,9 @@ pub mod test_env {
fn new_progress(&self, _message: &str) -> ProgressBar {
ProgressBar::discard()
}
fn with_suspend_all_spinners(&self, f: Box<dyn FnOnce() + '_>) {
f()
}
fn new_spinner(&self, _message: Cow<'static, str>) -> ProgressBar {
ProgressBar::discard()
}
Expand Down
23 changes: 15 additions & 8 deletions src/dfx/src/lib/operations/canister/install_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use crate::lib::operations::canister::motoko_playground::authorize_asset_uploade
use crate::lib::state_tree::canister_info::read_state_tree_canister_module_hash;
use crate::util::assets::wallet_wasm;
use crate::util::clap::install_mode::InstallModeHint;
use crate::util::{blob_from_arguments, get_candid_init_type, read_module_metadata};
use crate::util::{
ask_for_consent, blob_from_arguments, get_candid_init_type, read_module_metadata,
};
use anyhow::{anyhow, bail, Context};
use backoff::backoff::Backoff;
use backoff::ExponentialBackoff;
Expand All @@ -19,7 +21,6 @@ use dfx_core::canister::{
build_wallet_canister, install_canister_wasm, install_mode_to_past_tense,
install_mode_to_present_tense,
};
use dfx_core::cli::ask_for_consent;
use dfx_core::config::model::canister_id_store::CanisterIdStore;
use dfx_core::config::model::network_descriptor::NetworkDescriptor;
use dfx_core::identity::CallSender;
Expand Down Expand Up @@ -101,11 +102,11 @@ pub async fn install_canister(
Ok(None) => (),
Ok(Some(err)) => {
let msg = format!("Candid interface compatibility check failed for canister '{}'.\nYou are making a BREAKING change. Other canisters or frontend clients relying on your canister may stop working.\n\n", canister_info.get_name()) + &err;
ask_for_consent(&msg)?;
ask_for_consent(env, &msg)?;
}
Err(e) => {
let msg = format!("An error occurred during Candid interface compatibility check for canister '{}'.\n\n", canister_info.get_name()) + &e.to_string();
ask_for_consent(&msg)?;
ask_for_consent(env, &msg)?;
}
}
}
Expand All @@ -117,15 +118,15 @@ pub async fn install_canister(
Ok(StableCompatibility::Okay) => (),
Ok(StableCompatibility::Warning(details)) => {
let msg = format!("Stable interface compatibility check issued a WARNING for canister '{}'.\n\n", canister_info.get_name()) + &details;
ask_for_consent(&msg)?;
ask_for_consent(env, &msg)?;
}
Ok(StableCompatibility::Error(details)) => {
let msg = format!("Stable interface compatibility check issued an ERROR for canister '{}'.\nUpgrade will either FAIL or LOSE some stable variable data.\n\n", canister_info.get_name()) + &details;
ask_for_consent(&msg)?;
ask_for_consent(env, &msg)?;
}
Err(e) => {
let msg = format!("An error occurred during stable interface compatibility check for canister '{}'.\n\n", canister_info.get_name()) + &e.to_string();
ask_for_consent(&msg)?;
ask_for_consent(env, &msg)?;
}
}
}
Expand Down Expand Up @@ -221,7 +222,13 @@ The command line value will be used.",
mode,
call_sender,
wasm_module,
skip_consent,
|message| {
if skip_consent {
Ok(())
} else {
ask_for_consent(env, message)
}
},
)
.await?;
}
Expand Down
21 changes: 21 additions & 0 deletions src/dfx/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use candid::types::{value::IDLValue, Function, Type, TypeEnv, TypeInner};
use candid::{Decode, Encode, IDLArgs, Principal};
use candid_parser::error::pretty_wrap;
use candid_parser::utils::CandidSource;
use dfx_core::error::cli::UserConsent;
use dfx_core::fs::create_dir_all;
use fn_error_context::context;
use idl2json::{idl2json, Idl2JsonOptions};
Expand Down Expand Up @@ -425,6 +426,26 @@ async fn attempt_download(client: &Client, url: &Url) -> DfxResult<Option<Bytes>
}
}

pub fn ask_for_consent(env: &dyn Environment, message: &str) -> Result<(), UserConsent> {
let mut ans = Ok(());
env.with_suspend_all_spinners(Box::new(|| {
eprintln!("WARNING!");
eprintln!("{}", message);
eprintln!("Do you want to proceed? yes/No");
let mut input_string = String::new();
let res = stdin().read_line(&mut input_string);
if let Err(e) = res {
ans = Err(UserConsent::ReadError(e));
return;
}
let input_string = input_string.trim_end().to_lowercase();
if input_string != "yes" && input_string != "y" {
ans = Err(UserConsent::Declined);
}
}));
ans
}

#[cfg(test)]
mod tests {
use super::{download_file, format_as_trillions, pretty_thousand_separators};
Expand Down

0 comments on commit 63e296d

Please sign in to comment.