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] Fix set children rate limit #1037

Merged
merged 11 commits into from
Nov 29, 2024
9 changes: 4 additions & 5 deletions pallets/subtensor/src/utils/rate_limiting.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use super::*;
use sp_core::Get;

/// Enum representing different types of transactions
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -35,9 +34,9 @@ impl<T: Config> Pallet<T> {
// ==== Rate Limiting =====
// ========================
/// Get the rate limit for a specific transaction type
pub fn get_rate_limit(tx_type: &TransactionType) -> u64 {
pub fn get_rate_limit(tx_type: &TransactionType, _netuid: u16) -> u64 {
match tx_type {
TransactionType::SetChildren => (DefaultTempo::<T>::get().saturating_mul(2)).into(), // Cannot set children twice within the default tempo period.
TransactionType::SetChildren => 7200, // Cannot set children twice within a day
TransactionType::SetChildkeyTake => TxChildkeyTakeRateLimit::<T>::get(),
TransactionType::Unknown => 0, // Default to no limit for unknown types (no limit)
}
Expand All @@ -50,7 +49,7 @@ impl<T: Config> Pallet<T> {
netuid: u16,
) -> bool {
let block: u64 = Self::get_current_block_as_u64();
let limit: u64 = Self::get_rate_limit(tx_type);
let limit: u64 = Self::get_rate_limit(tx_type, netuid);
let last_block: u64 = Self::get_last_transaction_block(hotkey, netuid, tx_type);

// Allow the first transaction (when last_block is 0) or if the rate limit has passed
Expand All @@ -61,7 +60,7 @@ impl<T: Config> Pallet<T> {
pub fn passes_rate_limit_globally(tx_type: &TransactionType, hotkey: &T::AccountId) -> bool {
let netuid: u16 = u16::MAX;
let block: u64 = Self::get_current_block_as_u64();
let limit: u64 = Self::get_rate_limit(tx_type);
let limit: u64 = Self::get_rate_limit(tx_type, 0);
let last_block: u64 = Self::get_last_transaction_block(hotkey, netuid, tx_type);
block.saturating_sub(last_block) >= limit
}
Expand Down
103 changes: 102 additions & 1 deletion pallets/subtensor/tests/children.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ fn test_do_set_child_singular_old_children_cleanup() {
vec![(proportion, old_child)]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Set new child
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -260,6 +262,8 @@ fn test_do_set_child_singular_proportion_edge_cases() {
let children = SubtensorModule::get_children(&hotkey, netuid);
assert_eq!(children, vec![(min_proportion, child)]);

step_rate_limit(&TransactionType::SetChildren, netuid);

// Set child with maximum proportion
let max_proportion: u64 = u64::MAX;
assert_ok!(SubtensorModule::do_set_children(
Expand Down Expand Up @@ -306,6 +310,8 @@ fn test_do_set_child_singular_multiple_children() {
vec![(proportion1, child1)]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Set second child
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -351,6 +357,7 @@ fn test_add_singular_child() {
Err(Error::<Test>::SubNetworkDoesNotExist.into())
);
add_network(netuid, 0, 0);
step_rate_limit(&TransactionType::SetChildren, netuid);
assert_eq!(
SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand All @@ -361,6 +368,7 @@ fn test_add_singular_child() {
Err(Error::<Test>::NonAssociatedColdKey.into())
);
SubtensorModule::create_account_if_non_existent(&coldkey, &hotkey);
step_rate_limit(&TransactionType::SetChildren, netuid);
assert_eq!(
SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand All @@ -371,6 +379,7 @@ fn test_add_singular_child() {
Err(Error::<Test>::InvalidChild.into())
);
let child = U256::from(3);
step_rate_limit(&TransactionType::SetChildren, netuid);
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
hotkey,
Expand Down Expand Up @@ -467,6 +476,8 @@ fn test_do_revoke_child_singular_success() {
let children = SubtensorModule::get_children(&hotkey, netuid);
assert_eq!(children, vec![(proportion, child)]);

step_rate_limit(&TransactionType::SetChildren, netuid);

// Revoke child
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -764,6 +775,8 @@ fn test_do_set_children_multiple_old_children_cleanup() {
vec![(proportion, old_child)]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Set new children
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -854,6 +867,8 @@ fn test_do_set_children_multiple_overwrite_existing() {
vec![(proportion, child1), (proportion, child2)]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Overwrite with new children
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -999,7 +1014,7 @@ fn test_childkey_take_rate_limiting() {
&hotkey,
netuid,
);
let limit = SubtensorModule::get_rate_limit(&TransactionType::SetChildkeyTake);
let limit = SubtensorModule::get_rate_limit(&TransactionType::SetChildkeyTake, 0);
log::info!(
"Rate limit info: current_block: {}, last_block: {}, limit: {}, passes: {}, diff: {}",
current_block,
Expand Down Expand Up @@ -1199,6 +1214,8 @@ fn test_do_revoke_children_multiple_success() {
vec![(proportion1, child1), (proportion2, child2)]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Revoke multiple children
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -1313,6 +1330,8 @@ fn test_do_revoke_children_multiple_partial_revocation() {
]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Revoke only child3
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -1364,6 +1383,8 @@ fn test_do_revoke_children_multiple_non_existent_children() {
vec![(proportion, child1)]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Attempt to revoke existing and non-existent children
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -1450,6 +1471,8 @@ fn test_do_revoke_children_multiple_complex_scenario() {
]
));

step_rate_limit(&TransactionType::SetChildren, netuid);

// Revoke child2
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand All @@ -1466,6 +1489,8 @@ fn test_do_revoke_children_multiple_complex_scenario() {
let parents2 = SubtensorModule::get_parents(&child2, netuid);
assert!(parents2.is_empty());

step_rate_limit(&TransactionType::SetChildren, netuid);

// Revoke remaining children
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
Expand Down Expand Up @@ -2374,6 +2399,7 @@ fn test_dynamic_parent_child_relationships() {
step_block(11);

// Change parent-child relationships
step_rate_limit(&TransactionType::SetChildren, netuid);
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey_parent),
parent,
Expand Down Expand Up @@ -3702,3 +3728,78 @@ fn test_childkey_take_drain_validator_take() {
));
});
}

// 60: Test set_children rate limiting - Fail then succeed
// This test ensures that an immediate second `set_children` transaction fails due to rate limiting:
// - Sets up a network and registers a hotkey
// - Performs a `set_children` transaction
// - Attempts a second `set_children` transaction immediately
// - Verifies that the second transaction fails with `TxRateLimitExceeded`
// Then the rate limit period passes and the second transaction succeeds
// - Steps blocks for the rate limit period
// - Attempts the second transaction again and verifies it succeeds
// SKIP_WASM_BUILD=1 RUST_LOG=info cargo test --test children -- test_set_children_rate_limit_fail_then_succeed --exact --nocapture
#[test]
fn test_set_children_rate_limit_fail_then_succeed() {
new_test_ext(1).execute_with(|| {
let coldkey = U256::from(1);
let hotkey = U256::from(2);
let child = U256::from(3);
let child2 = U256::from(4);
let netuid: u16 = 1;
let tempo = 13;

// Add network and register hotkey
add_network(netuid, tempo, 0);
register_ok_neuron(netuid, hotkey, coldkey, 0);

// First set_children transaction
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
hotkey,
netuid,
vec![(100, child)]
));

// Immediate second transaction should fail due to rate limit
assert_noop!(
SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
hotkey,
netuid,
vec![(100, child2)]
),
Error::<Test>::TxRateLimitExceeded
);

// Verify first children assignment remains
let children = SubtensorModule::get_children(&hotkey, netuid);
assert_eq!(children, vec![(100, child)]);

// Try again after rate limit period has passed
// Check rate limit
let limit = SubtensorModule::get_rate_limit(&TransactionType::SetChildren, netuid);

// Step that many blocks
step_block(limit as u16);

// Verify rate limit passes
assert!(SubtensorModule::passes_rate_limit_on_subnet(
&TransactionType::SetChildren,
&hotkey,
netuid
));

// Try again
assert_ok!(SubtensorModule::do_set_children(
RuntimeOrigin::signed(coldkey),
hotkey,
netuid,
vec![(100, child2)]
));

// Verify children assignment has changed
let children = SubtensorModule::get_children(&hotkey, netuid);
assert_eq!(children, vec![(100, child2)]);
});
}
11 changes: 11 additions & 0 deletions pallets/subtensor/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use frame_support::{
use frame_system as system;
use frame_system::{limits, EnsureNever, EnsureRoot, RawOrigin};
use pallet_collective::MemberCount;
use pallet_subtensor::utils::rate_limiting::TransactionType;
use sp_core::{Get, H256, U256};
use sp_runtime::Perbill;
use sp_runtime::{
Expand Down Expand Up @@ -601,3 +602,13 @@ pub fn is_within_tolerance(actual: u64, expected: u64, tolerance: u64) -> bool {
};
difference <= tolerance
}

// Helper function to wait for the rate limit
#[allow(dead_code)]
pub fn step_rate_limit(transaction_type: &TransactionType, netuid: u16) {
// Check rate limit
let limit = SubtensorModule::get_rate_limit(transaction_type, netuid);

// Step that many blocks
step_block(limit as u16);
}
101,610 changes: 50,805 additions & 50,805 deletions plain_spec_finney.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion plain_spec_testfinney.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,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: 211,
spec_version: 212,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down
Loading