From e6d17caab435ce29a409ce67f3e017ad2b2421c2 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 2 May 2025 16:38:56 +0400 Subject: [PATCH 1/3] Introduce SameSubnetId error for move and swap stake extrinsics. --- pallets/subtensor/src/benchmarks.rs | 27 ++++--- pallets/subtensor/src/macros/errors.rs | 4 +- pallets/subtensor/src/staking/stake_utils.rs | 10 ++- pallets/subtensor/src/tests/move_stake.rs | 74 ++++++++++---------- 4 files changed, 66 insertions(+), 49 deletions(-) diff --git a/pallets/subtensor/src/benchmarks.rs b/pallets/subtensor/src/benchmarks.rs index 2d8db04620..717722c479 100644 --- a/pallets/subtensor/src/benchmarks.rs +++ b/pallets/subtensor/src/benchmarks.rs @@ -1122,44 +1122,49 @@ mod pallet_benchmarks { fn swap_stake() { let coldkey: T::AccountId = whitelisted_caller(); let hot: T::AccountId = account("A", 0, 9); - let netuid: u16 = 1; + let netuid1: u16 = 1; + let netuid2: u16 = 2; - SubtokenEnabled::::insert(netuid, true); - Subtensor::::init_new_network(netuid, 1); + SubtokenEnabled::::insert(netuid1, true); + Subtensor::::init_new_network(netuid1, 1); + SubtokenEnabled::::insert(netuid2, true); + Subtensor::::init_new_network(netuid2, 1); - let reg_fee = Subtensor::::get_burn_as_u64(netuid); + let reg_fee = Subtensor::::get_burn_as_u64(netuid1); let stake_tao: u64 = 1_000_000; let deposit = reg_fee.saturating_mul(2).saturating_add(stake_tao); Subtensor::::add_balance_to_coldkey_account(&coldkey, deposit); assert_ok!(Subtensor::::burned_register( RawOrigin::Signed(coldkey.clone()).into(), - netuid, + netuid1, hot.clone() )); - SubnetTAO::::insert(netuid, deposit); - SubnetAlphaIn::::insert(netuid, deposit); + SubnetTAO::::insert(netuid1, deposit); + SubnetAlphaIn::::insert(netuid1, deposit); + SubnetTAO::::insert(netuid2, deposit); + SubnetAlphaIn::::insert(netuid2, deposit); TotalStake::::set(deposit); assert_ok!(Subtensor::::add_stake_limit( RawOrigin::Signed(coldkey.clone()).into(), hot.clone(), - netuid, + netuid1, stake_tao, u64::MAX, false )); let alpha_to_swap: u64 = - Subtensor::::get_stake_for_hotkey_and_coldkey_on_subnet(&hot, &coldkey, netuid); + Subtensor::::get_stake_for_hotkey_and_coldkey_on_subnet(&hot, &coldkey, netuid1); #[extrinsic_call] _( RawOrigin::Signed(coldkey.clone()), hot.clone(), - netuid, - netuid, + netuid1, + netuid2, alpha_to_swap, ); } diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 8ace006010..a5bf028831 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -210,7 +210,9 @@ mod errors { InvalidRecoveredPublicKey, /// SubToken disabled now SubtokenDisabled, - /// Zero max stake amout + /// Zero max stake amount ZeroMaxStakeAmount, + /// Invalid netuid duplication + SameSubnetId, } } diff --git a/pallets/subtensor/src/staking/stake_utils.rs b/pallets/subtensor/src/staking/stake_utils.rs index a928c53e31..6c5647e032 100644 --- a/pallets/subtensor/src/staking/stake_utils.rs +++ b/pallets/subtensor/src/staking/stake_utils.rs @@ -999,7 +999,7 @@ impl Pallet { /// pub fn validate_stake_transition( origin_coldkey: &T::AccountId, - _destination_coldkey: &T::AccountId, + destination_coldkey: &T::AccountId, origin_hotkey: &T::AccountId, destination_hotkey: &T::AccountId, origin_netuid: u16, @@ -1009,6 +1009,14 @@ impl Pallet { maybe_allow_partial: Option, check_transfer_toggle: bool, ) -> Result<(), Error> { + // Ensure stake transition is actually happening + if origin_coldkey == destination_coldkey && origin_hotkey == destination_hotkey { + ensure!( + origin_netuid != destination_netuid, + Error::::SameSubnetId + ); + } + // Ensure that both subnets exist. ensure!( Self::if_subnet_exist(origin_netuid), diff --git a/pallets/subtensor/src/tests/move_stake.rs b/pallets/subtensor/src/tests/move_stake.rs index 0b7584a4f0..0590fb9a1c 100644 --- a/pallets/subtensor/src/tests/move_stake.rs +++ b/pallets/subtensor/src/tests/move_stake.rs @@ -566,11 +566,11 @@ fn test_do_move_wrong_origin() { }); } -// 14. test_do_move_same_hotkey -// Description: Attempt to move stake to the same hotkey, which should fail or have no effect -// SKIP_WASM_BUILD=1 RUST_LOG=debug cargo test --test move -- test_do_move_same_hotkey --exact --nocapture +// 14. test_do_move_same_hotkey_fails +// Description: Attempt to move stake to the same hotkey, which should fail +// SKIP_WASM_BUILD=1 RUST_LOG=debug cargo test --test move -- test_do_move_same_hotkey_fails --exact --nocapture #[test] -fn test_do_move_same_hotkey() { +fn test_do_move_same_hotkey_fails() { new_test_ext(1).execute_with(|| { let subnet_owner_coldkey = U256::from(1001); let subnet_owner_hotkey = U256::from(1002); @@ -587,20 +587,22 @@ fn test_do_move_same_hotkey() { SubtensorModule::get_stake_for_hotkey_and_coldkey_on_subnet(&hotkey, &coldkey, netuid); // Attempt to move stake to the same hotkey - assert_ok!(SubtensorModule::do_move_stake( - RuntimeOrigin::signed(coldkey), - hotkey, - hotkey, - netuid, - netuid, - alpha, - )); + assert_eq!( + SubtensorModule::do_move_stake( + RuntimeOrigin::signed(coldkey), + hotkey, + hotkey, + netuid, + netuid, + alpha, + ), + Err(Error::::SameSubnetId.into()) + ); // Check that stake remains unchanged - assert_abs_diff_eq!( + assert_eq!( SubtensorModule::get_stake_for_hotkey_and_coldkey_on_subnet(&hotkey, &coldkey, netuid), - alpha - fee, - epsilon = alpha / 1000 + alpha, ); }); } @@ -1151,7 +1153,8 @@ fn test_do_swap_nonexistent_subnet() { new_test_ext(1).execute_with(|| { let coldkey = U256::from(1); let hotkey = U256::from(2); - let nonexistent_netuid: u16 = 9999; + let nonexistent_netuid1: u16 = 9998; + let nonexistent_netuid2: u16 = 9999; let stake_amount = 1_000_000; SubtensorModule::create_account_if_non_existent(&coldkey, &hotkey); @@ -1160,8 +1163,8 @@ fn test_do_swap_nonexistent_subnet() { SubtensorModule::do_swap_stake( RuntimeOrigin::signed(coldkey), hotkey, - nonexistent_netuid, - nonexistent_netuid, + nonexistent_netuid1, + nonexistent_netuid2, stake_amount ), Error::::SubnetNotExists @@ -1257,7 +1260,8 @@ fn test_do_swap_minimum_stake_check() { new_test_ext(1).execute_with(|| { let subnet_owner_coldkey = U256::from(1001); let subnet_owner_hotkey = U256::from(1002); - let netuid = add_dynamic_network(&subnet_owner_hotkey, &subnet_owner_coldkey); + let netuid1 = add_dynamic_network(&subnet_owner_hotkey, &subnet_owner_coldkey); + let netuid2 = add_dynamic_network(&subnet_owner_hotkey, &subnet_owner_coldkey); let coldkey = U256::from(1); let hotkey = U256::from(3); @@ -1265,14 +1269,14 @@ fn test_do_swap_minimum_stake_check() { let swap_amount = 1; SubtensorModule::create_account_if_non_existent(&coldkey, &hotkey); - SubtensorModule::stake_into_subnet(&hotkey, &coldkey, netuid, total_stake, 0); + SubtensorModule::stake_into_subnet(&hotkey, &coldkey, netuid1, total_stake, 0); assert_err!( SubtensorModule::do_swap_stake( RuntimeOrigin::signed(coldkey), hotkey, - netuid, - netuid, + netuid1, + netuid2, swap_amount ), Error::::AmountTooLow @@ -1290,30 +1294,28 @@ fn test_do_swap_same_subnet() { let coldkey = U256::from(1); let hotkey = U256::from(2); let stake_amount = DefaultMinStake::::get() * 10; - let fee = DefaultStakingFee::::get(); SubtensorModule::create_account_if_non_existent(&coldkey, &hotkey); SubtensorModule::stake_into_subnet(&hotkey, &coldkey, netuid, stake_amount, 0); let alpha_before = SubtensorModule::get_stake_for_hotkey_and_coldkey_on_subnet(&hotkey, &coldkey, netuid); - let fee_as_alpha = SubtensorModule::swap_tao_for_alpha(netuid, fee); - assert_ok!(SubtensorModule::do_swap_stake( - RuntimeOrigin::signed(coldkey), - hotkey, - netuid, - netuid, - alpha_before - )); + assert_eq!( + SubtensorModule::do_swap_stake( + RuntimeOrigin::signed(coldkey), + hotkey, + netuid, + netuid, + alpha_before + ), + Err(Error::::SameSubnetId.into()) + ); let alpha_after = SubtensorModule::get_stake_for_hotkey_and_coldkey_on_subnet(&hotkey, &coldkey, netuid); - assert_abs_diff_eq!( - alpha_after, - alpha_before - fee_as_alpha, - epsilon = alpha_after / 10000 - ); + + assert_eq!(alpha_after, alpha_before,); }); } From 632c1c9918fb67c2fc52eeacf2d38860c505ee3c Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Tue, 6 May 2025 14:20:52 +0400 Subject: [PATCH 2/3] Update benchmark weights --- pallets/subtensor/src/macros/dispatches.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index 6fff6e5363..637042c456 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -1719,9 +1719,9 @@ mod dispatches { /// May emit a `StakeSwapped` event on success. #[pallet::call_index(87)] #[pallet::weight(( - Weight::from_parts(190_100_000, 0) - .saturating_add(T::DbWeight::get().reads(13)) - .saturating_add(T::DbWeight::get().writes(9)), + Weight::from_parts(221_600_000, 0) + .saturating_add(T::DbWeight::get().reads(25)) + .saturating_add(T::DbWeight::get().writes(16)), DispatchClass::Operational, Pays::No ))] From 48a87e294480d4b0be2ad178340929aa9da80dcb Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Tue, 6 May 2025 19:25:22 +0400 Subject: [PATCH 3/3] Rename error SameSubnetId -> SameNetuid --- pallets/subtensor/src/macros/errors.rs | 2 +- pallets/subtensor/src/staking/stake_utils.rs | 5 +---- pallets/subtensor/src/tests/move_stake.rs | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 4e6494650a..69a8fe1646 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -213,6 +213,6 @@ mod errors { /// Estimating the maximum stake for limited staking operations returned zero. ZeroMaxStakeAmount, /// Invalid netuid duplication - SameSubnetId, + SameNetuid, } } diff --git a/pallets/subtensor/src/staking/stake_utils.rs b/pallets/subtensor/src/staking/stake_utils.rs index a793a56edc..65280c8a43 100644 --- a/pallets/subtensor/src/staking/stake_utils.rs +++ b/pallets/subtensor/src/staking/stake_utils.rs @@ -1012,10 +1012,7 @@ impl Pallet { ) -> Result<(), Error> { // Ensure stake transition is actually happening if origin_coldkey == destination_coldkey && origin_hotkey == destination_hotkey { - ensure!( - origin_netuid != destination_netuid, - Error::::SameSubnetId - ); + ensure!(origin_netuid != destination_netuid, Error::::SameNetuid); } // Ensure that both subnets exist. diff --git a/pallets/subtensor/src/tests/move_stake.rs b/pallets/subtensor/src/tests/move_stake.rs index 0590fb9a1c..dd85ab9075 100644 --- a/pallets/subtensor/src/tests/move_stake.rs +++ b/pallets/subtensor/src/tests/move_stake.rs @@ -596,7 +596,7 @@ fn test_do_move_same_hotkey_fails() { netuid, alpha, ), - Err(Error::::SameSubnetId.into()) + Err(Error::::SameNetuid.into()) ); // Check that stake remains unchanged @@ -1309,7 +1309,7 @@ fn test_do_swap_same_subnet() { netuid, alpha_before ), - Err(Error::::SameSubnetId.into()) + Err(Error::::SameNetuid.into()) ); let alpha_after =