Skip to content

Commit

Permalink
Merge branch 'master' into ens/sdk-601-doc-identity-export
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Jan 23, 2024
2 parents f32315f + 935ffc7 commit eff7c13
Show file tree
Hide file tree
Showing 23 changed files with 183 additions and 91 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

# UNRELEASED

### fix!: always fetch did file from canister when making canister calls

`dfx canister call` will always fetch did file from the canister metadata. This is especially helpful for calling remote canisters. It's a breaking change in the sense that if the canister doesn't have the `candid:service` metadata, we will not read the local did file from build artifact, and dfx will issue a warning in this case to encourage canister developers to put the did file into canister metadata.

### fix: support `import` for local did file

If the local did file contains `import` or init args, dfx will rewrite the did file when storing in canister metadata.
Due to current limitations of the Candid parser, comments will be dropped during rewriting.
If the local did file doesn't contain `import` or init args, we will not perform the rewriting, thus preserving the comments.

### fix: subtyping check reports the special opt rule as error

# 0.16.0

### feat: large canister modules now supported
Expand Down
9 changes: 9 additions & 0 deletions e2e/assets/prebuilt_custom_canister/dfx.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@
"candid": "custom_with_build_step.did",
"wasm": "main.wasm",
"build": "echo just a build step"
},
"prebuilt_local_import": {
"type": "custom",
"candid": "local_import.did",
"wasm": "main.wasm",
"build": [],
"metadata": [
{ "name": "candid:service" }
]
}
},
"networks": {
Expand Down
2 changes: 2 additions & 0 deletions e2e/assets/prebuilt_custom_canister/local_import.did
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import service "prebuilt_custom_no_build.did";

3 changes: 3 additions & 0 deletions e2e/assets/upgrade/v1.mo
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ actor {
state += 1;
return state;
};
public func f() : async ?Int {
return ?42;
};
public query func read() : async Int { return state; };
}

3 changes: 3 additions & 0 deletions e2e/assets/upgrade/v2.mo
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ actor {
new_state += 1;
return new_state;
};
public func f() : async ?Int {
return ?42;
};
public query func read() : async Nat { return new_state; };
}

3 changes: 3 additions & 0 deletions e2e/assets/upgrade/v2_bad.mo
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ actor {
state += 1;
return state;
};
public func f() : async ?Int {
return ?42;
};
public query func read() : async Nat { return state; };
}

3 changes: 3 additions & 0 deletions e2e/assets/upgrade/v3_bad.mo
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ actor {
state += 1;
return state;
};
public func f() : async ?Int {
return ?42;
};
public query func read2() : async Int { return state; };
}

12 changes: 12 additions & 0 deletions e2e/assets/upgrade/v4_bad.mo
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
actor {
stable var state : Int = 0;
public func inc() : async Int {
state += 1;
return state;
};
public func f() : async ?Text {
return ?"";
};
public query func read() : async Int { return state; };
}

17 changes: 11 additions & 6 deletions e2e/tests-dfx/assetscanister.bash
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ check_permission_failure() {
FE_CANISTER_ID="$(dfx canister id e2e_project_frontend)"
rm .dfx/local/canister_ids.json
assert_command_fail dfx canister call "$FE_CANISTER_ID" validate_revoke_permission "(record { of_principal=principal \"$PREPARE_PRINCIPAL\"; permission = variant { FlyBeFree }; })"
assert_contains "trapped"
assert_contains "FlyBeFree not found"
}

@test "access control - fine-grained" {
Expand Down Expand Up @@ -905,24 +905,29 @@ check_permission_failure() {
dfx canister install e2e_project_frontend

assert_command dfx canister call --query e2e_project_frontend retrieve '("/binary/noise.txt")' --output idl
assert_eq '(blob "\b8\01\20\80\0a\77\31\32\20\00\78\79\0a\4b\4c\0b\0a\6a\6b")'
# shellcheck disable=SC2154
assert_eq '(blob "\b8\01\20\80\0a\77\31\32\20\00\78\79\0a\4b\4c\0b\0a\6a\6b")' "$stdout"

assert_command dfx canister call --query e2e_project_frontend retrieve '("/text-with-newlines.txt")' --output idl
assert_eq '(blob "cherries\0ait\27s cherry season\0aCHERRIES")'
# shellcheck disable=SC2154
assert_eq '(blob "cherries\0ait\27s cherry season\0aCHERRIES")' "$stdout"

assert_command dfx canister call --update e2e_project_frontend store '(record{key="AA"; content_type="text/plain"; content_encoding="identity"; content=blob "hello, world!"})'
assert_eq '()'
assert_command dfx canister call --update e2e_project_frontend store '(record{key="B"; content_type="application/octet-stream"; content_encoding="identity"; content=blob"XWV"})'
assert_eq '()'

assert_command dfx canister call --query e2e_project_frontend retrieve '("B")' --output idl
assert_eq '(blob "XWV")'
# shellcheck disable=SC2154
assert_eq '(blob "XWV")' "$stdout"

assert_command dfx canister call --query e2e_project_frontend retrieve '("AA")' --output idl
assert_eq '(blob "hello, world!")'
# shellcheck disable=SC2154
assert_eq '(blob "hello, world!")' "$stdout"

assert_command dfx canister call --query e2e_project_frontend retrieve '("B")' --output idl
assert_eq '(blob "XWV")'
# shellcheck disable=SC2154
assert_eq '(blob "XWV")' "$stdout"

assert_command_fail dfx canister call --query e2e_project_frontend retrieve '("C")'
}
Expand Down
6 changes: 4 additions & 2 deletions e2e/tests-dfx/build_granular.bash
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,12 @@ teardown() {
dfx canister install e2e_project_frontend

assert_command dfx canister call --query e2e_project_frontend retrieve '("/binary/noise.txt")' --output idl
assert_eq '(blob "\b8\01\20\80\0a\77\31\32\20\00\78\79\0a\4b\4c\0b\0a\6a\6b")'
# shellcheck disable=SC2154
assert_eq '(blob "\b8\01\20\80\0a\77\31\32\20\00\78\79\0a\4b\4c\0b\0a\6a\6b")' "$stdout"

assert_command dfx canister call --query e2e_project_frontend retrieve '("/text-with-newlines.txt")' --output idl
assert_eq '(blob "cherries\0ait\27s cherry season\0aCHERRIES")'
# shellcheck disable=SC2154
assert_eq '(blob "cherries\0ait\27s cherry season\0aCHERRIES")' "$stdout"
}

@test "cyclic dependencies are detected" {
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests-dfx/call.bash
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ teardown() {
CANISTER_ID=$(dfx canister id hello_backend)
rm .dfx/local/canister_ids.json

# if no candid file known, then no field names
# given a canister id, fetch the did file from metadata
assert_command dfx canister call "$CANISTER_ID" make_struct '("A", "B")'
assert_eq '(record { 99 = "A"; 100 = "B" })'
assert_eq '(record { c = "A"; d = "B" })'

# if passing the candid file, field names available
assert_command dfx canister call --candid .dfx/local/canisters/hello_backend/hello_backend.did "$CANISTER_ID" make_struct '("A", "B")'
Expand Down
6 changes: 4 additions & 2 deletions e2e/tests-dfx/identity.bash
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ teardown() {
assert_command dfx canister call e2e_project_frontend store '(record{key="B"; content_type="application/octet-stream"; content_encoding="identity"; content=blob"XWV"})' --identity alice
assert_eq '()'
assert_command dfx canister call --output idl e2e_project_frontend retrieve '("B")'
assert_eq '(blob "XWV")'
# shellcheck disable=SC2154
assert_eq '(blob "XWV")' "$stdout"
}

@test "after renaming an identity, the renamed identity is still initializer" {
Expand All @@ -181,7 +182,8 @@ teardown() {
assert_command dfx canister call e2e_project_frontend store '(record{key="B"; content_type="application/octet-stream"; content_encoding="identity"; content=blob "hello"})' --identity bob
assert_eq '()'
assert_command dfx canister call --output idl e2e_project_frontend retrieve '("B")'
assert_eq '(blob "hello")'
# shellcheck disable=SC2154
assert_eq '(blob "hello")' "$stdout"
}

@test "using an unencrypted identity on mainnet provokes a warning" {
Expand Down
4 changes: 4 additions & 0 deletions e2e/tests-dfx/metadata.bash
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ teardown() {
# this canister has a build step, but it is an empty array, so dfx leaves the candid:service metadata as-is
dfx canister metadata prebuilt_custom_empty_build candid:service >from_canister.txt
diff main.did from_canister.txt

# this canister has a local import in did file, the metadata should flatten the definitions
assert_command dfx canister metadata prebuilt_local_import candid:service
assert_eq "service : { getCanisterId : () -> (principal) query }"
}

@test "can read canister metadata from replica" {
Expand Down
14 changes: 14 additions & 0 deletions e2e/tests-dfx/upgrade_check.bash
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,17 @@ teardown() {
assert_command dfx canister call hello_backend read2 '()'
assert_match "(1 : int)"
}

@test "warning for using special opt rule" {
install_asset upgrade
dfx_start
dfx deploy
dfx canister call hello_backend f '()'
jq '.canisters.hello_backend.main="v4_bad.mo"' dfx.json | sponge dfx.json
echo yes | (
assert_command dfx deploy
assert_match "Candid interface compatibility check failed"
)
assert_command dfx canister call hello_backend f '()'
assert_match "(opt \"\")"
}
6 changes: 4 additions & 2 deletions e2e/tests-dfx/wallet.bash
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ teardown() {

assert_command dfx canister call "$WALLET" wallet_call \
"(record { canister = principal \"$(dfx canister id e2e_project_backend)\"; method_name = \"amInitializer\"; args = blob \"DIDL\00\00\"; cycles = (0:nat64)})"
assert_eq '(variant { 17_724 = record { 153_986_224 = blob "\44\49\44\4c\00\01\7e\01" } })' # True in DIDL.
# shellcheck disable=SC2154
assert_eq '(variant { 17_724 = record { 153_986_224 = blob "\44\49\44\4c\00\01\7e\01" } })' "$stdout" # True in DIDL.
}

@test "forward user call through wallet: deploy" {
Expand All @@ -147,7 +148,8 @@ teardown() {
assert_command dfx canister call e2e_project_backend amInitializer
assert_command dfx canister call "$WALLET" wallet_call \
"(record { canister = principal \"$(dfx canister id e2e_project_backend)\"; method_name = \"amInitializer\"; args = blob \"DIDL\00\00\"; cycles = (0:nat64)})"
assert_eq '(variant { 17_724 = record { 153_986_224 = blob "\44\49\44\4c\00\01\7e\01" } })' # True in DIDL.
# shellcheck disable=SC2154
assert_eq '(variant { 17_724 = record { 153_986_224 = blob "\44\49\44\4c\00\01\7e\01" } })' "$stdout" # True in DIDL.
}

@test "a 64-bit wallet can still be called in the 128-bit context" {
Expand Down
37 changes: 20 additions & 17 deletions src/dfx/src/commands/canister/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ use crate::lib::error::DfxResult;
use crate::lib::operations::canister::get_local_cid_and_candid_path;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::clap::parsers::{cycle_amount_parser, file_or_stdin_parser};
use crate::util::{arguments_from_file, blob_from_arguments, get_candid_type, print_idl_blob};
use crate::util::{
arguments_from_file, blob_from_arguments, get_candid_type, print_idl_blob, read_module_metadata,
};
use anyhow::{anyhow, Context};
use candid::Principal as CanisterId;
use candid::{CandidType, Decode, Deserialize, Principal};
use candid_parser::utils::CandidSource;
use clap::Parser;
use dfx_core::canister::build_wallet_canister;
use dfx_core::identity::CallSender;
Expand Down Expand Up @@ -218,29 +221,32 @@ pub async fn exec(
opts: CanisterCallOpts,
call_sender: &CallSender,
) -> DfxResult {
let agent = env.get_agent();
fetch_root_key_if_needed(env).await?;

let callee_canister = opts.canister_name.as_str();
let method_name = opts.method_name.as_str();
let canister_id_store = env.get_canister_id_store()?;

let (canister_id, maybe_candid_path) = match CanisterId::from_text(callee_canister) {
Ok(id) => {
if let Some(canister_name) = canister_id_store.get_name(callee_canister) {
get_local_cid_and_candid_path(env, canister_name, Some(id))?
} else {
// TODO fetch candid file from remote canister
(id, None)
}
}
let canister_id = match CanisterId::from_text(callee_canister) {
Ok(id) => id,
Err(_) => {
let canister_id = canister_id_store.get(callee_canister)?;
get_local_cid_and_candid_path(env, callee_canister, Some(canister_id))?
get_local_cid_and_candid_path(env, callee_canister, Some(canister_id))?.0
}
};
let maybe_candid_path = opts.candid.or(maybe_candid_path);
let method_type = if let Some(path) = opts.candid {
get_candid_type(CandidSource::File(&path), method_name)
} else {
read_module_metadata(agent, canister_id, "candid:service")
.await
.and_then(|did| get_candid_type(CandidSource::Text(&did), method_name))
};
if method_type.is_none() {
eprintln!("Cannot fetch Candid interface from canister metadata, sending arguments with inferred types.");
}

let is_management_canister = canister_id == CanisterId::management_canister();

let method_type = maybe_candid_path.and_then(|path| get_candid_type(&path, method_name));
let is_query_method = method_type.as_ref().map(|(_, f)| f.is_query());

let arguments_from_file = opts
Expand Down Expand Up @@ -275,9 +281,6 @@ 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 agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

// amount has been validated by cycle_amount_validator
let cycles = opts.with_cycles.unwrap_or(0);
Expand Down
6 changes: 4 additions & 2 deletions src/dfx/src/commands/canister/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::util::clap::parsers::file_or_stdin_parser;
use crate::util::{arguments_from_file, blob_from_arguments, get_candid_type};
use anyhow::{anyhow, bail, Context};
use candid::Principal;
use candid_parser::utils::CandidSource;
use clap::Parser;
use dfx_core::identity::CallSender;
use ic_agent::AgentError;
Expand Down Expand Up @@ -86,7 +87,7 @@ pub async fn exec(
if let Some(canister_name) = canister_id_store.get_name(callee_canister) {
get_local_cid_and_candid_path(env, canister_name, Some(id))?
} else {
// TODO fetch candid file from remote canister
// Sign works in offline mode, cannot fetch candid file from remote canister
(id, None)
}
}
Expand All @@ -96,7 +97,8 @@ pub async fn exec(
}
};

let method_type = maybe_candid_path.and_then(|path| get_candid_type(&path, method_name));
let method_type =
maybe_candid_path.and_then(|path| get_candid_type(CandidSource::File(&path), method_name));
let is_query_method = method_type.as_ref().map(|(_, f)| f.is_query());

let arguments_from_file = opts
Expand Down
6 changes: 3 additions & 3 deletions src/dfx/src/commands/deps/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ use crate::lib::deps::{
};
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::util::{check_candid_file, fuzzy_parse_argument};
use crate::util::fuzzy_parse_argument;
use anyhow::{anyhow, bail};
use candid::Principal;
use candid_parser::{types::IDLTypes, typing::ast_to_type};
use candid_parser::{types::IDLTypes, typing::ast_to_type, utils::CandidSource};
use clap::Parser;
use slog::{info, warn, Logger};

Expand Down Expand Up @@ -68,7 +68,7 @@ fn set_init(
.ok_or_else(|| anyhow!("Failed to find {canister_id} entry in pulled.json"))?;
let canister_prompt = get_canister_prompt(canister_id, pulled_canister);
let idl_path = get_pulled_service_candid_path(canister_id)?;
let (env, _) = check_candid_file(&idl_path)?;
let (env, _) = CandidSource::File(&idl_path).load()?;
let candid_args = pulled_json.get_candid_args(canister_id)?;
let candid_args_idl_types: IDLTypes = candid_args.parse()?;
let mut types = vec![];
Expand Down
4 changes: 2 additions & 2 deletions src/dfx/src/commands/remote/generate_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::lib::agent::create_agent_environment;
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::models::canister::CanisterPool;
use crate::util::check_candid_file;
use anyhow::Context;
use candid_parser::utils::CandidSource;
use clap::Parser;
use slog::info;

Expand Down Expand Up @@ -70,7 +70,7 @@ pub fn exec(env: &dyn Environment, opts: GenerateBindingOpts) -> DfxResult {
continue;
}
}
let (type_env, did_types) = check_candid_file(&candid)?;
let (type_env, did_types) = CandidSource::File(&candid).load()?;
let extension = main.extension().unwrap_or_default();
let bindings = if extension == "mo" {
Some(candid_parser::bindings::motoko::compile(
Expand Down
4 changes: 2 additions & 2 deletions src/dfx/src/lib/builders/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use crate::lib::canister_info::CanisterInfo;
use crate::lib::environment::Environment;
use crate::lib::error::{BuildError, DfxError, DfxResult};
use crate::lib::models::canister::CanisterPool;
use crate::util::check_candid_file;
use anyhow::{anyhow, bail, Context};
use candid::Principal as CanisterId;
use candid_parser::utils::CandidSource;
use dfx_core::config::model::dfinity::{Config, Profile};
use dfx_core::network::provider::get_network_context;
use dfx_core::util;
Expand Down Expand Up @@ -150,7 +150,7 @@ pub trait CanisterBuilder {

let generated_idl_path = self.generate_idl(pool, info, config)?;

let (env, ty) = check_candid_file(generated_idl_path.as_path())?;
let (env, ty) = CandidSource::File(generated_idl_path.as_path()).load()?;

// Typescript
if bindings.contains(&"ts".to_string()) {
Expand Down
Loading

0 comments on commit eff7c13

Please sign in to comment.