Merge pull request #809 from opentensor/hotfix/pending-emission-migration-patch

Hotfix/pending emission migration patch and fix set weights check

Author: unconst

Diff for: 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::<T>())
                 // Migrate Delegate Ids on chain
-                .saturating_add(migrations::migrate_chain_identity::migrate_set_hotkey_identities::<T>())
-				// Fix pending emissions
-				.saturating_add(migrations::migrate_fix_pending_emission::migrate_fix_pending_emission::<T>());
+                .saturating_add(migrations::migrate_chain_identity::migrate_set_hotkey_identities::<T>());
             weight
         }
 

Diff for: pallets/subtensor/src/migrations/migrate_fix_pending_emission.rs

@@ -226,9 +226,9 @@ impl<T: Config> Pallet<T> {
             Error::<T>::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::<T>::NotEnoughStakeToSetWeights
         );
 

Diff for: pallets/subtensor/src/subnets/weights.rs

@@ -17,7 +17,7 @@ impl<T: Config> Pallet<T> {
 
         // Calculate the total staked amount
         let mut total_staked: u64 = 0;
-        for (_account, _netuid, stake) in Stake::<T>::iter() {
+        for (_hotkey, _coldkey, stake) in Stake::<T>::iter() {
             total_staked = total_staked.saturating_add(stake);
         }
 

Diff for: pallets/subtensor/src/utils/try_state.rs

@@ -3237,3 +3237,194 @@ 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<u16> = vec![1]; // Only set weight for the child (UID 1)
+        let values: Vec<u16> = 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::<Test>::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));
+    });
+}
+
+// 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<u16> = vec![1]; // Set weights on the other hotkey
+        let values: Vec<u16> = 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::<Test>::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));
+    });
+}

Diff for: pallets/subtensor/tests/children.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::<Test>::on_runtime_upgrade();
 
     // Check if the migration has been marked as completed
     assert!(HasMigrationRun::<Test>::get(
@@ -463,34 +463,49 @@ 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 = "5GeRjQYsobRWFnrbBmGe5ugme3rfnDVF69N45YtdBpUFsJG8";
+        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;
+        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::<Test>::put(null_stake_total);
+        TotalColdkeyStake::<Test>::insert(null_account, null_stake_total);
+        TotalHotkeyStake::<Test>::insert(datura_old_hk_account, null_stake_datura);
+        TotalHotkeyStake::<Test>::insert(taostats_old_hk_account, null_stake_tao_stats);
 
         // Setup the old Datura hotkey with a pending emission
         PendingdHotkeyEmission::<Test>::insert(datura_old_hk_account, 10_000);
         // Setup the NEW Datura hotkey with a pending emission
         PendingdHotkeyEmission::<Test>::insert(datura_new_hk_account, 123_456_789);
+        Stake::<Test>::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::<Test>::insert(taostats_old_hk_account, 987_654);
         // Setup the new TaoStats hotkey with a pending emission
         PendingdHotkeyEmission::<Test>::insert(taostats_new_hk_account, 100_000);
         // Setup the old TaoStats hotkey with a null-key stake entry
-        Stake::<Test>::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::<Test>::insert(taostats_old_hk_account, null_account, null_stake_tao_stats);
+        let expected_taostats_new_hk_pending_emission: u64 = 987_654 + 100_000;
+
+        let total_issuance_before = SubtensorModule::get_total_issuance();
 
         // Run migration
         let first_weight = run_pending_emissions_migration_and_check(migration_name);
@@ -508,10 +523,67 @@ 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::<Test>::get(datura_new_hk_account),
-            expected_datura_new_hk_pending_emission
+            PendingdHotkeyEmission::<Test>::get(datura_old_hk_account),
+            0
+        );
+
+        assert_eq!(
+            PendingdHotkeyEmission::<Test>::get(taostats_old_hk_account),
+            0
+        );
+
+        // Check the stake entry is removed
+        assert_eq!(Stake::<Test>::get(datura_old_hk_account, null_account), 0);
+        assert_eq!(Stake::<Test>::get(taostats_old_hk_account, null_account), 0);
+
+        // 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 the SAME following the migration (no new TAO staked)
+        assert_eq!(TotalStake::<Test>::get(), expected_total_issuance);
+        // Check the total stake maps are updated following the migration (removal of old null_account stake entries)
+        assert_eq!(TotalColdkeyStake::<Test>::get(null_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::<Test>::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::<Test>::get(migration_account),
+            null_stake_total
         );
+        assert!(StakingHotkeys::<Test>::get(migration_account).contains(datura_old_hk_account));
+        assert!(StakingHotkeys::<Test>::get(migration_account).contains(taostats_old_hk_account));
     })
 }

Diff for: pallets/subtensor/tests/migration.rs

@@ -1075,10 +1075,12 @@ pub type SignedExtra = (
     frame_metadata_hash_extension::CheckMetadataHash<Runtime>,
 );
 
-type Migrations =
+type Migrations = (
     pallet_subtensor::migrations::migrate_init_total_issuance::initialise_total_issuance::Migration<
         Runtime,
-    >;
+    >,
+    pallet_subtensor::migrations::migrate_fix_pending_emission::migration::Migration<Runtime>,
+);
 
 // Unchecked extrinsic type as expected by this runtime.
 pub type UncheckedExtrinsic =