NNS1-1925 - Add the controllers field to the CanisterStatusResult s…

…erved by NNS Root `canister_status` API

rs/nervous_system/root/BUILD.bazel

@@ -11,6 +11,7 @@ DEPENDENCIES = [
     "//rs/types/ic00_types",
     "@crate_index//:candid",
     "@crate_index//:ic-cdk",
+    "@crate_index//:lazy_static",
     "@crate_index//:serde",
     "@crate_index//:serde_bytes",
 ]

rs/nervous_system/root/Cargo.toml

@@ -14,5 +14,6 @@ ic-cdk = { version = "0.6.0", default-features = false }
 ic-crypto-sha = { path = "../../crypto/sha/" }
 ic-nervous-system-common = { path="../common" }
 ic-ic00-types = { path = "../../types/ic00_types" }
+lazy_static = "1.4.0"
 serde = { version = "1.0.99", features = ["derive"] }
 serde_bytes = "0.11"

rs/nervous_system/root/src/lib.rs

@@ -4,13 +4,24 @@ use ic_base_types::PrincipalId;
 use ic_crypto_sha::Sha256;
 use ic_ic00_types::{CanisterInstallMode, InstallCodeArgs, IC_00};
 use ic_nervous_system_common::MethodAuthzChange;
-
+use lazy_static::lazy_static;
 use serde::Serialize;
+use std::str::FromStr;
 
 pub const LOG_PREFIX: &str = "[Root Canister] ";
 
+// Copied from /rs/replicated_state/src/canister_state/system_state.rs because the
+// fields are not exported from there. These values may be present in the response
+// from the replica and as such should be overridden.
+lazy_static! {
+    pub static ref DEFAULT_PRINCIPAL_MULTIPLE_CONTROLLERS: PrincipalId =
+        PrincipalId::from_str("ifxlm-aqaaa-multi-pleco-ntrol-lersa-h3ae").unwrap();
+    pub static ref DEFAULT_PRINCIPAL_ZERO_CONTROLLERS: PrincipalId =
+        PrincipalId::from_str("zrl4w-cqaaa-nocon-troll-eraaa-d5qc").unwrap();
+}
+
 /// Copied from ic-types::ic_00::CanisterIdRecord.
-#[derive(CandidType, Deserialize, Debug)]
+#[derive(CandidType, Deserialize, Debug, Clone, Copy)]
 pub struct CanisterIdRecord {
     canister_id: CanisterId,
 }
@@ -51,23 +62,86 @@ impl std::fmt::Display for CanisterStatusType {
     }
 }
 
-/// Partial copy-paste of ic-types::ic_00::CanisterStatusResult.
+/// Partial copy-paste of ic-types::ic_00::DefiniteCanisterSettings.
 ///
 /// Only the fields that we need are copied.
 /// Candid deserialization is supposed to be tolerant to having data for unknown
 /// fields (which is simply discarded).
 #[derive(CandidType, Debug, Deserialize, Eq, PartialEq)]
+pub struct DefiniteCanisterSettings {
+    pub controllers: Vec<PrincipalId>,
+}
+
+/// Partial copy-paste of ic-types::ic_00::CanisterStatusResult.
+///
+/// Only the fields that we need are copied.
+/// Candid deserialization is supposed to be tolerant to having data for unknown
+/// fields (which are simply discarded).
+#[derive(CandidType, Debug, Deserialize, Eq, PartialEq)]
 pub struct CanisterStatusResult {
     pub status: CanisterStatusType,
     pub module_hash: Option<Vec<u8>>,
+    // TODO NNS1-2170 - Remove this field when our clients no longer depend on it.
     pub controller: PrincipalId,
     pub memory_size: candid::Nat,
+    pub settings: DefiniteCanisterSettings,
+}
+
+/// Partial copy-paste of ic-types::ic_00::CanisterStatusResult.
+///
+/// Only the fields we need and are supported from the replica are copied.
+/// Notice that `controller` is not present. Candid deserialization is tolerant
+/// to having data for unknown fields (which are simply discarded).
+#[derive(CandidType, Debug, Deserialize, Eq, PartialEq)]
+struct CanisterStatusResultFromManagementCanister {
+    // no controller. This is fine regardless of whether it sends us controller.
+    pub status: CanisterStatusType,
+    pub module_hash: Option<Vec<u8>>,
+    pub memory_size: candid::Nat,
+    pub settings: DefiniteCanisterSettings,
 }
 
 impl CanisterStatusResult {
     pub fn controller(&self) -> PrincipalId {
         self.controller
     }
+
+    /// Overrides any value returned in the non-standard and deprecated field `controller`.
+    /// This field can be deprecated from the CanisterStatusResult after downstream clients
+    /// have moved from its use. For now, the method severs the tie between the response
+    /// from the IC Interface and the response served to clients of NNS Root.
+    ///
+    /// If the controllers field is empty, this method follows the convention set by the
+    /// IC Interface and fills in the Default Principal for the required controller field.
+    fn fill_controller_field(self) -> Self {
+        let controllers = self.settings.controllers.clone();
+
+        // Let's set `controller` to be the first principal in `controllers`.
+        return if let Some(controller) = controllers.first() {
+            Self {
+                controller: *controller,
+                ..self
+            }
+        } else {
+            Self {
+                controller: *DEFAULT_PRINCIPAL_ZERO_CONTROLLERS,
+                ..self
+            }
+        };
+    }
+}
+
+impl From<CanisterStatusResultFromManagementCanister> for CanisterStatusResult {
+    fn from(value: CanisterStatusResultFromManagementCanister) -> Self {
+        CanisterStatusResult {
+            controller: PrincipalId::new_anonymous(),
+            status: value.status,
+            module_hash: value.module_hash,
+            memory_size: value.memory_size,
+            settings: value.settings,
+        }
+        .fill_controller_field()
+    }
 }
 
 /// The payload to a proposal to upgrade a canister.
@@ -390,13 +464,83 @@ where
     }
 }
 
-pub async fn canister_status(canister_id_record: CanisterIdRecord) -> CanisterStatusResult {
+pub async fn canister_status(
+    canister_id_record: CanisterIdRecord,
+) -> Result<CanisterStatusResult, (Option<i32>, String)> {
     call(
         IC_00,
         "canister_status",
-        dfn_candid::candid,
+        dfn_candid::candid::<CanisterStatusResultFromManagementCanister, (CanisterIdRecord,)>,
         (canister_id_record,),
     )
     .await
-    .unwrap()
+    .map(CanisterStatusResult::from)
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::{
+        CanisterStatusResult, CanisterStatusResultFromManagementCanister, CanisterStatusType,
+        DefiniteCanisterSettings, DEFAULT_PRINCIPAL_ZERO_CONTROLLERS,
+    };
+    use ic_base_types::PrincipalId;
+
+    #[test]
+    fn test_canister_status_result_from_management_sets_controller_when_multiple_are_present() {
+        let test_principal_1 = PrincipalId::new_user_test_id(1);
+        let test_principal_2 = PrincipalId::new_user_test_id(2);
+        let status = CanisterStatusResult::from(CanisterStatusResultFromManagementCanister {
+            status: CanisterStatusType::Running,
+            module_hash: None,
+            memory_size: Default::default(),
+            settings: DefiniteCanisterSettings {
+                controllers: vec![test_principal_1, test_principal_2],
+            },
+        });
+        assert_eq!(status.controller(), test_principal_1);
+    }
+
+    #[test]
+    fn test_canister_status_result_from_management_sets_controller_when_none_are_present() {
+        let status = CanisterStatusResult::from(CanisterStatusResultFromManagementCanister {
+            memory_size: Default::default(),
+            settings: DefiniteCanisterSettings {
+                controllers: vec![],
+            },
+            status: CanisterStatusType::Running,
+            module_hash: None,
+        });
+        assert_eq!(status.controller(), *DEFAULT_PRINCIPAL_ZERO_CONTROLLERS);
+    }
+
+    #[test]
+    fn test_canister_status_result_from_trait_for_canister_status_result_from_management_canister()
+    {
+        let test_principal = PrincipalId::new_user_test_id(1);
+        let m = CanisterStatusResultFromManagementCanister {
+            status: CanisterStatusType::Running,
+            module_hash: Some(vec![1, 2, 3]),
+            memory_size: candid::Nat::from(100),
+            settings: DefiniteCanisterSettings {
+                controllers: vec![test_principal],
+            },
+        };
+
+        let expected_canister_status_result = CanisterStatusResult {
+            status: CanisterStatusType::Running,
+            module_hash: Some(vec![1, 2, 3]),
+            controller: test_principal,
+            memory_size: candid::Nat::from(100),
+            settings: DefiniteCanisterSettings {
+                controllers: vec![test_principal],
+            },
+        };
+
+        let actual_canister_status_result = CanisterStatusResult::from(m);
+
+        assert_eq!(
+            actual_canister_status_result,
+            expected_canister_status_result
+        );
+    }
 }

rs/nns/handlers/lifeline/tests/test.rs

@@ -1,6 +1,5 @@
 use dfn_candid::candid;
-use ic_ic00_types::CanisterIdRecord;
-use ic_nervous_system_root::CanisterStatusResult;
+use ic_ic00_types::{CanisterIdRecord, CanisterStatusResult};
 use ic_nns_common::init::LifelineCanisterInitPayloadBuilder;
 use ic_nns_test_utils::itest_helpers::{
     local_test_on_nns_subnet, set_up_lifeline_canister, set_up_universal_canister,

rs/nns/handlers/root/BUILD.bazel

@@ -15,6 +15,7 @@ filegroup(
 DEPENDENCIES = [
     "//rs/crypto/sha",
     "//rs/nervous_system/root",
+    "//rs/nervous_system/common",
     "//rs/nns/common",
     "//rs/nns/constants",
     "//rs/nns/governance",
@@ -25,10 +26,12 @@ DEPENDENCIES = [
     "//rs/rust_canisters/dfn_candid",
     "//rs/rust_canisters/dfn_core",
     "//rs/rust_canisters/on_wire",
+    "//rs/rust_canisters/http_types",
     "//rs/types/base_types",
     "//rs/types/ic00_types",
     "@crate_index//:build-info",
     "@crate_index//:candid",
+    "@crate_index//:ic-metrics-encoder",
     "@crate_index//:prost",
     "@crate_index//:serde",
     "@crate_index//:serde_bytes",
@@ -59,6 +62,7 @@ DEV_DEPENDENCIES = select({
         "@crate_index//:assert_matches",
         "@crate_index//:hex",
         "@crate_index//:lazy_static",
+        "@crate_index//:maplit",
     ],
 })
 

rs/nns/handlers/root/Cargo.toml

@@ -23,9 +23,12 @@ dfn_candid = {path="../../../rust_canisters/dfn_candid"}
 dfn_core = { path = "../../../rust_canisters/dfn_core" }
 dfn_macro = { path = "../../../rust_canisters/dfn_macro" }
 ic-base-types = {path="../../../types/base_types"}
+ic-canisters-http-types = { path = "../../../rust_canisters/http_types" }
 ic-crypto-sha = {path = "../../../crypto/sha/"}
 ic-ic00-types = {path="../../../types/ic00_types"}
+ic-metrics-encoder = "1"
 ic-nervous-system-common-build-metadata = { path = "../../../nervous_system/common/build_metadata" }
+ic-nervous-system-common = { path = "../../../nervous_system/common" }
 ic-nervous-system-root = { path = "../../../nervous_system/root" }
 ic-nns-common ={path="../../common"}
 ic-nns-constants ={path="../../constants"}
@@ -52,6 +55,7 @@ on_wire = { path = "../../../rust_canisters/on_wire" }
 assert_matches = "1.3.0"
 hex = "0.4.2"
 lazy_static = "1.4.0"
+maplit = "1.0.2"
 registry-canister = { path = "../../../registry/canister" }
 
 [build-dependencies]