fix: ledger create-canister mishandles controller argument (#3842)

CHANGELOG.md

@@ -52,6 +52,11 @@ which dfx will use to locate an extension release archive.
 However, if a version is specified with `--version`, and the installed version is different,
 then `dfx extension install` will still report an error.
 
+### fix: `dfx ledger create-canister` sets controller properly
+
+A recent [hotfix](https://forum.dfinity.org/t/nns-update-2024-05-15-cycles-minting-canister-hotfix-proposal-129728/30807) to the CMC changed how the arguments to `notify_create_canister` need to be passed.
+`dfx` now again properly calls that function.
+
 ### feat: display replica port in `dfx start`
 
 This replaces the dashboard link, which is now shown only in verbose mode. This should hopefully be less confusing for new users.

e2e/tests-dfx/ledger.bash

@@ -12,7 +12,7 @@ setup() {
 
   dfx_start_for_nns_install
 
-  dfx extension install nns --version 0.2.1
+  dfx extension install nns --version 0.4.3
   dfx nns install --ledger-accounts 345f723e9e619934daac6ae0f4be13a7b0ba57d6a608e511a00fd0ded5866752 22ca7edac648b814e81d7946e8bacea99280e07c5f51a04ba7a38009d8ad8e89 5a94fe181e9d411c58726cb87cbf2d016241b6c350bc3330e4869ca76e54ecbc
 }
 
@@ -217,6 +217,15 @@ tc_to_num() {
   # TODO: assert error message once registry is fixed
   assert_eq "$balance" "$(dfx ledger balance)"
 
+  # Verify that creating a canister under a different principal's control properly sets ownership
+  CONTROLLER_PRINCIPAL="$(dfx --identity default identity get-principal)"
+  assert_command dfx ledger create-canister --amount=100 "$CONTROLLER_PRINCIPAL"
+  echo "created with: $stdout"
+  created_canister_id=$(echo "$stdout" | sed '3q;d' | sed 's/Canister created with id: //;s/"//g')
+  assert_command dfx canister info "$created_canister_id"
+  assert_contains "Controllers: $CONTROLLER_PRINCIPAL"
+  assert_not_contains "$(dfx identity get-principal)"
+
   # Transaction Deduplication
   t=$(current_time_nanoseconds)
 

src/dfx/src/commands/ledger/create_canister.rs

@@ -10,7 +10,7 @@ use crate::lib::operations::cmc::{notify_create, transfer_cmc};
 use crate::lib::root_key::fetch_root_key_if_needed;
 use crate::util::clap::parsers::e8s_parser;
 use crate::util::clap::subnet_selection_opt::SubnetSelectionOpt;
-use anyhow::{bail, Context};
+use anyhow::{anyhow, bail, Context};
 use candid::Principal;
 use clap::Parser;
 
@@ -70,6 +70,7 @@ pub async fn exec(env: &dyn Environment, opts: CreateCanisterOpts) -> DfxResult
     })?;
 
     let agent = env.get_agent();
+    let calling_identity = agent.get_principal().map_err(|err| anyhow!(err))?;
 
     fetch_root_key_if_needed(env).await?;
 
@@ -85,7 +86,7 @@ pub async fn exec(env: &dyn Environment, opts: CreateCanisterOpts) -> DfxResult
         amount,
         fee,
         opts.from_subaccount,
-        controller,
+        calling_identity,
         opts.created_at_time,
     )
     .await?;

src/dfx/src/lib/error/notify_create_canister.rs

@@ -15,4 +15,7 @@ pub enum NotifyCreateCanisterError {
 
     #[error("Failure reported by notify_create_canister: {0:?}")]
     Notify(NotifyError),
+
+    #[error("Failed to determine caller principal: {0}")]
+    GetCallerPrincipal(String),
 }

src/dfx/src/lib/ledger_types/mod.rs

@@ -7,6 +7,7 @@ use crate::lib::nns_types::icpts::ICPTs;
 use candid::CandidType;
 use candid::Nat;
 use candid::Principal;
+use ic_utils::interfaces::management_canister::builders::CanisterSettings;
 use icrc_ledger_types::icrc1::account::Subaccount as ICRCSubaccount;
 use icrc_ledger_types::icrc1::transfer::BlockIndex as ICRCBlockIndex;
 use serde::{Deserialize, Serialize};
@@ -133,6 +134,7 @@ pub struct NotifyCreateCanisterArg {
     pub block_index: BlockIndex,
     pub controller: Principal,
     pub subnet_selection: Option<SubnetSelection>,
+    pub settings: Option<CanisterSettings>,
 }
 
 #[derive(CandidType)]

src/dfx/src/lib/operations/cmc.rs

@@ -12,6 +12,7 @@ use crate::lib::operations::ledger::transfer;
 use crate::util::clap::subnet_selection_opt::SubnetSelectionType;
 use candid::{Decode, Encode, Principal};
 use ic_agent::Agent;
+use ic_utils::interfaces::management_canister::builders::CanisterSettings;
 use icrc_ledger_types::icrc1::account::Subaccount as ICRCSubaccount;
 use icrc_ledger_types::icrc1::transfer::Memo as ICRCMemo;
 use slog::Logger;
@@ -54,6 +55,9 @@ pub async fn notify_create(
     subnet_selection: SubnetSelectionType,
 ) -> Result<Principal, NotifyCreateCanisterError> {
     let user_subnet_selection = subnet_selection.get_user_choice();
+    let caller = agent
+        .get_principal()
+        .map_err(NotifyCreateCanisterError::GetCallerPrincipal)?;
     let result = agent
         .update(
             &MAINNET_CYCLE_MINTER_CANISTER_ID,
@@ -62,8 +66,17 @@ pub async fn notify_create(
         .with_arg(
             Encode!(&NotifyCreateCanisterArg {
                 block_index: block_height,
-                controller,
+                controller: caller, // Must be the caller since https://forum.dfinity.org/t/nns-update-2024-05-15-cycles-minting-canister-hotfix-proposal-129728/30807
                 subnet_selection: user_subnet_selection,
+                settings: Some(CanisterSettings {
+                    controllers: Some(vec![controller]),
+                    compute_allocation: None,
+                    memory_allocation: None,
+                    freezing_threshold: None,
+                    reserved_cycles_limit: None,
+                    wasm_memory_limit: None,
+                    log_visibility: None,
+                })
             })
             .map_err(NotifyCreateCanisterError::EncodeArguments)?,
         )