Partially working POC without tests

rs/nns/test_utils/src/registry.rs

@@ -15,14 +15,16 @@ use ic_nervous_system_common_test_keys::{
     TEST_USER1_PRINCIPAL, TEST_USER2_PRINCIPAL, TEST_USER3_PRINCIPAL, TEST_USER4_PRINCIPAL,
     TEST_USER5_PRINCIPAL, TEST_USER6_PRINCIPAL, TEST_USER7_PRINCIPAL,
 };
-use ic_protobuf::registry::subnet::v1::{ChainKeyConfig, InitialNiDkgTranscriptRecord};
 use ic_protobuf::registry::{
     crypto::v1::{PublicKey, X509PublicKeyCert},
     node::v1::{ConnectionEndpoint, NodeRecord},
     node_operator::v1::NodeOperatorRecord,
     replica_version::v1::{BlessedReplicaVersions, ReplicaVersionRecord},
     routing_table::v1::RoutingTable as RoutingTablePB,
-    subnet::v1::{CatchUpPackageContents, SubnetListRecord, SubnetRecord},
+    subnet::v1::{
+        CatchUpPackageContents, ChainKeyConfig, InitialNiDkgTranscriptRecord, SubnetListRecord,
+        SubnetRecord,
+    },
 };
 use ic_registry_canister_api::AddNodePayload;
 use ic_registry_keys::{
@@ -42,9 +44,11 @@ use ic_registry_transport::{
     serialize_get_value_request, Error,
 };
 use ic_test_utilities_types::ids::{subnet_test_id, user_test_id};
-use ic_types::crypto::threshold_sig::ni_dkg::{NiDkgTag, NiDkgTargetId, NiDkgTranscript};
 use ic_types::{
-    crypto::{CurrentNodePublicKeys, KeyPurpose},
+    crypto::{
+        threshold_sig::ni_dkg::{NiDkgTag, NiDkgTargetId, NiDkgTranscript},
+        CurrentNodePublicKeys, KeyPurpose,
+    },
     NodeId, ReplicaVersion,
 };
 use maplit::btreemap;
@@ -54,8 +58,10 @@ use rand::RngCore;
 use registry_canister::mutations::node_management::{
     common::make_add_node_registry_mutations, do_add_node::connection_endpoint_from_string,
 };
-use std::collections::{BTreeMap, BTreeSet};
-use std::convert::TryFrom;
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    convert::TryFrom,
+};
 
 /// ID used in multiple tests.
 pub const TEST_ID: u64 = 999;
@@ -827,6 +833,7 @@ pub fn prepare_add_node_payload(mutation_id: u8) -> (AddNodePayload, ValidNodePu
         // Unused section follows
         p2p_flow_endpoints: Default::default(),
         prometheus_metrics_endpoint: Default::default(),
+        node_type: None,
     };
 
     (payload, node_public_keys)

rs/orchestrator/src/firewall.rs

@@ -1037,6 +1037,7 @@ mod tests {
                     hostos_version_id: None,
                     public_ipv4_config: None,
                     domain: None,
+                    node_type: None,
                 }),
             )
             .expect("Failed to add node record.");

rs/orchestrator/src/registration.rs

@@ -222,6 +222,7 @@ impl NodeRegistration {
             // Unused section follows
             p2p_flow_endpoints: Default::default(),
             prometheus_metrics_endpoint: Default::default(),
+            node_type: None,
         }
     }
 

rs/prep/src/node.rs

@@ -30,8 +30,7 @@ use ic_types::{
     consensus::certification::Certification, crypto::KeyPurpose, Height, NodeId, PrincipalId,
     RegistryVersion, SubnetId,
 };
-use std::net::SocketAddr;
-use std::os::unix::fs::PermissionsExt;
+use std::{net::SocketAddr, os::unix::fs::PermissionsExt};
 
 const CRYPTO_DIR: &str = "crypto";
 const STATE_DIR: &str = "state";
@@ -426,8 +425,7 @@ impl NodeSecretKeyStore {
 mod node_configuration {
     use super::*;
     use pretty_assertions::assert_eq;
-    use std::net::SocketAddr;
-    use std::str::FromStr;
+    use std::{net::SocketAddr, str::FromStr};
 
     #[test]
     fn into_proto_http() {
@@ -454,6 +452,7 @@ mod node_configuration {
             chip_id: None,
             public_ipv4_config: None,
             domain: None,
+            node_type: None,
         };
 
         assert_eq!(got, want);

rs/registry/canister/api/src/lib.rs

@@ -1,10 +1,7 @@
 use candid::{CandidType, Deserialize};
 use ic_base_types::NodeId;
 use serde::Serialize;
-use std::collections::HashSet;
-use std::fmt;
-use std::net::Ipv4Addr;
-use std::str::FromStr;
+use std::{collections::HashSet, fmt, net::Ipv4Addr, str::FromStr};
 use thiserror::Error;
 
 #[derive(Debug, Error)]
@@ -186,6 +183,10 @@ pub struct AddNodePayload {
     // TODO(NNS1-2444): The fields below are deprecated and they are not read anywhere.
     pub p2p_flow_endpoints: Vec<String>,
     pub prometheus_metrics_endpoint: String,
+
+    // String representation of the node type.  Must be a valid type.
+    // Currently valid types are type0, type1, type2, type3, and type3.1.
+    pub node_type: Option<String>,
 }
 
 /// The payload of a request to update keys of the existing node.

rs/registry/canister/canister/registry.did

@@ -42,6 +42,7 @@ type AddNodePayload = record {
   transport_tls_cert : blob;
   ni_dkg_dealing_encryption_pk : blob;
   p2p_flow_endpoints : vec text;
+  node_type : opt text;
 };
 
 type AddNodesToSubnetPayload = record {

rs/registry/canister/src/invariants/endpoint.rs

@@ -391,10 +391,7 @@ mod tests {
                     ip_addr: "200.1.1.3".to_string(),
                     port: 9001,
                 }),
-                hostos_version_id: None,
-                chip_id: None,
-                public_ipv4_config: None,
-                domain: None,
+                ..Default::default()
             }
             .encode_to_vec(),
         );
@@ -421,10 +418,8 @@ mod tests {
                     ip_addr: "200.1.1.1".to_string(),
                     port: 9001,
                 }),
-                hostos_version_id: None,
-                chip_id: None,
-                public_ipv4_config: None,
-                domain: None,
+
+                ..Default::default()
             }
             .encode_to_vec(),
         );
@@ -458,10 +453,7 @@ mod tests {
                     ip_addr: "200.1.1.2".to_string(),
                     port: 9001,
                 }),
-                hostos_version_id: None,
-                chip_id: None,
-                public_ipv4_config: None,
-                domain: None,
+                ..Default::default()
             }
             .encode_to_vec(),
         );

rs/registry/canister/src/invariants/firewall.rs

@@ -284,8 +284,9 @@ fn get_firewall_rules(snapshot: &RegistrySnapshot, record_key: String) -> Option
 #[cfg(test)]
 mod tests {
     use super::*;
-    use ic_protobuf::registry::subnet::v1::SubnetListRecord;
-    use ic_protobuf::registry::{firewall::v1::FirewallRuleDirection, node::v1::NodeRecord};
+    use ic_protobuf::registry::{
+        firewall::v1::FirewallRuleDirection, node::v1::NodeRecord, subnet::v1::SubnetListRecord,
+    };
     use ic_registry_keys::{make_node_record_key, make_subnet_list_record_key};
     use prost::Message;
 
@@ -339,6 +340,7 @@ mod tests {
             chip_id: None,
             public_ipv4_config: None,
             domain: None,
+            node_type: None,
         }
     }
 

rs/registry/canister/src/mutations/node_management/do_add_node.rs

@@ -10,7 +10,7 @@ use ic_crypto_node_key_validation::ValidNodePublicKeys;
 use ic_crypto_utils_basic_sig::conversions as crypto_basicsig_conversions;
 use ic_protobuf::registry::{
     crypto::v1::{PublicKey, X509PublicKeyCert},
-    node::v1::{ConnectionEndpoint, IPv4InterfaceConfig, NodeRecord},
+    node::v1::{ConnectionEndpoint, IPv4InterfaceConfig, NodeRecord, NodeType},
 };
 use idna::domain_to_ascii_strict;
 
@@ -75,11 +75,26 @@ impl Registry {
             ));
         }
 
-        // 3. Validate keys and get the node id
+        // 3. Get valid type if type is in request
+        let node_type = payload
+            .node_type
+            .as_ref()
+            .map(|t| {
+                try_str_to_node_type(t).map_err(|e| {
+                    format!(
+                        "{}do_add_node: Error parsing node type from payload: {}",
+                        LOG_PREFIX, e
+                    )
+                })
+            })
+            .transpose()?
+            .map(|node_type| node_type as i32);
+
+        // 4. Validate keys and get the node id
         let (node_id, valid_pks) = valid_keys_from_payload(&payload)
             .map_err(|err| format!("{}do_add_node: {}", LOG_PREFIX, err))?;
 
-        // 4. Validate the domain is valid
+        // 5. Validate the domain is valid
         let domain: Option<String> = payload
             .domain
             .as_ref()
@@ -93,7 +108,7 @@ impl Registry {
             })
             .transpose()?;
 
-        // 5. If there is an IPv4 config, make sure that the IPv4 is not used by anyone else
+        // 6. If there is an IPv4 config, make sure that the IPv4 is not used by anyone else
         let ipv4_intf_config = payload.public_ipv4_config.clone().map(|ipv4_config| {
             ipv4_config.panic_on_invalid();
             IPv4InterfaceConfig {
@@ -113,7 +128,7 @@ impl Registry {
 
         println!("{}do_add_node: The node id is {:?}", LOG_PREFIX, node_id);
 
-        // 6. Create the Node Record
+        // 7. Create the Node Record
         let node_record = NodeRecord {
             xnet: Some(connection_endpoint_from_string(&payload.xnet_endpoint)),
             http: Some(connection_endpoint_from_string(&payload.http_endpoint)),
@@ -125,18 +140,18 @@ impl Registry {
             node_type,
         };
 
-        // 7. Insert node, public keys, and crypto keys
+        // 8. Insert node, public keys, and crypto keys
         let mut mutations = make_add_node_registry_mutations(node_id, node_record, valid_pks);
 
-        // 8. Update the Node Operator record
+        // 9. Update the Node Operator record
         node_operator_record.node_allowance -= 1;
 
         let update_node_operator_record =
             make_update_node_operator_mutation(caller_id, &node_operator_record);
 
         mutations.push(update_node_operator_record);
 
-        // 9. Check invariants before applying mutations
+        // 10. Check invariants before applying mutations
         self.maybe_apply_mutation_internal(mutations);
 
         println!("{}do_add_node finished: {:?}", LOG_PREFIX, payload);
@@ -145,6 +160,18 @@ impl Registry {
     }
 }
 
+// try to convert input string inot NodeType enum
+fn try_str_to_node_type(type_string: &str) -> Result<NodeType, String> {
+    Ok(match type_string {
+        "type0" => NodeType::Type0,
+        "type1" => NodeType::Type1,
+        "type2" => NodeType::Type2,
+        "type3" => NodeType::Type3,
+        "type3.1" => NodeType::Type3dot1,
+        _ => return Err(format!("Invalid node type: {}", type_string)),
+    })
+}
+
 /// Parses the ConnectionEndpoint string
 ///
 /// The string is written in form: `ipv4:port` or `[ipv6]:port`.
@@ -302,6 +329,7 @@ mod tests {
             // Unused section follows
             p2p_flow_endpoints: Default::default(),
             prometheus_metrics_endpoint: Default::default(),
+            node_type: None,
         };
 
         (payload, node_public_keys)
@@ -340,6 +368,7 @@ mod tests {
             // Unused section follows
             p2p_flow_endpoints: Default::default(),
             prometheus_metrics_endpoint: Default::default(),
+            node_type: None,
         };
     }
 

rs/registry/helpers/src/firewall.rs

@@ -1,18 +1,15 @@
-use crate::deserialize_registry_value;
-use crate::node::NodeRecord;
+use crate::{deserialize_registry_value, node::NodeRecord};
 use ic_interfaces_registry::{RegistryClient, RegistryClientResult};
-use ic_protobuf::registry::firewall::v1::FirewallConfig;
-use ic_protobuf::registry::firewall::v1::FirewallRuleSet;
-use ic_protobuf::registry::node::v1::ConnectionEndpoint;
-use ic_registry_keys::get_node_record_node_id;
-use ic_registry_keys::make_firewall_config_record_key;
-use ic_registry_keys::make_firewall_rules_record_key;
-use ic_registry_keys::make_node_record_key;
-use ic_registry_keys::FirewallRulesScope;
-use ic_registry_keys::NODE_RECORD_KEY_PREFIX;
+use ic_protobuf::registry::{
+    firewall::v1::{FirewallConfig, FirewallRuleSet},
+    node::v1::ConnectionEndpoint,
+};
+use ic_registry_keys::{
+    get_node_record_node_id, make_firewall_config_record_key, make_firewall_rules_record_key,
+    make_node_record_key, FirewallRulesScope, NODE_RECORD_KEY_PREFIX,
+};
 use ic_types::{NodeId, RegistryVersion};
-use std::collections::HashSet;
-use std::net::IpAddr;
+use std::{collections::HashSet, net::IpAddr};
 
 /// A trait that allows access to firewall rules and ancillary information.
 pub trait FirewallRegistry {
@@ -147,11 +144,7 @@ mod tests {
                             ip_addr: ip.to_string(),
                             port: 2457,
                         }),
-                        node_operator_id: vec![],
-                        hostos_version_id: None,
-                        chip_id: None,
-                        public_ipv4_config: None,
-                        domain: None,
+                        ..Default::default()
                     },
                 )
             })