diff --git a/evm-tests/src/subtensor.ts b/evm-tests/src/subtensor.ts index ab1508e4b4..63a5ef6e8e 100644 --- a/evm-tests/src/subtensor.ts +++ b/evm-tests/src/subtensor.ts @@ -233,24 +233,6 @@ export async function setActivityCutoff(api: TypedApi, netuid: nu assert.equal(activityCutoff, await api.query.SubtensorModule.ActivityCutoff.getValue(netuid)) } -export async function setMaxAllowedUids(api: TypedApi, netuid: number, maxAllowedUids: number) { - const value = await api.query.SubtensorModule.MaxAllowedUids.getValue(netuid) - if (value === maxAllowedUids) { - return; - } - - const alice = getAliceSigner() - - const internalCall = api.tx.AdminUtils.sudo_set_max_allowed_uids({ - netuid: netuid, - max_allowed_uids: maxAllowedUids - }) - const tx = api.tx.Sudo.sudo({ call: internalCall.decodedCall }) - - await waitForTransactionWithRetry(api, tx, alice) - assert.equal(maxAllowedUids, await api.query.SubtensorModule.MaxAllowedUids.getValue(netuid)) -} - export async function setMinDelegateTake(api: TypedApi, minDelegateTake: number) { const value = await api.query.SubtensorModule.MinDelegateTake.getValue() if (value === minDelegateTake) { diff --git a/evm-tests/test/staking.precompile.reward.test.ts b/evm-tests/test/staking.precompile.reward.test.ts index 251fb41ea5..55a1312313 100644 --- a/evm-tests/test/staking.precompile.reward.test.ts +++ b/evm-tests/test/staking.precompile.reward.test.ts @@ -6,7 +6,7 @@ import { convertPublicKeyToSs58 } from "../src/address-utils" import { tao } from "../src/balance-math" import { forceSetBalanceToSs58Address, addNewSubnetwork, burnedRegister, - setTxRateLimit, setTempo, setWeightsSetRateLimit, setSubnetOwnerCut, setMaxAllowedUids, + setTxRateLimit, setTempo, setWeightsSetRateLimit, setSubnetOwnerCut, setMinDelegateTake, setActivityCutoff, addStake, setWeight, rootRegister, startCall, disableAdminFreezeWindowAndOwnerHyperparamRateLimit @@ -52,7 +52,6 @@ describe("Test neuron precompile reward", () => { await burnedRegister(api, netuid, convertPublicKeyToSs58(nominator.publicKey), coldkey) await setSubnetOwnerCut(api, 0) await setActivityCutoff(api, netuid, 65535) - await setMaxAllowedUids(api, netuid, 65535) await setMinDelegateTake(api, 0) }) diff --git a/pallets/admin-utils/src/benchmarking.rs b/pallets/admin-utils/src/benchmarking.rs index 67fba62b10..d10f36f7fe 100644 --- a/pallets/admin-utils/src/benchmarking.rs +++ b/pallets/admin-utils/src/benchmarking.rs @@ -263,7 +263,7 @@ mod benchmarks { ); #[extrinsic_call] - _(RawOrigin::Root, 1u16.into()/*netuid*/, 4097u16/*max_allowed_uids*/)/*sudo_set_max_allowed_uids*/; + _(RawOrigin::Root, 1u16.into()/*netuid*/, 2048u16/*max_allowed_uids*/)/*sudo_set_max_allowed_uids*/; } #[benchmark] diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 61e4b8a49b..7d8a97ba98 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/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,27 +524,44 @@ 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) - .saturating_add(::DbWeight::get().reads(2_u64)) + #[pallet::weight(Weight::from_parts(32_140_000, 0) + .saturating_add(::DbWeight::get().reads(5_u64)) .saturating_add(::DbWeight::get().writes(1_u64)))] pub fn sudo_set_max_allowed_uids( origin: OriginFor, netuid: NetUid, max_allowed_uids: u16, ) -> DispatchResult { - ensure_root(origin)?; + let maybe_owner = pallet_subtensor::Pallet::::ensure_sn_owner_or_root_with_limits( + origin, + netuid, + &[Hyperparameter::MaxAllowedUids.into()], + )?; ensure!( pallet_subtensor::Pallet::::if_subnet_exist(netuid), Error::::SubnetDoesNotExist ); ensure!( - pallet_subtensor::Pallet::::get_subnetwork_n(netuid) < max_allowed_uids, + max_allowed_uids >= pallet_subtensor::Pallet::::get_min_allowed_uids(netuid), + Error::::MaxAllowedUidsLessThanMinAllowedUids + ); + ensure!( + pallet_subtensor::Pallet::::get_subnetwork_n(netuid) <= max_allowed_uids, Error::::MaxAllowedUIdsLessThanCurrentUIds ); + ensure!( + max_allowed_uids <= DefaultMaxAllowedUids::::get(), + Error::::MaxAllowedUidsGreaterThanDefaultMaxAllowedUids + ); pallet_subtensor::Pallet::::set_max_allowed_uids(netuid, max_allowed_uids); + pallet_subtensor::Pallet::::record_owner_rl( + maybe_owner, + netuid, + &[Hyperparameter::MaxAllowedUids.into()], + ); log::debug!( "MaxAllowedUidsSet( netuid: {netuid:?} max_allowed_uids: {max_allowed_uids:?} ) " ); @@ -813,7 +837,7 @@ pub mod pallet { /// It is only callable by the root account or subnet owner. /// The extrinsic will call the Subtensor pallet to set the difficulty. #[pallet::call_index(24)] - #[pallet::weight(Weight::from_parts(26_230_000, 0) + #[pallet::weight(Weight::from_parts(38_500_000, 0) .saturating_add(::DbWeight::get().reads(3_u64)) .saturating_add(::DbWeight::get().writes(1_u64)))] pub fn sudo_set_difficulty( diff --git a/pallets/admin-utils/src/tests/mock.rs b/pallets/admin-utils/src/tests/mock.rs index 8c708adc9e..60ba7cbe09 100644 --- a/pallets/admin-utils/src/tests/mock.rs +++ b/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; diff --git a/pallets/admin-utils/src/tests/mod.rs b/pallets/admin-utils/src/tests/mod.rs index b83604b69a..4e534c3210 100644 --- a/pallets/admin-utils/src/tests/mod.rs +++ b/pallets/admin-utils/src/tests/mod.rs @@ -513,69 +513,107 @@ 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::::insert(netuid, 256); + TargetRegistrationsPerInterval::::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( - <::RuntimeOrigin>::signed(U256::from(0)), + <::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( <::RuntimeOrigin>::root(), - netuid.next(), + NetUid::from(42), to_be_set ), - Err(Error::::SubnetDoesNotExist.into()) + Error::::SubnetDoesNotExist ); - assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), init_value); - assert_ok!(AdminUtils::sudo_set_max_allowed_uids( - <::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( - <::RuntimeOrigin>::signed(U256::from(0)), + <::RuntimeOrigin>::root(), netuid, - to_be_set + SubtensorModule::get_min_allowed_uids(netuid) - 1 ), - Err(DispatchError::BadOrigin) + Error::::MaxAllowedUidsLessThanMinAllowedUids ); - assert_eq!( + + // Trying to set max allowed uids less than current uids + assert_noop!( AdminUtils::sudo_set_max_allowed_uids( <::RuntimeOrigin>::root(), - netuid.next(), - to_be_set + netuid, + SubtensorModule::get_subnetwork_n(netuid) - 1 ), - Err(Error::::SubnetDoesNotExist.into()) + Error::::MaxAllowedUIdsLessThanCurrentUIds ); - assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), init_value); + + // Trying to set max allowed uids greater than default max allowed uids + assert_noop!( + AdminUtils::sudo_set_max_allowed_uids( + <::RuntimeOrigin>::root(), + netuid, + DefaultMaxAllowedUids::::get() + 1 + ), + Error::::MaxAllowedUidsGreaterThanDefaultMaxAllowedUids + ); + + // Normal case assert_ok!(AdminUtils::sudo_set_max_allowed_uids( <::RuntimeOrigin>::root(), netuid, to_be_set )); + assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), to_be_set); + + // Exact current case + assert_ok!(AdminUtils::sudo_set_max_allowed_uids( + <::RuntimeOrigin>::root(), + netuid, + SubtensorModule::get_subnetwork_n(netuid) + )); + assert_eq!( + SubtensorModule::get_max_allowed_uids(netuid), + SubtensorModule::get_subnetwork_n(netuid) + ); + + // Lower bound case + SubtensorModule::set_min_allowed_uids(netuid, SubtensorModule::get_subnetwork_n(netuid)); assert_ok!(AdminUtils::sudo_set_max_allowed_uids( <::RuntimeOrigin>::root(), netuid, - to_be_set - 1 + SubtensorModule::get_min_allowed_uids(netuid) )); + assert_eq!( + SubtensorModule::get_max_allowed_uids(netuid), + SubtensorModule::get_min_allowed_uids(netuid) + ); + + // Upper bound case + assert_ok!(AdminUtils::sudo_set_max_allowed_uids( + <::RuntimeOrigin>::root(), + netuid, + DefaultMaxAllowedUids::::get(), + )); + assert_eq!( + SubtensorModule::get_max_allowed_uids(netuid), + DefaultMaxAllowedUids::::get() + ); }); } diff --git a/pallets/subtensor/src/utils/rate_limiting.rs b/pallets/subtensor/src/utils/rate_limiting.rs index e7028bc4e3..85f58cfc64 100644 --- a/pallets/subtensor/src/utils/rate_limiting.rs +++ b/pallets/subtensor/src/utils/rate_limiting.rs @@ -197,6 +197,7 @@ pub enum Hyperparameter { BondsResetEnabled = 22, ImmuneNeuronLimit = 23, RecycleOrBurn = 24, + MaxAllowedUids = 25, } impl Pallet { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index e597728a60..aeeb65c7c4 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -223,7 +223,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // `spec_version`, and `authoring_version` are the same between Wasm and native. // This value is set to 100 to notify Polkadot-JS App (https://polkadot.js.org/apps) to use // the compatible custom types. - spec_version: 323, + spec_version: 324, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1,