Skip to content

Commit

Permalink
chore: pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Samuel Dare committed Jul 4, 2024
1 parent 49d19b1 commit c82f82f
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 105 deletions.
10 changes: 5 additions & 5 deletions pallets/subtensor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> =
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions pallets/subtensor/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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() {
Expand All @@ -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));
});
Expand Down
6 changes: 3 additions & 3 deletions pallets/subtensor/src/staking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
38 changes: 19 additions & 19 deletions pallets/subtensor/src/swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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));
}

Expand Down
85 changes: 12 additions & 73 deletions pallets/subtensor/tests/swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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!(
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(|| {
Expand Down

0 comments on commit c82f82f

Please sign in to comment.