Merge pull request #1109 from opentensor/fix/pending-childkeys

Fix/pending childkeys

Author: sam0x17

pallets/subtensor/src/coinbase/run_coinbase.rs

@@ -65,7 +65,7 @@ impl<T: Config> Pallet<T> {
         // --- 3. Drain the subnet block emission and accumulate it as subnet emission, which increases until the tempo is reached in #4.
         // subnet_blockwise_emission -> subnet_pending_emission
         for netuid in subnets.clone().iter() {
-            if *netuid == 0 || !Self::is_registration_allowed(*netuid) {
+            if *netuid == 0 || !Self::get_network_registration_allowed(*netuid) {
                 continue;
             }
             // --- 3.1 Get the network's block-wise emission amount.
@@ -105,7 +105,10 @@ impl<T: Config> Pallet<T> {
                     };
                 }
 
-                // --- 4.3 Drain the subnet emission.
+                // 4.3 Apply pending childkeys of this subnet for the next epoch
+                Self::do_set_pending_children(*netuid);
+
+                // --- 4.4 Drain the subnet emission.
                 let mut subnet_emission: u64 = PendingEmission::<T>::get(*netuid);
                 PendingEmission::<T>::insert(*netuid, 0);
                 log::debug!(
@@ -114,39 +117,39 @@ impl<T: Config> Pallet<T> {
                     subnet_emission
                 );
 
-                // --- 4.4 Set last step counter.
+                // --- 4.5 Set last step counter.
                 Self::set_blocks_since_last_step(*netuid, 0);
                 Self::set_last_mechanism_step_block(*netuid, current_block);
 
-                if *netuid == 0 || !Self::is_registration_allowed(*netuid) {
+                if *netuid == 0 || !Self::get_network_registration_allowed(*netuid) {
                     // Skip netuid 0 payouts
                     continue;
                 }
 
-                // --- 4.5 Distribute owner take.
+                // --- 4.6 Distribute owner take.
                 if SubnetOwner::<T>::contains_key(netuid) {
                     // Does the subnet have an owner?
 
-                    // --- 4.5.1 Compute the subnet owner cut.
+                    // --- 4.6.1 Compute the subnet owner cut.
                     let owner_cut: I96F32 = I96F32::from_num(subnet_emission).saturating_mul(
                         I96F32::from_num(Self::get_subnet_owner_cut())
                             .saturating_div(I96F32::from_num(u16::MAX)),
                     );
 
-                    // --- 4.5.2 Remove the cut from the subnet emission
+                    // --- 4.6.2 Remove the cut from the subnet emission
                     subnet_emission = subnet_emission.saturating_sub(owner_cut.to_num::<u64>());
 
-                    // --- 4.5.3 Add the cut to the balance of the owner
+                    // --- 4.6.3 Add the cut to the balance of the owner
                     Self::add_balance_to_coldkey_account(
                         &Self::get_subnet_owner(*netuid),
                         owner_cut.to_num::<u64>(),
                     );
 
-                    // --- 4.5.4 Increase total issuance on the chain.
+                    // --- 4.6.4 Increase total issuance on the chain.
                     Self::coinbase(owner_cut.to_num::<u64>());
                 }
 
-                // 4.6 Pass emission through epoch() --> hotkey emission.
+                // 4.7 Pass emission through epoch() --> hotkey emission.
                 let hotkey_emission: Vec<(T::AccountId, u64, u64)> =
                     Self::epoch(*netuid, subnet_emission);
                 log::debug!(
@@ -155,7 +158,7 @@ impl<T: Config> Pallet<T> {
                     hotkey_emission
                 );
 
-                // 4.7 Accumulate the tuples on hotkeys:
+                // 4.8 Accumulate the tuples on hotkeys:
                 for (hotkey, mining_emission, validator_emission) in hotkey_emission {
                     // 4.8 Accumulate the emission on the hotkey and parent hotkeys.
                     Self::accumulate_hotkey_emission(
@@ -166,9 +169,6 @@ impl<T: Config> Pallet<T> {
                     );
                     log::debug!("Accumulated emissions on hotkey {:?} for netuid {:?}: mining {:?}, validator {:?}", hotkey, *netuid, mining_emission, validator_emission);
                 }
-
-                // 4.5 Apply pending childkeys of this subnet for the next epoch
-                Self::do_set_pending_children(*netuid);
             } else {
                 // No epoch, increase blocks since last step and continue
                 Self::set_blocks_since_last_step(

pallets/subtensor/src/staking/set_children.rs

@@ -110,10 +110,12 @@ impl<T: Config> Pallet<T> {
         }
 
         // Check that the parent key has at least the minimum own stake
+        // if children vector is not empty
         // (checking with check_weights_min_stake wouldn't work because it considers
         // grandparent stake in this case)
         ensure!(
-            Self::get_total_stake_for_hotkey(&hotkey) >= StakeThreshold::<T>::get(),
+            children.is_empty()
+                || Self::get_total_stake_for_hotkey(&hotkey) >= StakeThreshold::<T>::get(),
             Error::<T>::NotEnoughStakeToSetChildkeys
         );
 

pallets/subtensor/src/tests/children.rs

@@ -3747,3 +3747,108 @@ fn test_do_set_child_cooldown_period() {
         assert_eq!(children_after, vec![(proportion, child)]);
     });
 }
+
+// Test that revoking childkeys does not require minimum stake
+#[test]
+fn test_revoke_child_no_min_stake_check() {
+    new_test_ext(1).execute_with(|| {
+        let coldkey = U256::from(1);
+        let parent = U256::from(2);
+        let child = U256::from(3);
+        let netuid: u16 = 1;
+        let proportion: u64 = 1000;
+
+        // Add network and register hotkey
+        add_network(netuid, 13, 0);
+        register_ok_neuron(netuid, parent, coldkey, 0);
+
+        // Set minimum stake for setting children
+        let parent_total_stake_original = TotalHotkeyStake::<Test>::get(parent);
+        StakeThreshold::<Test>::put(1_000_000_000_000);
+        TotalHotkeyStake::<Test>::insert(parent, StakeThreshold::<Test>::get());
+
+        // Schedule parent-child relationship
+        assert_ok!(SubtensorModule::do_schedule_children(
+            RuntimeOrigin::signed(coldkey),
+            parent,
+            netuid,
+            vec![(proportion, child)],
+        ));
+
+        // Ensure the childkeys are not yet applied
+        let children_before = SubtensorModule::get_children(&parent, netuid);
+        assert_eq!(children_before, vec![]);
+
+        wait_and_set_pending_children(netuid);
+        TotalHotkeyStake::<Test>::insert(parent, parent_total_stake_original);
+
+        // Ensure the childkeys are applied
+        let children_after = SubtensorModule::get_children(&parent, netuid);
+        assert_eq!(children_after, vec![(proportion, child)]);
+
+        // Reduce the stake below required threshold
+        TotalHotkeyStake::<Test>::insert(parent, StakeThreshold::<Test>::get() - 1);
+
+        // Bypass tx rate limit
+        SubtensorModule::set_last_transaction_block_on_subnet(
+            &parent,
+            netuid,
+            &TransactionType::SetChildren,
+            0,
+        );
+
+        // Schedule parent-child relationship revokation
+        assert_ok!(SubtensorModule::do_schedule_children(
+            RuntimeOrigin::signed(coldkey),
+            parent,
+            netuid,
+            vec![],
+        ));
+
+        wait_and_set_pending_children(netuid);
+        TotalHotkeyStake::<Test>::insert(parent, parent_total_stake_original);
+
+        // Ensure the childkeys are revoked
+        let children_after = SubtensorModule::get_children(&parent, netuid);
+        assert_eq!(children_after, vec![]);
+    });
+}
+
+// Test that setting childkeys works even if subnet registration is disabled
+#[test]
+fn test_do_set_child_registration_disabled() {
+    new_test_ext(1).execute_with(|| {
+        let coldkey = U256::from(1);
+        let parent = U256::from(2);
+        let child = U256::from(3);
+        let netuid: u16 = 1;
+        let proportion: u64 = 1000;
+
+        // Add network and register hotkey
+        add_network(netuid, 13, 0);
+        register_ok_neuron(netuid, parent, coldkey, 0);
+
+        // Set minimum stake for setting children
+        let parent_total_stake_original = TotalHotkeyStake::<Test>::get(parent);
+        StakeThreshold::<Test>::put(1_000_000_000_000);
+        TotalHotkeyStake::<Test>::insert(parent, StakeThreshold::<Test>::get());
+
+        // Disable subnet registrations
+        NetworkRegistrationAllowed::<Test>::insert(netuid, false);
+
+        // Schedule parent-child relationship
+        assert_ok!(SubtensorModule::do_schedule_children(
+            RuntimeOrigin::signed(coldkey),
+            parent,
+            netuid,
+            vec![(proportion, child)],
+        ));
+
+        wait_and_set_pending_children(netuid);
+        TotalHotkeyStake::<Test>::insert(parent, parent_total_stake_original);
+
+        // Ensure the childkeys are applied
+        let children_after = SubtensorModule::get_children(&parent, netuid);
+        assert_eq!(children_after, vec![(proportion, child)]);
+    });
+}