chore: pr comments

Author: Samuel Dare

pallets/subtensor/src/lib.rs

@@ -364,7 +364,7 @@ pub mod pallet {
     pub type Owner<T: Config> =
         StorageMap<_, Blake2_128Concat, T::AccountId, T::AccountId, ValueQuery, DefaultAccount<T>>;
     #[pallet::storage] // --- MAP ( cold ) --> Vec<hot> | Returns the vector of hotkeys controlled by this coldkey.
-    pub type Owned<T: Config> =
+    pub type OwnedHotkeys<T: Config> =
         StorageMap<_, Blake2_128Concat, T::AccountId, Vec<T::AccountId>, ValueQuery>;
     #[pallet::storage] // --- MAP ( hot ) --> take | Returns the hotkey delegation take. And signals that this key is open for delegation.
     pub type Delegates<T: Config> =
@@ -1207,11 +1207,11 @@ pub mod pallet {
                     // Fill stake information.
                     Owner::<T>::insert(hotkey.clone(), coldkey.clone());
 
-                    // Update Owned map
-                    let mut hotkeys = Owned::<T>::get(coldkey);
+                    // Update OwnedHotkeys map
+                    let mut hotkeys = OwnedHotkeys::<T>::get(coldkey);
                     if !hotkeys.contains(hotkey) {
                         hotkeys.push(hotkey.clone());
-                        Owned::<T>::insert(coldkey, hotkeys);
+                        OwnedHotkeys::<T>::insert(coldkey, hotkeys);
                     }
 
                     TotalHotkeyStake::<T>::insert(hotkey.clone(), stake);
@@ -1336,7 +1336,7 @@ pub mod pallet {
                 .saturating_add(migration::migrate_delete_subnet_3::<T>())
                 // Doesn't check storage version. TODO: Remove after upgrade
                 .saturating_add(migration::migration5_total_issuance::<T>(false))
-                // Populate Owned map for coldkey swap. Doesn't update storage vesion.
+                // Populate OwnedHotkeys map for coldkey swap. Doesn't update storage vesion.
                 .saturating_add(migration::migrate_populate_owned::<T>());
 
             weight

pallets/subtensor/src/migration.rs

@@ -481,10 +481,10 @@ pub fn migrate_to_v2_fixed_total_stake<T: Config>() -> Weight {
 pub fn migrate_populate_owned<T: Config>() -> Weight {
     // Setup migration weight
     let mut weight = T::DbWeight::get().reads(1);
-    let migration_name = "Populate Owned map";
+    let migration_name = "Populate OwnedHotkeys map";
 
-    // Check if this migration is needed (if Owned map is empty)
-    let migrate = Owned::<T>::iter().next().is_none();
+    // Check if this migration is needed (if OwnedHotkeys map is empty)
+    let migrate = OwnedHotkeys::<T>::iter().next().is_none();
 
     // Only runs if we haven't already updated version past above new_storage_version.
     if migrate {
@@ -493,7 +493,7 @@ pub fn migrate_populate_owned<T: Config>() -> Weight {
         let mut longest_hotkey_vector = 0;
         let mut longest_coldkey: Option<T::AccountId> = None;
         Owner::<T>::iter().for_each(|(hotkey, coldkey)| {
-            let mut hotkeys = Owned::<T>::get(&coldkey);
+            let mut hotkeys = OwnedHotkeys::<T>::get(&coldkey);
             if !hotkeys.contains(&hotkey) {
                 hotkeys.push(hotkey);
                 if longest_hotkey_vector < hotkeys.len() {
@@ -502,7 +502,7 @@ pub fn migrate_populate_owned<T: Config>() -> Weight {
                 }
             }
 
-            Owned::<T>::insert(&coldkey, hotkeys);
+            OwnedHotkeys::<T>::insert(&coldkey, hotkeys);
 
             weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 1));
         });

pallets/subtensor/src/staking.rs

@@ -561,11 +561,11 @@ impl<T: Config> Pallet<T> {
             Stake::<T>::insert(hotkey, coldkey, 0);
             Owner::<T>::insert(hotkey, coldkey);
 
-            // Update Owned map
-            let mut hotkeys = Owned::<T>::get(coldkey);
+            // Update OwnedHotkeys map
+            let mut hotkeys = OwnedHotkeys::<T>::get(coldkey);
             if !hotkeys.contains(hotkey) {
                 hotkeys.push(hotkey.clone());
-                Owned::<T>::insert(coldkey, hotkeys);
+                OwnedHotkeys::<T>::insert(coldkey, hotkeys);
             }
         }
     }

pallets/subtensor/src/swap.rs

@@ -137,17 +137,17 @@ impl<T: Config> Pallet<T> {
             Error::<T>::NewColdKeyIsSameWithOld
         );
 
-        // Check if the new coldkey is already associated with any hotkeys
-        ensure!(
-            !Self::coldkey_has_associated_hotkeys(new_coldkey),
-            Error::<T>::ColdKeyAlreadyAssociated
-        );
+        // // Check if the new coldkey is already associated with any hotkeys
+        // ensure!(
+        //     !Self::coldkey_has_associated_hotkeys(new_coldkey),
+        //     Error::<T>::ColdKeyAlreadyAssociated
+        // );
 
         let block: u64 = Self::get_current_block_as_u64();
-        ensure!(
-            !Self::exceeds_tx_rate_limit(Self::get_last_tx_block(old_coldkey), block),
-            Error::<T>::ColdKeySwapTxRateLimitExceeded
-        );
+        // ensure!(
+        //     !Self::exceeds_tx_rate_limit(Self::get_last_tx_block(old_coldkey), block),
+        //     Error::<T>::ColdKeySwapTxRateLimitExceeded
+        // );
 
         // Note: we probably want to make this free
         let swap_cost = Self::get_coldkey_swap_cost();
@@ -224,13 +224,13 @@ impl<T: Config> Pallet<T> {
         Owner::<T>::remove(old_hotkey);
         Owner::<T>::insert(new_hotkey, coldkey.clone());
 
-        // Update Owned map
-        let mut hotkeys = Owned::<T>::get(coldkey);
+        // Update OwnedHotkeys map
+        let mut hotkeys = OwnedHotkeys::<T>::get(coldkey);
         if !hotkeys.contains(new_hotkey) {
             hotkeys.push(new_hotkey.clone());
         }
         hotkeys.retain(|hk| *hk != *old_hotkey);
-        Owned::<T>::insert(coldkey, hotkeys);
+        OwnedHotkeys::<T>::insert(coldkey, hotkeys);
 
         weight.saturating_accrue(T::DbWeight::get().writes(2));
     }
@@ -545,7 +545,7 @@ impl<T: Config> Pallet<T> {
         weight: &mut Weight,
     ) {
         // Find all hotkeys for this coldkey
-        let hotkeys = Owned::<T>::get(old_coldkey);
+        let hotkeys = OwnedHotkeys::<T>::get(old_coldkey);
         weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 0));
         for hotkey in hotkeys.iter() {
             let stake = Stake::<T>::get(&hotkey, old_coldkey);
@@ -572,7 +572,7 @@ impl<T: Config> Pallet<T> {
         new_coldkey: &T::AccountId,
         weight: &mut Weight,
     ) {
-        let hotkeys = Owned::<T>::get(old_coldkey);
+        let hotkeys = OwnedHotkeys::<T>::get(old_coldkey);
         weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 0));
         for hotkey in hotkeys.iter() {
             Owner::<T>::insert(&hotkey, new_coldkey);
@@ -599,7 +599,7 @@ impl<T: Config> Pallet<T> {
         weight: &mut Weight,
     ) {
         weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 0));
-        for hotkey in Owned::<T>::get(old_coldkey).iter() {
+        for hotkey in OwnedHotkeys::<T>::get(old_coldkey).iter() {
             let (stake, block) =
                 TotalHotkeyColdkeyStakesThisInterval::<T>::get(&hotkey, old_coldkey);
             TotalHotkeyColdkeyStakesThisInterval::<T>::remove(&hotkey, old_coldkey);
@@ -665,10 +665,10 @@ impl<T: Config> Pallet<T> {
         new_coldkey: &T::AccountId,
         weight: &mut Weight,
     ) {
-        // Update Owned map with new coldkey
-        let hotkeys = Owned::<T>::get(old_coldkey);
-        Owned::<T>::remove(old_coldkey);
-        Owned::<T>::insert(new_coldkey, hotkeys);
+        // Update OwnedHotkeys map with new coldkey
+        let hotkeys = OwnedHotkeys::<T>::get(old_coldkey);
+        OwnedHotkeys::<T>::remove(old_coldkey);
+        OwnedHotkeys::<T>::insert(new_coldkey, hotkeys);
         weight.saturating_accrue(T::DbWeight::get().reads_writes(0, 2));
     }
 

pallets/subtensor/tests/swap.rs

@@ -1088,8 +1088,8 @@ fn test_do_swap_coldkey_success() {
         assert_eq!(TotalColdkeyStake::<Test>::get(old_coldkey), stake_amount);
         assert_eq!(Stake::<Test>::get(hotkey, old_coldkey), stake_amount);
 
-        assert_eq!(Owned::<Test>::get(old_coldkey), vec![hotkey]);
-        assert!(!Owned::<Test>::contains_key(new_coldkey));
+        assert_eq!(OwnedHotkeys::<Test>::get(old_coldkey), vec![hotkey]);
+        assert!(!OwnedHotkeys::<Test>::contains_key(new_coldkey));
 
         // Get coldkey free balance before swap
         let balance = SubtensorModule::get_coldkey_balance(&old_coldkey);
@@ -1108,8 +1108,8 @@ fn test_do_swap_coldkey_success() {
         assert!(!TotalColdkeyStake::<Test>::contains_key(old_coldkey));
         assert_eq!(Stake::<Test>::get(hotkey, new_coldkey), stake_amount);
         assert!(!Stake::<Test>::contains_key(hotkey, old_coldkey));
-        assert_eq!(Owned::<Test>::get(new_coldkey), vec![hotkey]);
-        assert!(!Owned::<Test>::contains_key(old_coldkey));
+        assert_eq!(OwnedHotkeys::<Test>::get(new_coldkey), vec![hotkey]);
+        assert!(!OwnedHotkeys::<Test>::contains_key(old_coldkey));
 
         // Verify balance transfer
         assert_eq!(
@@ -1168,34 +1168,6 @@ fn test_do_swap_coldkey_same_keys() {
     });
 }
 
-#[test]
-fn test_do_swap_coldkey_new_key_already_associated() {
-    new_test_ext(1).execute_with(|| {
-        let old_coldkey = U256::from(1);
-        let new_coldkey = U256::from(2);
-        let hotkey1 = U256::from(3);
-        let hotkey2 = U256::from(4);
-        let netuid = 1u16;
-        let swap_cost = SubtensorModule::get_coldkey_swap_cost();
-
-        // Setup initial state
-        add_network(netuid, 13, 0);
-        register_ok_neuron(netuid, hotkey1, old_coldkey, 0);
-        register_ok_neuron(netuid, hotkey2, new_coldkey, 0);
-        SubtensorModule::add_balance_to_coldkey_account(&old_coldkey, swap_cost);
-
-        // Attempt the swap
-        assert_err!(
-            SubtensorModule::do_swap_coldkey(
-                <<Test as Config>::RuntimeOrigin>::signed(old_coldkey),
-                &old_coldkey,
-                &new_coldkey
-            ),
-            Error::<Test>::ColdKeyAlreadyAssociated
-        );
-    });
-}
-
 #[test]
 fn test_swap_total_coldkey_stake() {
     new_test_ext(1).execute_with(|| {
@@ -1235,8 +1207,8 @@ fn test_swap_stake_for_coldkey() {
         Stake::<Test>::insert(hotkey1, old_coldkey, stake_amount1);
         Stake::<Test>::insert(hotkey2, old_coldkey, stake_amount2);
 
-        // Populate Owned map
-        Owned::<Test>::insert(old_coldkey, vec![hotkey1, hotkey2]);
+        // Populate OwnedHotkeys map
+        OwnedHotkeys::<Test>::insert(old_coldkey, vec![hotkey1, hotkey2]);
 
         // Perform the swap
         SubtensorModule::swap_stake_for_coldkey(&old_coldkey, &new_coldkey, &mut weight);
@@ -1266,8 +1238,8 @@ fn test_swap_owner_for_coldkey() {
         Owner::<Test>::insert(hotkey1, old_coldkey);
         Owner::<Test>::insert(hotkey2, old_coldkey);
 
-        // Initialize Owned map
-        Owned::<Test>::insert(old_coldkey, vec![hotkey1, hotkey2]);
+        // Initialize OwnedHotkeys map
+        OwnedHotkeys::<Test>::insert(old_coldkey, vec![hotkey1, hotkey2]);
 
         // Perform the swap
         SubtensorModule::swap_owner_for_coldkey(&old_coldkey, &new_coldkey, &mut weight);
@@ -1297,8 +1269,8 @@ fn test_swap_total_hotkey_coldkey_stakes_this_interval_for_coldkey() {
         TotalHotkeyColdkeyStakesThisInterval::<Test>::insert(hotkey1, old_coldkey, stake1);
         TotalHotkeyColdkeyStakesThisInterval::<Test>::insert(hotkey2, old_coldkey, stake2);
 
-        // Populate Owned map
-        Owned::<Test>::insert(old_coldkey, vec![hotkey1, hotkey2]);
+        // Populate OwnedHotkeys map
+        OwnedHotkeys::<Test>::insert(old_coldkey, vec![hotkey1, hotkey2]);
 
         // Perform the swap
         SubtensorModule::swap_total_hotkey_coldkey_stakes_this_interval_for_coldkey(
@@ -1380,8 +1352,8 @@ fn test_do_swap_coldkey_with_subnet_ownership() {
         SubtensorModule::add_balance_to_coldkey_account(&old_coldkey, stake_amount + swap_cost);
         SubnetOwner::<Test>::insert(netuid, old_coldkey);
 
-        // Populate Owned map
-        Owned::<Test>::insert(old_coldkey, vec![hotkey]);
+        // Populate OwnedHotkeys map
+        OwnedHotkeys::<Test>::insert(old_coldkey, vec![hotkey]);
 
         // Perform the swap
         assert_ok!(SubtensorModule::do_swap_coldkey(
@@ -1395,39 +1367,6 @@ fn test_do_swap_coldkey_with_subnet_ownership() {
     });
 }
 
-#[test]
-fn test_do_swap_coldkey_tx_rate_limit() {
-    new_test_ext(1).execute_with(|| {
-        let old_coldkey = U256::from(1);
-        let new_coldkey = U256::from(2);
-        let swap_cost = SubtensorModule::get_coldkey_swap_cost();
-
-        // Set non-zero tx rate limit
-        SubtensorModule::set_tx_rate_limit(1);
-
-        // Setup initial state
-        SubtensorModule::add_balance_to_coldkey_account(&old_coldkey, swap_cost * 2);
-        SubtensorModule::add_balance_to_coldkey_account(&new_coldkey, swap_cost * 2);
-
-        // Perform first swap
-        assert_ok!(SubtensorModule::do_swap_coldkey(
-            <<Test as Config>::RuntimeOrigin>::signed(old_coldkey),
-            &old_coldkey,
-            &new_coldkey
-        ));
-
-        // Attempt second swap immediately
-        assert_err!(
-            SubtensorModule::do_swap_coldkey(
-                <<Test as Config>::RuntimeOrigin>::signed(new_coldkey),
-                &new_coldkey,
-                &old_coldkey
-            ),
-            Error::<Test>::ColdKeySwapTxRateLimitExceeded
-        );
-    });
-}
-
 #[test]
 fn test_coldkey_has_associated_hotkeys() {
     new_test_ext(1).execute_with(|| {