From c82f82feb7900dde4e7eb61075066e1151c5d4c5 Mon Sep 17 00:00:00 2001 From: Samuel Dare Date: Thu, 4 Jul 2024 15:44:14 +0400 Subject: [PATCH] chore: pr comments --- pallets/subtensor/src/lib.rs | 10 ++-- pallets/subtensor/src/migration.rs | 10 ++-- pallets/subtensor/src/staking.rs | 6 +-- pallets/subtensor/src/swap.rs | 38 ++++++------- pallets/subtensor/tests/swap.rs | 85 +++++------------------------- 5 files changed, 44 insertions(+), 105 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 395115935..ba768f950 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -364,7 +364,7 @@ pub mod pallet { pub type Owner = StorageMap<_, Blake2_128Concat, T::AccountId, T::AccountId, ValueQuery, DefaultAccount>; #[pallet::storage] // --- MAP ( cold ) --> Vec | Returns the vector of hotkeys controlled by this coldkey. - pub type Owned = + pub type OwnedHotkeys = StorageMap<_, Blake2_128Concat, T::AccountId, Vec, ValueQuery>; #[pallet::storage] // --- MAP ( hot ) --> take | Returns the hotkey delegation take. And signals that this key is open for delegation. pub type Delegates = @@ -1207,11 +1207,11 @@ pub mod pallet { // Fill stake information. Owner::::insert(hotkey.clone(), coldkey.clone()); - // Update Owned map - let mut hotkeys = Owned::::get(coldkey); + // Update OwnedHotkeys map + let mut hotkeys = OwnedHotkeys::::get(coldkey); if !hotkeys.contains(hotkey) { hotkeys.push(hotkey.clone()); - Owned::::insert(coldkey, hotkeys); + OwnedHotkeys::::insert(coldkey, hotkeys); } TotalHotkeyStake::::insert(hotkey.clone(), stake); @@ -1336,7 +1336,7 @@ pub mod pallet { .saturating_add(migration::migrate_delete_subnet_3::()) // Doesn't check storage version. TODO: Remove after upgrade .saturating_add(migration::migration5_total_issuance::(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::()); weight diff --git a/pallets/subtensor/src/migration.rs b/pallets/subtensor/src/migration.rs index 8263e347b..cc77ac978 100644 --- a/pallets/subtensor/src/migration.rs +++ b/pallets/subtensor/src/migration.rs @@ -481,10 +481,10 @@ pub fn migrate_to_v2_fixed_total_stake() -> Weight { pub fn migrate_populate_owned() -> 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::::iter().next().is_none(); + // Check if this migration is needed (if OwnedHotkeys map is empty) + let migrate = OwnedHotkeys::::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() -> Weight { let mut longest_hotkey_vector = 0; let mut longest_coldkey: Option = None; Owner::::iter().for_each(|(hotkey, coldkey)| { - let mut hotkeys = Owned::::get(&coldkey); + let mut hotkeys = OwnedHotkeys::::get(&coldkey); if !hotkeys.contains(&hotkey) { hotkeys.push(hotkey); if longest_hotkey_vector < hotkeys.len() { @@ -502,7 +502,7 @@ pub fn migrate_populate_owned() -> Weight { } } - Owned::::insert(&coldkey, hotkeys); + OwnedHotkeys::::insert(&coldkey, hotkeys); weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 1)); }); diff --git a/pallets/subtensor/src/staking.rs b/pallets/subtensor/src/staking.rs index eac47ba21..de2a54e4f 100644 --- a/pallets/subtensor/src/staking.rs +++ b/pallets/subtensor/src/staking.rs @@ -561,11 +561,11 @@ impl Pallet { Stake::::insert(hotkey, coldkey, 0); Owner::::insert(hotkey, coldkey); - // Update Owned map - let mut hotkeys = Owned::::get(coldkey); + // Update OwnedHotkeys map + let mut hotkeys = OwnedHotkeys::::get(coldkey); if !hotkeys.contains(hotkey) { hotkeys.push(hotkey.clone()); - Owned::::insert(coldkey, hotkeys); + OwnedHotkeys::::insert(coldkey, hotkeys); } } } diff --git a/pallets/subtensor/src/swap.rs b/pallets/subtensor/src/swap.rs index d56f60897..2971fbfe2 100644 --- a/pallets/subtensor/src/swap.rs +++ b/pallets/subtensor/src/swap.rs @@ -137,17 +137,17 @@ impl Pallet { Error::::NewColdKeyIsSameWithOld ); - // Check if the new coldkey is already associated with any hotkeys - ensure!( - !Self::coldkey_has_associated_hotkeys(new_coldkey), - Error::::ColdKeyAlreadyAssociated - ); + // // Check if the new coldkey is already associated with any hotkeys + // ensure!( + // !Self::coldkey_has_associated_hotkeys(new_coldkey), + // Error::::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::::ColdKeySwapTxRateLimitExceeded - ); + // ensure!( + // !Self::exceeds_tx_rate_limit(Self::get_last_tx_block(old_coldkey), block), + // Error::::ColdKeySwapTxRateLimitExceeded + // ); // Note: we probably want to make this free let swap_cost = Self::get_coldkey_swap_cost(); @@ -224,13 +224,13 @@ impl Pallet { Owner::::remove(old_hotkey); Owner::::insert(new_hotkey, coldkey.clone()); - // Update Owned map - let mut hotkeys = Owned::::get(coldkey); + // Update OwnedHotkeys map + let mut hotkeys = OwnedHotkeys::::get(coldkey); if !hotkeys.contains(new_hotkey) { hotkeys.push(new_hotkey.clone()); } hotkeys.retain(|hk| *hk != *old_hotkey); - Owned::::insert(coldkey, hotkeys); + OwnedHotkeys::::insert(coldkey, hotkeys); weight.saturating_accrue(T::DbWeight::get().writes(2)); } @@ -545,7 +545,7 @@ impl Pallet { weight: &mut Weight, ) { // Find all hotkeys for this coldkey - let hotkeys = Owned::::get(old_coldkey); + let hotkeys = OwnedHotkeys::::get(old_coldkey); weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 0)); for hotkey in hotkeys.iter() { let stake = Stake::::get(&hotkey, old_coldkey); @@ -572,7 +572,7 @@ impl Pallet { new_coldkey: &T::AccountId, weight: &mut Weight, ) { - let hotkeys = Owned::::get(old_coldkey); + let hotkeys = OwnedHotkeys::::get(old_coldkey); weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 0)); for hotkey in hotkeys.iter() { Owner::::insert(&hotkey, new_coldkey); @@ -599,7 +599,7 @@ impl Pallet { weight: &mut Weight, ) { weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 0)); - for hotkey in Owned::::get(old_coldkey).iter() { + for hotkey in OwnedHotkeys::::get(old_coldkey).iter() { let (stake, block) = TotalHotkeyColdkeyStakesThisInterval::::get(&hotkey, old_coldkey); TotalHotkeyColdkeyStakesThisInterval::::remove(&hotkey, old_coldkey); @@ -665,10 +665,10 @@ impl Pallet { new_coldkey: &T::AccountId, weight: &mut Weight, ) { - // Update Owned map with new coldkey - let hotkeys = Owned::::get(old_coldkey); - Owned::::remove(old_coldkey); - Owned::::insert(new_coldkey, hotkeys); + // Update OwnedHotkeys map with new coldkey + let hotkeys = OwnedHotkeys::::get(old_coldkey); + OwnedHotkeys::::remove(old_coldkey); + OwnedHotkeys::::insert(new_coldkey, hotkeys); weight.saturating_accrue(T::DbWeight::get().reads_writes(0, 2)); } diff --git a/pallets/subtensor/tests/swap.rs b/pallets/subtensor/tests/swap.rs index 273a21bcf..ebdb5aed7 100644 --- a/pallets/subtensor/tests/swap.rs +++ b/pallets/subtensor/tests/swap.rs @@ -1088,8 +1088,8 @@ fn test_do_swap_coldkey_success() { assert_eq!(TotalColdkeyStake::::get(old_coldkey), stake_amount); assert_eq!(Stake::::get(hotkey, old_coldkey), stake_amount); - assert_eq!(Owned::::get(old_coldkey), vec![hotkey]); - assert!(!Owned::::contains_key(new_coldkey)); + assert_eq!(OwnedHotkeys::::get(old_coldkey), vec![hotkey]); + assert!(!OwnedHotkeys::::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::::contains_key(old_coldkey)); assert_eq!(Stake::::get(hotkey, new_coldkey), stake_amount); assert!(!Stake::::contains_key(hotkey, old_coldkey)); - assert_eq!(Owned::::get(new_coldkey), vec![hotkey]); - assert!(!Owned::::contains_key(old_coldkey)); + assert_eq!(OwnedHotkeys::::get(new_coldkey), vec![hotkey]); + assert!(!OwnedHotkeys::::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( - <::RuntimeOrigin>::signed(old_coldkey), - &old_coldkey, - &new_coldkey - ), - Error::::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::::insert(hotkey1, old_coldkey, stake_amount1); Stake::::insert(hotkey2, old_coldkey, stake_amount2); - // Populate Owned map - Owned::::insert(old_coldkey, vec![hotkey1, hotkey2]); + // Populate OwnedHotkeys map + OwnedHotkeys::::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::::insert(hotkey1, old_coldkey); Owner::::insert(hotkey2, old_coldkey); - // Initialize Owned map - Owned::::insert(old_coldkey, vec![hotkey1, hotkey2]); + // Initialize OwnedHotkeys map + OwnedHotkeys::::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::::insert(hotkey1, old_coldkey, stake1); TotalHotkeyColdkeyStakesThisInterval::::insert(hotkey2, old_coldkey, stake2); - // Populate Owned map - Owned::::insert(old_coldkey, vec![hotkey1, hotkey2]); + // Populate OwnedHotkeys map + OwnedHotkeys::::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::::insert(netuid, old_coldkey); - // Populate Owned map - Owned::::insert(old_coldkey, vec![hotkey]); + // Populate OwnedHotkeys map + OwnedHotkeys::::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( - <::RuntimeOrigin>::signed(old_coldkey), - &old_coldkey, - &new_coldkey - )); - - // Attempt second swap immediately - assert_err!( - SubtensorModule::do_swap_coldkey( - <::RuntimeOrigin>::signed(new_coldkey), - &new_coldkey, - &old_coldkey - ), - Error::::ColdKeySwapTxRateLimitExceeded - ); - }); -} - #[test] fn test_coldkey_has_associated_hotkeys() { new_test_ext(1).execute_with(|| {