Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix/reapply before testnet merge #464

Merged
merged 13 commits into from
May 23, 2024
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
2 changes: 1 addition & 1 deletion pallets/subtensor/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ parameter_types! {
pub const InitialAdjustmentInterval: u16 = 100;
pub const InitialAdjustmentAlpha: u64 = 0; // no weight to previous value.
pub const InitialMaxRegistrationsPerBlock: u16 = 3;
pub const InitialTargetRegistrationsPerInterval: u16 = 2;
pub const InitialTargetRegistrationsPerInterval: u16 = 3;
camfairchild marked this conversation as resolved.
Show resolved Hide resolved
pub const InitialPruningScore : u16 = u16::MAX;
pub const InitialRegistrationRequirement: u16 = u16::MAX; // Top 100%
pub const InitialMinDifficulty: u64 = 1;
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
2 changes: 1 addition & 1 deletion runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,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
Loading