Skip to content

Commit

Permalink
Merge pull request #464 from opentensor/hotfix/reapply-before-testnet…
Browse files Browse the repository at this point in the history
…-merge

Hotfix/reapply before testnet merge
  • Loading branch information
distributedstatemachine committed May 23, 2024
2 parents 8153831 + 970e3ec commit dbef23b
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 299 deletions.
2 changes: 1 addition & 1 deletion pallets/subtensor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1988,7 +1988,7 @@ where
Pallet::<T>::get_registrations_this_interval(*netuid);
let max_registrations_per_interval =
Pallet::<T>::get_target_registrations_per_interval(*netuid);
if registrations_this_interval >= max_registrations_per_interval {
if registrations_this_interval >= (max_registrations_per_interval * 3) {
// If the registration limit for the interval is exceeded, reject the transaction
return InvalidTransaction::ExhaustsResources.into();
}
Expand Down
35 changes: 17 additions & 18 deletions pallets/subtensor/src/staking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,26 +436,18 @@ impl<T: Config> Pallet<T> {
Error::<T>::UnstakeRateLimitExceeded
);

// If this is a nomination stake, check if total stake after removing will be above
// the minimum required stake.

// If coldkey is not owner of the hotkey, it's a nomination stake.
if !Self::coldkey_owns_hotkey(&coldkey, &hotkey) {
let total_stake_after_remove =
Stake::<T>::get(&hotkey, &coldkey).saturating_sub(stake_to_be_removed);

ensure!(
total_stake_after_remove >= NominatorMinRequiredStake::<T>::get(),
Error::<T>::NomStakeBelowMinimumThreshold
);
}

// We remove the balance from the hotkey.
Self::decrease_stake_on_coldkey_hotkey_account(&coldkey, &hotkey, stake_to_be_removed);

// We add the balancer to the coldkey. If the above fails we will not credit this coldkey.
// We add the balance to the coldkey. If the above fails we will not credit this coldkey.
Self::add_balance_to_coldkey_account(&coldkey, stake_to_be_removed);

// If the stake is below the minimum, we clear the nomination from storage.
// This only applies to nominator stakes.
// If the coldkey does not own the hotkey, it's a nominator stake.
let new_stake = Self::get_stake_for_coldkey_and_hotkey(&coldkey, &hotkey);
Self::clear_small_nomination_if_required(&hotkey, &coldkey, new_stake);

// Set last block for rate limiting
let block: u64 = Self::get_current_block_as_u64();
Self::set_last_tx_block(&coldkey, block);
Expand Down Expand Up @@ -679,17 +671,24 @@ impl<T: Config> Pallet<T> {
/// It also removes the stake entry for the hotkey-coldkey pairing and adjusts the TotalStake
/// and TotalIssuance by subtracting the removed stake amount.
///
/// Returns the amount of stake that was removed.
///
/// # Arguments
///
/// * `coldkey` - A reference to the AccountId of the coldkey involved in the staking.
/// * `hotkey` - A reference to the AccountId of the hotkey associated with the coldkey.
pub fn empty_stake_on_coldkey_hotkey_account(coldkey: &T::AccountId, hotkey: &T::AccountId) {
pub fn empty_stake_on_coldkey_hotkey_account(
coldkey: &T::AccountId,
hotkey: &T::AccountId,
) -> u64 {
let current_stake: u64 = Stake::<T>::get(hotkey, coldkey);
TotalColdkeyStake::<T>::mutate(coldkey, |old| *old = old.saturating_sub(current_stake));
TotalHotkeyStake::<T>::mutate(hotkey, |stake| *stake = stake.saturating_sub(current_stake));
Stake::<T>::remove(hotkey, coldkey);
TotalStake::<T>::mutate(|stake| *stake = stake.saturating_sub(current_stake));
TotalIssuance::<T>::mutate(|issuance| *issuance = issuance.saturating_sub(current_stake));

current_stake
}

/// Clears the nomination for an account, if it is a nominator account and the stake is below the minimum required threshold.
Expand All @@ -704,9 +703,9 @@ impl<T: Config> Pallet<T> {
if stake < Self::get_nominator_min_required_stake() {
// Remove the stake from the nominator account. (this is a more forceful unstake operation which )
// Actually deletes the staking account.
Self::empty_stake_on_coldkey_hotkey_account(coldkey, hotkey);
let cleared_stake = Self::empty_stake_on_coldkey_hotkey_account(coldkey, hotkey);
// Add the stake to the coldkey account.
Self::add_balance_to_coldkey_account(coldkey, stake);
Self::add_balance_to_coldkey_account(coldkey, cleared_stake);
}
}
}
Expand Down
100 changes: 74 additions & 26 deletions pallets/subtensor/tests/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,10 @@ fn test_registration_rate_limit_exceeded() {
let coldkey_account_id = U256::from(667);
let who: <Test as frame_system::Config>::AccountId = hotkey_account_id;

let max_registrants = 1;
SubtensorModule::set_target_registrations_per_interval(netuid, max_registrants);
SubtensorModule::set_registrations_this_interval(netuid, 1);
let target_registrants = 1;
let max_registrants = target_registrants * 3;
SubtensorModule::set_target_registrations_per_interval(netuid, target_registrants);
SubtensorModule::set_registrations_this_interval(netuid, max_registrants);

let (nonce, work) = SubtensorModule::create_work_for_block_number(
netuid,
Expand Down Expand Up @@ -290,18 +291,18 @@ fn test_burned_registration_under_limit() {
let netuid: u16 = 1;
let hotkey_account_id: U256 = U256::from(1);
let coldkey_account_id = U256::from(667);
let who: <Test as frame_system::Config>::AccountId = hotkey_account_id;
let block_number: u64 = 0;
let who: <Test as frame_system::Config>::AccountId = coldkey_account_id;
let burn_cost = 1000;
// Set the burn cost
SubtensorModule::set_burn(netuid, burn_cost);

let (nonce, work) = SubtensorModule::create_work_for_block_number(
netuid,
block_number,
129123813,
&hotkey_account_id,
);
add_network(netuid, 13, 0); // Add the network
// Give it some TAO to the coldkey balance; more than the burn cost
SubtensorModule::add_balance_to_coldkey_account(&coldkey_account_id, burn_cost + 10_000);

let max_registrants = 2;
SubtensorModule::set_target_registrations_per_interval(netuid, max_registrants);
let target_registrants = 2;
let max_registrants = target_registrants * 3; // Maximum is 3 times the target
SubtensorModule::set_target_registrations_per_interval(netuid, target_registrants);

let call_burned_register: pallet_subtensor::Call<Test> =
pallet_subtensor::Call::burned_register {
Expand All @@ -317,21 +318,15 @@ fn test_burned_registration_under_limit() {
extension.validate(&who, &call_burned_register.into(), &info, 10);
assert_ok!(burned_register_result);

add_network(netuid, 13, 0);
//actually call register
assert_ok!(SubtensorModule::register(
<<Test as Config>::RuntimeOrigin>::signed(hotkey_account_id),
assert_ok!(SubtensorModule::burned_register(
<<Test as Config>::RuntimeOrigin>::signed(coldkey_account_id),
netuid,
block_number,
nonce,
work,
hotkey_account_id,
coldkey_account_id
));

let current_registrants = SubtensorModule::get_registrations_this_interval(netuid);
let target_registrants = SubtensorModule::get_target_registrations_per_interval(netuid);
assert!(current_registrants <= target_registrants);
assert!(current_registrants <= max_registrants);
});
}

Expand All @@ -340,11 +335,15 @@ fn test_burned_registration_rate_limit_exceeded() {
new_test_ext(1).execute_with(|| {
let netuid: u16 = 1;
let hotkey_account_id: U256 = U256::from(1);
let who: <Test as frame_system::Config>::AccountId = hotkey_account_id;
let max_registrants = 1;
let coldkey_account_id = U256::from(667);
let who: <Test as frame_system::Config>::AccountId = coldkey_account_id;

SubtensorModule::set_target_registrations_per_interval(netuid, max_registrants);
SubtensorModule::set_registrations_this_interval(netuid, 1);
let target_registrants = 1;
let max_registrants = target_registrants * 3; // Maximum is 3 times the target

SubtensorModule::set_target_registrations_per_interval(netuid, target_registrants);
// Set the current registrations to the maximum; should not be able to register more
SubtensorModule::set_registrations_this_interval(netuid, max_registrants);

let call_burned_register: pallet_subtensor::Call<Test> =
pallet_subtensor::Call::burned_register {
Expand All @@ -369,6 +368,55 @@ fn test_burned_registration_rate_limit_exceeded() {
});
}

#[test]
fn test_burned_registration_rate_allows_burn_adjustment() {
// We need to be able to register more than the *target* registrations per interval
new_test_ext(1).execute_with(|| {
let netuid: u16 = 1;
let hotkey_account_id: U256 = U256::from(1);
let coldkey_account_id = U256::from(667);
let who: <Test as frame_system::Config>::AccountId = coldkey_account_id;

let burn_cost = 1000;
// Set the burn cost
SubtensorModule::set_burn(netuid, burn_cost);

add_network(netuid, 13, 0); // Add the network
// Give it some TAO to the coldkey balance; more than the burn cost
SubtensorModule::add_balance_to_coldkey_account(&coldkey_account_id, burn_cost + 10_000);

let target_registrants = 1; // Target is 1, but we can register more than that, up to some maximum.
SubtensorModule::set_target_registrations_per_interval(netuid, target_registrants);
// Set the current registrations to above the target; we should be able to register at least 1 more
SubtensorModule::set_registrations_this_interval(netuid, target_registrants);

// Register one more, so the current registrations are above the target
let call_burned_register: pallet_subtensor::Call<Test> =
pallet_subtensor::Call::burned_register {
netuid,
hotkey: hotkey_account_id,
};

let info: DispatchInfo =
DispatchInfoOf::<<Test as frame_system::Config>::RuntimeCall>::default();
let extension = SubtensorSignedExtension::<Test>::new();
//does not actually call register
let burned_register_result =
extension.validate(&who, &call_burned_register.into(), &info, 10);
assert_ok!(burned_register_result);

//actually call register
assert_ok!(SubtensorModule::burned_register(
<<Test as Config>::RuntimeOrigin>::signed(coldkey_account_id),
netuid,
hotkey_account_id
));

let current_registrants = SubtensorModule::get_registrations_this_interval(netuid);
assert!(current_registrants > target_registrants); // Should be able to register more than the target
});
}

#[test]
fn test_burned_registration_ok() {
new_test_ext(1).execute_with(|| {
Expand Down
57 changes: 48 additions & 9 deletions pallets/subtensor/tests/staking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2698,16 +2698,55 @@ fn test_remove_stake_below_minimum_threshold() {
stake_amount_to_remove
));

// Nomination stake cannot stake below min threshold.
assert_noop!(
SubtensorModule::remove_stake(
<<Test as Config>::RuntimeOrigin>::signed(coldkey2),
hotkey1,
stake_amount_to_remove
),
Error::<Test>::NomStakeBelowMinimumThreshold
// Nomination stake cannot unstake below min threshold,
// without unstaking all and removing the nomination.
let total_hotkey_stake_before = SubtensorModule::get_total_stake_for_hotkey(&hotkey1);
let bal_before = Balances::free_balance(coldkey2);
let staked_before = SubtensorModule::get_stake_for_coldkey_and_hotkey(&coldkey2, &hotkey1);
let total_network_stake_before = SubtensorModule::get_total_stake();
let total_issuance_before = SubtensorModule::get_total_issuance();
// check the premise of the test is correct
assert!(initial_stake - stake_amount_to_remove < minimum_threshold);
assert_ok!(SubtensorModule::remove_stake(
<<Test as Config>::RuntimeOrigin>::signed(coldkey2),
hotkey1,
stake_amount_to_remove
));

// Has no stake now
assert_eq!(
SubtensorModule::get_stake_for_coldkey_and_hotkey(&coldkey2, &hotkey1),
0
);
})
let stake_removed = staked_before; // All stake was removed
// Has the full balance
assert_eq!(Balances::free_balance(coldkey2), bal_before + stake_removed);

// Stake map entry is removed
assert_eq!(
Stake::<Test>::try_get(hotkey1, coldkey2).is_err(),
true // Entry was removed
);
// Stake tracking is updated
assert_eq!(
TotalColdkeyStake::<Test>::try_get(coldkey2).unwrap(),
0 // Did not have any stake before; Entry is NOT removed
);
assert_eq!(
TotalHotkeyStake::<Test>::try_get(hotkey1).unwrap(),
total_hotkey_stake_before - stake_removed // Stake was removed from hotkey1 tracker
);
assert_eq!(
TotalStake::<Test>::try_get().unwrap(),
total_network_stake_before - stake_removed
);

// Total issuance is the same
assert_eq!(
SubtensorModule::get_total_issuance(),
total_issuance_before // Nothing was issued
);
});
}

// Verify delegate take can be decreased
Expand Down
15 changes: 3 additions & 12 deletions runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,12 @@ mod migrations;

use codec::{Decode, Encode, MaxEncodedLen};

use migrations::{account_data_migration, init_storage_versions};
use pallet_commitments::CanCommit;
use pallet_grandpa::{
fg_primitives, AuthorityId as GrandpaId, AuthorityList as GrandpaAuthorityList,
};

use frame_support::{
pallet_prelude::{DispatchError, DispatchResult, Get},
traits::OnRuntimeUpgrade,
};
use frame_support::pallet_prelude::{DispatchError, DispatchResult, Get};
use frame_system::{EnsureNever, EnsureRoot, RawOrigin};

use pallet_registry::CanRegisterIdentity;
Expand Down Expand Up @@ -137,7 +133,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// `spec_version`, and `authoring_version` are the same between Wasm and native.
// This value is set to 100 to notify Polkadot-JS App (https://polkadot.js.org/apps) to use
// the compatible custom types.
spec_version: 149,
spec_version: 150,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down Expand Up @@ -1171,12 +1167,7 @@ pub type SignedExtra = (
pallet_commitments::CommitmentsSignedExtension<Runtime>,
);

type Migrations = (
init_storage_versions::Migration,
account_data_migration::Migration,
pallet_multisig::migrations::v1::MigrateToV1<Runtime>,
pallet_preimage::migration::v1::Migration<Runtime>,
);
type Migrations = ();

// Unchecked extrinsic type as expected by this runtime.
pub type UncheckedExtrinsic =
Expand Down
Loading

0 comments on commit dbef23b

Please sign in to comment.