diff --git a/pallets/commitments/src/benchmarking.rs b/pallets/commitments/src/benchmarking.rs index fc1f5f92a4..e66f2a07e8 100644 --- a/pallets/commitments/src/benchmarking.rs +++ b/pallets/commitments/src/benchmarking.rs @@ -55,16 +55,6 @@ mod benchmarks { ); } - #[benchmark] - fn set_rate_limit() { - let new_limit: u32 = 42; - - #[extrinsic_call] - _(RawOrigin::Root, new_limit); - - assert_eq!(RateLimit::::get(), new_limit.into()); - } - #[benchmark] fn set_max_space() { let new_space: u32 = 1_000; diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index 519d361e66..34bf27566a 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -68,10 +68,6 @@ pub mod pallet { #[pallet::constant] type FieldDeposit: Get>; - /// The rate limit for commitments - #[pallet::constant] - type DefaultRateLimit: Get>; - /// Used to retreive the given subnet's tempo type TempoInterface: GetTempoInterface; } @@ -116,24 +112,12 @@ pub mod pallet { TooManyFieldsInCommitmentInfo, /// Account is not allow to make commitments to the chain AccountNotAllowedCommit, - /// Account is trying to commit data too fast, rate limit exceeded - CommitmentSetRateLimitExceeded, /// Space Limit Exceeded for the current interval SpaceLimitExceeded, /// Indicates that unreserve returned a leftover, which is unexpected. UnexpectedUnreserveLeftover, } - #[pallet::type_value] - /// *DEPRECATED* Default value for commitment rate limit. - pub fn DefaultRateLimit() -> BlockNumberFor { - T::DefaultRateLimit::get() - } - - /// *DEPRECATED* The rate limit for commitments - #[pallet::storage] - pub type RateLimit = StorageValue<_, BlockNumberFor, ValueQuery, DefaultRateLimit>; - /// Tracks all CommitmentOf that have at least one timelocked field. #[pallet::storage] #[pallet::getter(fn timelocked_index)] @@ -308,18 +292,18 @@ pub mod pallet { Ok(()) } - /// Sudo-set the commitment rate limit + /// *DEPRECATED* Sudo-set the commitment rate limit #[pallet::call_index(1)] #[pallet::weight(( Weight::from_parts(3_596_000, 0) - .saturating_add(T::DbWeight::get().reads(0_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)), - DispatchClass::Operational, - Pays::No - ))] - pub fn set_rate_limit(origin: OriginFor, rate_limit_blocks: u32) -> DispatchResult { + .saturating_add(T::DbWeight::get().reads(0_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)), + DispatchClass::Operational, + Pays::No + ))] + pub fn set_rate_limit(origin: OriginFor, _rate_limit_blocks: u32) -> DispatchResult { ensure_root(origin)?; - RateLimit::::set(rate_limit_blocks.into()); + // RateLimit::::set(rate_limit_blocks.into()); Ok(()) } diff --git a/pallets/commitments/src/mock.rs b/pallets/commitments/src/mock.rs index a5e9f1c4be..8f9dbb4e5f 100644 --- a/pallets/commitments/src/mock.rs +++ b/pallets/commitments/src/mock.rs @@ -100,7 +100,6 @@ impl pallet_commitments::Config for Test { type CanCommit = TestCanCommit; type FieldDeposit = ConstU64<0>; type InitialDeposit = ConstU64<0>; - type DefaultRateLimit = ConstU64<0>; type TempoInterface = MockTempoInterface; } diff --git a/pallets/commitments/src/tests.rs b/pallets/commitments/src/tests.rs index c9b14d188b..5d28e0733b 100644 --- a/pallets/commitments/src/tests.rs +++ b/pallets/commitments/src/tests.rs @@ -3,8 +3,8 @@ use sp_std::prelude::*; #[cfg(test)] use crate::{ - CommitmentInfo, CommitmentOf, Config, Data, Error, Event, MaxSpace, Pallet, RateLimit, - Registration, RevealedCommitments, TimelockedIndex, UsedSpaceOf, + CommitmentInfo, CommitmentOf, Config, Data, Error, Event, MaxSpace, Pallet, Registration, + RevealedCommitments, TimelockedIndex, UsedSpaceOf, mock::{ Balances, DRAND_QUICKNET_SIG_2000_HEX, DRAND_QUICKNET_SIG_HEX, RuntimeEvent, RuntimeOrigin, Test, TestMaxFields, insert_drand_pulse, new_test_ext, produce_ciphertext, @@ -150,39 +150,6 @@ fn set_commitment_too_many_fields_panics() { }); } -// DEPRECATED -// #[test] -// fn set_commitment_rate_limit_exceeded() { -// new_test_ext().execute_with(|| { -// let rate_limit = ::DefaultRateLimit::get(); -// System::::set_block_number(1); -// let info = Box::new(CommitmentInfo { -// fields: BoundedVec::try_from(vec![]).expect("Expected not to panic"), -// }); - -// assert_ok!(Pallet::::set_commitment( -// RuntimeOrigin::signed(1), -// 1, -// info.clone() -// )); - -// // Set block number to just before rate limit expires -// System::::set_block_number(rate_limit); -// assert_noop!( -// Pallet::::set_commitment(RuntimeOrigin::signed(1), 1, info.clone()), -// Error::::CommitmentSetRateLimitExceeded -// ); - -// // Set block number to after rate limit -// System::::set_block_number(rate_limit + 1); -// assert_ok!(Pallet::::set_commitment( -// RuntimeOrigin::signed(1), -// 1, -// info -// )); -// }); -// } - #[test] fn set_commitment_updates_deposit() { new_test_ext().execute_with(|| { @@ -226,22 +193,6 @@ fn set_commitment_updates_deposit() { }); } -#[test] -fn set_rate_limit_works() { - new_test_ext().execute_with(|| { - let default_rate_limit: u64 = ::DefaultRateLimit::get(); - assert_eq!(RateLimit::::get(), default_rate_limit); - - assert_ok!(Pallet::::set_rate_limit(RuntimeOrigin::root(), 200)); - assert_eq!(RateLimit::::get(), 200); - - assert_noop!( - Pallet::::set_rate_limit(RuntimeOrigin::signed(1), 300), - sp_runtime::DispatchError::BadOrigin - ); - }); -} - #[test] fn event_emission_works() { new_test_ext().execute_with(|| { diff --git a/pallets/subtensor/src/macros/hooks.rs b/pallets/subtensor/src/macros/hooks.rs index ce798456cf..78de392218 100644 --- a/pallets/subtensor/src/macros/hooks.rs +++ b/pallets/subtensor/src/macros/hooks.rs @@ -100,7 +100,9 @@ mod hooks { // Set subtoken enabled for all existed subnets .saturating_add(migrations::migrate_set_subtoken_enabled::migrate_set_subtoken_enabled::()) // Remove all entries in TotalHotkeyColdkeyStakesThisInterval - .saturating_add(migrations::migrate_remove_total_hotkey_coldkey_stakes_this_interval::migrate_remove_total_hotkey_coldkey_stakes_this_interval::()); + .saturating_add(migrations::migrate_remove_total_hotkey_coldkey_stakes_this_interval::migrate_remove_total_hotkey_coldkey_stakes_this_interval::()) + // Wipe the deprecated RateLimit storage item in the commitments pallet + .saturating_add(migrations::migrate_remove_commitments_rate_limit::migrate_remove_commitments_rate_limit::()); weight // Remove all entries in orphaned storage items diff --git a/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs b/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs new file mode 100644 index 0000000000..b32d4edc9f --- /dev/null +++ b/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs @@ -0,0 +1,54 @@ +use super::*; +use crate::HasMigrationRun; +use frame_support::{traits::Get, weights::Weight}; +use scale_info::prelude::string::String; +use sp_io::{KillStorageResult, hashing::twox_128, storage::clear_prefix}; + +pub fn migrate_remove_commitments_rate_limit() -> Weight { + let migration_name = b"migrate_remove_commitments_rate_limit".to_vec(); + let mut weight = T::DbWeight::get().reads(1); + if HasMigrationRun::::get(&migration_name) { + log::info!( + "Migration '{:?}' has already run. Skipping.", + migration_name + ); + return weight; + } + + log::info!( + "Running migration '{}'", + String::from_utf8_lossy(&migration_name) + ); + + // ------------------------------------------------------------- + // Step 1: Remove all entries under the `RateLimit` storage key + // ------------------------------------------------------------- + let mut rate_limit_prefix = Vec::new(); + rate_limit_prefix.extend_from_slice(&twox_128("Commitments".as_bytes())); + rate_limit_prefix.extend_from_slice(&twox_128("RateLimit".as_bytes())); + + let removal_result = clear_prefix(&rate_limit_prefix, Some(u32::MAX)); + let removed_entries = match removal_result { + KillStorageResult::AllRemoved(removed) => removed as u64, + KillStorageResult::SomeRemaining(removed) => { + log::warn!("Failed to remove some `RateLimit` entries."); + removed as u64 + } + }; + + weight = weight.saturating_add(T::DbWeight::get().writes(removed_entries)); + log::info!("Removed {} entries from `RateLimit`.", removed_entries); + + // ------------------------------------------------------------- + // Step 2: Mark this migration as completed + // ------------------------------------------------------------- + HasMigrationRun::::insert(&migration_name, true); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + + log::info!( + "Migration '{:?}' completed successfully.", + String::from_utf8_lossy(&migration_name) + ); + + weight +} diff --git a/pallets/subtensor/src/migrations/mod.rs b/pallets/subtensor/src/migrations/mod.rs index 064662db25..5c6347034f 100644 --- a/pallets/subtensor/src/migrations/mod.rs +++ b/pallets/subtensor/src/migrations/mod.rs @@ -15,6 +15,7 @@ pub mod migrate_init_total_issuance; pub mod migrate_orphaned_storage_items; pub mod migrate_populate_owned_hotkeys; pub mod migrate_rao; +pub mod migrate_remove_commitments_rate_limit; pub mod migrate_remove_stake_map; pub mod migrate_remove_total_hotkey_coldkey_stakes_this_interval; pub mod migrate_remove_unused_maps_and_values; diff --git a/pallets/subtensor/src/tests/migration.rs b/pallets/subtensor/src/tests/migration.rs index 100bbed24e..1dfac06ad5 100644 --- a/pallets/subtensor/src/tests/migration.rs +++ b/pallets/subtensor/src/tests/migration.rs @@ -767,3 +767,56 @@ fn test_remove_storage_item Weight>( assert!(!weight.is_zero(), "Migration weight should be non-zero."); }); } + +#[test] +fn test_migrate_remove_commitments_rate_limit() { + new_test_ext(1).execute_with(|| { + // ------------------------------ + // Step 1: Simulate Old Storage Entry + // ------------------------------ + const MIGRATION_NAME: &str = "migrate_remove_commitments_rate_limit"; + + // Build the raw storage key: twox128("Commitments") ++ twox128("RateLimit") + let pallet_prefix = twox_128("Commitments".as_bytes()); + let storage_prefix = twox_128("RateLimit".as_bytes()); + + let mut key = Vec::new(); + key.extend_from_slice(&pallet_prefix); + key.extend_from_slice(&storage_prefix); + + let original_value: u64 = 123; + put_raw(&key, &original_value.encode()); + + let stored_before = get_raw(&key).expect("Expected RateLimit to exist"); + assert_eq!( + u64::decode(&mut &stored_before[..]).expect("Failed to decode RateLimit"), + original_value + ); + + assert!( + !HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec()), + "Migration should not have run yet" + ); + + // ------------------------------ + // Step 2: Run the Migration + // ------------------------------ + let weight = crate::migrations::migrate_remove_commitments_rate_limit:: + migrate_remove_commitments_rate_limit::(); + + assert!( + HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec()), + "Migration should be marked as completed" + ); + + // ------------------------------ + // Step 3: Verify Migration Effects + // ------------------------------ + assert!( + get_raw(&key).is_none(), + "RateLimit storage should have been cleared" + ); + + assert!(!weight.is_zero(), "Migration weight should be non-zero"); + }); +} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 4d7ddf58ba..2c1a11d94b 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -955,7 +955,6 @@ parameter_types! { pub const MaxCommitFieldsInner: u32 = 1; pub const CommitmentInitialDeposit: Balance = 0; // Free pub const CommitmentFieldDeposit: Balance = 0; // Free - pub const CommitmentRateLimit: BlockNumber = 100; // Allow commitment every 100 blocks } #[subtensor_macros::freeze_struct("7c76bd954afbb54e")] @@ -991,7 +990,6 @@ impl pallet_commitments::Config for Runtime { type MaxFields = MaxCommitFields; type InitialDeposit = CommitmentInitialDeposit; type FieldDeposit = CommitmentFieldDeposit; - type DefaultRateLimit = CommitmentRateLimit; type TempoInterface = TempoInterface; }