diff --git a/crates/icp-canister-interfaces/src/engine_canister.rs b/crates/icp-canister-interfaces/src/engine_canister.rs new file mode 100644 index 000000000..ea9effbcc --- /dev/null +++ b/crates/icp-canister-interfaces/src/engine_canister.rs @@ -0,0 +1,87 @@ +//! Interface to the control-panel **engine-canister** — the registry that maps +//! an engine's subnet to its per-engine **engine-operator** canister. +//! +//! On a `SubnetType::CloudEngine` subnet, canister creation is delegated to the +//! subnet's engine-operator (which exposes a cycles-ledger-compatible +//! `create_canister`). To find that operator the CLI asks the engine-canister +//! "what is the engine-operator id for this subnet?" via +//! [`GET_ENGINE_OPERATOR_BY_SUBNET_METHOD`]. +//! +//! The argument and result are dedicated `opt`-field wrapper records +//! (`…Args` / `…Result`), mirroring the control-panel convention so the +//! interface can grow fields on either side without breaking the wire format. + +use std::env::{self, VarError}; + +use candid::{CandidType, Principal}; +use serde::Deserialize; + +/// Default engine-canister principal (staging/prod registry). +/// +/// Overridable at runtime via the [`ENGINE_CANISTER_ID_ENV`] environment +/// variable — see [`engine_canister_id`]. +pub const ENGINE_CANISTER_CID: &str = "q6cfj-fyaaa-aaaar-qb77q-cai"; + +/// Environment variable that overrides [`ENGINE_CANISTER_CID`]. +pub const ENGINE_CANISTER_ID_ENV: &str = "ENGINE_CANISTER_ID"; + +/// The engine-canister query that resolves a subnet to its engine-operator id. +pub const GET_ENGINE_OPERATOR_BY_SUBNET_METHOD: &str = "getEngineOperatorBySubnet"; + +/// Resolve the engine-canister principal to talk to. +/// +/// Uses the value of the `ENGINE_CANISTER_ID` environment variable when set and +/// non-empty, otherwise falls back to [`ENGINE_CANISTER_CID`]. Returns an error +/// when the override is set but invalid — either not a valid principal, or not +/// valid Unicode. Only an absent or empty (whitespace-only) variable uses the +/// default: a configured-but-invalid value must never silently route to the +/// built-in registry (and thus a different environment). +pub fn engine_canister_id() -> Result { + match env::var(ENGINE_CANISTER_ID_ENV) { + Ok(value) if !value.trim().is_empty() => Principal::from_text(value.trim()) + .map_err(|e| format!("invalid {ENGINE_CANISTER_ID_ENV}: {e}")), + // Set but non-Unicode is still a configured override — reject it rather + // than silently using the default and targeting the wrong environment. + Err(VarError::NotUnicode(_)) => Err(format!( + "invalid {ENGINE_CANISTER_ID_ENV}: not valid Unicode" + )), + // Unset or empty/whitespace-only: use the built-in default. + Ok(_) | Err(VarError::NotPresent) => Ok(Principal::from_text(ENGINE_CANISTER_CID) + .expect("ENGINE_CANISTER_CID is a valid principal")), + } +} + +/// Argument for [`GET_ENGINE_OPERATOR_BY_SUBNET_METHOD`]. +/// +/// A single `opt`-field record so the engine-canister can add inputs later +/// without changing the method's candid type. +#[derive(Clone, Debug, Default, CandidType, Deserialize)] +pub struct GetEngineOperatorBySubnetArgs { + /// The subnet whose engine-operator should be resolved. + pub subnet_id: Option, +} + +/// Result of [`GET_ENGINE_OPERATOR_BY_SUBNET_METHOD`]. +/// +/// `engine_operator_id` is `None` when no live engine is bound to the queried +/// subnet, or the matched engine has no operator recorded yet. The caller +/// treats a `None` here the same as "the subnet does not exist". +#[derive(Clone, Debug, Default, CandidType, Deserialize)] +pub struct GetEngineOperatorBySubnetResult { + /// The per-engine engine-operator canister id for the subnet, if any. + pub engine_operator_id: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_engine_canister_id_parses() { + // The built-in default must always be a valid principal. + assert_eq!( + engine_canister_id().unwrap(), + Principal::from_text(ENGINE_CANISTER_CID).unwrap() + ); + } +} diff --git a/crates/icp-canister-interfaces/src/lib.rs b/crates/icp-canister-interfaces/src/lib.rs index 9a95c416e..938756a3b 100644 --- a/crates/icp-canister-interfaces/src/lib.rs +++ b/crates/icp-canister-interfaces/src/lib.rs @@ -1,6 +1,7 @@ pub mod candid_ui; pub mod cycles_ledger; pub mod cycles_minting_canister; +pub mod engine_canister; pub mod governance; pub mod icp_ledger; pub mod internet_identity; diff --git a/crates/icp-cli/src/operations/create.rs b/crates/icp-cli/src/operations/create.rs index 9bc823619..d0f9163ed 100644 --- a/crates/icp-cli/src/operations/create.rs +++ b/crates/icp-cli/src/operations/create.rs @@ -3,7 +3,7 @@ use std::time::Duration; use bigdecimal::{BigDecimal, ToPrimitive}; use candid::{Decode, Encode, IDLArgs, IDLValue, Nat, Principal}; -use ic_agent::{Agent, AgentError, agent::SubnetType}; +use ic_agent::{Agent, AgentError, agent::RejectCode, agent::SubnetType}; use ic_ledger_types::{ AccountIdentifier, Memo, Subaccount, Tokens, TransferArgs, TransferError, TransferResult, }; @@ -22,12 +22,16 @@ use icp_canister_interfaces::{ CYCLES_MINTING_CANISTER_CID, CYCLES_MINTING_CANISTER_PRINCIPAL, MEMO_CREATE_CANISTER, NotifyCreateCanisterArg, NotifyCreateCanisterResponse, NotifyError, SubnetSelection, }, + engine_canister::{ + GET_ENGINE_OPERATOR_BY_SUBNET_METHOD, GetEngineOperatorBySubnetArgs, + GetEngineOperatorBySubnetResult, engine_canister_id, + }, icp_ledger::{ICP_LEDGER_BLOCK_FEE_E8S, ICP_LEDGER_PRINCIPAL}, }; use rand::seq::IndexedRandom; use snafu::{OptionExt, ResultExt, Snafu}; use tokio::{select, sync::OnceCell, time::sleep}; -use tracing::info; +use tracing::{info, warn}; use super::proxy::UpdateOrProxyError; use super::proxy_management; @@ -49,6 +53,20 @@ pub enum CreateOperationError { #[snafu(display("failed to get subnet for canister: {source}"))] GetSubnet { source: AgentError }, + #[snafu(display("invalid engine-canister id: {message}"))] + EngineCanisterId { message: String }, + + #[snafu(display("failed to query the engine-canister registry: {source}"))] + EngineCanisterQuery { source: AgentError }, + + #[snafu(display( + "could not resolve an engine-operator for CloudEngine subnet {subnet} via engine-canister {engine_registry} (no operator registered, or the registry is not deployed on this network)" + ))] + NoEngineOperator { + subnet: Principal, + engine_registry: Principal, + }, + #[snafu(display("registry error: {message}"))] Registry { message: String }, @@ -214,7 +232,28 @@ impl CreateOperation { .await .context(GetSubnetSnafu)?; let cid = if let Some(SubnetType::CloudEngine) = subnet_info.subnet_type() { - self.create_mgmt(settings, selected_subnet).await? + // Resolve the subnet's engine-operator first. Only a definitive + // "could not resolve an operator" resolution failure falls back to + // the legacy management-canister path — this covers both no operator + // being registered for the subnet and the engine-canister registry + // not being deployed on this network (a `DestinationInvalid` reject, + // mapped to `NoEngineOperator` in `resolve_engine_operator`). Every + // other resolution error (invalid registry id, other query/decode + // failures) is propagated. Crucially, the fallback is decided + // *before* any operator `create_canister` update is submitted: once + // we hand off to the operator, a failure may mean the canister was + // already created, so retrying via the management canister could + // double-create and double-charge. Those errors propagate too. + match self.resolve_engine_operator(selected_subnet).await { + Ok(operator) => self.create_via_operator(settings, operator).await?, + Err(e @ CreateOperationError::NoEngineOperator { .. }) => { + warn!( + "{e}; falling back to management-canister creation on this CloudEngine subnet" + ); + self.create_mgmt(settings, selected_subnet).await? + } + Err(e) => return Err(e), + } } else { self.create_ledger(settings, selected_subnet).await? }; @@ -273,16 +312,102 @@ impl CreateOperation { Ok(cid) } + /// Ask the engine-canister registry which engine-operator serves `subnet`. + async fn resolve_engine_operator( + &self, + subnet: Principal, + ) -> Result { + let engine_registry = engine_canister_id() + .map_err(|message| CreateOperationError::EngineCanisterId { message })?; + + let arg = GetEngineOperatorBySubnetArgs { + subnet_id: Some(subnet), + }; + let resp = match self + .inner + .agent + .query(&engine_registry, GET_ENGINE_OPERATOR_BY_SUBNET_METHOD) + .with_arg(Encode!(&arg).context(CandidEncodeSnafu)?) + .call() + .await + { + Ok(resp) => resp, + // The engine-canister registry itself is not deployed on this + // network (e.g. a local/test env). The replica rejects the query + // with `DestinationInvalid` ("Canister ... not found", IC0301). + // Treat a missing registry the same as "no operator registered": + // this happens before any operator update is submitted, so the + // legacy management-canister fallback is safe. + Err(e) if is_canister_not_found(&e) => { + return NoEngineOperatorSnafu { + subnet, + engine_registry, + } + .fail(); + } + Err(e) => return Err(e).context(EngineCanisterQuerySnafu), + }; + let resp = Decode!(&resp, GetEngineOperatorBySubnetResult).context(CandidDecodeSnafu)?; + + resp.engine_operator_id.context(NoEngineOperatorSnafu { + subnet, + engine_registry, + }) + } + + /// Delegate creation to a subnet's engine-operator canister. The operator's + /// `create_canister` is byte-compatible with the cycles ledger, so we reuse + /// the same request/response types and simply target the operator principal. + async fn create_via_operator( + &self, + settings: &CanisterSettings, + operator: Principal, + ) -> Result { + let creation_args = CreationArgs { + // CloudEngine subnets only create on themselves; the operator + // ignores subnet selection, so leave it unset. + subnet_selection: None, + settings: Some(settings.clone()), + }; + let arg = CreateCanisterArgs { + from_subaccount: None, + created_at_time: None, + amount: Nat::from(self.cycles()), + creation_args: Some(creation_args), + }; + + let resp = self + .inner + .agent + .update(&operator, "create_canister") + .with_arg(Encode!(&arg).context(CandidEncodeSnafu)?) + .call_and_wait() + .await + .context(AgentSnafu)?; + let resp: CreateCanisterResponse = + Decode!(&resp, CreateCanisterResponse).context(CandidDecodeSnafu)?; + let cid = match resp { + CreateCanisterResponse::Ok { canister_id, .. } => canister_id, + CreateCanisterResponse::Err(err) => { + return CreateCanisterSnafu { + message: err.format_error(self.cycles()), + } + .fail(); + } + }; + Ok(cid) + } + + /// Legacy CloudEngine creation: call the management canister's + /// `create_canister`, routing to the target subnet by its id (subnet-scoped, + /// so no canister on that subnet is needed to derive an effective canister + /// id). Requires subnet-administrator permissions. Kept as a fallback for + /// the engine-operator path. async fn create_mgmt( &self, settings: &CanisterSettings, subnet: Principal, ) -> Result { - // Call the management canister's create_canister, routing the request to - // the target subnet by its id (subnet-scoped, so no canister on that - // subnet is needed to derive an effective canister id) and forwarding the - // settings as a whole via `with_canister_settings`. Subnet-scoped routing - // requires subnet-administrator permissions, which the CloudEngine path has. let mgmt = ManagementCanister::create(&self.inner.agent); let (canister_id,) = mgmt .create_canister() @@ -541,6 +666,18 @@ pub(crate) fn shell_quote(value: &str) -> String { format!("'{}'", value.replace('\'', r"'\''")) } +/// Whether `err` is a replica rejection meaning the target canister does not +/// exist on this network — reject code `DestinationInvalid` (IC0301, +/// "Canister ... not found"). Used to treat a missing engine-canister registry +/// as "no operator registered" so the CloudEngine path can fall back safely. +fn is_canister_not_found(err: &AgentError) -> bool { + matches!( + err, + AgentError::CertifiedReject { reject, .. } | AgentError::UncertifiedReject { reject, .. } + if reject.reject_code == RejectCode::DestinationInvalid + ) +} + async fn get_available_subnets(agent: &Agent) -> Result, CreateOperationError> { let bs = agent .query(&CYCLES_MINTING_CANISTER_PRINCIPAL, "get_default_subnets") @@ -607,4 +744,47 @@ mod tests { // An embedded single quote is closed, escaped, and reopened via `'\''`. assert_eq!(shell_quote("a'b"), r"'a'\''b'"); } + + #[test] + fn detects_canister_not_found_rejections() { + use ic_agent::agent::RejectResponse; + + let not_found = |certified: bool| { + let reject = RejectResponse { + reject_code: RejectCode::DestinationInvalid, + reject_message: "Canister q6cfj-fyaaa-aaaar-qb77q-cai not found".to_string(), + error_code: Some("IC0301".to_string()), + }; + if certified { + AgentError::CertifiedReject { + reject, + operation: None, + } + } else { + AgentError::UncertifiedReject { + reject, + operation: None, + } + } + }; + + // A `DestinationInvalid` rejection means the registry canister is not + // deployed — both the certified and uncertified spellings count. + assert!(is_canister_not_found(¬_found(true))); + assert!(is_canister_not_found(¬_found(false))); + + // Other reject codes (e.g. a canister trap) must NOT be treated as + // "not found" — they should propagate rather than fall back. + assert!(!is_canister_not_found(&AgentError::CertifiedReject { + reject: RejectResponse { + reject_code: RejectCode::CanisterError, + reject_message: "trapped".to_string(), + error_code: None, + }, + operation: None, + })); + + // A non-reject error is never "not found". + assert!(!is_canister_not_found(&AgentError::InvalidReplicaStatus)); + } }