diff --git a/rs/registry/canister/src/mutations/node_management/do_add_node.rs b/rs/registry/canister/src/mutations/node_management/do_add_node.rs index f5bc0c26a2df..0339186907c2 100644 --- a/rs/registry/canister/src/mutations/node_management/do_add_node.rs +++ b/rs/registry/canister/src/mutations/node_management/do_add_node.rs @@ -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. /// @@ -136,10 +149,26 @@ 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() @@ -147,10 +176,14 @@ impl Registry { .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 { 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},\ @@ -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 = 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}") + }); + } + } + } + + #[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 = 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); diff --git a/rs/registry/canister/unreleased_changelog.md b/rs/registry/canister/unreleased_changelog.md index 93c14851bbe2..051deff26616 100644 --- a/rs/registry/canister/unreleased_changelog.md +++ b/rs/registry/canister/unreleased_changelog.md @@ -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.