From c86cc28f18d3735aeed9b36730a08871fbcde967 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Wed, 24 May 2023 16:19:03 -0400 Subject: [PATCH 1/5] post process candid --- e2e/tests-dfx/build.bash | 2 +- src/dfx/src/commands/canister/install.rs | 2 +- src/dfx/src/lib/canister_info.rs | 23 ++- src/dfx/src/lib/models/canister.rs | 160 ++++++++++++------ .../operations/canister/deploy_canisters.rs | 2 +- .../operations/canister/install_canister.rs | 6 +- 6 files changed, 140 insertions(+), 55 deletions(-) diff --git a/e2e/tests-dfx/build.bash b/e2e/tests-dfx/build.bash index b8b77bf604..a3ff9bc835 100644 --- a/e2e/tests-dfx/build.bash +++ b/e2e/tests-dfx/build.bash @@ -237,7 +237,7 @@ teardown() { # This test makes sure that the file is created under the .dfx/ directory, # which is where other temporary / build artifacts go. assert_file_not_exists ./main.old.did - assert_file_exists .dfx/local/canisters/custom/custom.old.did + assert_file_exists .dfx/local/canisters/custom/constructor.old.did } @test "custom canister build script picks local executable first" { diff --git a/src/dfx/src/commands/canister/install.rs b/src/dfx/src/commands/canister/install.rs index 2e436f802e..683a5938a9 100644 --- a/src/dfx/src/commands/canister/install.rs +++ b/src/dfx/src/commands/canister/install.rs @@ -143,7 +143,7 @@ pub async fn exec( let env_file = opts .output_env_file .or_else(|| config.get_config().output_env_file.clone()); - let idl_path = canister_info.get_build_idl_path(); + let idl_path = canister_info.get_constructor_idl_path(); let init_type = get_candid_init_type(&idl_path); let install_args = || blob_from_arguments(arguments, None, arg_type, &init_type); install_canister( diff --git a/src/dfx/src/lib/canister_info.rs b/src/dfx/src/lib/canister_info.rs index d301e26504..151edcb3df 100644 --- a/src/dfx/src/lib/canister_info.rs +++ b/src/dfx/src/lib/canister_info.rs @@ -266,14 +266,33 @@ impl CanisterInfo { self.output_root.join(&self.name).with_extension(ext) } - pub fn get_build_idl_path(&self) -> PathBuf { - self.output_root.join(&self.name).with_extension("did") + /// Path to the candid file which contains no init types. + /// + /// To be imported by dependents. + pub fn get_service_idl_path(&self) -> PathBuf { + self.output_root.join("service.did") + } + + /// Path to the candid file which contains init types. + /// + /// To be used when installing the canister. + pub fn get_constructor_idl_path(&self) -> PathBuf { + self.output_root.join("constructor.did") + } + + /// Path to the init_args.txt file which only contains init types. + /// + pub fn get_init_args_txt_path(&self) -> PathBuf { + self.output_root.join("init_args.txt") } pub fn get_index_js_path(&self) -> PathBuf { self.output_root.join("index").with_extension("js") } + /// Path to the candid file from canister builder which should contain init types. + /// + /// To be separated into service.did and init_args. pub fn get_output_idl_path(&self) -> Option { match &self.type_specific { CanisterTypeProperties::Motoko { .. } => self diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index e2316ef21e..ad687eb79f 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -6,7 +6,7 @@ use crate::lib::canister_info::CanisterInfo; use crate::lib::environment::Environment; use crate::lib::error::{BuildError, DfxError, DfxResult}; use crate::lib::metadata::dfx::DfxMetadata; -use crate::lib::metadata::names::{CANDID_SERVICE, DFX}; +use crate::lib::metadata::names::{CANDID_ARGS, CANDID_SERVICE, DFX}; use crate::lib::wasm::file::{compress_bytes, read_wasm_module}; use crate::util::{assets, check_candid_file}; use dfx_core::config::model::canister_id_store::CanisterIdStore; @@ -142,17 +142,14 @@ impl Canister { // metadata trace!(logger, "Attaching metadata"); - // Default to write public candid:service unless overwritten let mut metadata_sections = info.metadata().sections.clone(); - if (info.is_rust() || info.is_motoko()) && !metadata_sections.contains_key(CANDID_SERVICE) { - metadata_sections.insert( - CANDID_SERVICE.to_string(), - CanisterMetadataSection { - name: CANDID_SERVICE.to_string(), - visibility: MetadataVisibility::Public, - ..Default::default() - }, - ); + // Default to write public candid:service unless overwritten + let mut public_candid = false; + if (info.is_rust() || info.is_motoko()) + && !metadata_sections.contains_key(CANDID_SERVICE) + && !metadata_sections.contains_key(CANDID_ARGS) + { + public_candid = true; } if let Some(pullable) = info.get_pullable() { @@ -160,7 +157,6 @@ impl Canister { dfx_metadata.set_pullable(pullable); let content = serde_json::to_string_pretty(&dfx_metadata) .with_context(|| "Failed to serialize `dfx` metadata.".to_string())?; - metadata_sections.insert( DFX.to_string(), CanisterMetadataSection { @@ -170,13 +166,33 @@ impl Canister { ..Default::default() }, ); + public_candid = true; + } + + if public_candid { + metadata_sections.insert( + CANDID_SERVICE.to_string(), + CanisterMetadataSection { + name: CANDID_SERVICE.to_string(), + visibility: MetadataVisibility::Public, + ..Default::default() + }, + ); + + metadata_sections.insert( + CANDID_ARGS.to_string(), + CanisterMetadataSection { + name: CANDID_ARGS.to_string(), + visibility: MetadataVisibility::Public, + ..Default::default() + }, + ); } - let IdlBuildOutput::File(build_idl_path) = &build_output.idl; for (name, section) in &metadata_sections { if section.name == CANDID_SERVICE && info.is_motoko() { if let Some(specified_path) = §ion.path { - check_valid_subtype(build_idl_path, specified_path)? + check_valid_subtype(&info.get_service_idl_path(), specified_path)? } else { // Motoko compiler handles this continue; @@ -185,7 +201,10 @@ impl Canister { let data = match (section.path.as_ref(), section.content.as_ref()) { (None, None) if section.name == CANDID_SERVICE => { - dfx_core::fs::read(build_idl_path)? + dfx_core::fs::read(&info.get_service_idl_path())? + } + (None, None) if section.name == CANDID_ARGS => { + dfx_core::fs::read(&info.get_init_args_txt_path())? } (Some(path), None) => dfx_core::fs::read(path)?, (None, Some(s)) => s.clone().into_bytes(), @@ -235,6 +254,77 @@ impl Canister { Ok(()) } + + pub(crate) fn candid_post_process( + &self, + logger: &Logger, + build_config: &BuildConfig, + build_output: &BuildOutput, + ) -> DfxResult { + trace!(logger, "Post processing candid file"); + + let IdlBuildOutput::File(build_idl_path) = &build_output.idl; + + // 1. Copy the complete IDL file to .dfx/local/canisters/NAME/constructor.did. + let constructor_idl_path = self.info.get_constructor_idl_path(); + dfx_core::fs::composite::ensure_parent_dir_exists(&constructor_idl_path)?; + dfx_core::fs::copy(build_idl_path, &constructor_idl_path)?; + set_perms_readwrite(&constructor_idl_path)?; + + // 2. Separate into service.did and init_args + let (service_did, init_args) = separate_candid(build_idl_path)?; + + // 3. Save service.did into following places in .dfx/local/: + // - canisters/NAME/service.did + // - IDL_ROOT/CANISTER_ID.did + // - LSP_ROOT/CANISTER_ID.did + let mut targets = vec![]; + targets.push(self.info.get_service_idl_path()); + let canister_id = self.canister_id(); + targets.push( + build_config + .idl_root + .join(canister_id.to_text()) + .with_extension("did"), + ); + targets.push( + build_config + .lsp_root + .join(canister_id.to_text()) + .with_extension("did"), + ); + + for target in targets { + if &target == build_idl_path { + continue; + } + dfx_core::fs::composite::ensure_parent_dir_exists(&target)?; + dfx_core::fs::write(&target, &service_did)?; + set_perms_readwrite(&target)?; + } + + // 4. Save init_args into .dfx/local/canisters/NAME/init_args.txt + let init_args_txt_path = self.info.get_init_args_txt_path(); + dfx_core::fs::composite::ensure_parent_dir_exists(&init_args_txt_path)?; + dfx_core::fs::write(&init_args_txt_path, &init_args)?; + set_perms_readwrite(&init_args_txt_path)?; + Ok(()) + } +} + +fn separate_candid(path: &Path) -> DfxResult<(String, String)> { + let (env, actor) = check_candid_file(path)?; + let (actor, args) = if let Some(candid::types::internal::Type::Class(args, ty)) = actor { + (Some(*ty), args) + } else { + (actor, vec![]) + }; + let service_did = candid::bindings::candid::compile(&env, &actor); + use candid::bindings::candid::pp_ty; + use candid::pretty::{concat, enclose}; + let doc = concat(args.iter().map(pp_ty), ","); + let init_args = enclose("(", doc, ")").pretty(80).to_string(); + Ok((service_did, init_args)) } #[context("{} is not a valid subtype of {}", specified_idl_path.display(), compiled_idl_path.display())] @@ -470,37 +560,9 @@ impl CanisterPool { canister: &Canister, build_output: &BuildOutput, ) -> DfxResult<()> { - canister.wasm_post_process(self.get_logger(), build_output)?; + canister.candid_post_process(self.get_logger(), build_config, build_output)?; - // Copy the IDL file to three places in .dfx/local/: - // 1. canisters/NAME/NAME.did - // 2. IDL_ROOT/CANISTER_ID.did - // 3. LSP_ROOT/CANISTER_ID.did - let IdlBuildOutput::File(build_idl_path) = &build_output.idl; - let mut targets = vec![]; - targets.push(canister.info.get_build_idl_path()); - let canister_id = canister.canister_id(); - targets.push( - build_config - .idl_root - .join(canister_id.to_text()) - .with_extension("did"), - ); - targets.push( - build_config - .lsp_root - .join(canister_id.to_text()) - .with_extension("did"), - ); - - for target in targets { - if &target == build_idl_path { - continue; - } - dfx_core::fs::composite::ensure_parent_dir_exists(&target)?; - dfx_core::fs::copy(build_idl_path, &target)?; - set_perms_readwrite(&target)?; - } + canister.wasm_post_process(self.get_logger(), build_output)?; build_canister_js(&canister.canister_id(), &canister.info)?; @@ -709,12 +771,14 @@ fn decode_path_to_str(path: &Path) -> DfxResult<&str> { /// Create a canister JavaScript DID and Actor Factory. #[context("Failed to build canister js for canister '{}'.", canister_info.get_name())] fn build_canister_js(canister_id: &CanisterId, canister_info: &CanisterInfo) -> DfxResult { - let output_did_js_path = canister_info.get_build_idl_path().with_extension("did.js"); + let output_did_js_path = canister_info + .get_service_idl_path() + .with_extension("did.js"); let output_did_ts_path = canister_info - .get_build_idl_path() + .get_service_idl_path() .with_extension("did.d.ts"); - let (env, ty) = check_candid_file(&canister_info.get_build_idl_path())?; + let (env, ty) = check_candid_file(&canister_info.get_service_idl_path())?; let content = ensure_trailing_newline(candid::bindings::javascript::compile(&env, &ty)); std::fs::write(&output_did_js_path, content).with_context(|| { format!( diff --git a/src/dfx/src/lib/operations/canister/deploy_canisters.rs b/src/dfx/src/lib/operations/canister/deploy_canisters.rs index ad1429b23b..92accfc308 100644 --- a/src/dfx/src/lib/operations/canister/deploy_canisters.rs +++ b/src/dfx/src/lib/operations/canister/deploy_canisters.rs @@ -307,7 +307,7 @@ async fn install_canisters( let canister_id = canister_id_store.get(canister_name)?; let canister_info = CanisterInfo::load(config, canister_name, Some(canister_id))?; - let idl_path = canister_info.get_build_idl_path(); + let idl_path = canister_info.get_constructor_idl_path(); let init_type = get_candid_init_type(&idl_path); let install_args = || blob_from_arguments(argument, None, argument_type, &init_type); diff --git a/src/dfx/src/lib/operations/canister/install_canister.rs b/src/dfx/src/lib/operations/canister/install_canister.rs index 82fe0f77b5..0ca04ed4ce 100644 --- a/src/dfx/src/lib/operations/canister/install_canister.rs +++ b/src/dfx/src/lib/operations/canister/install_canister.rs @@ -216,8 +216,10 @@ fn check_candid_compatibility( candid: &str, ) -> anyhow::Result> { use crate::util::check_candid_file; - let candid_path = canister_info.get_build_idl_path(); - let deployed_path = canister_info.get_build_idl_path().with_extension("old.did"); + let candid_path = canister_info.get_constructor_idl_path(); + let deployed_path = canister_info + .get_constructor_idl_path() + .with_extension("old.did"); std::fs::write(&deployed_path, candid).with_context(|| { format!( "Failed to write candid to {}.", From b6eb1198703e1d8adb672295b9fded0f28199d39 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Wed, 24 May 2023 16:36:40 -0400 Subject: [PATCH 2/5] deps e2e have complete dependencies --- e2e/assets/deps/app/src/main.mo | 10 ++++++++++ e2e/assets/deps/onchain/src/b.mo | 8 +++++++- e2e/assets/deps/onchain/src/c.mo | 10 ++++++++-- e2e/tests-dfx/deps.bash | 4 ++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/e2e/assets/deps/app/src/main.mo b/e2e/assets/deps/app/src/main.mo index 3a39453f84..0973b5d0cd 100644 --- a/e2e/assets/deps/app/src/main.mo +++ b/e2e/assets/deps/app/src/main.mo @@ -15,4 +15,14 @@ actor { let res = await dep_c.get(); return res; }; + + public func get_b_times_a() : async Nat { + let res = await dep_b.times_a(); + return res; + }; + + public func get_c_times_a() : async Nat { + let res = await dep_c.times_a(); + return res; + }; }; diff --git a/e2e/assets/deps/onchain/src/b.mo b/e2e/assets/deps/onchain/src/b.mo index 228602846d..41afa0bccb 100644 --- a/e2e/assets/deps/onchain/src/b.mo +++ b/e2e/assets/deps/onchain/src/b.mo @@ -1,6 +1,12 @@ -// TODO: import a once SDK-1084 is fixed +import a "canister:a"; + actor { public query func get() : async Nat { return 2; }; + + public func times_a() : async Nat { + let res = 2 * (await a.get()); + return res; + }; }; diff --git a/e2e/assets/deps/onchain/src/c.mo b/e2e/assets/deps/onchain/src/c.mo index 275c6f1c2d..56a8d92e33 100644 --- a/e2e/assets/deps/onchain/src/c.mo +++ b/e2e/assets/deps/onchain/src/c.mo @@ -1,8 +1,14 @@ -// TODO: import a once SDK-1084 is fixed -actor class c (num : Nat) { +import a "canister:a"; + +actor class c(num : Nat) { stable var NUM : Nat = num; public query func get() : async Nat { return NUM; }; + + public func times_a() : async Nat { + let res = NUM * (await a.get()); + return res; + }; }; diff --git a/e2e/tests-dfx/deps.bash b/e2e/tests-dfx/deps.bash index a0e3a5219e..af3bbbbcda 100644 --- a/e2e/tests-dfx/deps.bash +++ b/e2e/tests-dfx/deps.bash @@ -438,6 +438,10 @@ Installing canister: $CANISTER_ID_C (dep_c)" assert_eq "(2 : nat)" "$output" assert_command dfx canister call app get_c assert_eq "(33 : nat)" "$output" # corresponding to "--argument 33" above + assert_command dfx canister call app get_b_times_a + assert_eq "(22 : nat)" "$output" # 2 * 11 + assert_command dfx canister call app get_c_times_a + assert_eq "(363 : nat)" "$output" # 33 * 11 # start a clean local replica dfx canister stop app From 85815551a92103ae7fa92dd087b63fc974670987 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Wed, 24 May 2023 16:51:00 -0400 Subject: [PATCH 3/5] changelog --- CHANGELOG.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd8acf2ed1..647b2b23b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,18 @@ ## DFX -### feat: `dfx canister delete` without stopping first +### fix: motoko canisters can import other canisters with service constructor + +After specific canister builder output wasm and candid file, `dfx` will do some post processing on the candid file. + +The complete IDL will be copied into `.dfx` folder with name `constructor.did`. +It will be used for type checking during canister installation. + +Then it is separated into two parts: `service.did` and `init_args.txt`, corresponding to canister metadata `candid:service` and `candid:args`. + +`service.did` will be imported during dependent canisters building. And it will also be used by the Motoko LSP to provide IDE support. + +### fix: `dfx canister delete` without stopping first When running `dfx canister delete` on a canister that has not been stopped, dfx will now confirm the deletion instead of erroring. From 75a804d56936530501b6577e3135c8894f0e7a4b Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Wed, 24 May 2023 18:43:40 -0400 Subject: [PATCH 4/5] use the original candid if no init args --- src/dfx/src/lib/models/canister.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index ad687eb79f..7dae280f31 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -319,12 +319,19 @@ fn separate_candid(path: &Path) -> DfxResult<(String, String)> { } else { (actor, vec![]) }; - let service_did = candid::bindings::candid::compile(&env, &actor); - use candid::bindings::candid::pp_ty; - use candid::pretty::{concat, enclose}; - let doc = concat(args.iter().map(pp_ty), ","); - let init_args = enclose("(", doc, ")").pretty(80).to_string(); - Ok((service_did, init_args)) + if args.is_empty() { + // avoid reordering items in the candid file + let service_did = dfx_core::fs::read_to_string(path)?; + let init_args = String::from("()"); + Ok((service_did, init_args)) + } else { + let service_did = candid::bindings::candid::compile(&env, &actor); + use candid::bindings::candid::pp_ty; + use candid::pretty::{concat, enclose}; + let doc = concat(args.iter().map(pp_ty), ","); + let init_args = enclose("(", doc, ")").pretty(80).to_string(); + Ok((service_did, init_args)) + } } #[context("{} is not a valid subtype of {}", specified_idl_path.display(), compiled_idl_path.display())] From 7a96b21cb01671e66de2a5240382e1a191983fbd Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Thu, 25 May 2023 10:31:52 -0400 Subject: [PATCH 5/5] if the original candid has empty init --- src/dfx/src/lib/models/canister.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 7dae280f31..3d36367920 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -314,23 +314,21 @@ impl Canister { fn separate_candid(path: &Path) -> DfxResult<(String, String)> { let (env, actor) = check_candid_file(path)?; - let (actor, args) = if let Some(candid::types::internal::Type::Class(args, ty)) = actor { - (Some(*ty), args) - } else { - (actor, vec![]) - }; - if args.is_empty() { - // avoid reordering items in the candid file - let service_did = dfx_core::fs::read_to_string(path)?; - let init_args = String::from("()"); - Ok((service_did, init_args)) - } else { - let service_did = candid::bindings::candid::compile(&env, &actor); + if let Some(candid::types::internal::Type::Class(args, ty)) = actor { use candid::bindings::candid::pp_ty; use candid::pretty::{concat, enclose}; + + let actor = Some(*ty); + let service_did = candid::bindings::candid::compile(&env, &actor); let doc = concat(args.iter().map(pp_ty), ","); let init_args = enclose("(", doc, ")").pretty(80).to_string(); Ok((service_did, init_args)) + } else { + // The original candid from builder output doesn't contain init_args + // Use it directly to avoid items reordering + let service_did = dfx_core::fs::read_to_string(path)?; + let init_args = String::from("()"); + Ok((service_did, init_args)) } }