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
10 changes: 5 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,10 @@ 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 {
let subnet_tempo = Self::get_tempo(netuid);
match tx_type {
TransactionType::SetChildren => (DefaultTempo::<T>::get().saturating_mul(2)).into(), // Cannot set children twice within the default tempo period.
TransactionType::SetChildren => (subnet_tempo.saturating_mul(2)).into(), // Cannot set children twice within the default tempo period.
TransactionType::SetChildkeyTake => TxChildkeyTakeRateLimit::<T>::get(),
TransactionType::Unknown => 0, // Default to no limit for unknown types (no limit)
}
Expand All @@ -50,7 +50,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 +61,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
77 changes: 76 additions & 1 deletion pallets/subtensor/tests/children.rs
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,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 @@ -3702,3 +3702,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)]);
});
}
101,652 changes: 50,826 additions & 50,826 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