Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
115 changes: 107 additions & 8 deletions rs/registry/canister/src/mutations/node_management/do_add_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,19 @@ use ic_registry_keys::{NODE_REWARDS_TABLE_KEY, make_replica_version_key};
use ic_types::{crypto::CurrentNodePublicKeys, time::Time};
use prost::Message;

/// Effective per-node-operator cap on the number of `type4.1`-`type4.4` nodes
/// that may be registered, used in lieu of the standard
/// `max_rewardable_nodes`-based quota for those reward types.
///
/// TODO(CLO-15): Remove this constant and the associated special case in
/// `do_add_node_` once the reward canister no longer treats `type4.5` rewards
/// as `type1.1`. Until then, we cannot meaningfully size
/// `max_rewardable_nodes` for the `type4.x` family without blocking legitimate
/// gen4 onboarding, so we substitute a large-but-bounded sentinel here. The
/// value is chosen to be comfortably above any realistic per-operator
/// deployment while still preventing runaway registrations.
const EXCESSIVE_NUMBER_OF_TYPE_4_NODES: u32 = 1_000;

impl Registry {
/// Adds a new node to the registry.
///
Expand Down Expand Up @@ -136,21 +149,41 @@ impl Registry {
"{LOG_PREFIX}do_add_node: Node reward type is required."
))?;

let max_rewardable_nodes_same_type = *node_operator_record
.max_rewardable_nodes
.get(&(node_reward_type.to_string()))
.ok_or(format!("{LOG_PREFIX}do_add_node: Node Operator does not have rewardable nodes for {node_reward_type}"))?;
// See `EXCESSIVE_NUMBER_OF_TYPE_4_NODES` at the top of this file for
// why type4.1-type4.4 are handled via a sentinel quota rather than
// the per-operator `max_rewardable_nodes` map. Type4.5 is
// explicitly excluded from this exemption.
let is_permissionless_type_4_x = matches!(
node_reward_type,
NodeRewardType::Type4dot1
| NodeRewardType::Type4dot2
| NodeRewardType::Type4dot3
| NodeRewardType::Type4dot4
);

let max_rewardable_nodes_same_type = if is_permissionless_type_4_x {
EXCESSIVE_NUMBER_OF_TYPE_4_NODES
} else {
*node_operator_record
.max_rewardable_nodes
.get(&(node_reward_type.to_string()))
.ok_or(format!("{LOG_PREFIX}do_add_node: Node Operator does not have rewardable nodes for {node_reward_type}"))?
};

let num_in_registry_same_type = get_node_operator_nodes(self, caller_id)
.into_iter()
.filter_map(|node| node.node_reward_type)
.filter(|t| t == &(node_reward_type as i32))
.count() as u32;

// Validate node operator's max_rewardable_nodes quota
if max_rewardable_nodes_same_type
<= num_in_registry_same_type.saturating_sub(num_removed_same_ip_same_type)
{
// Validate node operator's max_rewardable_nodes quota.
// TODO(@pietrodimarco-dfinity): confirm whether `>=` is intended
// here (i.e. `num_remaining_nodes == max_rewardable_nodes_same_type`
// should reject) or whether this should be `>`. Preserving the
// pre-existing behavior for now.
let num_remaining_nodes =
num_in_registry_same_type.saturating_sub(num_removed_same_ip_same_type);
if num_remaining_nodes >= max_rewardable_nodes_same_type {
Comment thread
daniel-wong-dfinity-org-twin marked this conversation as resolved.
return Err(format!(
"{LOG_PREFIX}do_add_node: Node Operator has reached max_rewardable_nodes quota for {node_reward_type}.\
Number of nodes in the registry with {node_reward_type} type = {num_in_registry_same_type},\
Expand Down Expand Up @@ -1124,6 +1157,72 @@ mod tests {
.unwrap();
}

#[test]
fn should_allow_arbitrarily_many_nodes_of_type4dot1_through_type4dot4() {
// TODO(CLO-15): Remove this test once we re-enable the
// max_rewardable_nodes quota check for type4.1-type4.4. See related TODO
// in `do_add_node_`.
for node_reward_type in [
NodeRewardType::Type4dot1,
NodeRewardType::Type4dot2,
NodeRewardType::Type4dot3,
NodeRewardType::Type4dot4,
] {
let mut registry = invariant_compliant_registry(0);

let (mutate_request, node_ids_and_dkg_pks) = prepare_registry_with_nodes(1, 1);
registry.maybe_apply_mutation_internal(mutate_request.mutations);
let node_ids: Vec<NodeId> = node_ids_and_dkg_pks.keys().cloned().collect();
// Note: `max_rewardable_nodes` intentionally does NOT contain
// an entry for `node_reward_type` here, demonstrating that the
// quota does not apply for these types.
let node_operator_id =
registry_add_node_operator_for_node(&mut registry, node_ids[0], btreemap! {});

// Adding several nodes of this type should all succeed even
// though no quota is configured. Note that for other reward types
// this would fail; the immediately following
// `should_panic_if_max_rewardable_nodes_is_exhausted_for_type4dot5`
// test demonstrates that the underlying quota check is still
// exercised, so passing here is meaningful and not simply due to
// the check being a no-op.
for i in 0..3_u8 {
let (payload, _) = prepare_add_node_payload(10 + i, node_reward_type);
registry
.do_add_node_(payload, node_operator_id, now_system_time())
.unwrap_or_else(|e| {
panic!("do_add_node_ failed for {node_reward_type} on iteration {i}: {e}")
Comment thread
NikolaMilosa marked this conversation as resolved.
});
}
}
}

#[test]
#[should_panic(
expected = "[Registry] do_add_node: Node Operator has reached max_rewardable_nodes quota for type4.5"
)]
fn should_panic_if_max_rewardable_nodes_is_exhausted_for_type4dot5() {
// type4.5 is explicitly excluded from the type4.1-type4.4 exemption
// because rewards of type4.5 are currently treated as type1.1. The
// standard max_rewardable_nodes quota check therefore still applies.
let mut registry = invariant_compliant_registry(0);

let (mutate_request, node_ids_and_dkg_pks) = prepare_registry_with_nodes(1, 1);
registry.maybe_apply_mutation_internal(mutate_request.mutations);
let node_ids: Vec<NodeId> = node_ids_and_dkg_pks.keys().cloned().collect();
let node_operator_id = registry_add_node_operator_for_node(
&mut registry,
node_ids[0],
btreemap! { NodeRewardType::Type4dot5 => 0 },
);

let (payload, _valid_pks) = prepare_add_node_payload(2, NodeRewardType::Type4dot5);

registry
.do_add_node_(payload, node_operator_id, now_system_time())
.unwrap();
}

#[test]
fn test_node_reward_type_is_required() {
let mut registry = invariant_compliant_registry(0);
Expand Down
14 changes: 14 additions & 0 deletions rs/registry/canister/unreleased_changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ on the process that this file is part of, see

## Changed

* Temporarily bypass the per-operator `max_rewardable_nodes` quota check in
`add_node` for node reward types `type4.1` through `type4.4`. Instead of the
configured quota, these types are subjected to a single high sentinel cap
(`EXCESSIVE_NUMBER_OF_TYPE_4_NODES`, currently 1000 per node operator),
chosen to be well above any realistic per-operator deployment while still
preventing runaway registrations. `type4.5` is explicitly excluded and
remains subject to the standard `max_rewardable_nodes` quota.

Motivation: node providers are starting to deploy gen4 hardware now, but the
reward canister currently still treats `type4.5` rewards as `type1.1`, which
means we cannot yet meaningfully size `max_rewardable_nodes` quotas for the
`type4.x` family. Enforcing the quota in the meantime would block legitimate
gen4 onboarding. The quota check will be restored once the reward-side
handling of `type4.5` is fixed (see CLO-15).
* One-time post-upgrade migration converting the reward type of 100 currently
unassigned nodes from `type1.1` to `type4.5`. The migration only mutates nodes
whose reward type is still `type1.1`, so it is idempotent across upgrades.
Expand Down
Loading