From 83ad744fa1b86c79856c48bcf636611410ae516d Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:06:59 -0700 Subject: [PATCH 01/10] remove deprecated commitments storage --- pallets/commitments/src/lib.rs | 38 ++++++++---------------- pallets/commitments/src/tests.rs | 51 +------------------------------- 2 files changed, 14 insertions(+), 75 deletions(-) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index 519d361e66..e5f66c6107 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -116,24 +116,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)] @@ -309,19 +297,19 @@ pub mod pallet { } /// 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 { - ensure_root(origin)?; - RateLimit::::set(rate_limit_blocks.into()); - Ok(()) - } + // #[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 { + // ensure_root(origin)?; + // RateLimit::::set(rate_limit_blocks.into()); + // Ok(()) + // } /// Sudo-set MaxSpace #[pallet::call_index(2)] diff --git a/pallets/commitments/src/tests.rs b/pallets/commitments/src/tests.rs index c9b14d188b..f3b7a26bbb 100644 --- a/pallets/commitments/src/tests.rs +++ b/pallets/commitments/src/tests.rs @@ -3,7 +3,7 @@ use sp_std::prelude::*; #[cfg(test)] use crate::{ - CommitmentInfo, CommitmentOf, Config, Data, Error, Event, MaxSpace, Pallet, RateLimit, + 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, @@ -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(|| { From 85fac141a09732c160dd6038f12ea3168bb58165 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:07:14 -0700 Subject: [PATCH 02/10] fmt --- pallets/commitments/src/lib.rs | 10 +++++----- pallets/commitments/src/tests.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index e5f66c6107..f31310670f 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -300,11 +300,11 @@ pub mod pallet { // #[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 - // ))] + // .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()); diff --git a/pallets/commitments/src/tests.rs b/pallets/commitments/src/tests.rs index f3b7a26bbb..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, - 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, From ef47e7a1413a41af62d9d31b5212a504b9e8db12 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:16:17 -0700 Subject: [PATCH 03/10] remove deprecated benchmark --- pallets/commitments/src/benchmarking.rs | 10 ---------- 1 file changed, 10 deletions(-) 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; From 5a6acbf61ca8576eccccf07a131a2eb5205bcbf1 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:21:19 -0700 Subject: [PATCH 04/10] add migrate_remove_commitments_rate_limit --- pallets/subtensor/src/macros/hooks.rs | 4 +- .../migrate_remove_commitments_rate_limit.rs | 51 +++++++++++++++++++ pallets/subtensor/src/migrations/mod.rs | 1 + 3 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs 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..6ed3e3605e --- /dev/null +++ b/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs @@ -0,0 +1,51 @@ +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..cf66b387fd 100644 --- a/pallets/subtensor/src/migrations/mod.rs +++ b/pallets/subtensor/src/migrations/mod.rs @@ -32,6 +32,7 @@ pub mod migrate_to_v2_fixed_total_stake; pub mod migrate_total_issuance; pub mod migrate_transfer_ownership_to_foundation; pub mod migrate_upgrade_revealed_commitments; +pub mod migrate_remove_commitments_rate_limit; pub(crate) fn migrate_storage( migration_name: &'static str, From 17715c13f72eb90dca65ce815f2aa63961f07127 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:30:54 -0700 Subject: [PATCH 05/10] add test_migrate_remove_commitments_rate_limit --- pallets/subtensor/src/tests/migration.rs | 53 ++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/pallets/subtensor/src/tests/migration.rs b/pallets/subtensor/src/tests/migration.rs index 100bbed24e..e0421b6cff 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"); + }); +} \ No newline at end of file From 6f57b8cb5d5170c88218bcb339274bcc17d725a4 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:31:04 -0700 Subject: [PATCH 06/10] fmt --- .../src/migrations/migrate_remove_commitments_rate_limit.rs | 5 ++++- pallets/subtensor/src/migrations/mod.rs | 2 +- pallets/subtensor/src/tests/migration.rs | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs b/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs index 6ed3e3605e..b32d4edc9f 100644 --- a/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs +++ b/pallets/subtensor/src/migrations/migrate_remove_commitments_rate_limit.rs @@ -8,7 +8,10 @@ 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); + log::info!( + "Migration '{:?}' has already run. Skipping.", + migration_name + ); return weight; } diff --git a/pallets/subtensor/src/migrations/mod.rs b/pallets/subtensor/src/migrations/mod.rs index cf66b387fd..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; @@ -32,7 +33,6 @@ pub mod migrate_to_v2_fixed_total_stake; pub mod migrate_total_issuance; pub mod migrate_transfer_ownership_to_foundation; pub mod migrate_upgrade_revealed_commitments; -pub mod migrate_remove_commitments_rate_limit; pub(crate) fn migrate_storage( migration_name: &'static str, diff --git a/pallets/subtensor/src/tests/migration.rs b/pallets/subtensor/src/tests/migration.rs index e0421b6cff..1dfac06ad5 100644 --- a/pallets/subtensor/src/tests/migration.rs +++ b/pallets/subtensor/src/tests/migration.rs @@ -819,4 +819,4 @@ fn test_migrate_remove_commitments_rate_limit() { assert!(!weight.is_zero(), "Migration weight should be non-zero"); }); -} \ No newline at end of file +} From 0ac6ea8abfa7857384d188c4e358de941656a869 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:41:57 -0700 Subject: [PATCH 07/10] remove rate limit config --- pallets/commitments/src/lib.rs | 4 ---- pallets/commitments/src/mock.rs | 1 - runtime/src/lib.rs | 2 -- 3 files changed, 7 deletions(-) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index f31310670f..d894d71b8f 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; } 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/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; } From 2db3c7ddb3031e10a86fe6c7d02ac703fde35fe3 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:44:59 -0700 Subject: [PATCH 08/10] clippy --- pallets/commitments/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index d894d71b8f..23f9ca5a1f 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -292,7 +292,7 @@ pub mod pallet { Ok(()) } - /// Sudo-set the commitment rate limit + // /// Sudo-set the commitment rate limit // #[pallet::call_index(1)] // #[pallet::weight(( // Weight::from_parts(3_596_000, 0) From 84d185a4c09df62d73f0edf4c859198b037f2011 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:48:49 -0700 Subject: [PATCH 09/10] don't fully remove --- pallets/commitments/src/lib.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index 23f9ca5a1f..34bf27566a 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -292,20 +292,20 @@ pub mod pallet { Ok(()) } - // /// 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 { - // ensure_root(origin)?; - // RateLimit::::set(rate_limit_blocks.into()); - // Ok(()) - // } + /// *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 { + ensure_root(origin)?; + // RateLimit::::set(rate_limit_blocks.into()); + Ok(()) + } /// Sudo-set MaxSpace #[pallet::call_index(2)] From 759052ef91b33949b4b9937d20194df12b9e491c Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Mon, 5 May 2025 10:52:08 -0700 Subject: [PATCH 10/10] ci: bump CI