refactor set max allowed uids

Author: l0r1s

pallets/admin-utils/src/lib.rs

@@ -25,7 +25,10 @@ pub mod pallet {
     use frame_support::{dispatch::DispatchResult, pallet_prelude::StorageMap};
     use frame_system::pallet_prelude::*;
     use pallet_evm_chain_id::{self, ChainId};
-    use pallet_subtensor::utils::rate_limiting::{Hyperparameter, TransactionType};
+    use pallet_subtensor::{
+        DefaultMaxAllowedUids,
+        utils::rate_limiting::{Hyperparameter, TransactionType},
+    };
     use sp_runtime::BoundedVec;
     use substrate_fixed::types::I96F32;
     use subtensor_runtime_common::{MechId, NetUid, TaoCurrency};
@@ -110,6 +113,10 @@ pub mod pallet {
         MinAllowedUidsGreaterThanCurrentUids,
         /// The minimum allowed UIDs must be less than the maximum allowed UIDs.
         MinAllowedUidsGreaterThanMaxAllowedUids,
+        /// The maximum allowed UIDs must be greater than the minimum allowed UIDs.
+        MaxAllowedUidsLessThanMinAllowedUids,
+        /// The maximum allowed UIDs must be less than the default maximum allowed UIDs.
+        MaxAllowedUidsGreaterThanDefaultMaxAllowedUids,
     }
     /// Enum for specifying the type of precompile operation.
     #[derive(
@@ -517,7 +524,7 @@ pub mod pallet {
         }
 
         /// The extrinsic sets the maximum allowed UIDs for a subnet.
-        /// It is only callable by the root account.
+        /// It is only callable by the root account and subnet owner.
         /// The extrinsic will call the Subtensor pallet to set the maximum allowed UIDs for a subnet.
         #[pallet::call_index(15)]
         #[pallet::weight(Weight::from_parts(18_800_000, 0)
@@ -528,16 +535,33 @@ pub mod pallet {
             netuid: NetUid,
             max_allowed_uids: u16,
         ) -> DispatchResult {
-            ensure_root(origin)?;
+            let maybe_owner = pallet_subtensor::Pallet::<T>::ensure_sn_owner_or_root_with_limits(
+                origin,
+                netuid,
+                &[Hyperparameter::MaxAllowedUids.into()],
+            )?;
             ensure!(
                 pallet_subtensor::Pallet::<T>::if_subnet_exist(netuid),
                 Error::<T>::SubnetDoesNotExist
             );
+            ensure!(
+                max_allowed_uids > pallet_subtensor::Pallet::<T>::get_min_allowed_uids(netuid),
+                Error::<T>::MaxAllowedUidsLessThanMinAllowedUids
+            );
             ensure!(
                 pallet_subtensor::Pallet::<T>::get_subnetwork_n(netuid) < max_allowed_uids,
                 Error::<T>::MaxAllowedUIdsLessThanCurrentUIds
             );
+            ensure!(
+                max_allowed_uids <= DefaultMaxAllowedUids::<T>::get(),
+                Error::<T>::MaxAllowedUidsGreaterThanDefaultMaxAllowedUids
+            );
             pallet_subtensor::Pallet::<T>::set_max_allowed_uids(netuid, max_allowed_uids);
+            pallet_subtensor::Pallet::<T>::record_owner_rl(
+                maybe_owner,
+                netuid,
+                &[Hyperparameter::MaxAllowedUids.into()],
+            );
             log::debug!(
                 "MaxAllowedUidsSet( netuid: {netuid:?} max_allowed_uids: {max_allowed_uids:?} ) "
             );

pallets/admin-utils/src/tests/mock.rs

@@ -92,7 +92,7 @@ parameter_types! {
     pub const SelfOwnership: u64 = 2;
     pub const InitialImmunityPeriod: u16 = 2;
     pub const InitialMinAllowedUids: u16 = 2;
-    pub const InitialMaxAllowedUids: u16 = 4;
+    pub const InitialMaxAllowedUids: u16 = 16;
     pub const InitialBondsMovingAverage: u64 = 900_000;
     pub const InitialBondsPenalty: u16 = u16::MAX;
     pub const InitialBondsResetOn: bool = false;

pallets/admin-utils/src/tests/mod.rs

@@ -513,69 +513,73 @@ fn test_sudo_set_min_allowed_weights() {
 fn test_sudo_set_max_allowed_uids() {
     new_test_ext().execute_with(|| {
         let netuid = NetUid::from(1);
-        let to_be_set: u16 = 10;
+        let to_be_set: u16 = 12;
         add_network(netuid, 10);
-        let init_value: u16 = SubtensorModule::get_max_allowed_uids(netuid);
-        assert_eq!(
+        MaxRegistrationsPerBlock::<Test>::insert(netuid, 256);
+        TargetRegistrationsPerInterval::<Test>::insert(netuid, 256);
+
+        // Register some neurons
+        for i in 0..=8 {
+            register_ok_neuron(netuid, U256::from(i * 1000), U256::from(i * 1000 + i), 0);
+        }
+
+        // Bad origin that is not root or subnet owner
+        assert_noop!(
             AdminUtils::sudo_set_max_allowed_uids(
-                <<Test as Config>::RuntimeOrigin>::signed(U256::from(0)),
+                <<Test as Config>::RuntimeOrigin>::signed(U256::from(42)),
                 netuid,
                 to_be_set
             ),
-            Err(DispatchError::BadOrigin)
+            DispatchError::BadOrigin
         );
-        assert_eq!(
+
+        // Random netuid that doesn't exist
+        assert_noop!(
             AdminUtils::sudo_set_max_allowed_uids(
                 <<Test as Config>::RuntimeOrigin>::root(),
-                netuid.next(),
+                NetUid::from(42),
                 to_be_set
             ),
-            Err(Error::<Test>::SubnetDoesNotExist.into())
+            Error::<Test>::SubnetDoesNotExist
         );
-        assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), init_value);
-        assert_ok!(AdminUtils::sudo_set_max_allowed_uids(
-            <<Test as Config>::RuntimeOrigin>::root(),
-            netuid,
-            to_be_set
-        ));
-        assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), to_be_set);
-    });
-}
 
-#[test]
-fn test_sudo_set_and_decrease_max_allowed_uids() {
-    new_test_ext().execute_with(|| {
-        let netuid = NetUid::from(1);
-        let to_be_set: u16 = 10;
-        add_network(netuid, 10);
-        let init_value: u16 = SubtensorModule::get_max_allowed_uids(netuid);
-        assert_eq!(
+        // Trying to set max allowed uids less than min allowed uids
+        assert_noop!(
             AdminUtils::sudo_set_max_allowed_uids(
-                <<Test as Config>::RuntimeOrigin>::signed(U256::from(0)),
+                <<Test as Config>::RuntimeOrigin>::root(),
                 netuid,
-                to_be_set
+                SubtensorModule::get_min_allowed_uids(netuid) - 1
             ),
-            Err(DispatchError::BadOrigin)
+            Error::<Test>::MaxAllowedUidsLessThanMinAllowedUids
         );
-        assert_eq!(
+
+        // Trying to set max allowed uids less than current uids
+        assert_noop!(
             AdminUtils::sudo_set_max_allowed_uids(
                 <<Test as Config>::RuntimeOrigin>::root(),
-                netuid.next(),
-                to_be_set
+                netuid,
+                SubtensorModule::get_subnetwork_n(netuid) - 1
             ),
-            Err(Error::<Test>::SubnetDoesNotExist.into())
+            Error::<Test>::MaxAllowedUIdsLessThanCurrentUIds
+        );
+
+        // Trying to set max allowed uids greater than default max allowed uids
+        assert_noop!(
+            AdminUtils::sudo_set_max_allowed_uids(
+                <<Test as Config>::RuntimeOrigin>::root(),
+                netuid,
+                DefaultMaxAllowedUids::<Test>::get() + 1
+            ),
+            Error::<Test>::MaxAllowedUidsGreaterThanDefaultMaxAllowedUids
         );
-        assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), init_value);
+
+        // Normal case
         assert_ok!(AdminUtils::sudo_set_max_allowed_uids(
             <<Test as Config>::RuntimeOrigin>::root(),
             netuid,
             to_be_set
         ));
-        assert_ok!(AdminUtils::sudo_set_max_allowed_uids(
-            <<Test as Config>::RuntimeOrigin>::root(),
-            netuid,
-            to_be_set - 1
-        ));
+        assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), to_be_set);
     });
 }
 

pallets/subtensor/src/utils/rate_limiting.rs

@@ -197,6 +197,7 @@ pub enum Hyperparameter {
     BondsResetEnabled = 22,
     ImmuneNeuronLimit = 23,
     RecycleOrBurn = 24,
+    MaxAllowedUids = 25,
 }
 
 impl<T: Config> Pallet<T> {