feat(registry): add invariant that cloud engines only contain type4 nodes

rs/registry/canister/src/common/test_helpers.rs

@@ -118,6 +118,7 @@ pub fn prepare_registry_with_nodes_and_node_operator_id(
         nodes,
         node_operator_id,
         false, // with_chip_id
+        NodeRewardType::Type1,
     )
 }
 
@@ -131,6 +132,22 @@ pub fn prepare_registry_with_nodes_and_chip_id(
         nodes,
         PrincipalId::new_user_test_id(999),
         true,
+        NodeRewardType::Type1,
+    )
+}
+
+/// Same as above, just with the possibility to have a node reward type.
+pub fn prepare_registry_with_nodes_and_reward_type(
+    start_mutation_id: u8,
+    nodes: u64,
+    node_reward_type: NodeRewardType,
+) -> (RegistryAtomicMutateRequest, BTreeMap<NodeId, PublicKey>) {
+    prepare_registry_raw(
+        start_mutation_id,
+        nodes,
+        PrincipalId::new_user_test_id(999),
+        false, // with_chip_id
+        node_reward_type,
     )
 }
 
@@ -139,6 +156,7 @@ pub fn prepare_registry_raw(
     nodes: u64,
     node_operator_id: PrincipalId,
     with_chip_id: bool,
+    node_reward_type: NodeRewardType,
 ) -> (RegistryAtomicMutateRequest, BTreeMap<NodeId, PublicKey>) {
     // Prepare a transaction to add the nodes to the registry
     let mut mutations = Vec::<RegistryMutation>::default();
@@ -161,7 +179,7 @@ pub fn prepare_registry_raw(
                 node_operator_id: node_operator_id.into_vec(),
                 // Preset this field to Some(), in order to allow seamless creation of ApiBoundaryNodeRecord if needed.
                 domain: Some(format!("node{effective_id}.example.com")),
-                node_reward_type: Some(NodeRewardType::Type1 as i32),
+                node_reward_type: Some(node_reward_type as i32),
                 chip_id: if with_chip_id {
                     Some(format!("chip-id-{effective_id}").into_bytes())
                 } else {

rs/registry/canister/src/invariants/subnet.rs

@@ -10,8 +10,11 @@ use crate::invariants::common::{
 
 use ic_base_types::{NodeId, PrincipalId, SubnetId};
 use ic_nns_common::registry::MAX_NUM_SSH_KEYS;
-use ic_protobuf::registry::subnet::v1::{CanisterCyclesCostSchedule, SubnetRecord, SubnetType};
-use ic_registry_keys::{SUBNET_RECORD_KEY_PREFIX, make_node_record_key, make_subnet_record_key};
+use ic_protobuf::registry::{
+    node::v1::{NodeRecord, NodeRewardType},
+    subnet::v1::{CanisterCyclesCostSchedule, SubnetRecord, SubnetType},
+};
+use ic_registry_keys::{SUBNET_RECORD_KEY_PREFIX, make_subnet_record_key};
 use prost::Message;
 
 /// Subnet invariants hold iff:
@@ -21,9 +24,13 @@ use prost::Message;
 ///    * Each subnet contains at least one node
 ///    * There is at least one system subnet
 ///    * Each subnet in the registry occurs in the subnet list and vice versa
-///    * Only application subnets can be rented and therefore have a "free" cycles cost schedule
+///    * Only application subnets (when rented) and cloud engines can have a "free" cycles cost schedule
+///    * Cloud engines must:
+///         * have a "free" cycles cost schedule
+///         * consist of nodes with reward type 4
+///    * Conversely, only cloud engines can have nodes with reward type 4
 ///    * SEV-enabled subnets consist of SEV-enabled nodes only (i.e. nodes with a chip ID in the node record)
-/// * Only rented subnets can have subnet admins set to a non-empty list
+///    * Only rented subnets can have subnet admins set to a non-empty list
 pub(crate) fn check_subnet_invariants(
     snapshot: &RegistrySnapshot,
 ) -> Result<(), InvariantCheckError> {
@@ -63,12 +70,17 @@ pub(crate) fn check_subnet_invariants(
             .collect();
 
         // Subnet membership must contain registered nodes only
-        for &node_id in &subnet_members {
-            let node_key = make_node_record_key(node_id);
-            if !snapshot.contains_key(node_key.as_bytes()) {
-                panic!("Node {node_id} does not exist in Subnet {subnet_id}");
-            }
-        }
+        let node_records = subnet_members
+            .iter()
+            .map(|&node_id| {
+                get_node_record_from_snapshot(node_id, snapshot).and_then(|opt| {
+                    opt.ok_or_else(|| InvariantCheckError {
+                        msg: format!("Node {node_id} does not exist in the registry"),
+                        source: None,
+                    })
+                })
+            })
+            .collect::<Result<Vec<_>, _>>()?;
 
         // Each node appears at most once in a subnet membership
         let num_nodes = subnet_record.membership.len();
@@ -128,6 +140,8 @@ pub(crate) fn check_subnet_invariants(
             });
         }
 
+        check_node_type4_iff_cloud_engine(subnet_id, &subnet_record, &node_records)?;
+
         // SEV-enabled subnets invariants
         if let Some(features) = subnet_record.features.as_ref()
             && features.sev_enabled == Some(true)
@@ -248,5 +262,31 @@ fn check_sev_subnet_invariants(
     Ok(())
 }
 
+fn check_node_type4_iff_cloud_engine(
+    subnet_id: SubnetId, // only used for error messages, so we can report which subnet is non-compliant
+    subnet_record: &SubnetRecord,
+    node_records: &[NodeRecord],
+) -> Result<(), InvariantCheckError> {
+    let is_cloud_engine = subnet_record.subnet_type == i32::from(SubnetType::CloudEngine);
+    let is_node_type4 =
+        |node: &NodeRecord| node.node_reward_type == Some(i32::from(NodeRewardType::Type4));
+    let is_node_ok = |node: &NodeRecord| is_cloud_engine == is_node_type4(node);
+
+    let ok = node_records.iter().all(is_node_ok);
+    if !ok {
+        let msg = if is_cloud_engine {
+            "is a cloud engine subnet but some nodes do not have reward type 4"
+        } else {
+            "is not a cloud engine subnet but some nodes have reward type 4"
+        };
+        return Err(InvariantCheckError {
+            msg: format!("Subnet {subnet_id:} {msg}"),
+            source: None,
+        });
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests;

rs/registry/canister/src/invariants/subnet/tests.rs

@@ -4,14 +4,14 @@ use crate::invariants::common::RegistrySnapshot;
 use ic_base_types::SubnetId;
 use ic_protobuf::{
     registry::{
-        node::v1::NodeRecord,
+        node::v1::{NodeRecord, NodeRewardType},
         subnet::v1::{
             CanisterCyclesCostSchedule, SubnetFeatures, SubnetListRecord, SubnetRecord, SubnetType,
         },
     },
     types::v1::PrincipalId as PrincipalIdPb,
 };
-use ic_registry_keys::{make_subnet_list_record_key, make_subnet_record_key};
+use ic_registry_keys::{make_node_record_key, make_subnet_list_record_key, make_subnet_record_key};
 use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, user_test_id};
 
 #[test]
@@ -39,15 +39,6 @@ fn only_application_subnets_and_engines_can_be_free_cycles_cost_schedule() {
     );
     check_subnet_invariants(&snapshot).unwrap();
 
-    // Another happy case: CloudEngine
-    test_subnet_record.subnet_type = i32::from(SubnetType::CloudEngine);
-    test_subnet_record.canister_cycles_cost_schedule = i32::from(CanisterCyclesCostSchedule::Free);
-    snapshot.insert(
-        make_subnet_record_key(test_subnet_id).into_bytes(),
-        test_subnet_record.encode_to_vec(),
-    );
-    check_subnet_invariants(&snapshot).unwrap();
-
     // System or verified application subnets cannot be on "free" cycles cost schedule.
     test_subnet_record.subnet_type = i32::from(SubnetType::System);
     snapshot.insert(
@@ -68,6 +59,14 @@ fn only_application_subnets_and_engines_can_be_free_cycles_cost_schedule() {
         &snapshot,
         "is not an application subnet or CloudEngine but has a free cycles cost schedule",
     );
+
+    // Another happy case: CloudEngine
+    turn_to_compliant_cloud_engine(&mut snapshot, &mut test_subnet_record);
+    snapshot.insert(
+        make_subnet_record_key(test_subnet_id).into_bytes(),
+        test_subnet_record.encode_to_vec(),
+    );
+    check_subnet_invariants(&snapshot).unwrap();
 }
 
 #[test]
@@ -226,8 +225,7 @@ fn cloud_engine_subnets_can_have_subnet_admins() {
         );
 
     // CloudEngine subnets can have subnet admins with free cost schedule.
-    test_subnet_record.subnet_type = i32::from(SubnetType::CloudEngine);
-    test_subnet_record.canister_cycles_cost_schedule = i32::from(CanisterCyclesCostSchedule::Free);
+    turn_to_compliant_cloud_engine(&mut snapshot, &mut test_subnet_record);
     test_subnet_record.subnet_admins = vec![PrincipalIdPb::from(user_test_id(1).get())];
     snapshot.insert(
         make_subnet_record_key(test_subnet_id).into_bytes(),
@@ -271,6 +269,97 @@ fn non_rented_application_subnets_cannot_have_subnet_admins() {
     );
 }
 
+#[test]
+fn cloud_engine_subnets_must_have_type4_nodes() {
+    let system_subnet_id = subnet_test_id(1);
+    let test_subnet_id = subnet_test_id(2);
+    let (mut snapshot, mut test_subnet_record) =
+        setup_minimal_registry_snapshot_for_check_subnet_invariants(
+            system_subnet_id,
+            test_subnet_id,
+            1,     // num_nodes_in_test_subnet
+            false, // with_chip_id
+        );
+
+    // Happy case: CloudEngine subnet with Type4 nodes.
+    turn_to_compliant_cloud_engine(&mut snapshot, &mut test_subnet_record);
+    snapshot.insert(
+        make_subnet_record_key(test_subnet_id).into_bytes(),
+        test_subnet_record.encode_to_vec(),
+    );
+    check_subnet_invariants(&snapshot).unwrap();
+
+    // Sad case: CloudEngine subnet with nodes that have a non-Type4 reward type.
+    for reward_type in [
+        None,
+        Some(NodeRewardType::Unspecified),
+        Some(NodeRewardType::Type0),
+        Some(NodeRewardType::Type1),
+        Some(NodeRewardType::Type2),
+        Some(NodeRewardType::Type3),
+        Some(NodeRewardType::Type3dot1),
+        Some(NodeRewardType::Type1dot1),
+    ] {
+        println!(
+            "Ensuring that a node with reward type {reward_type:?} cannot be part of a CloudEngine subnet"
+        );
+        set_node_reward_type(&mut snapshot, node_test_id(100), reward_type);
+        assert_non_compliant_record(
+            &snapshot,
+            "is a cloud engine subnet but some nodes do not have reward type 4",
+        );
+    }
+}
+
+#[test]
+fn only_cloud_engine_subnets_can_have_type4_nodes() {
+    let system_subnet_id = subnet_test_id(1);
+    let test_subnet_id = subnet_test_id(2);
+    let (mut snapshot, mut test_subnet_record) =
+        setup_minimal_registry_snapshot_for_check_subnet_invariants(
+            system_subnet_id,
+            test_subnet_id,
+            1,     // num_nodes_in_test_subnet
+            false, // with_chip_id
+        );
+
+    // Trivial case: non-cloud-engine subnet with non-Type4 nodes is compliant.
+    check_subnet_invariants(&snapshot).unwrap();
+
+    // Sad case: Application subnet with Type4 node.
+    set_node_reward_type(
+        &mut snapshot,
+        node_test_id(100),
+        Some(NodeRewardType::Type4),
+    );
+    assert_non_compliant_record(
+        &snapshot,
+        "is not a cloud engine subnet but some nodes have reward type 4",
+    );
+
+    // Sad case: System subnet with Type4 node.
+    test_subnet_record.subnet_type = i32::from(SubnetType::System);
+    snapshot.insert(
+        make_subnet_record_key(test_subnet_id).into_bytes(),
+        test_subnet_record.encode_to_vec(),
+    );
+    assert_non_compliant_record(
+        &snapshot,
+        "is not a cloud engine subnet but some nodes have reward type 4",
+    );
+
+    // Sad case: VerifiedApplication subnet with Type4 node.
+    test_subnet_record.subnet_type = i32::from(SubnetType::VerifiedApplication);
+    snapshot.insert(
+        make_subnet_record_key(test_subnet_id).into_bytes(),
+        test_subnet_record.encode_to_vec(),
+    );
+    assert_non_compliant_record(
+        &snapshot,
+        "is not a cloud engine subnet but some nodes have reward type 4",
+    );
+}
+
 fn setup_minimal_registry_snapshot_for_check_subnet_invariants(
     system_subnet_id: SubnetId,
     test_subnet_id: SubnetId,
@@ -341,6 +430,31 @@ fn setup_minimal_registry_snapshot_for_check_subnet_invariants(
     (snapshot, test_subnet_record)
 }
 
+fn turn_to_compliant_cloud_engine(
+    snapshot: &mut RegistrySnapshot,
+    subnet_record: &mut SubnetRecord,
+) {
+    subnet_record.subnet_type = i32::from(SubnetType::CloudEngine);
+    // An invariant ensures the cost schedule is free
+    subnet_record.canister_cycles_cost_schedule = i32::from(CanisterCyclesCostSchedule::Free);
+    // An invariant ensures that all nodes have reward type 4
+    for node_bytes in &subnet_record.membership {
+        let node_id = NodeId::from(PrincipalId::try_from(node_bytes.as_slice()).unwrap());
+        set_node_reward_type(snapshot, node_id, Some(NodeRewardType::Type4));
+    }
+}
+
+fn set_node_reward_type(
+    snapshot: &mut RegistrySnapshot,
+    node_id: NodeId,
+    reward_type: Option<NodeRewardType>,
+) {
+    let key = make_node_record_key(node_id).into_bytes();
+    let mut node_record = NodeRecord::decode(snapshot.get(&key).unwrap().as_slice()).unwrap();
+    node_record.node_reward_type = reward_type.map(i32::from);
+    snapshot.insert(key, node_record.encode_to_vec());
+}
+
 fn assert_non_compliant_record(snapshot: &RegistrySnapshot, error_msg: &str) {
     let Err(err) = check_subnet_invariants(snapshot) else {
         panic!("Expected Err, but got Ok!");

rs/registry/canister/src/mutations/do_split_subnet.rs

@@ -474,14 +474,15 @@ mod tests {
     use crate::{
         common::test_helpers::{
             add_fake_subnet, get_invariant_compliant_subnet_record, invariant_compliant_registry,
-            prepare_registry_with_nodes,
+            prepare_registry_with_nodes_and_reward_type,
         },
         flags::{temporarily_disable_subnet_splitting, temporarily_enable_subnet_splitting},
         mutations::routing_table::routing_table_into_registry_mutation,
     };
     use ic_management_canister_types_private::{EcdsaCurve, EcdsaKeyId, MasterPublicKeyId};
     use ic_protobuf::registry::{
         crypto::v1::PublicKey,
+        node::v1::NodeRewardType,
         subnet::v1::{ChainKeyConfig as ChainKeyConfigPb, KeyConfig as KeyConfigPb},
     };
     use ic_protobuf::types::v1::MasterPublicKeyId as MasterPublicKeyIdPb;
@@ -797,7 +798,16 @@ mod tests {
 
         // Add nodes to the registry
         let (mutate_request, source_node_ids_and_dkg_pks) =
-            prepare_registry_with_nodes(1, FAKE_NODE_IDS_IN_THE_REGISTRY.len() as u64);
+            prepare_registry_with_nodes_and_reward_type(
+                1,
+                FAKE_NODE_IDS_IN_THE_REGISTRY.len() as u64,
+                // There is an invariant that ensures that all cloud engine nodes have reward type 4
+                if source_subnet_info.subnet_type == SubnetType::CloudEngine {
+                    NodeRewardType::Type4
+                } else {
+                    NodeRewardType::Type1
+                },
+            );
         registry.maybe_apply_mutation_internal(mutate_request.mutations);
         let node_infos: BTreeMap<_, _> = FAKE_NODE_IDS_IN_THE_REGISTRY
             .iter()

rs/registry/canister/unreleased_changelog.md

@@ -15,6 +15,8 @@ on the process that this file is part of, see
   sets a soft limit on the maximum (replicated) state *delta* (kept in main memory) in bytes.
 * Added an optional field `resource_limits` to `UpdateSubnetPayload` which, when present,
   sets all subnet resource limits to the provided values.
+* New invariant ensuring that cloud engines contain only nodes with `type4` reward type and that
+  non-cloud engines do not contain any nodes with `type4` reward type.
 
 ## Changed