Skip to content

Commit

Permalink
Merge pull request #1037 from opentensor/hotfix/set-children-rate-limit
Browse files Browse the repository at this point in the history 
[hotfix] Fix set children rate limit
  • Loading branch information
@unconst
unconst authored Nov 29, 2024
2 parents 44d6859 + 7fdb692 commit 7364b18
Show file tree
Hide file tree
Showing 6 changed files with 50,924 additions and 50,813 deletions.
9 changes: 4 additions & 5 deletions pallets/subtensor/src/utils/rate_limiting.rs
View file
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
View file
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
View file
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
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion plain_spec_testfinney.json
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion runtime/src/lib.rs
View file
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

0 comments on commit 7364b18

Please sign in to comment.