Merge pull request #1755 from opentensor/fix/move-reg-check

move guard check for *any* sn reg

Author: sam0x17

pallets/subtensor/src/swap/swap_hotkey.rs

@@ -55,35 +55,36 @@ impl<T: Config> Pallet<T> {
 
         weight.saturating_accrue(T::DbWeight::get().reads(2));
 
-        // 7. Swap LastTxBlock
+        // 7. Ensure the new hotkey is not already registered on any network
+        ensure!(
+            !Self::is_hotkey_registered_on_any_network(new_hotkey),
+            Error::<T>::HotKeyAlreadyRegisteredInSubNet
+        );
+
+        // 8. Swap LastTxBlock
         // LastTxBlock( hotkey ) --> u64 -- the last transaction block for the hotkey.
         let last_tx_block: u64 = LastTxBlock::<T>::get(old_hotkey);
         LastTxBlock::<T>::insert(new_hotkey, last_tx_block);
         weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));
 
-        // 8. Swap LastTxBlockDelegateTake
+        // 9. Swap LastTxBlockDelegateTake
         // LastTxBlockDelegateTake( hotkey ) --> u64 -- the last transaction block for the hotkey delegate take.
         let last_tx_block_delegate_take: u64 = LastTxBlockDelegateTake::<T>::get(old_hotkey);
         LastTxBlockDelegateTake::<T>::insert(new_hotkey, last_tx_block_delegate_take);
         weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));
 
-        // 9. Swap LastTxBlockChildKeyTake
+        // 10. Swap LastTxBlockChildKeyTake
         // LastTxBlockChildKeyTake( hotkey ) --> u64 -- the last transaction block for the hotkey child key take.
         let last_tx_block_child_key_take: u64 = LastTxBlockChildKeyTake::<T>::get(old_hotkey);
         LastTxBlockChildKeyTake::<T>::insert(new_hotkey, last_tx_block_child_key_take);
         weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));
 
-        // 10. fork for swap hotkey on a specific subnet case after do the common check
+        // 11. fork for swap hotkey on a specific subnet case after do the common check
         if let Some(netuid) = netuid {
             return Self::swap_hotkey_on_subnet(&coldkey, old_hotkey, new_hotkey, netuid, weight);
         };
 
-        // Start to do everything for swap hotkey on all subnets case
-        // 11. Ensure the new hotkey is not already registered on any network
-        ensure!(
-            !Self::is_hotkey_registered_on_any_network(new_hotkey),
-            Error::<T>::HotKeyAlreadyRegisteredInSubNet
-        );
+        // ==== Start to do everything for swap hotkey on all subnets case ====
 
         // 12. Get the cost for swapping the key
         let swap_cost = Self::get_key_swap_cost();

pallets/subtensor/src/tests/swap_hotkey_with_subnet.rs

@@ -497,7 +497,8 @@ fn test_swap_hotkey_with_multiple_subnets() {
     new_test_ext(1).execute_with(|| {
         let old_hotkey = U256::from(1);
         let new_hotkey = U256::from(2);
-        let coldkey = U256::from(3);
+        let new_hotkey_2 = U256::from(3);
+        let coldkey = U256::from(4);
 
         SubtensorModule::add_balance_to_coldkey_account(&coldkey, u64::MAX);
 
@@ -519,12 +520,12 @@ fn test_swap_hotkey_with_multiple_subnets() {
         assert_ok!(SubtensorModule::do_swap_hotkey(
             RuntimeOrigin::signed(coldkey),
             &old_hotkey,
-            &new_hotkey,
+            &new_hotkey_2,
             Some(netuid2)
         ));
 
         assert!(IsNetworkMember::<Test>::get(new_hotkey, netuid1));
-        assert!(IsNetworkMember::<Test>::get(new_hotkey, netuid2));
+        assert!(IsNetworkMember::<Test>::get(new_hotkey_2, netuid2));
         assert!(!IsNetworkMember::<Test>::get(old_hotkey, netuid1));
         assert!(!IsNetworkMember::<Test>::get(old_hotkey, netuid2));
     });
@@ -628,8 +629,9 @@ fn test_swap_hotkey_with_multiple_coldkeys_and_subnets() {
     new_test_ext(1).execute_with(|| {
         let old_hotkey = U256::from(1);
         let new_hotkey = U256::from(2);
-        let coldkey1 = U256::from(3);
-        let coldkey2 = U256::from(4);
+        let new_hotkey_2 = U256::from(3);
+        let coldkey1 = U256::from(4);
+        let coldkey2 = U256::from(5);
         let netuid1 = 1;
         let netuid2 = 2;
         let stake = DefaultMinStake::<Test>::get() * 10;
@@ -687,7 +689,7 @@ fn test_swap_hotkey_with_multiple_coldkeys_and_subnets() {
         assert_ok!(SubtensorModule::do_swap_hotkey(
             RuntimeOrigin::signed(coldkey1),
             &old_hotkey,
-            &new_hotkey,
+            &new_hotkey_2,
             Some(netuid2)
         ));
 
@@ -697,6 +699,11 @@ fn test_swap_hotkey_with_multiple_coldkeys_and_subnets() {
             coldkey1
         );
         assert!(!SubtensorModule::get_owned_hotkeys(&coldkey2).contains(&new_hotkey));
+        assert_eq!(
+            SubtensorModule::get_owning_coldkey_for_hotkey(&new_hotkey_2),
+            coldkey1
+        );
+        assert!(!SubtensorModule::get_owned_hotkeys(&coldkey2).contains(&new_hotkey_2));
 
         // Check stake transfer
         assert_eq!(
@@ -709,7 +716,7 @@ fn test_swap_hotkey_with_multiple_coldkeys_and_subnets() {
         );
         assert_eq!(
             SubtensorModule::get_stake_for_hotkey_and_coldkey_on_subnet(
-                &new_hotkey,
+                &new_hotkey_2,
                 &coldkey2,
                 netuid2
             ),
@@ -739,7 +746,7 @@ fn test_swap_hotkey_with_multiple_coldkeys_and_subnets() {
         ));
         assert!(SubtensorModule::is_hotkey_registered_on_network(
             netuid2,
-            &new_hotkey
+            &new_hotkey_2
         ));
         assert!(!SubtensorModule::is_hotkey_registered_on_network(
             netuid1,
@@ -752,7 +759,8 @@ fn test_swap_hotkey_with_multiple_coldkeys_and_subnets() {
 
         // Check total stake transfer
         assert_eq!(
-            SubtensorModule::get_total_stake_for_hotkey(&new_hotkey),
+            SubtensorModule::get_total_stake_for_hotkey(&new_hotkey)
+                + SubtensorModule::get_total_stake_for_hotkey(&new_hotkey_2),
             total_hk_stake
         );
         assert_eq!(SubtensorModule::get_total_stake_for_hotkey(&old_hotkey), 0);
@@ -1143,7 +1151,8 @@ fn test_swap_multiple_subnets() {
     new_test_ext(1).execute_with(|| {
         let old_hotkey = U256::from(1);
         let new_hotkey = U256::from(2);
-        let coldkey = U256::from(3);
+        let new_hotkey_2 = U256::from(3);
+        let coldkey = U256::from(4);
         let netuid1 = add_dynamic_network(&old_hotkey, &coldkey);
         let netuid2 = add_dynamic_network(&old_hotkey, &coldkey);
 
@@ -1169,13 +1178,13 @@ fn test_swap_multiple_subnets() {
         assert_ok!(SubtensorModule::do_swap_hotkey(
             RuntimeOrigin::signed(coldkey),
             &old_hotkey,
-            &new_hotkey,
+            &new_hotkey_2,
             Some(netuid2)
         ),);
 
         // Verify the swap for both subnets
         assert_eq!(ChildKeys::<Test>::get(new_hotkey, netuid1), children1);
-        assert_eq!(ChildKeys::<Test>::get(new_hotkey, netuid2), children2);
+        assert_eq!(ChildKeys::<Test>::get(new_hotkey_2, netuid2), children2);
         assert!(ChildKeys::<Test>::get(old_hotkey, netuid1).is_empty());
         assert!(ChildKeys::<Test>::get(old_hotkey, netuid2).is_empty());
     });
@@ -1514,3 +1523,37 @@ fn test_swap_owner_check_swap_record_clean_up() {
         ));
     });
 }
+
+// SKIP_WASM_BUILD=1 RUST_LOG=debug cargo test --test swap_hotkey_with_subnet -- test_swap_hotkey_error_cases --exact --nocapture
+#[test]
+fn test_swap_hotkey_registered_on_other_subnet() {
+    new_test_ext(1).execute_with(|| {
+        let old_hotkey = U256::from(1);
+        let new_hotkey = U256::from(2);
+        let coldkey = U256::from(3);
+        let wrong_coldkey = U256::from(4);
+        let netuid = add_dynamic_network(&old_hotkey, &coldkey);
+        let other_netuid = add_dynamic_network(&old_hotkey, &coldkey);
+
+        // Set up initial state
+        Owner::<Test>::insert(old_hotkey, coldkey);
+        TotalNetworks::<Test>::put(1);
+        LastTxBlock::<Test>::insert(coldkey, 0);
+
+        let initial_balance = SubtensorModule::get_key_swap_cost() + 1000;
+        SubtensorModule::add_balance_to_coldkey_account(&coldkey, initial_balance);
+
+        // Test new hotkey already registered on other subnet
+        IsNetworkMember::<Test>::insert(new_hotkey, other_netuid, true);
+        System::set_block_number(System::block_number() + HotkeySwapOnSubnetInterval::get());
+        assert_noop!(
+            SubtensorModule::do_swap_hotkey(
+                RuntimeOrigin::signed(coldkey),
+                &old_hotkey,
+                &new_hotkey,
+                Some(netuid)
+            ),
+            Error::<Test>::HotKeyAlreadyRegisteredInSubNet
+        );
+    });
+}