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

feat: Candid assist #3546

Merged
merged 17 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

# UNRELEASED

### feat: candid assist feature
ericswanson-dfinity marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions e2e/assets/echo/echo.mo
Original file line number Diff line number Diff line change
@@ -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
}
};
1 change: 1 addition & 0 deletions e2e/assets/echo/patch.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
jq '.canisters.hello_backend.main="echo.mo"' dfx.json | sponge dfx.json
59 changes: 59 additions & 0 deletions e2e/assets/expect_scripts/candid_assist.exp
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions e2e/tests-dfx/call.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have one of these for deploy and install as well?

Here is the output I see when deploying to a canister declared as actor class c(p: { x: Nat; y: Int;}) :

Installing canisters...
Installing code for canister dd_backend, with canister ID bkyz2-fmaaa-aaaaa-qaaaq-cai
Enter field x : nat
  ✔ Enter a nat · 6
Enter field y : int
  ✔ Enter an int · 8
Sending the following argument:
(record { x = 6 : nat; y = 8 : int })

Do you want to send this message? [y/N]

Suggested wording:

Installing canisters...
Installing code for canister dd_backend, with canister ID bkyz2-fmaaa-aaaaa-qaaaq-cai

Canister dd_backend requires an initialization argument.
Enter field x : nat
  ✔ Enter a nat · 6
Enter field y : int
  ✔ Enter an int · 8

Initialization argument for dd_backend will be:
(record { x = 6 : nat; y = 8 : int })

Do you want to initialize canister dd_backend with this argument? [y/N]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I see now, when deploying a canister with declaration actor class c(x: Principal) {

Installing code for canister dd_backend, with canister ID bkyz2-fmaaa-aaaaa-qaaaq-cai
This canister requires the following initialization argument.
Auto-completions: alice, anonymous, bob, dd_backend, dd_frontend, default, mainnet
? Enter a principal › 

Note "This canister requires the following initialization argument." but no following description of the argument. The term "requires the following initialization argument" would have to refer to a "following" description of that argument. It can't be used to refer only to the upcoming prompt for that argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed back to "This canister requires an initialization argument."

@test "call subcommand accepts canister identifier as canister name" {
install_asset greet
dfx_start
Expand Down
3 changes: 2 additions & 1 deletion scripts/workflows/provision-linux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 9 additions & 15 deletions src/dfx-core/src/canister/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use ic_utils::{
},
Argument,
};
use slog::{info, Logger};

pub async fn build_wallet_canister(
id: Principal,
Expand All @@ -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,
Expand All @@ -38,7 +45,6 @@ pub async fn install_canister_wasm(
call_sender: &CallSender,
wasm_module: Vec<u8>,
skip_consent: bool,
logger: &Logger,
) -> Result<(), CanisterInstallError> {
let mgr = ManagementCanister::create(agent);
if !skip_consent && mode == InstallMode::Reinstall {
Expand All @@ -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
Expand Down
33 changes: 33 additions & 0 deletions src/dfx-core/src/config/model/canister_id_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> {
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<CanisterId> {
canister_ids
Expand Down
33 changes: 33 additions & 0 deletions src/dfx-core/src/identity/identity_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, String> {
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,
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 8 additions & 1 deletion src/dfx/src/commands/canister/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/commands/canister/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 9 additions & 4 deletions src/dfx/src/commands/canister/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -125,7 +131,6 @@ pub async fn exec(
call_sender,
wasm_module,
opts.yes,
env.get_logger(),
)
.await?;
Ok(())
Expand Down
9 changes: 8 additions & 1 deletion src/dfx/src/commands/canister/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/lib/integrations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading
Loading