diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e08055f5b..ad211ef5d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ # UNRELEASED +### feat: candid assist feature + +Ask for user input when Candid argument is not provided in `dfx canister call`, `dfx canister install` and `dfx deploy`. +Previously, we cannot call `dfx deploy --all` when multiple canisters require init args, unless the init args are specified in `dfx.json`. With the Candid assist feature, dfx now asks for init args in terminal when a canister requires init args. + ### fix: restored access to URLs like http://localhost:8080/api/v2/status through icx-proxy Pinned icx-proxy at 69e1408347723dbaa7a6cd2faa9b65c42abbe861, shipped with dfx 0.15.2 diff --git a/Cargo.lock b/Cargo.lock index 64ce796246..f70c657b1c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -811,7 +811,10 @@ dependencies = [ "arbitrary", "candid", "codespan-reporting", + "console", "convert_case 0.6.0", + "ctrlc", + "dialoguer", "fake", "hex", "lalrpop", diff --git a/e2e/assets/echo/echo.mo b/e2e/assets/echo/echo.mo new file mode 100644 index 0000000000..38f1ab8c8b --- /dev/null +++ b/e2e/assets/echo/echo.mo @@ -0,0 +1,7 @@ +actor class C(p: { x: Nat; y: Int }) { + type Profile = { name: Principal; kind: {#admin; #user; #guest }; age: ?Nat8; }; + type List = (Profile, ?List); + public query func echo(x: List) : async List { + x + } +}; diff --git a/e2e/assets/echo/patch.bash b/e2e/assets/echo/patch.bash new file mode 100644 index 0000000000..625ab08825 --- /dev/null +++ b/e2e/assets/echo/patch.bash @@ -0,0 +1 @@ +jq '.canisters.hello_backend.main="echo.mo"' dfx.json | sponge dfx.json diff --git a/e2e/assets/expect_scripts/candid_assist.exp b/e2e/assets/expect_scripts/candid_assist.exp new file mode 100755 index 0000000000..c20174a540 --- /dev/null +++ b/e2e/assets/expect_scripts/candid_assist.exp @@ -0,0 +1,59 @@ +#!/usr/bin/expect -df + +set timeout 30 +match_max 100000 + +spawn dfx deploy + +send "42\r" +send "42\r" +expect "Sending the following argument:\r +(record { x = 42 : nat; y = 42 : int })\r +\r +Do you want to initialize the canister with this argument? \[y/N\]\r +" +send "y\r" +expect eof + +spawn dfx canister call hello_backend echo + +# principal auto-completion +send "hello " +expect "bkyz2-fmaaa-aaaaa-qaaaq-cai" +send "\r" +# opt nat8 +send "y" +send "20" +send "\r" +# variant down arrow: user +send "\[B" +send "\[B" +send "\r" +send "n" +expect "Sending the following argument:\r +(\r + record {\r + record {\r + id = principal \"bkyz2-fmaaa-aaaaa-qaaaq-cai\";\r + age = opt (20 : nat8);\r + role = variant { user };\r + };\r + null;\r + },\r +)\r +\r +Do you want to send this message? \[y/N\]\r +" +send "y\r" +expect "y\r +(\r + record {\r + record {\r + id = principal \"bkyz2-fmaaa-aaaaa-qaaaq-cai\";\r + age = opt (20 : nat8);\r + role = variant { user };\r + };\r + null;\r + },\r +)\r" +expect eof diff --git a/e2e/tests-dfx/call.bash b/e2e/tests-dfx/call.bash index c90572ba2f..41de6d4c38 100644 --- a/e2e/tests-dfx/call.bash +++ b/e2e/tests-dfx/call.bash @@ -39,6 +39,12 @@ teardown() { assert_eq '(record { c = "A"; d = "B" })' } +@test "call without argument, using candid assistant" { + install_asset echo + dfx_start + assert_command "${BATS_TEST_DIRNAME}/../assets/expect_scripts/candid_assist.exp" +} + @test "call subcommand accepts canister identifier as canister name" { install_asset greet dfx_start diff --git a/scripts/workflows/provision-linux.sh b/scripts/workflows/provision-linux.sh index 25969266ec..855d2aa082 100755 --- a/scripts/workflows/provision-linux.sh +++ b/scripts/workflows/provision-linux.sh @@ -33,7 +33,8 @@ if [ "$E2E_TEST" = "tests-dfx/identity_encryption.bash" ] \ || [ "$E2E_TEST" = "tests-dfx/identity.bash" ] \ || [ "$E2E_TEST" = "tests-dfx/generate.bash" ] \ || [ "$E2E_TEST" = "tests-dfx/start.bash" ] \ - || [ "$E2E_TEST" = "tests-dfx/new.bash" ] + || [ "$E2E_TEST" = "tests-dfx/new.bash" ] \ + || [ "$E2E_TEST" = "tests-dfx/call.bash" ] then sudo apt-get install --yes expect fi diff --git a/src/dfx-core/src/canister/mod.rs b/src/dfx-core/src/canister/mod.rs index 9ef051589a..5e9cd3fd64 100644 --- a/src/dfx-core/src/canister/mod.rs +++ b/src/dfx-core/src/canister/mod.rs @@ -12,7 +12,6 @@ use ic_utils::{ }, Argument, }; -use slog::{info, Logger}; pub async fn build_wallet_canister( id: Principal, @@ -29,6 +28,14 @@ pub async fn build_wallet_canister( .map_err(CanisterBuilderError::WalletCanisterCaller) } +pub fn install_mode_to_prompt(mode: &InstallMode) -> &'static str { + match mode { + InstallMode::Install => "Installing", + InstallMode::Reinstall => "Reinstalling", + InstallMode::Upgrade { .. } => "Upgrading", + } +} + pub async fn install_canister_wasm( agent: &Agent, canister_id: Principal, @@ -38,7 +45,6 @@ pub async fn install_canister_wasm( call_sender: &CallSender, wasm_module: Vec, skip_consent: bool, - logger: &Logger, ) -> Result<(), CanisterInstallError> { let mgr = ManagementCanister::create(agent); if !skip_consent && mode == InstallMode::Reinstall { @@ -54,19 +60,7 @@ YOU WILL LOSE ALL DATA IN THE CANISTER. "#; ask_for_consent(&msg).map_err(CanisterInstallError::UserConsent)?; } - let mode_str = match mode { - InstallMode::Install => "Installing", - InstallMode::Reinstall => "Reinstalling", - InstallMode::Upgrade { .. } => "Upgrading", - }; - if let Some(name) = canister_name { - info!( - logger, - "{mode_str} code for canister {name}, with canister ID {canister_id}", - ); - } else { - info!(logger, "{mode_str} code for canister {canister_id}"); - } + match call_sender { CallSender::SelectedId => { let install_builder = mgr diff --git a/src/dfx-core/src/config/model/canister_id_store.rs b/src/dfx-core/src/config/model/canister_id_store.rs index 68693ac0f1..4f014d08b4 100644 --- a/src/dfx-core/src/config/model/canister_id_store.rs +++ b/src/dfx-core/src/config/model/canister_id_store.rs @@ -250,6 +250,39 @@ impl CanisterIdStore { .or_else(|| self.find_in(canister_name, &self.ids)) .or_else(|| self.pull_ids.get(canister_name).copied()) } + pub fn get_name_id_map(&self) -> BTreeMap { + let mut ids: BTreeMap<_, _> = self + .ids + .iter() + .filter_map(|(name, network_to_id)| { + Some(( + name.clone(), + network_to_id.get(&self.network_descriptor.name).cloned()?, + )) + }) + .collect(); + if let Some(remote_ids) = &self.remote_ids { + let mut remote = remote_ids + .iter() + .filter_map(|(name, network_to_id)| { + Some(( + name.clone(), + network_to_id.get(&self.network_descriptor.name).cloned()?, + )) + }) + .collect(); + ids.append(&mut remote); + } + let mut pull_ids = self + .pull_ids + .iter() + .map(|(name, id)| (name.clone(), id.to_text())) + .collect(); + ids.append(&mut pull_ids); + ids.into_iter() + .filter(|(name, _)| !name.starts_with("__")) + .collect() + } fn find_in(&self, canister_name: &str, canister_ids: &CanisterIds) -> Option { canister_ids diff --git a/src/dfx-core/src/identity/identity_manager.rs b/src/dfx-core/src/identity/identity_manager.rs index f1a514c02e..3868fce7ad 100644 --- a/src/dfx-core/src/identity/identity_manager.rs +++ b/src/dfx-core/src/identity/identity_manager.rs @@ -72,6 +72,7 @@ use sec1::EncodeEcPrivateKey; use serde::{Deserialize, Serialize}; use slog::{debug, trace, Logger}; use std::boxed::Box; +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::str::FromStr; use thiserror::Error; @@ -668,6 +669,38 @@ impl IdentityManager { self.get_identity_dir_path(identity).join(IDENTITY_JSON) } + /// Returns a map of (name, principal) pairs for unencrypted identities. + /// In the future, we may refactor the code to include encrypted principals as well. + pub fn get_unencrypted_principal_map(&self, log: &Logger) -> BTreeMap { + use ic_agent::Identity; + let mut res = if let Ok(names) = self.get_identity_names(log) { + names + .iter() + .filter_map(|name| { + let config = self.get_identity_config_or_default(name).ok()?; + if let IdentityConfiguration { + encryption: None, + keyring_identity_suffix: None, + hsm: None, + } = config + { + let sender = self.load_identity(name, log).ok()?.sender().ok()?; + Some((name.clone(), sender.to_text())) + } else { + None + } + }) + .collect() + } else { + BTreeMap::new() + }; + res.insert( + ANONYMOUS_IDENTITY_NAME.to_string(), + Principal::anonymous().to_string(), + ); + res + } + pub fn get_identity_config_or_default( &self, identity: &str, diff --git a/src/dfx/Cargo.toml b/src/dfx/Cargo.toml index 752ca77d06..2e6cf98ad8 100644 --- a/src/dfx/Cargo.toml +++ b/src/dfx/Cargo.toml @@ -39,7 +39,7 @@ base64.workspace = true byte-unit = { workspace = true, features = ["serde"] } bytes.workspace = true candid = { workspace = true } -candid_parser = { workspace = true, features = ["random"] } +candid_parser = { workspace = true, features = ["random", "assist"] } clap = { workspace = true, features = [ "derive", "env", diff --git a/src/dfx/src/commands/canister/call.rs b/src/dfx/src/commands/canister/call.rs index 2f4ecc7d22..eff2eac2b3 100644 --- a/src/dfx/src/commands/canister/call.rs +++ b/src/dfx/src/commands/canister/call.rs @@ -301,7 +301,14 @@ pub async fn exec( // Get the argument, get the type, convert the argument to the type and return // an error if any of it doesn't work. - let arg_value = blob_from_arguments(arguments, opts.random.as_deref(), arg_type, &method_type)?; + let arg_value = blob_from_arguments( + Some(env), + arguments, + opts.random.as_deref(), + arg_type, + &method_type, + false, + )?; // amount has been validated by cycle_amount_validator let cycles = opts.with_cycles.unwrap_or(0); diff --git a/src/dfx/src/commands/canister/delete.rs b/src/dfx/src/commands/canister/delete.rs index 877d316ae6..e2341a8cb0 100644 --- a/src/dfx/src/commands/canister/delete.rs +++ b/src/dfx/src/commands/canister/delete.rs @@ -199,7 +199,7 @@ async fn delete_canister( "Installing temporary wallet in canister {} to enable transfer of cycles.", canister ); - let args = blob_from_arguments(None, None, None, &None)?; + let args = blob_from_arguments(None, None, None, None, &None, false)?; let mode = InstallMode::Reinstall; let install_builder = mgr .install_code(&canister_id, &wasm_module) diff --git a/src/dfx/src/commands/canister/install.rs b/src/dfx/src/commands/canister/install.rs index 136af25788..d0df15d64a 100644 --- a/src/dfx/src/commands/canister/install.rs +++ b/src/dfx/src/commands/canister/install.rs @@ -8,7 +8,7 @@ use crate::{ lib::canister_info::CanisterInfo, util::{arguments_from_file, blob_from_arguments}, }; -use dfx_core::canister::install_canister_wasm; +use dfx_core::canister::{install_canister_wasm, install_mode_to_prompt}; use dfx_core::identity::CallSender; use anyhow::{anyhow, bail, Context}; @@ -109,13 +109,19 @@ pub async fn exec( let arguments = opts.argument.as_deref(); let argument_from_cli = arguments_from_file.as_deref().or(arguments); let arg_type = opts.argument_type.as_deref(); - // `opts.canister` is a Principal (canister ID) if let Ok(canister_id) = Principal::from_text(canister) { if let Some(wasm_path) = &opts.wasm { - let args = blob_from_arguments(argument_from_cli, None, arg_type, &None)?; + let args = + blob_from_arguments(Some(env), argument_from_cli, None, arg_type, &None, true)?; let wasm_module = dfx_core::fs::read(wasm_path)?; let mode = mode.context("The install mode cannot be auto when using --wasm")?; + info!( + env.get_logger(), + "{} code for canister {}", + install_mode_to_prompt(&mode), + canister_id, + ); install_canister_wasm( env.get_agent(), canister_id, @@ -125,7 +131,6 @@ pub async fn exec( call_sender, wasm_module, opts.yes, - env.get_logger(), ) .await?; Ok(()) diff --git a/src/dfx/src/commands/canister/sign.rs b/src/dfx/src/commands/canister/sign.rs index 18bc363ae8..71e08dd533 100644 --- a/src/dfx/src/commands/canister/sign.rs +++ b/src/dfx/src/commands/canister/sign.rs @@ -126,7 +126,14 @@ pub async fn exec( // Get the argument, get the type, convert the argument to the type and return // an error if any of it doesn't work. - let arg_value = blob_from_arguments(arguments, opts.random.as_deref(), arg_type, &method_type)?; + let arg_value = blob_from_arguments( + Some(env), + arguments, + opts.random.as_deref(), + arg_type, + &method_type, + false, + )?; let agent = env.get_agent(); let network = env diff --git a/src/dfx/src/lib/integrations/mod.rs b/src/dfx/src/lib/integrations/mod.rs index 1184dfde07..ee35e4062d 100644 --- a/src/dfx/src/lib/integrations/mod.rs +++ b/src/dfx/src/lib/integrations/mod.rs @@ -49,7 +49,7 @@ pub async fn initialize_integration_canister( }; try_create_canister(agent, logger, &canister_id, &pulled_canister).await?; - let install_arg = blob_from_arguments(Some(init_arg), None, None, &None)?; + let install_arg = blob_from_arguments(None, Some(init_arg), None, None, &None, true)?; install_canister(agent, logger, &canister_id, wasm, install_arg, name).await } diff --git a/src/dfx/src/lib/operations/canister/install_canister.rs b/src/dfx/src/lib/operations/canister/install_canister.rs index 9ce65a47c1..2c2a1e948e 100644 --- a/src/dfx/src/lib/operations/canister/install_canister.rs +++ b/src/dfx/src/lib/operations/canister/install_canister.rs @@ -13,7 +13,7 @@ use anyhow::{anyhow, bail, Context}; use backoff::backoff::Backoff; use backoff::ExponentialBackoff; use candid::Principal; -use dfx_core::canister::{build_wallet_canister, install_canister_wasm}; +use dfx_core::canister::{build_wallet_canister, install_canister_wasm, install_mode_to_prompt}; use dfx_core::cli::ask_for_consent; use dfx_core::config::model::canister_id_store::CanisterIdStore; use dfx_core::config::model::network_descriptor::NetworkDescriptor; @@ -71,6 +71,12 @@ pub async fn install_canister( InstallMode::Install } }); + let mode_str = install_mode_to_prompt(&mode); + let canister_name = canister_info.get_name(); + info!( + log, + "{mode_str} code for canister {canister_name}, with canister ID {canister_id}", + ); if !skip_consent && matches!(mode, InstallMode::Reinstall | InstallMode::Upgrade { .. }) { let candid = read_module_metadata(agent, canister_id, "candid:service").await; if let Some(candid) = &candid { @@ -132,6 +138,7 @@ pub async fn install_canister( } else { get_candid_init_type(&idl_path) }; + // The argument and argument_type from the CLI take precedence over the `init_arg` field in dfx.json let argument_from_json = canister_info.get_init_arg(); let (argument, argument_type) = match (argument_from_cli, argument_from_json) { @@ -157,7 +164,8 @@ The command line value will be used.", (None, Some(_)) => (argument_from_json, Some("idl")), // `init_arg` in dfx.json is always in Candid format (None, None) => (None, None), }; - let install_args = blob_from_arguments(argument, None, argument_type, &init_type)?; + let install_args = + blob_from_arguments(Some(env), argument, None, argument_type, &init_type, true)?; if let Some(timestamp) = canister_id_store.get_timestamp(canister_info.get_name()) { let new_timestamp = playground_install_code( env, @@ -184,7 +192,6 @@ The command line value will be used.", call_sender, wasm_module, skip_consent, - env.get_logger(), ) .await?; } diff --git a/src/dfx/src/util/mod.rs b/src/dfx/src/util/mod.rs index 19d2ade25b..a2289435ca 100644 --- a/src/dfx/src/util/mod.rs +++ b/src/dfx/src/util/mod.rs @@ -1,3 +1,4 @@ +use crate::lib::environment::Environment; use crate::lib::error::DfxResult; use crate::{error_invalid_argument, error_invalid_data, error_unknown}; use anyhow::{bail, Context}; @@ -14,7 +15,8 @@ use num_traits::FromPrimitive; use reqwest::{Client, StatusCode, Url}; use rust_decimal::Decimal; use socket2::{Domain, Socket}; -use std::io::{stdin, Read}; +use std::collections::BTreeMap; +use std::io::{stderr, stdin, stdout, IsTerminal, Read}; use std::net::{IpAddr, SocketAddr, TcpListener}; use std::path::Path; use std::time::Duration; @@ -170,10 +172,12 @@ pub fn arguments_from_file(file_name: &Path) -> DfxResult { #[context("Failed to create argument blob.")] pub fn blob_from_arguments( + dfx_env: Option<&dyn Environment>, arguments: Option<&str>, random: Option<&str>, arg_type: Option<&str>, method_type: &Option<(TypeEnv, Function)>, + is_init_arg: bool, ) -> DfxResult> { let arg_type = arg_type.unwrap_or("idl"); match arg_type { @@ -193,6 +197,8 @@ pub fn blob_from_arguments( .to_bytes() } Some((env, func)) => { + let is_terminal = + stdin().is_terminal() && stdout().is_terminal() && stderr().is_terminal(); if let Some(arguments) = arguments { fuzzy_parse_argument(arguments, env, &func.args) } else if func.args.is_empty() { @@ -224,6 +230,37 @@ pub fn blob_from_arguments( .context("Failed to create idl args.")?; eprintln!("Sending the following random argument:\n{}\n", args); args.to_bytes_with_types(env, &func.args) + } else if is_terminal { + use candid_parser::assist::{input_args, Context}; + let mut ctx = Context::new(env.clone()); + if let Some(env) = dfx_env { + let principals = gather_principals_from_env(env); + if !principals.is_empty() { + let mut map = BTreeMap::new(); + map.insert("principal".to_string(), principals); + ctx.set_completion(map); + } + } + if is_init_arg { + eprintln!("This canister requires an initialization argument."); + } else { + eprintln!("This method requires arguments."); + } + let args = input_args(&ctx, &func.args)?; + eprintln!("Sending the following argument:\n{}\n", args); + if is_init_arg { + eprintln!( + "Do you want to initialize the canister with this argument? [y/N]" + ); + } else { + eprintln!("Do you want to send this message? [y/N]"); + } + let mut input = String::new(); + stdin().read_line(&mut input)?; + if !["y", "Y", "yes", "Yes", "YES"].contains(&input.trim()) { + return Err(error_invalid_data!("User cancelled.")); + } + args.to_bytes_with_types(env, &func.args) } else { return Err(error_invalid_data!("Expected arguments but found none.")); } @@ -236,6 +273,20 @@ pub fn blob_from_arguments( } } +pub fn gather_principals_from_env(env: &dyn Environment) -> BTreeMap { + let mut res: BTreeMap = BTreeMap::new(); + if let Ok(mgr) = env.new_identity_manager() { + let logger = env.get_logger(); + let mut map = mgr.get_unencrypted_principal_map(logger); + res.append(&mut map); + } + if let Ok(canisters) = env.get_canister_id_store() { + let mut canisters = canisters.get_name_id_map(); + res.append(&mut canisters); + } + res +} + pub fn fuzzy_parse_argument( arg_str: &str, env: &TypeEnv,