From 1a5009a8370e559c2b91ed110df23e3772036f5e Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 12 Sep 2024 21:11:17 -0400 Subject: [PATCH 01/17] add migration patch; remove pending emission from old hotkey --- .../src/migrations/migrate_fix_pending_emission.rs | 1 + pallets/subtensor/tests/migration.rs | 14 ++++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs index 81da5538b..afdaac4af 100644 --- a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs +++ b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs @@ -26,6 +26,7 @@ fn migrate_pending_emissions_including_null_stake( // Get the pending emissions for the OLD hotkey let pending_emissions_old: u64 = PendingdHotkeyEmission::::get(old_hotkey); + PendingdHotkeyEmission::::remove(old_hotkey); weight.saturating_accrue(T::DbWeight::get().reads(1)); // Get the stake for the 0x000 key diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 0df00c449..6fb76a7e5 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -482,7 +482,8 @@ fn test_migrate_fix_pending_emissions() { PendingdHotkeyEmission::::insert(datura_old_hk_account, 10_000); // Setup the NEW Datura hotkey with a pending emission PendingdHotkeyEmission::::insert(datura_new_hk_account, 123_456_789); - let expected_datura_new_hk_pending_emission: u64 = 123_456_789 + 10_000; + Stake::::insert(datura_old_hk_account, null_account, 123_456_789); + let expected_datura_new_hk_pending_emission: u64 = 123_456_789 + 10_000 + 123_456_789; // Setup the old TaoStats hotkey with a pending emission PendingdHotkeyEmission::::insert(taostats_old_hk_account, 987_654); @@ -508,10 +509,15 @@ fn test_migrate_fix_pending_emissions() { expected_taostats_new_hk_pending_emission ); - // Check the pending emission is added to new the Datura hotkey + // Check the pending emission is removed from old ones assert_eq!( - PendingdHotkeyEmission::::get(datura_new_hk_account), - expected_datura_new_hk_pending_emission + PendingdHotkeyEmission::::get(datura_old_hk_account), + 0 + ); + + assert_eq!( + PendingdHotkeyEmission::::get(taostats_old_hk_account), + 0 ); }) } From d1c25d39f7d7f98c23b4e5d17cc3ffeaa8c2f416 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 12 Sep 2024 21:11:27 -0400 Subject: [PATCH 02/17] check stake after migration --- pallets/subtensor/tests/migration.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 6fb76a7e5..0f6913a50 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -519,5 +519,9 @@ fn test_migrate_fix_pending_emissions() { PendingdHotkeyEmission::::get(taostats_old_hk_account), 0 ); + + // Check the stake entry is removed + assert_eq!(Stake::::get(datura_old_hk_account, null_account), 0); + assert_eq!(Stake::::get(taostats_old_hk_account, null_account), 0); }) } From eea273ad2b9de3261071684db7a662c3c54b9aa7 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 12 Sep 2024 21:11:33 -0400 Subject: [PATCH 03/17] spec version --- runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 1aa30ac3c..f44f4eee2 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -142,7 +142,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: 202, + spec_version: 203, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From 16d636ab49e8938868708fc22afec600654354af Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 12 Sep 2024 22:06:38 -0400 Subject: [PATCH 04/17] use check helper in set weights call --- pallets/subtensor/src/subnets/weights.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 1a53e44cc..bcb70e183 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -226,9 +226,9 @@ impl Pallet { Error::::HotKeyNotRegisteredInSubNet ); - // --- 6. Check to see if the hotkey has enought stake to set weights. + // --- 6. Check to see if the hotkey has enough stake to set weights. ensure!( - Self::get_total_stake_for_hotkey(&hotkey) >= Self::get_weights_min_stake(), + Self::check_weights_min_stake(&hotkey, netuid), Error::::NotEnoughStakeToSetWeights ); From eea8d1afe661ba557fbc42b6df4298f05d366017 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 12 Sep 2024 22:07:02 -0400 Subject: [PATCH 05/17] add test for set weights checks on child keys --- pallets/subtensor/tests/children.rs | 108 ++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/pallets/subtensor/tests/children.rs b/pallets/subtensor/tests/children.rs index 2b99030ab..9fe524a5b 100644 --- a/pallets/subtensor/tests/children.rs +++ b/pallets/subtensor/tests/children.rs @@ -3237,3 +3237,111 @@ fn test_rank_trust_incentive_calculation_with_parent_child() { }); } + +// SKIP_WASM_BUILD=1 RUST_LOG=debug cargo test --package pallet-subtensor --test children -- test_childkey_set_weights_single_parent --exact --nocapture +#[test] +fn test_childkey_set_weights_single_parent() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + add_network(netuid, 1, 0); + + // Define hotkeys + let parent: U256 = U256::from(1); + let child: U256 = U256::from(2); + let weight_setter: U256 = U256::from(3); + + // Define coldkeys with more readable names + let coldkey_parent: U256 = U256::from(100); + let coldkey_child: U256 = U256::from(101); + let coldkey_weight_setter: U256 = U256::from(102); + + let stake_to_give_child = 109_999; + + // Register parent with minimal stake and child with high stake + SubtensorModule::add_balance_to_coldkey_account(&coldkey_parent, 1); + SubtensorModule::add_balance_to_coldkey_account(&coldkey_child, stake_to_give_child + 10); + SubtensorModule::add_balance_to_coldkey_account(&coldkey_weight_setter, 1_000_000); + + // Add neurons for parent, child and weight_setter + register_ok_neuron(netuid, parent, coldkey_parent, 1); + register_ok_neuron(netuid, child, coldkey_child, 1); + register_ok_neuron(netuid, weight_setter, coldkey_weight_setter, 1); + + SubtensorModule::increase_stake_on_coldkey_hotkey_account( + &coldkey_parent, + &parent, + stake_to_give_child, + ); + SubtensorModule::increase_stake_on_coldkey_hotkey_account( + &coldkey_weight_setter, + &weight_setter, + 1_000_000, + ); + + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + + // Set parent-child relationship + assert_ok!(SubtensorModule::do_set_children( + RuntimeOrigin::signed(coldkey_parent), + parent, + netuid, + vec![(u64::MAX, child)] + )); + step_block(7200 + 1); + // Set weights on the child using the weight_setter account + let origin = RuntimeOrigin::signed(weight_setter); + let uids: Vec = vec![1]; // Only set weight for the child (UID 1) + let values: Vec = vec![u16::MAX]; // Use maximum value for u16 + let version_key = SubtensorModule::get_weights_version_key(netuid); + assert_ok!(SubtensorModule::set_weights( + origin, + netuid, + uids.clone(), + values.clone(), + version_key + )); + + // Set the min stake very high + SubtensorModule::set_weights_min_stake(stake_to_give_child * 5); + + // Check the child has less stake than required + assert!( + SubtensorModule::get_stake_for_hotkey_on_subnet(&child, netuid) + < SubtensorModule::get_weights_min_stake() + ); + + // Check the child cannot set weights + assert_noop!( + SubtensorModule::set_weights( + RuntimeOrigin::signed(child), + netuid, + uids.clone(), + values.clone(), + version_key + ), + Error::::NotEnoughStakeToSetWeights + ); + + assert!(!SubtensorModule::check_weights_min_stake(&child, netuid)); + + // Set a minimum stake to set weights + SubtensorModule::set_weights_min_stake(stake_to_give_child - 5); + + // Check if the stake for the child is above + assert!( + SubtensorModule::get_stake_for_hotkey_on_subnet(&child, netuid) + >= SubtensorModule::get_weights_min_stake() + ); + + // Check the child can set weights + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(child), + netuid, + uids, + values, + version_key + )); + + assert!(SubtensorModule::check_weights_min_stake(&child, netuid)); + }); +} From a0b87da69ca883161d45bd184b568c40ac9145c8 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Thu, 12 Sep 2024 22:17:19 -0400 Subject: [PATCH 06/17] add test for regular keys to prevent regression --- pallets/subtensor/tests/children.rs | 83 +++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/pallets/subtensor/tests/children.rs b/pallets/subtensor/tests/children.rs index 9fe524a5b..0182888c0 100644 --- a/pallets/subtensor/tests/children.rs +++ b/pallets/subtensor/tests/children.rs @@ -3345,3 +3345,86 @@ fn test_childkey_set_weights_single_parent() { assert!(SubtensorModule::check_weights_min_stake(&child, netuid)); }); } + +// SKIP_WASM_BUILD=1 RUST_LOG=debug cargo test --package pallet-subtensor --test children -- test_set_weights_no_parent --exact --nocapture +#[test] +fn test_set_weights_no_parent() { + // Verify that a regular key without a parent delegation is effected by the minimum stake requirements + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + add_network(netuid, 1, 0); + + let hotkey: U256 = U256::from(2); + let spare_hk: U256 = U256::from(3); + + let coldkey: U256 = U256::from(101); + let spare_ck = U256::from(102); + + let stake_to_give_child = 109_999; + + SubtensorModule::add_balance_to_coldkey_account(&coldkey, stake_to_give_child + 10); + + // Is registered + register_ok_neuron(netuid, hotkey, coldkey, 1); + // Register a spare key + register_ok_neuron(netuid, spare_hk, spare_ck, 1); + + SubtensorModule::increase_stake_on_coldkey_hotkey_account( + &coldkey, + &hotkey, + stake_to_give_child, + ); + + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + + // Has stake and no parent + step_block(7200 + 1); + + let uids: Vec = vec![1]; // Set weights on the other hotkey + let values: Vec = vec![u16::MAX]; // Use maximum value for u16 + let version_key = SubtensorModule::get_weights_version_key(netuid); + + // Set the min stake very high + SubtensorModule::set_weights_min_stake(stake_to_give_child * 5); + + // Check the key has less stake than required + assert!( + SubtensorModule::get_stake_for_hotkey_on_subnet(&hotkey, netuid) + < SubtensorModule::get_weights_min_stake() + ); + + // Check the hotkey cannot set weights + assert_noop!( + SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + values.clone(), + version_key + ), + Error::::NotEnoughStakeToSetWeights + ); + + assert!(!SubtensorModule::check_weights_min_stake(&hotkey, netuid)); + + // Set a minimum stake to set weights + SubtensorModule::set_weights_min_stake(stake_to_give_child - 5); + + // Check if the stake for the hotkey is above + assert!( + SubtensorModule::get_stake_for_hotkey_on_subnet(&hotkey, netuid) + >= SubtensorModule::get_weights_min_stake() + ); + + // Check the hotkey can set weights + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids, + values, + version_key + )); + + assert!(SubtensorModule::check_weights_min_stake(&hotkey, netuid)); + }); +} From fff25299721b0c80a81dd3f1b4d5c3f363e5e818 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Fri, 13 Sep 2024 00:44:57 -0400 Subject: [PATCH 07/17] add test for total issuance/stake counters --- pallets/subtensor/tests/migration.rs | 34 ++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 0f6913a50..750e0679e 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -478,20 +478,33 @@ fn test_migrate_fix_pending_emissions() { let datura_old_hk_account: AccountId = get_account_id_from_ss58(datura_old_hotkey); let datura_new_hk_account: AccountId = get_account_id_from_ss58(datura_new_hotkey); + // "Issue" the TAO we're going to insert to stake + let null_stake_datura = 123_456_789; + let null_stake_tao_stats = 123_456_789; + let null_stake_total = null_stake_datura + null_stake_tao_stats; + SubtensorModule::set_total_issuance(null_stake_total); + TotalStake::::put(null_stake_total); + TotalColdkeyStake::::insert(null_account, null_stake_total); + TotalHotkeyStake::::insert(datura_old_hk_account, null_stake_datura); + TotalHotkeyStake::::insert(taostats_old_hk_account, null_stake_tao_stats); + // Setup the old Datura hotkey with a pending emission PendingdHotkeyEmission::::insert(datura_old_hk_account, 10_000); // Setup the NEW Datura hotkey with a pending emission - PendingdHotkeyEmission::::insert(datura_new_hk_account, 123_456_789); + PendingdHotkeyEmission::::insert(datura_new_hk_account, null_stake_datura); Stake::::insert(datura_old_hk_account, null_account, 123_456_789); - let expected_datura_new_hk_pending_emission: u64 = 123_456_789 + 10_000 + 123_456_789; + let expected_datura_new_hk_pending_emission: u64 = 123_456_789 + 10_000 + null_stake_datura; // Setup the old TaoStats hotkey with a pending emission PendingdHotkeyEmission::::insert(taostats_old_hk_account, 987_654); // Setup the new TaoStats hotkey with a pending emission PendingdHotkeyEmission::::insert(taostats_new_hk_account, 100_000); // Setup the old TaoStats hotkey with a null-key stake entry - Stake::::insert(taostats_old_hk_account, null_account, 123_456_789); - let expected_taostats_new_hk_pending_emission: u64 = 987_654 + 100_000 + 123_456_789; + Stake::::insert(taostats_old_hk_account, null_account, null_stake_tao_stats); + let expected_taostats_new_hk_pending_emission: u64 = + 987_654 + 100_000 + null_stake_tao_stats; + + let total_issuance_before = SubtensorModule::get_total_issuance(); // Run migration let first_weight = run_pending_emissions_migration_and_check(migration_name); @@ -523,5 +536,18 @@ fn test_migrate_fix_pending_emissions() { // Check the stake entry is removed assert_eq!(Stake::::get(datura_old_hk_account, null_account), 0); assert_eq!(Stake::::get(taostats_old_hk_account, null_account), 0); + + // Check the total issuance is decreased by the null stake removed + let expected_total_issuance = total_issuance_before - null_stake_total; + assert_eq!( + SubtensorModule::get_total_issuance(), + expected_total_issuance + ); + + // Check total stake is decreased by the null stake removed + assert_eq!(TotalStake::::get(), expected_total_issuance); + assert_eq!(TotalColdkeyStake::::get(null_account), 0); + assert_eq!(TotalHotkeyStake::::get(datura_old_hk_account), 0); + assert_eq!(TotalHotkeyStake::::get(taostats_old_hk_account), 0); }) } From 773670c4f96d00bf33fcbfab45506674870d51a8 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Fri, 13 Sep 2024 00:48:58 -0400 Subject: [PATCH 08/17] address tests --- .../migrations/migrate_fix_pending_emission.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs index afdaac4af..18fbc4204 100644 --- a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs +++ b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs @@ -21,7 +21,7 @@ fn migrate_pending_emissions_including_null_stake( new_hotkey: &T::AccountId, ) -> Weight { let mut weight = T::DbWeight::get().reads(0); - let null_account = DefaultAccount::::get(); + let null_account = &DefaultAccount::::get(); weight.saturating_accrue(T::DbWeight::get().reads(1)); // Get the pending emissions for the OLD hotkey @@ -30,18 +30,18 @@ fn migrate_pending_emissions_including_null_stake( weight.saturating_accrue(T::DbWeight::get().reads(1)); // Get the stake for the 0x000 key - let null_stake = Stake::::get(&old_hotkey, &null_account); + let null_stake = Stake::::get(&old_hotkey, null_account); weight.saturating_accrue(T::DbWeight::get().reads(1)); // Remove - Stake::::remove(&old_hotkey, &null_account); + Stake::::remove(&old_hotkey, null_account); weight.saturating_accrue(T::DbWeight::get().writes(1)); let new_total_coldkey_stake = - TotalColdkeyStake::::get(old_hotkey).saturating_sub(null_stake); + TotalColdkeyStake::::get(null_account).saturating_sub(null_stake); if new_total_coldkey_stake == 0 { - TotalColdkeyStake::::remove(old_hotkey); + TotalColdkeyStake::::remove(null_account); } else { - TotalColdkeyStake::::insert(old_hotkey, new_total_coldkey_stake); + TotalColdkeyStake::::insert(null_account, new_total_coldkey_stake); } weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); @@ -53,8 +53,10 @@ fn migrate_pending_emissions_including_null_stake( } weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + // Remove the stake from the total stake and total issuance (since it is re-emitted) TotalStake::::put(TotalStake::::get().saturating_sub(null_stake)); - weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + TotalIssuance::::put(TotalIssuance::::get().saturating_sub(null_stake)); + weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 2)); // Get the pending emissions for the NEW hotkey let pending_emissions_new: u64 = PendingdHotkeyEmission::::get(new_hotkey); From 9f4b90cc7109070dab6d584c1b8aea37c839f9e2 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Fri, 13 Sep 2024 12:09:23 -0400 Subject: [PATCH 09/17] add new migration method and more testing --- .../migrate_fix_pending_emission.rs | 65 ++++++++++++------ pallets/subtensor/tests/migration.rs | 68 ++++++++++++++----- 2 files changed, 96 insertions(+), 37 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs index 18fbc4204..a7220024e 100644 --- a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs +++ b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs @@ -19,6 +19,7 @@ fn get_account_id_from_ss58(ss58_str: &str) -> Result( old_hotkey: &T::AccountId, new_hotkey: &T::AccountId, + migration_account: &T::AccountId, ) -> Weight { let mut weight = T::DbWeight::get().reads(0); let null_account = &DefaultAccount::::get(); @@ -30,10 +31,10 @@ fn migrate_pending_emissions_including_null_stake( weight.saturating_accrue(T::DbWeight::get().reads(1)); // Get the stake for the 0x000 key - let null_stake = Stake::::get(&old_hotkey, null_account); + let null_stake = Stake::::get(old_hotkey, null_account); weight.saturating_accrue(T::DbWeight::get().reads(1)); // Remove - Stake::::remove(&old_hotkey, null_account); + Stake::::remove(old_hotkey, null_account); weight.saturating_accrue(T::DbWeight::get().writes(1)); let new_total_coldkey_stake = @@ -45,29 +46,35 @@ fn migrate_pending_emissions_including_null_stake( } weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); - let new_total_hotkey_stake = TotalHotkeyStake::::get(old_hotkey).saturating_sub(null_stake); - if new_total_hotkey_stake == 0 { - TotalHotkeyStake::::remove(old_hotkey); - } else { - TotalHotkeyStake::::insert(old_hotkey, new_total_hotkey_stake); - } + let new_staking_hotkeys = StakingHotkeys::::get(null_account); + let new_staking_hotkeys = new_staking_hotkeys + .into_iter() + .filter(|hk| hk != old_hotkey) + .collect::>(); + StakingHotkeys::::insert(null_account, new_staking_hotkeys); weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); - // Remove the stake from the total stake and total issuance (since it is re-emitted) - TotalStake::::put(TotalStake::::get().saturating_sub(null_stake)); - TotalIssuance::::put(TotalIssuance::::get().saturating_sub(null_stake)); - weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 2)); + // Insert the stake from the null account to the MIGRATION account under the OLD hotkey + Stake::::insert(old_hotkey, migration_account, null_stake); + TotalColdkeyStake::::insert( + migration_account, + TotalColdkeyStake::::get(migration_account).saturating_add(null_stake), + ); + let mut new_staking_hotkeys = StakingHotkeys::::get(migration_account); + if !new_staking_hotkeys.contains(old_hotkey) { + new_staking_hotkeys.push(old_hotkey.clone()); + } + StakingHotkeys::::insert(migration_account, new_staking_hotkeys); + weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 3)); // Get the pending emissions for the NEW hotkey let pending_emissions_new: u64 = PendingdHotkeyEmission::::get(new_hotkey); weight.saturating_accrue(T::DbWeight::get().reads(1)); - // Add stake to the pending emissions for the new hotkey and the old hotkey + // Add the pending emissions for the new hotkey and the old hotkey PendingdHotkeyEmission::::insert( new_hotkey, - pending_emissions_new - .saturating_add(pending_emissions_old) - .saturating_add(null_stake), + pending_emissions_new.saturating_add(pending_emissions_old), ); weight.saturating_accrue(T::DbWeight::get().writes(1)); @@ -80,19 +87,28 @@ pub fn do_migrate_fix_pending_emission() -> Weight { let taostats_old_hotkey = "5Hddm3iBFD2GLT5ik7LZnT3XJUnRnN8PoeCFgGQgawUVKNm8"; let taostats_new_hotkey = "5GKH9FPPnWSUoeeTJp19wVtd84XqFW4pyK2ijV2GsFbhTrP1"; + let migration_coldkey = "5D65DoFbapkYzJK17VRQo3HFs7FmMeicbaQern28UNDPypCT"; let taostats_old_hk_account = get_account_id_from_ss58::(taostats_old_hotkey); let taostats_new_hk_account = get_account_id_from_ss58::(taostats_new_hotkey); - - match (taostats_old_hk_account, taostats_new_hk_account) { - (Ok(taostats_old_hk_acct), Ok(taostats_new_hk_acct)) => { + let migration_ck_account = get_account_id_from_ss58::(migration_coldkey); + + match ( + taostats_old_hk_account, + taostats_new_hk_account, + migration_ck_account.clone(), + ) { + (Ok(taostats_old_hk_acct), Ok(taostats_new_hk_acct), Ok(migration_ck_account)) => { weight.saturating_accrue(migrate_pending_emissions_including_null_stake::( &taostats_old_hk_acct, &taostats_new_hk_acct, + &migration_ck_account, )); + log::info!("Migrated pending emissions from taostats old hotkey to new hotkey"); } _ => { log::warn!("Failed to get account id from ss58 for taostats hotkeys"); + return weight; } } @@ -102,15 +118,22 @@ pub fn do_migrate_fix_pending_emission() -> Weight { let datura_old_hk_account = get_account_id_from_ss58::(datura_old_hotkey); let datura_new_hk_account = get_account_id_from_ss58::(datura_new_hotkey); - match (datura_old_hk_account, datura_new_hk_account) { - (Ok(datura_old_hk_acct), Ok(datura_new_hk_acct)) => { + match ( + datura_old_hk_account, + datura_new_hk_account, + migration_ck_account, + ) { + (Ok(datura_old_hk_acct), Ok(datura_new_hk_acct), Ok(migration_ck_account)) => { weight.saturating_accrue(migrate_pending_emissions_including_null_stake::( &datura_old_hk_acct, &datura_new_hk_acct, + &migration_ck_account, )); + log::info!("Migrated pending emissions from datura old hotkey to new hotkey"); } _ => { log::warn!("Failed to get account id from ss58 for datura hotkeys"); + return weight; } } diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 750e0679e..59da65967 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -463,20 +463,22 @@ fn test_migrate_fix_pending_emissions() { new_test_ext(1).execute_with(|| { let migration_name = "fix_pending_emission"; - let null_account = U256::from(0); // The null account - let rand_coldkeys = [U256::from(1), U256::from(2), U256::from(3), U256::from(4)]; + let null_account = &U256::from(0); // The null account let taostats_old_hotkey = "5Hddm3iBFD2GLT5ik7LZnT3XJUnRnN8PoeCFgGQgawUVKNm8"; let taostats_new_hotkey = "5GKH9FPPnWSUoeeTJp19wVtd84XqFW4pyK2ijV2GsFbhTrP1"; - let taostats_old_hk_account: AccountId = get_account_id_from_ss58(taostats_old_hotkey); - let taostats_new_hk_account: AccountId = get_account_id_from_ss58(taostats_new_hotkey); + let taostats_old_hk_account: &AccountId = &get_account_id_from_ss58(taostats_old_hotkey); + let taostats_new_hk_account: &AccountId = &get_account_id_from_ss58(taostats_new_hotkey); let datura_old_hotkey = "5FKstHjZkh4v3qAMSBa1oJcHCLjxYZ8SNTSz1opTv4hR7gVB"; let datura_new_hotkey = "5GP7c3fFazW9GXK8Up3qgu2DJBk8inu4aK9TZy3RuoSWVCMi"; - let datura_old_hk_account: AccountId = get_account_id_from_ss58(datura_old_hotkey); - let datura_new_hk_account: AccountId = get_account_id_from_ss58(datura_new_hotkey); + let datura_old_hk_account: &AccountId = &get_account_id_from_ss58(datura_old_hotkey); + let datura_new_hk_account: &AccountId = &get_account_id_from_ss58(datura_new_hotkey); + + let migration_coldkey = "5D65DoFbapkYzJK17VRQo3HFs7FmMeicbaQern28UNDPypCT"; + let migration_account: &AccountId = &get_account_id_from_ss58(migration_coldkey); // "Issue" the TAO we're going to insert to stake let null_stake_datura = 123_456_789; @@ -491,9 +493,9 @@ fn test_migrate_fix_pending_emissions() { // Setup the old Datura hotkey with a pending emission PendingdHotkeyEmission::::insert(datura_old_hk_account, 10_000); // Setup the NEW Datura hotkey with a pending emission - PendingdHotkeyEmission::::insert(datura_new_hk_account, null_stake_datura); - Stake::::insert(datura_old_hk_account, null_account, 123_456_789); - let expected_datura_new_hk_pending_emission: u64 = 123_456_789 + 10_000 + null_stake_datura; + PendingdHotkeyEmission::::insert(datura_new_hk_account, 123_456_789); + Stake::::insert(datura_old_hk_account, null_account, null_stake_datura); + let expected_datura_new_hk_pending_emission: u64 = 123_456_789 + 10_000; // Setup the old TaoStats hotkey with a pending emission PendingdHotkeyEmission::::insert(taostats_old_hk_account, 987_654); @@ -501,8 +503,7 @@ fn test_migrate_fix_pending_emissions() { PendingdHotkeyEmission::::insert(taostats_new_hk_account, 100_000); // Setup the old TaoStats hotkey with a null-key stake entry Stake::::insert(taostats_old_hk_account, null_account, null_stake_tao_stats); - let expected_taostats_new_hk_pending_emission: u64 = - 987_654 + 100_000 + null_stake_tao_stats; + let expected_taostats_new_hk_pending_emission: u64 = 987_654 + 100_000; let total_issuance_before = SubtensorModule::get_total_issuance(); @@ -537,17 +538,52 @@ fn test_migrate_fix_pending_emissions() { assert_eq!(Stake::::get(datura_old_hk_account, null_account), 0); assert_eq!(Stake::::get(taostats_old_hk_account, null_account), 0); - // Check the total issuance is decreased by the null stake removed - let expected_total_issuance = total_issuance_before - null_stake_total; + // Check the total issuance is the SAME following migration (no TAO issued) + let expected_total_issuance = total_issuance_before; assert_eq!( SubtensorModule::get_total_issuance(), expected_total_issuance ); - // Check total stake is decreased by the null stake removed + // Check total stake is the SAME following the migration (no new TAO staked) assert_eq!(TotalStake::::get(), expected_total_issuance); + // Check the total stake maps are updated following the migration (removal of old null_account stake entries) assert_eq!(TotalColdkeyStake::::get(null_account), 0); - assert_eq!(TotalHotkeyStake::::get(datura_old_hk_account), 0); - assert_eq!(TotalHotkeyStake::::get(taostats_old_hk_account), 0); + assert_eq!( + SubtensorModule::get_stake_for_coldkey_and_hotkey(null_account, datura_old_hk_account), + 0 + ); + assert_eq!( + SubtensorModule::get_stake_for_coldkey_and_hotkey( + null_account, + taostats_old_hk_account + ), + 0 + ); + + // Check staking hotkeys is updated + assert_eq!(StakingHotkeys::::get(null_account), vec![]); + + // Check the migration key has stake with both *old* hotkeys + assert_eq!( + SubtensorModule::get_stake_for_coldkey_and_hotkey( + migration_account, + datura_old_hk_account + ), + null_stake_datura + ); + assert_eq!( + SubtensorModule::get_stake_for_coldkey_and_hotkey( + migration_account, + taostats_old_hk_account + ), + null_stake_tao_stats + ); + assert_eq!( + TotalColdkeyStake::::get(migration_account), + null_stake_total + ); + assert!(StakingHotkeys::::get(migration_account).contains(&datura_old_hk_account)); + assert!(StakingHotkeys::::get(migration_account).contains(&taostats_old_hk_account)); }) } From 870689fc72a0dc4e4f2c6e89a39a26e0a0c5c4f1 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Fri, 13 Sep 2024 12:24:50 -0400 Subject: [PATCH 10/17] chore: clippy --- pallets/subtensor/tests/migration.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 59da65967..e75f8790f 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -583,7 +583,7 @@ fn test_migrate_fix_pending_emissions() { TotalColdkeyStake::::get(migration_account), null_stake_total ); - assert!(StakingHotkeys::::get(migration_account).contains(&datura_old_hk_account)); - assert!(StakingHotkeys::::get(migration_account).contains(&taostats_old_hk_account)); + assert!(StakingHotkeys::::get(migration_account).contains(datura_old_hk_account)); + assert!(StakingHotkeys::::get(migration_account).contains(taostats_old_hk_account)); }) } From 60fd5284c15f5405b375aa58bfd98b7692cb9502 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Fri, 13 Sep 2024 12:25:50 -0400 Subject: [PATCH 11/17] spec version only 1 more than mainnet --- runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index f44f4eee2..1aa30ac3c 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -142,7 +142,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: 203, + spec_version: 202, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From 4ad2c5bc2ca78116f324c0c217260c9403a3e558 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Fri, 13 Sep 2024 13:53:04 -0400 Subject: [PATCH 12/17] use new migration multisig --- .../subtensor/src/migrations/migrate_fix_pending_emission.rs | 2 +- pallets/subtensor/tests/migration.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs index a7220024e..b41c9070b 100644 --- a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs +++ b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs @@ -87,7 +87,7 @@ pub fn do_migrate_fix_pending_emission() -> Weight { let taostats_old_hotkey = "5Hddm3iBFD2GLT5ik7LZnT3XJUnRnN8PoeCFgGQgawUVKNm8"; let taostats_new_hotkey = "5GKH9FPPnWSUoeeTJp19wVtd84XqFW4pyK2ijV2GsFbhTrP1"; - let migration_coldkey = "5D65DoFbapkYzJK17VRQo3HFs7FmMeicbaQern28UNDPypCT"; + let migration_coldkey = "5GeRjQYsobRWFnrbBmGe5ugme3rfnDVF69N45YtdBpUFsJG8"; let taostats_old_hk_account = get_account_id_from_ss58::(taostats_old_hotkey); let taostats_new_hk_account = get_account_id_from_ss58::(taostats_new_hotkey); diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index e75f8790f..cf1a205fc 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -477,7 +477,7 @@ fn test_migrate_fix_pending_emissions() { let datura_old_hk_account: &AccountId = &get_account_id_from_ss58(datura_old_hotkey); let datura_new_hk_account: &AccountId = &get_account_id_from_ss58(datura_new_hotkey); - let migration_coldkey = "5D65DoFbapkYzJK17VRQo3HFs7FmMeicbaQern28UNDPypCT"; + let migration_coldkey = "5GeRjQYsobRWFnrbBmGe5ugme3rfnDVF69N45YtdBpUFsJG8"; let migration_account: &AccountId = &get_account_id_from_ss58(migration_coldkey); // "Issue" the TAO we're going to insert to stake From 625fc9c08e0fd957e73632db7a04f9a85e98b784 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Mon, 16 Sep 2024 14:32:39 -0400 Subject: [PATCH 13/17] remove old migration runner --- runtime/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 1aa30ac3c..09ea95ef6 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1076,9 +1076,9 @@ pub type SignedExtra = ( ); type Migrations = - pallet_subtensor::migrations::migrate_init_total_issuance::initialise_total_issuance::Migration< + (pallet_subtensor::migrations::migrate_fix_pending_emission::migrate_fix_pending_emission::Migration< Runtime, - >; + >); // Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = From 67aad027dcfda927977890f2bc703416e0302346 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Mon, 16 Sep 2024 14:57:15 -0400 Subject: [PATCH 14/17] add try runtime check for new migration --- pallets/subtensor/src/macros/hooks.rs | 4 +- .../migrate_fix_pending_emission.rs | 328 ++++++++++++++++-- pallets/subtensor/tests/migration.rs | 6 +- runtime/src/lib.rs | 4 +- 4 files changed, 306 insertions(+), 36 deletions(-) diff --git a/pallets/subtensor/src/macros/hooks.rs b/pallets/subtensor/src/macros/hooks.rs index 865cabdc5..76f140002 100644 --- a/pallets/subtensor/src/macros/hooks.rs +++ b/pallets/subtensor/src/macros/hooks.rs @@ -70,9 +70,7 @@ mod hooks { // Storage version v8 -> v9 .saturating_add(migrations::migrate_fix_total_coldkey_stake::migrate_fix_total_coldkey_stake::()) // Migrate Delegate Ids on chain - .saturating_add(migrations::migrate_chain_identity::migrate_set_hotkey_identities::()) - // Fix pending emissions - .saturating_add(migrations::migrate_fix_pending_emission::migrate_fix_pending_emission::()); + .saturating_add(migrations::migrate_chain_identity::migrate_set_hotkey_identities::()); weight } diff --git a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs index b41c9070b..163093ff8 100644 --- a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs +++ b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs @@ -81,6 +81,9 @@ fn migrate_pending_emissions_including_null_stake( weight } +// This executes the migration to fix the pending emissions +// This also migrates the stake entry of (old_hotkey, 0x000) to the Migration Account for +// both the old hotkeys. pub fn do_migrate_fix_pending_emission() -> Weight { // Initialize the weight with one read operation. let mut weight = T::DbWeight::get().reads(1); @@ -139,39 +142,310 @@ pub fn do_migrate_fix_pending_emission() -> Weight { weight } -// Public migrate function to be called by Lib.rs on upgrade. -pub fn migrate_fix_pending_emission() -> Weight { - let migration_name = b"fix_pending_emission".to_vec(); - // Initialize the weight with one read operation. - let mut weight = T::DbWeight::get().reads(1); +/// Collection of storage item formats from the previous storage version. +/// +/// Required so we can read values in the v0 storage format during the migration. +#[cfg(feature = "try-runtime")] +mod v0 { + use frame_support::storage_alias; + use subtensor_macros::freeze_struct; - // Check if the migration has already run - if HasMigrationRun::::get(&migration_name) { - log::info!( - "Migration '{:?}' has already run. Skipping.", - migration_name - ); - return Weight::zero(); + #[freeze_struct("7adbed79987ae83d")] + #[derive(codec::Encode, codec::Decode, Clone, PartialEq, Debug)] + pub struct OldStorage { + pub total_issuance_before: u64, + pub expected_taostats_new_hk_pending_emission: u64, + pub expected_datura_new_hk_pending_emission: u64, + pub old_null_stake_taostats: u64, + pub old_null_stake_datura: u64, } - log::info!( - "Running migration '{}'", - String::from_utf8_lossy(&migration_name) - ); + /// V0 type for [`crate::Value`]. + #[storage_alias] + pub type Value = StorageValue, OldStorage>; +} - // Run the migration - weight.saturating_accrue(do_migrate_fix_pending_emission::()); +impl Pallet { + #[cfg(feature = "try-runtime")] + fn check_null_stake_invariants( + old_storage: v0::OldStorage, + ) -> Result<(), sp_runtime::TryRuntimeError> { + let null_account = &DefaultAccount::::get(); - // Mark the migration as completed - HasMigrationRun::::insert(&migration_name, true); - weight.saturating_accrue(T::DbWeight::get().writes(1)); + let taostats_old_hotkey = "5Hddm3iBFD2GLT5ik7LZnT3XJUnRnN8PoeCFgGQgawUVKNm8"; + let taostats_new_hotkey = "5GKH9FPPnWSUoeeTJp19wVtd84XqFW4pyK2ijV2GsFbhTrP1"; + let migration_coldkey = "5GeRjQYsobRWFnrbBmGe5ugme3rfnDVF69N45YtdBpUFsJG8"; - log::info!( - "Migration '{:?}' completed. Marked in storage.", - String::from_utf8_lossy(&migration_name) - ); + let taostats_old_hk_account = &get_account_id_from_ss58::(taostats_old_hotkey); + let taostats_new_hk_account = &get_account_id_from_ss58::(taostats_new_hotkey); + let migration_ck_account = &get_account_id_from_ss58::(migration_coldkey); - // Return the migration weight. - weight + let old = old_storage; + let null_stake_total = old + .old_null_stake_taostats + .saturating_add(old.old_null_stake_datura); + + match ( + taostats_old_hk_account, + taostats_new_hk_account, + migration_ck_account, + ) { + (Ok(taostats_old_hk_acct), Ok(taostats_new_hk_acct), Ok(migration_ck_acct)) => { + // Check the pending emission is added to new the TaoStats hotkey + assert_eq!( + PendingdHotkeyEmission::::get(taostats_new_hk_acct), + old.expected_taostats_new_hk_pending_emission + ); + + assert_eq!(PendingdHotkeyEmission::::get(taostats_old_hk_acct), 0); + + assert_eq!(Stake::::get(taostats_old_hk_acct, null_account), 0); + + assert!(StakingHotkeys::::get(migration_ck_acct).contains(taostats_old_hk_acct)); + + assert_eq!( + Self::get_stake_for_coldkey_and_hotkey(null_account, taostats_old_hk_acct), + 0 + ); + + let new_null_stake_taostats = + Self::get_stake_for_coldkey_and_hotkey(migration_ck_acct, taostats_old_hk_acct); + + assert_eq!(new_null_stake_taostats, old.old_null_stake_taostats); + } + _ => { + log::warn!("Failed to get account id from ss58 for taostats hotkeys"); + return Err("Failed to get account id from ss58 for taostats hotkeys".into()); + } + } + + let datura_old_hotkey = "5FKstHjZkh4v3qAMSBa1oJcHCLjxYZ8SNTSz1opTv4hR7gVB"; + let datura_new_hotkey = "5GP7c3fFazW9GXK8Up3qgu2DJBk8inu4aK9TZy3RuoSWVCMi"; + + let datura_old_hk_account = &get_account_id_from_ss58::(datura_old_hotkey); + let datura_new_hk_account = &get_account_id_from_ss58::(datura_new_hotkey); + + match ( + datura_old_hk_account, + datura_new_hk_account, + migration_ck_account, + ) { + (Ok(datura_old_hk_acct), Ok(datura_new_hk_acct), Ok(migration_ck_acct)) => { + // Check the pending emission is added to new Datura hotkey + assert_eq!( + crate::PendingdHotkeyEmission::::get(datura_new_hk_acct), + old.expected_datura_new_hk_pending_emission + ); + + // Check the pending emission is removed from old ones + assert_eq!(PendingdHotkeyEmission::::get(datura_old_hk_acct), 0); + + // Check the stake entry is removed + assert_eq!(Stake::::get(datura_old_hk_acct, null_account), 0); + + assert!(StakingHotkeys::::get(migration_ck_acct).contains(datura_old_hk_acct)); + + assert_eq!( + Self::get_stake_for_coldkey_and_hotkey(null_account, datura_old_hk_acct), + 0 + ); + + let new_null_stake_datura = + Self::get_stake_for_coldkey_and_hotkey(migration_ck_acct, datura_old_hk_acct); + + assert_eq!(new_null_stake_datura, old.old_null_stake_datura); + } + _ => { + log::warn!("Failed to get account id from ss58 for datura hotkeys"); + return Err("Failed to get account id from ss58 for datura hotkeys".into()); + } + } + + match migration_ck_account { + Ok(migration_ck_acct) => { + // Check the migration key has stake with both *old* hotkeys + assert_eq!( + TotalColdkeyStake::::get(migration_ck_acct), + null_stake_total + ); + } + _ => { + log::warn!("Failed to get account id from ss58 for migration coldkey"); + return Err("Failed to get account id from ss58 for migration coldkey".into()); + } + } + + // Check the total issuance is the SAME following migration (no TAO issued) + let expected_total_issuance = old.total_issuance_before; + assert_eq!(Self::get_total_issuance(), expected_total_issuance); + + // Check total stake is the SAME following the migration (no new TAO staked) + assert_eq!(TotalStake::::get(), expected_total_issuance); + // Check the total stake maps are updated following the migration (removal of old null_account stake entries) + assert_eq!(TotalColdkeyStake::::get(null_account), 0); + + // Check staking hotkeys is updated + assert_eq!(StakingHotkeys::::get(null_account), vec![]); + + Ok(()) + } +} + +pub mod migration { + use frame_support::pallet_prelude::Weight; + use frame_support::traits::OnRuntimeUpgrade; + use sp_core::Get; + + use super::*; + + pub struct Migration(PhantomData); + + #[cfg(feature = "try-runtime")] + fn get_old_storage_values() -> Result { + let null_account = &DefaultAccount::::get(); + + let taostats_old_hotkey = "5Hddm3iBFD2GLT5ik7LZnT3XJUnRnN8PoeCFgGQgawUVKNm8"; + let taostats_new_hotkey = "5GKH9FPPnWSUoeeTJp19wVtd84XqFW4pyK2ijV2GsFbhTrP1"; + + let taostats_old_hk_account = &get_account_id_from_ss58::(taostats_old_hotkey); + let taostats_new_hk_account = &get_account_id_from_ss58::(taostats_new_hotkey); + + let total_issuance_before = crate::Pallet::::get_total_issuance(); + let mut expected_taostats_new_hk_pending_emission: u64 = 0; + let mut expected_datura_new_hk_pending_emission: u64 = 0; + let old_null_stake_taostats: u64 = match (taostats_old_hk_account, taostats_new_hk_account) + { + (Ok(taostats_old_hk_acct), Ok(taostats_new_hk_acct)) => { + expected_taostats_new_hk_pending_emission = + expected_taostats_new_hk_pending_emission + .saturating_add(PendingdHotkeyEmission::::get(taostats_old_hk_acct)) + .saturating_add(PendingdHotkeyEmission::::get(taostats_new_hk_acct)); + + Ok::( + crate::Pallet::::get_stake_for_coldkey_and_hotkey( + null_account, + taostats_old_hk_acct, + ), + ) + } + _ => { + log::warn!("Failed to get account id from ss58 for taostats hotkeys"); + Err("Failed to get account id from ss58 for taostats hotkeys".into()) + } + }?; + + let datura_old_hotkey = "5FKstHjZkh4v3qAMSBa1oJcHCLjxYZ8SNTSz1opTv4hR7gVB"; + let datura_new_hotkey = "5GP7c3fFazW9GXK8Up3qgu2DJBk8inu4aK9TZy3RuoSWVCMi"; + + let datura_old_hk_account = &get_account_id_from_ss58::(datura_old_hotkey); + let datura_new_hk_account = &get_account_id_from_ss58::(datura_new_hotkey); + + let old_null_stake_datura: u64 = match (datura_old_hk_account, datura_new_hk_account) { + (Ok(datura_old_hk_acct), Ok(datura_new_hk_acct)) => { + expected_datura_new_hk_pending_emission = expected_datura_new_hk_pending_emission + .saturating_add(PendingdHotkeyEmission::::get(datura_old_hk_acct)) + .saturating_add(PendingdHotkeyEmission::::get(datura_new_hk_acct)); + + Ok::( + crate::Pallet::::get_stake_for_coldkey_and_hotkey( + null_account, + datura_old_hk_acct, + ), + ) + } + _ => { + log::warn!("Failed to get account id from ss58 for datura hotkeys"); + Err("Failed to get account id from ss58 for datura hotkeys".into()) + } + }?; + + let result = v0::OldStorage { + total_issuance_before, + expected_taostats_new_hk_pending_emission, + expected_datura_new_hk_pending_emission, + old_null_stake_taostats, + old_null_stake_datura, + }; + + Ok(result) + } + + impl OnRuntimeUpgrade for Migration { + /// Runs the migration to fix the pending emissions. + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + use codec::Encode; + + // Get the old storage values + let old_storage = get_old_storage_values::()?; + + // Return it as an encoded `Vec` + Ok(old_storage.encode()) + } + + // Runs the migrate function for the fix_pending_emission migration + fn on_runtime_upgrade() -> Weight { + let migration_name = b"fix_pending_emission".to_vec(); + + // Initialize the weight with one read operation. + let mut weight = T::DbWeight::get().reads(1); + + // Check if the migration has already run + if HasMigrationRun::::get(&migration_name) { + log::info!( + "Migration '{:?}' has already run. Skipping.", + migration_name + ); + return Weight::zero(); + } + + log::info!( + "Running migration '{}'", + String::from_utf8_lossy(&migration_name) + ); + + // Run the migration + weight.saturating_accrue( + migrations::migrate_fix_pending_emission::do_migrate_fix_pending_emission::(), + ); + + // Mark the migration as completed + HasMigrationRun::::insert(&migration_name, true); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + + log::info!( + "Migration '{:?}' completed. Marked in storage.", + String::from_utf8_lossy(&migration_name) + ); + + // Return the migration weight. + weight + } + + /// Performs post-upgrade checks to ensure the migration was successful. + /// + /// This function is only compiled when the "try-runtime" feature is enabled. + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + use codec::Decode; + + let maybe_old_value = + Option::::decode(&mut &state[..]).map_err(|_| { + sp_runtime::TryRuntimeError::Other("Failed to decode old value from storage") + })?; + + match maybe_old_value { + Some(old_value) => { + // Verify that all null stake invariants are satisfied after the migration + crate::Pallet::::check_null_stake_invariants(old_value)?; + } + None => { + log::warn!("Failed to decode old value from storage"); + return Err("Failed to decode old value from storage".into()); + } + }; + Ok(()) + } + } } diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index cf1a205fc..5cce9dfde 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -436,11 +436,11 @@ fn run_migration_and_check(migration_name: &'static str) -> frame_support::weigh fn run_pending_emissions_migration_and_check( migration_name: &'static str, ) -> frame_support::weights::Weight { + use frame_support::traits::OnRuntimeUpgrade; + // Execute the migration and store its weight let weight: frame_support::weights::Weight = - pallet_subtensor::migrations::migrate_fix_pending_emission::migrate_fix_pending_emission::< - Test, - >(); + pallet_subtensor::migrations::migrate_fix_pending_emission::migration::Migration::::on_runtime_upgrade(); // Check if the migration has been marked as completed assert!(HasMigrationRun::::get( diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 09ea95ef6..031328d5c 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1076,9 +1076,7 @@ pub type SignedExtra = ( ); type Migrations = - (pallet_subtensor::migrations::migrate_fix_pending_emission::migrate_fix_pending_emission::Migration< - Runtime, - >); + (pallet_subtensor::migrations::migrate_fix_pending_emission::migration::Migration,); // Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = From de41a3496bd9d799cbf45cdf6f62f0bc4aaa6069 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Mon, 16 Sep 2024 23:33:33 -0400 Subject: [PATCH 15/17] make check idempot --- .../migrate_fix_pending_emission.rs | 114 ++++++++++++------ 1 file changed, 80 insertions(+), 34 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs index 163093ff8..ed2f821b0 100644 --- a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs +++ b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs @@ -148,22 +148,20 @@ pub fn do_migrate_fix_pending_emission() -> Weight { /// Required so we can read values in the v0 storage format during the migration. #[cfg(feature = "try-runtime")] mod v0 { - use frame_support::storage_alias; use subtensor_macros::freeze_struct; - #[freeze_struct("7adbed79987ae83d")] + #[freeze_struct("2228babfc0580c62")] #[derive(codec::Encode, codec::Decode, Clone, PartialEq, Debug)] pub struct OldStorage { pub total_issuance_before: u64, + pub total_stake_before: u64, pub expected_taostats_new_hk_pending_emission: u64, pub expected_datura_new_hk_pending_emission: u64, + pub old_migration_stake_taostats: u64, pub old_null_stake_taostats: u64, + pub old_migration_stake_datura: u64, pub old_null_stake_datura: u64, } - - /// V0 type for [`crate::Value`]. - #[storage_alias] - pub type Value = StorageValue, OldStorage>; } impl Pallet { @@ -184,7 +182,9 @@ impl Pallet { let old = old_storage; let null_stake_total = old .old_null_stake_taostats - .saturating_add(old.old_null_stake_datura); + .saturating_add(old.old_null_stake_datura) + .saturating_add(old.old_migration_stake_taostats) + .saturating_add(old.old_migration_stake_datura); match ( taostats_old_hk_account, @@ -209,10 +209,19 @@ impl Pallet { 0 ); + // Check the total hotkey stake is the same + assert_eq!( + TotalHotkeyStake::::get(taostats_old_hk_acct), + old.old_null_stake_taostats + old.old_migration_stake_taostats + ); + let new_null_stake_taostats = Self::get_stake_for_coldkey_and_hotkey(migration_ck_acct, taostats_old_hk_acct); - assert_eq!(new_null_stake_taostats, old.old_null_stake_taostats); + assert_eq!( + new_null_stake_taostats, + old.old_null_stake_taostats + old.old_migration_stake_taostats + ); } _ => { log::warn!("Failed to get account id from ss58 for taostats hotkeys"); @@ -251,10 +260,19 @@ impl Pallet { 0 ); + // Check the total hotkey stake is the same + assert_eq!( + TotalHotkeyStake::::get(datura_old_hk_acct), + old.old_null_stake_datura + old.old_migration_stake_datura + ); + let new_null_stake_datura = Self::get_stake_for_coldkey_and_hotkey(migration_ck_acct, datura_old_hk_acct); - assert_eq!(new_null_stake_datura, old.old_null_stake_datura); + assert_eq!( + new_null_stake_datura, + old.old_null_stake_datura + old.old_migration_stake_datura + ); } _ => { log::warn!("Failed to get account id from ss58 for datura hotkeys"); @@ -278,10 +296,11 @@ impl Pallet { // Check the total issuance is the SAME following migration (no TAO issued) let expected_total_issuance = old.total_issuance_before; + let expected_total_stake = old.total_stake_before; assert_eq!(Self::get_total_issuance(), expected_total_issuance); // Check total stake is the SAME following the migration (no new TAO staked) - assert_eq!(TotalStake::::get(), expected_total_issuance); + assert_eq!(TotalStake::::get(), expected_total_stake); // Check the total stake maps are updated following the migration (removal of old null_account stake entries) assert_eq!(TotalColdkeyStake::::get(null_account), 0); @@ -303,7 +322,11 @@ pub mod migration { #[cfg(feature = "try-runtime")] fn get_old_storage_values() -> Result { + log::info!("Getting old storage values for migration"); + let null_account = &DefaultAccount::::get(); + let migration_coldkey = "5GeRjQYsobRWFnrbBmGe5ugme3rfnDVF69N45YtdBpUFsJG8"; + let migration_account = &get_account_id_from_ss58::(migration_coldkey); let taostats_old_hotkey = "5Hddm3iBFD2GLT5ik7LZnT3XJUnRnN8PoeCFgGQgawUVKNm8"; let taostats_new_hotkey = "5GKH9FPPnWSUoeeTJp19wVtd84XqFW4pyK2ijV2GsFbhTrP1"; @@ -314,20 +337,27 @@ pub mod migration { let total_issuance_before = crate::Pallet::::get_total_issuance(); let mut expected_taostats_new_hk_pending_emission: u64 = 0; let mut expected_datura_new_hk_pending_emission: u64 = 0; - let old_null_stake_taostats: u64 = match (taostats_old_hk_account, taostats_new_hk_account) - { - (Ok(taostats_old_hk_acct), Ok(taostats_new_hk_acct)) => { + let (old_null_stake_taostats, old_migration_stake_taostats) = match ( + taostats_old_hk_account, + taostats_new_hk_account, + migration_account, + ) { + (Ok(taostats_old_hk_acct), Ok(taostats_new_hk_acct), Ok(migration_acct)) => { expected_taostats_new_hk_pending_emission = expected_taostats_new_hk_pending_emission .saturating_add(PendingdHotkeyEmission::::get(taostats_old_hk_acct)) .saturating_add(PendingdHotkeyEmission::::get(taostats_new_hk_acct)); - Ok::( + Ok::<(u64, u64), sp_runtime::TryRuntimeError>(( crate::Pallet::::get_stake_for_coldkey_and_hotkey( null_account, taostats_old_hk_acct, ), - ) + crate::Pallet::::get_stake_for_coldkey_and_hotkey( + migration_acct, + taostats_old_hk_acct, + ), + )) } _ => { log::warn!("Failed to get account id from ss58 for taostats hotkeys"); @@ -341,18 +371,26 @@ pub mod migration { let datura_old_hk_account = &get_account_id_from_ss58::(datura_old_hotkey); let datura_new_hk_account = &get_account_id_from_ss58::(datura_new_hotkey); - let old_null_stake_datura: u64 = match (datura_old_hk_account, datura_new_hk_account) { - (Ok(datura_old_hk_acct), Ok(datura_new_hk_acct)) => { + let (old_null_stake_datura, old_migration_stake_datura) = match ( + datura_old_hk_account, + datura_new_hk_account, + migration_account, + ) { + (Ok(datura_old_hk_acct), Ok(datura_new_hk_acct), Ok(migration_acct)) => { expected_datura_new_hk_pending_emission = expected_datura_new_hk_pending_emission .saturating_add(PendingdHotkeyEmission::::get(datura_old_hk_acct)) .saturating_add(PendingdHotkeyEmission::::get(datura_new_hk_acct)); - Ok::( + Ok::<(u64, u64), sp_runtime::TryRuntimeError>(( crate::Pallet::::get_stake_for_coldkey_and_hotkey( null_account, datura_old_hk_acct, ), - ) + crate::Pallet::::get_stake_for_coldkey_and_hotkey( + migration_acct, + datura_old_hk_acct, + ), + )) } _ => { log::warn!("Failed to get account id from ss58 for datura hotkeys"); @@ -360,14 +398,21 @@ pub mod migration { } }?; + let total_stake_before: u64 = crate::Pallet::::get_total_stake(); + let result = v0::OldStorage { total_issuance_before, + total_stake_before, expected_taostats_new_hk_pending_emission, expected_datura_new_hk_pending_emission, + old_migration_stake_taostats, old_null_stake_taostats, + old_migration_stake_datura, old_null_stake_datura, }; + log::info!("Got old storage values for migration"); + Ok(result) } @@ -378,10 +423,18 @@ pub mod migration { use codec::Encode; // Get the old storage values - let old_storage = get_old_storage_values::()?; + match get_old_storage_values::() { + Ok(old_storage) => { + log::info!("Successfully got old storage values for migration"); + let encoded = old_storage.encode(); - // Return it as an encoded `Vec` - Ok(old_storage.encode()) + Ok(encoded) + } + Err(e) => { + log::error!("Failed to get old storage values for migration: {:?}", e); + Err("Failed to get old storage values for migration".into()) + } + } } // Runs the migrate function for the fix_pending_emission migration @@ -430,21 +483,14 @@ pub mod migration { fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { use codec::Decode; - let maybe_old_value = - Option::::decode(&mut &state[..]).map_err(|_| { + let old_storage: v0::OldStorage = + v0::OldStorage::decode(&mut &state[..]).map_err(|_| { sp_runtime::TryRuntimeError::Other("Failed to decode old value from storage") })?; - match maybe_old_value { - Some(old_value) => { - // Verify that all null stake invariants are satisfied after the migration - crate::Pallet::::check_null_stake_invariants(old_value)?; - } - None => { - log::warn!("Failed to decode old value from storage"); - return Err("Failed to decode old value from storage".into()); - } - }; + // Verify that all null stake invariants are satisfied after the migration + crate::Pallet::::check_null_stake_invariants(old_storage)?; + Ok(()) } } From c4215896d79cd47fb5f541045bdac95387f257ea Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Tue, 17 Sep 2024 11:55:41 -0400 Subject: [PATCH 16/17] chore: clippy --- .../src/migrations/migrate_fix_pending_emission.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs index ed2f821b0..b5e833aeb 100644 --- a/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs +++ b/pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs @@ -212,7 +212,8 @@ impl Pallet { // Check the total hotkey stake is the same assert_eq!( TotalHotkeyStake::::get(taostats_old_hk_acct), - old.old_null_stake_taostats + old.old_migration_stake_taostats + old.old_null_stake_taostats + .saturating_add(old.old_migration_stake_taostats) ); let new_null_stake_taostats = @@ -220,7 +221,8 @@ impl Pallet { assert_eq!( new_null_stake_taostats, - old.old_null_stake_taostats + old.old_migration_stake_taostats + old.old_null_stake_taostats + .saturating_add(old.old_migration_stake_taostats) ); } _ => { @@ -263,7 +265,8 @@ impl Pallet { // Check the total hotkey stake is the same assert_eq!( TotalHotkeyStake::::get(datura_old_hk_acct), - old.old_null_stake_datura + old.old_migration_stake_datura + old.old_null_stake_datura + .saturating_add(old.old_migration_stake_datura) ); let new_null_stake_datura = @@ -271,7 +274,8 @@ impl Pallet { assert_eq!( new_null_stake_datura, - old.old_null_stake_datura + old.old_migration_stake_datura + old.old_null_stake_datura + .saturating_add(old.old_migration_stake_datura) ); } _ => { From 4c2629e35e74707fd96ca572847d87086c420adb Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Tue, 17 Sep 2024 12:08:20 -0400 Subject: [PATCH 17/17] Add back old migration and fix var names --- pallets/subtensor/src/utils/try_state.rs | 2 +- runtime/src/lib.rs | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pallets/subtensor/src/utils/try_state.rs b/pallets/subtensor/src/utils/try_state.rs index 4763c0484..10abf970f 100644 --- a/pallets/subtensor/src/utils/try_state.rs +++ b/pallets/subtensor/src/utils/try_state.rs @@ -17,7 +17,7 @@ impl Pallet { // Calculate the total staked amount let mut total_staked: u64 = 0; - for (_account, _netuid, stake) in Stake::::iter() { + for (_hotkey, _coldkey, stake) in Stake::::iter() { total_staked = total_staked.saturating_add(stake); } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 031328d5c..270b0a069 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1075,8 +1075,12 @@ pub type SignedExtra = ( frame_metadata_hash_extension::CheckMetadataHash, ); -type Migrations = - (pallet_subtensor::migrations::migrate_fix_pending_emission::migration::Migration,); +type Migrations = ( + pallet_subtensor::migrations::migrate_init_total_issuance::initialise_total_issuance::Migration< + Runtime, + >, + pallet_subtensor::migrations::migrate_fix_pending_emission::migration::Migration, +); // Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic =