diff --git a/Cargo.lock b/Cargo.lock index 9e0312109c3..98e2195b8d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2878,6 +2878,7 @@ dependencies = [ "ic-ledger-core", "ic-management-canister-types", "ic-metrics-encoder", + "ic-nervous-system-common", "ic-nervous-system-common-build-metadata", "ic-nervous-system-governance", "ic-nns-common", diff --git a/rs/nervous_system/common/src/lib.rs b/rs/nervous_system/common/src/lib.rs index a39c8f184e0..3bea51646a3 100644 --- a/rs/nervous_system/common/src/lib.rs +++ b/rs/nervous_system/common/src/lib.rs @@ -5,6 +5,7 @@ use core::{ ops::{Add, AddAssign, Div, Mul, Sub}, }; use dfn_core::api::time_nanos; +use ic_base_types::CanisterId; use ic_canister_log::{export, GlobalBuffer, LogBuffer, LogEntry}; use ic_canisters_http_types::{HttpRequest, HttpResponse, HttpResponseBuilder}; use ic_ledger_core::tokens::{CheckedAdd, CheckedSub}; @@ -50,6 +51,9 @@ lazy_static! { .collect::>() }) .collect(); + + pub static ref NNS_DAPP_BACKEND_CANISTER_ID: CanisterId = + CanisterId::from_str("qoctq-giaaa-aaaaa-aaaea-cai").unwrap(); } // 10^8 diff --git a/rs/nns/cmc/BUILD.bazel b/rs/nns/cmc/BUILD.bazel index e2d2cd9fa27..8e42276b06c 100644 --- a/rs/nns/cmc/BUILD.bazel +++ b/rs/nns/cmc/BUILD.bazel @@ -7,6 +7,7 @@ package(default_visibility = ["//visibility:public"]) DEPENDENCIES = [ "//rs/crypto/getrandom_for_wasm", "//rs/crypto/tree_hash", + "//rs/nervous_system/common", "//rs/nervous_system/governance", "//rs/nns/common", "//rs/nns/constants", diff --git a/rs/nns/cmc/Cargo.toml b/rs/nns/cmc/Cargo.toml index c05d4afa09d..53d06df29f0 100644 --- a/rs/nns/cmc/Cargo.toml +++ b/rs/nns/cmc/Cargo.toml @@ -22,6 +22,7 @@ ic-crypto-tree-hash = { path = "../../crypto/tree_hash" } ic-management-canister-types = { path = "../../types/management_canister_types" } ic-ledger-core = { path = "../../rosetta-api/ledger_core" } ic-metrics-encoder = "1" +ic-nervous-system-common = { path = "../../nervous_system/common" } ic-nervous-system-common-build-metadata = { path = "../../nervous_system/common/build_metadata" } ic-nervous-system-governance = { path = "../../nervous_system/governance" } ic-nns-common = { path = "../../nns/common" } diff --git a/rs/nns/cmc/src/lib.rs b/rs/nns/cmc/src/lib.rs index a5ce48e508d..c6f46bf944e 100644 --- a/rs/nns/cmc/src/lib.rs +++ b/rs/nns/cmc/src/lib.rs @@ -68,10 +68,22 @@ pub struct NotifyTopUp { #[derive(Deserialize, CandidType, Clone, Debug, PartialEq, Eq)] pub struct NotifyCreateCanister { pub block_index: BlockIndex, + + /// If this not set to the caller's PrincipalId, notify_create_canister + /// returns Err. + /// + /// Thus, notify_create_canister cannot be called on behalf of another + /// principal. This might be surprising, but it is intentional. + /// + /// If controllers is not set in settings, controllers will be just + /// [controller]. (Without this "default" behavior, the controller of the + /// canister would be the Cycles Minting Canister itself.) pub controller: PrincipalId, + #[deprecated(note = "use subnet_selection instead")] pub subnet_type: Option, pub subnet_selection: Option, + pub settings: Option, } @@ -136,6 +148,8 @@ pub enum NotifyErrorCode { RefundFailed = 3, /// The subnet selection parameters are set in an invalid way. BadSubnetSelection = 4, + /// The caller is not allowed to perform the operation. + Unauthorized = 5, } impl NotifyError { diff --git a/rs/nns/cmc/src/main.rs b/rs/nns/cmc/src/main.rs index 679873a0e8a..5ddf7ad2965 100644 --- a/rs/nns/cmc/src/main.rs +++ b/rs/nns/cmc/src/main.rs @@ -21,6 +21,7 @@ use ic_management_canister_types::{ BoundedVec, CanisterIdRecord, CanisterSettingsArgs, CanisterSettingsArgsBuilder, CreateCanisterArgs, Method, IC_00, }; +use ic_nervous_system_common::NNS_DAPP_BACKEND_CANISTER_ID; use ic_nervous_system_governance::maturity_modulation::{ MAX_MATURITY_MODULATION_PERMYRIAD, MIN_MATURITY_MODULATION_PERMYRIAD, }; @@ -32,6 +33,7 @@ use icp_ledger::{ Subaccount, Tokens, TransactionNotification, DEFAULT_TRANSFER_FEE, }; use icrc_ledger_types::icrc1::account::Account; +use lazy_static::lazy_static; use on_wire::{FromWire, IntoWire, NewType}; use rand::{rngs::StdRng, seq::SliceRandom, SeedableRng}; use serde::{Deserialize, Serialize}; @@ -1357,11 +1359,29 @@ async fn notify_mint_cycles( /// Notify about create canister transaction /// +/// Calling this is the second step in a 2 step canister creation flow, which +/// goes as follows: +/// +/// 1. ICP is sent to a subaccount of the Cycles Minting Canister +/// corresponding to creator principal C. Note that while the sender of the +/// ICP is typically C, it makes no difference who sends the ICP. The only +/// thing that matters is the destination (sub)account. +/// +/// 2. C calls notify_create_canister. +/// /// # Arguments /// /// * `block_height` - The height of the block you would like to send a /// notification about. -/// * `controller` - PrincipalId of the canister controller. +/// * `controller` - The creator of the canister. Must match caller; otherwise, +/// Err is returned. This is also used when checking that the creator is +/// authorized to create canisters in subnets where authorization is required. +/// This is also used when `settings` does not specify `controllers`. +/// * `settings` - The settings of the canister. If controllers is not +/// populated, it will be initialized with a (singleton) vec containing just +/// `controller`. +/// * `subnet_selection` - Where to create the canister. +/// * `subnet_type` - Deprecated. Use subnet_selection instead. #[candid_method(update, rename = "notify_create_canister")] #[allow(deprecated)] async fn notify_create_canister( @@ -1373,6 +1393,8 @@ async fn notify_create_canister( settings, }: NotifyCreateCanister, ) -> Result { + authorize_caller_to_call_notify_create_canister_on_behalf_of_creator(caller(), controller)?; + let cmc_id = dfn_core::api::id(); let sub = Subaccount::from(&controller); let expected_destination_account = AccountIdentifier::new(cmc_id.get(), Some(sub)); @@ -1439,6 +1461,49 @@ async fn notify_create_canister( } } +/// Returns Err if caller is not authorized to call notify_create_canister on +/// behalf of creator. +/// +/// Of course, a principal can act on its own behalf. In other words, this +/// allows calls when caller == creator. +/// +/// In additional to that, there is another case where calls are allowed: the +/// nns-dapp backend canister is allowed to call notify_create_canister on +/// behalf of others. +/// +/// If Err is returned, the value will be NotifyError::Other with code +/// Unauthorized. +fn authorize_caller_to_call_notify_create_canister_on_behalf_of_creator( + caller: PrincipalId, + creator: PrincipalId, +) -> Result<(), NotifyError> { + if caller == creator { + return Ok(()); + } + + lazy_static! { + static ref ALLOWED_CALLERS: [PrincipalId; 1] = + [PrincipalId::from(*NNS_DAPP_BACKEND_CANISTER_ID),]; + } + + if ALLOWED_CALLERS.contains(&caller) { + return Ok(()); + } + + // Other is used, because adding a Unauthorized variant to NotifyError would + // confuse old clients. + let err = NotifyError::Other { + error_code: NotifyErrorCode::Unauthorized as u64, + error_message: format!( + "{} is not authorized to call notify_create_canister on behalf \ + of {}. (Do not retry, because the same result will occur.)", + caller, creator, + ), + }; + + Err(err) +} + #[candid_method(update, rename = "create_canister")] #[allow(deprecated)] async fn create_canister( @@ -1624,11 +1689,7 @@ async fn transaction_notification(tn: TransactionNotification) -> Result ( Ok(CyclesResponse::CanisterCreated(canister_id)), @@ -1693,6 +1754,52 @@ async fn transaction_notification(tn: TransactionNotification) -> Result Result { + let sender = transaction_notification.from; + + let creator = { + let to_subaccount = transaction_notification.to_subaccount.ok_or_else(|| { + format!( + "Transfer has no destination subaccount:\n{:#?}", + transaction_notification, + ) + })?; + + PrincipalId::try_from(&to_subaccount).map_err(|err| { + format!( + "Cannot determine creator principal from ICP transfer to Cycles \ + Minting Canister destination subaccount {}: {}", + to_subaccount, err, + ) + })? + }; + + if sender == creator { + return Ok(creator); + } + + Err(format!( + "Principal {} sent ICP to the Cycles Minting Canister on behalf of {} \ + in order to create a canister, but this is not allowed (anymore).", + sender, creator, + )) +} + // If conversion fails, log and return an error fn tokens_to_cycles(amount: Tokens) -> Result { with_state(|state| { @@ -2387,6 +2494,162 @@ mod tests { assert_eq!(state, state2); } + #[test] + fn test_authorize_caller_to_call_notify_create_canister_on_behalf_of_creator() { + let creator = PrincipalId::new_user_test_id(519_167_122); + let authorize = |caller| { + authorize_caller_to_call_notify_create_canister_on_behalf_of_creator(caller, creator) + }; + + let on_behalf_of_self_result = authorize(creator); + assert!( + on_behalf_of_self_result.is_ok(), + "{:#?}", + on_behalf_of_self_result, + ); + + let eve = PrincipalId::new_user_test_id(898_071_769); + let on_behalf_of_other_result = authorize(eve); + assert!( + on_behalf_of_other_result.is_err(), + "{:#?}", + on_behalf_of_other_result, + ); + let err = on_behalf_of_other_result.unwrap_err(); + match &err { + NotifyError::Other { + error_code, + error_message, + } => { + assert_eq!( + *error_code, + NotifyErrorCode::Unauthorized as u64, + "{:#?}", + err, + ); + + let error_message = error_message.to_lowercase(); + for key_word in ["authorize", "on behalf"] { + assert!( + error_message.contains(key_word), + "{} not in {:#?}", + key_word, + err, + ); + } + } + + _ => panic!("{:#?}", err), + } + + let caller_is_nns_dapp_result = authorize(PrincipalId::from(*NNS_DAPP_BACKEND_CANISTER_ID)); + assert!( + caller_is_nns_dapp_result.is_ok(), + "{:#?}", + caller_is_nns_dapp_result, + ); + } + + #[test] + fn test_authorize_sender_to_create_canister_via_ledger_notify() { + // Happy case. + let creator = PrincipalId::new_user_test_id(777); + let ok_transaction_notification = TransactionNotification { + from: creator, + to_subaccount: Some(Subaccount::from(&creator)), + + // These are not used. + memo: MEMO_CREATE_CANISTER, // Just for realism. + from_subaccount: None, + to: CanisterId::from_u64(111), + block_height: 222, + amount: Tokens::from_e8s(333), + }; + + // Evil case. + assert_eq!( + authorize_sender_to_create_canister_via_ledger_notify(&ok_transaction_notification,), + Ok(creator), + ); + + let evil = PrincipalId::new_user_test_id(666); + let evil_transaction_notification = TransactionNotification { + from: evil, + ..ok_transaction_notification.clone() + }; + + let evil_result = + authorize_sender_to_create_canister_via_ledger_notify(&evil_transaction_notification); + + let evil_result = match evil_result { + Err(err) => err.to_lowercase(), + wrong => panic!("Evil result is supposed to be Err, but was {:?}", wrong), + }; + for key_word in ["create", "canister", "on behalf of", "not allowed"] { + assert!( + evil_result.contains(key_word), + "{} not in {:?}", + key_word, + evil_result + ); + } + + // Invalid transfer case 1: no destination subaccount. + let no_destination_subaccount_transaction_notification = TransactionNotification { + to_subaccount: None, + ..ok_transaction_notification.clone() + }; + + let no_destination_subaccount_result = + authorize_sender_to_create_canister_via_ledger_notify( + &no_destination_subaccount_transaction_notification, + ); + + let no_destination_subaccount_result = match no_destination_subaccount_result { + Err(err) => err.to_lowercase(), + wrong => panic!( + "No destination subaccount result is supposed to be Err, but was {:?}", + wrong, + ), + }; + for key_word in ["has no", "destination", "subaccount"] { + assert!( + no_destination_subaccount_result.contains(key_word), + "{} not in {:?}", + key_word, + no_destination_subaccount_result, + ); + } + + // Invalid transfer case 2: destination subaccount present, but does not map to (creator) + // principal. + let garbage_subaccount = [42_u8; 32]; + let no_creator_transaction_notification = TransactionNotification { + to_subaccount: Some(Subaccount(garbage_subaccount)), + ..ok_transaction_notification + }; + + let no_creator_result = authorize_sender_to_create_canister_via_ledger_notify( + &no_creator_transaction_notification, + ); + + let no_creator_result = match no_creator_result { + Err(err) => err.to_lowercase(), + wrong => panic!( + "No destination subaccount result is supposed to be Err, but was {:?}", + wrong, + ), + }; + for key_word in ["determine", "creator", "subaccount"] { + assert!( + no_creator_result.contains(key_word), + "{} not in {:?}", + key_word, + no_creator_result, + ); + } + } + #[test] fn test_purge_notifications() { fn block_index_to_cycles(block_index: BlockIndex) -> Cycles { diff --git a/rs/tests/src/nns_tests/cycles_minting.rs b/rs/tests/src/nns_tests/cycles_minting.rs index 75dde83b20d..e1d52af66aa 100644 --- a/rs/tests/src/nns_tests/cycles_minting.rs +++ b/rs/tests/src/nns_tests/cycles_minting.rs @@ -45,8 +45,8 @@ use ic_types::{CanisterId, Cycles, PrincipalId}; use icp_ledger::protobuf::TipOfChainRequest; use icp_ledger::{ tokens_from_proto, AccountBalanceArgs, AccountIdentifier, Block, BlockArg, BlockIndex, - BlockRes, CyclesResponse, NotifyCanisterArgs, Operation, Subaccount, TipOfChainRes, Tokens, - DEFAULT_TRANSFER_FEE, + BlockRes, CyclesResponse, Memo, NotifyCanisterArgs, Operation, Subaccount, TipOfChainRes, + Tokens, TransferArgs, TransferError, DEFAULT_TRANSFER_FEE, }; use on_wire::{FromWire, IntoWire}; use rand::rngs::StdRng; @@ -140,6 +140,13 @@ pub fn test(env: TestEnv) { ); let (controller_user_keypair, controller_pid) = make_user_ed25519(7); + let controller_user = UserHandle::new( + &nns_node.get_public_url(), + &agent_client, + &controller_user_keypair, + LEDGER_CANISTER_ID, + CYCLES_MINTING_CANISTER_ID, + ); let xdr_permyriad_per_icp = 5_000; // = 0.5 XDR/ICP let icpts_to_cycles = TokensToCycles { @@ -259,7 +266,7 @@ pub fn test(env: TestEnv) { let send_amount = Tokens::new(2, 0).unwrap(); let (err, refund_block) = user1 - .create_canister_cmc(send_amount, None, &controller_pid, None, None) + .create_canister_cmc(send_amount, None, &controller_user, None, None) .await .unwrap_err(); @@ -269,13 +276,24 @@ pub fn test(env: TestEnv) { /* Check that the funds for the failed creation attempt are returned to use * (minus the fees). */ let refund_block = refund_block.unwrap(); - tst.check_refund(refund_block, send_amount, CREATE_CANISTER_REFUND_FEE) - .await; + tst.check_refund( + refund_block, + send_amount, + CREATE_CANISTER_REFUND_FEE, + *TEST_USER1_PRINCIPAL, + ) + .await; // remove when ledger notify goes away { - let (err, refund_block) = user1 - .create_canister_ledger(send_amount, None, &controller_pid) + user1 + .transfer( + Tokens::from_e8s(send_amount.get_e8s() + 2 * DEFAULT_TRANSFER_FEE.get_e8s()), + controller_user.principal_id(), + ) + .await; + let (err, refund_block) = controller_user + .create_canister_ledger(send_amount) .await .unwrap_err(); @@ -285,8 +303,13 @@ pub fn test(env: TestEnv) { /* Check that the funds for the failed creation attempt are returned to use * (minus the fees). */ let refund_block = refund_block.unwrap(); - tst.check_refund(refund_block, send_amount, CREATE_CANISTER_REFUND_FEE) - .await; + tst.check_refund( + refund_block, + send_amount, + CREATE_CANISTER_REFUND_FEE, + controller_user.principal_id(), + ) + .await; } /* Register a subnet. */ @@ -305,7 +328,7 @@ pub fn test(env: TestEnv) { let small_amount = Tokens::new(0, 500_000).unwrap(); let (err, refund_block) = user1 - .create_canister_cmc(small_amount, None, &controller_pid, None, None) + .create_canister_cmc(small_amount, None, &controller_user, None, None) .await .unwrap_err(); @@ -313,13 +336,24 @@ pub fn test(env: TestEnv) { assert!(err.contains("Creating a canister requires a fee of")); let refund_block = refund_block.unwrap(); - tst.check_refund(refund_block, small_amount, CREATE_CANISTER_REFUND_FEE) - .await; + tst.check_refund( + refund_block, + small_amount, + CREATE_CANISTER_REFUND_FEE, + *TEST_USER1_PRINCIPAL, + ) + .await; // remove when ledger notify goes away { - let (err, refund_block) = user1 - .create_canister_ledger(small_amount, None, &controller_pid) + user1 + .transfer( + Tokens::from_e8s(small_amount.get_e8s() + DEFAULT_TRANSFER_FEE.get_e8s()), + controller_user.principal_id(), + ) + .await; + let (err, refund_block) = controller_user + .create_canister_ledger(small_amount) .await .unwrap_err(); @@ -327,8 +361,13 @@ pub fn test(env: TestEnv) { assert!(err.contains("Creating a canister requires a fee of")); let refund_block = refund_block.unwrap(); - tst.check_refund(refund_block, small_amount, CREATE_CANISTER_REFUND_FEE) - .await; + tst.check_refund( + refund_block, + small_amount, + CREATE_CANISTER_REFUND_FEE, + controller_user.principal_id(), + ) + .await; } /* Create with funds < the refund fee. */ @@ -339,7 +378,7 @@ pub fn test(env: TestEnv) { .unwrap(); let (err, no_refund_block) = user1 - .create_canister_cmc(tiny_amount, None, &controller_pid, None, None) + .create_canister_cmc(tiny_amount, None, &controller_user, None, None) .await .unwrap_err(); @@ -367,8 +406,14 @@ pub fn test(env: TestEnv) { // remove when ledger notify goes away { - let (err, no_refund_block) = user1 - .create_canister_ledger(tiny_amount, None, &controller_pid) + user1 + .transfer( + Tokens::from_e8s(tiny_amount.get_e8s() + DEFAULT_TRANSFER_FEE.get_e8s()), + controller_user.principal_id(), + ) + .await; + let (err, no_refund_block) = controller_user + .create_canister_ledger(tiny_amount) .await .unwrap_err(); @@ -403,14 +448,14 @@ pub fn test(env: TestEnv) { let bh = user1 .pay_for_canister(initial_amount, None, &controller_pid) .await; - let new_canister_id = user1 + let new_canister_id = controller_user .notify_canister_create_cmc(bh, None, &controller_pid, None, None) .await .unwrap(); // second notify should return the success result together with canister id let tip = tst.get_tip().await.unwrap(); - let can_id = user1 + let can_id = controller_user .notify_canister_create_cmc(bh, None, &controller_pid, None, None) .await .unwrap(); @@ -568,8 +613,14 @@ pub fn test(env: TestEnv) { // remove when ledger notify goes away { - let new_canister_id = user1 - .create_canister_ledger(initial_amount, None, &controller_pid) + user1 + .transfer( + Tokens::from_e8s(initial_amount.get_e8s() + DEFAULT_TRANSFER_FEE.get_e8s()), + controller_user.principal_id(), + ) + .await; + let new_canister_id = controller_user + .create_canister_ledger(initial_amount) .await .unwrap(); @@ -666,7 +717,7 @@ pub fn test(env: TestEnv) { let nns_amount = Tokens::new(2, 0).unwrap(); let new_canister_id = user1 - .create_canister_cmc(nns_amount, None, &controller_pid, None, None) + .create_canister_cmc(nns_amount, None, &controller_user, None, None) .await .unwrap(); @@ -691,9 +742,14 @@ pub fn test(env: TestEnv) { // remove when ledger notify goes away { let nns_amount = Tokens::new(2, 0).unwrap(); - - let new_canister_id = user1 - .create_canister_ledger(nns_amount, None, &controller_pid) + user1 + .transfer( + Tokens::from_e8s(nns_amount.get_e8s() + DEFAULT_TRANSFER_FEE.get_e8s()), + controller_user.principal_id(), + ) + .await; + let new_canister_id = controller_user + .create_canister_ledger(nns_amount) .await .unwrap(); @@ -749,7 +805,7 @@ pub fn test(env: TestEnv) { let block = user1 .pay_for_canister(nns_amount, None, &controller_pid) .await; - let err = user1 + let err = controller_user .notify_canister_create_cmc(block, None, &controller_pid, None, None) .await .unwrap_err(); @@ -799,14 +855,20 @@ pub fn test(env: TestEnv) { info!(logger, "creating NNS canister"); - user1 + controller_user .notify_canister_create_cmc(block, None, &controller_pid, None, None) .await .unwrap(); // remove when ledger notify goes away user1 - .create_canister_ledger(nns_amount, None, &controller_pid) + .transfer( + Tokens::from_e8s(nns_amount.get_e8s() + DEFAULT_TRANSFER_FEE.get_e8s()), + controller_user.principal_id(), + ) + .await; + controller_user + .create_canister_ledger(nns_amount) .await .unwrap(); @@ -816,7 +878,7 @@ pub fn test(env: TestEnv) { let amount = Tokens::new(100_000, 0).unwrap(); let (err, refund_block) = user1 - .create_canister_cmc(amount, None, &controller_pid, None, None) + .create_canister_cmc(amount, None, &controller_user, None, None) .await .unwrap_err(); @@ -825,15 +887,25 @@ pub fn test(env: TestEnv) { .contains("cycles have been minted in the last 3600 seconds, please try again later")); let refund_block = refund_block.unwrap(); - tst.check_refund(refund_block, amount, CREATE_CANISTER_REFUND_FEE) - .await; + tst.check_refund( + refund_block, + amount, + CREATE_CANISTER_REFUND_FEE, + *TEST_USER1_PRINCIPAL, + ) + .await; // remove when ledger notify goes away { let amount = Tokens::new(100_000, 0).unwrap(); - - let (err, refund_block) = user1 - .create_canister_ledger(amount, None, &controller_pid) + user1 + .transfer( + Tokens::from_e8s(amount.get_e8s() + DEFAULT_TRANSFER_FEE.get_e8s()), + controller_user.principal_id(), + ) + .await; + let (err, refund_block) = controller_user + .create_canister_ledger(amount) .await .unwrap_err(); @@ -843,8 +915,13 @@ pub fn test(env: TestEnv) { )); let refund_block = refund_block.unwrap(); - tst.check_refund(refund_block, amount, CREATE_CANISTER_REFUND_FEE) - .await; + tst.check_refund( + refund_block, + amount, + CREATE_CANISTER_REFUND_FEE, + controller_user.principal_id(), + ) + .await; } /* Test getting the total number of cycles minted. */ @@ -891,7 +968,14 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { CYCLES_MINTING_CANISTER_ID, ); - let (controller_user_keypair, controller_pid) = make_user_ed25519(7); + let (controller_user_keypair, _controller_pid) = make_user_ed25519(7); + let controller_user = UserHandle::new( + &nns_node.get_public_url(), + &agent_client, + &controller_user_keypair, + LEDGER_CANISTER_ID, + CYCLES_MINTING_CANISTER_ID, + ); let xdr_permyriad_per_icp = 5_000; // = 0.5 XDR/ICP @@ -913,7 +997,7 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { let send_amount = Tokens::new(2, 0).unwrap(); let (err, refund_block) = user1 - .create_canister_cmc(send_amount, None, &controller_pid, None, None) + .create_canister_cmc(send_amount, None, &controller_user, None, None) .await .unwrap_err(); @@ -923,8 +1007,13 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { // Check that the funds for the failed creation attempt are returned to use // (minus the fees). let refund_block = refund_block.unwrap(); - tst.check_refund(refund_block, send_amount, CREATE_CANISTER_REFUND_FEE) - .await; + tst.check_refund( + refund_block, + send_amount, + CREATE_CANISTER_REFUND_FEE, + *TEST_USER1_PRINCIPAL, + ) + .await; // Register an authorized subnet and additionally assign a subnet to a type. info!(logger, "registering subnets"); @@ -980,7 +1069,7 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { let initial_amount = Tokens::new(10_000, 0).unwrap(); let canister_on_authorized_subnet = user1 - .create_canister_cmc(initial_amount, None, &controller_pid, None, None) + .create_canister_cmc(initial_amount, None, &controller_user, None, None) .await .unwrap(); @@ -988,7 +1077,7 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { .create_canister_cmc( initial_amount, None, - &controller_pid, + &controller_user, Some(type1.clone()), None, ) @@ -999,7 +1088,7 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { .create_canister_cmc( initial_amount, None, - &controller_pid, + &controller_user, None, Some(SubnetSelection::Filter(SubnetFilter { subnet_type: Some(type1), @@ -1012,7 +1101,7 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { .create_canister_cmc( initial_amount, None, - &controller_pid, + &controller_user, None, Some(SubnetSelection::Subnet { subnet: authorized_subnet, @@ -1025,7 +1114,7 @@ pub fn create_canister_on_specific_subnet_type(env: TestEnv) { .create_canister_cmc( initial_amount, None, - &controller_pid, + &controller_user, None, Some(SubnetSelection::Subnet { subnet: subnet_of_type1, @@ -1173,6 +1262,7 @@ impl TestAgent { refund_block: BlockIndex, send_amount: Tokens, refund_fee: Tokens, + expected_destination_principal_id: PrincipalId, ) { let block = self.get_block(refund_block).await.unwrap().unwrap(); let txn = block.transaction(); @@ -1187,7 +1277,10 @@ impl TestAgent { .unwrap(), send_amount ); - assert_eq!(to, (*TEST_USER1_PRINCIPAL).into()); + assert_eq!( + to, + AccountIdentifier::new(expected_destination_principal_id, None) + ); } _ => panic!("unexpected block {:?}", txn), } @@ -1212,6 +1305,7 @@ impl TestAgent { } pub struct UserHandle { + user_keypair: Ed25519KeyPair, agent: Agent, ledger_id: CanisterId, cmc_id: CanisterId, @@ -1231,7 +1325,10 @@ impl UserHandle { ic_url.clone(), Sender::from_keypair(user_keypair), ); + let user_keypair = *user_keypair; + Self { + user_keypair, agent, ledger_id, cmc_id, @@ -1279,38 +1376,50 @@ impl UserHandle { CandidOne::from_bytes(bytes).map(|c| c.0) } - pub async fn create_canister_ledger( - &self, - amount: Tokens, - sender_subaccount: Option, - controller_id: &PrincipalId, - ) -> CreateCanisterResult { + /// Creates a canister via the deprecated ledger.notify flow. That is, + /// + /// 1. Send ICP to the CMC. + /// + /// 2. Call ledger.noitfy. + pub async fn create_canister_ledger(&self, amount: Tokens) -> CreateCanisterResult { let block = self - .pay_for_canister(amount, sender_subaccount, controller_id) + .pay_for_canister(amount, None, &self.principal_id()) .await; - self.notify_canister_create_ledger(block, sender_subaccount, controller_id) + self.notify_canister_create_ledger(block, None, &self.principal_id()) .await } + /// Creates a canister using the notify_create_canister flow. That is, + /// + /// 1. Send ICP to the CMC. + /// + /// 2. Call CMC.notify_create_canister. pub async fn create_canister_cmc( &self, amount: Tokens, sender_subaccount: Option, - controller_id: &PrincipalId, + controller: &UserHandle, subnet_type: Option, subnet_selection: Option, ) -> CreateCanisterResult { let block = self - .pay_for_canister(amount, sender_subaccount, controller_id) + .pay_for_canister(amount, sender_subaccount, &controller.principal_id()) .await; - self.notify_canister_create_cmc( - block, - sender_subaccount, - controller_id, - subnet_type, - subnet_selection, - ) - .await + controller + .notify_canister_create_cmc( + block, + sender_subaccount, + &controller.principal_id(), + subnet_type, + subnet_selection, + ) + .await + } + + pub fn principal_id(&self) -> PrincipalId { + PrincipalId::new_self_authenticating(&ic_canister_client_sender::ed25519_public_key_to_der( + self.user_keypair.public_key.to_vec(), + )) } pub async fn top_up_canister_ledger( @@ -1343,6 +1452,24 @@ impl UserHandle { AccountIdentifier::new(self.cmc_id.into(), Some(target_canister_id.into())) } + async fn transfer(&self, amount: Tokens, destination_principal_id: PrincipalId) { + let transfer_args = TransferArgs { + amount, + to: AccountIdentifier::new(destination_principal_id, None).to_address(), + + fee: DEFAULT_TRANSFER_FEE, + memo: Memo(0), + from_subaccount: None, + created_at_time: None, + }; + + let result: Result = self + .update_did(&self.ledger_id, "transfer", transfer_args) + .await + .unwrap(); + result.unwrap(); + } + pub async fn pay_for_canister( &self, amount: Tokens,