From a7aaf3e7a4967a2eafa7952ddd646b2fe99b276d Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Mon, 10 Mar 2025 15:00:54 -0400 Subject: [PATCH 1/4] add impl and test --- pallets/admin-utils/src/lib.rs | 34 +++++++++- pallets/admin-utils/src/tests/mod.rs | 65 +++++++++++++++++++- pallets/subtensor/src/lib.rs | 10 +++ pallets/subtensor/src/utils/rate_limiting.rs | 11 +++- 4 files changed, 115 insertions(+), 5 deletions(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 645eaa92c4..85d4188b3b 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -25,9 +25,13 @@ pub mod pallet { use super::*; use frame_support::pallet_prelude::*; use frame_support::traits::tokens::Balance; - use frame_support::{dispatch::DispatchResult, pallet_prelude::StorageMap}; + use frame_support::{ + dispatch::{DispatchResult, RawOrigin}, + pallet_prelude::StorageMap, + }; use frame_system::pallet_prelude::*; use pallet_evm_chain_id::{self, ChainId}; + use pallet_subtensor::utils::rate_limiting::TransactionType; use sp_runtime::BoundedVec; use substrate_fixed::types::I96F32; @@ -249,12 +253,38 @@ pub mod pallet { netuid: u16, weights_version_key: u64, ) -> DispatchResult { - pallet_subtensor::Pallet::::ensure_subnet_owner_or_root(origin, netuid)?; + pallet_subtensor::Pallet::::ensure_subnet_owner_or_root(origin.clone(), netuid)?; ensure!( pallet_subtensor::Pallet::::if_subnet_exist(netuid), Error::::SubnetDoesNotExist ); + + match origin.into() { + Ok(RawOrigin::Signed(who)) => { + // SN Owner + // Ensure the origin passes the rate limit. + ensure!( + pallet_subtensor::Pallet::::passes_rate_limit_on_subnet( + &TransactionType::SetWeightsVersionKey, + &who, + netuid, + ), + pallet_subtensor::Error::::TxRateLimitExceeded + ); + + // Set last transaction block + let current_block = pallet_subtensor::Pallet::::get_current_block_as_u64(); + pallet_subtensor::Pallet::::set_last_transaction_block_on_subnet( + &who, + netuid, + &TransactionType::SetWeightsVersionKey, + current_block, + ); + } + _ => (), + } + pallet_subtensor::Pallet::::set_weights_version_key(netuid, weights_version_key); log::debug!( "WeightsVersionKeySet( netuid: {:?} weights_version_key: {:?} ) ", diff --git a/pallets/admin-utils/src/tests/mod.rs b/pallets/admin-utils/src/tests/mod.rs index ae3aa022cc..9abaa847aa 100644 --- a/pallets/admin-utils/src/tests/mod.rs +++ b/pallets/admin-utils/src/tests/mod.rs @@ -5,7 +5,7 @@ use frame_support::{ traits::Hooks, }; use frame_system::Config; -use pallet_subtensor::Error as SubtensorError; +use pallet_subtensor::{Error as SubtensorError, SubnetOwner, Tempo, WeightsVersionKeyRateLimit}; // use pallet_subtensor::{migrations, Event}; use pallet_subtensor::Event; use sp_consensus_grandpa::AuthorityId as GrandpaId; @@ -162,6 +162,69 @@ fn test_sudo_set_weights_version_key() { }); } +#[test] +fn test_sudo_set_weights_version_key_rate_limit() { + new_test_ext().execute_with(|| { + let netuid: u16 = 1; + let to_be_set: u64 = 10; + + let sn_owner = U256::from(1); + add_network(netuid, 10); + // Set the Subnet Owner + SubnetOwner::insert(netuid, sn_owner); + + let init_value: u64 = SubtensorModule::get_weights_version_key(netuid); + let rate_limit = WeightsVersionKeyRateLimit::::get(); + let tempo: u16 = Tempo::::get(netuid); + + let rate_limit_period = rate_limit * (tempo as u64); + + assert_ok!(AdminUtils::sudo_set_weights_version_key( + <::RuntimeOrigin>::signed(sn_owner), + netuid, + to_be_set + )); + assert_eq!(SubtensorModule::get_weights_version_key(netuid), to_be_set); + + // Try to set again with + // Assert rate limit not passed + assert!(SubtensorModule::passes_rate_limit_on_subnet( + &TransactionType::SetWeightsVersionKey, + &sn_owner, + netuid + )); + + // Try transaction + assert_noop!( + AdminUtils::sudo_set_weights_version_key( + <::RuntimeOrigin>::signed(sn_owner), + netuid, + to_be_set + 1 + ), + Error::::TxRateLimitExceeded + ); + + // Wait for rate limit to pass + run_to_block(rate_limit_period + 2); + assert!(SubtensorModule::passes_rate_limit_on_subnet( + &TransactionType::SetWeightsVersionKey, + &sn_owner, + netuid + )); + + // Try transaction + assert_ok!(AdminUtils::sudo_set_weights_version_key( + <::RuntimeOrigin>::signed(sn_owner), + netuid, + to_be_set + 1 + )); + assert_eq!( + SubtensorModule::get_weights_version_key(netuid), + to_be_set + 1 + ); + }); +} + #[test] fn test_sudo_set_weights_set_rate_limit() { new_test_ext().execute_with(|| { diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index b355e96243..f3fccbfcef 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -509,6 +509,12 @@ pub mod pallet { T::InitialNetworkRateLimit::get() } #[pallet::type_value] + /// Default value for weights version key rate limit. + /// In units of tempos. + pub fn DefaultWeightsVersionKeyRateLimit() -> u64 { + 5 // 5 tempos + } + #[pallet::type_value] /// Default value for pending emission. pub fn DefaultPendingEmission() -> u64 { 0 @@ -1077,6 +1083,10 @@ pub mod pallet { pub type NetworkRateLimit = StorageValue<_, u64, ValueQuery, DefaultNetworkRateLimit>; #[pallet::storage] // --- ITEM( nominator_min_required_stake ) pub type NominatorMinRequiredStake = StorageValue<_, u64, ValueQuery, DefaultZeroU64>; + #[pallet::storage] + /// ITEM( weights_version_key_rate_limit ) --- Rate limit in tempos. + pub type WeightsVersionKeyRateLimit = + StorageValue<_, u64, ValueQuery, DefaultWeightsVersionKeyRateLimit>; /// ============================ /// ==== Subnet Locks ===== diff --git a/pallets/subtensor/src/utils/rate_limiting.rs b/pallets/subtensor/src/utils/rate_limiting.rs index 8b30f9665b..c37a78d2e4 100644 --- a/pallets/subtensor/src/utils/rate_limiting.rs +++ b/pallets/subtensor/src/utils/rate_limiting.rs @@ -7,6 +7,7 @@ pub enum TransactionType { SetChildkeyTake, Unknown, RegisterNetwork, + SetWeightsVersionKey, } /// Implement conversion from TransactionType to u16 @@ -17,6 +18,7 @@ impl From for u16 { TransactionType::SetChildkeyTake => 1, TransactionType::Unknown => 2, TransactionType::RegisterNetwork => 3, + TransactionType::SetWeightsVersionKey => 4, } } } @@ -28,6 +30,7 @@ impl From for TransactionType { 0 => TransactionType::SetChildren, 1 => TransactionType::SetChildkeyTake, 3 => TransactionType::RegisterNetwork, + 4 => TransactionType::SetWeightsVersionKey, _ => TransactionType::Unknown, } } @@ -41,14 +44,18 @@ impl Pallet { match tx_type { TransactionType::SetChildren => 150, // 30 minutes TransactionType::SetChildkeyTake => TxChildkeyTakeRateLimit::::get(), - TransactionType::Unknown => 0, // Default to no limit for unknown types (no limit) TransactionType::RegisterNetwork => NetworkRateLimit::::get(), + + TransactionType::Unknown => 0, // Default to no limit for unknown types (no limit) + _ => 0, } } - pub fn get_rate_limit_on_subnet(tx_type: &TransactionType, _netuid: u16) -> u64 { + pub fn get_rate_limit_on_subnet(tx_type: &TransactionType, netuid: u16) -> u64 { #[allow(clippy::match_single_binding)] match tx_type { + TransactionType::SetWeightsVersionKey => (Tempo::::get(netuid) as u64) + .saturating_mul(WeightsVersionKeyRateLimit::::get()), _ => Self::get_rate_limit(tx_type), } } From 86d56d3df6a7af216fed23a69c64e341ab0f88bc Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Mon, 10 Mar 2025 15:11:06 -0400 Subject: [PATCH 2/4] chore: clippy --- pallets/admin-utils/src/lib.rs | 39 +++++++++++++--------------- pallets/admin-utils/src/tests/mod.rs | 9 +++---- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 85d4188b3b..70e4cdd753 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -260,29 +260,26 @@ pub mod pallet { Error::::SubnetDoesNotExist ); - match origin.into() { - Ok(RawOrigin::Signed(who)) => { - // SN Owner - // Ensure the origin passes the rate limit. - ensure!( - pallet_subtensor::Pallet::::passes_rate_limit_on_subnet( - &TransactionType::SetWeightsVersionKey, - &who, - netuid, - ), - pallet_subtensor::Error::::TxRateLimitExceeded - ); - - // Set last transaction block - let current_block = pallet_subtensor::Pallet::::get_current_block_as_u64(); - pallet_subtensor::Pallet::::set_last_transaction_block_on_subnet( + if let Ok(RawOrigin::Signed(who)) = origin.into() { + // SN Owner + // Ensure the origin passes the rate limit. + ensure!( + pallet_subtensor::Pallet::::passes_rate_limit_on_subnet( + &TransactionType::SetWeightsVersionKey, &who, netuid, - &TransactionType::SetWeightsVersionKey, - current_block, - ); - } - _ => (), + ), + pallet_subtensor::Error::::TxRateLimitExceeded + ); + + // Set last transaction block + let current_block = pallet_subtensor::Pallet::::get_current_block_as_u64(); + pallet_subtensor::Pallet::::set_last_transaction_block_on_subnet( + &who, + netuid, + &TransactionType::SetWeightsVersionKey, + current_block, + ); } pallet_subtensor::Pallet::::set_weights_version_key(netuid, weights_version_key); diff --git a/pallets/admin-utils/src/tests/mod.rs b/pallets/admin-utils/src/tests/mod.rs index 9abaa847aa..2153ec498e 100644 --- a/pallets/admin-utils/src/tests/mod.rs +++ b/pallets/admin-utils/src/tests/mod.rs @@ -171,9 +171,8 @@ fn test_sudo_set_weights_version_key_rate_limit() { let sn_owner = U256::from(1); add_network(netuid, 10); // Set the Subnet Owner - SubnetOwner::insert(netuid, sn_owner); + SubnetOwner::::insert(netuid, sn_owner); - let init_value: u64 = SubtensorModule::get_weights_version_key(netuid); let rate_limit = WeightsVersionKeyRateLimit::::get(); let tempo: u16 = Tempo::::get(netuid); @@ -189,7 +188,7 @@ fn test_sudo_set_weights_version_key_rate_limit() { // Try to set again with // Assert rate limit not passed assert!(SubtensorModule::passes_rate_limit_on_subnet( - &TransactionType::SetWeightsVersionKey, + &pallet_subtensor::utils::rate_limiting::TransactionType::SetWeightsVersionKey, &sn_owner, netuid )); @@ -201,13 +200,13 @@ fn test_sudo_set_weights_version_key_rate_limit() { netuid, to_be_set + 1 ), - Error::::TxRateLimitExceeded + pallet_subtensor::Error::::TxRateLimitExceeded ); // Wait for rate limit to pass run_to_block(rate_limit_period + 2); assert!(SubtensorModule::passes_rate_limit_on_subnet( - &TransactionType::SetWeightsVersionKey, + &pallet_subtensor::utils::rate_limiting::TransactionType::SetWeightsVersionKey, &sn_owner, netuid )); From d819d094af2b84635a7e57d2dc1d7168737b08ea Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Mon, 10 Mar 2025 15:11:12 -0400 Subject: [PATCH 3/4] oops: negation --- pallets/admin-utils/src/tests/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/admin-utils/src/tests/mod.rs b/pallets/admin-utils/src/tests/mod.rs index 2153ec498e..aaf1d9d1b3 100644 --- a/pallets/admin-utils/src/tests/mod.rs +++ b/pallets/admin-utils/src/tests/mod.rs @@ -187,7 +187,7 @@ fn test_sudo_set_weights_version_key_rate_limit() { // Try to set again with // Assert rate limit not passed - assert!(SubtensorModule::passes_rate_limit_on_subnet( + assert!(!SubtensorModule::passes_rate_limit_on_subnet( &pallet_subtensor::utils::rate_limiting::TransactionType::SetWeightsVersionKey, &sn_owner, netuid From 55aef29f4b61885b0843f1ee4649721907073a0c Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Mon, 10 Mar 2025 15:13:31 -0400 Subject: [PATCH 4/4] add test for root no rate limit --- pallets/admin-utils/src/tests/mod.rs | 39 ++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/pallets/admin-utils/src/tests/mod.rs b/pallets/admin-utils/src/tests/mod.rs index aaf1d9d1b3..259961e656 100644 --- a/pallets/admin-utils/src/tests/mod.rs +++ b/pallets/admin-utils/src/tests/mod.rs @@ -224,6 +224,45 @@ fn test_sudo_set_weights_version_key_rate_limit() { }); } +#[test] +fn test_sudo_set_weights_version_key_rate_limit_root() { + // root should not be effected by rate limit + new_test_ext().execute_with(|| { + let netuid: u16 = 1; + let to_be_set: u64 = 10; + + let sn_owner = U256::from(1); + add_network(netuid, 10); + // Set the Subnet Owner + SubnetOwner::::insert(netuid, sn_owner); + + let rate_limit = WeightsVersionKeyRateLimit::::get(); + let tempo: u16 = Tempo::::get(netuid); + + let rate_limit_period = rate_limit * (tempo as u64); + // Verify the rate limit is more than 0 blocks + assert!(rate_limit_period > 0); + + assert_ok!(AdminUtils::sudo_set_weights_version_key( + <::RuntimeOrigin>::root(), + netuid, + to_be_set + )); + assert_eq!(SubtensorModule::get_weights_version_key(netuid), to_be_set); + + // Try transaction + assert_ok!(AdminUtils::sudo_set_weights_version_key( + <::RuntimeOrigin>::signed(sn_owner), + netuid, + to_be_set + 1 + )); + assert_eq!( + SubtensorModule::get_weights_version_key(netuid), + to_be_set + 1 + ); + }); +} + #[test] fn test_sudo_set_weights_set_rate_limit() { new_test_ext().execute_with(|| {