-
Notifications
You must be signed in to change notification settings - Fork 168
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
Fix Stake rate limit for target>1 #1091
Labels
bug
Something isn't working
Comments
Example: #[test]
fn test_add_stake_under_limit_resets_old_stakes() {
new_test_ext(1).execute_with(|| {
let hotkey_account_id = U256::from(561337);
let coldkey_account_id = U256::from(61337);
let netuid: u16 = 1;
let start_nonce: u64 = 0;
let tempo: u16 = 13;
let max_stakes = 5;
let stake_interval = 10;
SubtensorModule::set_stake_interval(stake_interval);
SubtensorModule::set_target_stakes_per_interval(max_stakes);
add_network(netuid, tempo, 0);
register_ok_neuron(netuid, hotkey_account_id, coldkey_account_id, start_nonce);
SubtensorModule::add_balance_to_coldkey_account(&coldkey_account_id, 60000);
// Stake one less time than the max
for _ in 0..(max_stakes - 1) {
assert_ok!(SubtensorModule::add_stake(
<<Test as Config>::RuntimeOrigin>::signed(coldkey_account_id),
hotkey_account_id,
1,
));
} // Should all succeed
assert_eq!(SubtensorModule::get_stakes_this_interval_for_coldkey_hotkey(&coldkey_account_id, &hotkey_account_id), max_stakes - 1);
// Wait one block less than the interval
step_block((stake_interval - 1) as u16);
// Stake one more time to reach the limit
assert_ok!(SubtensorModule::add_stake(
<<Test as Config>::RuntimeOrigin>::signed(coldkey_account_id),
hotkey_account_id,
1,
));
// Check the stakes are now at the limit
assert_eq!(SubtensorModule::get_stakes_this_interval_for_coldkey_hotkey(&coldkey_account_id, &hotkey_account_id), max_stakes);
// Should fail
assert_err!(SubtensorModule::add_stake(
<<Test as Config>::RuntimeOrigin>::signed(coldkey_account_id),
hotkey_account_id,
1,
), Error::<Test>::StakeRateLimitExceeded);
// Wait one block more than the interval
step_block(1); // This should reset the OLD stakes
// Check the stakes are reset
// Should be 1 because we only staked once within the interval
assert_eq!(SubtensorModule::get_stakes_this_interval_for_coldkey_hotkey(&coldkey_account_id, &hotkey_account_id), 1);
// Should succeed in this same block
assert_ok!(SubtensorModule::add_stake(
<<Test as Config>::RuntimeOrigin>::signed(coldkey_account_id),
hotkey_account_id,
1,
));
});
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Currently, we rate limit staking transactions by checking how many staking transactions were done within a set interval, and limiting if it exceeds the set target.
The way we do this is flawed. In the
get_stakes_this_interval_for_coldkey_hotkey
function, we take the value stored inTotalHotkeyColdkeyStakesThisInterval
of(num_stakes, last_stake_block)
.We reset
num_stakes
to0
if it has been more thanstake_interval
blocks sincelast_stake_block
.Otherwise we return
num_stakes
with no reset.This passes the rate limit, and then we do the transaction. We then increment
num_stakes
and setlast_stake_block
to the current block.This is fine for
stake_interval == 1
, but for anything greater, we run into an issue.Example with
stake_interval = n
wheren > 1
:n - 1
times at blockX
stake_interval - 1
blocks1
time at blockX + stake_interval - 1
Why does this fail?
In 4., we should expect the rate limit count to reset to
1
as we have only staked once in the laststake_interval
blocks. However, because of the faulty logic, we actually just increase the count and rate limit for anotherstake_interval
blocks.The text was updated successfully, but these errors were encountered: