From 0e27f97d55d8b90e9ae3c271619527f965a2860f Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 24 Aug 2023 08:28:12 -0400 Subject: [PATCH 1/5] remove todo --- pallets/subtensor/src/migration.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pallets/subtensor/src/migration.rs b/pallets/subtensor/src/migration.rs index 9eb9d1958..700352426 100644 --- a/pallets/subtensor/src/migration.rs +++ b/pallets/subtensor/src/migration.rs @@ -9,8 +9,6 @@ use frame_support::{ inherent::Vec }; -// TODO (camfairchild): TEST MIGRATION - const LOG_TARGET: &str = "loadedemissionmigration"; pub mod deprecated_loaded_emission_format { @@ -31,7 +29,7 @@ pub fn migrate_to_v2_separate_emission() -> Weight { let onchain_version = Pallet::::on_chain_storage_version(); // Only runs if we haven't already updated version to 2. - if onchain_version < 2 { + if onchain_version < 2 { info!(target: LOG_TARGET, ">>> Updating the LoadedEmission to a new format {:?}", onchain_version); // We transform the storage values from the old into the new format. @@ -51,13 +49,13 @@ pub fn migrate_to_v2_separate_emission() -> Weight { // Translate the old storage values into the new format. LoadedEmission::::translate::, u64)>, _>( |netuid: u16, netuid_emissions: Vec<(AccountIdOf, u64)>| -> Option, u64, u64)>> { - info!(target: LOG_TARGET, " Do migration of netuid: {:?}...", netuid); - - // We will assume all loaded emission is validator emissions, + info!(target: LOG_TARGET, " Do migration of netuid: {:?}...", netuid); + + // We will assume all loaded emission is validator emissions, // so this will get distributed over delegatees (nominators), if there are any - // This will NOT effect any servers that are not (also) a delegate validator. + // This will NOT effect any servers that are not (also) a delegate validator. // server_emission will be 0 for any alread loaded emission. - + let mut new_netuid_emissions = Vec::new(); for (server, validator_emission) in netuid_emissions { new_netuid_emissions.push((server, 0 as u64, validator_emission)); @@ -74,10 +72,10 @@ pub fn migrate_to_v2_separate_emission() -> Weight { StorageVersion::new(1).put::>(); // Update to version 2 so we don't run this again. // One write to storage version weight.saturating_accrue(T::DbWeight::get().writes(1)); - + weight } else { info!(target: LOG_TARGET, "Migration to v2 already done!"); Weight::zero() } -} \ No newline at end of file +} From 5688a28ca9cbd5b6dc7519217454c1daf815d35a Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 24 Aug 2023 08:30:35 -0400 Subject: [PATCH 2/5] fix version check --- pallets/subtensor/src/lib.rs | 2 +- pallets/subtensor/src/migration.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index d61478160..ec1d529c9 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -986,7 +986,7 @@ pub mod pallet { // --- Migrate to v2 use crate::migration; - migration::migrate_to_v2_separate_emission::() + migration::migrate_to_v1_separate_emission::() } } diff --git a/pallets/subtensor/src/migration.rs b/pallets/subtensor/src/migration.rs index 700352426..e8b9e3eb9 100644 --- a/pallets/subtensor/src/migration.rs +++ b/pallets/subtensor/src/migration.rs @@ -20,7 +20,7 @@ pub mod deprecated_loaded_emission_format { pub(super) type LoadedEmission = StorageMap< Pallet, Identity, u16, Vec<(AccountIdOf, u64)>, OptionQuery >; } -pub fn migrate_to_v2_separate_emission() -> Weight { +pub fn migrate_to_v1_separate_emission() -> Weight { use deprecated_loaded_emission_format as old; // Check storage version let mut weight = T::DbWeight::get().reads_writes(1, 0); @@ -28,8 +28,8 @@ pub fn migrate_to_v2_separate_emission() -> Weight { // Grab current version let onchain_version = Pallet::::on_chain_storage_version(); - // Only runs if we haven't already updated version to 2. - if onchain_version < 2 { + // Only runs if we haven't already updated version to 1. + if onchain_version < 1 { info!(target: LOG_TARGET, ">>> Updating the LoadedEmission to a new format {:?}", onchain_version); // We transform the storage values from the old into the new format. @@ -69,13 +69,13 @@ pub fn migrate_to_v2_separate_emission() -> Weight { ); // Update storage version. - StorageVersion::new(1).put::>(); // Update to version 2 so we don't run this again. + StorageVersion::new(1).put::>(); // Update to version 1 so we don't run this again. // One write to storage version weight.saturating_accrue(T::DbWeight::get().writes(1)); weight } else { - info!(target: LOG_TARGET, "Migration to v2 already done!"); + info!(target: LOG_TARGET, "Migration to v1 already done!"); Weight::zero() } } From ab31a1e7835c03f9c56c9b5588a9776ebea9d936 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 24 Aug 2023 09:11:40 -0400 Subject: [PATCH 3/5] add migration for total stake fixes --- pallets/subtensor/src/lib.rs | 10 ++++- pallets/subtensor/src/migration.rs | 68 ++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index ec1d529c9..dfcfb8df2 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -983,10 +983,16 @@ pub mod pallet { } fn on_runtime_upgrade() -> frame_support::weights::Weight { - // --- Migrate to v2 + // --- Migrate storage use crate::migration; - migration::migrate_to_v1_separate_emission::() + let mut weight = frame_support::weights::Weight::from_ref_time(0); + + weight = weight + .saturating_add( migration::migrate_to_v1_separate_emission::() ) + .saturating_add( migration::migrate_to_v2_fixed_total_stake::() ); + + return weight; } } diff --git a/pallets/subtensor/src/migration.rs b/pallets/subtensor/src/migration.rs index e8b9e3eb9..0c3c1351f 100644 --- a/pallets/subtensor/src/migration.rs +++ b/pallets/subtensor/src/migration.rs @@ -79,3 +79,71 @@ pub fn migrate_to_v1_separate_emission() -> Weight { Weight::zero() } } + + +const LOG_TARGET_1: &str = "fixtotalstakestorage"; + +pub fn migrate_to_v2_fixed_total_stake() -> Weight { + let new_storage_version = 2; + + // Check storage version + let mut weight = T::DbWeight::get().reads(1); + + // Grab current version + let onchain_version = Pallet::::on_chain_storage_version(); + + // Only runs if we haven't already updated version past above new_storage_version. + if onchain_version < new_storage_version { + info!(target: LOG_TARGET_1, ">>> Fixing the TotalStake and TotalColdkeyStake storage {:?}", onchain_version); + + // Stake and TotalHotkeyStake are known to be accurate + // TotalColdkeyStake is known to be inaccurate + // TotalStake is known to be inaccurate + + TotalStake::::put(0); // Set to 0 + weight.saturating_accrue(T::DbWeight::get().writes(1)); + + // We iterate over TotalColdkeyStake keys and set them to 0 + let total_coldkey_stake_keys = TotalColdkeyStake::::iter_keys().collect::>(); + for coldkey in total_coldkey_stake_keys { + weight.saturating_accrue(T::DbWeight::get().reads(1)); + TotalColdkeyStake::::insert(coldkey, 0); // Set to 0 + weight.saturating_accrue(T::DbWeight::get().writes(1)); + } + + // Now we iterate over the entire stake map, and sum each coldkey stake + // We also track TotalStake + for (_, coldkey, stake) in Stake::::iter() { + weight.saturating_accrue(T::DbWeight::get().reads(1)); + // Get the current coldkey stake + let mut total_coldkey_stake = TotalColdkeyStake::::get(coldkey.clone()); + weight.saturating_accrue(T::DbWeight::get().reads(1)); + // Add the stake to the coldkey stake + total_coldkey_stake = total_coldkey_stake.saturating_add(stake); + // Update the coldkey stake + TotalColdkeyStake::::insert(coldkey, total_coldkey_stake); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + + // Get the current total stake + let mut total_stake = TotalStake::::get(); + weight.saturating_accrue(T::DbWeight::get().reads(1)); + // Add the stake to the total stake + total_stake = total_stake.saturating_add(stake); + // Update the total stake + TotalStake::::put(total_stake); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + } + + // Now both TotalStake and TotalColdkeyStake are accurate + + // Update storage version. + StorageVersion::new(new_storage_version).put::>(); // Update to version so we don't run this again. + // One write to storage version + weight.saturating_accrue(T::DbWeight::get().writes(1)); + + weight + } else { + info!(target: LOG_TARGET_1, "Migration to v2 already done!"); + Weight::zero() + } +} From cd8d94782cc4765a2725890d80c76bb0b12b587a Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 24 Aug 2023 09:29:05 -0400 Subject: [PATCH 4/5] make migration module public for testing --- pallets/subtensor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index dfcfb8df2..82f3adade 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -51,7 +51,7 @@ pub mod subnet_info; // apparently this is stabilized since rust 1.36 extern crate alloc; -mod migration; +pub mod migration; #[frame_support::pallet] pub mod pallet { From 4dc423aa85631f6065b9ba7ea9dda21adf62f446 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 24 Aug 2023 09:30:52 -0400 Subject: [PATCH 5/5] add test for migration --- pallets/subtensor/tests/migration.rs | 87 ++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 pallets/subtensor/tests/migration.rs diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs new file mode 100644 index 000000000..d0054eac7 --- /dev/null +++ b/pallets/subtensor/tests/migration.rs @@ -0,0 +1,87 @@ +mod mock; +use mock::*; +use sp_core::U256; + +#[test] +fn test_migration_fix_total_stake_maps() { + new_test_ext().execute_with(|| { + let ck1 = U256::from(1); + let ck2 = U256::from(2); + let ck3 = U256::from(3); + + let hk1 = U256::from(1 + 100); + let hk2 = U256::from(2 + 100); + + let mut total_stake_amount = 0; + + // Give each coldkey some stake in the maps + SubtensorModule::increase_stake_on_coldkey_hotkey_account( + &ck1, + &hk1, + 100 + ); + total_stake_amount += 100; + + SubtensorModule::increase_stake_on_coldkey_hotkey_account( + &ck2, + &hk1, + 10_101 + ); + total_stake_amount += 10_101; + + SubtensorModule::increase_stake_on_coldkey_hotkey_account( + &ck3, + &hk2, + 100_000_000 + ); + total_stake_amount += 100_000_000; + + SubtensorModule::increase_stake_on_coldkey_hotkey_account( + &ck1, + &hk2, + 1_123_000_000 + ); + total_stake_amount += 1_123_000_000; + + // Check that the total stake is correct + assert_eq!(SubtensorModule::get_total_stake(), total_stake_amount); + + // Check that the total coldkey stake is correct + assert_eq!(SubtensorModule::get_total_stake_for_coldkey(&ck1), 100 + 1_123_000_000); + assert_eq!(SubtensorModule::get_total_stake_for_coldkey(&ck2), 10_101); + assert_eq!(SubtensorModule::get_total_stake_for_coldkey(&ck3), 100_000_000); + + // Check that the total hotkey stake is correct + assert_eq!(SubtensorModule::get_total_stake_for_hotkey(&hk1), 100 + 10_101); + assert_eq!(SubtensorModule::get_total_stake_for_hotkey(&hk2), 100_000_000 + 1_123_000_000); + + // Mess up the total coldkey stake + pallet_subtensor::TotalColdkeyStake::::insert(ck1, 0); + // Verify that the total coldkey stake is now 0 for ck1 + assert_eq!(SubtensorModule::get_total_stake_for_coldkey(&ck1), 0); + + // Mess up the total stake + pallet_subtensor::TotalStake::::put(123_456_789); + // Verify that the total stake is now wrong + assert_ne!(SubtensorModule::get_total_stake(), total_stake_amount); + + // Run the migration to fix the total stake maps + pallet_subtensor::migration::migrate_to_v2_fixed_total_stake::(); + + // Verify that the total stake is now correct + assert_eq!(SubtensorModule::get_total_stake(), total_stake_amount); + // Verify that the total coldkey stake is now correct for each coldkey + assert_eq!(SubtensorModule::get_total_stake_for_coldkey(&ck1), 100 + 1_123_000_000); + assert_eq!(SubtensorModule::get_total_stake_for_coldkey(&ck2), 10_101); + assert_eq!(SubtensorModule::get_total_stake_for_coldkey(&ck3), 100_000_000); + + // Verify that the total hotkey stake is STILL correct for each hotkey + assert_eq!(SubtensorModule::get_total_stake_for_hotkey(&hk1), 100 + 10_101); + assert_eq!(SubtensorModule::get_total_stake_for_hotkey(&hk2), 100_000_000 + 1_123_000_000); + + // Verify that the Stake map has no extra entries + assert_eq!(pallet_subtensor::Stake::::iter().count(), 4); // 4 entries total + assert_eq!(pallet_subtensor::Stake::::iter_key_prefix(hk1).count(), 2); // 2 stake entries for hk1 + assert_eq!(pallet_subtensor::Stake::::iter_key_prefix(hk2).count(), 2); // 2 stake entries for hk2 + }) +}