Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(registry): Add node_reward_type to NodeRecord #1608

Merged
merged 8 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions rs/nns/test_utils/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions rs/orchestrator/src/firewall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
1 change: 1 addition & 0 deletions rs/orchestrator/src/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ impl NodeRegistration {
// Unused section follows
p2p_flow_endpoints: Default::default(),
prometheus_metrics_endpoint: Default::default(),
node_type: None,
max-dfinity marked this conversation as resolved.
Show resolved Hide resolved
max-dfinity marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
7 changes: 3 additions & 4 deletions rs/prep/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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() {
Expand All @@ -454,6 +452,7 @@ mod node_configuration {
chip_id: None,
public_ipv4_config: None,
domain: None,
node_type: None,
};

assert_eq!(got, want);
Expand Down
19 changes: 19 additions & 0 deletions rs/protobuf/def/registry/node/v1/node.proto
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ message IPv4InterfaceConfig {
uint32 prefix_length = 3;
}

// The type of the node.
enum NodeType {
max-dfinity marked this conversation as resolved.
Show resolved Hide resolved
NODE_TYPE_UNSPECIFIED = 0;
// type0
NODE_TYPE_TYPE0 = 1;
// type1
NODE_TYPE_TYPE1 = 2;
// type2
NODE_TYPE_TYPE2 = 3;
// type3
NODE_TYPE_TYPE3 = 4;
// type3.1
NODE_TYPE_TYPE3DOT1 = 5;
// type1.1
NODE_TYPE_TYPE1DOT1 = 6;
}

// A node: one machine running a replica instance.
message NodeRecord {
// The endpoint where this node receives xnet messages.
Expand All @@ -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 NodeType node_type = 20;
max-dfinity marked this conversation as resolved.
Show resolved Hide resolved

reserved 1, 2, 3, 4, 7, 8, 9, 10, 11, 12, 13, 14;
reserved "node_id";
reserved "gossip_advert";
Expand Down
62 changes: 62 additions & 0 deletions rs/protobuf/src/gen/registry/registry.node.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "NodeType", optional, tag = "20")]
pub node_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 NodeType {
Unspecified = 0,
/// type0
Type0 = 1,
/// type1
Type1 = 2,
/// type2
Type2 = 3,
/// type3
Type3 = 4,
/// type3.1
Type3dot1 = 5,
/// type1.1
Type1dot1 = 6,
}
impl NodeType {
/// 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 {
NodeType::Unspecified => "NODE_TYPE_UNSPECIFIED",
NodeType::Type0 => "NODE_TYPE_TYPE0",
NodeType::Type1 => "NODE_TYPE_TYPE1",
NodeType::Type2 => "NODE_TYPE_TYPE2",
NodeType::Type3 => "NODE_TYPE_TYPE3",
NodeType::Type3dot1 => "NODE_TYPE_TYPE3DOT1",
NodeType::Type1dot1 => "NODE_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_TYPE_UNSPECIFIED" => Some(Self::Unspecified),
"NODE_TYPE_TYPE0" => Some(Self::Type0),
"NODE_TYPE_TYPE1" => Some(Self::Type1),
"NODE_TYPE_TYPE2" => Some(Self::Type2),
"NODE_TYPE_TYPE3" => Some(Self::Type3),
"NODE_TYPE_TYPE3DOT1" => Some(Self::Type3dot1),
"NODE_TYPE_TYPE1DOT1" => Some(Self::Type1dot1),
_ => None,
}
}
}
18 changes: 18 additions & 0 deletions rs/protobuf/src/registry/node.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
use crate::registry::node::v1::NodeType;

#[allow(clippy::all)]
#[path = "../gen/registry/registry.node.v1.rs"]
pub mod v1;

impl From<NodeType> for String {
fn from(value: NodeType) -> Self {
max-dfinity marked this conversation as resolved.
Show resolved Hide resolved
match value {
NodeType::Unspecified => {
panic!("Cannot create a node type string from unspecified")
}
NodeType::Type0 => "type0".to_string(),
NodeType::Type1 => "type1".to_string(),
NodeType::Type2 => "type2".to_string(),
NodeType::Type3 => "type3".to_string(),
NodeType::Type3dot1 => "type3.1".to_string(),
NodeType::Type1dot1 => "type1.1".to_string(),
}
}
}
10 changes: 6 additions & 4 deletions rs/registry/canister/api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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 type. Must be a valid type.
// See registry/canister/src/mutations/do_add_node.rs, fn `try_str_to_node_type` for currently
// accepted types.
pub node_type: Option<String>,
}

/// The payload of a request to update keys of the existing node.
Expand Down
1 change: 1 addition & 0 deletions rs/registry/canister/canister/registry.did
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 4 additions & 12 deletions rs/registry/canister/src/invariants/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
Expand All @@ -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(),
);
Expand Down Expand Up @@ -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(),
);
Expand Down
6 changes: 4 additions & 2 deletions rs/registry/canister/src/invariants/firewall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -339,6 +340,7 @@ mod tests {
chip_id: None,
public_ipv4_config: None,
domain: None,
node_type: None,
}
}

Expand Down
Loading
Loading