feat(registry): Add node_reward_type to NodeRecord (#1608)

This adds support for node_reward_type to NodeRecord and to AddNodePayload. It adds it as a string to AddNodePayload, but as an enum with validation to NodeRecord.
dfinity · Oct 15, 2024 · 9fc5ab4 · 9fc5ab4
1 parent 0501f7b
commit 9fc5ab4
Show file tree

Hide file tree

Showing 15 changed files with 257 additions and 74 deletions.
diff --git a/rs/nns/test_utils/src/registry.rs b/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_reward_type: None,
     };
 
     (payload, node_public_keys)

diff --git a/rs/orchestrator/src/firewall.rs b/rs/orchestrator/src/firewall.rs
@@ -1036,6 +1036,7 @@ mod tests {
                     hostos_version_id: None,
                     public_ipv4_config: None,
                     domain: None,
+                    node_reward_type: None,
                 }),
             )
             .expect("Failed to add node record.");

diff --git a/rs/orchestrator/src/registration.rs b/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_reward_type: None,
         }
     }
 

diff --git a/rs/prep/src/node.rs b/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_reward_type: None,
         };
 
         assert_eq!(got, want);

diff --git a/rs/protobuf/def/registry/node/v1/node.proto b/rs/protobuf/def/registry/node/v1/node.proto
@@ -22,6 +22,23 @@ message IPv4InterfaceConfig {
   uint32 prefix_length = 3;
 }
 
+// The type of the node.
+enum NodeRewardType {
+  NODE_REWARD_TYPE_UNSPECIFIED = 0;
+  // type0
+  NODE_REWARD_TYPE_TYPE0 = 1;
+  // type1
+  NODE_REWARD_TYPE_TYPE1 = 2;
+  // type2
+  NODE_REWARD_TYPE_TYPE2 = 3;
+  // type3
+  NODE_REWARD_TYPE_TYPE3 = 4;
+  // type3.1
+  NODE_REWARD_TYPE_TYPE3DOT1 = 5;
+  // type1.1
+  NODE_REWARD_TYPE_TYPE1DOT1 = 6;
+}
+
 // A node: one machine running a replica instance.
 message NodeRecord {
   // The endpoint where this node receives xnet messages.
@@ -46,6 +63,8 @@ message NodeRecord {
   // If a Node is to be converted into the ApiBoundaryNode, the domain field should be set.
   optional string domain = 19;
 
+  optional NodeRewardType node_reward_type = 20;
+
   reserved 1, 2, 3, 4, 7, 8, 9, 10, 11, 12, 13, 14;
   reserved "node_id";
   reserved "gossip_advert";

diff --git a/rs/protobuf/src/gen/registry/registry.node.v1.rs b/rs/protobuf/src/gen/registry/registry.node.v1.rs
@@ -50,4 +50,66 @@ pub struct NodeRecord {
     /// If a Node is to be converted into the ApiBoundaryNode, the domain field should be set.
     #[prost(string, optional, tag = "19")]
     pub domain: ::core::option::Option<::prost::alloc::string::String>,
+    #[prost(enumeration = "NodeRewardType", optional, tag = "20")]
+    pub node_reward_type: ::core::option::Option<i32>,
+}
+/// The type of the node.
+#[derive(
+    serde::Serialize,
+    serde::Deserialize,
+    Clone,
+    Copy,
+    Debug,
+    PartialEq,
+    Eq,
+    Hash,
+    PartialOrd,
+    Ord,
+    ::prost::Enumeration,
+)]
+#[repr(i32)]
+pub enum NodeRewardType {
+    Unspecified = 0,
+    /// type0
+    Type0 = 1,
+    /// type1
+    Type1 = 2,
+    /// type2
+    Type2 = 3,
+    /// type3
+    Type3 = 4,
+    /// type3.1
+    Type3dot1 = 5,
+    /// type1.1
+    Type1dot1 = 6,
+}
+impl NodeRewardType {
+    /// String value of the enum field names used in the ProtoBuf definition.
+    ///
+    /// The values are not transformed in any way and thus are considered stable
+    /// (if the ProtoBuf definition does not change) and safe for programmatic use.
+    pub fn as_str_name(&self) -> &'static str {
+        match self {
+            Self::Unspecified => "NODE_REWARD_TYPE_UNSPECIFIED",
+            Self::Type0 => "NODE_REWARD_TYPE_TYPE0",
+            Self::Type1 => "NODE_REWARD_TYPE_TYPE1",
+            Self::Type2 => "NODE_REWARD_TYPE_TYPE2",
+            Self::Type3 => "NODE_REWARD_TYPE_TYPE3",
+            Self::Type3dot1 => "NODE_REWARD_TYPE_TYPE3DOT1",
+            Self::Type1dot1 => "NODE_REWARD_TYPE_TYPE1DOT1",
+        }
+    }
+    /// Creates an enum from field names used in the ProtoBuf definition.
+    pub fn from_str_name(value: &str) -> ::core::option::Option<Self> {
+        match value {
+            "NODE_REWARD_TYPE_UNSPECIFIED" => Some(Self::Unspecified),
+            "NODE_REWARD_TYPE_TYPE0" => Some(Self::Type0),
+            "NODE_REWARD_TYPE_TYPE1" => Some(Self::Type1),
+            "NODE_REWARD_TYPE_TYPE2" => Some(Self::Type2),
+            "NODE_REWARD_TYPE_TYPE3" => Some(Self::Type3),
+            "NODE_REWARD_TYPE_TYPE3DOT1" => Some(Self::Type3dot1),
+            "NODE_REWARD_TYPE_TYPE1DOT1" => Some(Self::Type1dot1),
+            _ => None,
+        }
+    }
 }
diff --git a/rs/protobuf/src/registry/node.rs b/rs/protobuf/src/registry/node.rs
@@ -1,3 +1,21 @@
+use crate::registry::node::v1::NodeRewardType;
+
 #[allow(clippy::all)]
 #[path = "../gen/registry/registry.node.v1.rs"]
 pub mod v1;
+
+impl From<NodeRewardType> for String {
+    fn from(value: NodeRewardType) -> Self {
+        match value {
+            NodeRewardType::Unspecified => {
+                panic!("Cannot create a node type string from unspecified")
+            }
+            NodeRewardType::Type0 => "type0".to_string(),
+            NodeRewardType::Type1 => "type1".to_string(),
+            NodeRewardType::Type2 => "type2".to_string(),
+            NodeRewardType::Type3 => "type3".to_string(),
+            NodeRewardType::Type3dot1 => "type3.1".to_string(),
+            NodeRewardType::Type1dot1 => "type1.1".to_string(),
+        }
+    }
+}
diff --git a/rs/registry/canister/api/src/lib.rs b/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,11 @@ 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 reward type.  Must be a valid type.
+    // See registry/canister/src/mutations/do_add_node.rs, fn `validate_str_as_node_reward_type` for currently
+    // accepted types.
+    pub node_reward_type: Option<String>,
 }
 
 /// The payload of a request to update keys of the existing node.

diff --git a/rs/registry/canister/canister/registry.did b/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_reward_type : opt text;
 };
 
 type AddNodesToSubnetPayload = record {

diff --git a/rs/registry/canister/src/invariants/endpoint.rs b/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(),
         );

diff --git a/rs/registry/canister/src/invariants/firewall.rs b/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_reward_type: None,
         }
     }