fix: motoko canisters can import other canisters with service constructor

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.
 

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;
+    };
 };

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;
+    };
 };

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;
+    };
 };

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" {

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

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(

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<PathBuf> {
         match &self.type_specific {
             CanisterTypeProperties::Motoko { .. } => self

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,25 +142,21 @@ 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() {
             let mut dfx_metadata = DfxMetadata::default();
             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) = &section.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,82 @@ 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)?;
+    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))
+    }
 }
 
 #[context("{} is not a valid subtype of {}", specified_idl_path.display(), compiled_idl_path.display())]
@@ -470,37 +565,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 +776,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!(

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);
 

src/dfx/src/lib/operations/canister/install_canister.rs

@@ -216,8 +216,10 @@ fn check_candid_compatibility(
     candid: &str,
 ) -> anyhow::Result<Option<String>> {
     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 {}.",